[libcamera-devel] [PATCH] libcamera: buffer: Improve timestamp documentation

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

> The Buffer timestamp documentation is short and does not explain that
> the timestamp is based on a monotonic time source, as opposed to the
> real/wall-time clocks available in the system.
> 
> Expand upon the statements to detail that it can not be used directly as
> a wall-clock for the captured date/time without a further reference
> point, and that by taking a reference point - the user will be bringing
> in potential inconsistencies which must be considered.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  src/libcamera/buffer.cpp | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp
> index a094737d0392..c08ca4234b29 100644
> --- a/src/libcamera/buffer.cpp
> +++ b/src/libcamera/buffer.cpp
> @@ -92,7 +92,13 @@ LOG_DEFINE_CATEGORY(Buffer)
>   * The timestamp is expressed as a number of nanoseconds relative to the system
>   * clock since an unspecified time point.
>   *
> - * \todo Be more precise on what timestamps refer to.
> + * This timestamp is monotonic and not affected by changes in system time,
> + * however it is still susceptible to small adjustments made by NTP.
> + *
> + * This clock has no specification on its time base, so can not be directly be
> + * converted to a 'real' wall clock time without a comparable reference point.
> + * However, continued conversions from the reference point, may bring in
> + * inaccuracies due to changes on the real time clock which must be considered.

I'd keep a todo here, as we still haven't decided what clock source is
the most suitable. Describing what is being done today is of course a
good idea, but one issue is that we get the timestamp directly from
V4L2, and V4L2 doesn't guarantee what timestamp source is used. I've had
a look at Linus' master branch, and we have, in drivers/media/,

    103 ktime_get
      6 ktime_get_boottime
      6 ktime_get_real

ktime_get_boottime is only used in DVB, so we're fine there.
ktime_get_real is used by

drivers/media/pci/ttpci/av7110.c:       ktime_get_real_ts64(&ts);
drivers/media/rc/streamzap.c:           sz->signal_start = ktime_get_real();
drivers/media/rc/streamzap.c:   sz->signal_start = ktime_get_real();
drivers/media/test-drivers/vivid/vivid-rds-gen.c:                       time64_to_tm(ktime_get_real_seconds(), 0, &tm);
drivers/media/test-drivers/vivid/vivid-vbi-gen.c:       time64_to_tm(ktime_get_real_seconds(), 0, &tm);
drivers/media/usb/uvc/uvc_video.c:              return ktime_get_real();

UVC supports selecting either the monotonic or real clock, with the
monotonic clock being the default, so I think we're fine too.

We can thus assume for the time being that we only get CLOCK_MONOTONIC
timestamp. Should we simplify the doc by just telling we use
CLOCK_MONOTONIC, with a todo comment to revisit the topic and possibly
select a different clock, or make the clock source configurable ?

>   */
>  
>  /**

-- 
Regards,

Laurent Pinchart