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

[Hans de Goede <hansg@kernel.org>]

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