Re: [PATCH] media: uvcvideo: Fix buffer sequence in frame gaps

[Ricardo Ribalda]

Hi Laurent

On Fri, 13 Mar 2026 at 17:51, Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>
> On Tue, Mar 10, 2026 at 11:54:24AM +0000, Ricardo Ribalda wrote:
> > In UVC, the FID flips with every frame. For every FID flip, we increase
> > the sequence number. Userpace use that information to figure out if
> > there has been a gap between frames.
> >
> > Now, we only update the sequence number on the first FID flip, which
> > results in frames being incorrectly numbered just after a frame gap.
>
> The sequence number is increased on every FID flip. I assume you mean
> that we don't set the buffer sequence number in some cases. "on the
> first FID flip" doesn't seem right. Could you please improve the commit
> message to describe more clearly what the problem is ?

Will do my best in v2 :)

>
> > This patch rewrites the sequence number of the buffer in those
> > situations.
> >
> > Cc: stable@xxxxxxxxxx
> > Fixes: 650b95feee35 ("[media] uvcvideo: Generate discontinuous sequence numbers when frames are lost")
> > Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
> > ---
> >  drivers/media/usb/uvc/uvc_video.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> > index 40c76c051da2..ec769a504569 100644
> > --- a/drivers/media/usb/uvc/uvc_video.c
> > +++ b/drivers/media/usb/uvc/uvc_video.c
> > @@ -1176,6 +1176,14 @@ static int uvc_video_decode_start(struct uvc_streaming *stream,
> >               stream->sequence++;
> >               if (stream->sequence)
> >                       uvc_video_stats_update(stream);
> > +
> > +             /*
> > +              * If we have not received any data and FID flips, update the
> > +              * sequence number of the buffer to tell userspace exactly
> > +              * where the frame gap is.
> > +              */
> > +             if (buf && !buf->bytesused)
> > +                     buf->buf.sequence = stream->sequence;
>
> Do we still need to set buf->buf.sequence below ? And how about the
> buffer timestamp, isn't it also not set in the case that this patch
> addresses ?

Yep, it makes more sense to move the whole buffer init here. Let me do
that and add a comment explaining why it also works if the hardware
does not set the FID properly.

Thanks!

>
> >       }
> >
> >       uvc_video_clock_decode(stream, buf, data, len);
> >
> > ---
> > base-commit: a7da7fb57f2a787412da1a62292a17fa00fbfbdf
> > change-id: 20260310-uvc-fid-e1e55447b6f1
>
> --
> Regards,
>
> Laurent Pinchart



-- 
Ricardo Ribalda