[PATCH v3 0/2] media: uvcvideo: Fixes for frame sequence number

[PATCH v3 0/2] media: uvcvideo: Fixes for frame sequence number

[Ricardo Ribalda]

This series fixes a couple of corner cases where the frame sequence
number is not properly handled.

Please note that the second patch has not been tested in a camera
without EOF.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Changes in v3:
- Fix typo in commit message.
- Add new patch
- Link to v2: https://lore.kernel.org/r/20260313-uvc-fid-v2-1-3f7a996d9047@chromium.org

Changes in v2 (Thanks Laurent):
- Improve commit message.
- Remove original timestamp and sequence assignment. It is not neeed
- Link to v1: https://lore.kernel.org/r/20260310-uvc-fid-v1-1-5e37dc3c7024@chromium.org

---
Ricardo Ribalda (2):
      media: uvcvideo: Fix buffer sequence in frame gaps
      media: uvcvideo: Fix sequence number when no EOF

 drivers/media/usb/uvc/uvc_video.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)
---
base-commit: a7da7fb57f2a787412da1a62292a17fa00fbfbdf
change-id: 20260310-uvc-fid-e1e55447b6f1

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

[PATCH v3 1/2] media: uvcvideo: Fix buffer sequence in frame gaps

[Ricardo Ribalda]

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 has 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 40c76c051da2..9e06b1d0f0f9 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1176,6 +1176,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 driver already takes care of injecting FID flips for
+		 * UVC_QUIRK_STREAM_NO_FID and UVC_QUIRK_MJPEG_NO_EOF.
+		 */
+		if (buf && !buf->bytesused) {
+			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);
@@ -1216,10 +1230,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;
 	}

-- 
2.53.0.851.ga537e3e6e9-goog

[PATCH v3 2/2] media: uvcvideo: Fix sequence number when no EOF

[Ricardo Ribalda]

If the driver could not detect the EOF, the sequence number is increased
twice:
 1) When we enter uvc_video_decode_start() with the old buffer and FID has
   fliped => We return -EAGAIN and last_fid is not flipped
 2) When we enter uvc_video_decode_start() with the new buffer.

Fix this issue by saving last_fid on the first FID flip.

Cc: stable@kernel.org
Fixes: 650b95feee35 ("[media] uvcvideo: Generate discontinuous sequence numbers when frames are lost")
Reported-by: Hans de Goede <hansg@kernel.org>
Closes: https://lore.kernel.org/linux-media/CANiDSCuj4cPuB5_v2xyvAagA5FjoN8V5scXiFFOeD3aKDMqkCg@mail.gmail.com/T/#me39fb134e8c2c085567a31548c3403eb639625e4
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_video.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 9e06b1d0f0f9..3e6ded69388f 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1254,6 +1254,12 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
 		uvc_dbg(stream->dev, FRAME,
 			"Frame complete (FID bit toggled)\n");
 		buf->state = UVC_BUF_STATE_READY;
+
+		/*
+		 * If the EOF detection has failed, we need to save the last_fid
+		 * to avoid increasing the sequence number twice.
+		 */
+		stream->last_fid = fid;
 		return -EAGAIN;
 	}
 

-- 
2.53.0.851.ga537e3e6e9-goog

Re: [PATCH v3 2/2] media: uvcvideo: Fix sequence number when no EOF

[Hans de Goede]

Hi,

On 16-Mar-26 14:30, Ricardo Ribalda wrote:
> If the driver could not detect the EOF, the sequence number is increased
> twice:
>  1) When we enter uvc_video_decode_start() with the old buffer and FID has
>    fliped => We return -EAGAIN and last_fid is not flipped
>  2) When we enter uvc_video_decode_start() with the new buffer.
> 
> Fix this issue by saving last_fid on the first FID flip.
> 
> Cc: stable@kernel.org
> Fixes: 650b95feee35 ("[media] uvcvideo: Generate discontinuous sequence numbers when frames are lost")
> Reported-by: Hans de Goede <hansg@kernel.org>
> Closes: https://lore.kernel.org/linux-media/CANiDSCuj4cPuB5_v2xyvAagA5FjoN8V5scXiFFOeD3aKDMqkCg@mail.gmail.com/T/#me39fb134e8c2c085567a31548c3403eb639625e4
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/usb/uvc/uvc_video.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index 9e06b1d0f0f9..3e6ded69388f 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1254,6 +1254,12 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
>  		uvc_dbg(stream->dev, FRAME,
>  			"Frame complete (FID bit toggled)\n");
>  		buf->state = UVC_BUF_STATE_READY;
> +
> +		/*
> +		 * If the EOF detection has failed, we need to save the last_fid
> +		 * to avoid increasing the sequence number twice.
> +		 */
> +		stream->last_fid = fid;

