[PATCH v3 0/6] media: uvcvideo: Fixes for hw timestamping

[PATCH v3 0/6] media: uvcvideo: Fixes for hw timestamping

[Ricardo Ribalda]

This series introduces fixes for the hardware timestamp calculations.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Changes in v3:
- Improve the sof_diff function logic.
- Split "Do not run expensive code if not needed" patchset in two
- Link to v2: https://lore.kernel.org/r/20260512-uvc-hwtimestamp-v2-0-3c2905c733bb@chromium.org

Changes in v2:
- Fix comments
- Add UCV_ prefix
- Improve commit messages
- Add "Do not run expensive code if not needed" patchset
- Link to v1: https://lore.kernel.org/r/20260323-uvc-hwtimestamp-v1-0-aa42e3865204@chromium.org

---
Ricardo Ribalda (6):
      media: uvcvideo: Fix dev_sof filtering in hw timestamp
      media: uvcvideo: Use hw timestaming if the clock buffer is full
      media: uvcvideo: Relax the constrains for interpolating the hw clock
      media: uvcvideo: Do not add clock samples with small sof delta
      media: uvcvideo: Do not add samples if dev_sof has not changed
      media: uvcvideo: Only do uvc_video_get_time() if needed

 drivers/media/usb/uvc/uvc_video.c | 72 ++++++++++++++++++++++++++++-----------
 drivers/media/usb/uvc/uvcvideo.h  |  3 +-
 2 files changed, 54 insertions(+), 21 deletions(-)
---
base-commit: 10f943b12e7cb338da00f10e129043ae27b33af4
change-id: 20260309-uvc-hwtimestamp-f25dc27f5711

Best regards,
-- 
Ricardo Ribalda <ribalda@chromium.org>

[PATCH v3 1/6] media: uvcvideo: Fix dev_sof filtering in hw timestamp

[Ricardo Ribalda]

To avoid filling the clock circular buffer with duplicated data we only
add it if the new value sof is different than the last added sof.

The issue is that we compare the unprocess sof with the processed sof.
If there is a sof_offset, or UVC_QUIRK_INVALID_DEVICE_SOF is enabled,
the comparison will not work as expected.

This patch moves the comparison to the right place.

Fixes: 141270bd95d4 ("media: uvcvideo: Refactor clock circular buffer")
Cc: stable@vger.kernel.org
Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
Tested-by: Yunke Cao <yunkec@google.com>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_video.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 62db4db4e565..2ad36a1f4948 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -583,16 +583,7 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
 	if (!has_scr)
 		return;
 
-	/*
-	 * To limit the amount of data, drop SCRs with an SOF identical to the
-	 * previous one. This filtering is also needed to support UVC 1.5, where
-	 * all the data packets of the same frame contains the same SOF. In that
-	 * case only the first one will match the host_sof.
-	 */
 	sample.dev_sof = get_unaligned_le16(&data[header_size - 2]);
-	if (sample.dev_sof == stream->clock.last_sof)
-		return;
-
 	sample.dev_stc = get_unaligned_le32(&data[header_size - 6]);
 
 	/*
@@ -664,6 +655,16 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
 	}
 
 	sample.dev_sof = (sample.dev_sof + stream->clock.sof_offset) & 2047;
+
+	/*
+	 * To limit the amount of data, drop SCRs with an SOF identical to the
+	 * previous one. This filtering is also needed to support UVC 1.5, where
+	 * all the data packets of the same frame contains the same SOF. In that
+	 * case only the first one will match the host_sof.
+	 */
+	if (sample.dev_sof == stream->clock.last_sof)
+		return;
+
 	uvc_video_clock_add_sample(&stream->clock, &sample);
 	stream->clock.last_sof = sample.dev_sof;
 }

-- 
2.54.0.563.g4f69b47b94-goog

[PATCH v3 2/6] media: uvcvideo: Use hw timestaming if the clock buffer is full

[Ricardo Ribalda]

In some situations, even with a full clock buffer, it does not contain
250msec of data. This results in the driver jumping back from software
to hardware timestapsing creating a nasty artifact in the video.

