From 07fcd4d1b542e3b2b68d164909e63659c0c68537 Mon Sep 17 00:00:00 2001 From: Mathieu Duponchelle Date: Wed, 20 May 2020 17:32:48 +0200 Subject: [PATCH] queue2: don't post unnecessary buffering message, refine locking This is a follow up to review comments in !297 + The posting of the buffering message in READY_TO_PAUSED isn't needed, removing it made the test fail, but the correct fix was simply to link elements together + Move code to relock the queue and set last_posted_buffering_percent and percent_changed inside the buffering_post_lock in create_write(). This makes locking consistent with post_buffering() Part-of: --- plugins/elements/gstqueue2.c | 13 +++---------- tests/check/elements/queue2.c | 2 ++ 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/plugins/elements/gstqueue2.c b/plugins/elements/gstqueue2.c index 9d9fb12c23..057ae1e7e2 100644 --- a/plugins/elements/gstqueue2.c +++ b/plugins/elements/gstqueue2.c @@ -2192,7 +2192,6 @@ gst_queue2_create_write (GstQueue2 * queue, GstBuffer * buffer) g_mutex_lock (&queue->buffering_post_lock); post_ok = gst_element_post_message (GST_ELEMENT_CAST (queue), msg); - g_mutex_unlock (&queue->buffering_post_lock); GST_QUEUE2_MUTEX_LOCK (queue); @@ -2204,8 +2203,10 @@ gst_queue2_create_write (GstQueue2 * queue, GstBuffer * buffer) queue->last_posted_buffering_percent = queue->percent_changed; queue->percent_changed = FALSE; GST_DEBUG_OBJECT (queue, "successfully posted buffering message"); - } else + } else { GST_DEBUG_OBJECT (queue, "could not post buffering message"); + } + g_mutex_unlock (&queue->buffering_post_lock); } } @@ -3780,14 +3781,6 @@ gst_queue2_change_state (GstElement * element, GstStateChange transition) gst_event_replace (&queue->stream_start_event, NULL); GST_QUEUE2_MUTEX_UNLOCK (queue); query_downstream_bitrate (queue); - - /* Post a buffering message now to make sure the application receives - * a buffering message as early as possible. This prevents situations - * where the pipeline's state is set to PLAYING too early, before - * buffering actually finished. */ - update_buffering (queue); - gst_queue2_post_buffering (queue); - break; case GST_STATE_CHANGE_PAUSED_TO_PLAYING: break; diff --git a/tests/check/elements/queue2.c b/tests/check/elements/queue2.c index a9c56827bd..c27e973073 100644 --- a/tests/check/elements/queue2.c +++ b/tests/check/elements/queue2.c @@ -698,11 +698,13 @@ GST_START_TEST (test_ready_paused_buffering_message) fail_unless (fakesink != NULL); gst_bin_add_many (GST_BIN (pipe), fakesrc, queue2, fakesink, NULL); + gst_element_link_many (fakesrc, queue2, fakesink, NULL); /* Set the pipeline to PAUSED. This should cause queue2 to attempt to post * a buffering message during its READY->PAUSED state change. And this should * succeed, since queue2 has been added to pipe by now. */ gst_element_set_state (pipe, GST_STATE_PAUSED); + gst_element_get_state (pipe, NULL, NULL, GST_CLOCK_TIME_NONE); /* Look for the expected 0% buffering message. */ CHECK_FOR_BUFFERING_MSG (pipe, 0);