[vlc-devel] [PATCH 6/7] egl_display: improve support detection

[Rémi Denis-Courmont]

Le keskiviikkona 3. maaliskuuta 2021, 19.36.39 EET Romain Vimont a écrit :
> A module egl_display must only provide EGL displays where eglTerminate()
> can be called, even if other modules are using EGL (typically because it
> is internally refcounted).
> 
> This is always the case on Android, so egl_display was initially only
> enabled on Android:
> https://android.googlesource.com/platform/frameworks/native/+/master/opengl/
> libs/EGL/egl_display.cpp
> 
> But this may also be the case on other platforms which support
> EGL_KHR_display_reference:
> https://www.khronos.org/registry/EGL/extensions/KHR/EGL_KHR_display_referenc
> e.txt
> 
> Co-authored-by: Alexandre Janniaux <ajanni at videolabs.io>
> ---
>  modules/video_output/opengl/Makefile.am       |  2 +-
>  .../video_output/opengl/egl_display_generic.c | 36 +++++++++++++++++++
>  2 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/modules/video_output/opengl/Makefile.am
> b/modules/video_output/opengl/Makefile.am index 3bee347ef0..c7cbdedfb6
> 100644
> --- a/modules/video_output/opengl/Makefile.am
> +++ b/modules/video_output/opengl/Makefile.am
> @@ -92,7 +92,7 @@ endif # HAVE_GL
>  libegl_display_generic_plugin_la_SOURCES =
> video_output/opengl/egl_display_generic.c
> libegl_display_generic_plugin_la_CPPFLAGS = $(AM_CPPFLAGS) $(EGL_FLAGS)
> libegl_display_generic_plugin_la_LIBADD = $(EGL_LIBS)
> -if HAVE_ANDROID
> +if HAVE_EGL
>  vout_LTLIBRARIES += libegl_display_generic_plugin.la
>  endif
> 
> diff --git a/modules/video_output/opengl/egl_display_generic.c
> b/modules/video_output/opengl/egl_display_generic.c index
> dfa68125c4..cca13beec8 100644
> --- a/modules/video_output/opengl/egl_display_generic.c
> +++ b/modules/video_output/opengl/egl_display_generic.c
> @@ -31,11 +31,47 @@
> 
>  #include "egl_display.h"
> 
> +static EGLenum GetPlatform(const char *extensions)
> +{
> +#ifdef EGL_KHR_platform_x11
> +    if (vlc_gl_StrHasToken(extensions, "EGL_EXT_platform_x11"))
> +        return EGL_PLATFORM_X11_KHR;
> +#endif
> +
> +#ifdef EGL_KHR_platform_wayland
> +    if (vlc_gl_StrHasToken(extensions, "EGL_EXT_platform_wayland"))
> +        return EGL_PLATFORM_WAYLAND_KHR;
> +#endif
> +
> +    return 0;
> +}
> +
>  static vlc_egl_display_open_fn Open;
>  static int
>  Open(struct vlc_egl_display *display)
>  {
> +#ifdef __ANDROID__
> +    /* The default display is refcounted on Android */
>      display->display = eglGetDisplay(EGL_DEFAULT_DISPLAY);
> +#elif defined(EGL_KHR_display_reference)
> +    const char *extensions = eglQueryString(EGL_NO_DISPLAY,
> EGL_EXTENSIONS); +
> +    if (!vlc_gl_StrHasToken(extensions, "EGL_KHR_display_reference"))
> +        return VLC_EGENERIC;
> +
> +    EGLenum platform = GetPlatform(extensions);
> +    if (!platform)
> +        return VLC_EGENERIC;

It's nonobvious why you care about the platform here.
At least it needs an in-line comment IMO.

> +
> +    const EGLAttrib attribs[] = {
> +        EGL_TRACK_REFERENCES_KHR, EGL_TRUE,
> +        EGL_NONE,
> +    };
> +
> +    display->display =
> +        eglGetPlatformDisplay(platform, EGL_DEFAULT_DISPLAY, attribs);

Won't this fail badly with EGL < 1.5 ?

> +#endif
> +
>      if (display->display == EGL_NO_DISPLAY)
>          return VLC_EGENERIC;


-- 
Rémi Denis-Courmont
http://www.remlab.net/