After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 685938 - [decodebin] Issues with group switching algorithm
[decodebin] Issues with group switching algorithm
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
unspecified
Other Linux
: Normal normal
: 1.0.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-10-11 09:06 UTC by David Corvoysier
Modified: 2012-10-14 09:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch (4.22 KB, patch)
2012-10-11 09:45 UTC, David Corvoysier
committed Details | Review

Description David Corvoysier 2012-10-11 09:06:27 UTC
The Context
-----------

Decodebin supports scenarios allowing to seamlessly switch from one stream to another inside the same "decoding chain".
To achieve that, it combines the elements it autoplugged in chains and groups, allowing only one decoding group to be active at a given time.

Typically, in the case of chained ogg, the following tree is created:

Chain "Ogg Demux"
 |_ Group "Stream 0"
 |   |_ Chain "Theora"
 |   |_ Chain "Vorbis"
 |_ Group "Stream 1"
     |_ Chain "Theora"
     |_ Chain "Vorbis"

When switching from Stream 0 to Stream 1 an EOS is sent on each end pad corresponding to Stream 0, triggering the "drain" state to propagate upstream.
Once both EOS have been processed, the "Stream 0" group is completely drained, and decodebin2 will switch to the "Stream 1" group.

One can take advantage of that design to implement even more complex use cases where one element presenting itself as a demux gathers inputs coming from heterogeneous sources and pushes them into the actual demux.
This is exactly what happens for instance when dealing with HTTP fragments such as those used in HLS or DASH.

Typically, in the case of DASH, assuming that each fragment contains both audio and video, the following tree would be created:

chain "DASH Demux"
 |_ group "Representation 1"
 |   |_ chain "Qt Demux 0"
 |       |_ group "Stream 0"
 |           |_ chain "H264"
 |           |_ chain "AAC"
 |_ group "Representation 2"
     |_ chain "Qt Demux 1"
         |_ group "Stream 1"
             |_ chain "H264"
             |_ chain "AAC"

An even more complicated tree corresponds to the case when audio and video are contained in separate fragments:

chain "DASH Demux"
 |_ group "Representation 1"
 |   |_ chain "Qt Demux 0"
 |   |   |_ group "Stream 0"
 |   |       |_ chain "H264"
 |   |_ chain "Qt Demux 1"
 |       |_ group "Stream 1"
 |           |_ chain "AAC" 
 |_ group "Representation 2"
     |_ chain "Qt Demux 3"
     |   |_ group "Stream 2"
     |       |_ chain "H264"
     |_ chain "Qt Demux 4"
         |_ group "Stream 3"
             |_ chain "AAC" 

Again, in both cases, after having received two EOS on its endpads, a group is marked as drained and decodebin can switch to the next one.

The issues
----------

There are two issues with the current decodebin2 "drain" propagation algorithm:

Issue 1: It operates with no memory of what has been drained or not, leading to multiple checks for chains/groups that are already drained.

Note: the "drained" property of GstDecodeGroup is in that respect misleading as it is only set in gst_decode_group, that is itself never called.
The two gst_decode_group_is_drained/gst_decode_chain_is_drained are actually dead code, as they are the only ones calling each other.

Issue 2: When receiving an EOS, it will only detect that a higher-level chain is drained if it contains the pad receiving the EOS.
More specifically, a chain is drained if its groups are drained (subdrained) and the EOS was received by a sub chain (handled):

    if (handled && subdrained && !*switched) {
      if (chain->next_groups) {
        ...
        *drained = FALSE;
      } else {
        GST_DEBUG ("Group %p was the last in chain %p", chain->active_group,
            chain);
        *drained = TRUE;
        /* We're drained ! */
      }

This works for the first two trees described above, but fails for the third one, where the two upper levels chains of each representation will be drained separately: in that case, when traversing the first drained chain for the second time, its "drain" state will not be detected.  

The Proposals:
--------------

- Add a drained property to the GstDecodeChain element
- Set both chain and groups "drained" properties when detecting these objects are drained
- Get out of drain_and_switch_chains as soon as possible if the chain is drained (this is already done in drain_and_switch_groups, but never happens today as the drained property is never set)

Sorry for the long description, but I tried to be as thorough as possible (still hope I didn't overlook something).
Comment 1 David Corvoysier 2012-10-11 09:45:46 UTC
Created attachment 226241 [details] [review]
Proposed patch
Comment 2 Sebastian Dröge (slomo) 2012-10-14 08:59:33 UTC
Thanks for finding this issue and providing the patch, completely makes sense :)