[vlc-devel] [PATCH] vout: fix low framerate stuttering

[Steve Lhomme]

> On 6 Aug 2019, at 16:36, Romain Vimont <rom1v at videolabs.io> wrote:
> 
>> On Tue, Aug 06, 2019 at 03:58:54PM +0200, Steve Lhomme wrote:
>> It seems that you are overriding more cases than the "refresh" one. What are
>> the consequences when first and drop_next_frame are false ?
> 
> (you mean when first or drop_next_frame are _true_)

Yes

> In those cases, Thread() will wait until the deadline written (if it is
> lower than 100ms) instead of 100ms (which is arbitrary).
> 
> For "first", it seems better (the following frame can be before 100ms).
> 
> For "drop_next_frame", I don't know: waiting 100ms or until the next
> frame date before calling ThreadDisplayPicture() again is arbitrary.
> Moreover, it seems that the current code immediately renders the next
> frame (whatever its date) if drop_next_frame is set anyway (so dates are
> not respected).
> 
> I did it that way because it made the code more straightforward, but
> this is a weak reason. Would it be better to keep 100ms if
> drop_next_frame is true?

Given I don't know that code much I have no opinion. The rest LGTM.

>>> On 2019-08-06 15:07, Romain Vimont wrote:
>>> In ThreadDisplayPicture(), when "refresh" was true, the output parameter
>>> deadline was not written and the function returned a non-zero value.
>>> 
>>> As a consequence, in video_output.c:Thread(), the next loop iteration
>>> waited for the max deadline (100ms). When the following frame target
>>> date was before this deadline, the video was stuttering.
>>> 
>>> To avoid the problem, write the deadline before returning from
>>> ThreadDisplayPicture(), so that Thread() does not wait more than
>>> expected.
>>> 
>>> Since an existing frame is refreshed only every 80ms
>>> (VOUT_REDISPLAY_DELAY), this happened only on low framerate videos
>>> (<12.5 fps). Otherwise, "refresh" was always false and the problem never
>>> occurred.
>>> ---
>>> You can test with this video at 10fps: https://tmp.rom1v.com/10fps.mp4
>>> 
>>>  src/video_output/video_output.c | 13 +++++++------
>>>  1 file changed, 7 insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
>>> index 4a2cde9f98..ef0d9488ae 100644
>>> --- a/src/video_output/video_output.c
>>> +++ b/src/video_output/video_output.c
>>> @@ -1243,13 +1243,14 @@ static int ThreadDisplayPicture(vout_thread_t *vout, vlc_tick_t *deadline)
>>>      }
>>>      bool force_refresh = !drop_next_frame && refresh;
>>> +    if (!frame_by_frame) {
>>> +        if (date_refresh != VLC_TICK_INVALID)
>>> +            *deadline = date_refresh;
>>> +        if (date_next != VLC_TICK_INVALID && date_next < *deadline)
>>> +            *deadline = date_next;
>>> +    }
>>> +
>>>      if (!first && !refresh && !drop_next_frame) {
>>> -        if (!frame_by_frame) {
>>> -            if (date_refresh != VLC_TICK_INVALID)
>>> -                *deadline = date_refresh;
>>> -            if (date_next != VLC_TICK_INVALID && date_next < *deadline)
>>> -                *deadline = date_next;
>>> -        }
>>>          return VLC_EGENERIC;
>>>      }
>>> -- 
>>> 2.23.0.rc1
>>> 
>>> _______________________________________________
>>> 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