[vlc-devel] [PATCH 01/16] vlc_common: add helper function to convert milliseconds to/from mtime_t

[Steve Lhomme]

On 2018-06-09 8:03 AM, Rémi Denis-Courmont wrote:
> Le perjantaina 8. kesäkuuta 2018, 9.27.15 EEST Steve Lhomme a écrit :
>> On 2018-06-07 1:15 PM, Rémi Denis-Courmont wrote:
>>> Should there be a VLC prefix? How to handle rounding errors? :/
>> I would say yes on the prefix but that's going to be very lengthy names.
>> Also mtime_t is not vlc_mtime_t.
>> The rounding errors it's a little tricky for a macro. I was thinking
>> about using macros for hardcoded values and a function for local values.
>>
>> There are currently many static_assert on CLOCK_FREQ like this one
>> http://git.videolan.org/?p=vlc.git;a=blob;f=modules/audio_output/wasapi.c;hb
>> =HEAD#l130
>>
>> For now CLOCK_FREQ it's pretty much lock into 1000000 so the only thing
>> that can happen here is MTIME_TO_MS() overflowing when it shouldn't.
>>
>> I would like to make the code more flexible though to allow more
>> arbitrary values like that Facebook tick or using 100ns on Windows where
>> it's more natural.
> AFAIK, m refers to micro here. If you are going to change the frequency, then

Yes it's microseconds.
If we're going to change the name I propose vlc_tick_t.

> you should change the type name as well. But I doubt that changing the
> frequency is realistic.

I think that's a goal/possibility we should aim at. In an ideal world we 
could use whatever value and things would still just work. Maybe less 
than 1ms precision would probably create some issues. But that's 
probably issues that would just be more visible in this case like race 
conditions.
In an ideal world we would also have some regression tests that check 
timestamp shifting doesn't occur due to bad rounding (be it with a small 
precision clock or an odd number).

We are from that stage but I think we should aim for that.

>
> And we should keep something that is a divisor of one billion, otherwise
> conversion to/from timeval, timespec and MS becomes a mess. Since we probably
> don't want to decrease the precision, that leaves only 8 alternatives to the
> current value: 1000000 * 2^{0..3} * 5^{0..3}
>