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 750098 - New element to convert GST_FLOW_ERROR into GST_FLOW_OK
New element to convert GST_FLOW_ERROR into GST_FLOW_OK
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal enhancement
: 1.5.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Reported: 2015-05-29 11:49 UTC by Vivia Nikolaidou
Modified: 2015-06-23 08:28 UTC
See Also:
GNOME target: ---
GNOME version: ---

The patch. (10.17 KB, patch)
2015-05-29 11:49 UTC, Vivia Nikolaidou
none Details | Review
Reviewed patch. (13.40 KB, patch)
2015-06-12 14:20 UTC, Vivia Nikolaidou
none Details | Review
Reviewed(2) patch. (13.54 KB, patch)
2015-06-18 10:42 UTC, Vivia Nikolaidou
none Details | Review
Reviewed(3) patch (13.58 KB, patch)
2015-06-22 10:26 UTC, Vivia Nikolaidou
committed Details | Review

Description Vivia Nikolaidou 2015-05-29 11:49:15 UTC
Created attachment 304240 [details] [review]
The patch.

Can be used to fix misbehaving sinks. It will pass through all buffers
until it encounters GST_FLOW_ERROR. At that point it will unref the buffers
and return GST_FLOW_OK - until the next READY_TO_PAUSED or FLUSH_STOP.
Comment 1 Vivia Nikolaidou 2015-05-29 11:52:01 UTC
If there are two sinks and one is misbehaving and needs to be removed, the whole pipeline will stall. This element can be placed before suspicious sinks to keep the pipeline up and running until they can be safely removed.
Comment 2 Sebastian Dröge (slomo) 2015-06-01 11:41:52 UTC
Thanks for the patch, this would've been useful for me already a few times and I worked around it with pad probes.

It feels like this should be more configurable though, like
- a property to enable/disable error ignorign
- ignore NOT_LINKED, NOT_NEGOTIATED or others too

It should probably also just go into the debugutils plugin in gst-plugins-bad for now, no need for another micro plugin :)
Comment 3 Sebastian Dröge (slomo) 2015-06-01 11:43:02 UTC
Review of attachment 304240 [details] [review]:

::: gst/errorignore/gsterrorignore.c
@@ +41,3 @@

These #defines should go away, They are properly #defined by the gst-plugins-bad build system
Comment 4 Thiago Sousa Santos 2015-06-01 11:48:31 UTC
Wouldn't it be useful to convert GST_FLOW_ERROR to GST_FLOW_NOT_LINKED?

If you have multiple sinks and one fails it could be ok but if all fails is there any reason to keep the pipeline going? Treating the misbehaving sinks as 'not-linked' seems to make this possible.
Comment 5 Vivia Nikolaidou 2015-06-12 14:20:17 UTC
Created attachment 305151 [details] [review]
Reviewed patch.
Comment 6 Vivia Nikolaidou 2015-06-12 14:22:37 UTC
Thanks for the feedback, moved to debugutils, also now I'm converting ERROR and NOT_NEGOTIATED into NOT_LINKED by default (configurable) :) Please see the new patch attached!
Comment 7 Sebastian Dröge (slomo) 2015-06-12 14:55:05 UTC
Comment on attachment 305151 [details] [review]
Reviewed patch.

Looks good to me, I don't like the property names but I also don't know better ones :)

Comment 8 Thiago Sousa Santos 2015-06-17 14:33:49 UTC
Review of attachment 305151 [details] [review]:

Looks good. One thing I noticed is that it likely needs to reset keep_pushing when a reconfigure event is received. Maybe check with gst_pad_check_reconfigure() or add a source event handler to catch it.

Just some minor suggestions below, otherwise looks good.

::: gst/debugutils/gsterrorignore.c
@@ +23,3 @@
+ *
+ * Passes through all packets, but converts GST_FLOW_ERROR into GST_FLOW_OK
+ * (e.g. to allow other sinks to keep playing if one is misbehaving)

I think the documentation got outdated after the latest changes.

Might be worth mentioning the default values that are ignored.

@@ +120,3 @@
+  g_object_class_install_property (object_class, PROP_CONVERT_TO,
+      g_param_spec_enum ("convert-to", "GstFlowReturn to convert to",

convert-to looks not optimal to me but I don't have good ideas as well. 'converted-return'? 'ignored-return'?

But don't let this block you from pushing. I'd ask around in IRC for suggestions, if no better options arise then I'd merge it as is.
Comment 9 Vivia Nikolaidou 2015-06-18 10:42:29 UTC
Created attachment 305541 [details] [review]
Reviewed(2) patch.

Thanks for the review!
Is this what you meant with reconfigure? Does it look OK now?
Comment 10 Thiago Sousa Santos 2015-06-18 14:48:50 UTC
Review of attachment 305541 [details] [review]:

::: gst/debugutils/gsterrorignore.c
@@ +206,3 @@
+static gboolean
+gst_error_ignore_sink_event (GstPad * pad, GstObject * parent, GstEvent * event)

RECONFIGURE is an upstream event, so it should be in a src_event handler.

It will be useful here because it can be sent, for example, when a new element is linked downstream so errorignore can try pushing data again.
Comment 11 Sebastian Dröge (slomo) 2015-06-18 19:43:22 UTC
Instead of catching the event, just check gst_pad_check_reconfigure() in the chain function before handling any buffer. And if it's returning TRUE, reset keep_pushing. (But also call gst_pad_check_reconfigure() if keep_pushing is TRUE, always call it)
Comment 12 Vivia Nikolaidou 2015-06-22 10:26:18 UTC
Created attachment 305813 [details] [review]
Reviewed(3) patch

Thanks, added another version of the patch :)
Comment 13 Thiago Sousa Santos 2015-06-22 21:15:05 UTC
Review of attachment 305813 [details] [review]:

Looks good to me, no more comments.
Comment 14 Sebastian Dröge (slomo) 2015-06-23 08:28:41 UTC
commit 9664d1a6b1796491048eb579e919ab592c1ec2f6
Author: Vivia Nikolaidou <>
Date:   Fri May 29 14:27:24 2015 +0300

    error-ignore: New element to convert some GstFlowReturn types into others
    Can be used to fix misbehaving sinks. It will pass through all buffers
    until it encounters GST_FLOW_ERROR or GST_FLOW_NOT_NEGOTIATED (configurable).
    At that point it will unref the buffers and return GST_FLOW_NOT_LINKED
    (configurable) - until the next READY_TO_PAUSED or FLUSH_STOP.