All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: "Pol Fernández Fernández" <fernandezfernandezpol@gmail.com>
Cc: linux-media@vger.kernel.org, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] media: uvcvideo: add quirks for per-device stream error handling
Date: Mon, 29 Jun 2026 21:23:47 +0300	[thread overview]
Message-ID: <20260629182347.GI3054459@killaraus.ideasonboard.com> (raw)
In-Reply-To: <20260629163805.37879-1-fernandezfernandezpol@gmail.com>

On Mon, Jun 29, 2026 at 06:38:02PM +0200, Pol Fernández Fernández wrote:
> The UVC_STREAM_ERR bit in the payload header signals that the device
> could not deliver a frame correctly. By default, uvcvideo delivers
> these frames to userspace with V4L2_BUF_FLAG_ERROR when uvc_no_drop_param
> is set (the default). Applications that rely on strict JPEG decoders
> (e.g. libjpeg) will display gray bands for the incomplete portion of
> the image, since libjpeg fills undecodable MCU rows with a neutral gray
> when the EOI marker is missing.
> 
> Add UVC_QUIRK_DROP_STREAM_ERR to force-drop frames flagged with
> UVC_STREAM_ERR for specific devices, regardless of the uvc_no_drop_param
> module parameter. This ensures applications never receive truncated frames
> from devices known to set UVC_STREAM_ERR on genuine encoding errors.

That completely defeats the point of uvc_no_drop_param. The issue should
be fixed in userspace, where you should ignore frames with the error
flag set if desired.

> The existing uvc_queue_to_stream() helper is used to reach the device
> quirks from within uvc_queue_buffer_complete().
> 
> Additionally, uvcvideo unconditionally overrides the camera's probed
> wCompQuality with the maximum value returned by GET_MAX. For devices
> whose firmware treats wCompQuality=0 as an adaptive encoding mode (where
> the encoder adjusts quality dynamically to fit within the available USB
> bandwidth),

The UVC 1.1 specification documents the range wCompQuality values as

"Values for this property range from 0 to 10000 (0 indicates the lowest
quality, 10000 the highest)."

While the UVC 1.5 specification documents it as

"Values for this property range from 1 to 10000 (1 indicates the lowest
quality, 10000 the highest)."

There is no mention of 0 being allowed as a magic value. Where did you
find information about the adaptive encoding mode ?

> this override increases encoding pressure and can contribute
> to UVC_STREAM_ERR events. Add UVC_QUIRK_NO_FORCE_QUALITY to preserve
> the camera's default quality value during probe negotiation.
> 
> Signed-off-by: Pol Fernández Fernández <fernandezfernandezpol@gmail.com>

Please split this patch in two as it addresses two different issues.

> ---
>  drivers/media/usb/uvc/uvc_queue.c | 3 ++-
>  drivers/media/usb/uvc/uvc_video.c | 3 ++-
>  drivers/media/usb/uvc/uvcvideo.h  | 2 ++
>  3 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index XXXXXXX..XXXXXXX 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -81,6 +81,8 @@
>  #define UVC_QUIRK_INVALID_DEVICE_SOF	0x00010000
>  #define UVC_QUIRK_MJPEG_NO_EOF		0x00020000
>  #define UVC_QUIRK_MSXU_META		0x00040000
> +#define UVC_QUIRK_DROP_STREAM_ERR	0x00080000
> +#define UVC_QUIRK_NO_FORCE_QUALITY	0x00100000
> 
>  /* Format flags */
>  #define UVC_FMT_FLAG_COMPRESSED		0x00000001
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index XXXXXXX..XXXXXXX 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -451,7 +451,8 @@ int uvc_probe_video(struct uvc_streaming *stream,
>  		if (ret < 0)
>  			goto done;
> 
> -		probe->wCompQuality = probe_max.wCompQuality;
> +		if (!(stream->dev->quirks & UVC_QUIRK_NO_FORCE_QUALITY))
> +			probe->wCompQuality = probe_max.wCompQuality;
>  	}
> 
>  	for (i = 0; i < 2; ++i) {
> diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
> index XXXXXXX..XXXXXXX 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -357,7 +357,8 @@ static void uvc_queue_buffer_complete(struct kref *ref)
>  	struct vb2_buffer *vb = &buf->buf.vb2_buf;
>  	struct uvc_video_queue *queue = vb2_get_drv_priv(vb->vb2_queue);
> 
> -	if (buf->error && !uvc_no_drop_param) {
> +	if (buf->error && (!uvc_no_drop_param ||
> +	    (uvc_queue_to_stream(queue)->dev->quirks & UVC_QUIRK_DROP_STREAM_ERR))) {
>  		uvc_queue_buffer_requeue(queue, buf);
>  		return;
>  	}

-- 
Regards,

Laurent Pinchart

      parent reply	other threads:[~2026-06-29 18:23 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-29 16:38 [PATCH 1/2] media: uvcvideo: add quirks for per-device stream error handling Pol Fernández Fernández
2026-06-29 16:38 ` [PATCH 2/2] media: uvcvideo: add device entry for Logitech C920 (046d:08e5) Pol Fernández Fernández
2026-06-29 16:55 ` [PATCH 1/2] media: uvcvideo: add quirks for per-device stream error handling Ricardo Ribalda
     [not found]   ` <CAD0bCtmJ88fC3zXhU5nDxugagjNMmY=FS7sO40x3OsAe41L+qg@mail.gmail.com>
2026-06-29 18:55     ` Ricardo Ribalda
2026-06-29 18:23 ` Laurent Pinchart [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260629182347.GI3054459@killaraus.ideasonboard.com \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=fernandezfernandezpol@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.