media: uvcvideo: Fix sequence number when no EOF

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>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index f6c8e322379686d8e21a0fc665677f82ddbbc75d..3182f5e9d9a0962cb597d8ae79bfd5a3702e6dc2 100644 (file)
--- 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;