[FFmpeg-devel] [PATCH 1/6 v2] lavu: add av_gettime_relative()

[Olivier Langlois]

On Tue, 2014-05-13 at 23:23 +0200, Reimar Döffinger wrote:
> On Tue, May 13, 2014 at 03:31:04PM -0400, Olivier Langlois wrote:
> > On Tue, 2014-05-13 at 15:23 -0400, Olivier Langlois wrote:
> > > On Tue, 2014-05-13 at 20:58 +0200, Reimar Döffinger wrote:
> > > > On Mon, May 12, 2014 at 04:26:37PM -0400, Olivier Langlois wrote:
> > > > > It is very easy to experience the issues that this patch attempt to address
> > > > > by rewinding back in the past the system time while ffmpeg is running.
> > > > > 
> > > > > this is breaking the ffmpeg report printing (ffmepg.c:print_report()) and
> > > > > the the rate emulator functionality (-re) without the patch.
> > > > 
> > > > I still think these should be fixed to not be so brittle and handle
> > > > strange clock values more gracefully.
> > > 
> > > These are fixed and rock solid on platforms supporting
> > > POSIX_CLOCK_MONOTONIC (Linux, maybe macos) with part 2/6 of this patch:
> 
> Definitely not "rock solid".

You are correct. To be more accurate, I should have said, in the context
of the described test case, it is "rock solid"....

> The monotonic clock necessarily will ignore any time adjustments done
> manually by the user, so in such cases it would drift compared to the
> actual real time (obviously such adjustments are a bad idea and should
> be handled by NTP via gradual adjustments, but still).

This is not totally accurate. monotonic clock should not drift compared
to realtime because:

CLOCK_MONOTONIC definition from man page:

Clock that cannot be set and represents monotonic time since some
unspecified starting point.  This clock is not affected by discontinuous
jumps  in  the system  time  (e.g.,  if  the system administrator
manually changes the clock), but is affected by the incremental
adjustments performed by adjtime(3) and NTP.

and from 'ntpdate' man page:

If the error is less than 0.5 seconds, by default, it slews the clock's
time with the offset,  by  way of  a  call to adjtime(2).

So beside returning an arbitrary value, it has 2 properties that
realtime does not have:

1. it is monotonic
2. it does not have discontinuous jumps in the returned time

this makes it a superior choice when a clock is used to measure relative
intervals.

> There is in fact no requirement at all in the POSIX spec that
> POSIX_CLOCK_MONOTONIC is at all in sync with the real time clock,
> which could lead to ugly and surprising behaviour.
> It is not either specified how POSIX_CLOCK_MONOTONIC should behave over
> suspend.
> On systems without an RTC there is no way it can continue to count time
> during a suspend to disk.
> Even on systems with one I think it will not be adjusted for accumulated
> clock skew during the time the system was suspended.
> Whether we actually _want_ it to increase at all during suspend is a
> completely different question.
> So sorry, but IMHO this isn't anywhere even close to "rock solid".
> I'll give you "good enough for almost anyone".


The posix monotonic clock measures seconds and nanoseconds and in Linux
both monotonic and the realtime clock comes from the same system
counter. The only difference is that an offset is applied to the time in
the case of the realtime clock.

if you want to verify by yourself

http://lxr.free-electrons.com/source/kernel/time/timekeeping.c

Here is what the posix spec looks like concerning the CLOCK_MONOTONIC:

If the Monotonic Clock option is supported, all implementations shall
support a clock_id of CLOCK_MONOTONIC defined in <time.h>. This clock
represents the monotonic clock for the system. For this clock, the value
returned by clock_gettime() represents the amount of time (in seconds
and nanoseconds) since an unspecified point in the past (for example,
system start-up time, or the Epoch). This point does not change after
system start-up time. The value of the CLOCK_MONOTONIC clock cannot be
set via clock_settime(). This function shall fail if it is invoked with
a clock_id argument of CLOCK_MONOTONIC.

That being said, Linux does not follow the spec where time during
suspend isn't accounted in CLOCK_MONOTONIC and this is in contradiction
with the sentence 'This point does not change after system start-up
time'.

There is an excellent article about Waking systems from suspend and
comments specifically regarding clocks:

http://lwn.net/Articles/429925/

There is another clock worth considering:

       CLOCK_BOOTTIME (since Linux 2.6.39; Linux-specific)
              Identical  to  CLOCK_MONOTONIC,  except it also includes
any time that the system is suspended.  This allows applications to get
a suspend-aware monotonic
              clock without having to deal with the complications of
CLOCK_REALTIME, which may have discontinuities if the time is changed
using settimeofday(2).

but IMHO, suspending a system during live capture and/or live streaming
is not something that should happen and if you use ffmpeg for plain
transcoding or plain playback, realtime should not matter where I am now
using av_gettime_relative() hence I think CLOCK_MONOTONIC is a better
choice just because it is more standard over the Linux specific
CLOCK_BOOTTIME.

I'll just end this argument by saying this: Yes my patch is not perfect
or fully 'rock solid' in all circumstances but:

1. It does solve a real life problem for me on embedded system with no
CMOS always rebooting in 1970 until NTP kicks in.
2. It does not cause regression on platforms not supporting monotonic
clocks.
3. ALSA is using monotonic internally. V4L2 is using monotonic as well
(http://www.spinics.net/lists/linux-media/msg53576.html). If monotonic
time was bad, all these multimedia subsystems would not convert from
realtime to monotonic clock. Beside my end-goal, once this patch is
finally accepted, it would be to optionally make some Linux specific
devices in lavd use their native monotonic ts.

While not perfect, I think that a consensus around these 3 points is
possible and hence make my patch considered as an improvement so it can
finally be accepted!

This point is important because I do not know what you had exactely in
mind when you wrote:

> > > I still think these should be fixed to not be so brittle and
handle
> > > > strange clock values more gracefully.

IMHO, while you are probably right, this is a different issue and should
not block my patch to be accepted.

> 
> > > https://ffmpeg.org/pipermail/ffmpeg-devel/2014-May/157346.html
> > > 
> > > If possible, someone could adapt av_gettime_relative() implementation to
> > > make it monotonic on Windows or some other platforms as well.
> > > 
> > How about making these 2 ffmpeg features more robust even on platforms
> > where av_gettime_relative() is not monotonic?
> > 
> > This could be done by using for the first time the new function
> > av_gettime_relative_is_monotonic() and comparing that the second time is
> > greater or equal before performing the substraction.
> 
> Assuming we are using 64 bit numbers I don't see why you'd need a
> av_gettime_relative_is_monotonic.
> Worst that can happen is that you mitigate actual bugs in the
> monotonic clock implementation.

av_gettime_relative() will only be monotonic on POSIX platforms, in case
that it is important for the user code to know beforehand,
av_gettime_relative_is_monotonic() is there. This is to make the API
more future proof and complete. This was a suggestion from Nicolas
George that I accepted:

https://ffmpeg.org/pipermail/ffmpeg-devel/2014-April/157023.html

However after having considered to use it, I realise that since it is
essentially a preprocessor defines driven function, it should either be
implemented as a macro or as an inline function. I'll look in ffmpeg
header files for similar functions to see what is the usual way to
handle that type of functions.

That is quite possible that I come up with a v3 with this small
improvement.

Greetings,
Olivier