Re: [PATCH 3/3] media: uvcvideo: Allow userspace to increase the meta buffersize

[Hans de Goede <hansg@kernel.org>]

Hi,

On 9-Mar-26 4:01 PM, Ricardo Ribalda wrote:
> Now we have the metadata size hardcoded to 10 KiB, this is a value that
> works fine for bulk cameras or frames with no extra metadata. But not
> for all usecases.
> 
> We have seen some cameras that produce more metadata per frame. Eg:
> Frame 1 captured (Bytes: 11154)
> Frame 2 captured (Bytes: 11616)
> Frame 3 captured (Bytes: 11374)
> Frame 4 captured (Bytes: 11132)
> Frame 5 captured (Bytes: 11594)
> Frame 6 captured (Bytes: 11352)
> Frame 7 captured (Bytes: 11110)
> Frame 8 captured (Bytes: 11572)
> Frame 9 captured (Bytes: 11308)
> 
> When this happens, the driver (correctly) marks the metadata as ERROR.
> 
> This patch let userspace set bigger buffersize via S_FMT.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>

Regards,

Hans




> ---
>  drivers/media/usb/uvc/uvc_metadata.c | 9 +++++++--
>  drivers/media/usb/uvc/uvc_queue.c    | 2 +-
>  drivers/media/usb/uvc/uvcvideo.h     | 3 ++-
>  3 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
> index 0a906ae3f971..9de8aba1229e 100644
> --- a/drivers/media/usb/uvc/uvc_metadata.c
> +++ b/drivers/media/usb/uvc/uvc_metadata.c
> @@ -50,7 +50,7 @@ static int uvc_meta_v4l2_get_format(struct file *file, void *priv,
>  		return -EINVAL;
>  
>  	fmt->dataformat = stream->meta.format;
> -	fmt->buffersize = UVC_METADATA_BUF_SIZE;
> +	fmt->buffersize = stream->meta.buffersize;
>  
>  	return 0;
>  }
> @@ -63,6 +63,7 @@ static int uvc_meta_v4l2_try_format(struct file *file, void *priv,
>  	struct uvc_device *dev = stream->dev;
>  	struct v4l2_meta_format *fmt = &format->fmt.meta;
>  	u32 fmeta = V4L2_META_FMT_UVC;
> +	u32 buffersize;
>  
>  	if (format->type != vfh->vdev->queue->type)
>  		return -EINVAL;
> @@ -74,10 +75,12 @@ static int uvc_meta_v4l2_try_format(struct file *file, void *priv,
>  		}
>  	}
>  
> +	buffersize = max(UVC_METADATA_BUF_MIN_SIZE, fmt->buffersize);
> +
>  	memset(fmt, 0, sizeof(*fmt));
>  
>  	fmt->dataformat = fmeta;
> -	fmt->buffersize = UVC_METADATA_BUF_SIZE;
> +	fmt->buffersize = buffersize;
>  
>  	return 0;
>  }
> @@ -103,6 +106,7 @@ static int uvc_meta_v4l2_set_format(struct file *file, void *priv,
>  		return -EBUSY;
>  
>  	stream->meta.format = fmt->dataformat;
> +	stream->meta.buffersize = fmt->buffersize;
>  
>  	return 0;
>  }
> @@ -229,6 +233,7 @@ int uvc_meta_register(struct uvc_streaming *stream)
>  	struct uvc_video_queue *queue = &stream->meta.queue;
>  
>  	stream->meta.format = V4L2_META_FMT_UVC;
> +	stream->meta.buffersize = UVC_METADATA_BUF_MIN_SIZE;
>  
>  	return uvc_register_video_device(dev, stream, queue,
>  					 V4L2_BUF_TYPE_META_CAPTURE,
> diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
> index 68ed2883edb2..89206f761006 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -83,7 +83,7 @@ static int uvc_queue_setup(struct vb2_queue *vq,
>  
>  	switch (vq->type) {
>  	case V4L2_BUF_TYPE_META_CAPTURE:
> -		size = UVC_METADATA_BUF_SIZE;
> +		size = stream->meta.buffersize;
>  		break;
>  
>  	default:
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 9b4849fda12f..5ba698d2a23d 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -409,7 +409,7 @@ struct uvc_stats_stream {
>  	unsigned int max_sof;		/* Maximum STC.SOF value */
>  };
>  
> -#define UVC_METADATA_BUF_SIZE 10240
> +#define UVC_METADATA_BUF_MIN_SIZE 10240
>  
>  /**
>   * struct uvc_copy_op: Context structure to schedule asynchronous memcpy
> @@ -482,6 +482,7 @@ struct uvc_streaming {
>  	struct {
>  		struct uvc_video_queue queue;
>  		u32 format;
> +		u32 buffersize;
>  	} meta;
>  
>  	/* Context data used by the bulk completion handler. */
>