[vlc-devel] [PATCH] opengl: prevent opengl calls from different threads
Alexandre Janniaux
ajanni at videolabs.io
Tue Oct 1 15:22:43 CEST 2019
Indeed, I hope we can find a way to avoid having ten different
OpenGL display with similar features given how OpenGL output is
already splitted into different modules.
Thank you for noting this.
Regards,
--
Alexandre Janniaux
Videolabs
On Tue, Oct 01, 2019 at 11:06:48AM +0200, Steve Lhomme wrote:
> OK for me too. iOS seem to have a local bool for that as well (doublon ?).
>
> In glwin32 the vout_display_opengl_SetWindowAspectRatio() /
> vout_display_opengl_Viewport() is done every time during Prepare.
>
> On 2019-10-01 10:25, Thomas Guillem wrote:
> > Not from me, I'm OK for a push.
> >
> > On Tue, Oct 1, 2019, at 09:28, Alexandre Janniaux wrote:
> > > Hi,
> > >
> > > While it was the opportunity to discuss broader subjects for the
> > > future, this patch is quite really simple and currently only fix
> > > bad-citizen behaviour, especially when trying to work on video
> > > integration.
> > >
> > > Are they any more remarks?
> > >
> > > Regards,
> > > --
> > > Alexandre Janniaux
> > > Videolabs
> > >
> > > On Wed, Sep 25, 2019 at 04:36:19PM +0200, 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.
> > > > ---
> > > > 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;
> > > > + }
> > > > +
> > > > 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:
> > > > --
> > > > 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
> >
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
More information about the vlc-devel
mailing list