[vlc-devel] [PATCH] video_output: wait half the extra time we have without getting the display_lock

[Steve Lhomme]

On 2021-02-19 14:45, Thomas Guillem wrote:
> 
> 
> On Fri, Feb 19, 2021, at 14:12, Steve Lhomme wrote:
>> This is more a workaround than a clean solution.
>>
>> In an ideal world we would never wait between prepare and display, they
>> would have predictable time and we could estimate exactly when we must
>> do the prepare to be on-time for the display.
> 
> In an ideal world, most common "vout display" can handle asynchronous rendering. In that case, only the vd->prepare callback is needed, and the plugin take care of swapping at the given picture date. This is done like that with recent Android versions when using MediaCodec, cf. https://developer.android.com/reference/android/media/MediaCodec#releaseOutputBuffer(int,%20long)
> 
> vlc_clock_Wait() is called even if vd->display is null (plugin handle async swap), this is an error, that need to be fixed.

Technically it's not an error to wait for a deadline which either passed 
or will pass by the time the swap is actually done. It's just a tiny 
waste of resource.

If the time to "prepare" the data to render is faked (for example if it 
waits for a vsync to start drawing) it will also mess with the core, 
which is trying to estimate when frames will be late in order to skip 
them properly.

On Windows there's no way to schedule a swap at a given time (I just 
checked again). And on top of that the deadline handling is inaccurate 
(can be missed by more than VOUT_MWAIT_TOLERANCE).

A cleaner fix would be to way as much time in the control loop until 
it's really time to do the render of the scheduled displayed.current. 
But without accurate timers it's almost impossible. (we have the same 
problem between the prepare and display but the shorter time to wait 
makes it more accurate for Windows).

So because of those limitations this fix is a sufficient workaround for 
now. I don't mind changing the we allow the display_lock to remain 
unlocked. But IMO it should be more time, not less than the proposed 
patch, so the UI (which might block on display_lock) is as responsive as 
possible.

>>
>> That would also leave all that time to the vout control loop to process
>> more mouse events that might occur before we start the displaying.
>>
>> Looking at how we handle the deadline in Windows, it seems we sometimes
>> missing the deadline by too far and we end up having to skip the frame
>> we wanted to display (with its deadline) because now we won't have
>> enough time to render it in time.
>>
>> On 2021-02-19 12:16, Steve Lhomme wrote:
>>> Since f1bf7ce5b4480a3d38d54c7ae1d1564f0670d83f we start the rendering much
>>> earlier than before, so we have more time to do things if necessary.
>>>
>>> The problem is that for almost all the duration of the call display_lock is
>>> held. So it's blocking all other UI related events from being handled. Even if
>>> we are going to spend most of the time waiting until we can do the display.
>>>
>>> This patch waits a bit before locking the display_lock when we estimate we will
>>> have so extra time before doing the display/swap.
>>>
>>> Fixes #25479
>>> ---
>>>    src/video_output/video_output.c | 17 +++++++++++++++++
>>>    1 file changed, 17 insertions(+)
>>>
>>> diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
>>> index d04d3f4d111..c176c274334 100644
>>> --- a/src/video_output/video_output.c
>>> +++ b/src/video_output/video_output.c
>>> @@ -1119,6 +1119,23 @@ static int ThreadDisplayRenderPicture(vout_thread_sys_t *vout, bool render_now)
>>>        if (filtered->date != sys->displayed.current->date)
>>>            msg_Warn(&vout->obj, "Unsupported timestamp modifications done by chain_interactive");
>>>    
>>> +    if (!render_now)
>>> +    {
>>> +        vlc_tick_t now = vlc_tick_now();
>>> +        vlc_tick_t system_pts = vlc_clock_ConvertToSystem(sys->clock, now, filtered->date, sys->rate);
>>> +        if (unlikely(system_pts != INT64_MAX))
>>> +        {
>>> +            const vlc_tick_t late = now - system_pts;
>>> +            if (unlikely(late <= 0))
>>> +            {
>>> +                // wait half the extra time we have without the display_lock
>>> +                vlc_tick_t max_time_to_render = sys->render.avg;
>>> +                vlc_clock_Wait(sys->clock, now , filtered->date - max_time_to_render / 2, sys->rate,
>>> +                            VOUT_REDISPLAY_DELAY);
>>> +            }
>>> +        }
>>> +    }
>>> +
>>>        vout_display_t *vd = sys->display;
>>>    
>>>        vlc_mutex_lock(&sys->display_lock);
>>> -- 
>>> 2.29.2
>>>
>>> _______________________________________________
>>> 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
>