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: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/297>
This commit is contained in:
Mathieu Duponchelle 2020-05-20 17:32:48 +02:00 committed by Tim-Philipp Müller
parent f66959c275
commit fa8ea66418
2 changed files with 5 additions and 10 deletions

View file

@ -2202,7 +2202,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);
@ -2214,8 +2213,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);
}
}
@ -3785,14 +3786,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;

View file

@ -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);