[vlc-devel] [PATCH 2/2] egl: tune eglTerminate behaviour with display_reference availability

[Alexandre Janniaux]

Hi,

For the record:

It's currently a compile time constant, but it's ought to be a
conditionnal settings, checking whether the extension is available
and if we can create a display with the attribute set. I took the
opportunity in this patch to document what should be done, as I
am mostly fixing the use case for the wayland integration currently
and don't have this extension on my personal computer for now.

Meanwhile, there are also implementation which seems to support
EGL_TRACK_REFERENCE_KHR while not exposing the
EGL_KHR_display_reference. I'm not really sure now what to do in
this case, but we could try opening a display with EGL_TRACK_REFERENCE
and query the display attribute afterward, which is not compile time.

I'll move it into a compile time constant without the additional
documentation.

Regards,
--
Alexandre Janniaux
Videolabs

On Wed, Sep 25, 2019 at 11:43:20AM +0300, Rémi Denis-Courmont wrote:
> Hi,
>
> If it's a compile-time constant, better keep it a compile time constant than store it in memory IMO.
>
> Le 25 septembre 2019 11:24:40 GMT+03:00, Alexandre Janniaux <ajanni at videolabs.io> a écrit :
> >EGL_KHR_display_reference specifies the behaviour of eglTerminate
> >with regards to reference counting. In addition, it specifies that
> >the default is that reference counting is disabled X11 or wayland,
> >but enabled on Android.
> >
> >As we can prevent EGL GL provider from terminating its state, it
> >makes sure the state is correctly refcounted in any case.
> >---
> > modules/video_output/opengl/egl.c | 23 +++++++++++++++++++++--
> > 1 file changed, 21 insertions(+), 2 deletions(-)
> >
> >diff --git a/modules/video_output/opengl/egl.c
> >b/modules/video_output/opengl/egl.c
> >index 6d6ab05738f..dd8e0cf9887 100644
> >--- a/modules/video_output/opengl/egl.c
> >+++ b/modules/video_output/opengl/egl.c
> >@@ -58,6 +58,7 @@ typedef struct vlc_gl_sys_t
> >     PFNEGLDESTROYIMAGEKHRPROC   eglDestroyImageKHR;
> >
> >     bool prevent_terminate;
> >+    bool has_display_reference;
> > } vlc_gl_sys_t;
> >
> > static int MakeCurrent (vlc_gl_t *gl)
> >@@ -188,8 +189,10 @@ static void Close(vlc_gl_t *gl)
> >         if (sys->surface != EGL_NO_SURFACE)
> >             eglDestroySurface(sys->display, sys->surface);
> >
> >-        /* Kill the egl state only if we own it. */
> >-        if (!sys->prevent_terminate)
> >+        /* If has_display_reference == true, sys->display is
> >refcounted by
> >+         * eglInitialize and eglTerminate, so call it anyway.
> >+         * Otherwise, kill the egl state only if we own it. */
> >+        if (sys->has_display_reference || !sys->prevent_terminate)
> >             eglTerminate(sys->display);
> >         eglReleaseThread();
> >     }
> >@@ -224,6 +227,7 @@ static int Open(vlc_gl_t *gl, const struct gl_api
> >*api,
> >     sys->surface = EGL_NO_SURFACE;
> >     sys->eglCreateImageKHR = NULL;
> >     sys->eglDestroyImageKHR = NULL;
> >+    sys->has_display_reference = false;
> >
> >     vout_window_t *wnd = gl->surface;
> >EGLSurface (*createSurface)(EGLDisplay, EGLConfig, void *, const EGLint
> >*)
> >@@ -234,6 +238,21 @@ static int Open(vlc_gl_t *gl, const struct gl_api
> >*api,
> >      * the associated EGL display. */
> >    sys->prevent_terminate = var_GetBool(wnd, "egl-prevent-terminate");
> >
> >+    /* See
> >https://www.khronos.org/registry/EGL/extensions/KHR/EGL_KHR_display_reference.txt
> >*/
> >+    /* If EGL_KHR_display_reference is not present, eglTerminate will
> >+     * terminate the EGLDisplay which is shared between all clients.
> >+     * This extension turns the behaviour of eglTerminate into
> >reference
> >+     * counting mode and prevents this issues.
> >+     * TODO: currently, we're using the default behaviour of this
> >extension. */
> >+#ifdef USE_PLATFORM_ANDROID
> >+    sys->has_display_reference = true; /* enforced by Android */
> >+#else
> >+    /* If we don't conditionnaly add the EGL_TRACK_REFERENCE_KHR,
> >EGL_TRUE
> >+     * parameter to display query parameters, the default is false for
> >other
> >+     * platforms. */
> >+    sys->has_display_reference = false;
> >+#endif
> >+
> > #ifdef USE_PLATFORM_X11
> >     sys->x11 = NULL;
> >
> >--
> >2.23.0
> >
> >_______________________________________________
> >vlc-devel mailing list
> >To unsubscribe or modify your subscription options:
> >https://mailman.videolan.org/listinfo/vlc-devel
>
> --
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.

> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel