GNOME Bugzilla – Bug 707636
dashdemux: offline playback not buffering correctly
Last modified: 2014-02-22 23:14:01 UTC
Seems like every dash stream is being handled as 'live' and buffering doesn't work correctly. When network isn't fast enough, playback is compromised and far from smooth. Here is a link for a test dash stream: http://demo.unified-streaming.com/video/machete/machete.ism/machete.mpd?format=mp4&session_id=63129
Created attachment 254252 [details] [review] uridecodebin: pass on the buffering property for adaptive streams This patch attempts to pass on the use-buffering property to the multiqueues after an adaptive demuxer, so that they would emit buffering messages allowing application to pause and wait until enough data is queued. Might still need some more patching to fully work
Doesn't this cause weird interactions with the buffering messages emitted by e.g. hlsdemux? Otherwise looks sensible
Actually this patch already makes it work, you also need to enable buffering flag for playbin. Why isn't it default? It would make sense to enable it forcefully for adaptive streams. I particularly don't like the idea of demuxers emitting buffering messages. It seems that hlsdemux does it only on start up to measure progress of the the initial buffer accumulation before starting to stream. Isn't it better to let queues handle buffering as a demuxer might not know how much data is already available downstream? What is the purpose of these buffering messages in hls?
The purpose there is that playback does not start before 3 fragments are buffered, which is iirc mandated by the HLS spec. And you want to show the application some information about your progress :) But I agree with your argument here, this would better be solved in a queue element after the demuxer.
Created attachment 260120 [details] [review] multiqueue: post 100% buffering if single queue is not linked This makes buffering stop in case a stream switch happens. This is important for adaptive streams that can disable not-linked streams to avoid consuming the network bandwidth. Otherwise we can get a deadlock if the stream is disabled on the adaptive demuxer and no buffers are pushed anymore.
(In reply to comment #4) > The purpose there is that playback does not start before 3 fragments are > buffered, which is iirc mandated by the HLS spec. And you want to show the > application some information about your progress :) > > But I agree with your argument here, this would better be solved in a queue > element after the demuxer. I'm considering pushing this patch leaving hls commented out. We could have DASH and MSS working while we don't have a solution for hls.
Why commented out for HLS?
I thought we wanted to avoid any buffering clashes between hlsdemux and multiqueue messages
Yes, this needs to be consistently implemented in all adaptive streaming demuxers. So the plan now would be to always do the buffering outside the demuxer?
AFAIK the issue it that HLS spec mentions that the client must buffer 3 fragments before starting playback. The other demuxers don't do that and consequently won't post buffering messages themselves.
Well, maybe we should just ignore the HLS spec there and use our sensible defaults for the multiqueue buffering? Apart from that, is there still a way to disable buffering now?
DASH and MSS demuxers won't do any internal buffering now. I removed that in favor of using multiqueues. For enabling buffering you use the playbin flags for it. I agree on use our buffering in favor of hlsdemux. But I'd like an opinion from someone that actually has read the spec. I couldn't find anything about it from a quick search. Additionally hlsdemux doesn't seem to cope well with buffering enabled (on playbin) and deadlocks early on playback. Going to try to understand what's happening.
Comment on attachment 254252 [details] [review] uridecodebin: pass on the buffering property for adaptive streams Also the queue2 between source and decodebin should be disabled for adaptive streams. Otherwise you'll get a 100% buffering message in the very beginning from queue2 :)
Pushed with lots of follow-up fixes and some major refactoring of hlsdemux: commit b47a4faf5f4899ad20fe4c09b3a98a48720528ad Author: Sebastian Dröge <sebastian@centricular.com> Date: Thu Feb 20 15:09:36 2014 +0100 ext: Use Codec/Demuxer/Adaptive for the adaptive streaming demuxers commit a51116add3f7db0924628dde08bf9439dd3cb935 Author: Sebastian Dröge <sebastian@centricular.com> Date: Mon Feb 17 09:19:32 2014 +0100 hlsdemux: Refactor threading and downloading We now download fragments as fast as possible and push them downstream while another thread is just responsible for updating live playlists every now and then. This simplifies the code a lot and together with the new buffering mode for adaptive streams in multiqueue makes streams start much faster. Also simplify threading a bit and hopefully make the GstTask usage safer. commit 76e74547c73eda04399af5068a9ad5c002554933 Author: Sebastian Dröge <sebastian@centricular.com> Date: Wed Feb 19 09:35:45 2014 +0100 hlsdemux: Only switch pads if the caps are changing commit 2df1e56bb7240e158373313d2dfa577c40f10766 Author: Sebastian Dröge <sebastian@centricular.com> Date: Fri Feb 21 09:53:09 2014 +0100 decodebin: If we have a demuxer without dynamic srcpads, just assume no-more-pads Otherwise we will wait until the multiqueue after the demuxer will overrun, which is clearly not needed then. commit f149c27a619a5c77f8da39c1baf45c1a6c2605db Author: Sebastian Dröge <sebastian@centricular.com> Date: Fri Feb 21 09:43:38 2014 +0100 decodebin: Also make sure to not duplicate an element factory after a group If we are using an adaptive stream demuxer, which outputs a non-container stream, we are putting another multiqueue after the *parser* following the adaptive stream demuxer. We do not want to add another instance of the same parser right after this multiqueue. commit 41117606dd84002fc94045f9e6969a5ab12939b7 Author: Sebastian Dröge <sebastian@centricular.com> Date: Thu Feb 20 15:38:48 2014 +0100 decodebin: During pre-rolling always use the auto-preroll limits on multiqueues Even if we're buffering in the multiqueues. commit 2d2aa02b77d68bfb9ae9fca286c23f5b653aa8b1 Author: Sebastian Dröge <sebastian@centricular.com> Date: Thu Feb 20 15:37:54 2014 +0100 decodebin: Pass through the seekability information when setting multiqueue limits commit db771185ed750627a6a1824c42b651d739e1b4a4 Author: Sebastian Dröge <sebastian@centricular.com> Date: Thu Feb 20 15:36:47 2014 +0100 decodebin: During exposing of pads don't set the multiqueue limits multiple times to different values Instead just set them once in the very end to the correct values. commit c4caeb73cef79470334c70ba7cc85d07de860e44 Author: Sebastian Dröge <sebastian@centricular.com> Date: Thu Feb 20 15:07:26 2014 +0100 decodebin: Only enable multiqueue buffering once we're pre-rolled Otherwise we will emit buffering messages not just from the last multiqueue but also from previous multiqueues... confusing the application with different percentages during pre-rolling. commit 4f320109167d4e998cd15f5bd7b81f6260e71c9a Author: Sebastian Dröge <sebastian@centricular.com> Date: Thu Feb 20 15:02:09 2014 +0100 decodebin: Make sure that we always have a second multiqueue for adaptive streaming demuxers For adaptive streaming demuxer we insert a multiqueue after this demuxer. This multiqueue will get one fragment per buffer. Now for the case where we have a container stream inside these buffers, another demuxer will be plugged and after this second demuxer there will be a second multiqueue. This second multiqueue will get smaller buffers and will be the one emitting buffering messages. If we don't have a container stream inside the fragment buffers, we'll insert a multiqueue below right after the next element after the adaptive streaming demuxer. This is going to be a parser or decoder, and will output smaller buffers. commit ad51b38b7cd9c7829431cb39ea9f89add0e7156f Author: Sebastian Dröge <sebastian@centricular.com> Date: Wed Feb 19 10:21:16 2014 +0100 uridecodebin: Always use buffering in multiqueue for adaptive streams commit a2837a22a53071e42f610d994ffd8b2b87a8e609 Author: Sebastian Dröge <sebastian@centricular.com> Date: Wed Feb 19 10:06:13 2014 +0100 uridecodebin: Only add a queue2 for buffering for non-adaptive streaming streams commit 89c9e23bfecead322d2f5a8a0487a1acc90d8a22 Author: Thiago Santos <thiago.sousa.santos@collabora.com> Date: Wed Feb 6 08:46:58 2013 -0300 uridecodebin: pass on the buffering property for adaptive streams Adaptive streams should download its data inside the demuxer, so we want to use multiqueue's buffering messages to control the pipeline flow and avoid losing sync if download rates are low; https://bugzilla.gnome.org/show_bug.cgi?id=707636