From ad8cb458baf875c4bf77c8831fc7aeb926c41d82 Mon Sep 17 00:00:00 2001 From: Nirbheek Chauhan Date: Tue, 19 May 2015 16:08:08 +0530 Subject: [PATCH] audioaggregator: Sync pad values before aggregating We need to sync the pad values before taking the aggregator and pad locks otherwise the element will just deadlock if there's any property changes scheduled using GstController since that involves taking the aggregator and pad locks. Also add a test for this. https://bugzilla.gnome.org/show_bug.cgi?id=749574 --- gst/audiomixer/gstaudioaggregator.c | 37 ++++++++++---- tests/check/Makefile.am | 4 +- tests/check/elements/audiomixer.c | 76 +++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 12 deletions(-) diff --git a/gst/audiomixer/gstaudioaggregator.c b/gst/audiomixer/gstaudioaggregator.c index 01704f5330..38e8709678 100644 --- a/gst/audiomixer/gstaudioaggregator.c +++ b/gst/audiomixer/gstaudioaggregator.c @@ -739,7 +739,6 @@ gst_audio_aggregator_fill_buffer (GstAudioAggregator * aagg, GstClockTime start_time, end_time; gboolean discont = FALSE; guint64 start_offset, end_offset; - GstClockTime timestamp, stream_time = GST_CLOCK_TIME_NONE; gint rate, bpf; GstAggregatorPad *aggpad = GST_AGGREGATOR_PAD (pad); @@ -762,15 +761,6 @@ gst_audio_aggregator_fill_buffer (GstAudioAggregator * aagg, goto done; } - timestamp = GST_BUFFER_PTS (inbuf); - stream_time = gst_segment_to_stream_time (&aggpad->segment, GST_FORMAT_TIME, - timestamp); - - /* sync object properties on stream time */ - /* TODO: Ideally we would want to do that on every sample */ - if (GST_CLOCK_TIME_IS_VALID (stream_time)) - gst_object_sync_values (GST_OBJECT (pad), stream_time); - start_time = GST_BUFFER_PTS (inbuf); end_time = start_time + gst_util_uint64_scale_ceil (pad->priv->size, GST_SECOND, @@ -964,6 +954,29 @@ gst_audio_aggregator_create_output_buffer (GstAudioAggregator * aagg, return outbuf; } +static gboolean +sync_pad_values (GstAudioAggregator * aagg, GstAudioAggregatorPad * pad) +{ + GstAggregatorPad *bpad = GST_AGGREGATOR_PAD (pad); + GstClockTime timestamp, stream_time; + + if (pad->priv->buffer == NULL) + return TRUE; + + timestamp = GST_BUFFER_PTS (pad->priv->buffer); + GST_OBJECT_LOCK (bpad); + stream_time = gst_segment_to_stream_time (&bpad->segment, GST_FORMAT_TIME, + timestamp); + GST_OBJECT_UNLOCK (bpad); + + /* sync object properties on stream time */ + /* TODO: Ideally we would want to do that on every sample */ + if (GST_CLOCK_TIME_IS_VALID (stream_time)) + gst_object_sync_values (GST_OBJECT (pad), stream_time); + + return TRUE; +} + static GstFlowReturn gst_audio_aggregator_aggregate (GstAggregator * agg, gboolean timeout) { @@ -1011,6 +1024,10 @@ gst_audio_aggregator_aggregate (GstAggregator * agg, gboolean timeout) element = GST_ELEMENT (agg); aagg = GST_AUDIO_AGGREGATOR (agg); + /* Sync pad properties to the stream time */ + gst_aggregator_iterate_sinkpads (agg, + (GstAggregatorPadForeachFunc) sync_pad_values, NULL); + GST_AUDIO_AGGREGATOR_LOCK (aagg); GST_OBJECT_LOCK (agg); diff --git a/tests/check/Makefile.am b/tests/check/Makefile.am index 233302ce15..7aef48ea06 100644 --- a/tests/check/Makefile.am +++ b/tests/check/Makefile.am @@ -296,8 +296,8 @@ AM_CFLAGS = $(GST_CFLAGS) $(GST_CHECK_CFLAGS) $(GST_OPTION_CFLAGS) \ -UG_DISABLE_ASSERT -UG_DISABLE_CAST_CHECKS LDADD = $(GST_CHECK_LIBS) -elements_audiomixer_LDADD = $(GST_BASE_LIBS) -lgstbase-@GST_API_VERSION@ $(LDADD) -elements_audiomixer_CFLAGS = $(GST_PLUGINS_BASE_CFLAGS) $(GST_BASE_CFLAGS) $(AM_CFLAGS) +elements_audiomixer_LDADD = $(GST_BASE_LIBS) $(GST_CONTROLLER_LIBS) -lgstbase-@GST_API_VERSION@ $(LDADD) +elements_audiomixer_CFLAGS = $(GST_PLUGINS_BASE_CFLAGS) $(GST_BASE_CFLAGS) $(GST_CONTROLLER_CFLAGS) $(AM_CFLAGS) elements_audiointerleave_LDADD = $(GST_BASE_LIBS) -lgstbase-@GST_API_VERSION@ -lgstaudio-@GST_API_VERSION@ $(LDADD) elements_audiointerleave_CFLAGS = $(GST_PLUGINS_BASE_CFLAGS) $(GST_BASE_CFLAGS) $(AM_CFLAGS) diff --git a/tests/check/elements/audiomixer.c b/tests/check/elements/audiomixer.c index ddfdbe188e..8fe329d82f 100644 --- a/tests/check/elements/audiomixer.c +++ b/tests/check/elements/audiomixer.c @@ -35,6 +35,8 @@ #include #include #include +#include +#include static GMainLoop *main_loop; @@ -1835,6 +1837,79 @@ GST_START_TEST (test_segment_base_handling) GST_END_TEST; +static void +set_pad_volume_fade (GstPad * pad, GstClockTime start, gdouble start_value, + GstClockTime end, gdouble end_value) +{ + GstControlSource *cs; + GstTimedValueControlSource *tvcs; + + cs = gst_interpolation_control_source_new (); + fail_unless (gst_object_add_control_binding (GST_OBJECT_CAST (pad), + gst_direct_control_binding_new_absolute (GST_OBJECT_CAST (pad), + "volume", cs))); + + /* set volume interpolation mode */ + g_object_set (cs, "mode", GST_INTERPOLATION_MODE_LINEAR, NULL); + + tvcs = (GstTimedValueControlSource *) cs; + fail_unless (gst_timed_value_control_source_set (tvcs, start, start_value)); + fail_unless (gst_timed_value_control_source_set (tvcs, end, end_value)); + gst_object_unref (cs); +} + +GST_START_TEST (test_sinkpad_property_controller) +{ + GstBus *bus; + GstMessage *msg; + GstElement *pipeline, *sink, *mix, *src1; + GstPad *srcpad, *sinkpad; + GError *error = NULL; + gchar *debug; + + pipeline = gst_pipeline_new ("pipeline"); + mix = gst_element_factory_make ("audiomixer", "audiomixer"); + sink = gst_element_factory_make ("fakesink", "sink"); + src1 = gst_element_factory_make ("audiotestsrc", "src1"); + g_object_set (src1, "num-buffers", 100, NULL); + gst_bin_add_many (GST_BIN (pipeline), src1, mix, sink, NULL); + fail_unless (gst_element_link (mix, sink)); + + srcpad = gst_element_get_static_pad (src1, "src"); + sinkpad = gst_element_get_request_pad (mix, "sink_0"); + fail_unless (gst_pad_link (srcpad, sinkpad) == GST_PAD_LINK_OK); + set_pad_volume_fade (sinkpad, 0, 0, 1.0, 2.0); + gst_object_unref (sinkpad); + gst_object_unref (srcpad); + + gst_element_set_state (pipeline, GST_STATE_PLAYING); + + bus = gst_pipeline_get_bus (GST_PIPELINE (pipeline)); + msg = gst_bus_timed_pop_filtered (bus, GST_CLOCK_TIME_NONE, + GST_MESSAGE_EOS | GST_MESSAGE_ERROR); + switch (GST_MESSAGE_TYPE (msg)) { + case GST_MESSAGE_ERROR: + gst_message_parse_error (msg, &error, &debug); + g_printerr ("ERROR from element %s: %s\n", + GST_OBJECT_NAME (msg->src), error->message); + g_printerr ("Debug info: %s\n", debug); + g_error_free (error); + g_free (debug); + break; + case GST_MESSAGE_EOS: + break; + default: + g_assert_not_reached (); + } + gst_message_unref (msg); + g_object_unref (bus); + + gst_element_set_state (pipeline, GST_STATE_NULL); + gst_object_unref (pipeline); +} + +GST_END_TEST; + static Suite * audiomixer_suite (void) { @@ -1859,6 +1934,7 @@ audiomixer_suite (void) tcase_add_test (tc_chain, test_sync_discont); tcase_add_test (tc_chain, test_sync_unaligned); tcase_add_test (tc_chain, test_segment_base_handling); + tcase_add_test (tc_chain, test_sinkpad_property_controller); /* Use a longer timeout */ #ifdef HAVE_VALGRIND