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 740753 - vaapisink: add support for EGL
vaapisink: add support for EGL
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gstreamer-vaapi
git master
Other Linux
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 736713 758397
 
 
Reported: 2014-11-26 14:29 UTC by Gwenole Beauchesne
Modified: 2018-11-03 15:46 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Working EGL platform first prototype (4.60 KB, patch)
2017-07-24 10:32 UTC, Daniel van Vugt
none Details | Review
Working EGL platform second prototype (5.35 KB, patch)
2017-07-25 10:12 UTC, Daniel van Vugt
none Details | Review
Third working prototype (7.28 KB, patch)
2017-08-01 09:33 UTC, Daniel van Vugt
none Details | Review

Description Gwenole Beauchesne 2014-11-26 14:29:26 UTC
Add support for EGL and provide a new "use-egl" property to enable that. This replaces support for GLX, which is now obsolete and removed.
Comment 1 Víctor Manuel Jáquez Leal 2015-09-07 13:29:42 UTC
Right now there's some support for EGL in gstreamer-vaapi, but it is not complete or either working right.

As said in the IRC:

2015-09-07 13:57:05   vliaskov   hi ceyusa should vaapi detect egl context here automatically? GST_GL_WINDOW=x11 GST_GL_API=opengl GST_GL_PLATFORM=egl playbin uri=file:///opt/test.mp4 video-sink=glimagesink ? It is trying to use glx context and segfaults:  http://pastebin.com/qgC8g0Ea

From the backtrace

gl_create_context (dpy=0x7fffe0021010, screen=0, parent=0x7fffc591ebe0) at gstvaapiutils_glx.c:315
315         screen = DefaultScreen (parent->display);
(gdb) bt
  • #0 gl_create_context
    at gstvaapiutils_glx.c line 315
  • #1 create_objects
    at gstvaapitexture_glx.c line 138
  • #2 create_texture_unlocked
    at gstvaapitexture_glx.c line 177
  • #3 gst_vaapi_texture_glx_create
    at gstvaapitexture_glx.c line 186
  • #4 gst_vaapi_texture_allocate
    at gstvaapitexture.c line 61
  • #5 gst_vaapi_texture_new_internal
    at gstvaapitexture.c line 81
  • #6 gst_vaapi_texture_glx_new_wrapped
    at gstvaapitexture_glx.c line 289
  • #7 gst_vaapi_texture_upload
    at gstvaapivideometa_texture.c line 187
  • #8 _do_upload_with_meta
    at gstglupload.c line 604

It looks also that the EGL texture upload code path is not used at all!!
Comment 2 Víctor Manuel Jáquez Leal 2015-09-07 13:32:08 UTC
This bug is for vaapisink, not for the decoder.

Sorry for the noise. I'll open a new bug.
Comment 3 Daniel van Vugt 2017-07-21 06:55:10 UTC
I'm looking at this bug at the moment, although if someone could beat me to a fix that would be excellent.

It appears there are two initial problems:

1. The EGL texture upload code path is not used (at least not with the clutter video sink) because vaapipostproc deletes that cap:

   if ((GST_VAAPI_PLUGIN_BASE_SRC_PAD_CAN_DMABUF (postproc)
           || !gst_vaapi_display_has_opengl (GST_VAAPI_PLUGIN_BASE_DISPLAY
               (postproc)))
       && gl_upload_meta_idx > -1) {
     gst_caps_remove_structure (caps, gl_upload_meta_idx);
   }

