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 740432 - valve drop=true doesn't allow EOS to flow downstream.
valve drop=true doesn't allow EOS to flow downstream.
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Windows
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-11-20 13:18 UTC by Keith Thornton
Modified: 2018-11-03 12:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
valve: Add support for eos-pass property. (3.53 KB, patch)
2016-12-09 14:57 UTC, stevenhoving
none Details | Review

Description Keith Thornton 2014-11-20 13:18:45 UTC
The following pipeline hangs after all frames have flowed. This is probably because valve drop=true doesn't pass the EOS event downstream.

gst-launch-1.0.exe -v videotestsrc do-timestamp=true pattern=snow is-live=true num-buffers=90 !video/x-raw,width=1920,height=1080,framerate=30000/1001,format=YV12 ! tee name=t1 ! queue name=displ -queue max-size-bytes=0 leaky=2 ! valve name=display-valve drop=true ! glimagesink sync=false async=false t1. ! queue name=encode-queue max-size-bytes=124416000 ! valve name=encode-valve drop=false ! x264enc bittrate=32768 ! h264parse ! qtmux ! filesink location="f:/temp/out.mp4"
Comment 1 Olivier Crête 2014-11-20 15:53:40 UTC
This is by design, the valve will block all events, queries, etc, so you can disconnect what is after without worrying. If we let EOS through, we'd have to let FLUSH through (to remove it), etc.
Comment 2 Keith Thornton 2014-11-21 06:48:16 UTC
Would adding a parameter which would drop buffers but allow all events to flow, get the pipeline above working or would this just cause something else to break?
Comment 3 Sebastian Dröge (slomo) 2016-12-07 12:25:37 UTC
The problem with dropping EOS is, that after EOS nothing will ever come again (except for maybe a flush). So when switching to drop=false after receiving EOS, just nothing will happen (as nothing would trigger the EOS being sent downstream).

It doesn't seem like a good design to always drop EOS here. Also always dropping flushes does not seem ideal either: downstream might have data queued from before the flush, then drop=false, then flush, then drop=true and completely new data is sent that might cause confusion in combination with the old data. Also while the direct downstream of valve might not be blocked, a further downstream of it (after e.g. a queue) might be blocked regardless of drop=true or not... and a flush would help getting this unblocked.
Comment 4 Olivier Crête 2016-12-07 16:19:22 UTC
After EOS, you could flush and re-connect some other branch. Forwarding flushes is also scary because you could be in the middle of modifying the the downstream pipeline and have the flush start and stop go to different places. If someone plays with a valve, they need to figure out flushing I think. It was really designed to make dynamic pipelines a bit easier and not as a mute switch. 

Possibly we need a "muter" element for the other use-case, maybe one that replaces buffers with gap events.
Comment 5 Keith Thornton 2016-12-08 06:34:40 UTC
I gave up on valve because without the ability to pass EOS I find it virtually useless. I can achieve much better results with pad-probes but this does not help people building gst-launch pipelines
Comment 6 Sebastian Dröge (slomo) 2016-12-08 10:01:20 UTC
Yeah, the way how valve is right now it's basically useless for all but a very specific use case (the one Olivier mentioned). It should probably get a few properties to modify the behaviour so it becomes more useful.

A new element does not seem needed, I would rather say that people nowadays already expect valve to behave different usually.
Comment 7 stevenhoving 2016-12-09 14:57:07 UTC
Created attachment 341679 [details] [review]
valve: Add support for eos-pass property.

I added support for passing the eos event some time ago, but actually never used it (found a better way to solve the problem back then).
Comment 8 GStreamer system administrator 2018-11-03 12:24:30 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gstreamer/issues/84.