Re: [PATCH] media: uvcvideo: Enable full UVC metadata for all devices

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 26-Feb-25 14:00, Ricardo Ribalda wrote:
> The UVC driver provides two metadata types V4L2_META_FMT_UVC, and
> V4L2_META_FMT_D4XX. The only difference between the two of them is that
> V4L2_META_FMT_UVC only copies PTS, SCR, size and flags, and
> V4L2_META_FMT_D4XX copies the whole metadata section.
> 
> Now we only enable V4L2_META_FMT_D4XX for the Intel D4xx family of
> devices, but it is useful for any device where vendors include other
> metadata, such as the one described by Microsoft:
> - https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/mf-capture-metadata
> 
> This patch removes the UVC_INFO_META macro and enables
> V4L2_META_FMT_D4XX for every device. It also updates the documentation
> to reflect the change.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans


> ---
>  .../userspace-api/media/v4l/metafmt-d4xx.rst       | 19 +++--
>  .../userspace-api/media/v4l/metafmt-uvc.rst        |  6 +-
>  drivers/media/usb/uvc/uvc_driver.c                 | 83 ----------------------
>  drivers/media/usb/uvc/uvc_metadata.c               | 15 ++--
>  drivers/media/usb/uvc/uvcvideo.h                   |  1 -
>  5 files changed, 23 insertions(+), 101 deletions(-)
> 
> diff --git a/Documentation/userspace-api/media/v4l/metafmt-d4xx.rst b/Documentation/userspace-api/media/v4l/metafmt-d4xx.rst
> index 0686413b16b2..1b18ef056934 100644
> --- a/Documentation/userspace-api/media/v4l/metafmt-d4xx.rst
> +++ b/Documentation/userspace-api/media/v4l/metafmt-d4xx.rst
> @@ -6,12 +6,23 @@
>  V4L2_META_FMT_D4XX ('D4XX')
>  *******************************
>  
> -Intel D4xx UVC Cameras Metadata
> +UVC Full Payload Header Data (formerly known as Intel D4xx UVC Cameras
> +Metadata).
>  
>  
>  Description
>  ===========
>  
> +V4L2_META_FMT_D4XX buffers follow the metadata buffer layout of
> +V4L2_META_FMT_UVC with the only difference, that it also includes proprietary
> +payload header data. It was originally implemented for Intel D4xx cameras, and
> +thus the name, but now it can be used by any UVC device, when userspace wants
> +full access to the UVC Metadata.
> +
> +
> +Intel D4xx Metadata
> +===================
> +
>  Intel D4xx (D435, D455 and others) cameras include per-frame metadata in their UVC
>  payload headers, following the Microsoft(R) UVC extension proposal [1_]. That
>  means, that the private D4XX metadata, following the standard UVC header, is
> @@ -21,10 +32,8 @@ types are MetadataId_CaptureStats (ID 3), MetadataId_CameraExtrinsics (ID 4),
>  and MetadataId_CameraIntrinsics (ID 5). For their description see [1_]. This
>  document describes proprietary metadata types, used by D4xx cameras.
>  
> -V4L2_META_FMT_D4XX buffers follow the metadata buffer layout of
> -V4L2_META_FMT_UVC with the only difference, that it also includes proprietary
> -payload header data. D4xx cameras use bulk transfers and only send one payload
> -per frame, therefore their headers cannot be larger than 255 bytes.
> +D4xx cameras use bulk transfers and only send one payload per frame, therefore
> +their headers cannot be larger than 255 bytes.
>  
>  This document implements Intel Configuration version 3 [9_].
>  
> diff --git a/Documentation/userspace-api/media/v4l/metafmt-uvc.rst b/Documentation/userspace-api/media/v4l/metafmt-uvc.rst
> index 784346d14bbd..a3aae580e89e 100644
> --- a/Documentation/userspace-api/media/v4l/metafmt-uvc.rst
> +++ b/Documentation/userspace-api/media/v4l/metafmt-uvc.rst
> @@ -6,7 +6,7 @@
>  V4L2_META_FMT_UVC ('UVCH')
>  *******************************
>  
> -UVC Payload Header Data
> +UVC Partial Payload Header Data (formerly known as UVC Payload Header Data).
>  
>  
>  Description
> @@ -44,7 +44,9 @@ Each individual block contains the following fields:
>          them
>      * - :cspan:`1` *The rest is an exact copy of the UVC payload header:*
>      * - __u8 length;
> -      - length of the rest of the block, including this field
> +      - length of the rest of the block, including this field (please note that
> +        regardless of this value, the driver will never copy more than 12
> +        bytes).
>      * - __u8 flags;
>        - Flags, indicating presence of other standard UVC fields
>      * - __u8 buf[];
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index deadbcea5e22..f19dcd4a7ac6 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -2488,8 +2488,6 @@ static const struct uvc_device_info uvc_quirk_force_y8 = {
>  };
>  
>  #define UVC_INFO_QUIRK(q) (kernel_ulong_t)&(struct uvc_device_info){.quirks = q}
> -#define UVC_INFO_META(m) (kernel_ulong_t)&(struct uvc_device_info) \
> -	{.meta_format = m}
>  
>  /*
>   * The Logitech cameras listed below have their interface class set to
> @@ -3107,87 +3105,6 @@ static const struct usb_device_id uvc_ids[] = {
>  	  .bInterfaceSubClass	= 1,
>  	  .bInterfaceProtocol	= 0,
>  	  .driver_info		= UVC_INFO_QUIRK(UVC_QUIRK_DISABLE_AUTOSUSPEND) },
> -	/* Intel D410/ASR depth camera */
> -	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
> -				| USB_DEVICE_ID_MATCH_INT_INFO,
> -	  .idVendor		= 0x8086,
> -	  .idProduct		= 0x0ad2,
> -	  .bInterfaceClass	= USB_CLASS_VIDEO,
> -	  .bInterfaceSubClass	= 1,
> -	  .bInterfaceProtocol	= 0,
> -	  .driver_info		= UVC_INFO_META(V4L2_META_FMT_D4XX) },
> -	/* Intel D415/ASRC depth camera */
> -	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
> -				| USB_DEVICE_ID_MATCH_INT_INFO,
> -	  .idVendor		= 0x8086,
> -	  .idProduct		= 0x0ad3,
> -	  .bInterfaceClass	= USB_CLASS_VIDEO,
> -	  .bInterfaceSubClass	= 1,
> -	  .bInterfaceProtocol	= 0,
> -	  .driver_info		= UVC_INFO_META(V4L2_META_FMT_D4XX) },
> -	/* Intel D430/AWG depth camera */
> -	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
> -				| USB_DEVICE_ID_MATCH_INT_INFO,
> -	  .idVendor		= 0x8086,
> -	  .idProduct		= 0x0ad4,
> -	  .bInterfaceClass	= USB_CLASS_VIDEO,
> -	  .bInterfaceSubClass	= 1,
> -	  .bInterfaceProtocol	= 0,
> -	  .driver_info		= UVC_INFO_META(V4L2_META_FMT_D4XX) },
> -	/* Intel RealSense D4M */
> -	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
> -				| USB_DEVICE_ID_MATCH_INT_INFO,
> -	  .idVendor		= 0x8086,
> -	  .idProduct		= 0x0b03,
> -	  .bInterfaceClass	= USB_CLASS_VIDEO,
> -	  .bInterfaceSubClass	= 1,
> -	  .bInterfaceProtocol	= 0,
> -	  .driver_info		= UVC_INFO_META(V4L2_META_FMT_D4XX) },
> -	/* Intel D435/AWGC depth camera */
> -	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
> -				| USB_DEVICE_ID_MATCH_INT_INFO,
> -	  .idVendor		= 0x8086,
> -	  .idProduct		= 0x0b07,
> -	  .bInterfaceClass	= USB_CLASS_VIDEO,
> -	  .bInterfaceSubClass	= 1,
> -	  .bInterfaceProtocol	= 0,
> -	  .driver_info		= UVC_INFO_META(V4L2_META_FMT_D4XX) },
> -	/* Intel D435i depth camera */
> -	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
> -				| USB_DEVICE_ID_MATCH_INT_INFO,
> -	  .idVendor		= 0x8086,
> -	  .idProduct		= 0x0b3a,
> -	  .bInterfaceClass	= USB_CLASS_VIDEO,
> -	  .bInterfaceSubClass	= 1,
> -	  .bInterfaceProtocol	= 0,
> -	  .driver_info		= UVC_INFO_META(V4L2_META_FMT_D4XX) },
> -	/* Intel D405 Depth Camera */
> -	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
> -				| USB_DEVICE_ID_MATCH_INT_INFO,
> -	  .idVendor		= 0x8086,
> -	  .idProduct		= 0x0b5b,
> -	  .bInterfaceClass	= USB_CLASS_VIDEO,
> -	  .bInterfaceSubClass	= 1,
> -	  .bInterfaceProtocol	= 0,
> -	  .driver_info		= UVC_INFO_META(V4L2_META_FMT_D4XX) },
> -	/* Intel D455 Depth Camera */
> -	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
> -				| USB_DEVICE_ID_MATCH_INT_INFO,
> -	  .idVendor		= 0x8086,
> -	  .idProduct		= 0x0b5c,
> -	  .bInterfaceClass	= USB_CLASS_VIDEO,
> -	  .bInterfaceSubClass	= 1,
> -	  .bInterfaceProtocol	= 0,
> -	  .driver_info		= UVC_INFO_META(V4L2_META_FMT_D4XX) },
> -	/* Intel D421 Depth Module */
> -	{ .match_flags		= USB_DEVICE_ID_MATCH_DEVICE
> -				| USB_DEVICE_ID_MATCH_INT_INFO,
> -	  .idVendor		= 0x8086,
> -	  .idProduct		= 0x1155,
> -	  .bInterfaceClass	= USB_CLASS_VIDEO,
> -	  .bInterfaceSubClass	= 1,
> -	  .bInterfaceProtocol	= 0,
> -	  .driver_info		= UVC_INFO_META(V4L2_META_FMT_D4XX) },
>  	/* Generic USB Video Class */
>  	{ USB_INTERFACE_INFO(USB_CLASS_VIDEO, 1, UVC_PC_PROTOCOL_UNDEFINED) },
>  	{ USB_INTERFACE_INFO(USB_CLASS_VIDEO, 1, UVC_PC_PROTOCOL_15) },
> diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
> index 82de7781f5b6..5c44e6cdb83c 100644
> --- a/drivers/media/usb/uvc/uvc_metadata.c
> +++ b/drivers/media/usb/uvc/uvc_metadata.c
> @@ -60,18 +60,16 @@ static int uvc_meta_v4l2_try_format(struct file *file, void *fh,
>  				    struct v4l2_format *format)
>  {
>  	struct v4l2_fh *vfh = file->private_data;
> -	struct uvc_streaming *stream = video_get_drvdata(vfh->vdev);
> -	struct uvc_device *dev = stream->dev;
>  	struct v4l2_meta_format *fmt = &format->fmt.meta;
> -	u32 fmeta = fmt->dataformat;
> +	u32 fmeta = fmt->dataformat == V4L2_META_FMT_D4XX ?
> +		    V4L2_META_FMT_D4XX : V4L2_META_FMT_UVC;
>  
>  	if (format->type != vfh->vdev->queue->type)
>  		return -EINVAL;
>  
>  	memset(fmt, 0, sizeof(*fmt));
>  
> -	fmt->dataformat = fmeta == dev->info->meta_format
> -			? fmeta : V4L2_META_FMT_UVC;
> +	fmt->dataformat = fmeta;
>  	fmt->buffersize = UVC_METADATA_BUF_SIZE;
>  
>  	return 0;
> @@ -110,19 +108,16 @@ static int uvc_meta_v4l2_enum_formats(struct file *file, void *fh,
>  				      struct v4l2_fmtdesc *fdesc)
>  {
>  	struct v4l2_fh *vfh = file->private_data;
> -	struct uvc_streaming *stream = video_get_drvdata(vfh->vdev);
> -	struct uvc_device *dev = stream->dev;
>  	u32 index = fdesc->index;
>  
> -	if (fdesc->type != vfh->vdev->queue->type ||
> -	    index > 1U || (index && !dev->info->meta_format))
> +	if (fdesc->type != vfh->vdev->queue->type || index > 1U)
>  		return -EINVAL;
>  
>  	memset(fdesc, 0, sizeof(*fdesc));
>  
>  	fdesc->type = vfh->vdev->queue->type;
>  	fdesc->index = index;
> -	fdesc->pixelformat = index ? dev->info->meta_format : V4L2_META_FMT_UVC;
> +	fdesc->pixelformat = index ? V4L2_META_FMT_D4XX : V4L2_META_FMT_UVC;
>  
>  	return 0;
>  }
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 5e388f05f3fc..cc2092ae9987 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -534,7 +534,6 @@ static inline u32 uvc_urb_index(const struct uvc_urb *uvc_urb)
>  
>  struct uvc_device_info {
>  	u32	quirks;
> -	u32	meta_format;
>  	u16	uvc_version;
>  };
>  
> 
> ---
> base-commit: d98e9213a768a3cc3a99f5e1abe09ad3baff2104
> change-id: 20250226-uvc-metadata-2e7e445966de
> 
> Best regards,