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 703343 - Add EGLImage support (consume and provide buffer pool)
Add EGLImage support (consume and provide buffer pool)
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal blocker
: 1.3.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
: 727844 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2013-06-30 12:23 UTC by Sebastian Dröge (slomo)
Modified: 2014-04-15 17:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gl: fix crash if _build_extension_string is not called (994 bytes, patch)
2014-03-24 12:44 UTC, Julien Isorce
committed Details | Review
pkgconfig: add gstreamer-gl (1.84 KB, patch)
2014-03-24 12:44 UTC, Julien Isorce
committed Details | Review
gl: remove commented and unsued code in x11 Makefile.am (823 bytes, patch)
2014-03-24 12:44 UTC, Julien Isorce
committed Details | Review
gl: deploy egl headers in gst/gl/egl instead of gst/gl (2.35 KB, patch)
2014-03-24 12:45 UTC, Julien Isorce
committed Details | Review
remove libgstegl and eglglessink (225.14 KB, patch)
2014-03-24 12:46 UTC, Julien Isorce
committed Details | Review
gl: add EGLImage support (39.94 KB, patch)
2014-03-24 12:46 UTC, Julien Isorce
none Details | Review
gl: allow to include GLES/gl.h (742 bytes, patch)
2014-03-24 12:47 UTC, Julien Isorce
committed Details | Review
omxvideodec: fix for new gl api changes (2.73 KB, patch)
2014-03-24 12:47 UTC, Julien Isorce
committed Details | Review
configure.ac: check for gstgl since gstegl is deprecated (10.42 KB, patch)
2014-03-24 12:48 UTC, Julien Isorce
committed Details | Review
testegl: port to gstgl API (29.53 KB, patch)
2014-03-24 12:48 UTC, Julien Isorce
none Details | Review
gl: add EGLImage support (41.40 KB, patch)
2014-03-24 16:17 UTC, Julien Isorce
committed Details | Review
testegl: port to gstgl API (31.18 KB, patch)
2014-03-24 16:19 UTC, Julien Isorce
needs-work Details | Review

Description Sebastian Dröge (slomo) 2013-06-30 12:23:03 UTC
gst-plugins-gl should get support for EGLImage, both as a consumer but also as a provider of a buffer pool to upstream elements. This could then e.g. be used by gst-omx on the Raspberry Pi.

eglglessink has some code for this already.
Comment 1 Julien Isorce 2014-01-21 16:21:38 UTC
Just in case, does someone already started something ? :)
It's not planed that I'll work on this but just to know.
Comment 2 Sebastian Dröge (slomo) 2014-01-22 08:46:27 UTC
I don't think so
Comment 3 Julien Isorce 2014-02-06 12:32:04 UTC
I think we could create an "eglimagetrans" element that propose a GstEGLImageBufferPool to upstream.

It would work in several different ways depending on what upstream and downstream can support.

It only make sense to use the eglimagetrans element if downstream supports GstVideoGLTextureUploadMeta.

A: If downstream also propose a GL context then it uses this context (or do GL context sharing if there is a thread boundary).

Then it creates a GstEGLImageBufferPool using eglCreateImageKHR (gl_context, EGL_GL_TEXTURE_2D_KHR, texture).