AFAICT, there is still a problem after this patch:

1. We have an incomplete frame, so no EOF, first run through
uvc_video_decode_start()

2. We hit the first if (stream->last_fid != fid) check do
stream->sequence++ . And after patch 1/2 we do NOT update
"buf->buf.sequence = stream->sequence" because buf->bytesused != 0
(which is good, the incomplete frame should not get the new
sequence-no).

3. Still first run through uvc_video_decode_start() we hit the:
if (fid != stream->last_fid && buf->bytesused != 0) check further
down, update "stream->last_fid = fid" (after this patch) and
return -EAGAIN.

4. Second run through uvc_video_decode_start() the first
if (stream->last_fid != fid) check no longer triggers, we
don't increase the sequence-no (good) but we also do not
set "buf->buf.sequence = stream->sequence" for the new buffer
we are called with on the second run!

So the combination of these 2 fixes is broken.

Regards,

Hans

Re: [PATCH v3 2/2] media: uvcvideo: Fix sequence number when no EOF

[Ricardo Ribalda]

Hi Hans

Good catch. Sending a new version.

Hope that is less broken :P

On Fri, 20 Mar 2026 at 13:17, Hans de Goede <hansg@kernel.org> wrote:
>
> Hi,
>
> On 16-Mar-26 14:30, Ricardo Ribalda wrote:
> > If the driver could not detect the EOF, the sequence number is increased
> > twice:
> >  1) When we enter uvc_video_decode_start() with the old buffer and FID has
> >    fliped => We return -EAGAIN and last_fid is not flipped
> >  2) When we enter uvc_video_decode_start() with the new buffer.
> >
> > Fix this issue by saving last_fid on the first FID flip.
> >
> > Cc: stable@kernel.org
> > Fixes: 650b95feee35 ("[media] uvcvideo: Generate discontinuous sequence numbers when frames are lost")
> > Reported-by: Hans de Goede <hansg@kernel.org>
> > Closes: https://lore.kernel.org/linux-media/CANiDSCuj4cPuB5_v2xyvAagA5FjoN8V5scXiFFOeD3aKDMqkCg@mail.gmail.com/T/#me39fb134e8c2c085567a31548c3403eb639625e4
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> >  drivers/media/usb/uvc/uvc_video.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> > index 9e06b1d0f0f9..3e6ded69388f 100644
> > --- a/drivers/media/usb/uvc/uvc_video.c
> > +++ b/drivers/media/usb/uvc/uvc_video.c
> > @@ -1254,6 +1254,12 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
> >               uvc_dbg(stream->dev, FRAME,
> >                       "Frame complete (FID bit toggled)\n");
> >               buf->state = UVC_BUF_STATE_READY;
> > +
> > +             /*
> > +              * If the EOF detection has failed, we need to save the last_fid
> > +              * to avoid increasing the sequence number twice.
> > +              */
> > +             stream->last_fid = fid;
>
> AFAICT, there is still a problem after this patch:
>
> 1. We have an incomplete frame, so no EOF, first run through
> uvc_video_decode_start()
>
> 2. We hit the first if (stream->last_fid != fid) check do
> stream->sequence++ . And after patch 1/2 we do NOT update
> "buf->buf.sequence = stream->sequence" because buf->bytesused != 0
> (which is good, the incomplete frame should not get the new
> sequence-no).
>
> 3. Still first run through uvc_video_decode_start() we hit the:
> if (fid != stream->last_fid && buf->bytesused != 0) check further
> down, update "stream->last_fid = fid" (after this patch) and
> return -EAGAIN.
>
> 4. Second run through uvc_video_decode_start() the first
> if (stream->last_fid != fid) check no longer triggers, we
> don't increase the sequence-no (good) but we also do not
> set "buf->buf.sequence = stream->sequence" for the new buffer
> we are called with on the second run!
>
> So the combination of these 2 fixes is broken.
>
> Regards,
>
> Hans
>
>


-- 
Ricardo Ribalda