[PATCH weston v2 03/11] timespec: Add timespec_to_msec helper

[Daniel Stone]

Hi Pekka,

On 8 March 2017 at 12:27, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> On Wed, 8 Mar 2017 13:34:18 +0200
> Pekka Paalanen <ppaalanen at gmail.com> wrote:
>
>> On Wed,  1 Mar 2017 11:34:02 +0000
>> Daniel Stone <daniels at collabora.com> wrote:
>>
>> > Paralleling timespec_to_nsec, converts to milliseconds.
>> >
>> > Signed-off-by: Daniel Stone <daniels at collabora.com>
>> > Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
>> > ---
>> >  libweston/compositor.c |  2 +-
>> >  shared/timespec-util.h | 11 +++++++++++
>> >  tests/timespec-test.c  |  9 +++++++++
>> >  3 files changed, 21 insertions(+), 1 deletion(-)
>> >
>> > v2: No changes.
>> >
>
>> > diff --git a/shared/timespec-util.h b/shared/timespec-util.h
>> > index 13948b1..c8e3cd6 100644
>> > --- a/shared/timespec-util.h
>> > +++ b/shared/timespec-util.h
>> > @@ -93,6 +93,17 @@ timespec_to_nsec(const struct timespec *a)
>> >     return (int64_t)a->tv_sec * NSEC_PER_SEC + a->tv_nsec;
>> >  }
>> >
>> > +/* Convert timespec to milliseconds
>> > + *
>> > + * \param a timespec
>> > + * \return milliseconds
>>
>> Hi,
>>
>> might want to note, that this rounds to integer milliseconds towards
>> zero. It's not round() nor floor(), it's trunc().
>>
>> Shall I just add that note when merging?

Would be good to add, yeah.

>> > + */
>> > +static inline int64_t
>> > +timespec_to_msec(const struct timespec *a)
>> > +{
>> > +   return (int64_t)a->tv_sec * 1000 + a->tv_nsec / 1000000;
>> > +}
>
> Wait, I'm wrong, aren't I?
>
> The seconds value is always taken as is, nsec value (never negative) is
> always truncated towards zero.
>
> timespec { -2, 100 } > -2.0 s but it gets rounded to -2000 ms, i.e.
> down. That's not trunc(), that's floor(). Right?

Right you are. I did take this from previous usage, but also I think
it's what we want. In almost every positive value, floor() seems like
what we want: the maximum error is 1ms (minus 1ns), and scheduling our
paint 1ms earlier means that we're strictly more likely to meet the
deadline, as there is a latency involved when waking up from a timer.
As for negative values, it doesn't really matter, as unless we're
looking at a deadline missed so badly that we're on the verge of
postponing a repaint to the next cycle, all that matters for us is
that the value is negative, not the actual value itself per se.

Cheers,
Daniel