If the clock buffer is full, use it to calculate the timestamp instead
of defaulting to software stamps, the reduced accuracy is less visible
than jumping from one timestamping mechanism to the other.

Fixes: 6243c83be6ee8 ("media: uvcvideo: Allow hw clock updates with buffers not full")
Cc: stable@vger.kernel.org
Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
Tested-by: Yunke Cao <yunkec@google.com>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_video.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 2ad36a1f4948..01dcb81d96fd 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -834,15 +834,22 @@ 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.
+	 * If the buffer is not full, we want to gather at least 1/4th of
+	 * timestamps before using HW timestamping. We do this to avoid jitter
+	 * on the initial frames.
+	 *
+	 * If the buffer is full we would use it regardless of how much data
+	 * it represents. This could be solved with an infinite big circular
+	 * buffer, but RAM is expensive these days, specially the infinitely
+	 * big.
+	 *
+	 * The value of 1/4th of a second was determined by running Android's
+	 * CTS on different devices.
 	 *
 	 * dev_sof runs at 1KHz, and we have a fixed point precision of
 	 * 16 bits.
 	 */
-	if ((y2 - y1) < ((1000 / 4) << 16))
+	if (clock->size != clock->count && (y2 - y1) < ((1000 / 4) << 16))
 		goto done;
 
 	y = (u64)(y2 - y1) * (1ULL << 31) + (u64)y1 * (u64)x2

-- 
2.54.0.563.g4f69b47b94-goog

[PATCH v3 3/6] media: uvcvideo: Relax the constrains for interpolating the hw clock

[Ricardo Ribalda]

In the initial version we set the min value to 250msec. Looks like
100msec can also provide a good value.

Now that we are at it, add a macro to make it cleaner.

Fixes: 6243c83be6ee8 ("media: uvcvideo: Allow hw clock updates with buffers not full")
Cc: stable@vger.kernel.org
Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
Tested-by: Yunke Cao <yunkec@google.com>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_video.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 01dcb81d96fd..355b9bfb799e 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 UVC_MIN_HW_TIMESTAMP_DIFF 100
+
 static inline ktime_t uvc_video_get_time(void)
 {
 	if (uvc_clock_param == CLOCK_MONOTONIC)
@@ -843,13 +850,13 @@ void uvc_video_clock_update(struct uvc_streaming *stream,
 	 * buffer, but RAM is expensive these days, specially the infinitely
 	 * big.
 	 *
-	 * The value of 1/4th of a second was determined by running Android's
-	 * CTS on different devices.
+	 * The value of UVC_MIN_HW_TIMESTAMP_DIFF was determined by running
+	 * Android's CTS on different devices.
 	 *
-	 * 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) < (UVC_MIN_HW_TIMESTAMP_DIFF << 16))
 		goto done;
 
 	y = (u64)(y2 - y1) * (1ULL << 31) + (u64)y1 * (u64)x2

-- 
2.54.0.563.g4f69b47b94-goog

[PATCH v3 4/6] media: uvcvideo: Do not add clock samples with small sof delta

[Ricardo Ribalda]

Some UVC 1.1 cameras running in fast isochronous mode tend to spam the
USB host with a lot of empty packets. These packets contain clock
information and are added to the clock buffer but do not add any
accuracy to the calculation. In fact, it is quite the opposite, in our
calculations, only the first and the last timestamp is used, and we only
have 32 slots.

Ignore the samples that will produce less than MIN_HW_TIMESTAMP_DIFF
data.

Fixes: 141270bd95d4 ("media: uvcvideo: Refactor clock circular buffer")
Cc: stable@vger.kernel.org
Tested-by: Yunke Cao <yunkec@google.com>
Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_video.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 355b9bfb799e..63850b779e24 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -544,6 +544,15 @@ static void uvc_video_clock_add_sample(struct uvc_clock *clock,
 	spin_unlock_irqrestore(&clock->lock, flags);
 }
 
