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 768602 - EGL_DMA_Buf: Wrong attribute list type for EGL 1.5
EGL_DMA_Buf: Wrong attribute list type for EGL 1.5
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.9.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Reported: 2016-07-09 03:05 UTC by kevin
Modified: 2016-08-16 05:35 UTC
See Also:
GNOME target: ---
GNOME version: ---

Patch to fix the issue. (4.92 KB, patch)
2016-07-13 09:27 UTC, kevin
needs-work Details | Review
Replace old patch (4.92 KB, patch)
2016-07-13 09:35 UTC, kevin
none Details | Review
Updated patch. (6.93 KB, patch)
2016-07-14 02:33 UTC, kevin
none Details | Review
For EGL 1.5 spec, the attribute list type should be EGLAttrib (6.96 KB, patch)
2016-07-14 07:25 UTC, kevin
none Details | Review
For EGL 1.5 spec, the attribute list type should be EGLAttrib. (9.30 KB, patch)
2016-08-15 07:37 UTC, kevin
committed Details | Review

Description kevin 2016-07-09 03:05:18 UTC
Below attribute list type is wrong for EGL 1.5. It is right for EGL version below 1.5.

GstEGLImage *
gst_egl_image_from_dmabuf (GstGLContext * context,
    gint dmabuf, GstVideoInfo * in_info, gint plane, gsize offset)
  EGLint attribs[13];

For EGL 1.5 spec, the attribute list type should be EGLAttrib. EGLAttrib is same as intptr_t. intptr_t will be 64bit type on 64bit system. So create EGL image from DMA Buf FD will fail and then fall back to raw data upload.

Below is the state in GEL 1.5 spec:

"EGLAttrib is  an  integral  type  defined  to  be  equivalent  to  the  ISO  C
intptr_t type. It  is  used  in  the  commands eglCreateImage, eglCreateSync, eglCreatePlatformWindowSurface, eglCreatePlatformPixmapSurface, eglGetPlatformDisplay,  and eglGetSyncAttrib,  and  will  be  used  for  all
similar commands in the future which take attribute lists or return attribute values, since such commands might at some point need to represent handle and pointer values in attribute lists as well as other integral types"
Comment 1 Sebastian Dröge (slomo) 2016-07-11 06:32:46 UTC
Do you want to provide a patch? It shouldn't be an #ifdef though, it should be a runtime check for the EGL version and then two code paths.
Comment 2 Nicolas Dufresne (ndufresne) 2016-07-11 12:15:19 UTC
How come there is a difference between (EGLInt *) and (intptr_t) ? What error do you get ?
Comment 3 kevin 2016-07-12 03:53:57 UTC
On 64bit platform, EGLInt is 32 bit value. intptr_t is 64 bit value point. So the attribute value will be wrong.
Comment 4 kevin 2016-07-12 10:54:00 UTC
as EGLAttrib only defined in EGL 1.5, so i should use #ifdef.
Comment 5 Sebastian Dröge (slomo) 2016-07-12 10:59:33 UTC
Yes but you additionally need the runtime check too... so that using EGL 1.4 or below still works when having compiled with EGL 1.5.
Comment 6 kevin 2016-07-13 09:27:28 UTC
Created attachment 331389 [details] [review]
Patch to fix the issue.
Comment 7 kevin 2016-07-13 09:35:38 UTC
Created attachment 331391 [details] [review]
Replace old patch
Comment 8 Nicolas Dufresne (ndufresne) 2016-07-13 13:03:17 UTC
Review of attachment 331389 [details] [review]:

::: gst-libs/gst/gl/egl/gsteglimage.c
@@ +261,3 @@
       GST_VIDEO_INFO_COMP_HEIGHT (in_info, plane));
+#ifdef EGL_VERSION_1_5

It must be  runtime check, since you coul dhave EGL 1.5 but run using older context.
Comment 9 Nicolas Dufresne (ndufresne) 2016-07-13 13:03:34 UTC
Review of attachment 331391 [details] [review]:

Comment 10 Nicolas Dufresne (ndufresne) 2016-07-13 13:04:10 UTC
Review of attachment 331391 [details] [review]:

Comment 11 Nicolas Dufresne (ndufresne) 2016-07-13 13:07:21 UTC
Review of attachment 331391 [details] [review]:

::: gst-libs/gst/gl/egl/gsteglimage.c
@@ +277,3 @@
+    attribs_1_5[atti] = EGL_NONE;
+    for (int i = 0; i < atti; i++)
+      GST_LOG ("attr %i: %08X", i, attribs_1_5[i]);

