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 733169 - decodebin: improve deadend pads handling
decodebin: improve deadend pads handling
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
unspecified
Other All
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-07-14 16:53 UTC by Thiago Sousa Santos
Modified: 2014-08-13 15:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
decodebin: consider all deadend pads as drained (1.32 KB, patch)
2014-07-14 16:53 UTC, Thiago Sousa Santos
committed Details | Review
decodebin: handle group switching for deadend group (6.27 KB, patch)
2014-07-14 16:53 UTC, Thiago Sousa Santos
committed Details | Review

Description Thiago Sousa Santos 2014-07-14 16:53:00 UTC
Currently deadend pads aren't properly handled for pad switching. The following patches fix this situation.

The issues happen when:
1) A deadend pad is present on a group that has finished (eos on all pads). As it isn't considered drained it will prevent group switching

2) All pads are deadend on a group, leading to an error about no buffers being pushed on a pad before EOS, even when there is already a next group to expose.
Comment 1 Thiago Sousa Santos 2014-07-14 16:53:03 UTC
Created attachment 280664 [details] [review]
decodebin: consider all deadend pads as drained

Otherwise when switching out a group with a deadend pad it will block
as it would be waiting for EOS on a deadend that already got one
Comment 2 Thiago Sousa Santos 2014-07-14 16:53:10 UTC
Created attachment 280665 [details] [review]
decodebin: handle group switching for deadend group

Gracefully handle switching groups that all pads are deadend.

This can happen when quickly switching programs on mpegts as the
output is unaligned it can happen that not enough data was accumulated at
parsers to generate any buffers, causing the stream to receive EOS before
any data can be decoded.

To handle this scenario, the _expose function now also gets if there is
any next group to be exposed along with the list of endpads. If there are
no endpads and there is another group to expose it will switch to this next
group and then retry exposing the streams.

Also, the requirement to only switch from the chain that has the endpad had
to be modified to care for when the drainpad is NULL
Comment 3 Sebastian Dröge (slomo) 2014-07-16 15:04:52 UTC
Comment on attachment 280664 [details] [review]
decodebin: consider all deadend pads as drained

Not 100% sure this makes sense. A deadend pad could still receive buffers and then at some point an EOS. Right?

So this would be an optimization that allows deadend pads to be considered drained earlier already if they're the last ones?

Or what am I missing here?
Comment 4 Thiago Sousa Santos 2014-07-17 23:00:22 UTC
(In reply to comment #3)
> (From update of attachment 280664 [details] [review])
> Not 100% sure this makes sense. A deadend pad could still receive buffers and
> then at some point an EOS. Right?

The drained is being only set on the EOS handling, so definitely there was an EOS on the chain. Can it happen that the pad isn't the last one?

> 
> So this would be an optimization that allows deadend pads to be considered
> drained earlier already if they're the last ones?

I haven't reinforced a check that they are the last ones, but in my tests this seems to happen only with them. If they are not marked as drained then the next group switch will never happen.

> 
> Or what am I missing here?

I might also be missing a scenario where this EOS handler is called for a non final pad. Is this possible?
Comment 5 Sebastian Dröge (slomo) 2014-07-23 16:55:05 UTC
So let's get this in now and see what happens? :)
Comment 6 Tim-Philipp Müller 2014-07-23 16:59:02 UTC
Maybe we should wait until we've sorted out the fallout from all the previous stuff?
Comment 7 Sebastian Dröge (slomo) 2014-08-01 12:29:04 UTC
Good idea... what about now? :)
Comment 8 Sebastian Dröge (slomo) 2014-08-13 15:51:13 UTC
commit 02c8b3de65f78740402a7fbdf1e4be5140791522
Author: Thiago Santos <ts.santos@sisa.samsung.com>
Date:   Mon Jul 14 12:29:50 2014 -0300

    decodebin: handle group switching for deadend group
    
    Gracefully handle switching groups that all pads are deadend.
    
    This can happen when quickly switching programs on mpegts as the
    output is unaligned it can happen that not enough data was accumulated at
    parsers to generate any buffers, causing the stream to receive EOS before
    any data can be decoded.
    
    To handle this scenario, the _expose function now also gets if there is
    any next group to be exposed along with the list of endpads. If there are
    no endpads and there is another group to expose it will switch to this next
    group and then retry exposing the streams.
    
    Also, the requirement to only switch from the chain that has the endpad had
    to be modified to care for when the drainpad is NULL
    
    https://bugzilla.gnome.org/show_bug.cgi?id=733169

commit c5617fd373d2eeb9b2d9a1a02dec6212be714034
Author: Thiago Santos <ts.santos@sisa.samsung.com>
Date:   Fri Jul 11 18:51:44 2014 -0300

    decodebin: consider all deadend pads as drained
    
    Otherwise when switching out a group with a deadend pad it will block
    as it would be waiting for EOS on a deadend that already got one
    
    https://bugzilla.gnome.org/show_bug.cgi?id=733169