From 3fbcf5fcf3b609f201b4670745ebc4f999caac2e Mon Sep 17 00:00:00 2001 From: Carlos Rafael Giani Date: Wed, 29 Mar 2023 15:34:36 +0200 Subject: [PATCH] qtdemux: Only set appsink sync property and check for async state changes By keeping async to TRUE, a deadlock is avoided where the appsink is filled with data after a flushing seek but before its PAUSED->PLAYING state change finishes. If that happens, the appsink is stuck, because its internal condition variable waits for the appsink to have more room for data. The basesink's preroll lock is held during this, and it also tries to acquire that lock during the state change -> deadlock. By keeping async to TRUE, this flood of data does not happen. Also, setting the max-buffers property to 1 is unnecessary - the test runner will anyway detect excess memory usage if it happens. Other property adjustments turned out to just be redundant. Part-of: --- .../tests/check/elements/qtdemux.c | 49 +++++++++---------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/subprojects/gst-plugins-good/tests/check/elements/qtdemux.c b/subprojects/gst-plugins-good/tests/check/elements/qtdemux.c index c98a0d655e..ff4305ab7a 100644 --- a/subprojects/gst-plugins-good/tests/check/elements/qtdemux.c +++ b/subprojects/gst-plugins-good/tests/check/elements/qtdemux.c @@ -1461,11 +1461,26 @@ finish: gst_object_unref (GST_OBJECT (appsink_pad)); } +static void +switch_state_with_async_wait (GstElement * pipeline, GstState state) +{ + GstStateChangeReturn state_ret; + + state_ret = gst_element_set_state (pipeline, GST_STATE_PLAYING); + fail_unless (state_ret != GST_STATE_CHANGE_FAILURE); + + if (state_ret == GST_STATE_CHANGE_ASYNC) { + GST_LOG ("waiting for pipeline to reach %s state", + gst_element_state_get_name (state)); + state_ret = gst_element_get_state (pipeline, NULL, NULL, -1); + fail_unless_equals_int (state_ret, GST_STATE_CHANGE_SUCCESS); + } +} + static void perform_gapless_test (GaplessTestInfo * info) { GstElement *source, *demux, *appsink, *pipeline; - GstStateChangeReturn state_ret; guint frame_num; pipeline = gst_pipeline_new (NULL); @@ -1488,19 +1503,9 @@ perform_gapless_test (GaplessTestInfo * info) g_free (full_filename); } - g_object_set (G_OBJECT (appsink), "async", FALSE, "sync", FALSE, - "max-buffers", 1, "enable-last-sample", FALSE, "processing-deadline", - G_MAXUINT64, NULL); + g_object_set (G_OBJECT (appsink), "sync", FALSE, NULL); - state_ret = gst_element_set_state (pipeline, GST_STATE_PLAYING); - - fail_unless (state_ret != GST_STATE_CHANGE_FAILURE); - - if (state_ret == GST_STATE_CHANGE_ASYNC) { - GST_LOG ("waiting for pipeline to reach PAUSED state"); - state_ret = gst_element_get_state (pipeline, NULL, NULL, -1); - fail_unless_equals_int (state_ret, GST_STATE_CHANGE_SUCCESS); - } + switch_state_with_async_wait (pipeline, GST_STATE_PLAYING); /* Verify all frames from the test signal. */ for (frame_num = 0; frame_num < info->num_aac_frames; ++frame_num) @@ -1535,11 +1540,9 @@ perform_gapless_test (GaplessTestInfo * info) * with PTS 0, and all of those buffers except the last will have a * duration of 0. */ { - fail_unless_equals_int (gst_element_set_state (pipeline, GST_STATE_PAUSED), - GST_STATE_CHANGE_SUCCESS); + switch_state_with_async_wait (pipeline, GST_STATE_PAUSED); gst_element_seek_simple (pipeline, GST_FORMAT_TIME, GST_SEEK_FLAG_FLUSH, 0); - fail_unless_equals_int (gst_element_set_state (pipeline, GST_STATE_PLAYING), - GST_STATE_CHANGE_SUCCESS); + switch_state_with_async_wait (pipeline, GST_STATE_PLAYING); check_parsed_aac_frame (info, 0); } @@ -1554,12 +1557,10 @@ perform_gapless_test (GaplessTestInfo * info) gst_util_uint64_scale_int (info->num_samples_in_first_valid_frame, GST_SECOND, info->sample_rate); - fail_unless_equals_int (gst_element_set_state (pipeline, GST_STATE_PAUSED), - GST_STATE_CHANGE_SUCCESS); + switch_state_with_async_wait (pipeline, GST_STATE_PAUSED); gst_element_seek_simple (pipeline, GST_FORMAT_TIME, GST_SEEK_FLAG_FLUSH, position); - fail_unless_equals_int (gst_element_set_state (pipeline, GST_STATE_PLAYING), - GST_STATE_CHANGE_SUCCESS); + switch_state_with_async_wait (pipeline, GST_STATE_PLAYING); check_parsed_aac_frame (info, info->first_frame_with_valid_samples + 1); } @@ -1574,12 +1575,10 @@ perform_gapless_test (GaplessTestInfo * info) info->num_samples_without_padding - info->num_samples_per_frame, GST_SECOND, info->sample_rate); - fail_unless_equals_int (gst_element_set_state (pipeline, GST_STATE_PAUSED), - GST_STATE_CHANGE_SUCCESS); + switch_state_with_async_wait (pipeline, GST_STATE_PAUSED); gst_element_seek_simple (pipeline, GST_FORMAT_TIME, GST_SEEK_FLAG_FLUSH, position); - fail_unless_equals_int (gst_element_set_state (pipeline, GST_STATE_PLAYING), - GST_STATE_CHANGE_SUCCESS); + switch_state_with_async_wait (pipeline, GST_STATE_PLAYING); check_parsed_aac_frame (info, info->last_frame_with_valid_samples); }