I think the string format is not correct here. Should probably be G_GUINTPTR_FORMAT.

@@ +304,3 @@
+#ifdef EGL_VERSION_1_5
+  if (GST_GL_CHECK_GL_VERSION (context->majorVersion, context->minorVersion, 1, 5)) {

I would just duplicate the assertions rather then doing the run-time check twice.
Comment 12 Matthew Waters (ystreet00) 2016-07-13 14:51:04 UTC
Review of attachment 331391 [details] [review]:

I was trying to think of a way to deduplicate the attribute declarations across both EGL versions and the only thing that I could see that made sense was allocating some memory to hold the attributes and performing pointer arithmetic to set the parameters.  Kind of like what GArray does.  This is more a suggestion though if you feel like implementing that.

::: gst-libs/gst/gl/egl/gsteglimage.c
@@ +279,3 @@
+      GST_LOG ("attr %i: %08X", i, attribs_1_5[i]);
+  } else {

This should be:

} else

to avoid the #ifdef at the end of the block

@@ +308,3 @@
+        EGL_LINUX_DMA_BUF_EXT, NULL, attribs_1_5);
+  } else {

} else

::: gst-libs/gst/gl/gstglcontext.h
@@ +74,3 @@
+  gint majorVersion;
+  gint minorVersion;

No camelCase please.

Should be major/minor_version
Comment 13 kevin 2016-07-14 02:33:02 UTC
Created attachment 331456 [details] [review]
Updated patch.
Comment 14 Matthew Waters (ystreet00) 2016-07-14 06:57:17 UTC
Review of attachment 331456 [details] [review]:

This won't compile with EGL_VERSION_1_5 defined

::: gst-libs/gst/gl/egl/gsteglimage.c
@@ +262,3 @@
+#ifdef EGL_VERSION_1_5
+  if (GST_GL_CHECK_GL_VERSION (context->majorVersion, context->minorVersion, 1, 5)) {

erm, aren't these egl_major and egl_minor now?  Also, don't you want to access the ctx_egl instance, not the parent class context instance?

::: gst-libs/gst/gl/gstglcontext.h
@@ +74,3 @@
+  gint egl_major;
+  gint egl_minor;

Erm, adding egl versions to the parent class is a bad idea.

This should go in GstGLContextEGL.
Comment 15 kevin 2016-07-14 07:25:00 UTC
Created attachment 331465 [details] [review]
For EGL 1.5 spec, the attribute list type should be EGLAttrib

Update for typo.
Comment 16 kevin 2016-07-22 03:29:02 UTC
Can help to review?
Comment 17 Matthew Waters (ystreet00) 2016-07-26 05:17:44 UTC
Review of attachment 331465 [details] [review]:

This fails to compile here:

gsteglimage.c: In function ‘gst_egl_image_from_dmabuf’:
gsteglimage.c:285:38: error: passing argument 5 of ‘ctx_egl->eglCreateImage’ from incompatible pointer type [-Werror=incompatible-pointer-types]
         EGL_LINUX_DMA_BUF_EXT, NULL, attribs_1_5);
gsteglimage.c:285:38: note: expected ‘const EGLint * {aka const int *}’ but argument is of type ‘EGLAttrib * {aka long int *}’

Unfortunately, it seems that we will need two different function declarations for eglCreateImage to overcome the EGLInt vs EGLAttrib issue.
Comment 18 kevin 2016-07-28 01:35:32 UTC
I only verified the patch on GStreamer 1.8.1. I will update the patch later.
Comment 19 Matthew Waters (ystreet00) 2016-07-28 01:50:51 UTC
(In reply to kevin from comment #18)
> I only verified the patch on GStreamer 1.8.1. I will update the patch later.

GStreamer version has nothing to do with that error.  It's the incompatible definitions of eglCreateImage between EGL 1.5 and the EGL extension for < 1.5 now that causes that error.
Comment 20 kevin 2016-08-15 07:37:47 UTC
Created attachment 333313 [details] [review]
For EGL 1.5 spec, the attribute list type should be EGLAttrib.

Update for build fail.
Comment 21 Matthew Waters (ystreet00) 2016-08-16 05:35:12 UTC
commit 4e04f28478fee0b85b677b9f636c50d6887a6165
Author: Song Bing <>
Date:   Wed Jul 13 17:15:44 2016 +0800

    gl/egl/dmabuf: Wrong attribute list type for EGL 1.5
    For EGL 1.5 spec, the attribute list type should be EGLAttrib.