GNOME Bugzilla – Bug 741104
dashdemux: add support for I frame based trick play
Last modified: 2016-08-18 10:30:11 UTC
Dashdemux has support for "trick play" (playback at rates other than 1) that is implemented by downloading fragments more rapidly than when rate==1. In some situations this approach does not work very well, due to the demands it places upon the bandwidth of the network and the performance of the downstream demux/decode/display elements. For example trick play at rate==32 requires 32x the network bandwidth and the "qtdemux ! h264parse ! avdec-h264" pipeline to cope with 32x the amount of compressed data. A lower bitrate Representation might be able to reduce the demands, but typically there are a limited range of available bitrates. It is unlikely for there to be a suitable Representation for every trickplay rate. It would be beneficial to enable an I-frame only mode of trick play, whereby only the I-frames from segments are downloaded. This can be used to reduce the burden on the network and also reduces the demands on the downstream elements. This feature has been enabled for HLS streams. Perhaps as part of the move to the GstAdaptiveDemux base class, this behaviour could be harmonised in the base class? Obviously the method of finding the I-frames is very different between DASH and HLS, but the rate adaptation logic could be common. Downloading the first I-frame from each segment should be fairly straightforward, as the DASH spec mandates that the first frame in each segment is an IDR. Finding the other I-frames within the segment is more complex, as it would require parsing a box (such as the trun) to find their location.
Created attachment 292295 [details] [review] Make dashdemux fetch initial I frame from each segment Attached is a proof-of-concept of I-frame based trickplay for dashdemux. The patch modifies dashdemux so that only the first 0.5 seconds (approximately) of each segment is fetched when |rate| > 1.0. Note that this patch was made before dashdemux was rebased on top of the new adaptivedemux base class; but, as you'll see from the patch, the changes are very small and self-contained.
Created attachment 292297 [details] [review] Make qtdemux output only first sample from received segments Attached is a proof-of-concept patch for qtdemux that adds support for I-frame based trickplay. The patch modifies qtdemux so that when |rate| > 1.0 it extracts and pushes downstream only the first sample from each fragment that it receives, throwing away any remaining data from the fragment that comes after this first sample. Together with the patch to dashdemux, (patch 292295), this allows I-frame only trickplay for DASH to be demonstrated.
Created attachment 332521 [details] [review] dash: Add helper for parsing box headers
Created attachment 332522 [details] [review] dashdemux: Implement parsing of ISOBMFF boxes
Created attachment 332523 [details] [review] dashdemux: Store box fourccs in the header at a central place
Created attachment 332524 [details] [review] dashdemux: Move code around to keep all sidx related functions together
Created attachment 332525 [details] [review] dashdemux: Implement parsing of moof box
Created attachment 332526 [details] [review] dash: Add unit test for ISOBFF box header parsing
Created attachment 332527 [details] [review] dash: Add test for parsing a moof box
Created attachment 332528 [details] [review] dashdemux: Store parsed moof and extract offsets of sync samples in it
Created attachment 332529 [details] [review] dashdemux: Implement downloading of only sync samples
Created attachment 332530 [details] [review] dashdemux: Only do keyframe-only playback if the corresponding seek flag is given
Created attachment 332531 [details] [review] dashdemux: Only enable key-unit trick mode for video streams
Created attachment 332532 [details] [review] dashdemux: Mark every first buffer of moov, sidx, moof and mdat as DISCONT in keyframe-only mode We need to mark every first buffer of a key unit as discont, and also every first buffer of a moov and moof. This ensures that qtdemux takes note of our buffer offsets for each of those buffers instead of keeping track of them itself from the first buffer. We need offsets to be consistent between moof and mdat
Created attachment 332533 [details] [review] adaptivedemux: Add API for allowing subclasses to download URLs in chunks This allows to gradually download part of a fragment when the final size is not known and only a part of it should be downloaded. For example when only the moof should be parsed and/or a single keyframe should be downloaded.
Created attachment 332534 [details] [review] dashdemux: Use chunked downloading for the moof in KEY_UNITS mode Allows us to reuse the HTTP connection and reduce latencies a lot.
Created attachment 332535 [details] [review] dashdemux: Also allow key-unit only mode if (some) sample flags are given by trex but we can still find sync frames
Created attachment 332536 [details] [review] dashdemux: Remember if for a stream we could do key-units only mode This makes sure we don't even try going into that mode if we previously saw that the stream does not have the suitable metadata.
Created attachment 332537 [details] [review] dashdemux: If a fragment contains no sync samples, disable key-unit mode
Created attachment 332538 [details] [review] dashdemux: Download any sync-sample following the moof directly in key-units only mode We don't have to do yet another additional request but can just download the data directly. Also unify the key-unit only mode buffer pushing and extract it into its own function now that it became more complicated.
Created attachment 332539 [details] [review] dashdemux: Collect average moof and first sync sample sizes And always request those in the beginning so that ideally we get the moof and the first sync sample all together with the first HTTP request.
On top of this, a few optimizations are still pending. Namely deciding to jump ahead a few fragments/keyframes if the rate is high enough and/or we can't keep up due to network/CPU reasons, but apart from that this (together with latest GIT of everything which also contains relevant changes) works on many DASH streams.
Created attachment 332548 [details] [review] adaptivedemux: Add API for allowing subclasses to download URLs in chunks This allows to gradually download part of a fragment when the final size is not known and only a part of it should be downloaded. For example when only the moof should be parsed and/or a single keyframe should be downloaded.
Created attachment 332576 [details] [review] dashdemux: Download any sync-sample following the moof directly in key-units only mode We don't have to do yet another additional request but can just download the data directly. Also unify the key-unit only mode buffer pushing and extract it into its own function now that it became more complicated.
Created attachment 332577 [details] [review] dashdemux: Collect average moof and first sync sample sizes And always request those in the beginning so that ideally we get the moof and the first sync sample all together with the first HTTP request.
Created attachment 332603 [details] [review] dashdemux: Download any sync-sample following the moof directly in key-units only mode We don't have to do yet another additional request but can just download the data directly. Also unify the key-unit only mode buffer pushing and extract it into its own function now that it became more complicated.
Created attachment 332608 [details] [review] dashdemux: Remember if for a stream we could do key-units only mode This makes sure we don't even try going into that mode if we previously saw that the stream does not have the suitable metadata.
It crashes for me soon after selecting a fast-forward speed: [...] 0:00:05.220959076 25082 0x89cbd50 WARN adaptivedemux gstadaptivedemux.c:2194:_src_chain:<dashdemux0> Bitrate for fragment not available 0:00:05.221459957 25082 0x8c15890 WARN qtdemux qtdemux.c:6422:gst_qtdemux_process_adapter:<qtdemux0> Unknown fourcc while parsing header : gd. 0:00:05.221503952 25082 0x8c15890 WARN qtdemux qtdemux.c:6235:gst_qtdemux_process_adapter:<qtdemux0> error: This file is invalid and cannot be played. 0:00:05.221513928 25082 0x8c15890 WARN qtdemux qtdemux.c:6235:gst_qtdemux_process_adapter:<qtdemux0> error: atom ...j has bogus size 3808834240 0:00:05.221755729 25082 0x89cbd50 WARN adaptivedemux gstadaptivedemux.c:2228:_src_chain:<dashdemux0> error: stream stopped, reason error 0:00:05.222964724 25082 0x8c157b0 WARN adaptivedemux gstadaptivedemux.c:3291:gst_adaptive_demux_stream_download_loop:<dashdemux0> Error while downloading fragment ** GStreamer:ERROR:gstpoll.c:1361:gst_poll_wait: assertion failed: ("Value of time " "timeout" " is out of timespec's range" && ((timeout) / GST_SECOND) < G_MAXLONG) Aborted (That was using GST_SEEK_FLAG_FLUSH | GST_SEEK_FLAG_TRICKMODE_KEY_UNITS as seek flags.)
On which stream, and are you using GIT master of everything?
http://test-media.youview.co.uk/ondemand/bbb/trickplay/client_manifest.mpd I applied the patches on top of commit dc6e4ccb of plugins-bad (i.e., from 12:54 yesterday). Everything else was the git master as of Monday morning. But I'll try updating everything to see if it makes a difference.
Created attachment 332710 [details] [review] dashdemux: Implement parsing of ISOBMFF boxes
Created attachment 332711 [details] [review] dashdemux: Store box fourccs in the header at a central place
Created attachment 332712 [details] [review] dashdemux: Move code around to keep all sidx related functions together
Created attachment 332713 [details] [review] dashdemux: Implement parsing of moof box
Created attachment 332714 [details] [review] dash: Add unit test for ISOBFF box header parsing
Created attachment 332715 [details] [review] dash: Add test for parsing a moof box
Created attachment 332716 [details] [review] dashdemux: Store parsed moof and extract offsets of sync samples in it
Created attachment 332717 [details] [review] dashdemux: Implement downloading of only sync samples
Created attachment 332718 [details] [review] dashdemux: Only do keyframe-only playback if the corresponding seek flag is given
Created attachment 332719 [details] [review] dashdemux: Only enable key-unit trick mode for video streams
Created attachment 332720 [details] [review] dashdemux: Mark every first buffer of moov, sidx, moof and mdat as DISCONT in keyframe-only mode We need to mark every first buffer of a key unit as discont, and also every first buffer of a moov and moof. This ensures that qtdemux takes note of our buffer offsets for each of those buffers instead of keeping track of them itself from the first buffer. We need offsets to be consistent between moof and mdat
Created attachment 332721 [details] [review] adaptivedemux: Add API for allowing subclasses to download URLs in chunks This allows to gradually download part of a fragment when the final size is not known and only a part of it should be downloaded. For example when only the moof should be parsed and/or a single keyframe should be downloaded.
Created attachment 332722 [details] [review] dashdemux: Use chunked downloading for the moof in KEY_UNITS mode Allows us to reuse the HTTP connection and reduce latencies a lot.
Created attachment 332723 [details] [review] dashdemux: Also allow key-unit only mode if (some) sample flags are given by trex but we can still find sync frames
Created attachment 332724 [details] [review] dashdemux: Remember if for a stream we could do key-units only mode This makes sure we don't even try going into that mode if we previously saw that the stream does not have the suitable metadata.
Created attachment 332725 [details] [review] dashdemux: If a fragment contains no sync samples, disable key-unit mode
Created attachment 332726 [details] [review] dashdemux: Download any sync-sample following the moof directly in key-units only mode We don't have to do yet another additional request but can just download the data directly. Also unify the key-unit only mode buffer pushing and extract it into its own function now that it became more complicated.
Created attachment 332727 [details] [review] dashdemux: Collect average moof and first sync sample sizes And always request those in the beginning so that ideally we get the moof and the first sync sample all together with the first HTTP request.
Created attachment 332728 [details] [review] dashdemux: When doing chunked downloading on SIDX, clip requests on the SIDX entry boundaries
(In reply to Chris Bass from comment #30) > http://test-media.youview.co.uk/ondemand/bbb/trickplay/client_manifest.mpd > > I applied the patches on top of commit dc6e4ccb of plugins-bad (i.e., from > 12:54 yesterday). Everything else was the git master as of Monday morning. > But I'll try updating everything to see if it makes a difference. It's important that you have the latest of everything at the time I created the patches. Edward and I added various other commits to other elements which were pushed already.
(In reply to Sebastian Dröge (slomo) from comment #50) [...] > > It's important that you have the latest of everything at the time I created > the patches. Edward and I added various other commits to other elements > which were pushed already. It didn't make a difference, unfortunately. And getting the same problem with your latest patches.
Created attachment 332730 [details] [review] dashdemux: When doing chunked downloading on SIDX, clip requests on the SIDX entry boundaries
(In reply to Chris Bass from comment #51) > (In reply to Sebastian Dröge (slomo) from comment #50) > [...] > > > > It's important that you have the latest of everything at the time I created > > the patches. Edward and I added various other commits to other elements > > which were pushed already. > > It didn't make a difference, unfortunately. And getting the same problem > with your latest patches. That's curious. Are you testing with gst-plugins-base/tests/examples/playback ? If not, please use that so we have the same base and then get me a debug log with GST_DEBUG=3,adaptive*:6,dash*:6 The error you get looks like you use qtdemux without the gap support though.
Created attachment 332732 [details] Debug log Debug log from gst-plugins-base/tests/examples/playback.
That's missing this debug output for example: https://cgit.freedesktop.org/gstreamer/gst-plugins-good/tree/gst/isomp4/qtdemux.c#n6036
Created attachment 332734 [details] [review] dashdemux: Download any sync-sample following the moof directly in key-units only mode We don't have to do yet another additional request but can just download the data directly. Also unify the key-unit only mode buffer pushing and extract it into its own function now that it became more complicated.
Created attachment 332736 [details] Debug log Debug log from gst-plugins-base/tests/examples/playback. ...now featuring qtdemux:6, too :)
I think that one is fixed as part for attachment 332734 [details] [review] (which I just attached).
(In reply to Sebastian Dröge (slomo) from comment #58) > I think that one is fixed as part for attachment 332734 [details] [review] [review] > (which I just attached). Yes, that's fixed the crashing - thanks.
For streams using sidx, you also have to revert this revert: https://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=aea2c13fc1672a7260679d706d0a084a34ba7531 Other than what the commit message says, the commit is correct from my current understanding. Just the unit test is wrong: it claims to pass dash with mp4 and sidx to the demuxer, but the actual media is random bytes. As such when we use the fragment info from the sidx (which we should as the manifest might not contain anything at all, always sending us back to 0 on seeks) we would have no info and go to the inaccurate fallback case. IMHO these unit tests should be disabled until they produce valid data, or completely replaced with validate tests on real streams. As they're now they make no sense and only work because of how the demuxer currently works exactly.
Review of attachment 332716 [details] [review]: ::: ext/dash/gstdashdemux.c @@ +2021,3 @@ + + /* Non-non-sync sample aka sync sample */ + if (!GST_ISOFF_SAMPLE_FLAGS_SAMPLE_IS_NON_SYNC_SAMPLE (sample_flags)) { The code is only looking for sync samples, so when it runs it only extracts the IDR frame at the beginning of the segments, and not the other I-frames in the segment. It needs also to add frames whose sample_flags include sample_depends_on == 2. I tried adding the following here: if (!GST_ISOFF_SAMPLE_FLAGS_SAMPLE_IS_NON_SYNC_SAMPLE (sample_flags) || (GST_ISOFF_SAMPLE_FLAGS_SAMPLE_DEPENDS_ON (sample_flags) == 0x2)) { ...and that worked well for fast-forward speeds up to about 10x, beyond which there wasn't enough network throughput to download all the I-frames from each segment. It didn't appear to work so well for fast-reverse - it definitely didn't look like all the I-frames were being displayed in that case. If the intention for this code is to assemble a table of all IDR & I-frames in a segment, then the use of *sync_samples in the names here is a bit misleading - it would probably be better to rename things (e.g., *independent_samples, or something like that).
Attachment 332521 [details] pushed as 37ff8ab - dash: Add helper for parsing box headers Attachment 332710 [details] pushed as 0b0a1a5 - dashdemux: Implement parsing of ISOBMFF boxes Attachment 332711 [details] pushed as c4ad30d - dashdemux: Store box fourccs in the header at a central place Attachment 332712 [details] pushed as 7d4f6ca - dashdemux: Move code around to keep all sidx related functions together Attachment 332713 [details] pushed as fff814b - dashdemux: Implement parsing of moof box Attachment 332714 [details] pushed as e3805e4 - dash: Add unit test for ISOBFF box header parsing Attachment 332715 [details] pushed as 6dbfb11 - dash: Add test for parsing a moof box Attachment 332716 [details] pushed as 5b94313 - dashdemux: Store parsed moof and extract offsets of sync samples in it Attachment 332717 [details] pushed as 70bc183 - dashdemux: Implement downloading of only sync samples Attachment 332718 [details] pushed as 9dd8789 - dashdemux: Only do keyframe-only playback if the corresponding seek flag is given Attachment 332719 [details] pushed as 12c2415 - dashdemux: Only enable key-unit trick mode for video streams Attachment 332720 [details] pushed as 7b4fe1e - dashdemux: Mark every first buffer of moov, sidx, moof and mdat as DISCONT in keyframe-only mode Attachment 332721 [details] pushed as a9e38d3 - adaptivedemux: Add API for allowing subclasses to download URLs in chunks Attachment 332722 [details] pushed as 47ef88f - dashdemux: Use chunked downloading for the moof in KEY_UNITS mode Attachment 332723 [details] pushed as b936c00 - dashdemux: Also allow key-unit only mode if (some) sample flags are given by trex but we can still find sync frames Attachment 332724 [details] pushed as cfad48c - dashdemux: Remember if for a stream we could do key-units only mode Attachment 332725 [details] pushed as f8eddab - dashdemux: If a fragment contains no sync samples, disable key-unit mode Attachment 332727 [details] pushed as 9c04d1e - dashdemux: Collect average moof and first sync sample sizes Attachment 332730 [details] pushed as 18e5e64 - dashdemux: When doing chunked downloading on SIDX, clip requests on the SIDX entry boundaries Attachment 332734 [details] pushed as 7f1f777 - dashdemux: Download any sync-sample following the moof directly in key-units only mode
(In reply to Chris Bass from comment #61) > Review of attachment 332716 [details] [review] [review]: > > ::: ext/dash/gstdashdemux.c > @@ +2021,3 @@ > + > + /* Non-non-sync sample aka sync sample */ > + if (!GST_ISOFF_SAMPLE_FLAGS_SAMPLE_IS_NON_SYNC_SAMPLE > (sample_flags)) { > > The code is only looking for sync samples, so when it runs it only extracts > the IDR frame at the beginning of the segments, and not the other I-frames > in the segment. It needs also to add frames whose sample_flags include > sample_depends_on == 2. > > I tried adding the following here: > > if (!GST_ISOFF_SAMPLE_FLAGS_SAMPLE_IS_NON_SYNC_SAMPLE (sample_flags) > || (GST_ISOFF_SAMPLE_FLAGS_SAMPLE_DEPENDS_ON (sample_flags) > == 0x2)) { > > ...and that worked well for fast-forward speeds up to about 10x, beyond > which there wasn't enough network throughput to download all the I-frames > from each segment. It didn't appear to work so well for fast-reverse - it > definitely didn't look like all the I-frames were being displayed in that > case. That's also solved now: http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/commit/?id=e786f737ae83fb009ffcf599c3e0dce6ecab67a7