From b9a422d8d0c2c68e86db2ebc7389dd8e7fcbbd07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20Cr=C3=AAte?= Date: Wed, 1 Apr 2015 21:45:01 -0400 Subject: [PATCH] aggregator: Flushing is always in pad lock, no need to atomics The usage of atomics was always doubtful as it was used to release a GCond https://bugzilla.gnome.org/show_bug.cgi?id=747220 --- gst-libs/gst/base/gstaggregator.c | 60 +++++++++++-------------------- 1 file changed, 21 insertions(+), 39 deletions(-) diff --git a/gst-libs/gst/base/gstaggregator.c b/gst-libs/gst/base/gstaggregator.c index 77c940bc7b..eb1a11096c 100644 --- a/gst-libs/gst/base/gstaggregator.c +++ b/gst-libs/gst/base/gstaggregator.c @@ -111,16 +111,16 @@ GST_DEBUG_CATEGORY_STATIC (aggregator_debug); #define PAD_WAIT_EVENT(pad) G_STMT_START { \ - GST_LOG_OBJECT (pad, "Waiting for EVENT on thread %p", \ + GST_LOG_OBJECT (pad, "Waiting for buffer to be consumed thread %p", \ g_thread_self()); \ g_cond_wait(&(((GstAggregatorPad* )pad)->priv->event_cond), \ (&((GstAggregatorPad*)pad)->priv->lock)); \ - GST_LOG_OBJECT (pad, "DONE Waiting for EVENT on thread %p", \ + GST_LOG_OBJECT (pad, "DONE Waiting for buffer to be consumed on thread %p", \ g_thread_self()); \ } G_STMT_END #define PAD_BROADCAST_EVENT(pad) G_STMT_START { \ - GST_LOG_OBJECT (pad, "Signaling EVENT from thread %p", \ + GST_LOG_OBJECT (pad, "Signaling buffer consumed from thread %p", \ g_thread_self()); \ g_cond_broadcast(&(((GstAggregatorPad* )pad)->priv->event_cond)); \ } G_STMT_END @@ -176,10 +176,8 @@ GST_DEBUG_CATEGORY_STATIC (aggregator_debug); struct _GstAggregatorPadPrivate { - /* To always be used atomically */ - gboolean flushing; - /* Following fields are protected by the PAD_LOCK */ + gboolean flushing; gboolean pending_flush_start; gboolean pending_flush_stop; gboolean pending_eos; @@ -795,7 +793,7 @@ static void gst_aggregator_pad_set_flushing (GstAggregatorPad * aggpad) { PAD_LOCK (aggpad); - g_atomic_int_set (&aggpad->priv->flushing, TRUE); + aggpad->priv->flushing = TRUE; gst_buffer_replace (&aggpad->priv->buffer, NULL); PAD_BROADCAST_EVENT (aggpad); PAD_UNLOCK (aggpad); @@ -1801,23 +1799,18 @@ gst_aggregator_pad_chain (GstPad * pad, GstObject * object, GstBuffer * buffer) PAD_FLUSH_LOCK (aggpad); - if (g_atomic_int_get (&aggpad->priv->flushing) == TRUE) - goto flushing; - PAD_LOCK (aggpad); if (aggpad->priv->pending_eos == TRUE) goto eos; - while (aggpad->priv->buffer - && g_atomic_int_get (&aggpad->priv->flushing) == FALSE) { - GST_DEBUG_OBJECT (aggpad, "Waiting for buffer to be consumed"); + while (aggpad->priv->buffer && !aggpad->priv->flushing) PAD_WAIT_EVENT (aggpad); - } - PAD_UNLOCK (aggpad); - if (g_atomic_int_get (&aggpad->priv->flushing) == TRUE) + if (aggpad->priv->flushing) goto flushing; + PAD_UNLOCK (aggpad); + if (aggclass->clip) { aggclass->clip (self, aggpad, buffer, &actual_buf); } @@ -1842,6 +1835,7 @@ gst_aggregator_pad_chain (GstPad * pad, GstObject * object, GstBuffer * buffer) return flow_return; flushing: + PAD_UNLOCK (aggpad); PAD_FLUSH_UNLOCK (aggpad); gst_buffer_unref (buffer); @@ -1869,26 +1863,20 @@ gst_aggregator_pad_query_func (GstPad * pad, GstObject * parent, if (GST_QUERY_IS_SERIALIZED (query)) { PAD_LOCK (aggpad); - if (g_atomic_int_get (&aggpad->priv->flushing) == TRUE) { - PAD_UNLOCK (aggpad); - goto flushing; - } - - while (aggpad->priv->buffer - && g_atomic_int_get (&aggpad->priv->flushing) == FALSE) { - GST_DEBUG_OBJECT (aggpad, "Waiting for buffer to be consumed"); + while (aggpad->priv->buffer && !aggpad->priv->flushing) PAD_WAIT_EVENT (aggpad); - } - PAD_UNLOCK (aggpad); - if (g_atomic_int_get (&aggpad->priv->flushing) == TRUE) + if (aggpad->priv->flushing) goto flushing; + + PAD_UNLOCK (aggpad); } return klass->sink_query (GST_AGGREGATOR (parent), GST_AGGREGATOR_PAD (pad), query); flushing: + PAD_UNLOCK (aggpad); GST_DEBUG_OBJECT (aggpad, "Pad is flushing, dropping query"); return FALSE; } @@ -1904,28 +1892,22 @@ gst_aggregator_pad_event_func (GstPad * pad, GstObject * parent, && GST_EVENT_TYPE (event) != GST_EVENT_SEGMENT_DONE) { PAD_LOCK (aggpad); - if (g_atomic_int_get (&aggpad->priv->flushing) == TRUE - && GST_EVENT_TYPE (event) != GST_EVENT_FLUSH_STOP) { - PAD_UNLOCK (aggpad); - goto flushing; - } - while (aggpad->priv->buffer - && g_atomic_int_get (&aggpad->priv->flushing) == FALSE) { - GST_DEBUG_OBJECT (aggpad, "Waiting for buffer to be consumed"); + while (aggpad->priv->buffer && !aggpad->priv->flushing) PAD_WAIT_EVENT (aggpad); - } - PAD_UNLOCK (aggpad); - if (g_atomic_int_get (&aggpad->priv->flushing) == TRUE + if (aggpad->priv->flushing && GST_EVENT_TYPE (event) != GST_EVENT_FLUSH_STOP) goto flushing; + + PAD_UNLOCK (aggpad); } return klass->sink_event (GST_AGGREGATOR (parent), GST_AGGREGATOR_PAD (pad), event); flushing: + PAD_UNLOCK (aggpad); GST_DEBUG_OBJECT (aggpad, "Pad is flushing, dropping event"); if (GST_EVENT_IS_STICKY (event)) gst_pad_store_sticky_event (pad, event); @@ -1943,7 +1925,7 @@ gst_aggregator_pad_activate_mode_func (GstPad * pad, gst_aggregator_pad_set_flushing (aggpad); } else { PAD_LOCK (aggpad); - g_atomic_int_set (&aggpad->priv->flushing, FALSE); + aggpad->priv->flushing = FALSE; PAD_BROADCAST_EVENT (aggpad); PAD_UNLOCK (aggpad); }