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 728234 - gl: Don't unconditonally use EGL extensions
gl: Don't unconditonally use EGL extensions
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.3.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-04-15 05:25 UTC by Eric Toombs
Modified: 2014-05-20 13:46 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
config.log - no gtkdoc (330.76 KB, text/x-log)
2014-04-16 06:06 UTC, Eric Toombs
  Details
config.log - AUR vanilla (331.85 KB, text/x-log)
2014-04-16 06:15 UTC, Eric Toombs
  Details
gl/context: add generic feature checking support (3.06 KB, patch)
2014-05-14 08:02 UTC, Matthew Waters (ystreet00)
committed Details | Review
gl/eglimage: add eglimage context feature (10.60 KB, patch)
2014-05-14 08:04 UTC, Matthew Waters (ystreet00)
reviewed Details | Review

Description Eric Toombs 2014-04-15 05:25:37 UTC
./configure --enable-egl=no and --without-egl don't seem to do anything at all, because EGL builds anyway.

--with-egl-window-system=none will prevent EGL from being built, but the build fails later on here:
  DOC   Introspecting gobjects
libtool: link: cannot find the library `../../gst-libs/gst/gl/libgstgl-1.0.la' or unhandled argument `../../gst-libs/gst/gl/libgstgl-1.0.la'

The whole build command for that last error:
./autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var \
    --with-gtk=3.0 --with-egl-window-system=none
make

As a temporary workaround, I tried disabling gl with --disable-gl. It looks for libgstgl anyway, and doesn't find it, just like before.


Now, you might be wondering why I want to disable EGL. It's because nvidia's EGL implementation doesn't support the *KHR API extension that is being used inside gst-libs/gst/egl/egl.c. In particular, linking fails with the unresolved symbol "eglDestroyImageKHR". I am pretty sure there isn't a new version of nvidia with *KHR implemented, so until there is, anybody who wants to use gst-plugins-bad with nvidia will need to be able to disable EGL.
Comment 1 Matthew Waters (ystreet00) 2014-04-15 06:47:36 UTC
So, at the moment, there are two egl implementations in gst-plugins-bad.  The one in gst-libs/gst/egl (1) and the one in gst-libs/gst/gl/egl (2).

(1) one is going to be removed for 1.4.  (2) implements all the parts of (1) now.

--with-egl-window-system is for (1)
--disable-egl is for (2)

As for the error building the docs.  gtk-doc doesn't, last time I checked, handle c preprocesser #if's very well in the .types file.  So it uses libgstgl (from (2)) irrespective of whether it was built.  Although, (2) can also be built against GLX unless you disable that too.

You could workaround by passing --disable-gtk-doc.

Could you please attach your config.log ?
Comment 2 Sebastian Dröge (slomo) 2014-04-15 07:59:05 UTC
We should probably get eglDestroyImageKHR() and friends at runtime (and also check for their existence then). These are extensions to EGL and not guaranteed to be there.

Of course independent of that disabling of EGL support should also work :)
Comment 3 Eric Toombs 2014-04-16 06:01:02 UTC
Well, as I was about to test the effect of --disable-gtk-doc, I synced and it appears the old EGL code was ripped out just today. So the first thing I did was test my original options

  ./autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var \
    --disable-static --enable-experimental --disable-fatal-warnings \
    --with-gtk=3.0
  make

