GNOME Bugzilla – Bug 726423
playbin/decodebin: aggregate buffering messages
Last modified: 2014-05-30 16:24:54 UTC
With adaptive streams using multiple demuxers (dash can have a demuxer per stream, smooth streaming too) the multiqueues doing buffering are posting the messages independently and the application will react based on the last one received. So, what can happen is: pipeline is playing and: 1) multiqueue-1 posts 50% buffering 2) pipeline is buffering 3) multiqueue-2 posts 20% buffering 4) multiqueue-1 posts 100% buffering 5) pipeline is now playing (oops) We should aggregate those buffering messages to deliver it to the application or document that the application should do it by itself. Questions: Where? Decodebin? Playbin? GstBin? GstPipeline? It is safer to start with decodebin or playbin, and then we can evaluate if it could be moved to GstBin or GstPipeline (without breaking ABI).
I think this should be handled inside decodebin. It should aggregate all the buffering messages by always taking the current minimum percentage, like what multiqueue is doing already. Inside GstBin/GstPipeline might not be ideal as there might be cases when you want a different behaviour.
Created attachment 272081 [details] [review] decodebin: aggregate buffering messages Aggregate buffering messages to only post the lower value to avoid setting pipeline to playing while any multiqueue is still buffering. Initial implementation, still need to implement some code to clean the list when multiqueues are flushing
Looks good to me, and solve a rather important problem. I would vote for merging this so we get some testing before 1.4 ?
Review of attachment 272081 [details] [review]: Apart from the neeeds-work Thiago mentioned in his commit, what also needs to be done is only considering the buffering messages of the current active group(s). Otherwise you might go to buffering because the next group just started, while it could just buffer in the background while we're finishing the current group.
Created attachment 277292 [details] [review] multiqueue: post buffering message when queues flush The buffering status goes back to 0, so inform the application about it
Created attachment 277293 [details] [review] decodebin: aggregate buffering messages Aggregate buffering messages to only post the lower value to avoid setting pipeline to playing while any multiqueue is still buffering. There are 3 scenarios where the entries should be removed from the list: 1) When decodebin is set to READY 2) When an element posts a 100% buffering (already implemented) 3) When a multiqueue is removed from decodebin. For item 3 we don't need to handle it because this should only happen when either 1 is hapenning or when it is playing a chained file, for which number 2 should have happened for the previous stream to finish
Review of attachment 277293 [details] [review]: Looks good, just minor comment ::: gst/playback/gstdecodebin2.c @@ +4667,3 @@ + smaller = msg; + } + dbin->buffering_status = g_list_prepend (dbin->buffering_status, msg); The refcounting here is a bit convoluted... can you clean that up? Always store a new ref in the list and ... @@ +4676,3 @@ + /* we are posting the original received msg */ + } else { + gst_message_replace (&msg, smaller); Here make sure to unref the original message or forward-post it
Created attachment 277497 [details] [review] decodebin: aggregate buffering messages Aggregate buffering messages to only post the lower value to avoid setting pipeline to playing while any multiqueue is still buffering. There are 3 scenarios where the entries should be removed from the list: 1) When decodebin is set to READY 2) When an element posts a 100% buffering (already implemented) 3) When a multiqueue is removed from decodebin. For item 3 we don't need to handle it because this should only happen when either 1 is hapenning or when it is playing a chained file, for which number 2 should have happened for the previous stream to finish
Pushed, thanks for the review. base: commit 783195ccefa8690cc4a3ccffbfd77c2e86b2c391 Author: Thiago Santos <ts.santos@sisa.samsung.com> Date: Sun Mar 16 14:27:30 2014 -0300 decodebin: aggregate buffering messages Aggregate buffering messages to only post the lower value to avoid setting pipeline to playing while any multiqueue is still buffering. There are 3 scenarios where the entries should be removed from the list: 1) When decodebin is set to READY 2) When an element posts a 100% buffering (already implemented) 3) When a multiqueue is removed from decodebin. For item 3 we don't need to handle it because this should only happen when either 1 is hapenning or when it is playing a chained file, for which number 2 should have happened for the previous stream to finish https://bugzilla.gnome.org/show_bug.cgi?id=726423 core: commit 01c9ae26309cdf4ce0876ae878e5a72e7fcc7f54 Author: Thiago Santos <ts.santos@sisa.samsung.com> Date: Tue May 27 08:09:36 2014 -0300 multiqueue: post buffering message when queues flush The buffering status goes back to 0, so inform the application about it https://bugzilla.gnome.org/show_bug.cgi?id=726423