B: If downstream does not propose a gl context then it attempts to create a GstEGLImageBufferPool using eglCreateImageKHR (EGL_NO_CONTEXT, EGL_IMAGE_BRCM_MULTIMEDIA, bits) on RPI or EGL_NATIVE_BUFFER_ANDROID on android  (Depends if the platform supports http://www.khronos.org/registry/egl/extensions/KHR/EGL_KHR_image_pixmap.txt)

In any case A or B, eglimagetrans attachs a GstVideoGLTextureUploadMeta to each buffer that will call glEGLImageTargetTexture2DOES.

Finally eglimagetrans propose its pool to upstream. omxvideodec already knows how to use it.

It avoid to manually make use of GstEGLImageBufferPool in every external element that would like to provide it to upstream. (ex: playbin uri=foo video-sink="eglimagetrans ! webkitvideosink)

Should be implemented in a way that eglimagetrans would be hidden when doing omxvideodec ! glimagesink (factorized in GstGLUpload).
So that eglimagetrans is only required when using an external sink (= sink which is not part of gst-plugins-gl)

Or re-enable glupload element
or use GstGLUpload in the external sink.

(Also it should be possible to do the reverse way, gleffects ! gstomxvideoencoder)

Depends on https://bugzilla.gnome.org/show_bug.cgi?id=706054
Comment 4 Sebastian Dröge (slomo) 2014-02-06 22:21:48 UTC
I think resurrecting glupload and gldownload elements would make sense and also other helper elements. But all this functionality should also be available in public gst-plugins-gl API.

The name eglimagetrans seems a bit weird, but an element doing something like that seems useful to me.
Comment 5 Matthew Waters (ystreet00) 2014-02-07 09:38:11 UTC
I think that if we integrate the EGLImageBufferpool handling into GstGLFilter, then glcolorscale could be used for this purpose instead.  glcolorscale essentially provides the functionality of GstGLFilter to the pipeline with no effects/transformations.  The other do-nothing element is 'gleffects effect=0'
Comment 6 Julien Isorce 2014-03-24 12:44:14 UTC
Created attachment 272767 [details] [review]
gl: fix crash if _build_extension_string is not called
Comment 7 Julien Isorce 2014-03-24 12:44:30 UTC
Created attachment 272768 [details] [review]
pkgconfig: add gstreamer-gl
Comment 8 Julien Isorce 2014-03-24 12:44:58 UTC
Created attachment 272769 [details] [review]
gl: remove commented and unsued code in x11 Makefile.am
Comment 9 Julien Isorce 2014-03-24 12:45:16 UTC
Created attachment 272770 [details] [review]
gl: deploy egl headers in gst/gl/egl instead of gst/gl
Comment 10 Julien Isorce 2014-03-24 12:46:26 UTC
Created attachment 272771 [details] [review]
remove libgstegl and eglglessink

Note that we should add eagl support to gstgl since eglglessink supported it
Comment 11 Julien Isorce 2014-03-24 12:46:45 UTC
Created attachment 272773 [details] [review]
gl: add EGLImage support
Comment 12 Julien Isorce 2014-03-24 12:47:09 UTC
Created attachment 272774 [details] [review]
gl: allow to include GLES/gl.h
Comment 13 Julien Isorce 2014-03-24 12:47:38 UTC
Created attachment 272775 [details] [review]
omxvideodec: fix for new gl api changes
Comment 14 Julien Isorce 2014-03-24 12:48:03 UTC
Created attachment 272776 [details] [review]
configure.ac: check for gstgl since gstegl is deprecated
Comment 15 Julien Isorce 2014-03-24 12:48:29 UTC
Created attachment 272777 [details] [review]
testegl: port to gstgl API
Comment 16 Julien Isorce 2014-03-24 16:17:52 UTC
Created attachment 272799 [details] [review]
gl: add EGLImage support

Forgot to add caps feature for eglimage memory in glfilter and glimagesink
Comment 17 Julien Isorce 2014-03-24 16:19:59 UTC
Created attachment 272800 [details] [review]
testegl: port to gstgl API

Still need to port the example to GLESv2 or add GLESv1 support in libgstgl
Comment 18 Matthew Waters (ystreet00) 2014-03-25 01:46:26 UTC
Review of attachment 272799 [details] [review]:

Looks good.

::: gst-libs/gst/gl/egl/gstglcontext_egl.h
@@ +59,3 @@
+ * add gst_gl_context_egl_new_gl_no_context that only manages the display
+ * add gst_gl_context_egl_is_gl_no_context () */
+

I'm just curious as to the case for EGL_NO_CONTEXT.  ie when would we use it?

::: gst-libs/gst/gl/gl.h
@@ +43,3 @@
+#include <gst/gl/egl/gstgldisplay_egl.h>
+#include <gst/gl/egl/gsteglimagememory.h>
+#endif

Are you sure you want to do this?  I guess if we don't, then applications/elements need to do this themselves.
Comment 19 Matthew Waters (ystreet00) 2014-03-25 01:56:48 UTC
Review of attachment 272771 [details] [review]:

Looks good

::: configure.ac
@@ -557,3 @@
-                            EGL_LIBS="-lGLESv2 -lEGL -lMali -lUMP"
-                            EGL_CFLAGS=""
-                            AC_DEFINE(USE_EGL_MALI_FB, [1], [Use Mali FB EGL window system])

Move this along with the rpi check?
Comment 20 Julien Isorce 2014-03-25 18:08:42 UTC
(In reply to comment #18)
> Review of attachment 272799 [details] [review]:
> 
> Looks good.
> 
> ::: gst-libs/gst/gl/egl/gstglcontext_egl.h
> @@ +59,3 @@
> + * add gst_gl_context_egl_new_gl_no_context that only manages the display
> + * add gst_gl_context_egl_is_gl_no_context () */
> +
> 
> I'm just curious as to the case for EGL_NO_CONTEXT.  ie when would we use it?
> 

It's theoretically possible to create an eglimage with glcontext input param set to EGL_NO_CONTEXT. See #c3. I recently tried without any success though :)
So as long now an eglimage memory needs a gl context to be created (so that it keeps a ref on it so that the eglimage source which is a gl texture, can be deleted in the right gl context) then having GstGLContext_EGL_NO_CONTEXT make senses because it own the EGLDisplay. And you need it to create an eglimage (even with EGL_NO_CONTEXT).  Not sure to be clear :)

> ::: gst-libs/gst/gl/gl.h
> @@ +43,3 @@
> +#include <gst/gl/egl/gstgldisplay_egl.h>
> +#include <gst/gl/egl/gsteglimagememory.h>
> +#endif
> 
> Are you sure you want to do this?  I guess if we don't, then
> applications/elements need to do this themselves.

You right I'll do the change. Do you think gsteglimagememory.h is a right name ? or maybe gstglmemory_eglimage.h or gstglmemory_egl.h ?
Comment 21 Julien Isorce 2014-03-25 18:11:01 UTC
(In reply to comment #19)
> Review of attachment 272771 [details] [review]:
> 
> Looks good
> 
> ::: configure.ac
> @@ -557,3 @@
> -                            EGL_LIBS="-lGLESv2 -lEGL -lMali -lUMP"
> -                            EGL_CFLAGS=""
> -                            AC_DEFINE(USE_EGL_MALI_FB, [1], [Use Mali FB EGL
> window system])
> 
> Move this along with the rpi check?

Right. I have not the right platform to check though.
Comment 22 Matthew Waters (ystreet00) 2014-03-26 00:02:31 UTC
(In reply to comment #20)
> (In reply to comment #18)
> It's theoretically possible to create an eglimage with glcontext input param
> set to EGL_NO_CONTEXT. See #c3. I recently tried without any success though :)
> So as long now an eglimage memory needs a gl context to be created (so that it
> keeps a ref on it so that the eglimage source which is a gl texture, can be
> deleted in the right gl context) then having GstGLContext_EGL_NO_CONTEXT make
> senses because it own the EGLDisplay. And you need it to create an eglimage
> (even with EGL_NO_CONTEXT).  Not sure to be clear :)

So, it's kind of like GL's pbuffers for X but inside EGL, interesting.

> > ::: gst-libs/gst/gl/gl.h
> > @@ +43,3 @@
> > +#include <gst/gl/egl/gstgldisplay_egl.h>
> > +#include <gst/gl/egl/gsteglimagememory.h>
> > +#endif
> > 
> > Are you sure you want to do this?  I guess if we don't, then
> > applications/elements need to do this themselves.
> 
> You right I'll do the change. Do you think gsteglimagememory.h is a right name
> ? or maybe gstglmemory_eglimage.h or gstglmemory_egl.h ?

I'd probably go with gstglmemory_eglimage.h personally but whatever.
Comment 23 Julien Isorce 2014-03-26 15:06:16 UTC
commit 6d10548e7f088281238e62f0cb89fde43937059e
Author: Julien Isorce <julien.isorce@collabora.co.uk>
Date:   Mon Mar 24 12:12:42 2014 +0000

    gl: deploy egl headers in gst/gl/egl instead of gst/gl
    
    https://bugzilla.gnome.org/show_bug.cgi?id=703343

commit 5bb4c4e8666a295ca4d3830ca0f31a6c78faa7de
Author: Julien Isorce <julien.isorce@collabora.co.uk>
Date:   Mon Mar 24 12:10:00 2014 +0000

    gl: remove commented and unsued code in x11 Makefile.am
    
    https://bugzilla.gnome.org/show_bug.cgi?id=703343

commit c5833625ebe360c37b8c273a92bbd91fae4b6fd3
Author: Julien Isorce <julien.isorce@collabora.co.uk>
Date:   Sun Mar 23 21:55:34 2014 +0000

    pkgconfig: add gstreamer-gl
    
    https://bugzilla.gnome.org/show_bug.cgi?id=703343

commit 2893a70aa094574a765d8195941a2de010c1c89f
Author: Julien Isorce <julien.isorce@collabora.co.uk>
Date:   Mon Mar 24 12:04:08 2014 +0000

    gl: fix crash if _build_extension_string is not called
    
    On GLES2 then (gl->GetIntegerv && gl->GetStringi) is false
    regression introduced by cc6df204e2f58fffda5cbe90f3450aeba95889c4
    
    https://bugzilla.gnome.org/show_bug.cgi?id=703343
Comment 24 Julien Isorce 2014-03-26 19:11:38 UTC
Comment on attachment 272799 [details] [review]
gl: add EGLImage support

commit 0ae3c984aa84f310f91b0764f44a5c3d84a20e46
Author: Julien Isorce <julien.isorce@collabora.co.uk>
Date:   Sat Mar 22 22:01:49 2014 +0000

    gl: add EGLImage support
    
    * picked from old libgstegl:
      - GstEGLImageMemory
      - GstEGLImageAllocator
      - last_buffer management from removed GstEGLImageBufferPool
    
    * add-ons:
      - GstEGLImageMemory now old a reference on GstGLContext
        so that it can delete the EGLImage and its gltexture source
        while having the associated gl context being current.
      - add EGLImage support for GstVideoGLTextureUploadMeta which
        mainly call EGLImageTargetTexture2D
      - GstGLBufferPool now supports GstEGLImageAllocator
      - glimagesink / glfilters / etc.. now propose GstEGLImageAllocator
        to upstream
    
    https://bugzilla.gnome.org/show_bug.cgi?id=703343
Comment 25 Jan Schmidt 2014-04-12 17:44:20 UTC
This breaks the android static plugins build because libgstgl and libgstegl now export overlapping symbols:

gst_is_egl_image_memory
gst_egl_image_memory_get_image
gst_egl_image_memory_get_display
gst_egl_image_memory_get_orientation
gst_egl_image_memory_set_orientation
gst_egl_image_allocator_get_type
Comment 26 Sebastian Dröge (slomo) 2014-04-12 19:36:30 UTC
libgstegl and eglglessink are going to be removed rsn
Comment 27 Sebastian Dröge (slomo) 2014-04-12 20:33:42 UTC
*** Bug 727844 has been marked as a duplicate of this bug. ***
Comment 28 Sebastian Dröge (slomo) 2014-04-13 08:23:53 UTC
Julien, I would say that this is good to be merged now.
Comment 29 Sebastian Dröge (slomo) 2014-04-13 08:25:13 UTC
Comment on attachment 272800 [details] [review]
testegl: port to gstgl API

This should just be ported to GLES2, shouldn't be that much work. But this shouldn't block merging the other patches.
Comment 30 Julien Isorce 2014-04-13 09:04:15 UTC
(In reply to comment #28)
> Julien, I would say that this is good to be merged now.

oki good!  The only thing blocking to me here is testegl.c in omx, Not sure to have time to finalize its portage and make it work this week. And if I push the removal of libgstegl+eglglessink now it will break the current version of this omx/testegl.c.

To unblock the situation #25 we could either just move current -bad/gst-libs/gst/egl/egl.h-c to omx/examples/egl/ or disable its build until I finish to port it.
Comment 31 Sebastian Dröge (slomo) 2014-04-13 09:26:10 UTC
Disable it for now :)
Comment 32 Julien Isorce 2014-04-15 17:36:16 UTC
Comment on attachment 272800 [details] [review]
testegl: port to gstgl API

Will be done a bit later this week, I'll open a new bug for that
Comment 33 Julien Isorce 2014-04-15 17:36:36 UTC
commit 09116bf10d515d50bdd1e9a20278be816cb762aa
Author: Julien Isorce <julien.isorce@collabora.co.uk>
Date:   Tue Apr 15 17:58:34 2014 +0100

    egl/eglglessink: remove since EGLImage and iOS support have been added in glimagesink
    
    https://bugzilla.gnome.org/show_bug.cgi?id=703343

commit d93ed2b8704f93482b73718493d39fe51f5dfcc7
Author: Julien Isorce <julien.isorce@collabora.co.uk>
Date:   Mon Mar 24 12:08:43 2014 +0000

    gl: allow to include GLES/gl.h
    
    https://bugzilla.gnome.org/show_bug.cgi?id=703343
Comment 34 Julien Isorce 2014-04-15 17:36:53 UTC
commit 21b3c2b16a8cb24af48fb75376122259a4dcb482
Author: Julien Isorce <julien.isorce@collabora.co.uk>
Date:   Tue Apr 15 17:30:13 2014 +0100

    example: disable testegl since libgstegl has been removed
    
    As decided in bug #703343
    
    Not compatible with the new libgstgl API.
    A portage has been started, attachment 272800 [details] [review].
    
    https://bugzilla.gnome.org/show_bug.cgi?id=703343

commit 499fb23e9c9979d3afc525534159ea7e5ae14206
Author: Julien Isorce <julien.isorce@collabora.co.uk>
Date:   Tue Apr 15 17:11:08 2014 +0100

    omxvideodec: use new libgstgl API since libgstegl has been removed
    
    There is no point to retrieve a ref/unref type
    instead of an EGLDisplay directly. It's like for EGLImage.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=703343

commit d4bb7cb4c74e8d455e1c53d32c9a1106cd8c2d27
Author: Julien Isorce <julien.isorce@collabora.co.uk>
Date:   Tue Apr 15 17:06:38 2014 +0100

    configure.ac: check for libgstgl since libgstegl has been removed
    
    https://bugzilla.gnome.org/show_bug.cgi?id=703343