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 741856 - omxvideodec: add a hack to workaround egl_render signalling EOS prematurely
omxvideodec: add a hack to workaround egl_render signalling EOS prematurely
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-omx
git master
Other Linux
: Normal normal
: 1.10.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-12-22 13:17 UTC by George Kiagiadakis
Modified: 2016-12-22 12:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (3.39 KB, patch)
2014-12-22 13:17 UTC, George Kiagiadakis
none Details | Review
omxvideodec: add a signals-premature-eos hack for egl_render (3.56 KB, patch)
2016-12-01 17:25 UTC, Philippe Normand
none Details | Review
omxvideodec: add a signals-premature-eos hack for egl_render (3.50 KB, patch)
2016-12-01 17:26 UTC, Philippe Normand
none Details | Review
updated patch (3.71 KB, patch)
2016-12-08 14:36 UTC, Philippe Normand
committed Details | Review

Description George Kiagiadakis 2014-12-22 13:17:04 UTC
Created attachment 293178 [details] [review]
patch

The egl_render component of the broadcom (raspberry pi) implementation has a bug that causes it to signal OMX_BUFFERFLAG_ENDOFFRAME | OMX_BUFFERFLAG_EOS  at some point earlier than the actual end of the file. After that, violating the OMX specification, it continues pushing out some more buffers without any further indication of when it has really finished.

This patch adds a hack that causes gst-omx to wait a bit more time after egl_render has signalled EOS before pushing EOS further to the gstreamer video sink. In case a new buffer arrives at this point, omxvideodec pushes it normally downstream and waits again. If this timeout passes without any new frame from egl_render, then EOS is properly sent to the video sink too.
Comment 1 Mart Raudsepp 2015-02-25 12:40:06 UTC
Perhaps this could be fixed upstream instead via detailed report to https://github.com/raspberrypi/userland/issues ?
Comment 2 Mart Raudsepp 2015-02-25 12:44:37 UTC
Actually https://github.com/raspberrypi/firmware/issues might be more appropriate; after looking I mostly just find MMAL processing delivering the EOS ( https://github.com/raspberrypi/userland/blob/master/interface/mmal/components/avcodec_video_decoder.c#L416 ), so likely firmware sends it wrong (but didn't look too hard)
Comment 3 Sebastian Dröge (slomo) 2015-03-04 09:03:21 UTC
Comment on attachment 293178 [details] [review]
patch

Looks good but doesn't apply to master.
Comment 4 Julien Isorce 2015-03-24 13:48:59 UTC
Does someone with a Pi can rebase and validate the attached patch ?
Comment 5 Philippe Normand 2016-12-01 17:25:21 UTC
Created attachment 341169 [details] [review]
omxvideodec: add a signals-premature-eos hack for egl_render

egl_render seems to have a bug and signals EOS before it has finished
pushing out all data; this hack simply makes acquire_buffer() wait
a bit more before signalling EOS, in case egl_render decides to spit
out some more data.
Comment 6 Philippe Normand 2016-12-01 17:25:57 UTC
Rebased and validated.
Comment 7 Philippe Normand 2016-12-01 17:26:55 UTC
Created attachment 341170 [details] [review]
omxvideodec: add a signals-premature-eos hack for egl_render

egl_render seems to have a bug and signals EOS before it has finished
pushing out all data; this hack simply makes acquire_buffer() wait
a bit more before signalling EOS, in case egl_render decides to spit
out some more data.
Comment 8 Sebastian Dröge (slomo) 2016-12-05 09:29:51 UTC
Review of attachment 341170 [details] [review]:

Makes sense to me. Ugly but... :)
Did you also file a bug against the RPi firmware for this? Please do and link this here.


Just a minor thing needs to be changed:

::: omx/gstomx.h
@@ +49,3 @@
+ * out all of its buffers. Happens with egl_render on the rpi.
+ */
+#define GST_OMX_HACK_SIGNALS_PREMATURE_EOS                            G_GUINT64_CONSTANT (0x0000000000000400)

You also want to add a mapping from a gstomx.conf string to this enum in gstomx.c
Comment 9 Philippe Normand 2016-12-08 14:35:35 UTC
(In reply to Sebastian Dröge (slomo) from comment #8)
> Review of attachment 341170 [details] [review] [review]:
> 
> Makes sense to me. Ugly but... :)
> Did you also file a bug against the RPi firmware for this? Please do and
> link this here.
> 

I'm using some rather old kernel/firmware/userland and didn't have the time to check with newer versions so I haven't filled a bug upstream. Sorry about that :)
Comment 10 Philippe Normand 2016-12-08 14:36:10 UTC
Created attachment 341617 [details] [review]
updated patch
Comment 11 Sebastian Dröge (slomo) 2016-12-12 08:41:51 UTC
Comment on attachment 341617 [details] [review]
updated patch

Looks good, but please file a bug against RPi before merging this.
Comment 12 Sebastian Dröge (slomo) 2016-12-13 09:39:03 UTC
commit 05b137a256f0e90926c76b0c8b76f16e35ccc65c
Author: George Kiagiadakis <george.kiagiadakis@collabora.com>
Date:   Thu Dec 1 18:23:50 2016 +0100

    Add a signals-premature-eos hack for egl_render
    
    egl_render seems to have a bug and signals EOS before it has finished
    pushing out all data; this hack simply makes acquire_buffer() wait
    a bit more before signalling EOS, in case egl_render decides to spit
    out some more data.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=741856