GNOME Bugzilla – Bug 775132
adaptivedemux: Download fragment only for activated stream
Last modified: 2018-11-03 14:00:30 UTC
MPEGDASH supports multi-video/audio tracks by representing them as AdaptationSets (I'm not sure whether the latest version of HLS or MSS are supporting it or not). And downloading all fragments for the all tracks might be waste of bandwidth. As an optional way, downloading fragments of selected track could be optimal for some usecase.
Hello Sebastian Dröge Thanks for your suggestion about this topic on devel mailing list. I'm still thinking about how to implement this feature... So, I'd like to hear other developers opinions. Currently my design is, Firstly, I'll expose all streams whatever activated or not, and keep pushing fragments toward downstream until pipeline's state goes to PLAYING. This is to ensuring combiner configuration (inputselector), or in order to post complete stream-collection toward surrounding bin. Then, stopping download task of deactivated streams with pushing gap event in PLAYING state. Meanwhile, because Track switching is handled by playbin/decodebin, track switching is some complicated and seamless switching is even more, I think. Whatever how to implement this, replacing GstAdpativeStream to GstStreams based object seems to be useful. So, can I try this GstStreams one first?
Created attachment 341108 [details] [review] adaptivedemux: Make use of GstStream API on AdaptiveDemuxStream
Current "not-linked" flow return based enable/disable downloading does not seem to be the worst, but it causes playing stuck when switching "Disable" to "enable audio". I'm keep trying to find optimal way anyway.
Review of attachment 341108 [details] [review]: Generally looks good - just the one comment below. One thing we might want to do (but can do later) is sub-class GstStream to our own type, and then we can keep a pointer to the adaptivedemux stream object directly. That will be useful when receiving back the GstStream in a select-streams event from the app ::: gst-libs/gst/adaptivedemux/gstadaptivedemux.c @@ +1172,3 @@ + + stream->object = gst_stream_new (stream_id, NULL, + GST_STREAM_TYPE_CONTAINER, GST_STREAM_FLAG_NONE); I'm not sure about using GST_STREAM_TYPE_CONTAINER here - no all sub-classes of adaptivedemux output containers. HLS can output raw AAC, for example. It should probably start as GST_STREAM_TYPE_UNKNOWN, and we modify it once we know the caps (might need extra adaptivedemux API for that, or an extra argument on the set_caps() function)
Created attachment 341144 [details] [review] adaptivedemux: Make use of GstStream API on AdaptiveDemuxStream
(In reply to Jan Schmidt from comment #4) > Review of attachment 341108 [details] [review] [review]: > Thank you for your review. I agreed with your opinion. I did not know that HLS could have a RAW format :) In other patch, I'd like to add a function in dashdemux which is for setting stream-type.
Created attachment 341736 [details] [review] adaptivedemux: Add new APIs for setting GstStream{Flags,Types} Introduce new APIs _stream_set_stream_{flags,type}
Created attachment 341737 [details] [review] dashdemux: Set StreamType and StreamFlags Set StreamType and StreamFlags. Parent bin such as urisourcebin can utilize those information, for example enable/disable buffering based on StreamType and StreamFlags.
Note that since 1.10 you can know whether the parent element (i.e. urisourcebin) can handle streams (dis-)appearing at any point in time with the GST_BIN_FLAGS_STREAMS_AWARE flag : https://gstreamer.freedesktop.org/data/doc/gstreamer/head/gstreamer/html/GstBin.html#GstBinFlags In READY=>PAUSED just check if your parent bin has that flag set (like tsdemux here : https://cgit.freedesktop.org/gstreamer/gst-plugins-bad/tree/gst/mpegtsdemux/mpegtsbase.c#n213 ) if it's set, you don't need to expose/download all streams
(In reply to Edward Hervey from comment #9) Thanks for your suggestion. It seems to be very helpful :). With GST_BIN_FLAGS_STREAMS_AWARE, I think exposing only activated stream seems not to be difficult. But, there is another issue on switching streams if we expose only activated ones. Based on current playbin3 implementation, stream-collection and select-streams event are handled by decodebin3. So, if we expose only activated tracks, track switching seems not to be easy now.
Created attachment 341796 [details] [review] adaptivedemux: Make use of GstStream API on AdaptiveDemuxStream Patch updated. Set stream on stream-start event using gst_event_set_stream()
Hello all Originally, I was planning to expose only activated streams, but discussion for it seems to be delayed I think. More urgent issue is multi-track with multi-period (in dash) or bitrate switch (in hls) issue with playbin3. Obviously, exposed pads by hlsdemux can have identical caps. Then, with bitrate switch, new pads are exposed. At this moment, how can we map previously used streams with new streams? For example, "bitrate A" has 2 streams, one is for video and the other is audio. Then, with bitrate switching to "bitrate B", a video stream in "bitrate B" should be linked to the video stream of "bitrate A" To allow above things, we should use of GstStream API both in adaptivedemux and urisourcebin.
Created attachment 343306 [details] [review] dashdemux: Set StreamType and StreamFlags Set StreamType and StreamFlags, for downstream elements to know what current stream is.
Created attachment 343307 [details] [review] hlsdemux: Set StreamType and StreamFlags
Created attachment 343308 [details] [review] urisourcebin: Link pending pad to slot by using GstStream API slot's caps comparison with pending pad's caps cannot guarantee that they are fully compatible ones. For example, in case of hls streaming, caps for both video/audio streams can be "video/mpegts". So, newly exposed pads by adaptivedemux can be inappropriately linked with an existing slot. (e.g., link video stream's pad to audio stream's slot) So, urisourcebin should refer to Caps/StreamType/taglist for linking them. Until finding unique pending pad for target slot, following rule will be applied * Firstly, extract pending pad by comparing mime-type * Among the group of pads which have identical mime-type, compare StreamType * Also, compare taglist (currently, comparing only language) If urisourcebin failed to find unique pending pad, target pad will be selected randomly
Created attachment 343336 [details] [review] urisourcebin: Link pending pad to slot by using GstStream API Fix invalid string free
Created attachment 343337 [details] [review] hlsdemux: Set StreamType and StreamFlags
Created attachment 343338 [details] [review] urisourcebin: Link pending pad to slot by using GstStream API
Review of attachment 343338 [details] [review]: Makes sense. Can you check it applies cleanly to master ? And it should maybe be moved to a separate bug report (and mark it as blocking this bug report) ::: gst/playback/gsturisourcebin.c @@ +1298,3 @@ case GST_EVENT_STREAM_START: + { + GstStream *stream = gst_pad_get_stream (pad); Use gst_event_parse_stream() instead. The GstStream should be in the STREAM_START event.
Other patches look good to me to. Can you rebase them against current master ?
Created attachment 355493 [details] [review] [1/5] adaptivedemux: Make use of GstStream API on adaptivedemux ... and subclass can set GstStream{Flags,Types} by using newly introduced _stream_set_stream_{flags,type} methods
Created attachment 355494 [details] [review] [2/5] dashdemux: Set StreamType and StreamFlags
Created attachment 355495 [details] [review] [3/5] hlsdemux: Set StreamType and StreamFlags
Created attachment 355496 [details] [review] [4/5] mssdemux: Set StreamType on stream
Created attachment 355497 [details] [review] [5/5] urisourcebin: Link pending pad to slot by using GstStream API slot's caps comparison with pending pad's caps cannot guarantee that they are fully compatible ones. For example, in case of hls streaming, caps for both video/audio streams can be "video/mpegts". So, newly exposed pads by adaptivedemux can be inappropriately linked with an existing slot. (e.g., link video stream's pad to audio stream's slot) When urisourcebin is trying to link a slot with pending pads, * Check only mime-type first, and if there are multiple candidates, * Sort candidates by using GstStreamType, Language tag, and GstStreamFlags Finally select the first element from sorted list.
*** Bug 758689 has been marked as a duplicate of this bug. ***
Sorry about the delay. Any chance you could refactor them against current base ? For some reason they don't apply cleanly. Thanks !
(In reply to Edward Hervey from comment #27) > Sorry about the delay. Any chance you could refactor them against current > base ? For some reason they don't apply cleanly. Thanks ! I'll update them soon :)
Created attachment 371886 [details] [review] adaptivedemux: Make use of GstStream API on adaptivedemux
Created attachment 371888 [details] [review] dashdemux: Set StreamType and StreamFlags
Created attachment 371889 [details] [review] hlsdemux: Set StreamType and StreamFlags
Created attachment 371890 [details] [review] mssdemux: Set StreamType on stream
(In reply to Edward Hervey from comment #19) > Review of attachment 343338 [details] [review] [review]: > > Makes sense. Can you check it applies cleanly to master ? And it should > maybe be moved to a separate bug report (and mark it as blocking this bug > report) I opened new Bug #796000 for it
Review of attachment 371886 [details] [review]: ::: gst-libs/gst/adaptivedemux/gstadaptivedemux.c @@ +947,3 @@ } + event = gst_event_new_stream_start (gst_stream_get_stream_id (stream_obj)); + gst_event_set_stream (event, stream_obj); Do we need to check streams-aware in here?? (to decide whether do _set_stream() or not)
Review of attachment 371886 [details] [review]: ::: gst-libs/gst/adaptivedemux/gstadaptivedemux.c @@ +947,3 @@ } + event = gst_event_new_stream_start (gst_stream_get_stream_id (stream_obj)); + gst_event_set_stream (event, stream_obj); I think it's fine to send extra information (such as stream objects in stream event). Elements and applications that aren't streams-aware can avoid that. What's *not* possible is to change the expected behaviour if the parent is not streams-aware. Just keep it for simplicity.
Unless I'm missing something, these patches only partially solve the issue at hand, correct ? I don't see any code making usage of the fact that it doesn't expose pads that aren't selected.
(In reply to Edward Hervey from comment #36) > Unless I'm missing something, these patches only partially solve the issue > at hand, correct ? I don't see any code making usage of the fact that it > doesn't expose pads that aren't selected. Correct. It's not yet updated. My Idea for next step is that, Step 1. Posting stream collection message by adpativedemux self and the message must be handled by application (or playbin3). --> uridecodebin3/playbin3 should be able to handle and distinguish the message's origin whether it's from decodebin3 or adaptivedemux. Step 2. Similar to decodebin3's default stream selection behavior (it selects only one stream per type depending on sorted stream order), adaptivedemux will choose default stream. https://cgit.freedesktop.org/gstreamer/gst-plugins-base/tree/gst/playback/gstdecodebin3.c#n1074 Step 3. uridecodebin3 should be able to forward select-streams event to adaptivedemux, and adaptivedemux must be able to change stream based on the event. The issue is that, the decision of which select-streams event should forwarded to adaptivedemux is unclear now. Since Step 3 might be out of scope in this bug, I'll do up to Step 2, if you agree.
Regarding upstream handling of stream-selection, there are other problems that need to be handled. I have a WIP design doc for solving most of those issues which I hope to put somewhere online within the coming week. Note that this doesn't change your existing patches afaics.
Created attachment 372017 [details] [review] adaptivedemux: Make use of GstStream API on adaptivedemux
Created attachment 372019 [details] [review] dashdemux: Set StreamType and StreamFlags Remove GST_STREAM_TYPE_CONTAINER from GstStreamType flags. It seems not mandatory feature, and which make stream decision difficult.
Created attachment 372020 [details] [review] hlsdemux: Set StreamType and StreamFlags
Created attachment 372022 [details] [review] mssdemux: Set StreamType on stream
Created attachment 372023 [details] [review] adaptivedemux: Post stream-collection message stream-collection message posted by adaptivedemux will be used for following use case * Notifying meta information of adaptive streaming protocol level * Allowing selection of specific variant by parent or application
Created attachment 372024 [details] [review] adaptivedemux: Post streams-selected message streams-selected and stream-collection messages are paired interface
Created attachment 372025 [details] [review] adaptivedemux: Increase preroll stream count on _prepare_streams() ... instead of on _stream_new(). Some streams might not be activated depending on select-streams events. So we need to wait preroll of only activated streams
Created attachment 372027 [details] [review] adaptivedemux: Download fragment only for activated Parent bin or application could select only required streams by using select-streams event Note that, this feature is enabled only for streams-aware use case
Created attachment 372028 [details] [review] urisourcebin: Set streams-aware flag
Created attachment 372029 [details] [review] playbin3: handle stream-collection from source This patch is to avoid misunderstanding of stream-collection message which is posted by adaptivedemux. Currently, playbin3 expects stream-collection message must be posted by decodebin3 and the message is used for playsink configure.
Created attachment 372031 [details] [review] [DO NOT MERGE] gst-play: Send select-streams event to adaptivedemux just for testing
(In reply to Edward Hervey from comment #38) > Regarding upstream handling of stream-selection, there are other problems > that need to be handled. I have a WIP design doc for solving most of those > issues which I hope to put somewhere online within the coming week. Note > that this doesn't change your existing patches afaics. Since how the WIP design is unknown, I just guess select-streams event could be forwarded to adaptivedemux for now. I'm still working on "exposing only activated stream" feature and it's WIP. Current status is that, initial "exposing only activated stream" is done, however run-time track change should be implemented. I expect the feature might be made in this weekend (or early next week)
(In reply to Edward Hervey from comment #38) > Regarding upstream handling of stream-selection, there are other problems > that need to be handled. I have a WIP design doc for solving most of those > issues which I hope to put somewhere online within the coming week. Note > that this doesn't change your existing patches afaics. Was the WIP design document opened somewhere? I tried some ways with existing streams API, but since some unclear select-stream concept of current playbin3, adding *activation of only selected ones* feature is tricky :-( So implementing *selecting stream per stream type* (I mentioned above) seems to meaningless for now, since we must be able to select other stream. Otherwise, it's regression.
(In reply to Seungha Yang from comment #51) > (In reply to Edward Hervey from comment #38) > > Regarding upstream handling of stream-selection, there are other problems > > that need to be handled. I have a WIP design doc for solving most of those > > issues which I hope to put somewhere online within the coming week. Note > > that this doesn't change your existing patches afaics. > > Was the WIP design document opened somewhere? I tried some ways with > existing streams API, but since some unclear select-stream concept of > current playbin3, adding *activation of only selected ones* feature is > tricky :-( It's on my todo list, I hope to put it somewhere within the coming weeks. It introduces a new (backwards-compatible) API.
(In reply to Edward Hervey from comment #52) > It's on my todo list, I hope to put it somewhere within the coming weeks. > It introduces a new (backwards-compatible) API. Thans for this information. The most tricky part for me is that, there is no explicit way to inform decodebin3 of upstream element's stream change. When adpativedemux got select-stream event (whatever the way of forwarding upstream select-stream event to adaptivedemux), decodebin3 requires the selected stream's configuration (most likely this might be the point of new API will work) in order to switch decodebin3's active slot as soon as possible. To do this without new API, I tried EOS based approach (sending EOS to old stream by adaptivedemux) but it's not a solution since it's sticky event and there might be some queued buffers between adaptivedemux and decodebin3 (i.e., queue2 in urisourcebin, multiqueue in decodebin3)
(In reply to Seungha Yang from comment #53) > Thans for this information. > The most tricky part for me is that, there is no explicit way to inform > decodebin3 of upstream element's stream change. Exactly. And my new API introduces a new (out-of-band) STREAMS_SELECTED event. Which is roughly the same idea as GST_MESSAGE_STREAMS_SELECTED except in the data-stream. > > When adpativedemux got select-stream event (whatever the way of forwarding > upstream select-stream event to adaptivedemux), decodebin3 requires the > selected stream's configuration (most likely this might be the point of new > API will work) in order to switch decodebin3's active slot as soon as > possible. To do this without new API, I tried EOS based approach (sending > EOS to old stream by adaptivedemux) but it's not a solution since it's > sticky event and there might be some queued buffers between adaptivedemux > and decodebin3 (i.e., queue2 in urisourcebin, multiqueue in decodebin3) And that's why we need this new event and why it's out-of-band :) I should be able to post my initial API/design next week. Will keep you updated.
(In reply to Edward Hervey from comment #54) > And that's why we need this new event and why it's out-of-band :) > > I should be able to post my initial API/design next week. Will keep you > updated. That's great! Thanks :)
Took longer than I expected :) Anyway, I've put the WIP design doc here : https://gitlab.freedesktop.org/bilboed/gst-docs/blob/master/markdown/design/scalable-codecs.md It's aiming for the *bigger* picture (supporting scalable codecs), but also contains design for: * offloading stream-selection to any element (and not just decodebin3 itself) * Properly notifying of stream selections * Being able to inform upstream elements ASAP of incompatible/unusable streams * Exposing "variants" of streams (like in adaptive demux).
(In reply to Edward Hervey from comment #56) > Took longer than I expected :) Anyway, I've put the WIP design doc here : > https://gitlab.freedesktop.org/bilboed/gst-docs/blob/master/markdown/design/ > scalable-codecs.md > > It's aiming for the *bigger* picture (supporting scalable codecs), but also > contains design for: > * offloading stream-selection to any element (and not just decodebin3 itself) > * Properly notifying of stream selections > * Being able to inform upstream elements ASAP of incompatible/unusable > streams > * Exposing "variants" of streams (like in adaptive demux). Very interesting doc :) Thanks. Because adpativedemux could expose the structure of manifest to user with the new APIs, I just have a suggestion about some addition (might not be closely related to *scalable* though). Depending on Spec, content provider could suggest associated track group per view point. For example, DASH: The role of ViewPoint element in MPD is to associating each video/audio/text track per view point (e.g., https://github.com/Dash-Industry-Forum/Conformance-Software/blob/master/webfe/mpdvalidator/schematron/examples/ex57.mpd) HLS: combination of variant stream with GROUP-ID of rendition streams results to a association group. ISOBMFF: "alternate_group" in thkd box takes the role. ... Is there a way to signalling the association of each track in Gstreamer? If not, (since "group-id" in Gstreamer is representing temporal association of each track if my understanding is correct), new field for assassination of each track seems to be required such as gboolean gst_event_set_association_id (GstEvent *event, guint association_id, const *gchar nickname (nullable)); gboolean gst_event_parse_association_id (GstEvent *event, guint *association_id, const **gchar nickname); And similar APIs for GstStream. Anyway, I'll take more look at the shared document.
(In reply to Seungha Yang from comment #57) > Very interesting doc :) Thanks. Because adpativedemux could expose the > structure of manifest to user with the new APIs, Note that exposing the structure of the manifest is more for the other elements/pipelines/apps than for the end-user. An end-user shouldn't have to care about which codec/bitrate/... is available. > I just have a suggestion > about some addition (might not be closely related to *scalable* though). > Depending on Spec, content provider could suggest associated track group per > view point. For example, > > DASH: The role of ViewPoint element in MPD is to associating each > video/audio/text track per view point (e.g., > https://github.com/Dash-Industry-Forum/Conformance-Software/blob/master/ > webfe/mpdvalidator/schematron/examples/ex57.mpd) > > HLS: combination of variant stream with GROUP-ID of rendition streams > results to a association group. > > ISOBMFF: "alternate_group" in thkd box takes the role. > ... > > Is there a way to signalling the association of each track in Gstreamer? If > not, (since "group-id" in Gstreamer is representing temporal association of > each track if my understanding is correct), new field for assassination of > each track seems to be required such as > gboolean gst_event_set_association_id (GstEvent *event, guint > association_id, const *gchar nickname (nullable)); > > gboolean gst_event_parse_association_id (GstEvent *event, guint > *association_id, const **gchar nickname); > And similar APIs for GstStream. > Having some kind of "association" system in collections was in the original design of GstStreamCollection ... but never implement because more basic things had to be fixed first :) I'd say this kind of association information should be in the collection also. It's *slightly* similar to the idea of "stream components" (where you specify which streams should be used together to produce another (richer) stream). Except in this case it's not producing "one" stream the end (but a collection of streams). Just adding an association id system in the collection would make it work, but I'm wondering whether we shouldn't have a more "end-user-friendly" system in the end. Ex: The user can simply view "oh, I can choose viewpoint 1 or viewpoint 2". And what meanings those "association" have (it's a different angle, or it's a different mixing, ...).
Review of attachment 372031 [details] [review]: This shouldn't be needed. I'm fixing gsturidecodebin3 to properly forward the event to the sourcebin if they're not linked yet. If they were linked then db3 should be sending them upstream.
Review of attachment 372029 [details] [review]: Not a fan of this. This whole "have playbin3 do the selection on its own" was just for backwards-compatibility (i.e. changing tracks via property), which we shouldn't have cared about.
One last missing part here imho, is that if adaptivedemux is in a streams-aware situation, posts the collection *and* didn't receive a SELECT_STREAMS by the time the message was posted, it should make an initial stream selection on its own.
-- 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/483.