.../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(-)
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>
---
.../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,
--
Ricardo Ribalda <ribalda@chromium.org>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,
On Mon, Mar 03, 2025 at 03:47:51PM +0100, Hans de Goede wrote:
> 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>
I don't quite agree, sorry.
The reason why the current mechanism has been implemented this way is to
ensure we have documentation for the metadata format of devices that
expose metadata to userspace.
If you want to expose the MS metadata, you can create a new metadata
format for that, and enable it on devices that implement it.
> > ---
> > .../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
--
Regards,
Laurent Pinchart
Hi,
On 3-Mar-25 16:13, Laurent Pinchart wrote:
> On Mon, Mar 03, 2025 at 03:47:51PM +0100, Hans de Goede wrote:
>> 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>
>
> I don't quite agree, sorry.
>
> The reason why the current mechanism has been implemented this way is to
> ensure we have documentation for the metadata format of devices that
> expose metadata to userspace.
>
> If you want to expose the MS metadata, you can create a new metadata
> format for that, and enable it on devices that implement it.
Looking at the long list of quirks this removes just for the D4xx
cameras I do not believe that having quirked based relaying of
which metadata fmt is used to userspace is something which scales
on the long term. Given the large amount of different UVC cameras
I really think we should move the USB VID:PID -> metadata format
mapping out of the kernel.
I do agree that using V4L2_META_FMT_D4XX for every device is
probably not the best idea. So my suggestion would be to add
a new V4L2_META_FMT_CUSTOM metadata fmt and for index 1
metadata fmt use V4L2_META_FMT_D4XX for the currently quirked
cams and use V4L2_META_FMT_CUSTOM for other cameras.
This can then be combined with a udev-rule + hwdb to provide
info of what V4L2_META_FMT_CUSTOM should be mapped to in userspace,
moving further VID:PID -> extended-metadata fmt mappings out of
the kernel.
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
>
On Mon, Mar 03, 2025 at 04:43:50PM +0100, Hans de Goede wrote:
> On 3-Mar-25 16:13, Laurent Pinchart wrote:
> > On Mon, Mar 03, 2025 at 03:47:51PM +0100, Hans de Goede wrote:
> >> 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>
> >
> > I don't quite agree, sorry.
> >
> > The reason why the current mechanism has been implemented this way is to
> > ensure we have documentation for the metadata format of devices that
> > expose metadata to userspace.
> >
> > If you want to expose the MS metadata, you can create a new metadata
> > format for that, and enable it on devices that implement it.
>
> Looking at the long list of quirks this removes just for the D4xx
> cameras I do not believe that having quirked based relaying of
> which metadata fmt is used to userspace is something which scales
> on the long term. Given the large amount of different UVC cameras
> I really think we should move the USB VID:PID -> metadata format
> mapping out of the kernel.
If we can find a solution to ensure the metadata format gets documented,
sure.
When it comes to the MS metadata format, if I recall correctly, Ricardo
said there's an XU with a known GUID to detect the metadata format. We
therefore wouldn't need quirks.
> I do agree that using V4L2_META_FMT_D4XX for every device is
> probably not the best idea. So my suggestion would be to add
> a new V4L2_META_FMT_CUSTOM metadata fmt and for index 1
> metadata fmt use V4L2_META_FMT_D4XX for the currently quirked
> cams and use V4L2_META_FMT_CUSTOM for other cameras.
>
> This can then be combined with a udev-rule + hwdb to provide
> info of what V4L2_META_FMT_CUSTOM should be mapped to in userspace,
> moving further VID:PID -> extended-metadata fmt mappings out of
> the kernel.
>
> >>> ---
> >>> .../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
--
Regards,
Laurent Pinchart
On Mon, 3 Mar 2025 at 17:11, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Mon, Mar 03, 2025 at 04:43:50PM +0100, Hans de Goede wrote:
> > On 3-Mar-25 16:13, Laurent Pinchart wrote:
> > > On Mon, Mar 03, 2025 at 03:47:51PM +0100, Hans de Goede wrote:
> > >> 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>
> > >
> > > I don't quite agree, sorry.
> > >
> > > The reason why the current mechanism has been implemented this way is to
> > > ensure we have documentation for the metadata format of devices that
> > > expose metadata to userspace.
> > >
> > > If you want to expose the MS metadata, you can create a new metadata
> > > format for that, and enable it on devices that implement it.
> >
> > Looking at the long list of quirks this removes just for the D4xx
> > cameras I do not believe that having quirked based relaying of
> > which metadata fmt is used to userspace is something which scales
> > on the long term. Given the large amount of different UVC cameras
> > I really think we should move the USB VID:PID -> metadata format
> > mapping out of the kernel.
>
> If we can find a solution to ensure the metadata format gets documented,
> sure.
MS default metadata is already documented:
https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/mf-capture-metadata
I would not worry too much about vendors abusing the metadata for
custom magic if that is your concern.
This would not work with Windows default driver, and that is what most
camera modules are tested against.
>
> When it comes to the MS metadata format, if I recall correctly, Ricardo
> said there's an XU with a known GUID to detect the metadata format. We
> therefore wouldn't need quirks.
There is MXSU control MSXU_CONTROL_METADATA
https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5
But not all the vendors use it.
ChromeOS is working to define a XU... but that will take time to land.
Plus there are a lot of devices today that can benefit from frame metadata.
>
> > I do agree that using V4L2_META_FMT_D4XX for every device is
> > probably not the best idea. So my suggestion would be to add
> > a new V4L2_META_FMT_CUSTOM metadata fmt and for index 1
> > metadata fmt use V4L2_META_FMT_D4XX for the currently quirked
> > cams and use V4L2_META_FMT_CUSTOM for other cameras.
> >
> > This can then be combined with a udev-rule + hwdb to provide
> > info of what V4L2_META_FMT_CUSTOM should be mapped to in userspace,
> > moving further VID:PID -> extended-metadata fmt mappings out of
> > the kernel.
> >
> > >>> ---
> > >>> .../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
>
> --
> Regards,
>
> Laurent Pinchart
--
Ricardo Ribalda
Hi, On 3-Mar-25 5:22 PM, Ricardo Ribalda wrote: > On Mon, 3 Mar 2025 at 17:11, Laurent Pinchart > <laurent.pinchart@ideasonboard.com> wrote: >> >> On Mon, Mar 03, 2025 at 04:43:50PM +0100, Hans de Goede wrote: >>> On 3-Mar-25 16:13, Laurent Pinchart wrote: >>>> On Mon, Mar 03, 2025 at 03:47:51PM +0100, Hans de Goede wrote: >>>>> 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> >>>> >>>> I don't quite agree, sorry. >>>> >>>> The reason why the current mechanism has been implemented this way is to >>>> ensure we have documentation for the metadata format of devices that >>>> expose metadata to userspace. >>>> >>>> If you want to expose the MS metadata, you can create a new metadata >>>> format for that, and enable it on devices that implement it. >>> >>> Looking at the long list of quirks this removes just for the D4xx >>> cameras I do not believe that having quirked based relaying of >>> which metadata fmt is used to userspace is something which scales >>> on the long term. Given the large amount of different UVC cameras >>> I really think we should move the USB VID:PID -> metadata format >>> mapping out of the kernel. >> >> If we can find a solution to ensure the metadata format gets documented, >> sure. > > MS default metadata is already documented: > https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/mf-capture-metadata > > I would not worry too much about vendors abusing the metadata for > custom magic if that is your concern. > This would not work with Windows default driver, and that is what most > camera modules are tested against. > > >> >> When it comes to the MS metadata format, if I recall correctly, Ricardo >> said there's an XU with a known GUID to detect the metadata format. We >> therefore wouldn't need quirks. > > There is MXSU control > https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5 > But not all the vendors use it. Right so I actually already checked: https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5 yesterday before my udev hwdb suggestion I was wondering if there was a way to just get if MSmetadata was used from the camera itself. What I found was this: "UVC metadata is opt-in. Every IHV/OEM that needs metadata support must be enabled through a custom INF file." which lead me to the udev + hwdb suggestion. It is good to know that some cameras have a a way to fet this from a known XU GUID, but the official way seems to be to have per camera info in .inf files. So for Linux that would translate to having a list of vid:pid combinations somewhere. The question then becomes where do we put the vid:pid list and IMHO hwdb is much better (easier to maintain / update) then hardcoding this in the kernel. Regards, Hans
On Wed, Mar 05, 2025 at 02:03:58PM +0100, Hans de Goede wrote: > On 3-Mar-25 5:22 PM, Ricardo Ribalda wrote: > > On Mon, 3 Mar 2025 at 17:11, Laurent Pinchart wrote: > >> On Mon, Mar 03, 2025 at 04:43:50PM +0100, Hans de Goede wrote: > >>> On 3-Mar-25 16:13, Laurent Pinchart wrote: > >>>> On Mon, Mar 03, 2025 at 03:47:51PM +0100, Hans de Goede wrote: > >>>>> 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> > >>>> > >>>> I don't quite agree, sorry. > >>>> > >>>> The reason why the current mechanism has been implemented this way is to > >>>> ensure we have documentation for the metadata format of devices that > >>>> expose metadata to userspace. > >>>> > >>>> If you want to expose the MS metadata, you can create a new metadata > >>>> format for that, and enable it on devices that implement it. > >>> > >>> Looking at the long list of quirks this removes just for the D4xx > >>> cameras I do not believe that having quirked based relaying of > >>> which metadata fmt is used to userspace is something which scales > >>> on the long term. Given the large amount of different UVC cameras > >>> I really think we should move the USB VID:PID -> metadata format > >>> mapping out of the kernel. > >> > >> If we can find a solution to ensure the metadata format gets documented, > >> sure. > > > > MS default metadata is already documented: > > https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/mf-capture-metadata > > > > I would not worry too much about vendors abusing the metadata for > > custom magic if that is your concern. > > This would not work with Windows default driver, and that is what most > > camera modules are tested against. > > > >> When it comes to the MS metadata format, if I recall correctly, Ricardo > >> said there's an XU with a known GUID to detect the metadata format. We > >> therefore wouldn't need quirks. > > > > There is MXSU control > > https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5 > > But not all the vendors use it. > > Right so I actually already checked: > > https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5 > > yesterday before my udev hwdb suggestion I was wondering if there was a way > to just get if MSmetadata was used from the camera itself. What I found was this: > > "UVC metadata is opt-in. Every IHV/OEM that needs metadata support must be enabled through a custom INF file." > > which lead me to the udev + hwdb suggestion. > > It is good to know that some cameras have a a way to fet this from a known XU GUID, > but the official way seems to be to have per camera info in .inf files. So for Linux > that would translate to having a list of vid:pid combinations somewhere. > > The question then becomes where do we put the vid:pid list and IMHO hwdb is much > better (easier to maintain / update) then hardcoding this in the kernel. Additional questions: where do we store documentation for the metadata format, how do we convey what format is supported to applications, and how do we enable metadata capture when a device is listed in the DB ? -- Regards, Laurent Pinchart
Hi, On 5-Mar-25 2:11 PM, Laurent Pinchart wrote: > On Wed, Mar 05, 2025 at 02:03:58PM +0100, Hans de Goede wrote: >> On 3-Mar-25 5:22 PM, Ricardo Ribalda wrote: >>> On Mon, 3 Mar 2025 at 17:11, Laurent Pinchart wrote: >>>> On Mon, Mar 03, 2025 at 04:43:50PM +0100, Hans de Goede wrote: >>>>> On 3-Mar-25 16:13, Laurent Pinchart wrote: >>>>>> On Mon, Mar 03, 2025 at 03:47:51PM +0100, Hans de Goede wrote: >>>>>>> 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> >>>>>> >>>>>> I don't quite agree, sorry. >>>>>> >>>>>> The reason why the current mechanism has been implemented this way is to >>>>>> ensure we have documentation for the metadata format of devices that >>>>>> expose metadata to userspace. >>>>>> >>>>>> If you want to expose the MS metadata, you can create a new metadata >>>>>> format for that, and enable it on devices that implement it. >>>>> >>>>> Looking at the long list of quirks this removes just for the D4xx >>>>> cameras I do not believe that having quirked based relaying of >>>>> which metadata fmt is used to userspace is something which scales >>>>> on the long term. Given the large amount of different UVC cameras >>>>> I really think we should move the USB VID:PID -> metadata format >>>>> mapping out of the kernel. >>>> >>>> If we can find a solution to ensure the metadata format gets documented, >>>> sure. >>> >>> MS default metadata is already documented: >>> https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/mf-capture-metadata >>> >>> I would not worry too much about vendors abusing the metadata for >>> custom magic if that is your concern. >>> This would not work with Windows default driver, and that is what most >>> camera modules are tested against. >>> >>>> When it comes to the MS metadata format, if I recall correctly, Ricardo >>>> said there's an XU with a known GUID to detect the metadata format. We >>>> therefore wouldn't need quirks. >>> >>> There is MXSU control >>> https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5 >>> But not all the vendors use it. >> >> Right so I actually already checked: >> >> https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5 >> >> yesterday before my udev hwdb suggestion I was wondering if there was a way >> to just get if MSmetadata was used from the camera itself. What I found was this: >> >> "UVC metadata is opt-in. Every IHV/OEM that needs metadata support must be enabled through a custom INF file." >> >> which lead me to the udev + hwdb suggestion. >> >> It is good to know that some cameras have a a way to fet this from a known XU GUID, >> but the official way seems to be to have per camera info in .inf files. So for Linux >> that would translate to having a list of vid:pid combinations somewhere. >> >> The question then becomes where do we put the vid:pid list and IMHO hwdb is much >> better (easier to maintain / update) then hardcoding this in the kernel. > > Additional questions: where do we store documentation for the metadata > format, hwdb config files typically have a comment block with which key:value pairs that hwdb file will set. We can add known/supported formats + links to the documentation per format there. > how do we convey what format is supported to applications This would be a udev property on the /dev/video# node. > , and > how do we enable metadata capture when a device is listed in the DB ? The idea would be to at the kernel side just add a single new CUSTOM metadata fmt and when that is set copy the entire length of the received metadata to userspace like we are already doing for the D4xx format. Yes this will also possibly send extra metadata in other formats to userspace, without us having documented the format but I don't really think this is a showstopper here. It is not like this metadata is going to hide some highly secret processing info which we need to know on the Linux side, since all the processing is already done on the camera and we get a ready to use image out of the camera. Regards, Hans
On Mon, Mar 03, 2025 at 05:22:47PM +0100, Ricardo Ribalda wrote:
> On Mon, 3 Mar 2025 at 17:11, Laurent Pinchart wrote:
> > On Mon, Mar 03, 2025 at 04:43:50PM +0100, Hans de Goede wrote:
> > > On 3-Mar-25 16:13, Laurent Pinchart wrote:
> > > > On Mon, Mar 03, 2025 at 03:47:51PM +0100, Hans de Goede wrote:
> > > >> 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>
> > > >
> > > > I don't quite agree, sorry.
> > > >
> > > > The reason why the current mechanism has been implemented this way is to
> > > > ensure we have documentation for the metadata format of devices that
> > > > expose metadata to userspace.
> > > >
> > > > If you want to expose the MS metadata, you can create a new metadata
> > > > format for that, and enable it on devices that implement it.
> > >
> > > Looking at the long list of quirks this removes just for the D4xx
> > > cameras I do not believe that having quirked based relaying of
> > > which metadata fmt is used to userspace is something which scales
> > > on the long term. Given the large amount of different UVC cameras
> > > I really think we should move the USB VID:PID -> metadata format
> > > mapping out of the kernel.
> >
> > If we can find a solution to ensure the metadata format gets documented,
> > sure.
>
> MS default metadata is already documented:
> https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/mf-capture-metadata
>
> I would not worry too much about vendors abusing the metadata for
> custom magic if that is your concern.
> This would not work with Windows default driver, and that is what most
> camera modules are tested against.
How do Intel D4xx cameras work on Windows then, do they require a custom
driver ?
> > When it comes to the MS metadata format, if I recall correctly, Ricardo
> > said there's an XU with a known GUID to detect the metadata format. We
> > therefore wouldn't need quirks.
>
> There is MXSU control MSXU_CONTROL_METADATA
> https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5
> But not all the vendors use it.
>
> ChromeOS is working to define a XU... but that will take time to land.
>
> Plus there are a lot of devices today that can benefit from frame metadata.
>
> > > I do agree that using V4L2_META_FMT_D4XX for every device is
> > > probably not the best idea. So my suggestion would be to add
> > > a new V4L2_META_FMT_CUSTOM metadata fmt and for index 1
> > > metadata fmt use V4L2_META_FMT_D4XX for the currently quirked
> > > cams and use V4L2_META_FMT_CUSTOM for other cameras.
> > >
> > > This can then be combined with a udev-rule + hwdb to provide
> > > info of what V4L2_META_FMT_CUSTOM should be mapped to in userspace,
> > > moving further VID:PID -> extended-metadata fmt mappings out of
> > > the kernel.
> > >
> > > >>> ---
> > > >>> .../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
--
Regards,
Laurent Pinchart
On Mon, 3 Mar 2025 at 17:52, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Mon, Mar 03, 2025 at 05:22:47PM +0100, Ricardo Ribalda wrote:
> > On Mon, 3 Mar 2025 at 17:11, Laurent Pinchart wrote:
> > > On Mon, Mar 03, 2025 at 04:43:50PM +0100, Hans de Goede wrote:
> > > > On 3-Mar-25 16:13, Laurent Pinchart wrote:
> > > > > On Mon, Mar 03, 2025 at 03:47:51PM +0100, Hans de Goede wrote:
> > > > >> 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>
> > > > >
> > > > > I don't quite agree, sorry.
> > > > >
> > > > > The reason why the current mechanism has been implemented this way is to
> > > > > ensure we have documentation for the metadata format of devices that
> > > > > expose metadata to userspace.
> > > > >
> > > > > If you want to expose the MS metadata, you can create a new metadata
> > > > > format for that, and enable it on devices that implement it.
> > > >
> > > > Looking at the long list of quirks this removes just for the D4xx
> > > > cameras I do not believe that having quirked based relaying of
> > > > which metadata fmt is used to userspace is something which scales
> > > > on the long term. Given the large amount of different UVC cameras
> > > > I really think we should move the USB VID:PID -> metadata format
> > > > mapping out of the kernel.
> > >
> > > If we can find a solution to ensure the metadata format gets documented,
> > > sure.
> >
> > MS default metadata is already documented:
> > https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/mf-capture-metadata
> >
> > I would not worry too much about vendors abusing the metadata for
> > custom magic if that is your concern.
> > This would not work with Windows default driver, and that is what most
> > camera modules are tested against.
>
> How do Intel D4xx cameras work on Windows then, do they require a custom
> driver ?
Not a windows uvc expert here and I do not have a D4xx camera.
But most likely they are providing a uvc inf file:
https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/providing-a-uvc-inf-file
and declaring a plugin there.
>
> > > When it comes to the MS metadata format, if I recall correctly, Ricardo
> > > said there's an XU with a known GUID to detect the metadata format. We
> > > therefore wouldn't need quirks.
> >
> > There is MXSU control MSXU_CONTROL_METADATA
> > https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5
> > But not all the vendors use it.
> >
> > ChromeOS is working to define a XU... but that will take time to land.
> >
> > Plus there are a lot of devices today that can benefit from frame metadata.
> >
> > > > I do agree that using V4L2_META_FMT_D4XX for every device is
> > > > probably not the best idea. So my suggestion would be to add
> > > > a new V4L2_META_FMT_CUSTOM metadata fmt and for index 1
> > > > metadata fmt use V4L2_META_FMT_D4XX for the currently quirked
> > > > cams and use V4L2_META_FMT_CUSTOM for other cameras.
> > > >
> > > > This can then be combined with a udev-rule + hwdb to provide
> > > > info of what V4L2_META_FMT_CUSTOM should be mapped to in userspace,
> > > > moving further VID:PID -> extended-metadata fmt mappings out of
> > > > the kernel.
> > > >
> > > > >>> ---
> > > > >>> .../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
>
> --
> Regards,
>
> Laurent Pinchart
--
Ricardo Ribalda
Hi
On Mon, 3 Mar 2025 at 16:44, Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 3-Mar-25 16:13, Laurent Pinchart wrote:
> > On Mon, Mar 03, 2025 at 03:47:51PM +0100, Hans de Goede wrote:
> >> 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>
> >
> > I don't quite agree, sorry.
> >
> > The reason why the current mechanism has been implemented this way is to
> > ensure we have documentation for the metadata format of devices that
> > expose metadata to userspace.
> >
> > If you want to expose the MS metadata, you can create a new metadata
> > format for that, and enable it on devices that implement it.
>
> Looking at the long list of quirks this removes just for the D4xx
> cameras I do not believe that having quirked based relaying of
> which metadata fmt is used to userspace is something which scales
> on the long term. Given the large amount of different UVC cameras
> I really think we should move the USB VID:PID -> metadata format
> mapping out of the kernel.
+1000 to this. ChromeOS alone, this will be 200+ quirks per year.
>
> I do agree that using V4L2_META_FMT_D4XX for every device is
> probably not the best idea. So my suggestion would be to add
> a new V4L2_META_FMT_CUSTOM metadata fmt and for index 1
> metadata fmt use V4L2_META_FMT_D4XX for the currently quirked
> cams and use V4L2_META_FMT_CUSTOM for other cameras.
That works for me. What I wanted to avoid was to add a new metadata
format that is identical to V4L2_META_FMT_D4XX.
V4L2_META_FMT_D4XX is just Microsoft metadata id 0x80000000
>
> This can then be combined with a udev-rule + hwdb to provide
> info of what V4L2_META_FMT_CUSTOM should be mapped to in userspace,
> moving further VID:PID -> extended-metadata fmt mappings out of
> the kernel.
I am not aware of cameras implementing other metadata types different
to Microsoft's.
The rule is basically going to be *:* -> Microsoft Metadata.
But yeah, having a way to support weird cameras is always good :)
>
> 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
> >
>
--
Ricardo Ribalda
© 2016 - 2025 Red Hat, Inc.