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 677340 - Event errors ignored, even if caused by dataflow errors
Event errors ignored, even if caused by dataflow errors
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal critical
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-06-02 15:23 UTC by Sebastian Dröge (slomo)
Modified: 2018-11-03 12:15 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Sebastian Dröge (slomo) 2012-06-02 15:23:12 UTC
Consider the following pipeline:
audiotestsrc num-buffers=1 ! vorbisenc ! vorbisdec ! sink

vorbisdec will only push buffers to the sink as a result of the EOS event sent downstream from audiotestsrc and vorbisenc. So all gst_pad_push() in vorbisenc and audiotestsrc will always see GST_FLOW_OK.

If the pushing of data in vorbisdec (remember: triggered by the EOS event) now fails audiotestsrc (the element driving the pipeline and the element that should post error messages on the bus if pushing data fails) can't post an error message on the bus as the return values of event pushing does do not contain enough information to decide if an (and which) error happened. Thus the pipeline silently fails and goes into EOS state without any error messages on the bus (note: this requires 0ca7b85ead53e6b8be76971d5b7003ef55c8e08c, without this the pipeline just waits for EOS forever).


This could be solved by handling data-pushing-caused-by-EOS as a special case, and requiring elements to post error messages themselves on the bus in that case (instead of letting it handled by the element driving the pipeline as usual). Or we need flow returns for events.
Comment 1 Sebastian Dröge (slomo) 2012-06-04 09:06:22 UTC
As discussed on IRC the idea would be to let EOS return FALSE immediately, unlike other sticky events that only fail during data flow, if something goes wrong. This makes sense because no data will come after EOS anymore that could trigger a failure.

This OTOH also means that demuxers and other multi-src elements need to combine event return values. If it returns FALSE for all pads, FALSE is it in general. If one returns TRUE it's TRUE.

Elements driving the pipeline must post an error if pushing a sticky event failed.


And elements that are failing in draining, i.e. due to an EOS event, have to post an error message on the bus too.
Comment 2 Sebastian Dröge (slomo) 2012-06-04 09:26:40 UTC
First part:

commit 5a8c901507774aa6634c72e5ab0254b06dcbe2b2
Author: Sebastian Dröge <sebastian.droege@collabora.co.uk>
Date:   Mon Jun 4 11:25:47 2012 +0200

    pad: Always return errors for EOS events immediately
    
    For non-EOS events things will error out later during data
    flow but after EOS events no data flow is happening.
    
    See bug #677340.
Comment 3 Thiago Sousa Santos 2013-11-07 21:02:45 UTC
what happens if you have queues in the pipeline? The EOS push will succeed for the source, how does the error gets posted in this case?
Comment 4 Sebastian Dröge (slomo) 2013-11-11 14:08:27 UTC
The queue probably has to do that then, which might also make sense because it's driving that part of the pipeline.
Comment 5 Tim-Philipp Müller 2013-12-11 16:15:54 UTC
Just spotted, but not sure if it actually fully does what's needed here

commit e5a73cc2ffec5146171fc8aeb691e0c5580b4266
Author: Wim Taymans <wtaymans@redhat.com>
Date:   Wed Dec 11 14:42:34 2013 +0100

    queue: don't ignore event return value
    
    Pass the event return value upstream.
    Remove strange goto construct.
Comment 6 Sebastian Dröge (slomo) 2013-12-17 09:06:15 UTC
It's not complete, no. But a bug that needed to be fixed for the above mentioned to work at all ;)
Comment 7 Edward Hervey 2018-05-10 05:43:37 UTC
Is this still applicable ? If not, could the new event_full pad implementation help ? (they return GstFlowReturn instead of just booleans)
Comment 8 Sebastian Dröge (slomo) 2018-05-10 06:40:14 UTC
Yes, still happening. Just run the pipeline in the first comment :)
Comment 9 GStreamer system administrator 2018-11-03 12:15:26 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/24.