Re: [PATCH 3/4] media: uvcvideo: Relax the constrains for interpolating the hw clock

[Ricardo Ribalda]

Hi Laurent

On Mon, 11 May 2026 at 17:51, Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>
> On Mon, Mar 23, 2026 at 01:10:30PM +0000, Ricardo Ribalda wrote:
> > In the initial version we set the min value to 250msec. Looks like
> > 100msec can also provide a good value.
>
> I'd like to know where the value comes from and how it has been tested.

I used the Android CTS framework for testing. It checks in multiple
places that the timestamps are stable.

>
> > Now that we are at it, refactor a bit the code to make it cleaner.
>
> Do you mean using a macro ? You can mention that explicitly here.
>
> > Fixes: 6243c83be6ee8 ("media: uvcvideo: Allow hw clock updates with buffers not full")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
> > ---
> >  drivers/media/usb/uvc/uvc_video.c | 18 +++++++++++-------
> >  1 file changed, 11 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> > index c7ebedb3450f..dcbc0941ffe6 100644
> > --- a/drivers/media/usb/uvc/uvc_video.c
> > +++ b/drivers/media/usb/uvc/uvc_video.c
> > @@ -494,6 +494,13 @@ static int uvc_commit_video(struct uvc_streaming *stream,
> >   * Clocks and timestamps
> >   */
> >
> > +/*
> > + * The accuracy of the hardware timestamping depends on having enough data to
> > + * interpolate between the different clock domains. This value is sof cycles,
> > + * this is, milliseconds.
> > + */
> > +#define MIN_HW_TIMESTAMP_DIFF 100
>
> UVC prefix.
>
> > +
> >  static inline ktime_t uvc_video_get_time(void)
> >  {
> >       if (uvc_clock_param == CLOCK_MONOTONIC)
> > @@ -834,15 +841,12 @@ void uvc_video_clock_update(struct uvc_streaming *stream,
> >               y2 += 2048 << 16;
> >
> >       /*
> > -      * Have at least 1/4 of a second of timestamps before we
> > -      * try to do any calculation. Otherwise we do not have enough
> > -      * precision. This value was determined by running Android CTS
> > -      * on different devices.
> > +      * Check that we have enough data to do the interpolation.
> >        *
> > -      * dev_sof runs at 1KHz, and we have a fixed point precision of
> > -      * 16 bits.
> > +      * y1 and y2 are dev_sof with a fixed point precision of 16 bits.
> >        */
> > -     if (clock->size != clock->count && (y2 - y1) < ((1000 / 4) << 16))
> > +     if (clock->size != clock->count &&
> > +         (y2 - y1) < (MIN_HW_TIMESTAMP_DIFF << 16))
> >               goto done;
> >
> >       y = (u64)(y2 - y1) * (1ULL << 31) + (u64)y1 * (u64)x2
>
> --
> Regards,
>
> Laurent Pinchart



-- 
Ricardo Ribalda