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

[Laurent Pinchart]

On Mon, May 11, 2026 at 05:58:30PM +0200, Ricardo Ribalda wrote:
> On Mon, 11 May 2026 at 17:51, Laurent Pinchart 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.

You deleted the information from the comment below, and didn't mention
anything in the commit message, so I was wondering if the situation has
changed. I'd like to retain the information somewhere.

> > > 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