Re: [PATCH v5 2/2] media: uvcvideo: Fix buffer sequence in frame gaps

Hans de Goede
2026-03-30T15:31:37Z

Hi,

On 23-Mar-26 10:53, Ricardo Ribalda wrote:
> In UVC, the FID flips with every frame. For every FID flip, we increase
> the stream sequence number.
>
> Now, if a FID flips multiple times and there is no data transferred between
> the flips, the buffer sequence number will be set to the value of the
> stream sequence number after the first flip.
>
> Userspace uses the buffer sequence number to determine if there have been
> missing frames. With the current behaviour, userspace will think that the
> gap is in the wrong location.
>
> This patch modifies uvc_video_decode_start() to provide the correct buffer
> sequence number and timestamp.
>
> Cc: stable@kernel.org
> Fixes: 650b95feee35 ("[media] uvcvideo: Generate discontinuous sequence numbers when frames are lost")
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> drivers/media/usb/uvc/uvc_video.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index eddb4821b205..32c3469f26c6 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1223,6 +1223,20 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
> stream->sequence++;
> if (stream->sequence)
> uvc_video_stats_update(stream);
> +
> + /*
> + * If there is a FID flip and the buffer has no data,
> + * initialize its sequence number and timestamp.

The "and the buffer has no data" part of the comment is a bit weird
now that the buf->bytes_used == 0 check is dropped. I'll drop
this part of the comment while merging this.

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

Regards,

Hans

> + *
> + * The driver already takes care of injecting FID flips for
> + * UVC_QUIRK_STREAM_NO_FID and UVC_QUIRK_MJPEG_NO_EOF.
> + */
> + if (buf) {
> + buf->buf.field = V4L2_FIELD_NONE;
> + buf->buf.sequence = stream->sequence;
> + buf->buf.vb2_buf.timestamp =
> + ktime_to_ns(uvc_video_get_time());
> + }
> }
>
> uvc_video_clock_decode(stream, buf, data, len);
> @@ -1263,10 +1277,6 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
> return -ENODATA;
> }
>
> - buf->buf.field = V4L2_FIELD_NONE;
> - buf->buf.sequence = stream->sequence;
> - buf->buf.vb2_buf.timestamp = ktime_to_ns(uvc_video_get_time());
> -
> /* TODO: Handle PTS and SCR. */
> buf->state = UVC_BUF_STATE_ACTIVE;
> }
>