GNOME Bugzilla – Bug 711849
smoothstreaming: Better handling of multi audio tracks
Last modified: 2013-12-20 04:26:04 UTC
At the moment, gstmssdemux exposes one source pad per audio track (one per <StreamIndex> in the manifest file). Each of this track is downloaded even if most of the time only once will actually be used. One way to fix this could be to only download the stream if the pad is actually linked. But this is not going to help when using input-selector as all pads would be linked anyway in that case.
input-selector will return not-linked for the unselected pads, and send a reconfigure event once it becomes selected. That could be used. But better would be an interface that playbin could use to proxy the current-audio (etc) properties to demuxers. This is also needed for HLS and DASH and DVDs. I'm sure there was a bug about that somewhere but can't find it currently :)
sample stream: http://streams.smooth.vertigo.com/elephantsdream/Elephants_Dream_1024-h264-st-aac.ism/manifest
Created attachment 260122 [details] [review] mssdemux: avoid downloading not-linked streams When a stream gets a not-linked return, it will be marked as so and won't download any more new fragments until a reconfigure event is received. This will make mssdemux expose all pads, but only download fragments for the streams that are actually being used. Relying on the pads being linked/unlinked isn't enough in this scenario as there might be an input-selector downstream that is actually discarding buffers for a given linked pad.
Review of attachment 260122 [details] [review]: ::: ext/smoothstreaming/gstmssdemux.c @@ +1400,3 @@ + gst_pad_push (stream->pad, GST_BUFFER_CAST (object)); + GST_DEBUG_OBJECT (mssdemux, "Pushed on pad %s:%s result: %d (%s)", stream, + GST_DEBUG_PAD_NAME (stream->pad), ret, gst_flow_get_name (ret)); gstmssdemux.c:1401:5: error: format '%s' expects argument of type 'char *', but argument 8 has type 'struct GstMssDemuxStream *' [-Werror=format=] GST_DEBUG_OBJECT (mssdemux, "Pushed on pad %s:%s result: %d (%s)", stream, ^ gstmssdemux.c:1401:5: error: format '%d' expects argument of type 'int', but argument 10 has type 'const char *' [-Werror=format=] gstmssdemux.c:1401:5: error: format '%s' expects argument of type 'char *', but argument 11 has type 'int' [-Werror=format=] gstmssdemux.c:1401:5: error: too many arguments for format [-Werror=format-extra-args]
Created attachment 260124 [details] [review] mssdemux: avoid downloading not-linked streams When a stream gets a not-linked return, it will be marked as so and won't download any more new fragments until a reconfigure event is received. This will make mssdemux expose all pads, but only download fragments for the streams that are actually being used. Relying on the pads being linked/unlinked isn't enough in this scenario as there might be an input-selector downstream that is actually discarding buffers for a given linked pad.
This still downloads one of each at startup, right? I'm sure we can do something better. Maybe do a non-serialized downstream query and only start streaming if that returns TRUE?
(In reply to comment #6) > This still downloads one of each at startup, right? I'm sure we can do > something better. Maybe do a non-serialized downstream query and only start > streaming if that returns TRUE? Yes. I want to do that on a future improvement. For now I just want them to switch. I have made it work with a few patches to mssdemux and core/base elements. The main issue now is that after a switch the new stream starting might push buffers a little bit behind the downstream position and because of input-selector and discont flag handling, basesink/audiosink will actually play those buffers and the stream's audio will go back and forth for a few seconds. I'd assume some element in playbin would drop those buffers because they are 'late' (behind current playback position) and only play from the correct position to make sure streams are aligned after switching. Any ideas on how to handle this?
(In reply to comment #5) > Created an attachment (id=260124) [details] [review] > mssdemux: avoid downloading not-linked streams > > When a stream gets a not-linked return, it will be marked as so and > won't download any more new fragments until a reconfigure event > is received. This will make mssdemux expose all pads, but only download > fragments for the streams that are actually being used. Didn't read the patch but make sure to start again if you receive a RECONFIGURE event on a stream. input-selector sends that when it switches pads. Also input-selector usually (unless configured otherwise, playbin does that for external subtitles) returns not-linked for unselected pads.
Created attachment 263165 [details] [review] mssdemux: avoid downloading not-linked streams When a stream gets a not-linked return, it will be marked as so and won't download any more new fragments until a reconfigure event is received. This will make mssdemux expose all pads, but only download fragments for the streams that are actually being used. Relying on the pads being linked/unlinked isn't enough in this scenario as there might be an input-selector downstream that is actually discarding buffers for a given linked pad.
Created attachment 263167 [details] [review] mssdemux: prevent deadlock by pushing buffers from different threads When streams are switching, the old active stream can be blocked because input-selector will block not-linked streams. In case the mssdemux's stream loop is blocked pushing a buffer to a full queue downstream it will never unblock as the queue will not drain (input-selector is blocking). In this scenario, stream switching will deadlock as input-selector is waiting for the newly active stream data and the stream_loop that would push this data is blocked waiting for input-selector. To solve this issue, whenever an stream is reactivated on a reconfigure it will enter into the 'catch up mode', in this mode it can push buffers from its download thread until a certain timestamp. This works because this timestamp will always be behind or equal to the maximum timestamp pushed for all streams, after pushing data for this timestamp, the stream will go back to default and be pushed sequentially from the main streaming thread. By this time, the input-selector should have already released the thread.
Created attachment 263168 [details] [review] mssdemux: properly combine flows Use a proper flow combination function to maintain correct return to make sure a not-linked isn't returned while a stream switch is happening
Created attachment 263169 [details] [review] mssdemux: fix deadlock on stream switching Make sure the dataqueue is set to flushing to unblock a possible _peek on the queue from a stream that is being put to sleep.
This is partially fixed, the mssdemux part is done. Fixes are still needed on baseaudiosink and pipeline buffering. commit 67f3190301bfdd6673cbfd4ceb20d9791187df17 Author: Thiago Santos <ts.santos@sisa.samsung.com> Date: Mon Dec 16 16:14:24 2013 -0300 mssdemux: remove the stream loop task Download and push from the same task, makes code a lot simpler to maintain. Also pushing from separate threads avoids deadlocking when gst_pad_push blocks due to downstream queues being full commit 4e6e1315da7b12a31ae535d889652101611d9f6b Author: Thiago Santos <ts.santos@sisa.samsung.com> Date: Fri Dec 13 17:31:11 2013 -0300 mssdemux: Improve logging Show the stream's pad on log messages to make easier to debug issues in the multiple threads commit 056420940e62bf41ce0451cc4b8417acba500454 Author: Thiago Santos <ts.santos@sisa.samsung.com> Date: Tue Dec 10 18:08:40 2013 -0300 mssdemux: improve flow return handling Handle different flow returns both from the streaming and the downloading loops commit e847bea1e1e2d7d68ee2830861201f5957ede7ee Author: Thiago Santos <ts.santos@sisa.samsung.com> Date: Tue Dec 10 15:41:00 2013 -0300 mssdemux: remove stream locks Simplify the locking by using a single lock instead of having one lock per stream. This still works and is simpler to maintain. commit 2ce4f6a8e4c8aa33d08d9f5fe28a83779fadf03f Author: Thiago Santos <ts.santos@sisa.samsung.com> Date: Tue Nov 12 09:58:31 2013 -0300 mssdemux: avoid downloading not-linked streams When a stream gets a not-linked return, it will be marked as so and won't download any more new fragments until a reconfigure event is received. This will make mssdemux expose all pads, but only download fragments for the streams that are actually being used. Relying on the pads being linked/unlinked isn't enough in this scenario as there might be an input-selector downstream that is actually discarding buffers for a given linked pad. When streams are switching, the old active stream can be blocked because input-selector will block not-linked streams. In case the mssdemux's stream loop is blocked pushing a buffer to a full queue downstream it will never unblock as the queue will not drain (input-selector is blocking). In this scenario, stream switching will deadlock as input-selector is waiting for the newly active stream data and the stream_loop that would push this data is blocked waiting for input-selector. To solve this issue, whenever an stream is reactivated on a reconfigure it will enter into the 'catch up mode', in this mode it can push buffers from its download thread until it reaches the currrent GstSegment's position. This works because this timestamp will always be behind or equal to the maximum timestamp pushed for all streams, after pushing data for this timestamp, the stream will go back to default and be pushed sequentially from the main streaming thread. By this time, the input-selector should have already released the thread. https://bugzilla.gnome.org/show_bug.cgi?id=711849
This is now working. Marking as fixed.