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 727498 - videodecoder: deactivates downstream bufferpool
videodecoder: deactivates downstream bufferpool
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other All
: Normal normal
: 1.3.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-04-02 15:29 UTC by Thiago Sousa Santos
Modified: 2014-04-07 08:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
videodecoder: do not deactivate the bufferpool, just unref (1.89 KB, patch)
2014-04-02 15:29 UTC, Thiago Sousa Santos
committed Details | Review

Description Thiago Sousa Santos 2014-04-02 15:29:15 UTC
In playbin, we end up with the following branch:

videodecoder ! ... ! deinterlace ! ... ! videosink

When changing from or to passthrough, deinterlace currently renegotiates and might discard or acquire a new bufferpool. When it changes from passthrough to non-passthrough it acquires a new bufferpool provided by the videosink.

This same bufferpool is also being used by the videodecoder because it does a late renegotiation (only when the next buffer arrives) and when it renegotiates it will deactivate its current bufferpool before asking for another one. This bufferpool is the same that is being used by deinterlace and it will fail on the
next buffer with FLUSHING when trying to get a buffer.

To prevent this, videodecoder should not deactivate the buffer pool when renegotiating, it will be deactivated by itself when it is freed.
Comment 1 Thiago Sousa Santos 2014-04-02 15:29:58 UTC
Created attachment 273483 [details] [review]
videodecoder: do not deactivate the bufferpool, just unref

Videodecoder does late renegotiation, it will wait for the next
buffer before renegotiating its caps and bufferpool. It might happen
that downstream element switched from passthrough to non-passthrough
and sent a reconfigure upstream (that caused this renegotiation).
This downstream element will ask the video sink below for the bufferpool
with an allocation query and will get the same bufferpool that
videodecoder is holding, too.

When renegotiating, if videodecoder deactivates its bufferpool it
might be deactivating the bufferpool that some element downstream
is using and cause the pipeline to fail.
Comment 2 Thiago Sousa Santos 2014-04-02 15:32:16 UTC
Reproducible with file from https://bugzilla.gnome.org/show_bug.cgi?id=596772
Comment 3 Thiago Sousa Santos 2014-04-04 17:25:20 UTC
commit 05e957106f7c23255e6cb908a547e7214a9f8540
Author: Thiago Santos <ts.santos@sisa.samsung.com>
Date:   Wed Apr 2 07:20:43 2014 -0300

    videodecoder: do not deactivate the bufferpool, just unref
    
    Videodecoder does late renegotiation, it will wait for the next
    buffer before renegotiating its caps and bufferpool. It might happen
    that downstream element switched from passthrough to non-passthrough
    and sent a reconfigure upstream (that caused this renegotiation).
    This downstream element will ask the video sink below for the bufferpool
    with an allocation query and will get the same bufferpool that
    videodecoder is holding, too.
    
    When renegotiating, if videodecoder deactivates its bufferpool it
    might be deactivating the bufferpool that some element downstream
    is using and cause the pipeline to fail.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=727498
Comment 4 Nicolas Dufresne (ndufresne) 2014-04-04 17:52:11 UTC
Interesting, that means the sink is handing over active pool, hence a pool that cannot be configured. If max is not 0, that seems rather dangerous. We will have to dig into that at some point.
Comment 5 Sebastian Dröge (slomo) 2014-04-07 08:15:02 UTC
(In reply to comment #4)
> Interesting, that means the sink is handing over active pool, hence a pool that
> cannot be configured. If max is not 0, that seems rather dangerous. We will
> have to dig into that at some point.

Can you open a new bug about that problem so we don't forget?