GNOME Bugzilla – Bug 735564
gdkpixbufdec: Error when using gdkpixbufdec with ImageFreeze element
Last modified: 2014-08-28 09:16:20 UTC
Created attachment 284654 [details] [review] Patch to remove error while using ImageFreeze element When i try to use the following command gst-launch-1.0 -v filesrc location=../MARBLES.GIF ! decodebin ! videoconvert ! imagefreeze ! videoconvert ! ximagesink i get the below error ERROR: from element /GstPipeline:pipeline0/GstDecodeBin:decodebin0/GstGdkPixbufDec:gdkpixbufdec0: GStreamer encountered a general stream error. Additional debug info: gstgdkpixbufdec.c(416): gst_gdk_pixbuf_dec_sink_event (): /GstPipeline:pipeline0/GstDecodeBin:decodebin0/GstGdkPixbufDec:gdkpixbufdec0: Flow: eos ERROR: pipeline doesn't want to preroll. Setting pipeline to NULL ... on debugging the issue found that, in gst_gdk_pixbuf_dec_sink_event(), gst_gdk_pixbuf_dec_flush() returns GST_FLOW_EOS, since in ImageFreeze element's gst_image_freeze_sink_chain(), the default return value is GST_FLOW_EOS but in gst_gdk_pixbuf_dec_sink_event(), we are checking the below conditions and returning error otherwise if (res != GST_FLOW_OK && res != GST_FLOW_FLUSHING ) There are two ways to fix this issue, 1) we can add extra condition as below in GdkPixBufDec if (res != GST_FLOW_OK && res != GST_FLOW_FLUSHING && res != GST_FLOW_EOS) 2) or in gst_image_freeze_sink_chain(), we can return GST_FLOW_OK, as i dont find any reason for returning GST_FLOW_EOS My patch is with 2nd method. Please review if this is ok, or we should follow 1st method or any other way.
Comment on attachment 284654 [details] [review] Patch to remove error while using ImageFreeze element GST_FLOW_EOS is returned here because no further buffers are accepted. The real fix would be something like 1). gdkpixbufdec shouldn't post error messages based on downstream flow returns but only propagate them further upstream so a demuxer or source can handle them correctly. Especially OK, FLUSHING, EOS and also NOT_LINKED (if another strem is linked but only the demuxer can know) should not be considered as errors.
Created attachment 284664 [details] [review] New patch as per comments Hi Sebastian, Thanks for the review. I have updated the patch along the lines of 1st method by adding EOS and NOT_LINKED And i have one question regarding ImageFreeze, now i understood EOS is being passed because only the first buffer needs to be shown, then in that case, the below condition will never satisfy right? Should we have this condition? static GstFlowReturn gst_image_freeze_sink_chain (GstPad * pad, GstObject * parent, GstBuffer * buffer) { ... if (self->buffer) { GST_DEBUG_OBJECT (pad, "Already have a buffer, dropping"); gst_buffer_unref (buffer); g_mutex_unlock (&self->lock); return GST_FLOW_EOS; } ... ... } Regards, Vineeth
It can't happen anymore because GstPad already rejects buffers after EOS but that was not always the case. commit 0509e86b433aaa7976b67c2b4c8a7de5827bdd77 Author: Vineeth T M <vineeth.tm@samsung.com> Date: Thu Aug 28 13:53:23 2014 +0530 gdkpixbufdec: EOS and NOT_LINKED are no errors in general Don't post an error message for them but let upstream handle anything accordingly. https://bugzilla.gnome.org/show_bug.cgi?id=735564
Hi, Thanks for committing the patch.. Sorry i am asking the question again.. First time a buffer is passed to imagefreeze, in gst_image_freeze_sink_chain(), self->buffer will be null(so the above condition will not be satisfied) and assigned with the buffer being passed. At this point we are passing EOS. That means next time, GstPad rejects the buffer and chain function will be called anymore. This inturn means the above condition will never be satisfied right.. Sorry if i am wrong. I am new to Gstreamer and still not exactly good with the understanding. Regards, Vineeth
Yes, what you said is absolutely correct. For newer versions of GStreamer. In previous versions GstPad did not reject buffers after EOS. The check there could be removed and replaced by a comment or g_return_val_if_fail() I guess.