[vlc-devel] [PATCH] opengl: prevent opengl calls from different threads

[Rémi Denis-Courmont]

Le keskiviikkona 25. syyskuuta 2019, 17.36.19 EEST Alexandre Janniaux a écrit 
:
> Most display controls, especially sizing ones, have been made synchronous,
> while an OpenGL context should be used in only one thread at a time and
> should not interfere with the context of the control calling thread.
> It is especially harmful when the calling context is using OpenGL, like
> in interfaces.
> ---
>  modules/video_output/opengl/display.c | 30 ++++++++++++++-------------
>  1 file changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/modules/video_output/opengl/display.c
> b/modules/video_output/opengl/display.c index 456304ebdfb..cc9989ec056
> 100644
> --- a/modules/video_output/opengl/display.c
> +++ b/modules/video_output/opengl/display.c
> @@ -73,6 +73,8 @@ struct vout_display_sys_t
>      vout_display_opengl_t *vgl;
>      vlc_gl_t *gl;
>      picture_pool_t *pool;
> +    vout_display_place_t place;
> +    bool place_changed;
>  };
> 
>  /* Display callbacks */
> @@ -93,6 +95,7 @@ static int Open(vout_display_t *vd, const
> vout_display_cfg_t *cfg,
> 
>      sys->gl = NULL;
>      sys->pool = NULL;
> +    sys->place_changed = false;
> 
>      vout_window_t *surface = cfg->window;
>      char *gl_name = var_InheritString(surface, MODULE_VARNAME);
> @@ -208,6 +211,15 @@ static void PictureDisplay (vout_display_t *vd,
> picture_t *pic)
> 
>      if (vlc_gl_MakeCurrent (sys->gl) == VLC_SUCCESS)
>      {
> +        if (sys->place_changed)
> +        {
> +            float window_ar = (float)sys->place.width / sys->place.height;
> +            vout_display_opengl_SetWindowAspectRatio(sys->vgl, window_ar);
> +            vout_display_opengl_Viewport(sys->vgl, sys->place.x,
> sys->place.y,
> +                                         sys->place.width,
> sys->place.height);
> +            sys->place_changed = false;

AFAIK, Wayland requires that you update the size and position synchronously as 
a general rule. Is this so that that requirement only applies to 
vlc_gl_Resize() and not the view port?

> +        }
> +
>          vout_display_opengl_Display (sys->vgl, &vd->source);
>          vlc_gl_ReleaseCurrent (sys->gl);
>      }
> @@ -230,7 +242,6 @@ static int Control (vout_display_t *vd, int query,
> va_list ap) {
>          vout_display_cfg_t c = *va_arg (ap, const vout_display_cfg_t *);
>          const video_format_t *src = &vd->source;
> -        vout_display_place_t place;
> 
>          /* Reverse vertical alignment as the GL tex are Y inverted */
>          if (c.align.vertical == VLC_VIDEO_ALIGN_TOP)
> @@ -238,13 +249,9 @@ static int Control (vout_display_t *vd, int query,
> va_list ap) else if (c.align.vertical == VLC_VIDEO_ALIGN_BOTTOM)
>              c.align.vertical = VLC_VIDEO_ALIGN_TOP;
> 
> -        vout_display_PlacePicture(&place, src, &c);
> +        vout_display_PlacePicture(&sys->place, src, &c);
> +        sys->place_changed = true;
>          vlc_gl_Resize (sys->gl, c.display.width, c.display.height);
> -        if (vlc_gl_MakeCurrent (sys->gl) != VLC_SUCCESS)
> -            return VLC_SUCCESS;
> -        vout_display_opengl_SetWindowAspectRatio(sys->vgl,
> (float)place.width / place.height); -       
> vout_display_opengl_Viewport(sys->vgl, place.x, place.y, place.width,
> place.height);
> -        vlc_gl_ReleaseCurrent (sys->gl);
>          return VLC_SUCCESS;
>        }
> 
> @@ -252,14 +259,9 @@ static int Control (vout_display_t *vd, int query,
> va_list ap) case VOUT_DISPLAY_CHANGE_SOURCE_CROP:
>        {
>          const vout_display_cfg_t *cfg = va_arg (ap, const
> vout_display_cfg_t *); -        vout_display_place_t place;
> 
> -        vout_display_PlacePicture(&place, &vd->source, cfg);
> -        if (vlc_gl_MakeCurrent (sys->gl) != VLC_SUCCESS)
> -            return VLC_SUCCESS;
> -        vout_display_opengl_SetWindowAspectRatio(sys->vgl,
> (float)place.width / place.height); -       
> vout_display_opengl_Viewport(sys->vgl, place.x, place.y, place.width,
> place.height); -        vlc_gl_ReleaseCurrent (sys->gl);
> +        vout_display_PlacePicture(&sys->place, &vd->source, cfg);
> +        sys->place_changed = true;
>          return VLC_SUCCESS;
>        }
>        case VOUT_DISPLAY_CHANGE_VIEWPOINT:


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