[vlc-devel] [PATCH] vout: don't drop window size when there is no display plugins yet

[erwan.tulou at gmail.com]

On 10/12/2019 13:53, Alexandre Janniaux wrote:
> Hi,
>
> You mean before the end of the window Open module, or before
> opening the display

That's a good question.

It seems to me that the window Open module should indeed always provide 
a valid width and height in addition to window_ID.

Even if the window is not visible (and therefore the actual size may 
still be unknown), at the opening of the module, the vout window module 
already knows the resizing policy. Basically for Qt for instance, it is 
either accepting the preferred size provided by the caller (the original 
size of the video) or if --no-qt-video-autoresize, stick to the current 
size of the video widget. For simpler vout window modules, it is just 
accepting the preferred size.

At least, that allows vlc core to directly use a size that is most 
likely the size that will be required. ! This helps minimizing the 
number of resizing.

Rgds

> Regards,
> --
> Alexandre Janniaux
> Videolabs
>
> On Tue, Dec 10, 2019 at 01:09:33PM +0200, Rémi Denis-Courmont wrote:
>> I already looked at it. It's still used to set a display configuration if the window size is not yet known.
>>
>> We should probably require vout window to report a size up-front.
>>
>> Le 10 décembre 2019 12:06:38 GMT+02:00, Thomas Guillem <thomas at gllm.fr> a écrit :
>>>
>>> On Mon, Dec 9, 2019, at 20:44, Remi Denis-Courmont wrote:
>>>> Le 2019-12-09 19:12, Thomas Guillem a écrit :
>>>>> This fixes the window size that was ignored when the size was
>>> updated
>>>>> from the
>>>>> open callback of the window plugin.
>>>>>
>>>>> Fixes #22674
>>>>> ---
>>>>>   src/video_output/video_output.c  | 12 ++++++++++++
>>>>>   src/video_output/vout_internal.h |  2 ++
>>>>>   2 files changed, 14 insertions(+)
>>>>>
>>>>> diff --git a/src/video_output/video_output.c
>>>>> b/src/video_output/video_output.c
>>>>> index 659ede546ba..225e97318ad 100644
>>>>> --- a/src/video_output/video_output.c
>>>>> +++ b/src/video_output/video_output.c
>>>>> @@ -425,6 +425,10 @@ void vout_ChangeDisplaySize(vout_thread_t
>>> *vout,
>>>>>       /* DO NOT call this outside the vout window callbacks */
>>>>>       vlc_mutex_lock(&sys->display_lock);
>>>>> +
>>>>> +    sys->display_default_width = width;
>>>>> +    sys->display_default_height = height;
>>>>> +
>>>>>       if (sys->display != NULL)
>>>>>           vout_display_SetSize(sys->display, width, height);
>>>>>       vlc_mutex_unlock(&sys->display_lock);
>>>>> @@ -1606,6 +1610,13 @@ static int vout_Start(vout_thread_t *vout,
>>>>> vlc_video_context *vctx, const vout_c
>>>>>       vlc_mutex_lock(&sys->display_lock);
>>>>>       vlc_mutex_unlock(&sys->window_lock);
>>>>>
>>>>> +    /* Setup the initial size of the window, if not already forced
>>> by
>>>>> the user */
>>>>> +    if (dcfg.display.width == 0 && dcfg.display.height == 0)
>>>>> +    {
>>>>> +        dcfg.display.width = sys->display_default_width;
>>>>> +        dcfg.display.height = sys->display_default_height;
>>>>> +    }
>>>>> +
>>>> AFAICT, this makes most of vout_display_GetDefaultDisplaySize()
>>> either
>>>> dead or no-op code - which is suspicious, albeit not necessarily
>>> wrong.
>>>
>>> I'll have a look. I didn't know this function existed.
>>>
>>>>>       sys->display = vout_OpenWrapper(vout, sys->splitter_name,
>>> &dcfg,
>>>>> vctx);
>>>>>       if (sys->display == NULL) {
>>>>>           vlc_mutex_unlock(&sys->display_lock);
>>>>> @@ -1945,6 +1956,7 @@ vout_thread_t *vout_Create(vlc_object_t
>>> *object)
>>>>>       /* Display */
>>>>>       sys->display = NULL;
>>>>> +    sys->display_default_width = sys->display_default_height = 0;
>>>>>       vlc_mutex_init(&sys->display_lock);
>>>>>
>>>>>       /* Window */
>>>>> diff --git a/src/video_output/vout_internal.h
>>>>> b/src/video_output/vout_internal.h
>>>>> index 0ca81871fee..8ed3fdca4e0 100644
>>>>> --- a/src/video_output/vout_internal.h
>>>>> +++ b/src/video_output/vout_internal.h
>>>>> @@ -182,6 +182,8 @@ struct vout_thread_sys_t
>>>>>       /* Video output display */
>>>>>       vout_display_cfg_t display_cfg;
>>>>>       vout_display_t *display;
>>>>> +    unsigned        display_default_width;
>>>>> +    unsigned        display_default_height;
>>>> Those are pretty bad/confusing names, IMO. They're the actual window
>>>> size, not some default setting.
>>> OK.
>>>
>>>>>       vlc_mutex_t     display_lock;
>>>>>
>>>>>       picture_pool_t  *private_pool;
>>>> --
>>>> Rémi Denis-Courmont
>>>>
>>> _______________________________________________
>>> vlc-devel mailing list
>>> To unsubscribe or modify your subscription options:
>>> https://mailman.videolan.org/listinfo/vlc-devel
>> --
>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
>> _______________________________________________
>> 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