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

[Alexandre Janniaux]

Hi,

Thank you for your review,

> but, is it possible to call SetWindowAspectRatio() and Viewport() only if the place changed ?
> Maybe the optimization is not worth it...

SetWindowAspectRatio isn't an OpenGL one, and while Viewport is, it's
only a state modification for the following OpenGL commands so I know
if it is worth it currently.

Anyway, my opengl filter branch brings a much higher cost to these kind
of update so I'm fine with adding a check for changes. I'll send a new
patch.

Regards,
--
Alexandre Janniaux
Videolabs

On Wed, Sep 25, 2019 at 10:53:50AM +0200, Thomas Guillem wrote:
> FIne with me
>
> On Tue, Sep 24, 2019, at 10:27, Alexandre Janniaux wrote:
> > 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.
>
> OK with the rational.
>
> > ---
> >  modules/video_output/opengl/display.c | 21 +++++++--------------
> >  1 file changed, 7 insertions(+), 14 deletions(-)
> >
> > diff --git a/modules/video_output/opengl/display.c
> > b/modules/video_output/opengl/display.c
> > index 456304ebdf..8d4c73cb97 100644
> > --- a/modules/video_output/opengl/display.c
> > +++ b/modules/video_output/opengl/display.c
> > @@ -73,6 +73,7 @@ struct vout_display_sys_t
> >      vout_display_opengl_t *vgl;
> >      vlc_gl_t *gl;
> >      picture_pool_t *pool;
> > +    vout_display_place_t place;
> >  };
> >
> >  /* Display callbacks */
> > @@ -208,6 +209,10 @@ static void PictureDisplay (vout_display_t *vd,
> > picture_t *pic)
> >
> >      if (vlc_gl_MakeCurrent (sys->gl) == VLC_SUCCESS)
> >      {
> > +        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);
>
> but, is it possible to call SetWindowAspectRatio() and Viewport() only if the place changed ?
> Maybe the optimization is not worth it...
>
> >          vout_display_opengl_Display (sys->vgl, &vd->source);
> >          vlc_gl_ReleaseCurrent (sys->gl);
> >      }
> > @@ -230,7 +235,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 +242,8 @@ 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);
> >          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 +251,8 @@ 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);
> >          return VLC_SUCCESS;
> >        }
> >        case VOUT_DISPLAY_CHANGE_VIEWPOINT:
> > --
> > 2.23.0
> > _______________________________________________
> > vlc-devel mailing list
> > To unsubscribe or modify your subscription options:
> > https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel