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 784365 - gst-omx: make generic the OMX_UseEGLImage code path
gst-omx: make generic the OMX_UseEGLImage code path
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-omx
git master
Other All
: Normal enhancement
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-06-29 22:16 UTC by Julien Isorce
Modified: 2017-08-01 09:46 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
omxvideodec: fix buffer leak when eglimage setup fails (1.46 KB, patch)
2017-06-29 22:20 UTC, Julien Isorce
committed Details | Review
omxvideodec: make generic the OMX_UseEGLImage code path (11.01 KB, patch)
2017-06-29 22:24 UTC, Julien Isorce
none Details | Review
omxvideodec: make generic the OMX_UseEGLImage code path (11.69 KB, patch)
2017-07-12 13:22 UTC, Julien Isorce
none Details | Review
omxvideodec: make generic the OMX_UseEGLImage code path (11.78 KB, patch)
2017-08-01 08:07 UTC, Julien Isorce
none Details | Review
omxvideodec: make generic the OMX_UseEGLImage code path (11.68 KB, patch)
2017-08-01 08:26 UTC, Julien Isorce
committed Details | Review

Description Julien Isorce 2017-06-29 22:16:37 UTC
Currently the code path, that negotiates and register eglimages with OMX_UseEGLImage, is guarded with #ifdef RPI.

It makes it hard to maintain and improve. But thanks to Tizonia that works and Desktop and which supports OMX_UseEGLImage it is now possible to enable and test this code path on desktop.

At the moment there is *no* Tizonia component that returns success when calling OMX_UseEGLImage but at least this allows to run the code and validate the fallback mechanism.

But soon enough we should have a Tizonia hw component working on desktop so we will be able to fully test this omx/eglimage code path.
Comment 1 Julien Isorce 2017-06-29 22:20:43 UTC
Created attachment 354709 [details] [review]
omxvideodec: fix buffer leak when eglimage setup fails
Comment 2 Julien Isorce 2017-06-29 22:24:57 UTC
Created attachment 354711 [details] [review]
omxvideodec: make generic the OMX_UseEGLImage code path

Tested on Desktop in a VM with Tizonia with pipeline:
GST_GL_PLATFORM=egl GST_GL_API=gles2 GST_OMX_CONFIG_DIR=tizonia gst-launch-1.0 filesrc location=test.webm ! matroskademux ! omxvp8dec ! glimagesink

This validate the build and the code path where OMX_UseEGLImage fails and the decoder fallbacks to classic path.
Comment 3 Julien Isorce 2017-07-12 13:22:36 UTC
Created attachment 355426 [details] [review]
omxvideodec: make generic the OMX_UseEGLImage code path

Only include required gstgl headers (#include <gst/gl/egl/gstglmemoryegl.h>) instead of all gstgl headers (#include <gst/gl/gl.h>).
Comment 4 Julien Isorce 2017-07-17 16:14:58 UTC
Comment on attachment 354709 [details] [review]
omxvideodec: fix buffer leak when eglimage setup fails

commit 7b114b131f018dddf06acacab1c9356299492725
Author: Julien Isorce <jisorce@oblong.com>
Date:   Thu Jun 29 23:17:26 2017 +0100

    omxvideodec: fix buffer leak when eglimage setup fails
    
    Can happen if gst_buffer_pool_acquire_buffer succeeds but
    gst_buffer_n_memory (buffer) is not exactly 1.
    
    In theory this should not happen because the decoder requests
    EGLImage(RGBA) but better to fix any leak on corner cases.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=784365
Comment 5 Julien Isorce 2017-08-01 08:07:32 UTC
Created attachment 356687 [details] [review]
omxvideodec: make generic the OMX_UseEGLImage code path

Just rebase
Comment 6 Julien Isorce 2017-08-01 08:26:29 UTC
Created attachment 356688 [details] [review]
omxvideodec: make generic the OMX_UseEGLImage code path

Build fix
Comment 7 Guillaume Desmottes 2017-08-01 08:32:31 UTC
Built and tested on rpi3. Doesn't seem to cause any regression.
Comment 8 Julien Isorce 2017-08-01 09:44:38 UTC
Comment on attachment 355426 [details] [review]
omxvideodec: make generic the OMX_UseEGLImage code path

commit 86d9a2c81c60a3a246c7052e70e9d50b8a12b156
Author: Julien Isorce <jisorce@oblong.com>
Date:   Mon Jul 17 21:06:47 2017 +0100

    omxvideodec: make generic the OMX_UseEGLImage code path
    
    Will be easier to maintain and to make enhancements.
    
    Tested with Tizonia on Desktop.
    Also tested with Bellagio to make sure it does not crash when
    calling OMX_UseEGLImage and indeed it returns NotImplemented.
    Then gst-omx fallback to OMX_UseBuffer if it can and so on.
    
    Also tested on rpi to make sure there is no regression.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=784365
Comment 9 Julien Isorce 2017-08-01 09:44:52 UTC
(In reply to Guillaume Desmottes from comment #7)
> Built and tested on rpi3. Doesn't seem to cause any regression.

Thx a lot!
Comment 10 Julien Isorce 2017-08-01 09:46:08 UTC
Comment on attachment 356688 [details] [review]
omxvideodec: make generic the OMX_UseEGLImage code path

commit 86d9a2c81c60a3a246c7052e70e9d50b8a12b156
Author: Julien Isorce <jisorce@oblong.com>
Date:   Mon Jul 17 21:06:47 2017 +0100

    omxvideodec: make generic the OMX_UseEGLImage code path
    
    Will be easier to maintain and to make enhancements.
    
    Tested with Tizonia on Desktop.
    Also tested with Bellagio to make sure it does not crash when
    calling OMX_UseEGLImage and indeed it returns NotImplemented.
    Then gst-omx fallback to OMX_UseBuffer if it can and so on.
    
    Also tested on rpi to make sure there is no regression.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=784365