[vlc-devel] [PATCH 04/15] vlc_common: add helper function for explicit nanosecond to/from mtime_t conversion

[Steve Lhomme]

On 2018-06-16 10:53 AM, Rémi Denis-Courmont wrote:
> Le lauantaina 16. kesäkuuta 2018, 11.32.50 EEST Steve Lhomme a écrit :
>> On 2018-06-16 10:23 AM, Rémi Denis-Courmont wrote:
>>> Le lauantaina 16. kesäkuuta 2018, 10.34.18 EEST Steve Lhomme a écrit :
>>>> I used milli instead of ms so it's not confused with msftime_t. And so
>>>> micro and nano seems like logical siblings.
>>> Then keep mtime and screw this series.
>> I find mtime misleading. Especially because it assumes a certain value
>> and in many parts of the code 1000 or 1000000 is used instead of proper
>> conversion because what could possibly go wrong ?
> Well, these things regularly go wrong:
> - failing to convert, typically the caching values between ms and µs,
> - failing to expand before computation, typically from 32-bits to 64-bits.
>
> mtime_t is not a particularly good name, but it's anyway assumed same as
> int64_t all over the place, so the name does not matter than much.
>
>> With a tick (or something else) you know you have to take care of a
>> conversion in the proper time format you're handling.
> But that's exactly the point. If you use tick, jiffy or whatever other non-
> standard unit, then you have to define what the other unit is.

I see it the other way. One should not know what a tick is nor the 
precision it can have. The API around it should hide all the 
intricacies. The fact the name implies it's in microseconds lead to a 
lot of dirty code we have right now.

> That's why ns, us/µs, ms are far better names than nano, micro and milli.

I would like nsec, msec and µsec but it's not possible and using ms may 
lead to confusion. And us doesn't sound anything like µs it just looks 
the same.
>
>> mtime_t will stay for backward compatibility but IMO it should not be
>> used anymore.
> I don't mind changing the name to something short and correctly namespaced.
> But it would be utterly vain in any case. The code assumes int64_t in a
> variety of subtle and not-so-subtle ways, no matter the typedef alias.

This is all fixed. See https://code.videolan.org/robUx4/vlc/tree/clock/43

>
>>> As far as I am concerned, if you don't specify the time unit in a computer
>>> program, you are using local jiffies, not seconds. Talk about confusing.
>>>
>>> ISO C uses nsec, not nano. SI uses ms, µs and ns with us as the defacto
>>> standard transliteration of µs.
>> We could use nsec instead of nano to align with the standard. But then
>> msec could be either milliseconds or microseconds.
> Not really. VLC did confuse m for micro. But in the standards, it's always u/µ
> for micro and m for milli.
>

I don't like it. But if everyone agrees on this fine. I can change that 
easily in my patches.