* [PATCH v5 0/2] media: uvcvideo: Fixes for frame sequence number
@ 2026-03-23 9:53 Ricardo Ribalda
2026-03-23 9:53 ` [PATCH v5 1/2] media: uvcvideo: Fix sequence number when no EOF Ricardo Ribalda
2026-03-23 9:53 ` [PATCH v5 2/2] media: uvcvideo: Fix buffer sequence in frame gaps Ricardo Ribalda
0 siblings, 2 replies; 5+ messages in thread
From: Ricardo Ribalda @ 2026-03-23 9:53 UTC (permalink / raw)
To: Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab
Cc: linux-media, linux-kernel, Yunke Cao, Ricardo Ribalda, stable
This series fixes a couple of corner cases where the frame sequence
number is not properly handled.
Please note that the first patch has not been tested in a camera
without EOF. To emulate it I have used this:
diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index b66d701f2582d..097bed2f7845f 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1360,6 +1360,8 @@ static void uvc_video_decode_end(struct uvc_streaming *stream,
{
/* Mark the buffer as done if the EOF marker is set. */
if (data[1] & UVC_STREAM_EOF && buf->bytesused != 0) {
+ printk(KERN_ERR "Ignoring EOF\n");
+ return;
uvc_dbg(stream->dev, FRAME, "Frame complete (EOF found)\n");
if (data[0] == len)
uvc_dbg(stream->dev, FRAME, "EOF in empty payload\n");
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Changes in v5 (Thanks Hans):
- Swap order of patches
- Remove duplicated conditions.
- Link to v4: https://lore.kernel.org/r/20260320-uvc-fid-v4-0-f24f168ca2f9@chromium.org
Changes in v4 (Thanks Hans):
- Fix 2/2 logic.
- Link to v3: https://lore.kernel.org/r/20260316-uvc-fid-v3-0-c793354469b5@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 sequence number when no EOF
media: uvcvideo: Fix buffer sequence in frame gaps
drivers/media/usb/uvc/uvc_video.c | 110 +++++++++++++++++++++-----------------
1 file changed, 61 insertions(+), 49 deletions(-)
---
base-commit: a7da7fb57f2a787412da1a62292a17fa00fbfbdf
change-id: 20260310-uvc-fid-e1e55447b6f1
Best regards,
--
Ricardo Ribalda <ribalda@chromium.org>
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v5 1/2] media: uvcvideo: Fix sequence number when no EOF
2026-03-23 9:53 [PATCH v5 0/2] media: uvcvideo: Fixes for frame sequence number Ricardo Ribalda
@ 2026-03-23 9:53 ` Ricardo Ribalda
2026-03-24 0:20 ` Laurent Pinchart
2026-03-23 9:53 ` [PATCH v5 2/2] media: uvcvideo: Fix buffer sequence in frame gaps Ricardo Ribalda
1 sibling, 1 reply; 5+ messages in thread
From: Ricardo Ribalda @ 2026-03-23 9:53 UTC (permalink / raw)
To: Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab
Cc: linux-media, linux-kernel, Yunke Cao, Ricardo Ribalda, stable
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
flipped => 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 moving the new frame detection logic earlier in
uvc_video_decode_start().
This also has some nice side affects:
- The error status from the new packet will no longer get propagated
to the previous frame-buffer.
- uvc_video_clock_decode() will no longer update the previous frame
buf->stf with info from the new packet.
- uvc_video_clock_decode() and uvc_video_stats_decode() will no longer
get called twice for the same packet.
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 | 92 ++++++++++++++++++++-------------------
1 file changed, 47 insertions(+), 45 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index 40c76c051da2..eddb4821b205 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1168,6 +1168,53 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
header_len = data[0];
fid = data[1] & UVC_STREAM_FID;
+ /*
+ * Mark the buffer as done if we're at the beginning of a new frame.
+ * End of frame detection is better implemented by checking the EOF
+ * bit (FID bit toggling is delayed by one frame compared to the EOF
+ * bit), but some devices don't set the bit at end of frame (and the
+ * last payload can be lost anyway). We thus must check if the FID has
+ * been toggled.
+ *
+ * stream->last_fid is initialized to -1, and buf->bytesused to 0,
+ * so the first isochronous frame will never trigger an end of frame
+ * detection.
+ *
+ * Empty buffers (bytesused == 0) don't trigger end of frame detection
+ * as it doesn't make sense to return an empty buffer. This also
+ * avoids detecting end of frame conditions at FID toggling if the
+ * previous payload had the EOF bit set.
+ */
+ if (fid != stream->last_fid && buf && buf->bytesused != 0) {
+ uvc_dbg(stream->dev, FRAME,
+ "Frame complete (FID bit toggled)\n");
+ buf->state = UVC_BUF_STATE_READY;
+
+ return -EAGAIN;
+ }
+
+ /*
+ * Some cameras, when running two parallel streams (one MJPEG alongside
+ * another non-MJPEG stream), are known to lose the EOF packet for a frame.
+ * We can detect the end of a frame by checking for a new SOI marker, as
+ * the SOI always lies on the packet boundary between two frames for
+ * these devices.
+ */
+ if (stream->dev->quirks & UVC_QUIRK_MJPEG_NO_EOF &&
+ (stream->cur_format->fcc == V4L2_PIX_FMT_MJPEG ||
+ stream->cur_format->fcc == V4L2_PIX_FMT_JPEG)) {
+ const u8 *packet = data + header_len;
+
+ if (len >= header_len + 2 &&
+ packet[0] == 0xff && packet[1] == JPEG_MARKER_SOI &&
+ buf && buf->bytesused != 0) {
+ buf->state = UVC_BUF_STATE_READY;
+ buf->error = 1;
+ stream->last_fid ^= UVC_STREAM_FID;
+ return -EAGAIN;
+ }
+ }
+
/*
* Increase the sequence number regardless of any buffer states, so
* that discontinuous sequence numbers always indicate lost frames.
@@ -1224,51 +1271,6 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
buf->state = UVC_BUF_STATE_ACTIVE;
}
- /*
- * Mark the buffer as done if we're at the beginning of a new frame.
- * End of frame detection is better implemented by checking the EOF
- * bit (FID bit toggling is delayed by one frame compared to the EOF
- * bit), but some devices don't set the bit at end of frame (and the
- * last payload can be lost anyway). We thus must check if the FID has
- * been toggled.
- *
- * stream->last_fid is initialized to -1, so the first isochronous
- * frame will never trigger an end of frame detection.
- *
- * Empty buffers (bytesused == 0) don't trigger end of frame detection
- * as it doesn't make sense to return an empty buffer. This also
- * avoids detecting end of frame conditions at FID toggling if the
- * previous payload had the EOF bit set.
- */
- if (fid != stream->last_fid && buf->bytesused != 0) {
- uvc_dbg(stream->dev, FRAME,
- "Frame complete (FID bit toggled)\n");
- buf->state = UVC_BUF_STATE_READY;
- return -EAGAIN;
- }
-
- /*
- * Some cameras, when running two parallel streams (one MJPEG alongside
- * another non-MJPEG stream), are known to lose the EOF packet for a frame.
- * We can detect the end of a frame by checking for a new SOI marker, as
- * the SOI always lies on the packet boundary between two frames for
- * these devices.
- */
- if (stream->dev->quirks & UVC_QUIRK_MJPEG_NO_EOF &&
- (stream->cur_format->fcc == V4L2_PIX_FMT_MJPEG ||
- stream->cur_format->fcc == V4L2_PIX_FMT_JPEG)) {
- const u8 *packet = data + header_len;
-
- if (len >= header_len + 2 &&
- packet[0] == 0xff && packet[1] == JPEG_MARKER_SOI &&
- buf->bytesused != 0) {
- buf->state = UVC_BUF_STATE_READY;
- buf->error = 1;
- stream->last_fid ^= UVC_STREAM_FID;
- return -EAGAIN;
- }
- }
-
stream->last_fid = fid;
return header_len;
--
2.53.0.959.g497ff81fa9-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v5 2/2] media: uvcvideo: Fix buffer sequence in frame gaps
2026-03-23 9:53 [PATCH v5 0/2] media: uvcvideo: Fixes for frame sequence number Ricardo Ribalda
2026-03-23 9:53 ` [PATCH v5 1/2] media: uvcvideo: Fix sequence number when no EOF Ricardo Ribalda
@ 2026-03-23 9:53 ` Ricardo Ribalda
1 sibling, 0 replies; 5+ messages in thread
From: Ricardo Ribalda @ 2026-03-23 9:53 UTC (permalink / raw)
To: Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab
Cc: linux-media, linux-kernel, Yunke Cao, Ricardo Ribalda, stable
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 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;
}
--
2.53.0.959.g497ff81fa9-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v5 1/2] media: uvcvideo: Fix sequence number when no EOF
2026-03-23 9:53 ` [PATCH v5 1/2] media: uvcvideo: Fix sequence number when no EOF Ricardo Ribalda
@ 2026-03-24 0:20 ` Laurent Pinchart
2026-03-24 7:23 ` Ricardo Ribalda
0 siblings, 1 reply; 5+ messages in thread
From: Laurent Pinchart @ 2026-03-24 0:20 UTC (permalink / raw)
To: Ricardo Ribalda
Cc: Hans de Goede, Mauro Carvalho Chehab, linux-media, linux-kernel,
Yunke Cao, stable
On Mon, Mar 23, 2026 at 09:53:52AM +0000, 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
> flipped => 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 moving the new frame detection logic earlier in
> uvc_video_decode_start().
>
> This also has some nice side affects:
>
> - The error status from the new packet will no longer get propagated
> to the previous frame-buffer.
> - uvc_video_clock_decode() will no longer update the previous frame
> buf->stf with info from the new packet.
> - uvc_video_clock_decode() and uvc_video_stats_decode() will no longer
> get called twice for the same packet.
>
> 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 | 92 ++++++++++++++++++++-------------------
> 1 file changed, 47 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index 40c76c051da2..eddb4821b205 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1168,6 +1168,53 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
> header_len = data[0];
> fid = data[1] & UVC_STREAM_FID;
>
> + /*
> + * Mark the buffer as done if we're at the beginning of a new frame.
> + * End of frame detection is better implemented by checking the EOF
> + * bit (FID bit toggling is delayed by one frame compared to the EOF
> + * bit), but some devices don't set the bit at end of frame (and the
> + * last payload can be lost anyway). We thus must check if the FID has
> + * been toggled.
> + *
> + * stream->last_fid is initialized to -1, and buf->bytesused to 0,
> + * so the first isochronous frame will never trigger an end of frame
> + * detection.
> + *
> + * Empty buffers (bytesused == 0) don't trigger end of frame detection
> + * as it doesn't make sense to return an empty buffer. This also
> + * avoids detecting end of frame conditions at FID toggling if the
> + * previous payload had the EOF bit set.
> + */
> + if (fid != stream->last_fid && buf && buf->bytesused != 0) {
> + uvc_dbg(stream->dev, FRAME,
> + "Frame complete (FID bit toggled)\n");
> + buf->state = UVC_BUF_STATE_READY;
> +
> + return -EAGAIN;
> + }
> +
> + /*
> + * Some cameras, when running two parallel streams (one MJPEG alongside
> + * another non-MJPEG stream), are known to lose the EOF packet for a frame.
> + * We can detect the end of a frame by checking for a new SOI marker, as
> + * the SOI always lies on the packet boundary between two frames for
> + * these devices.
> + */
> + if (stream->dev->quirks & UVC_QUIRK_MJPEG_NO_EOF &&
> + (stream->cur_format->fcc == V4L2_PIX_FMT_MJPEG ||
> + stream->cur_format->fcc == V4L2_PIX_FMT_JPEG)) {
> + const u8 *packet = data + header_len;
> +
> + if (len >= header_len + 2 &&
> + packet[0] == 0xff && packet[1] == JPEG_MARKER_SOI &&
> + buf && buf->bytesused != 0) {
How about moving the buf && buf->bytesused != 0 to the outer condition,
so that we don't inspect the packet when we don't need to ? It will also
make the two end of frame detection blocks more similar.
The patch otherwise looks fine, I don't see anything below this code
that would need to be performed before.
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> + buf->state = UVC_BUF_STATE_READY;
> + buf->error = 1;
> + stream->last_fid ^= UVC_STREAM_FID;
> + return -EAGAIN;
> + }
> + }
> +
> /*
> * Increase the sequence number regardless of any buffer states, so
> * that discontinuous sequence numbers always indicate lost frames.
> @@ -1224,51 +1271,6 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
> buf->state = UVC_BUF_STATE_ACTIVE;
> }
>
> - /*
> - * Mark the buffer as done if we're at the beginning of a new frame.
> - * End of frame detection is better implemented by checking the EOF
> - * bit (FID bit toggling is delayed by one frame compared to the EOF
> - * bit), but some devices don't set the bit at end of frame (and the
> - * last payload can be lost anyway). We thus must check if the FID has
> - * been toggled.
> - *
> - * stream->last_fid is initialized to -1, so the first isochronous
> - * frame will never trigger an end of frame detection.
> - *
> - * Empty buffers (bytesused == 0) don't trigger end of frame detection
> - * as it doesn't make sense to return an empty buffer. This also
> - * avoids detecting end of frame conditions at FID toggling if the
> - * previous payload had the EOF bit set.
> - */
> - if (fid != stream->last_fid && buf->bytesused != 0) {
> - uvc_dbg(stream->dev, FRAME,
> - "Frame complete (FID bit toggled)\n");
> - buf->state = UVC_BUF_STATE_READY;
> - return -EAGAIN;
> - }
> -
> - /*
> - * Some cameras, when running two parallel streams (one MJPEG alongside
> - * another non-MJPEG stream), are known to lose the EOF packet for a frame.
> - * We can detect the end of a frame by checking for a new SOI marker, as
> - * the SOI always lies on the packet boundary between two frames for
> - * these devices.
> - */
> - if (stream->dev->quirks & UVC_QUIRK_MJPEG_NO_EOF &&
> - (stream->cur_format->fcc == V4L2_PIX_FMT_MJPEG ||
> - stream->cur_format->fcc == V4L2_PIX_FMT_JPEG)) {
> - const u8 *packet = data + header_len;
> -
> - if (len >= header_len + 2 &&
> - packet[0] == 0xff && packet[1] == JPEG_MARKER_SOI &&
> - buf->bytesused != 0) {
> - buf->state = UVC_BUF_STATE_READY;
> - buf->error = 1;
> - stream->last_fid ^= UVC_STREAM_FID;
> - return -EAGAIN;
> - }
> - }
> -
> stream->last_fid = fid;
>
> return header_len;
>
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v5 1/2] media: uvcvideo: Fix sequence number when no EOF
2026-03-24 0:20 ` Laurent Pinchart
@ 2026-03-24 7:23 ` Ricardo Ribalda
0 siblings, 0 replies; 5+ messages in thread
From: Ricardo Ribalda @ 2026-03-24 7:23 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Hans de Goede, Mauro Carvalho Chehab, linux-media, linux-kernel,
Yunke Cao, stable
Hi Laurent
On Tue, 24 Mar 2026 at 01:20, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Mon, Mar 23, 2026 at 09:53:52AM +0000, 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
> > flipped => 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 moving the new frame detection logic earlier in
> > uvc_video_decode_start().
> >
> > This also has some nice side affects:
> >
> > - The error status from the new packet will no longer get propagated
> > to the previous frame-buffer.
> > - uvc_video_clock_decode() will no longer update the previous frame
> > buf->stf with info from the new packet.
> > - uvc_video_clock_decode() and uvc_video_stats_decode() will no longer
> > get called twice for the same packet.
> >
> > 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 | 92 ++++++++++++++++++++-------------------
> > 1 file changed, 47 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> > index 40c76c051da2..eddb4821b205 100644
> > --- a/drivers/media/usb/uvc/uvc_video.c
> > +++ b/drivers/media/usb/uvc/uvc_video.c
> > @@ -1168,6 +1168,53 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
> > header_len = data[0];
> > fid = data[1] & UVC_STREAM_FID;
> >
> > + /*
> > + * Mark the buffer as done if we're at the beginning of a new frame.
> > + * End of frame detection is better implemented by checking the EOF
> > + * bit (FID bit toggling is delayed by one frame compared to the EOF
> > + * bit), but some devices don't set the bit at end of frame (and the
> > + * last payload can be lost anyway). We thus must check if the FID has
> > + * been toggled.
> > + *
> > + * stream->last_fid is initialized to -1, and buf->bytesused to 0,
> > + * so the first isochronous frame will never trigger an end of frame
> > + * detection.
> > + *
> > + * Empty buffers (bytesused == 0) don't trigger end of frame detection
> > + * as it doesn't make sense to return an empty buffer. This also
> > + * avoids detecting end of frame conditions at FID toggling if the
> > + * previous payload had the EOF bit set.
> > + */
> > + if (fid != stream->last_fid && buf && buf->bytesused != 0) {
> > + uvc_dbg(stream->dev, FRAME,
> > + "Frame complete (FID bit toggled)\n");
> > + buf->state = UVC_BUF_STATE_READY;
> > +
> > + return -EAGAIN;
> > + }
> > +
> > + /*
> > + * Some cameras, when running two parallel streams (one MJPEG alongside
> > + * another non-MJPEG stream), are known to lose the EOF packet for a frame.
> > + * We can detect the end of a frame by checking for a new SOI marker, as
> > + * the SOI always lies on the packet boundary between two frames for
> > + * these devices.
> > + */
> > + if (stream->dev->quirks & UVC_QUIRK_MJPEG_NO_EOF &&
> > + (stream->cur_format->fcc == V4L2_PIX_FMT_MJPEG ||
> > + stream->cur_format->fcc == V4L2_PIX_FMT_JPEG)) {
> > + const u8 *packet = data + header_len;
> > +
> > + if (len >= header_len + 2 &&
> > + packet[0] == 0xff && packet[1] == JPEG_MARKER_SOI &&
> > + buf && buf->bytesused != 0) {
>
> How about moving the buf && buf->bytesused != 0 to the outer condition,
> so that we don't inspect the packet when we don't need to ? It will also
> make the two end of frame detection blocks more similar.
SGTM. I will change this locally and upload it after someone reviews
2/2 to avoid bloating the list
Thanks!
>
> The patch otherwise looks fine, I don't see anything below this code
> that would need to be performed before.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> > + buf->state = UVC_BUF_STATE_READY;
> > + buf->error = 1;
> > + stream->last_fid ^= UVC_STREAM_FID;
> > + return -EAGAIN;
> > + }
> > + }
> > +
> > /*
> > * Increase the sequence number regardless of any buffer states, so
> > * that discontinuous sequence numbers always indicate lost frames.
> > @@ -1224,51 +1271,6 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
> > buf->state = UVC_BUF_STATE_ACTIVE;
> > }
> >
> > - /*
> > - * Mark the buffer as done if we're at the beginning of a new frame.
> > - * End of frame detection is better implemented by checking the EOF
> > - * bit (FID bit toggling is delayed by one frame compared to the EOF
> > - * bit), but some devices don't set the bit at end of frame (and the
> > - * last payload can be lost anyway). We thus must check if the FID has
> > - * been toggled.
> > - *
> > - * stream->last_fid is initialized to -1, so the first isochronous
> > - * frame will never trigger an end of frame detection.
> > - *
> > - * Empty buffers (bytesused == 0) don't trigger end of frame detection
> > - * as it doesn't make sense to return an empty buffer. This also
> > - * avoids detecting end of frame conditions at FID toggling if the
> > - * previous payload had the EOF bit set.
> > - */
> > - if (fid != stream->last_fid && buf->bytesused != 0) {
> > - uvc_dbg(stream->dev, FRAME,
> > - "Frame complete (FID bit toggled)\n");
> > - buf->state = UVC_BUF_STATE_READY;
> > - return -EAGAIN;
> > - }
> > -
> > - /*
> > - * Some cameras, when running two parallel streams (one MJPEG alongside
> > - * another non-MJPEG stream), are known to lose the EOF packet for a frame.
> > - * We can detect the end of a frame by checking for a new SOI marker, as
> > - * the SOI always lies on the packet boundary between two frames for
> > - * these devices.
> > - */
> > - if (stream->dev->quirks & UVC_QUIRK_MJPEG_NO_EOF &&
> > - (stream->cur_format->fcc == V4L2_PIX_FMT_MJPEG ||
> > - stream->cur_format->fcc == V4L2_PIX_FMT_JPEG)) {
> > - const u8 *packet = data + header_len;
> > -
> > - if (len >= header_len + 2 &&
> > - packet[0] == 0xff && packet[1] == JPEG_MARKER_SOI &&
> > - buf->bytesused != 0) {
> > - buf->state = UVC_BUF_STATE_READY;
> > - buf->error = 1;
> > - stream->last_fid ^= UVC_STREAM_FID;
> > - return -EAGAIN;
> > - }
> > - }
> > -
> > stream->last_fid = fid;
> >
> > return header_len;
> >
>
> --
> Regards,
>
> Laurent Pinchart
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-03-24 7:24 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-23 9:53 [PATCH v5 0/2] media: uvcvideo: Fixes for frame sequence number Ricardo Ribalda
2026-03-23 9:53 ` [PATCH v5 1/2] media: uvcvideo: Fix sequence number when no EOF Ricardo Ribalda
2026-03-24 0:20 ` Laurent Pinchart
2026-03-24 7:23 ` Ricardo Ribalda
2026-03-23 9:53 ` [PATCH v5 2/2] media: uvcvideo: Fix buffer sequence in frame gaps Ricardo Ribalda
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox