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

[Ricardo Ribalda]

Hi Laurent

On Tue, 12 May 2026 at 14:38, Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>
> 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.

I believe it should be fixed in v2. Looks like I have to send a v3
anyway :P so I will double check that it is still there.

Thanks!


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