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

[Ricardo Ribalda]

On Fri, 20 Mar 2026 at 14:35, Ricardo Ribalda <ribalda@xxxxxxxxxxxx> 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 moving the new frame detection logic earlier in
> uvc_video_decode_start().
>
> Cc: stable@xxxxxxxxxx
> Fixes: 650b95feee35 ("[media] uvcvideo: Generate discontinuous sequence numbers when frames are lost")
> Reported-by: Hans de Goede <hansg@xxxxxxxxxx>
> Closes: https://lore.kernel.org/linux-media/CANiDSCuj4cPuB5_v2xyvAagA5FjoN8V5scXiFFOeD3aKDMqkCg@xxxxxxxxxxxxxx/T/#me39fb134e8c2c085567a31548c3403eb639625e4
> Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
> ---
>  drivers/media/usb/uvc/uvc_video.c | 48 ++++++++++++++++++++-------------------
>  1 file changed, 25 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index 9e06b1d0f0f9..2218e4d8e564 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1168,6 +1168,31 @@ 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, 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 (stream->last_fid != -1 && fid != stream->last_fid &&
> +           buf && buf->bytesused != 0) {

This should be
if (stream->last_fid != (u8) -1 && fid != stream->last_fid &&
    buf && buf->bytesused != 0) {


I will wait a bit for more comments before sending a v5

Have a good weekend

> +               uvc_dbg(stream->dev, FRAME,
> +                       "Frame complete (FID bit toggled)\n");
> +               buf->state = UVC_BUF_STATE_READY;
> +
> +               return -EAGAIN;
> +       }
> +
>         /*
>          * Increase the sequence number regardless of any buffer states, so
>          * that discontinuous sequence numbers always indicate lost frames.
> @@ -1234,29 +1259,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.
>
> --
> 2.53.0.959.g497ff81fa9-goog
>


-- 
Ricardo Ribalda