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

Fri Feb 19 14:02:24 UTC 2021

On Fri, Feb 19, 2021, at 14:57, Steve Lhomme wrote:
> 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.

OK, then my patch 2/2 is not sufficient or correct.

> 
> 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
> > 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel