[vlc-devel] [PATCH 09/10] vlc_tick: add msftime_t to express 100ns time

[Rémi Denis-Courmont]

Sure. But it means that:
1/ The conversion macros will all be only multiplication, or only division, so they will not cause intermediate overflow. If it makes you feel better, add static_assert in mtime.c.

There are issues in some parts of this patch.

2/ date_t must be used when the conversion is not to/from a multiple or divisor of CLOCK_FREQ. We cannot do magic.

Le 23 juin 2018 07:32:03 GMT+01:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
>On 2018-06-22 9:19 PM, Rémi Denis-Courmont wrote:
>> As I already pointed out, CLOCK_FREQ must remain a multiple of one 
>> million and a divisor of one billion. Too much problems with overflow
>
>> and rounding otherwise.
>
>100,000,000 is one of these.
>
>
>
>>
>> Besides POSIX, ISO and Microsoft all agree there.
>>
>> Le 22 juin 2018 14:28:08 GMT+01:00, Steve Lhomme <robux4 at ycbcr.xyz> a
>
>> écrit :
>>
>>     On 2018-06-22 3:17 PM, Romain Vimont wrote:
>>
>>         On Fri, Jun 22, 2018 at 02:53:26PM +0200, Steve Lhomme wrote:
>>
>>             Such values currently have a better resolution than
>>             vlc_tick_t Also provide conversion macros/functions
>>             to/from vlc_tick_t --- include/vlc_tick.h | 18
>>             ++++++++++++++++++ 1 file changed, 18 insertions(+) diff
>>             --git a/include/vlc_tick.h b/include/vlc_tick.h index
>>             9e7f909e149..6953ba346d4 100644 --- a/include/vlc_tick.h
>>             +++ b/include/vlc_tick.h @@ -138,6 +138,24 @@ static
>>             inline int64_t ns_from_vlc_tick(vlc_tick_t vtk) return
>vtk
>>             * INT64_C(1000000000) / CLOCK_FREQ; } +/** + * Typically
>>             Microsoft time with 100ns resolutions + */ +typedef
>>             int64_t msftime_t; +#define MSFTIME_FROM_VLC_TICK(vtk)
>>             ((msftime_t)((vtk) * (INT64_C(10000000) / CLOCK_FREQ)))
>>             +#define VLC_TICK_FROM_MSFTIME(msft) ((vlc_tick_t)((msft)
>>             / (INT64_C(10000000) / CLOCK_FREQ))) +#define
>>             MSFTIME_FROM_SEC(sec) ((msftime_t)((sec) *
>>             INT64_C(10000000))) /* seconds in msftime_t */ + +static
>>             inline vlc_tick_t vlc_tick_from_msftime(msftime_t t) +{ +
>>             return t * CLOCK_FREQ / INT64_C(10000000); 
>>
>>         The order of operations is not the same as in the macro, so
>in
>>         theory the results (in extreme cases) might be different. If
>>         10000000 is a multiple of CLOCK_FREQ, the macro
>implementation
>>         is always better (less overflows). In practice, this is
>>         probably not important. 
>>
>>
>>     It doesn't overflow because of the current value of CLOCK_FREQ.
>If
>>     CLOCK_FREQ changes (to 100,000,000 for example) then the macro is
>doing
>>     division by 0. The function is correct except it might overflow.
>>
>>     To avoid this I would like the add static asserts in the various
>>     functions added by these patches. But that means moving the code.
>Or
>>     putting the assert somewhere else (with relevant explanation).
>>
>>     We could also do some #if depending on CLOCK_FREQ > divider or
>not and
>>     do the operations the other way around in that case.
>>
>>             +} + +static inline msftime_t
>>             msftime_from_vlc_tick(vlc_tick_t vtk) +{ + return vtk *
>>             INT64_C(10000000) / CLOCK_FREQ; +} +
>>            
>/*****************************************************************************
>>             * MSTRTIME_MAX_SIZE: maximum possible size of mstrtime
>>            
>*****************************************************************************
>>             -- 2.17.0
>>            
>------------------------------------------------------------------------
>>             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
>>
>>
>> -- 
>> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez 
>> excuser ma brièveté.
>>
>>
>> _______________________________________________
>> 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

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180623/2f6718d8/attachment.html>