+static inline u16 sof_diff(u16 a, u16 b)
+{
+	/*
+	 * Because the result is modulo 2048 (via & 2047), we do not need a
+	 * special case for a < b.
+	 */
+	return (a - b) & 2047;
+}
+
 static void
 uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
 		       const u8 *data, int len)
@@ -664,12 +673,13 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
 	sample.dev_sof = (sample.dev_sof + stream->clock.sof_offset) & 2047;
 
 	/*
-	 * To limit the amount of data, drop SCRs with an SOF identical to the
+	 * To limit the amount of data, drop SCRs with an SOF similar to the
 	 * previous one. This filtering is also needed to support UVC 1.5, where
 	 * all the data packets of the same frame contains the same SOF. In that
 	 * case only the first one will match the host_sof.
 	 */
-	if (sample.dev_sof == stream->clock.last_sof)
+	if (sof_diff(sample.dev_sof, stream->clock.last_sof) <=
+	    (UVC_MIN_HW_TIMESTAMP_DIFF / stream->clock.size))
 		return;
 
 	uvc_video_clock_add_sample(&stream->clock, &sample);

-- 
2.54.0.563.g4f69b47b94-goog

Re: [PATCH v3 4/6] media: uvcvideo: Do not add clock samples with small sof delta

[Ricardo Ribalda]

Hi Hans

On Wed, 13 May 2026 at 13:49, Ricardo Ribalda <ribalda@chromium.org> wrote:
>
> Some UVC 1.1 cameras running in fast isochronous mode tend to spam the
> USB host with a lot of empty packets. These packets contain clock
> information and are added to the clock buffer but do not add any
> accuracy to the calculation. In fact, it is quite the opposite, in our
> calculations, only the first and the last timestamp is used, and we only
> have 32 slots.
>
> Ignore the samples that will produce less than MIN_HW_TIMESTAMP_DIFF
> data.
>
> Fixes: 141270bd95d4 ("media: uvcvideo: Refactor clock circular buffer")
> Cc: stable@vger.kernel.org
> Tested-by: Yunke Cao <yunkec@google.com>
> Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/usb/uvc/uvc_video.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index 355b9bfb799e..63850b779e24 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -544,6 +544,15 @@ static void uvc_video_clock_add_sample(struct uvc_clock *clock,
>         spin_unlock_irqrestore(&clock->lock, flags);
>  }
>
> +static inline u16 sof_diff(u16 a, u16 b)
> +{
> +       /*
> +        * Because the result is modulo 2048 (via & 2047), we do not need a
> +        * special case for a < b.
> +        */
> +       return (a - b) & 2047;
> +}
I have modified this function but kept your R-b. Hope that it is fine.

Thanks!


> +
>  static void
>  uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
>                        const u8 *data, int len)
> @@ -664,12 +673,13 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
>         sample.dev_sof = (sample.dev_sof + stream->clock.sof_offset) & 2047;
>
>         /*
> -        * To limit the amount of data, drop SCRs with an SOF identical to the
> +        * To limit the amount of data, drop SCRs with an SOF similar to the
>          * previous one. This filtering is also needed to support UVC 1.5, where
>          * all the data packets of the same frame contains the same SOF. In that
>          * case only the first one will match the host_sof.
>          */
> -       if (sample.dev_sof == stream->clock.last_sof)
> +       if (sof_diff(sample.dev_sof, stream->clock.last_sof) <=
> +           (UVC_MIN_HW_TIMESTAMP_DIFF / stream->clock.size))
>                 return;
>
>         uvc_video_clock_add_sample(&stream->clock, &sample);
>
> --
> 2.54.0.563.g4f69b47b94-goog
>


-- 
Ricardo Ribalda

[PATCH v3 5/6] media: uvcvideo: Do not add samples if dev_sof has not changed

[Ricardo Ribalda]

