public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] media: uvcvideo: Fix buffer sequence in frame gaps
@ 2026-03-10 11:54 Ricardo Ribalda
  2026-03-13 16:51 ` Laurent Pinchart
  0 siblings, 1 reply; 3+ messages in thread
From: Ricardo Ribalda @ 2026-03-10 11:54 UTC (permalink / raw)
  To: Laurent Pinchart, Hans de Goede, Mauro Carvalho Chehab
  Cc: linux-media, linux-kernel, Yunke Cao, stable, Ricardo Ribalda

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.

This patch rewrites the sequence number of the buffer in those
situations.

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 | 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;
 	}
 
 	uvc_video_clock_decode(stream, buf, data, len);

---
base-commit: a7da7fb57f2a787412da1a62292a17fa00fbfbdf
change-id: 20260310-uvc-fid-e1e55447b6f1

Best regards,
-- 
Ricardo Ribalda <ribalda@chromium.org>


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] media: uvcvideo: Fix buffer sequence in frame gaps
  2026-03-10 11:54 [PATCH] media: uvcvideo: Fix buffer sequence in frame gaps Ricardo Ribalda
@ 2026-03-13 16:51 ` Laurent Pinchart
  2026-03-13 18:16   ` Ricardo Ribalda
  0 siblings, 1 reply; 3+ messages in thread
From: Laurent Pinchart @ 2026-03-13 16:51 UTC (permalink / raw)
  To: Ricardo Ribalda
  Cc: Hans de Goede, Mauro Carvalho Chehab, linux-media, linux-kernel,
	Yunke Cao, stable

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 ?

> This patch rewrites the sequence number of the buffer in those
> situations.
> 
> 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 | 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 ?

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

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] media: uvcvideo: Fix buffer sequence in frame gaps
  2026-03-13 16:51 ` Laurent Pinchart
@ 2026-03-13 18:16   ` Ricardo Ribalda
  0 siblings, 0 replies; 3+ messages in thread
From: Ricardo Ribalda @ 2026-03-13 18:16 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Hans de Goede, Mauro Carvalho Chehab, linux-media, linux-kernel,
	Yunke Cao, stable

Hi Laurent

On Fri, 13 Mar 2026 at 17:51, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> 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@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 | 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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-03-13 18:17 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-10 11:54 [PATCH] media: uvcvideo: Fix buffer sequence in frame gaps Ricardo Ribalda
2026-03-13 16:51 ` Laurent Pinchart
2026-03-13 18:16   ` Ricardo Ribalda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox