On Mon Mar 23 09:53:52 2026 +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: [email protected]
> Fixes: 650b95feee35 ("[media] uvcvideo: Generate discontinuous sequence
> numbers when frames are lost")
> Reported-by: Hans de Goede <[email protected]>
> Closes:
> https://lore.kernel.org/linux-media/canidscuj4cpub5_v2xyvaaga5fjon8v5scxiffoed3akdmq...@mail.gmail.com/T/#me39fb134e8c2c085567a31548c3403eb639625e4
> Signed-off-by: Ricardo Ribalda <[email protected]>
> Reviewed-by: Laurent Pinchart <[email protected]>
> Reviewed-by: Hans de Goede <[email protected]>
> Signed-off-by: Hans de Goede <[email protected]>
> Signed-off-by: Hans Verkuil <[email protected]>
Patch committed.
Thanks,
Hans Verkuil
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 f6c8e3223796..3182f5e9d9a0 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) &&
+ buf && buf->bytesused != 0) {
+ const u8 *packet = data + header_len;
+
+ if (len >= header_len + 2 &&
+ packet[0] == 0xff && packet[1] == JPEG_MARKER_SOI) {
+ 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;
_______________________________________________
linuxtv-commits mailing list -- [email protected]
To unsubscribe send an email to [email protected]
