GNOME Bugzilla – Bug 758387
hlsdemux: downloads same fragment of live playlist at different bitrates
Last modified: 2018-11-03 13:43:00 UTC
At startup of a live HLS stream, gst_m3u8_client_update will select a starting position 3 fragments from the end of the file. If hlsdemux performs a bitrate switch, gst_hls_demux_update_playlist incorrectly re-enforces the rule about starting 3 fragments from the end, causing the wrong fragment to be selected. For example, consider the case of two bitrates containing 6 fragments: 800 0 1 2 3 4 5 1500 0 1 2 3 4 5 At startup, the 800KBps stream is selected and fragment 3 is selected as the start position. After downloading 3 and 4, hlsdemux switches to the 1500KBps stream. At this point gst_hls_demux_update_playlist forces the position back to three fragments from the end of the 1500KBps playlist. By the time this switch takes place, the live playlist has moved on: 1500 1 2 3 4 5 6 This means that hlsdemux will select fragment 4 when performing the bitrate switch. This results in the following fragments being sent from hlsdemux: 800/3.ts 800/4.ts 1500/4.ts 1500/5.ts There is no need for the code in gst_hls_demux_update_playlist because gst_m3u8_client_update enforces the 3 fragment rule at start-up and gst_m3u8_client_get_seek_range_no_lock enforces the rule for seeking.
Created attachment 315955 [details] [review] adaptivedemux: add "select-bitrate" signal to control ABR In order to be able to create a repeatable unit test that reproduces this bug, I needed to be able to control the adaptive bitrate logic in GstAdaptiveDemux. This patch adds a signal "select-bitrate" that allows an application to take control of the adaptive bitrate logic in GstAdaptiveDemux.
Created attachment 315958 [details] [review] hlsdemux: tests: add test of ABR with live playlists Add a unit test to check how hlsdemux handles changing playlists of live streams. This test should fail, showing that there is an issue with fragment selection in hlsdemux. I will upload a patch that fixes the issue, which then causes the test to pass. This patch requires the GstAdaptiveDemux test engine that is attached to bug 758384 https://bugzilla.gnome.org/show_bug.cgi?id=758384
Created attachment 315960 [details] [review] hlsdemux: don't download same fragment at different bitrates.
This bug causes a similar issue as https://bugzilla.gnome.org/show_bug.cgi?id=752858 but the playlist attached to that bug is using a single bitrate, therefore I think these are different bugs.
Review of attachment 315955 [details] [review]: This is a nice idea but how does the application know what are the available bitrates to make the decision? It would be better to give the application the measured bitrate as well as a list of options to select from, with only the bitrate all it can do is just some window average math or some other heuristic on returning an ideal bitrate to use. Then adaptive demux would pick the greatest not higher than this limit. Given a list the applications can actually force some bitrate or have more information on selecting a bitrate. Additionally, bonus points for having another signal to allow the application to select the initial bitrate, but this can be done as a separate work after we settle on this one. How to provide the available options? Post a message when parsing the mpd/playlist/manifest? Pad properties? (Maybe the new stream selection API already covers providing this list, would be good to check with Edward/bilboed about it) ::: gst-libs/gst/adaptivedemux/gstadaptivedemux.h @@ +426,3 @@ + * + * Signal that apps can connect to if they wish to control the + * adaptive bitrate control in #GstAdaptiveDemux. Missing a Since marker, as well as the parameter description have the same name, looks like one of those should be the return.
(In reply to Thiago Sousa Santos from comment #5) > Review of attachment 315955 [details] [review] [review]: > > This is a nice idea but how does the application know what are the available > bitrates to make the decision? > Yes, that would be an improvement. GstAdaptiveDemux is already posting "adaptive-streaming-statistics" messages. It could post a message every time a manifest is successfully parsed. Currently GstAdaptiveDemux does not have visibility of any of the information about the available representations. Either each demux element would be required to implement sending this message, or a new API would be required to allow GstAdaptiveDemux to query the representations that are available in the demux element. Doing it in each element seems to go against DRY, but has the advantage that each demux could provide additional format-specific information. > It would be better to give the application the measured bitrate as well as a > list of options to select from, with only the bitrate all it can do is just > some window average math or some other heuristic on returning an ideal > bitrate to use. Then adaptive demux would pick the greatest not higher than > this limit. Given a list the applications can actually force some bitrate or > have more information on selecting a bitrate. > I am wondering if it might be better to have the signal handler to be passed an identifier of the current representation, rather than the bitrate of the current representation. That, coupled with a message describing the available representations, should allow for the case you describe, without having to enumerate all representations in every call to the signal handler. > Additionally, bonus points for having another signal to allow the > application to select the initial bitrate, but this can be done as a > separate work after we settle on this one. > Yes, that should be fairly easy to do. Might even be able to use the same signal. > How to provide the available options? Post a message when parsing the > mpd/playlist/manifest? Pad properties? (Maybe the new stream selection API > already covers providing this list, would be good to check with > Edward/bilboed about it) > Is there a BZ ticket for that?
Created attachment 316583 [details] [review] adaptivedemux: add "select-bitrate" signal to control ABR Patch updated to provide some information to a signal handler, with a new message type GST_ADAPTIVE_DEMUX_REPRESENTATION_MESSAGE_NAME that is used by demux elements to post information about the available bitrates.
Created attachment 322381 [details] [review] adaptivedemux: add "select-bitrate" signal to control ABR Updated patch to provide a new message that is used by demux elements to post information about the available bitrates. Also, the signal handler has been updated, so that there is a default handler in gstadaptivedemux.c
Created attachment 322384 [details] [review] hlsdemux: tests: add test of ABR with live playlists Re-based to master and also updated to use the GstSystemClock changes to allow the test to run in non-realtime. This patch requires the patches from: https://bugzilla.gnome.org/show_bug.cgi?id=762147
Created attachment 322385 [details] [review] hlsdemux: don't download same fragment at different bitrates. Re-based to master.
Hi there, I've recently integrated Alex's changes to generate the adaptive-streaming-representations message and the select-bitrate signal into our system, and had to made a couple of improvements in the process. - The adaptive-streaming-representations message now comprises a single message containing all the representations (i.e. bitrates) available in the stream in a GPtrArray, rather than one message - dashdemux now includes the media type of each representation, to allow audio and video to be distinguished - The select-bitrate signal also now includes the MIME type of the media being played out, again to allow audio and video to be distinguished in the case of DASH The two patches attached supersede "adaptivedemux: add "select-bitrate" signal to control ABR"
Created attachment 351207 [details] [review] Patch implementing adaptive-streaming-representations based on current master
Created attachment 351208 [details] [review] Patch implementing select-bitrate based on current master
Created attachment 358938 [details] [review] adaptivedemux: add "select-bitrate" signal to control ABR Thanks Tom for updating the patch. I tried it today and it didn't apply for me, due to changes in gst_dash_demux_setup_all_streams(). It was easy enough to fix, so I'm attaching a new version of the patch.
Created attachment 358940 [details] [review] tests: hlsdemux: check select-bitrate signal handler I've split up the "adaptivedemux: add "select-bitrate" signal to control ABR" patch so that it only contains the changes to add the "select-bitrate" signal handler. This patch adds a test to hls_demux that makes use of the select-bitrate signal.
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/325.