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 735357 - pad: should not allow flush-stop on inactive pads
pad: should not allow flush-stop on inactive pads
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal normal
: 1.5.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 734688 735359
Blocks:
 
 
Reported: 2014-08-25 07:46 UTC by Wim Taymans
Modified: 2014-09-23 16:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
possible patch (2.92 KB, patch)
2014-08-25 07:49 UTC, Wim Taymans
committed Details | Review
pad: allow flush-stops in inactive pads, but don't reset flushing (1.57 KB, patch)
2014-09-11 23:52 UTC, Thiago Sousa Santos
none Details | Review

Description Wim Taymans 2014-08-25 07:46:59 UTC
+++ This bug was initially created as a clone of Bug #734688 +++
Comment 1 Wim Taymans 2014-08-25 07:49:04 UTC
Created attachment 284379 [details] [review]
possible patch
Comment 2 Wim Taymans 2014-09-02 10:11:05 UTC
commit 7e33f52961d0215d8be10657417e6889bf9cc518
Author: Wim Taymans <wtaymans@redhat.com>
Date:   Mon Aug 25 11:34:48 2014 +0200

    tests: add flush-stop on inactive pad test
    
    Check that pushing flush-stop on an inactive pad does not clear the
    flushing flag.

commit 060b16ac75ac227d4cfe1db89ccdc4f4b31545ff
Author: Wim Taymans <wtaymans@redhat.com>
Date:   Thu Aug 21 15:49:17 2014 +0200

    pad: don't accept flush-stop on inactive pads
    
    Inactive pads should at all times have the flushing flag set. This means
    that when we get a flush-stop on an inactive pad we must ignore it.
    
    On sinkpads, make this more explicit. We used to not clear the flush
    flag but remove the events and then return an error because the flushing
    flag was set. Now just simply refuse the event without doing anything.
    
    On srcpads, check that we are trying to push a flush-stop event and
    refuse it. We would allow this and mark the srcpad as non-flushing
    anymore.
    
    Fixes https://bugzilla.gnome.org/show_bug.cgi?id=735357
Comment 3 Thiago Sousa Santos 2014-09-11 23:52:18 UTC
Created attachment 285969 [details] [review]
pad: allow flush-stops in inactive pads, but don't reset flushing

Flush stops will flow all the same, but they won't reset the flushing
state as it will be done when the pad is activated.

The use case for this is restarting a source branch on a pipeline
without resetting the whole pipeline or iterating over the branch
element pads to deactivate/activate them one by one.
Comment 4 Thiago Sousa Santos 2014-09-12 00:05:10 UTC
This bug seems to make an use case harder, I propose the above patch instead of it.

The use case is in adaptive demuxers to restart the http source for downloading a new fragment after it has gone EOS.

1) The httpsrc finishes downloading and pushes EOS
2) EOS is intercepted and dropped in some pad downstream
3) The elements are put to READY, the src is put to NULL and gets a new URI
4) src is brought back to READY
5) A flush-start/stop pair is sent on this branch to remove the EOS status of the elements.

Does this make sense? The other options I have are to put all the elements in the branch to NULL or iterate over the pads and clear the flag by deactivating/activating them.

Reopening the bug so we can discuss this.
Comment 5 Sebastian Dröge (slomo) 2014-09-12 06:51:56 UTC
(In reply to comment #4)

> 4) src is brought back to READY
> 5) A flush-start/stop pair is sent on this branch to remove the EOS status of
> the elements.

Setting the elements to READY will already drop the EOS flag. I think in the adaptive demuxers the problem is the ghostpad, which is not deactivated and thus keeps the EOS state.
Comment 6 Thiago Sousa Santos 2014-09-18 16:34:16 UTC
This has been fixed in dashdemux by using the sticky events foreach to remove the EOS event and then unsetting the EOS flag. Due to the special nature of dashdemux's ghostpad using pad deactivation doesn't work.

Should we close this? Or is there anything left for discussion? I still think that the flush events help with resetting in delicate application driven scenarios and it might be useful to fix this with the patch proposed above. But then I don't have a real use case to defend it so I'm ok with closing this for now.
Comment 7 Sebastian Dröge (slomo) 2014-09-23 16:43:23 UTC
I would close it, having it the way it is now is much more consistent with the general behaviour of pads.