Re: [PATCH] media: uvcvideo: Handle payloads starting in the middle of a bulk URB

[Ricardo Ribalda]

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@xxxxxxxxx> 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@xxxxxxxxx>
> ---
>
> 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