2. create_objects (GstVaapiTextureEGL * texture, GLuint texture_id) fails in a couple of places with EGL_BAD_PARAMETER. This seems to be because the texture_id is from the downstream GL context (the thread that's doing the upload), and gstreamer-vaapi is using some other completely wrong GL context. So EGL reports EGL_BAD_PARAMETER because texture_id is not valid for the context we're trying to use.
Comment 4 Daniel van Vugt 2017-07-24 10:32:48 UTC
Created attachment 356285 [details] [review]
Working EGL platform first prototype

Here's a prototype of working EGL support, developed against 1.12 and under Wayland (this also fixes bug 784369). It needs more work, so maybe others can step in and help.

Here are the three main fixes being applied:

gstvaapipostproc.c: Don't drop GL texture upload support. The below changes make it work, and besides, if you drop texture upload support then you could be left with a tiled buffer that then gets uploaded linearly in clutter-gst causing visual corruption (bug 784369).

gstvaapitexture_egl.c: Make GL texture -> EGLImage -> GEM name -> VA surface binding work. So now libVA renders directly into the GL texture supplied by the application. FIXME: No portable way to discover the true buffer size, which is risky :(

gstvaapitexture_egl.c: Set the correct texture->egl_context (that of the calling thread) so that texture_id is valid and doesn't get rejected by eglCreateImageKHR.

With this patch totem and gst-launch-1.0 using vaapidecodebin with either clutterautovideosink/glimagesink now work, incurring very low CPU.
Comment 5 Daniel van Vugt 2017-07-25 10:12:41 UTC
Created attachment 356349 [details] [review]
Working EGL platform second prototype

This version of the patch aims to be a bit more correct and safer.
Comment 6 Víctor Manuel Jáquez Leal 2017-07-27 09:19:40 UTC
(In reply to Daniel van Vugt from comment #5)
> Created attachment 356349 [details] [review] [review]
> Working EGL platform second prototype
> 
> This version of the patch aims to be a bit more correct and safer.

This version obsoletes the other one? To mark the first one as "obsolete"
Comment 7 Daniel van Vugt 2017-08-01 09:33:43 UTC
Created attachment 356704 [details] [review]
Third working prototype

This version removes the possibility of buffer overruns. Instead of guessing that it's OK to overestimate the buffer size, we now get the actual GEM buffer size via ioctl. A new guess is required after that still to work around an i965 driver bug, but the new one only rounds down so won't ever cause an overrun.

It's still not pretty. We don't have a decisive way to determine the correct DRM fd to use. But this version is safer.
Comment 8 Víctor Manuel Jáquez Leal 2017-08-01 14:56:52 UTC
@Daniel,

Thanks for your patches! If I understand them correctly, they re-enable GLUploadTexture in EGL.  But this bug is completely unrelated to it: the bug is to make vaapisink to work using EGL. (That's why your patches stumbled me a bit).

If my understanding is correct would you mind to open a new bug and post your patches in there?
Comment 9 Daniel van Vugt 2017-08-02 02:05:08 UTC
I do already have another bug describing my main motivation. See bug 784369.

But re-enabling GLUploadTexture also solves a Wayland-specific portion of bug 783850.

It's a patch that's solving multiple problems, and I think this bug is still related. The EGL platform in gstreamer-vaapi seemingly never worked and the above patch makes the EGL code path work. Hence I chose this bug. But if you really want then I can move to bug 784369 instead.
Comment 10 Daniel van Vugt 2017-08-02 03:18:27 UTC
If it helps to make the patch more relevant to this bug then I can also look at adding the aforementioned use-egl property. Presumably that would aid testing on X11 too.
Comment 11 Daniel van Vugt 2017-08-02 07:21:04 UTC
I'm revising the patch again. Turns out the ugly part has been solved upstream in intel-vaapi-driver 1.8.4 and later:
  https://github.com/01org/intel-vaapi-driver/issues/222

So when that fix is more widely distributed I'll be able to delete the ugly GEM sizing hack that worked around it.
Comment 12 Víctor Manuel Jáquez Leal 2017-08-02 13:49:44 UTC
Hi Daniel

(In reply to Daniel van Vugt from comment #9)
> I do already have another bug describing my main motivation. See bug 784369.

I see. Your motivation is to improve the experience of totem using gstreamer-vaapi under Wayland. Definitely there is a ton of things to do. For example, as you mentioned, there are incompatibilities among gstreamer-gl and gstreamer-vaapi that we need to fix.

Nonetheless, the issue discussed in this bug report is related with vaapisink, which is not used by Totem, as you know. Totem uses clutter-gst.

What I would recommend you is to create a new bug issue where to re-enable GLUploadTexture for Wayland in gstreamer-vaapi (something that worked in the past, but now it is deprecated).

> But re-enabling GLUploadTexture also solves a Wayland-specific portion of
> bug 783850.

Please see bug 773453

There is discussed how to enable GLUploadTexture in cluttergst. The other side of the equation.

As you found in bug 779965 and 779966, GLUploadTexture is considered obsolete because gstreamer-gl offers a better mechanism to transfer images to textures using GstGLMemory.

But, 1\ clutter-gst  doesn't use gtreamer-gl and 2\ gstreamer-vaapi still doesn't support GstGLMemory (bug 784207),

Still, there is another option: dmabuf (bug 759209), which is supported by gstreamer-vaapi, but it hasn't been merged into clutter-gst.


> It's a patch that's solving multiple problems, and I think this bug is still
> related. The EGL platform in gstreamer-vaapi seemingly never worked and the
> above patch makes the EGL code path work. Hence I chose this bug. But if you
> really want then I can move to bug 784369 instead.

It worked somewhat, but it never got finished. Until the GLUploadMeta was superseded by dmabuf in the specific case of EGL.

My proposal is, if you wish, you can help by solving the problem of gstreamer-gl/gstreamer-vaapi display instantiation mismatch, or merging dmabuf support in clutter-gst and clean up the rough corners. Or help with bug 784207 :)
Comment 13 Daniel van Vugt 2017-08-03 02:25:50 UTC
Thanks.

I will move my existing patchwork to bug 773453. If still nothing better exists soon we'll be distro-patching it into Ubuntu 17.10. But the work in bug 759209 certainly sounds like a good way forward in the long term.
Comment 14 GStreamer system administrator 2018-11-03 15:46:08 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gstreamer-vaapi/issues/24.