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

[Steve Lhomme]

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
>