[vlc-devel] [PATCH] video_output: rename next_system_pts

[Steve Lhomme]

On 2021-02-04 11:19, Alexandre Janniaux wrote:
> On Thu, Feb 04, 2021 at 10:56:36AM +0100, Steve Lhomme wrote:
>> On 2021-02-04 10:47, Alexandre Janniaux wrote:
>>> Hi,
>>>
>>> On Thu, Feb 04, 2021 at 10:31:34AM +0100, Steve Lhomme wrote:
>>>> On 2021-02-04 10:23, Alexandre Janniaux wrote:
>>>>> Hi,
>>>>>
>>>>> I'm not sure system_current_pts is less confusing than
>>>>> next_system_pts. Maybe a variation including the words
>>>>> «system» to indicate the reference, and «deadline» to
>>>>> indicate that the timestamp must be reached is better?
>>>>>
>>>>> Maybe something like system_render_deadline ?
>>>>
>>>> There's no indication this system vlc_tick_t is for the current of the next
>>>> picture, which is what this patch is trying to fix.
>>>>
>>>> I also dislike the word deadline in general. I only kept it for
>>>> compatibility. It's not a time that the rendering should happen any time
>>>> before that. It's actually the exact time the display swap should occur.
>>>>
>>>> How about "system_current_swap_pts" ?
>>>
>>> Actually, it might be easier to find an alternative if
>>> I indicate why I don't think it's less confusing.
>>>
>>> Basically you wrote:
>>>
>>>> It is a confusing name as it's now the system PTS of displayed.current.
>>>
>>> But this date is still likely in the future, so I find that
>>> using «current» here is more confusing than using «next».
>>
>> Not with "system_current_swap_pts", it is the swap time that is currently
>> set to be used. It's also the system PTS of the current picture.
> 
> Documentation-wise, it makes sense, but if you mention confusion,
> it's still confusing with "the time when the swap has happened"
> because of "current".
> 
> I agree that deadline has some unwanted semantic, but I don't
> feel like what you suggest is clearer than the current code.

OK, forget about this patch then.

> Regards,
> --
> Alexandre Janniaux
> Videolabs
> 
>>>>> Regards,
>>>>> --
>>>>> Alexandre Janniaux
>>>>> Videolabs
>>>>>
>>>>> On Thu, Feb 04, 2021 at 09:46:50AM +0100, Steve Lhomme wrote:
>>>>>> It is a confusing name as it's now the system PTS of displayed.current.
>>>>>>
>>>>>> Also rework the late test to be more readable.
>>>>>> ---
>>>>>>     src/video_output/video_output.c | 7 +++----
>>>>>>     1 file changed, 3 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/src/video_output/video_output.c b/src/video_output/video_output.c
>>>>>> index 323b40c0846..cc24a74a5a7 100644
>>>>>> --- a/src/video_output/video_output.c
>>>>>> +++ b/src/video_output/video_output.c
>>>>>> @@ -1521,13 +1521,12 @@ static int ThreadDisplayPicture(vout_thread_sys_t *vout, vlc_tick_t *deadline)
>>>>>>             }
>>>>>>             else if (!paused)
>>>>>>             {
>>>>>> -            const vlc_tick_t next_system_pts =
>>>>>> +            const vlc_tick_t system_current_pts =
>>>>>>                     vlc_clock_ConvertToSystem(sys->clock, system_now,
>>>>>>                                               sys->displayed.current->date, sys->rate);
>>>>>> -            if (likely(next_system_pts != INT64_MAX))
>>>>>> +            if (likely(system_current_pts != INT64_MAX))
>>>>>>                 {
>>>>>> -                vlc_tick_t date_next = next_system_pts - render_delay;
>>>>>> -                if (date_next <= system_now)
>>>>>> +                if (system_now + render_delay >= system_current_pts)
>>>>>>                     {
>>>>>>                         // the current frame will be late, look for the next not late one
>>>>>>                         next =
>>>>>> --
>>>>>> 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
>>>
>> _______________________________________________
>> 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
>