We only save relevant samples into the circular buffer. If the data is
very similar to the previous one, exit early, this allows us to avoid
some expensive operations such as usb_get_current_frame_number().

Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_video.c | 16 +++++++++++-----
 drivers/media/usb/uvc/uvcvideo.h  |  3 ++-
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 63850b779e24..6794031cd0fb 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -524,7 +524,7 @@ static void uvc_video_clock_add_sample(struct uvc_clock *clock,
 
 	spin_lock_irqsave(&clock->lock, flags);
 
-	if (clock->count > 0 && clock->last_sof > sample->dev_sof) {
+	if (clock->count > 0 && clock->last_sof_processed > sample->dev_sof) {
 		/*
 		 * Remove data from the circular buffer that is older than the
 		 * last SOF overflow. We only support one SOF overflow per
@@ -599,7 +599,12 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
 	if (!has_scr)
 		return;
 
-	sample.dev_sof = get_unaligned_le16(&data[header_size - 2]);
+	sample.dev_sof = get_unaligned_le16(&data[header_size - 2]) & 2047;
+	/* If the sample SOF is identical to the previous one, quit early. */
+	if (stream->clock.last_sof_raw == sample.dev_sof)
+		return;
+	stream->clock.last_sof_raw = sample.dev_sof;
+
 	sample.dev_stc = get_unaligned_le32(&data[header_size - 6]);
 
 	/*
@@ -678,19 +683,20 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
 	 * all the data packets of the same frame contains the same SOF. In that
 	 * case only the first one will match the host_sof.
 	 */
-	if (sof_diff(sample.dev_sof, stream->clock.last_sof) <=
+	if (sof_diff(sample.dev_sof, stream->clock.last_sof_processed) <=
 	    (UVC_MIN_HW_TIMESTAMP_DIFF / stream->clock.size))
 		return;
 
 	uvc_video_clock_add_sample(&stream->clock, &sample);
-	stream->clock.last_sof = sample.dev_sof;
+	stream->clock.last_sof_processed = sample.dev_sof;
 }
 
 static void uvc_video_clock_reset(struct uvc_clock *clock)
 {
 	clock->head = 0;
 	clock->count = 0;
-	clock->last_sof = -1;
+	clock->last_sof_processed = -1;
+	clock->last_sof_raw = -1;
 	clock->last_sof_overflow = -1;
 	clock->sof_offset = -1;
 }
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 4ba35727e954..b6bcee4a222f 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -522,7 +522,8 @@ struct uvc_streaming {
 		unsigned int size;
 		unsigned int last_sof_overflow;
 
-		u16 last_sof;
+		u16 last_sof_processed;
+		u16 last_sof_raw;
 		u16 sof_offset;
 
 		u8 last_scr[6];

-- 
2.54.0.563.g4f69b47b94-goog

Re: [PATCH v3 5/6] media: uvcvideo: Do not add samples if dev_sof has not changed

[Hans de Goede]

Hi,

On 13-May-26 1:49 PM, Ricardo Ribalda wrote:
> We only save relevant samples into the circular buffer. If the data is
> very similar to the previous one, exit early, this allows us to avoid
> some expensive operations such as usb_get_current_frame_number().
> 
> Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>

Regards,

Hans


> ---
>  drivers/media/usb/uvc/uvc_video.c | 16 +++++++++++-----
>  drivers/media/usb/uvc/uvcvideo.h  |  3 ++-
>  2 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index 63850b779e24..6794031cd0fb 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -524,7 +524,7 @@ static void uvc_video_clock_add_sample(struct uvc_clock *clock,
>  
>  	spin_lock_irqsave(&clock->lock, flags);
>  
> -	if (clock->count > 0 && clock->last_sof > sample->dev_sof) {
> +	if (clock->count > 0 && clock->last_sof_processed > sample->dev_sof) {
>  		/*
>  		 * Remove data from the circular buffer that is older than the
>  		 * last SOF overflow. We only support one SOF overflow per
> @@ -599,7 +599,12 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
>  	if (!has_scr)
>  		return;
>  
> -	sample.dev_sof = get_unaligned_le16(&data[header_size - 2]);
> +	sample.dev_sof = get_unaligned_le16(&data[header_size - 2]) & 2047;
> +	/* If the sample SOF is identical to the previous one, quit early. */
> +	if (stream->clock.last_sof_raw == sample.dev_sof)
> +		return;
> +	stream->clock.last_sof_raw = sample.dev_sof;
> +
>  	sample.dev_stc = get_unaligned_le32(&data[header_size - 6]);
>  
>  	/*
> @@ -678,19 +683,20 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
>  	 * all the data packets of the same frame contains the same SOF. In that
>  	 * case only the first one will match the host_sof.
>  	 */
> -	if (sof_diff(sample.dev_sof, stream->clock.last_sof) <=
> +	if (sof_diff(sample.dev_sof, stream->clock.last_sof_processed) <=
>  	    (UVC_MIN_HW_TIMESTAMP_DIFF / stream->clock.size))
>  		return;
>  
>  	uvc_video_clock_add_sample(&stream->clock, &sample);
> -	stream->clock.last_sof = sample.dev_sof;
> +	stream->clock.last_sof_processed = sample.dev_sof;
>  }
>  
>  static void uvc_video_clock_reset(struct uvc_clock *clock)
>  {
>  	clock->head = 0;
>  	clock->count = 0;
> -	clock->last_sof = -1;
> +	clock->last_sof_processed = -1;
> +	clock->last_sof_raw = -1;
>  	clock->last_sof_overflow = -1;
>  	clock->sof_offset = -1;
>  }
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 4ba35727e954..b6bcee4a222f 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -522,7 +522,8 @@ struct uvc_streaming {
>  		unsigned int size;
>  		unsigned int last_sof_overflow;
>  
> -		u16 last_sof;
> +		u16 last_sof_processed;
> +		u16 last_sof_raw;
>  		u16 sof_offset;
>  
>  		u8 last_scr[6];
> 

[PATCH v3 6/6] media: uvcvideo: Only do uvc_video_get_time() if needed

[Ricardo Ribalda]

There is no need to calculate the current time if the sample is going to
be filtered.

Move the assignment close to uvc_video_clock_add_sample().

Suggested-by: Hans de Goede <hansg@kernel.org>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_video.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 6794031cd0fb..1cc86a18b2bb 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -645,8 +645,6 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
 	if (stream->dev->quirks & UVC_QUIRK_INVALID_DEVICE_SOF)
 		sample.dev_sof = sample.host_sof;
 
-	sample.host_time = uvc_video_get_time();
-
 	/*
 	 * The UVC specification allows device implementations that can't obtain
 	 * the USB frame number to keep their own frame counters as long as they
@@ -687,6 +685,9 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
 	    (UVC_MIN_HW_TIMESTAMP_DIFF / stream->clock.size))
 		return;
 
+	/* This is expensive, only do it if the sample will be added. */
+	sample.host_time = uvc_video_get_time();
+
 	uvc_video_clock_add_sample(&stream->clock, &sample);
 	stream->clock.last_sof_processed = sample.dev_sof;
 }

-- 
2.54.0.563.g4f69b47b94-goog

Re: [PATCH v3 6/6] media: uvcvideo: Only do uvc_video_get_time() if needed

[Yunke Cao]

Hi Ricardo,

I tested v3 and verified that it fixes the hw timestamp for the
SunplusIT Inc 1080p FHD Camera (2b7e:c877).

Tested-by: Yunke Cao <yunkec@google.com>

Best,
Yunke

On Wed, May 13, 2026 at 8:49 PM Ricardo Ribalda <ribalda@chromium.org> wrote:
>
> There is no need to calculate the current time if the sample is going to
> be filtered.
>
> Move the assignment close to uvc_video_clock_add_sample().
>
> Suggested-by: Hans de Goede <hansg@kernel.org>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/usb/uvc/uvc_video.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index 6794031cd0fb..1cc86a18b2bb 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -645,8 +645,6 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
>         if (stream->dev->quirks & UVC_QUIRK_INVALID_DEVICE_SOF)
>                 sample.dev_sof = sample.host_sof;
>
> -       sample.host_time = uvc_video_get_time();
> -
>         /*
>          * The UVC specification allows device implementations that can't obtain
>          * the USB frame number to keep their own frame counters as long as they
> @@ -687,6 +685,9 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
>             (UVC_MIN_HW_TIMESTAMP_DIFF / stream->clock.size))
>                 return;
>
> +       /* This is expensive, only do it if the sample will be added. */
> +       sample.host_time = uvc_video_get_time();
> +
>         uvc_video_clock_add_sample(&stream->clock, &sample);
>         stream->clock.last_sof_processed = sample.dev_sof;
>  }
>
> --
> 2.54.0.563.g4f69b47b94-goog
>

Re: [PATCH v3 6/6] media: uvcvideo: Only do uvc_video_get_time() if needed

[Hans de Goede]

Hi,

On 13-May-26 1:49 PM, Ricardo Ribalda wrote:
> There is no need to calculate the current time if the sample is going to
> be filtered.
> 
> Move the assignment close to uvc_video_clock_add_sample().
> 
> Suggested-by: Hans de Goede <hansg@kernel.org>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>

Regards,

Hans


> ---
>  drivers/media/usb/uvc/uvc_video.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index 6794031cd0fb..1cc86a18b2bb 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -645,8 +645,6 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
>  	if (stream->dev->quirks & UVC_QUIRK_INVALID_DEVICE_SOF)
>  		sample.dev_sof = sample.host_sof;
>  
> -	sample.host_time = uvc_video_get_time();
> -
>  	/*
>  	 * The UVC specification allows device implementations that can't obtain
>  	 * the USB frame number to keep their own frame counters as long as they
> @@ -687,6 +685,9 @@ uvc_video_clock_decode(struct uvc_streaming *stream, struct uvc_buffer *buf,
>  	    (UVC_MIN_HW_TIMESTAMP_DIFF / stream->clock.size))
>  		return;
>  
> +	/* This is expensive, only do it if the sample will be added. */
> +	sample.host_time = uvc_video_get_time();
> +
>  	uvc_video_clock_add_sample(&stream->clock, &sample);
>  	stream->clock.last_sof_processed = sample.dev_sof;
>  }
> 

Re: [PATCH v3 0/6] media: uvcvideo: Fixes for hw timestamping

[Hans de Goede]

Hi,

On 13-May-26 1:49 PM, Ricardo Ribalda wrote:
> This series introduces fixes for the hardware timestamp calculations.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> Changes in v3:
> - Improve the sof_diff function logic.
> - Split "Do not run expensive code if not needed" patchset in two
> - Link to v2: https://lore.kernel.org/r/20260512-uvc-hwtimestamp-v2-0-3c2905c733bb@chromium.org

Thank you.

I've merged this series and pushed it out to the uvc/for-next branch.

Note I've also rebased uvc/for-next on top of the latest media-committers/next .

Regards,

Hans



> 
> Changes in v2:
> - Fix comments
> - Add UCV_ prefix
> - Improve commit messages
> - Add "Do not run expensive code if not needed" patchset
> - Link to v1: https://lore.kernel.org/r/20260323-uvc-hwtimestamp-v1-0-aa42e3865204@chromium.org
> 
> ---
> Ricardo Ribalda (6):
>       media: uvcvideo: Fix dev_sof filtering in hw timestamp
>       media: uvcvideo: Use hw timestaming if the clock buffer is full
>       media: uvcvideo: Relax the constrains for interpolating the hw clock
>       media: uvcvideo: Do not add clock samples with small sof delta
>       media: uvcvideo: Do not add samples if dev_sof has not changed
>       media: uvcvideo: Only do uvc_video_get_time() if needed
> 
>  drivers/media/usb/uvc/uvc_video.c | 72 ++++++++++++++++++++++++++++-----------
>  drivers/media/usb/uvc/uvcvideo.h  |  3 +-
>  2 files changed, 54 insertions(+), 21 deletions(-)
> ---
> base-commit: 10f943b12e7cb338da00f10e129043ae27b33af4
> change-id: 20260309-uvc-hwtimestamp-f25dc27f5711
> 
> Best regards,