[Bug 740753] vaapisink: add support for EGL

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=740753

sreerenj <***@gmail.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Component|general |gstreamer-vaapi
Assignee|gstreamer-vaapi-***@gnome |gstreamer-***@lists.freede
|.bugs |sktop.org
Product|gstreamer-vaapi |GStreamer
Target Milestone|--- |git master
QA Contact|gstreamer-vaapi-***@gnome |gstreamer-***@lists.freede
|.bugs |sktop.org
Severity|normal |enhancement

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=740753

Daniel van Vugt <***@canonical.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@canonical.c
| |om

--- Comment #3 from Daniel van Vugt <***@canonical.com> ---
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.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=740753

Julien Isorce <***@gmail.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@gmail.com

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=740753

Hyunjun Ko <***@igalia.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@igalia.com

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=740753

--- Comment #4 from Daniel van Vugt <***@canonical.com> ---
Created attachment 356285
--> https://bugzilla.gnome.org/attachment.cgi?id=356285&action=edit
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.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=740753

--- Comment #5 from Daniel van Vugt <***@canonical.com> ---
Created attachment 356349
--> https://bugzilla.gnome.org/attachment.cgi?id=356349&action=edit
Working EGL platform second prototype

This version of the patch aims to be a bit more correct and safer.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=740753

--- Comment #9 from Daniel van Vugt <***@canonical.com> ---
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.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=740753

Daniel van Vugt <***@canonical.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #356349|0 |1
is obsolete| |

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=740753

--- Comment #12 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
Hi Daniel

(In reply to Daniel van Vugt from comment #9)

Post by "GStreamer" (GNOME Bugzilla)
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).

Post by "GStreamer" (GNOME Bugzilla)
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.

Post by "GStreamer" (GNOME Bugzilla)
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
:)

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=740753

--- Comment #7 from Daniel van Vugt <***@canonical.com> ---
Created attachment 356704
--> https://bugzilla.gnome.org/attachment.cgi?id=356704&action=edit
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.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=740753

Daniel van Vugt <***@canonical.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #356704|0 |1
is obsolete| |

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=740753

--- Comment #8 from Víctor Manuel Jáquez Leal <***@igalia.com> ---
@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?

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=740753

--- Comment #11 from Daniel van Vugt <***@canonical.com> ---
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.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=740753

--- Comment #10 from Daniel van Vugt <***@canonical.com> ---
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.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=740753

--- Comment #13 from Daniel van Vugt <***@canonical.com> ---
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.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=740753

GStreamer system administrator <***@gstreamer.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |OBSOLETE

--- Comment #14 from GStreamer system administrator <***@gstreamer.net> ---
-- 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.