videoaggregator: Do not to release VIDEO_AGGREGATOR_LOCK while setting format info

We should be able to always keep the VIDEO_AGGREGATOR_LOCK while
negotiating caps, this patch introduce that change.

That also implies that we do not need the SETCAPS_LOCK anymore because
now VIDEO_AGGREGATOR_LOCK guarantees that setcaps is not called from
several threads and the gst_aggregator_set_caps method is now
protected.

https://bugzilla.gnome.org/show_bug.cgi?id=735042
This commit is contained in:
Thibault Saunier 2014-09-18 17:14:22 +02:00
parent 1b4547ff93
commit 3637296ee9

View file

@ -245,31 +245,11 @@ gst_videoaggregator_child_proxy_init (gpointer g_iface, gpointer iface_data)
} G_STMT_END } G_STMT_END
#define GST_VIDEO_AGGREGATOR_GET_SETCAPS_LOCK(vagg) (&GST_VIDEO_AGGREGATOR(vagg)->priv->setcaps_lock)
#define GST_VIDEO_AGGREGATOR_SETCAPS_LOCK(vagg) G_STMT_START { \
GST_LOG_OBJECT (vagg, "Taking SETCAPS lock from thread %p", \
g_thread_self()); \
g_mutex_lock(GST_VIDEO_AGGREGATOR_GET_SETCAPS_LOCK(vagg)); \
GST_LOG_OBJECT (vagg, "Took SETCAPS lock from thread %p", \
g_thread_self()); \
} G_STMT_END
#define GST_VIDEO_AGGREGATOR_SETCAPS_UNLOCK(vagg) G_STMT_START { \
GST_LOG_OBJECT (vagg, "Releasing SETCAPS lock from thread %p", \
g_thread_self()); \
g_mutex_unlock(GST_VIDEO_AGGREGATOR_GET_SETCAPS_LOCK(vagg)); \
GST_LOG_OBJECT (vagg, "Took SETCAPS lock from thread %p", \
g_thread_self()); \
} G_STMT_END
struct _GstVideoAggregatorPrivate struct _GstVideoAggregatorPrivate
{ {
/* Lock to prevent the state to change while aggregating */ /* Lock to prevent the state to change while aggregating */
GMutex lock; GMutex lock;
/* Lock to prevent two src setcaps from happening at the same time */
GMutex setcaps_lock;
/* Current downstream segment */ /* Current downstream segment */
GstClockTime ts_offset; GstClockTime ts_offset;
guint64 nframes; guint64 nframes;
@ -360,6 +340,11 @@ _find_best_video_format (GstVideoAggregator * vagg, GstCaps * downstream_caps,
g_hash_table_unref (formats_table); g_hash_table_unref (formats_table);
} }
/* WITH GST_VIDEO_AGGREGATOR_LOCK TAKEN
* NOTE: After calling that method you **have to** call
* gst_videoaggregator_update_src_caps (without releasing
* the GST_VIDEO_AGGREGATOR_LOCK in between)
*/
static gboolean static gboolean
gst_videoaggregator_update_converters (GstVideoAggregator * vagg) gst_videoaggregator_update_converters (GstVideoAggregator * vagg)
{ {
@ -476,6 +461,7 @@ gst_videoaggregator_update_converters (GstVideoAggregator * vagg)
return TRUE; return TRUE;
} }
/* WITH GST_VIDEO_AGGREGATOR_LOCK TAKEN */
static gboolean static gboolean
gst_videoaggregator_src_setcaps (GstVideoAggregator * vagg, GstCaps * caps) gst_videoaggregator_src_setcaps (GstVideoAggregator * vagg, GstCaps * caps)
{ {
@ -492,8 +478,6 @@ gst_videoaggregator_src_setcaps (GstVideoAggregator * vagg, GstCaps * caps)
ret = TRUE; ret = TRUE;
GST_VIDEO_AGGREGATOR_LOCK (vagg);
if (GST_VIDEO_INFO_FPS_N (&vagg->info) != GST_VIDEO_INFO_FPS_N (&info) || if (GST_VIDEO_INFO_FPS_N (&vagg->info) != GST_VIDEO_INFO_FPS_N (&info) ||
GST_VIDEO_INFO_FPS_D (&vagg->info) != GST_VIDEO_INFO_FPS_D (&info)) { GST_VIDEO_INFO_FPS_D (&vagg->info) != GST_VIDEO_INFO_FPS_D (&info)) {
if (agg->segment.position != -1) { if (agg->segment.position != -1) {
@ -505,18 +489,21 @@ gst_videoaggregator_src_setcaps (GstVideoAggregator * vagg, GstCaps * caps)
vagg->info = info; vagg->info = info;
GST_VIDEO_AGGREGATOR_UNLOCK (vagg);
if (vagg->priv->current_caps == NULL || if (vagg->priv->current_caps == NULL ||
gst_caps_is_equal (caps, vagg->priv->current_caps) == FALSE) { gst_caps_is_equal (caps, vagg->priv->current_caps) == FALSE) {
gst_caps_replace (&vagg->priv->current_caps, caps); gst_caps_replace (&vagg->priv->current_caps, caps);
GST_VIDEO_AGGREGATOR_UNLOCK (vagg);
gst_aggregator_set_src_caps (agg, caps); gst_aggregator_set_src_caps (agg, caps);
GST_VIDEO_AGGREGATOR_LOCK (vagg);
} }
done: done:
return ret; return ret;
} }
/* WITH GST_VIDEO_AGGREGATOR_LOCK TAKEN */
static gboolean static gboolean
gst_videoaggregator_update_src_caps (GstVideoAggregator * vagg) gst_videoaggregator_update_src_caps (GstVideoAggregator * vagg)
{ {
@ -529,8 +516,6 @@ gst_videoaggregator_update_src_caps (GstVideoAggregator * vagg)
GstVideoAggregatorClass *vagg_klass = (GstVideoAggregatorClass *) klass; GstVideoAggregatorClass *vagg_klass = (GstVideoAggregatorClass *) klass;
GstAggregator *agg = GST_AGGREGATOR (vagg); GstAggregator *agg = GST_AGGREGATOR (vagg);
GST_VIDEO_AGGREGATOR_SETCAPS_LOCK (vagg);
GST_VIDEO_AGGREGATOR_LOCK (vagg);
GST_OBJECT_LOCK (vagg); GST_OBJECT_LOCK (vagg);
for (l = GST_ELEMENT (vagg)->sinkpads; l; l = l->next) { for (l = GST_ELEMENT (vagg)->sinkpads; l; l = l->next) {
GstVideoAggregatorPad *mpad = l->data; GstVideoAggregatorPad *mpad = l->data;
@ -596,7 +581,6 @@ gst_videoaggregator_update_src_caps (GstVideoAggregator * vagg)
if (vagg_klass->update_info) { if (vagg_klass->update_info) {
if (!vagg_klass->update_info (vagg, &info)) { if (!vagg_klass->update_info (vagg, &info)) {
ret = FALSE; ret = FALSE;
GST_VIDEO_AGGREGATOR_UNLOCK (vagg);
goto done; goto done;
} }
} }
@ -619,7 +603,6 @@ gst_videoaggregator_update_src_caps (GstVideoAggregator * vagg)
if (gst_caps_is_empty (caps)) { if (gst_caps_is_empty (caps)) {
GST_DEBUG_OBJECT (vagg, "empty caps"); GST_DEBUG_OBJECT (vagg, "empty caps");
ret = FALSE; ret = FALSE;
GST_VIDEO_AGGREGATOR_UNLOCK (vagg);
goto done; goto done;
} }
@ -638,21 +621,15 @@ gst_videoaggregator_update_src_caps (GstVideoAggregator * vagg)
gst_caps_unref (caps); gst_caps_unref (caps);
caps = gst_video_info_to_caps (&info); caps = gst_video_info_to_caps (&info);
GST_VIDEO_AGGREGATOR_UNLOCK (vagg);
if (gst_videoaggregator_src_setcaps (vagg, caps)) { if (gst_videoaggregator_src_setcaps (vagg, caps)) {
if (vagg_klass->negotiated_caps) if (vagg_klass->negotiated_caps)
ret = ret =
GST_VIDEO_AGGREGATOR_GET_CLASS (vagg)->negotiated_caps (vagg, caps); GST_VIDEO_AGGREGATOR_GET_CLASS (vagg)->negotiated_caps (vagg, caps);
} }
gst_caps_unref (caps); gst_caps_unref (caps);
} else {
GST_VIDEO_AGGREGATOR_UNLOCK (vagg);
} }
done: done:
GST_VIDEO_AGGREGATOR_SETCAPS_UNLOCK (vagg);
return ret; return ret;
} }
@ -692,11 +669,12 @@ gst_videoaggregator_pad_sink_setcaps (GstPad * pad, GstObject * parent,
vaggpad->info = info; vaggpad->info = info;
ret = gst_videoaggregator_update_converters (vagg); ret = gst_videoaggregator_update_converters (vagg);
GST_VIDEO_AGGREGATOR_UNLOCK (vagg);
if (ret) if (ret)
ret = gst_videoaggregator_update_src_caps (vagg); ret = gst_videoaggregator_update_src_caps (vagg);
GST_VIDEO_AGGREGATOR_UNLOCK (vagg);
beach: beach:
return ret; return ret;
} }
@ -1192,11 +1170,11 @@ gst_videoaggregator_aggregate (GstAggregator * agg)
return GST_FLOW_NOT_NEGOTIATED; return GST_FLOW_NOT_NEGOTIATED;
} }
GST_VIDEO_AGGREGATOR_LOCK (vagg);
if (gst_pad_check_reconfigure (agg->srcpad)) if (gst_pad_check_reconfigure (agg->srcpad))
gst_videoaggregator_update_src_caps (vagg); gst_videoaggregator_update_src_caps (vagg);
GST_VIDEO_AGGREGATOR_LOCK (vagg);
if (agg->segment.position == -1) if (agg->segment.position == -1)
output_start_time = agg->segment.start; output_start_time = agg->segment.start;
else else
@ -1718,7 +1696,6 @@ gst_videoaggregator_release_pad (GstElement * element, GstPad * pad)
gst_videoaggregator_update_converters (vagg); gst_videoaggregator_update_converters (vagg);
gst_buffer_replace (&vaggpad->buffer, NULL); gst_buffer_replace (&vaggpad->buffer, NULL);
update_caps = GST_VIDEO_INFO_FORMAT (&vagg->info) != GST_VIDEO_FORMAT_UNKNOWN; update_caps = GST_VIDEO_INFO_FORMAT (&vagg->info) != GST_VIDEO_FORMAT_UNKNOWN;
GST_VIDEO_AGGREGATOR_UNLOCK (vagg);
gst_child_proxy_child_removed (GST_CHILD_PROXY (vagg), G_OBJECT (vaggpad), gst_child_proxy_child_removed (GST_CHILD_PROXY (vagg), G_OBJECT (vaggpad),
GST_OBJECT_NAME (vaggpad)); GST_OBJECT_NAME (vaggpad));
@ -1729,6 +1706,7 @@ gst_videoaggregator_release_pad (GstElement * element, GstPad * pad)
if (update_caps) if (update_caps)
gst_videoaggregator_update_src_caps (vagg); gst_videoaggregator_update_src_caps (vagg);
GST_VIDEO_AGGREGATOR_UNLOCK (vagg);
return; return;
} }
@ -1850,7 +1828,6 @@ gst_videoaggregator_finalize (GObject * o)
GstVideoAggregator *vagg = GST_VIDEO_AGGREGATOR (o); GstVideoAggregator *vagg = GST_VIDEO_AGGREGATOR (o);
g_mutex_clear (&vagg->priv->lock); g_mutex_clear (&vagg->priv->lock);
g_mutex_clear (&vagg->priv->setcaps_lock);
G_OBJECT_CLASS (gst_videoaggregator_parent_class)->finalize (o); G_OBJECT_CLASS (gst_videoaggregator_parent_class)->finalize (o);
} }
@ -1946,7 +1923,6 @@ gst_videoaggregator_init (GstVideoAggregator * vagg)
vagg->priv->current_caps = NULL; vagg->priv->current_caps = NULL;
g_mutex_init (&vagg->priv->lock); g_mutex_init (&vagg->priv->lock);
g_mutex_init (&vagg->priv->setcaps_lock);
/* initialize variables */ /* initialize variables */
gst_videoaggregator_reset (vagg); gst_videoaggregator_reset (vagg);
} }