to see what the changes did. (That was what came with the PKGBUILD I'm using. If you don't know archlinux, don't worry about it.) Now, it fails at the gtk-doc step, but not at the compile and link step, which I thought was pretty weird. It failed due to the same missing *KHR, though. I think what happened was it linked with mesa's EGL, but gtkdoc dynamically linked it with nvidia's EGL. I then disabled gtk-doc with the same options and it compiled fine. Thanks for that tip, by the way, Matthew Waters.

Now, it seems the problem is solely with gtk-doc, but I am not sure. Indeed, this may not be a problem that can be reasonably solved. The best solution may be just to disable gtkdoc when compiling on hosts that use nvidia's proprietary drivers. In any case, I have attached config.logs for both builds.
Comment 4 Eric Toombs 2014-04-16 06:06:19 UTC
Created attachment 274418 [details]
config.log - no gtkdoc

This is the config.log of the following config:
  ./autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var \
    --disable-static --enable-experimental --disable-fatal-warnings \
    --with-gtk=3.0 --disable-gtk-doc
  make
Comment 5 Eric Toombs 2014-04-16 06:15:30 UTC
Created attachment 274419 [details]
config.log - AUR vanilla

This is the config.log with the following configuration, which came from the AUR's gst-plugins-bad-git PKGBUILD[1]:
  ./autogen.sh --prefix=/usr --sysconfdir=/etc --localstatedir=/var \
    --disable-static --enable-experimental --disable-fatal-warnings \
    --with-gtk=3.0
  make


This is the error produced:

Making all in docs
make[2]: Entering directory '/home/eric/pkg/gst-plugins-bad-git/src/gst-plugins-bad/docs'
Making all in libs
make[3]: Entering directory '/home/eric/pkg/gst-plugins-bad-git/src/gst-plugins-bad/docs/libs'
  DOC   Scanning header files
  DOC   Introspecting gobjects
../../gst-libs/gst/gl/.libs/libgstgl-1.0.so: undefined reference to `eglDestroyImageKHR'
../../gst-libs/gst/gl/.libs/libgstgl-1.0.so: undefined reference to `eglCreateImageKHR'
collect2: error: ld returned 1 exit status
Linking of scanner failed: 
Makefile:1020: recipe for target 'scan-build.stamp' failed
make[3]: *** [scan-build.stamp] Error 1
make[3]: Leaving directory '/home/eric/pkg/gst-plugins-bad-git/src/gst-plugins-bad/docs/libs'

[1] https://aur.archlinux.org/packages/gst-plugins-bad-git
Comment 6 Sebastian Dröge (slomo) 2014-04-16 07:17:02 UTC
That's not a gtk-doc problem. The library has unresolved symbols, which is what needs to be fixed.

We should not unconditionally use the EGLImage related functions but dlsym() them like the other GL symbols. Then this problem would disappear.
Comment 7 Eric Toombs 2014-04-16 09:55:24 UTC
(In reply to comment #6)
> That's not a gtk-doc problem. The library has unresolved symbols, which is what
> needs to be fixed.
> 
> We should not unconditionally use the EGLImage related functions but dlsym()
> them like the other GL symbols. Then this problem would disappear.

Oh. I see. Very sensible.
Comment 8 Matthew Waters (ystreet00) 2014-05-14 08:02:58 UTC
Created attachment 276514 [details] [review]
gl/context: add generic feature checking support
Comment 9 Matthew Waters (ystreet00) 2014-05-14 08:04:11 UTC
Created attachment 276515 [details] [review]
gl/eglimage: add eglimage context feature

I have no idea if this works but it should be close ;).
Comment 10 Sebastian Dröge (slomo) 2014-05-14 08:57:42 UTC
Generally looks good, but we also need to check for glEGLImageTargetTexture2DOES I think. And IMHO check_feature() should use the actual extension name instead of something custom, just like the get_proc_address stuff.
Comment 11 Matthew Waters (ystreet00) 2014-05-16 03:03:59 UTC
glEGLImageTargetTexture2DOES is in the client API and is already checked by the gstglfeature code.  The reason I went custom is that the same functionality could be provided by different platforms and/or different extension names.  I wonder about the cost of doing both.

Alternatively, I could try and make the get_proc_address stuff work with platform features.
Comment 12 Sebastian Dröge (slomo) 2014-05-16 07:05:41 UTC
I think having a separate vfunc for that makes sense. But the extension names should be just their plain names IMHO. There's never going to be a EGLImage extension in a non-EGL platform :)
Comment 13 Matthew Waters (ystreet00) 2014-05-20 13:39:37 UTC
Ok, makes sens.

commit 37c08c58c52ea519213e723c8f69e6e061301b27
Author: Matthew Waters <ystreet00@gmail.com>
Date:   Wed May 14 17:59:52 2014 +1000

    gl/eglimage: add eglimage context feature
    
    Allows us to selectively use EGLImages only when available
    
    https://bugzilla.gnome.org/show_bug.cgi?id=728234
Comment 14 Sebastian Dröge (slomo) 2014-05-20 13:44:41 UTC
Review of attachment 276515 [details] [review]:

::: gst-libs/gst/gl/egl/gstglcontext_egl.c
@@ +415,3 @@
+    egl->eglDestroyImage = gst_gl_context_get_proc_address (context,
+        "eglDestroyImage");
+  } else if (gst_gl_check_extension ("EGL_KHR_image_base", egl_exts)) {

This needs to use eglQueryString() with EGL_EXTENSIONS, see https://www.khronos.org/registry/egl/sdk/docs/man/html/eglQueryString.xhtml
Comment 15 Sebastian Dröge (slomo) 2014-05-20 13:46:13 UTC
Actually ignore me, we do exactly that!