* [PATCH] media: uvcvideo: Handle payloads starting in the middle of a bulk URB
@ 2026-10-09 16:52 Muhammad Waleed Badar
2026-10-12 1:54 ` Ricardo Ribalda
0 siblings, 1 reply; 2+ messages in thread
From: Muhammad Waleed Badar @ 2026-10-09 16:52 UTC (permalink / raw)
To: linux-media, Laurent Pinchart, Hans de Goede
Cc: Mauro Carvalho Chehab, linux-kernel, Muhammad Waleed Badar
The bulk decoder assumes that every payload starts at the beginning of
a URB. It detects the end of a payload either by a short URB or by the
accumulated payload size reaching dwMaxPayloadTransferSize, and then
decodes the start of the next URB as a new payload header.
Some devices, such as the Realtek USB Camera (0bda:579f) integrated in
Razer Blade laptops, send payloads of dwMaxPayloadTransferSize bytes
back to back without aligning them to URB boundaries. When a frame
spans more than one payload, the next payload header lands in the
middle of a URB. The driver then copies that header and the data that
follows into the current frame, and parses frame data at the start of
the next URB as a header. This shows up as "Marking buffer as bad
(error bit set)" and "Dropping payload (out of sync)" trace messages,
and most frames larger than one payload are returned empty or
truncated. On this camera every MJPEG mode above 1280x720 is affected.
Fix it by splitting URBs at payload boundaries and decoding each part
separately, so that a payload ending in the middle of a URB is
completed and the remainder of the URB is decoded as a new payload.
Devices whose payloads end on URB boundaries are unaffected, as the
whole URB is then processed in a single iteration.
Fixes: c0efd232929c ("V4L/DVB (8145a): USB Video Class driver")
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=207045
Assisted-by: LLM
Signed-off-by: Muhammad Waleed Badar <walid.badar@gmail.com>
---
Notes:
Tested on a Razer Blade with the integrated Realtek 0bda:579f camera
(bulk endpoint, wMaxPacketSize 512), on Fedora kernel 7.1.10 with the
patch applied to the matching uvcvideo sources. 150 MJPEG frames were
captured per mode with ffmpeg and checked for a valid JPEG EOI marker:
mode before after
1920x1080 25-36 valid 150 valid
1280x1024 37 valid 150 valid
1280x800 38 valid 148 valid
1280x720 150 valid 150 valid
640x480 150 valid 150 valid
GNOME Snapshot (PipeWire) no longer reports JPEG decode errors at
1920x1080. The patch also builds against 7.2.8. It has not been tested
on other bulk-mode UVC cameras.
The root cause analysis and the patch were produced with an AI coding
assistant (Claude Code) and reviewed by me.
drivers/media/usb/uvc/uvc_video.c | 70 ++++++++++++++++++++++++-------
1 file changed, 54 insertions(+), 16 deletions(-)
diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index fc3536a43..fe1714ccc 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1612,27 +1612,20 @@ static void uvc_video_decode_isoc(struct uvc_urb *uvc_urb,
}
}
-static void uvc_video_decode_bulk(struct uvc_urb *uvc_urb,
- struct uvc_buffer *buf, struct uvc_buffer *meta_buf)
+static void uvc_video_decode_bulk_chunk(struct uvc_urb *uvc_urb,
+ struct uvc_buffer **bufp,
+ struct uvc_buffer **meta_bufp,
+ u8 *mem, int len, bool short_urb)
{
- struct urb *urb = uvc_urb->urb;
struct uvc_streaming *stream = uvc_urb->stream;
- u8 *mem;
- int len, ret;
-
- /*
- * Ignore ZLPs if they're not part of a frame, otherwise process them
- * to trigger the end of payload detection.
- */
- if (urb->actual_length == 0 && stream->bulk.header_size == 0)
- return;
+ struct uvc_buffer *buf = *bufp;
+ struct uvc_buffer *meta_buf = *meta_bufp;
+ int ret;
- mem = urb->transfer_buffer;
- len = urb->actual_length;
stream->bulk.payload_size += len;
/*
- * If the URB is the first of its payload, decode and save the
+ * If the chunk is the first of its payload, decode and save the
* header.
*/
if (stream->bulk.header_size == 0 && !stream->bulk.skip_payload) {
@@ -1671,7 +1664,7 @@ static void uvc_video_decode_bulk(struct uvc_urb *uvc_urb,
* Detect the payload end by a URB smaller than the maximum size (or
* a payload size equal to the maximum) and process the header again.
*/
- if (urb->actual_length < urb->transfer_buffer_length ||
+ if (short_urb ||
stream->bulk.payload_size >= stream->bulk.max_payload_size) {
if (!stream->bulk.skip_payload && buf != NULL) {
uvc_video_decode_end(stream, buf, stream->bulk.header,
@@ -1684,6 +1677,51 @@ static void uvc_video_decode_bulk(struct uvc_urb *uvc_urb,
stream->bulk.skip_payload = 0;
stream->bulk.payload_size = 0;
}
+
+ *bufp = buf;
+ *meta_bufp = meta_buf;
+}
+
+static void uvc_video_decode_bulk(struct uvc_urb *uvc_urb,
+ struct uvc_buffer *buf, struct uvc_buffer *meta_buf)
+{
+ struct urb *urb = uvc_urb->urb;
+ struct uvc_streaming *stream = uvc_urb->stream;
+ bool short_urb = urb->actual_length < urb->transfer_buffer_length;
+ u8 *mem;
+ int len;
+
+ /*
+ * Ignore ZLPs if they're not part of a frame, otherwise process them
+ * to trigger the end of payload detection.
+ */
+ if (urb->actual_length == 0 && stream->bulk.header_size == 0)
+ return;
+
+ mem = urb->transfer_buffer;
+ len = urb->actual_length;
+
+ /*
+ * Some devices (such as the Realtek 0bda:579f) start the next payload
+ * immediately after the previous one instead of in a new URB, so a
+ * payload boundary can fall in the middle of a URB. Split the URB at
+ * payload boundaries and decode each piece separately.
+ */
+ do {
+ int chunk = len;
+
+ if (stream->bulk.payload_size + chunk >
+ stream->bulk.max_payload_size)
+ chunk = stream->bulk.max_payload_size -
+ stream->bulk.payload_size;
+ if (chunk <= 0)
+ chunk = len;
+
+ uvc_video_decode_bulk_chunk(uvc_urb, &buf, &meta_buf, mem,
+ chunk, short_urb && chunk == len);
+ mem += chunk;
+ len -= chunk;
+ } while (len > 0);
}
static void uvc_video_encode_bulk(struct uvc_urb *uvc_urb,
--
2.55.0
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [PATCH] media: uvcvideo: Handle payloads starting in the middle of a bulk URB
2026-10-09 16:52 [PATCH] media: uvcvideo: Handle payloads starting in the middle of a bulk URB Muhammad Waleed Badar
@ 2026-10-12 1:54 ` Ricardo Ribalda
0 siblings, 0 replies; 2+ messages in thread
From: Ricardo Ribalda @ 2026-10-12 1:54 UTC (permalink / raw)
To: Muhammad Waleed Badar
Cc: linux-media, Laurent Pinchart, Hans de Goede,
Mauro Carvalho Chehab, linux-kernel
Hi Muhammad
I understand that this is a RFC. Let's focus on the problem before we
discuss the code.
Currently, both for isoc and bulk, if the camera appends some data at
the end of the frame (let's call it trailer data) it is silently
discarded. With your proposal, this trailer data will be considered
part of the next frame.
The trailer data is out of spec (check figure 2-12 of the spec), but
having multiple frames in the same bulk package is also out of spec
(For some reason the bulk cameras are the worst at following the spec,
no idea why).
Your proposal (again, not looking at the code yet), will fix the
problem for some of the broken cameras but ti will completely break
the driver for other family of broken cameras (the ones that have
trailer data).
If we want to automatically handle your family of broken cameras we should:
- Add heuristics to also handle the camera with trailer camera (does
the extra data look like a uvc header?)
- Add a non compliance warning so the vendors have some feedback about
their wrongness.
Regards!
On Sat, 10 Oct 2026 at 00:58, Muhammad Waleed Badar
<walid.badar@gmail.com> wrote:
>
> The bulk decoder assumes that every payload starts at the beginning of
> a URB. It detects the end of a payload either by a short URB or by the
> accumulated payload size reaching dwMaxPayloadTransferSize, and then
> decodes the start of the next URB as a new payload header.
>
> Some devices, such as the Realtek USB Camera (0bda:579f) integrated in
> Razer Blade laptops, send payloads of dwMaxPayloadTransferSize bytes
> back to back without aligning them to URB boundaries. When a frame
> spans more than one payload, the next payload header lands in the
> middle of a URB. The driver then copies that header and the data that
> follows into the current frame, and parses frame data at the start of
> the next URB as a header. This shows up as "Marking buffer as bad
> (error bit set)" and "Dropping payload (out of sync)" trace messages,
> and most frames larger than one payload are returned empty or
> truncated. On this camera every MJPEG mode above 1280x720 is affected.
>
> Fix it by splitting URBs at payload boundaries and decoding each part
> separately, so that a payload ending in the middle of a URB is
> completed and the remainder of the URB is decoded as a new payload.
> Devices whose payloads end on URB boundaries are unaffected, as the
> whole URB is then processed in a single iteration.
>
> Fixes: c0efd232929c ("V4L/DVB (8145a): USB Video Class driver")
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=207045
> Assisted-by: LLM
> Signed-off-by: Muhammad Waleed Badar <walid.badar@gmail.com>
> ---
>
> Notes:
> Tested on a Razer Blade with the integrated Realtek 0bda:579f camera
> (bulk endpoint, wMaxPacketSize 512), on Fedora kernel 7.1.10 with the
> patch applied to the matching uvcvideo sources. 150 MJPEG frames were
> captured per mode with ffmpeg and checked for a valid JPEG EOI marker:
>
> mode before after
> 1920x1080 25-36 valid 150 valid
> 1280x1024 37 valid 150 valid
> 1280x800 38 valid 148 valid
> 1280x720 150 valid 150 valid
> 640x480 150 valid 150 valid
>
> GNOME Snapshot (PipeWire) no longer reports JPEG decode errors at
> 1920x1080. The patch also builds against 7.2.8. It has not been tested
> on other bulk-mode UVC cameras.
>
> The root cause analysis and the patch were produced with an AI coding
> assistant (Claude Code) and reviewed by me.
>
> drivers/media/usb/uvc/uvc_video.c | 70 ++++++++++++++++++++++++-------
> 1 file changed, 54 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index fc3536a43..fe1714ccc 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -1612,27 +1612,20 @@ static void uvc_video_decode_isoc(struct uvc_urb *uvc_urb,
> }
> }
>
> -static void uvc_video_decode_bulk(struct uvc_urb *uvc_urb,
> - struct uvc_buffer *buf, struct uvc_buffer *meta_buf)
> +static void uvc_video_decode_bulk_chunk(struct uvc_urb *uvc_urb,
> + struct uvc_buffer **bufp,
> + struct uvc_buffer **meta_bufp,
> + u8 *mem, int len, bool short_urb)
> {
> - struct urb *urb = uvc_urb->urb;
> struct uvc_streaming *stream = uvc_urb->stream;
> - u8 *mem;
> - int len, ret;
> -
> - /*
> - * Ignore ZLPs if they're not part of a frame, otherwise process them
> - * to trigger the end of payload detection.
> - */
> - if (urb->actual_length == 0 && stream->bulk.header_size == 0)
> - return;
> + struct uvc_buffer *buf = *bufp;
> + struct uvc_buffer *meta_buf = *meta_bufp;
> + int ret;
>
> - mem = urb->transfer_buffer;
> - len = urb->actual_length;
> stream->bulk.payload_size += len;
>
> /*
> - * If the URB is the first of its payload, decode and save the
> + * If the chunk is the first of its payload, decode and save the
> * header.
> */
> if (stream->bulk.header_size == 0 && !stream->bulk.skip_payload) {
> @@ -1671,7 +1664,7 @@ static void uvc_video_decode_bulk(struct uvc_urb *uvc_urb,
> * Detect the payload end by a URB smaller than the maximum size (or
> * a payload size equal to the maximum) and process the header again.
> */
> - if (urb->actual_length < urb->transfer_buffer_length ||
> + if (short_urb ||
> stream->bulk.payload_size >= stream->bulk.max_payload_size) {
> if (!stream->bulk.skip_payload && buf != NULL) {
> uvc_video_decode_end(stream, buf, stream->bulk.header,
> @@ -1684,6 +1677,51 @@ static void uvc_video_decode_bulk(struct uvc_urb *uvc_urb,
> stream->bulk.skip_payload = 0;
> stream->bulk.payload_size = 0;
> }
> +
> + *bufp = buf;
> + *meta_bufp = meta_buf;
> +}
> +
> +static void uvc_video_decode_bulk(struct uvc_urb *uvc_urb,
> + struct uvc_buffer *buf, struct uvc_buffer *meta_buf)
> +{
> + struct urb *urb = uvc_urb->urb;
> + struct uvc_streaming *stream = uvc_urb->stream;
> + bool short_urb = urb->actual_length < urb->transfer_buffer_length;
> + u8 *mem;
> + int len;
> +
> + /*
> + * Ignore ZLPs if they're not part of a frame, otherwise process them
> + * to trigger the end of payload detection.
> + */
> + if (urb->actual_length == 0 && stream->bulk.header_size == 0)
> + return;
> +
> + mem = urb->transfer_buffer;
> + len = urb->actual_length;
> +
> + /*
> + * Some devices (such as the Realtek 0bda:579f) start the next payload
> + * immediately after the previous one instead of in a new URB, so a
> + * payload boundary can fall in the middle of a URB. Split the URB at
> + * payload boundaries and decode each piece separately.
> + */
> + do {
> + int chunk = len;
> +
> + if (stream->bulk.payload_size + chunk >
> + stream->bulk.max_payload_size)
> + chunk = stream->bulk.max_payload_size -
> + stream->bulk.payload_size;
> + if (chunk <= 0)
> + chunk = len;
> +
> + uvc_video_decode_bulk_chunk(uvc_urb, &buf, &meta_buf, mem,
> + chunk, short_urb && chunk == len);
> + mem += chunk;
> + len -= chunk;
> + } while (len > 0);
> }
>
> static void uvc_video_encode_bulk(struct uvc_urb *uvc_urb,
> --
> 2.55.0
>
>
--
Ricardo Ribalda
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-12 1:54 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-09 16:52 [PATCH] media: uvcvideo: Handle payloads starting in the middle of a bulk URB Muhammad Waleed Badar
2026-10-12 1:54 ` Ricardo Ribalda
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®