GNOME Bugzilla – Bug 735357
pad: should not allow flush-stop on inactive pads
Last modified: 2014-09-23 16:43:23 UTC
+++ This bug was initially created as a clone of Bug #734688 +++
Created attachment 284379 [details] [review] possible patch
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
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.
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.
(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.
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.
I would close it, having it the way it is now is much more consistent with the general behaviour of pads.