[PATCH v3 0/3] media: uvcvideo: Introduce V4L2_META_FMT_UVC_CUSTOM + other meta fixes

[PATCH v3 0/3] media: uvcvideo: Introduce V4L2_META_FMT_UVC_CUSTOM + other meta fixes

[Ricardo Ribalda]

This series introduces a new metadata format for UVC cameras and adds a
couple of improvements to the UVC metadata handling.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Changes in v3:
- Fix doc syntax errors.
- Link to v2: https://lore.kernel.org/r/20250306-uvc-metadata-v2-0-7e939857cad5@chromium.org

Changes in v2:
- Add metadata invalid fix
- Move doc note to a separate patch
- Introuce V4L2_META_FMT_UVC_CUSTOM (thanks HdG!).
- Link to v1: https://lore.kernel.org/r/20250226-uvc-metadata-v1-1-6cd6fe5ec2cb@chromium.org

---
Ricardo Ribalda (3):
      media: uvcvideo: Do not mark valid metadata as invalid
      media: Documentation: Add note about UVCH length field
      media: uvcvideo: Introduce V4L2_META_FMT_UVC_CUSTOM

 .../userspace-api/media/v4l/meta-formats.rst       |  1 +
 .../userspace-api/media/v4l/metafmt-uvc-custom.rst | 31 +++++++++++++++++
 .../userspace-api/media/v4l/metafmt-uvc.rst        |  4 ++-
 MAINTAINERS                                        |  1 +
 drivers/media/usb/uvc/uvc_metadata.c               | 40 ++++++++++++++++++----
 drivers/media/usb/uvc/uvc_video.c                  | 12 +++----
 drivers/media/v4l2-core/v4l2-ioctl.c               |  1 +
 include/uapi/linux/videodev2.h                     |  1 +
 8 files changed, 78 insertions(+), 13 deletions(-)
---
base-commit: 36cef585e2a31e4ddf33a004b0584a7a572246de
change-id: 20250226-uvc-metadata-2e7e445966de

Best regards,
-- 
Ricardo Ribalda <ribalda@chromium.org>

[PATCH v3 1/3] media: uvcvideo: Do not mark valid metadata as invalid

[Ricardo Ribalda]

Currently, the driver performs a length check of the metadata buffer
before the actual metadata size is known and before the metadata is
decided to be copied. This results in valid metadata buffers being
incorrectly marked as invalid.

Move the length check to occur after the metadata size is determined and
is decided to be copied.

Cc: stable@vger.kernel.org
Fixes: 088ead255245 ("media: uvcvideo: Add a metadata device node")
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_video.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index e3567aeb0007c1f0a766f331e4e744359e95a863..b113297dac61f1b2eecd72c36ea61ef2c1e7d28a 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1433,12 +1433,6 @@ static void uvc_video_decode_meta(struct uvc_streaming *stream,
 	if (!meta_buf || length == 2)
 		return;
 
-	if (meta_buf->length - meta_buf->bytesused <
-	    length + sizeof(meta->ns) + sizeof(meta->sof)) {
-		meta_buf->error = 1;
-		return;
-	}
-
 	has_pts = mem[1] & UVC_STREAM_PTS;
 	has_scr = mem[1] & UVC_STREAM_SCR;
 
@@ -1459,6 +1453,12 @@ static void uvc_video_decode_meta(struct uvc_streaming *stream,
 				  !memcmp(scr, stream->clock.last_scr, 6)))
 		return;
 
+	if (meta_buf->length - meta_buf->bytesused <
+	    length + sizeof(meta->ns) + sizeof(meta->sof)) {
+		meta_buf->error = 1;
+		return;
+	}
+
 	meta = (struct uvc_meta_buf *)((u8 *)meta_buf->mem + meta_buf->bytesused);
 	local_irq_save(flags);
 	time = uvc_video_get_time();

-- 
2.49.0.rc0.332.g42c0ae87b1-goog

[PATCH v3 2/3] media: Documentation: Add note about UVCH length field

[Ricardo Ribalda]

The documentation currently describes the UVC length field as the "length
of the rest of the block", which can be misleading. The driver limits the
data copied to a maximum of 12 bytes.

This change adds a clarifying sentence to the documentation to make this
restriction explicit.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 Documentation/userspace-api/media/v4l/metafmt-uvc.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/userspace-api/media/v4l/metafmt-uvc.rst b/Documentation/userspace-api/media/v4l/metafmt-uvc.rst
index 784346d14bbdbf28348262084d5b0646d30bd1da..42599875331c0066cf529153caccb731148023b9 100644
--- a/Documentation/userspace-api/media/v4l/metafmt-uvc.rst
+++ b/Documentation/userspace-api/media/v4l/metafmt-uvc.rst
@@ -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 the this value, for V4L2_META_FMT_UVC the kernel will
+        never copy more than 2-12 bytes.
     * - __u8 flags;
       - Flags, indicating presence of other standard UVC fields
     * - __u8 buf[];

-- 
2.49.0.rc0.332.g42c0ae87b1-goog

[PATCH v3 3/3] media: uvcvideo: Introduce V4L2_META_FMT_UVC_CUSTOM

[Ricardo Ribalda]

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 to have the whole metadata section 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 introduces a new format V4L2_META_FMT_UVC_CUSTOM, that is
identical to V4L2_META_FMT_D4XX but it is available to all the UVC
devices.

Suggested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 .../userspace-api/media/v4l/meta-formats.rst       |  1 +
 .../userspace-api/media/v4l/metafmt-uvc-custom.rst | 31 +++++++++++++++++
 MAINTAINERS                                        |  1 +
 drivers/media/usb/uvc/uvc_metadata.c               | 40 ++++++++++++++++++----
 drivers/media/v4l2-core/v4l2-ioctl.c               |  1 +
 include/uapi/linux/videodev2.h                     |  1 +
 6 files changed, 69 insertions(+), 6 deletions(-)

diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst
index 86ffb3bc8ade2e0c563dd84441572ecea1a571a6..9fd83f4a3cc8509702a2a9f032fdc04bf6c6d1bc 100644
--- a/Documentation/userspace-api/media/v4l/meta-formats.rst
+++ b/Documentation/userspace-api/media/v4l/meta-formats.rst
@@ -19,6 +19,7 @@ These formats are used for the :ref:`metadata` interface only.
     metafmt-pisp-fe
     metafmt-rkisp1
     metafmt-uvc
+    metafmt-uvc-custom
     metafmt-vivid
     metafmt-vsp1-hgo
     metafmt-vsp1-hgt
diff --git a/Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst b/Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst
new file mode 100644
index 0000000000000000000000000000000000000000..9f150fc2b6f379cc4707ff45041dd014956ae11a
--- /dev/null
+++ b/Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst
@@ -0,0 +1,31 @@
+.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
+
+.. _v4l2-meta-fmt-uvc-custom:
+
+*********************************
+V4L2_META_FMT_UVC_CUSTOM ('UVCC')
+*********************************
+
+UVC Custom Payload Metadata.
+
+
+Description
+===========
+
+V4L2_META_FMT_UVC_CUSTOM buffers follow the metadata buffer layout of
+V4L2_META_FMT_UVC with the only difference that it includes all the UVC
+metadata, not just the first 2-12 bytes.
+
+The most common metadata format is the one proposed by Microsoft(R)'s UVC
+extension [1_], but other vendors might have different formats.
+
+Applications might use information from the Hardware Database (hwdb)[2_] to
+process the camera's metadata accordingly.
+
+.. _1:
+
+[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5
+
+.. _2:
+
+[2] https://www.freedesktop.org/software/systemd/man/latest/hwdb.html
diff --git a/MAINTAINERS b/MAINTAINERS
index 29b4471574982bf3f8d03158cd5edcb94bc9fab9..4e8e8096951ff0e7159d7f3916cf7b014a6ef95f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -24665,6 +24665,7 @@ S:	Maintained
 W:	http://www.ideasonboard.org/uvc/
 T:	git git://linuxtv.org/media.git
 F:	Documentation/userspace-api/media/drivers/uvcvideo.rst
+F:	Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst
 F:	Documentation/userspace-api/media/v4l/metafmt-uvc.rst
 F:	drivers/media/common/uvc.c
 F:	drivers/media/usb/uvc/
diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
index 82de7781f5b6b70c5ba16bcba9e0741231231904..b257524d279a141f650e2fbb376a35cc17252c2e 100644
--- a/drivers/media/usb/uvc/uvc_metadata.c
+++ b/drivers/media/usb/uvc/uvc_metadata.c
@@ -63,15 +63,21 @@ static int uvc_meta_v4l2_try_format(struct file *file, void *fh,
 	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;
+
+	if (fmt->dataformat == dev->info->meta_format)
+		fmeta = dev->info->meta_format;
+	else if (fmt->dataformat == V4L2_META_FMT_UVC_CUSTOM)
+		fmeta = V4L2_META_FMT_UVC_CUSTOM;
+	else
+		fmeta = 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;
@@ -106,6 +112,24 @@ static int uvc_meta_v4l2_set_format(struct file *file, void *fh,
 	return ret;
 }
 
+static u32 uvc_meta_idx_to_fmeta(struct uvc_device *dev, u32 index)
+{
+	switch (index) {
+	case 0:
+		return V4L2_META_FMT_UVC;
+	case 1:
+		if (dev->info->meta_format)
+			return dev->info->meta_format;
+		return V4L2_META_FMT_UVC_CUSTOM;
+	case 2:
+		if (dev->info->meta_format)
+			return V4L2_META_FMT_UVC_CUSTOM;
+		return 0;
+	}
+
+	return 0;
+}
+
 static int uvc_meta_v4l2_enum_formats(struct file *file, void *fh,
 				      struct v4l2_fmtdesc *fdesc)
 {
@@ -113,16 +137,20 @@ static int uvc_meta_v4l2_enum_formats(struct file *file, void *fh,
 	struct uvc_streaming *stream = video_get_drvdata(vfh->vdev);
 	struct uvc_device *dev = stream->dev;
 	u32 index = fdesc->index;
+	u32 fmeta;
+
+	if (fdesc->type != vfh->vdev->queue->type)
+		return -EINVAL;
 
-	if (fdesc->type != vfh->vdev->queue->type ||
-	    index > 1U || (index && !dev->info->meta_format))
+	fmeta = uvc_meta_idx_to_fmeta(dev, fdesc->index);
+	if (!fmeta)
 		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 = fmeta;
 
 	return 0;
 }
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index a16fb44c7246e35f3710306fde5dfc15329b4d95..1ffbf38823b226ff7044c798c4b982d52137e904 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1457,6 +1457,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
 	case V4L2_META_FMT_VSP1_HGO:	descr = "R-Car VSP1 1-D Histogram"; break;
 	case V4L2_META_FMT_VSP1_HGT:	descr = "R-Car VSP1 2-D Histogram"; break;
 	case V4L2_META_FMT_UVC:		descr = "UVC Payload Header Metadata"; break;
+	case V4L2_META_FMT_UVC_CUSTOM:	descr = "UVC Custom Payload Metadata"; break;
 	case V4L2_META_FMT_D4XX:	descr = "Intel D4xx UVC Metadata"; break;
 	case V4L2_META_FMT_VIVID:       descr = "Vivid Metadata"; break;
 	case V4L2_META_FMT_RK_ISP1_PARAMS:	descr = "Rockchip ISP1 3A Parameters"; break;
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index c8cb2796130f8d1b864d669267d2b31f73b839aa..b0af18b7621296be0885d5b65494ec01bc425c9c 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -851,6 +851,7 @@ struct v4l2_pix_format {
 #define V4L2_META_FMT_VSP1_HGT    v4l2_fourcc('V', 'S', 'P', 'T') /* R-Car VSP1 2-D Histogram */
 #define V4L2_META_FMT_UVC         v4l2_fourcc('U', 'V', 'C', 'H') /* UVC Payload Header metadata */
 #define V4L2_META_FMT_D4XX        v4l2_fourcc('D', '4', 'X', 'X') /* D4XX Payload Header metadata */
+#define V4L2_META_FMT_UVC_CUSTOM  v4l2_fourcc('U', 'V', 'C', 'C') /* UVC Custom Payload metadata */
 #define V4L2_META_FMT_VIVID	  v4l2_fourcc('V', 'I', 'V', 'D') /* Vivid Metadata */
 
 /* Vendor specific - used for RK_ISP1 camera sub-system */

-- 
2.49.0.rc0.332.g42c0ae87b1-goog

Re: [PATCH v3 3/3] media: uvcvideo: Introduce V4L2_META_FMT_UVC_CUSTOM

[Mauro Carvalho Chehab]

Em Thu, 13 Mar 2025 12:06:27 +0000
Ricardo Ribalda <ribalda@chromium.org> escreveu:

> 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 to have the whole metadata section 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 introduces a new format V4L2_META_FMT_UVC_CUSTOM, that is
> identical to V4L2_META_FMT_D4XX but it is available to all the UVC
> devices.
> 
> Suggested-by: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  .../userspace-api/media/v4l/meta-formats.rst       |  1 +
>  .../userspace-api/media/v4l/metafmt-uvc-custom.rst | 31 +++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  drivers/media/usb/uvc/uvc_metadata.c               | 40 ++++++++++++++++++----
>  drivers/media/v4l2-core/v4l2-ioctl.c               |  1 +
>  include/uapi/linux/videodev2.h                     |  1 +
>  6 files changed, 69 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst
> index 86ffb3bc8ade2e0c563dd84441572ecea1a571a6..9fd83f4a3cc8509702a2a9f032fdc04bf6c6d1bc 100644
> --- a/Documentation/userspace-api/media/v4l/meta-formats.rst
> +++ b/Documentation/userspace-api/media/v4l/meta-formats.rst
> @@ -19,6 +19,7 @@ These formats are used for the :ref:`metadata` interface only.
>      metafmt-pisp-fe
>      metafmt-rkisp1
>      metafmt-uvc
> +    metafmt-uvc-custom
>      metafmt-vivid
>      metafmt-vsp1-hgo
>      metafmt-vsp1-hgt
> diff --git a/Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst b/Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst
> new file mode 100644
> index 0000000000000000000000000000000000000000..9f150fc2b6f379cc4707ff45041dd014956ae11a
> --- /dev/null
> +++ b/Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst
> @@ -0,0 +1,31 @@
> +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
> +
> +.. _v4l2-meta-fmt-uvc-custom:
> +
> +*********************************
> +V4L2_META_FMT_UVC_CUSTOM ('UVCC')
> +*********************************
> +
> +UVC Custom Payload Metadata.
> +
> +
> +Description
> +===========
> +
> +V4L2_META_FMT_UVC_CUSTOM buffers follow the metadata buffer layout of
> +V4L2_META_FMT_UVC with the only difference that it includes all the UVC
> +metadata, not just the first 2-12 bytes.
> +
> +The most common metadata format is the one proposed by Microsoft(R)'s UVC
> +extension [1_], but other vendors might have different formats.
> +
> +Applications might use information from the Hardware Database (hwdb)[2_] to
> +process the camera's metadata accordingly.

Having something like that at the userspace API shouldn't be handled
lightly. This sounds to me that passing a blank check for vendors to stream
whatever they want without any requirements to provide and sort of
documentation for the usersace to decode it.

Also, it would be hard for userspace to distinguish what metatata
is contained for a random UVC camera. Please let's not do that.

As the specific issue here is to support an already known extension,
which is already documented, Just add an specific format for it, e.g. 
you could add something like that at the documentation:

	V4L2_META_FMT_MSXU_UVC_1_5
	   Microsoft extensions to USB Video Class 1.5 specification.
	
	   For more details, see: https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5	

And then add the corresponding format to V4L2 API.

Regards,
Mauro

Re: [PATCH v3 3/3] media: uvcvideo: Introduce V4L2_META_FMT_UVC_CUSTOM

[Ricardo Ribalda]

Hi Mauro

On Fri, 14 Mar 2025 at 07:35, Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> Em Thu, 13 Mar 2025 12:06:27 +0000
> Ricardo Ribalda <ribalda@chromium.org> escreveu:
>
> > 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 to have the whole metadata section 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 introduces a new format V4L2_META_FMT_UVC_CUSTOM, that is
> > identical to V4L2_META_FMT_D4XX but it is available to all the UVC
> > devices.
> >
> > Suggested-by: Hans de Goede <hdegoede@redhat.com>
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> >  .../userspace-api/media/v4l/meta-formats.rst       |  1 +
> >  .../userspace-api/media/v4l/metafmt-uvc-custom.rst | 31 +++++++++++++++++
> >  MAINTAINERS                                        |  1 +
> >  drivers/media/usb/uvc/uvc_metadata.c               | 40 ++++++++++++++++++----
> >  drivers/media/v4l2-core/v4l2-ioctl.c               |  1 +
> >  include/uapi/linux/videodev2.h                     |  1 +
> >  6 files changed, 69 insertions(+), 6 deletions(-)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > index 86ffb3bc8ade2e0c563dd84441572ecea1a571a6..9fd83f4a3cc8509702a2a9f032fdc04bf6c6d1bc 100644
> > --- a/Documentation/userspace-api/media/v4l/meta-formats.rst
> > +++ b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > @@ -19,6 +19,7 @@ These formats are used for the :ref:`metadata` interface only.
> >      metafmt-pisp-fe
> >      metafmt-rkisp1
> >      metafmt-uvc
> > +    metafmt-uvc-custom
> >      metafmt-vivid
> >      metafmt-vsp1-hgo
> >      metafmt-vsp1-hgt
> > diff --git a/Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst b/Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..9f150fc2b6f379cc4707ff45041dd014956ae11a
> > --- /dev/null
> > +++ b/Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst
> > @@ -0,0 +1,31 @@
> > +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
> > +
> > +.. _v4l2-meta-fmt-uvc-custom:
> > +
> > +*********************************
> > +V4L2_META_FMT_UVC_CUSTOM ('UVCC')
> > +*********************************
> > +
> > +UVC Custom Payload Metadata.
> > +
> > +
> > +Description
> > +===========
> > +
> > +V4L2_META_FMT_UVC_CUSTOM buffers follow the metadata buffer layout of
> > +V4L2_META_FMT_UVC with the only difference that it includes all the UVC
> > +metadata, not just the first 2-12 bytes.
> > +
> > +The most common metadata format is the one proposed by Microsoft(R)'s UVC
> > +extension [1_], but other vendors might have different formats.
> > +
> > +Applications might use information from the Hardware Database (hwdb)[2_] to
> > +process the camera's metadata accordingly.
>
> Having something like that at the userspace API shouldn't be handled
> lightly. This sounds to me that passing a blank check for vendors to stream
> whatever they want without any requirements to provide and sort of
> documentation for the usersace to decode it.

As HdG previously mentioned, all the processing is done in the camera
so the metadata is not going to hide highly secret required for
processing:
https://lore.kernel.org/linux-media/67c1a857-7656-421f-994c-751709b6ae01@redhat.com/

>
> Also, it would be hard for userspace to distinguish what metatata
> is contained for a random UVC camera. Please let's not do that.

Userspace will use hwdb info to properly parse the metadata.


>
> As the specific issue here is to support an already known extension,
> which is already documented, Just add an specific format for it, e.g.
> you could add something like that at the documentation:

The problem here is how do we know from the driver if a device
supports V4L2_META_FMT_MSXU_UVC_1_5 or not.

In Windows it seems that vendors add that information to the device
.inf file. That is equivalent to the hwdb proposal.
In ChromeOS we are trying to push vendors to use an extension saying
if there is metadata or not. But that will take some time to land and
there are thousands of modules out there not ChromeOS compliant.


>
>         V4L2_META_FMT_MSXU_UVC_1_5
>            Microsoft extensions to USB Video Class 1.5 specification.
>
>            For more details, see: https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5
>
> And then add the corresponding format to V4L2 API.
>
> Regards,
> Mauro



-- 
Ricardo Ribalda

Re: [PATCH v3 3/3] media: uvcvideo: Introduce V4L2_META_FMT_UVC_CUSTOM

[Laurent Pinchart]

Hi Ricardo,

On Fri, Mar 14, 2025 at 09:28:34AM +0100, Ricardo Ribalda wrote:
> On Fri, 14 Mar 2025 at 07:35, Mauro Carvalho Chehab wrote:
> > Em Thu, 13 Mar 2025 12:06:27 +0000 Ricardo Ribalda escreveu:
> >
> > > 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 to have the whole metadata section 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 introduces a new format V4L2_META_FMT_UVC_CUSTOM, that is
> > > identical to V4L2_META_FMT_D4XX but it is available to all the UVC
> > > devices.
> > >
> > > Suggested-by: Hans de Goede <hdegoede@redhat.com>
> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > ---
> > >  .../userspace-api/media/v4l/meta-formats.rst       |  1 +
> > >  .../userspace-api/media/v4l/metafmt-uvc-custom.rst | 31 +++++++++++++++++
> > >  MAINTAINERS                                        |  1 +
> > >  drivers/media/usb/uvc/uvc_metadata.c               | 40 ++++++++++++++++++----
> > >  drivers/media/v4l2-core/v4l2-ioctl.c               |  1 +
> > >  include/uapi/linux/videodev2.h                     |  1 +
> > >  6 files changed, 69 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > > index 86ffb3bc8ade2e0c563dd84441572ecea1a571a6..9fd83f4a3cc8509702a2a9f032fdc04bf6c6d1bc 100644
> > > --- a/Documentation/userspace-api/media/v4l/meta-formats.rst
> > > +++ b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > > @@ -19,6 +19,7 @@ These formats are used for the :ref:`metadata` interface only.
> > >      metafmt-pisp-fe
> > >      metafmt-rkisp1
> > >      metafmt-uvc
> > > +    metafmt-uvc-custom
> > >      metafmt-vivid
> > >      metafmt-vsp1-hgo
> > >      metafmt-vsp1-hgt
> > > diff --git a/Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst b/Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst
> > > new file mode 100644
> > > index 0000000000000000000000000000000000000000..9f150fc2b6f379cc4707ff45041dd014956ae11a
> > > --- /dev/null
> > > +++ b/Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst
> > > @@ -0,0 +1,31 @@
> > > +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
> > > +
> > > +.. _v4l2-meta-fmt-uvc-custom:
> > > +
> > > +*********************************
> > > +V4L2_META_FMT_UVC_CUSTOM ('UVCC')
> > > +*********************************
> > > +
> > > +UVC Custom Payload Metadata.
> > > +
> > > +
> > > +Description
> > > +===========
> > > +
> > > +V4L2_META_FMT_UVC_CUSTOM buffers follow the metadata buffer layout of
> > > +V4L2_META_FMT_UVC with the only difference that it includes all the UVC
> > > +metadata, not just the first 2-12 bytes.
> > > +
> > > +The most common metadata format is the one proposed by Microsoft(R)'s UVC
> > > +extension [1_], but other vendors might have different formats.
> > > +
> > > +Applications might use information from the Hardware Database (hwdb)[2_] to
> > > +process the camera's metadata accordingly.
> >
> > Having something like that at the userspace API shouldn't be handled
> > lightly. This sounds to me that passing a blank check for vendors to stream
> > whatever they want without any requirements to provide and sort of
> > documentation for the usersace to decode it.
> 
> As HdG previously mentioned, all the processing is done in the camera
> so the metadata is not going to hide highly secret required for
> processing:
> https://lore.kernel.org/linux-media/67c1a857-7656-421f-994c-751709b6ae01@redhat.com/

Without judging whether or not such an undocumented format should be
supported by the driver, a correction is needed here: the issue is not
"secrets required for processing", but giving closed-source application
an unfair advantage.

> > Also, it would be hard for userspace to distinguish what metatata
> > is contained for a random UVC camera. Please let's not do that.
> 
> Userspace will use hwdb info to properly parse the metadata.

I don't have experience with that, so I would like to see the effort
being started on hwdb support to see how it will look like before we
merge this patch. A few cameras should be added as examples, and a
stategy to ensure the hwdb will be properly populated should be
proposed.

> > As the specific issue here is to support an already known extension,
> > which is already documented, Just add an specific format for it, e.g.
> > you could add something like that at the documentation:
> 
> The problem here is how do we know from the driver if a device
> supports V4L2_META_FMT_MSXU_UVC_1_5 or not.
> 
> In Windows it seems that vendors add that information to the device
> .inf file. That is equivalent to the hwdb proposal.
> In ChromeOS we are trying to push vendors to use an extension saying
> if there is metadata or not. But that will take some time to land and
> there are thousands of modules out there not ChromeOS compliant.
> 
> >
> >         V4L2_META_FMT_MSXU_UVC_1_5
> >            Microsoft extensions to USB Video Class 1.5 specification.
> >
> >            For more details, see: https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5
> >
> > And then add the corresponding format to V4L2 API.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 3/3] media: uvcvideo: Introduce V4L2_META_FMT_UVC_CUSTOM

[Ricardo Ribalda]

On Fri, 14 Mar 2025 at 10:09, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Ricardo,
>
> On Fri, Mar 14, 2025 at 09:28:34AM +0100, Ricardo Ribalda wrote:
> > On Fri, 14 Mar 2025 at 07:35, Mauro Carvalho Chehab wrote:
> > > Em Thu, 13 Mar 2025 12:06:27 +0000 Ricardo Ribalda escreveu:
> > >
> > > > 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 to have the whole metadata section 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 introduces a new format V4L2_META_FMT_UVC_CUSTOM, that is
> > > > identical to V4L2_META_FMT_D4XX but it is available to all the UVC
> > > > devices.
> > > >
> > > > Suggested-by: Hans de Goede <hdegoede@redhat.com>
> > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > ---
> > > >  .../userspace-api/media/v4l/meta-formats.rst       |  1 +
> > > >  .../userspace-api/media/v4l/metafmt-uvc-custom.rst | 31 +++++++++++++++++
> > > >  MAINTAINERS                                        |  1 +
> > > >  drivers/media/usb/uvc/uvc_metadata.c               | 40 ++++++++++++++++++----
> > > >  drivers/media/v4l2-core/v4l2-ioctl.c               |  1 +
> > > >  include/uapi/linux/videodev2.h                     |  1 +
> > > >  6 files changed, 69 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > > > index 86ffb3bc8ade2e0c563dd84441572ecea1a571a6..9fd83f4a3cc8509702a2a9f032fdc04bf6c6d1bc 100644
> > > > --- a/Documentation/userspace-api/media/v4l/meta-formats.rst
> > > > +++ b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > > > @@ -19,6 +19,7 @@ These formats are used for the :ref:`metadata` interface only.
> > > >      metafmt-pisp-fe
> > > >      metafmt-rkisp1
> > > >      metafmt-uvc
> > > > +    metafmt-uvc-custom
> > > >      metafmt-vivid
> > > >      metafmt-vsp1-hgo
> > > >      metafmt-vsp1-hgt
> > > > diff --git a/Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst b/Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst
> > > > new file mode 100644
> > > > index 0000000000000000000000000000000000000000..9f150fc2b6f379cc4707ff45041dd014956ae11a
> > > > --- /dev/null
> > > > +++ b/Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst
> > > > @@ -0,0 +1,31 @@
> > > > +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
> > > > +
> > > > +.. _v4l2-meta-fmt-uvc-custom:
> > > > +
> > > > +*********************************
> > > > +V4L2_META_FMT_UVC_CUSTOM ('UVCC')
> > > > +*********************************
> > > > +
> > > > +UVC Custom Payload Metadata.
> > > > +
> > > > +
> > > > +Description
> > > > +===========
> > > > +
> > > > +V4L2_META_FMT_UVC_CUSTOM buffers follow the metadata buffer layout of
> > > > +V4L2_META_FMT_UVC with the only difference that it includes all the UVC
> > > > +metadata, not just the first 2-12 bytes.
> > > > +
> > > > +The most common metadata format is the one proposed by Microsoft(R)'s UVC
> > > > +extension [1_], but other vendors might have different formats.
> > > > +
> > > > +Applications might use information from the Hardware Database (hwdb)[2_] to
> > > > +process the camera's metadata accordingly.
> > >
> > > Having something like that at the userspace API shouldn't be handled
> > > lightly. This sounds to me that passing a blank check for vendors to stream
> > > whatever they want without any requirements to provide and sort of
> > > documentation for the usersace to decode it.
> >
> > As HdG previously mentioned, all the processing is done in the camera
> > so the metadata is not going to hide highly secret required for
> > processing:
> > https://lore.kernel.org/linux-media/67c1a857-7656-421f-994c-751709b6ae01@redhat.com/
>
> Without judging whether or not such an undocumented format should be
> supported by the driver, a correction is needed here: the issue is not
> "secrets required for processing", but giving closed-source application
> an unfair advantage.

We could argue that vendors already have the possibility to pass
secrets to userspace:
- A camera could add proprietary information inside the frame, only
parseable by a closed-source application
- They can use undocumented UVC controls
- They can exploit documented controls to do something else that they
are designed for.
Given these existing possibilities, I question whether "evil metadata"
 offers any fundamentally new capabilities that cannot be achieved
through these established methods.

If we have to talk about unfair advantage, Linux is at an unfair
advantage right now: there is no way to use the *documented*
information provided by the metadata. Other OSs can use it.
The way I see it, with this artificial limitation we are not blocking
evil vendors but punishing good users.

if it makes you feel more comfortable we can start enabling only
V4L2_META_FMT_UVC_CUSTOM (or V4L2_META_FMT_MSXU_UVC_1_5 as proposed by
Mauro) to devices that support MSXU_CONTROL_METADATA, the future
ChromeOS XU, or quirks. But that artificial limitation will hurt a lot
of current cameras for no real reason.

>
> > > Also, it would be hard for userspace to distinguish what metatata
> > > is contained for a random UVC camera. Please let's not do that.
> >
> > Userspace will use hwdb info to properly parse the metadata.
>
> I don't have experience with that, so I would like to see the effort
> being started on hwdb support to see how it will look like before we
> merge this patch. A few cameras should be added as examples, and a
> stategy to ensure the hwdb will be properly populated should be
> proposed.

We can start by mapping the D4XX cameras. D4XX format follows
Microsoft standard.

Would that work for you?

>
> > > As the specific issue here is to support an already known extension,
> > > which is already documented, Just add an specific format for it, e.g.
> > > you could add something like that at the documentation:
> >
> > The problem here is how do we know from the driver if a device
> > supports V4L2_META_FMT_MSXU_UVC_1_5 or not.
> >
> > In Windows it seems that vendors add that information to the device
> > .inf file. That is equivalent to the hwdb proposal.
> > In ChromeOS we are trying to push vendors to use an extension saying
> > if there is metadata or not. But that will take some time to land and
> > there are thousands of modules out there not ChromeOS compliant.
> >
> > >
> > >         V4L2_META_FMT_MSXU_UVC_1_5
> > >            Microsoft extensions to USB Video Class 1.5 specification.
> > >
> > >            For more details, see: https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5
> > >
> > > And then add the corresponding format to V4L2 API.
>
> --
> Regards,
>
> Laurent Pinchart



-- 
Ricardo Ribalda

Re: [PATCH v3 3/3] media: uvcvideo: Introduce V4L2_META_FMT_UVC_CUSTOM

[Ricardo Ribalda]

Hi Laurent

On Fri, 14 Mar 2025 at 11:17, Ricardo Ribalda <ribalda@chromium.org> wrote:
>
> On Fri, 14 Mar 2025 at 10:09, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> >
> > Hi Ricardo,
> >
> > On Fri, Mar 14, 2025 at 09:28:34AM +0100, Ricardo Ribalda wrote:
> > > On Fri, 14 Mar 2025 at 07:35, Mauro Carvalho Chehab wrote:
> > > > Em Thu, 13 Mar 2025 12:06:27 +0000 Ricardo Ribalda escreveu:
> > > >
> > > > > 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 to have the whole metadata section 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 introduces a new format V4L2_META_FMT_UVC_CUSTOM, that is
> > > > > identical to V4L2_META_FMT_D4XX but it is available to all the UVC
> > > > > devices.
> > > > >
> > > > > Suggested-by: Hans de Goede <hdegoede@redhat.com>
> > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > > ---
> > > > >  .../userspace-api/media/v4l/meta-formats.rst       |  1 +
> > > > >  .../userspace-api/media/v4l/metafmt-uvc-custom.rst | 31 +++++++++++++++++
> > > > >  MAINTAINERS                                        |  1 +
> > > > >  drivers/media/usb/uvc/uvc_metadata.c               | 40 ++++++++++++++++++----
> > > > >  drivers/media/v4l2-core/v4l2-ioctl.c               |  1 +
> > > > >  include/uapi/linux/videodev2.h                     |  1 +
> > > > >  6 files changed, 69 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > > > > index 86ffb3bc8ade2e0c563dd84441572ecea1a571a6..9fd83f4a3cc8509702a2a9f032fdc04bf6c6d1bc 100644
> > > > > --- a/Documentation/userspace-api/media/v4l/meta-formats.rst
> > > > > +++ b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > > > > @@ -19,6 +19,7 @@ These formats are used for the :ref:`metadata` interface only.
> > > > >      metafmt-pisp-fe
> > > > >      metafmt-rkisp1
> > > > >      metafmt-uvc
> > > > > +    metafmt-uvc-custom
> > > > >      metafmt-vivid
> > > > >      metafmt-vsp1-hgo
> > > > >      metafmt-vsp1-hgt
> > > > > diff --git a/Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst b/Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst
> > > > > new file mode 100644
> > > > > index 0000000000000000000000000000000000000000..9f150fc2b6f379cc4707ff45041dd014956ae11a
> > > > > --- /dev/null
> > > > > +++ b/Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst
> > > > > @@ -0,0 +1,31 @@
> > > > > +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
> > > > > +
> > > > > +.. _v4l2-meta-fmt-uvc-custom:
> > > > > +
> > > > > +*********************************
> > > > > +V4L2_META_FMT_UVC_CUSTOM ('UVCC')
> > > > > +*********************************
> > > > > +
> > > > > +UVC Custom Payload Metadata.
> > > > > +
> > > > > +
> > > > > +Description
> > > > > +===========
> > > > > +
> > > > > +V4L2_META_FMT_UVC_CUSTOM buffers follow the metadata buffer layout of
> > > > > +V4L2_META_FMT_UVC with the only difference that it includes all the UVC
> > > > > +metadata, not just the first 2-12 bytes.
> > > > > +
> > > > > +The most common metadata format is the one proposed by Microsoft(R)'s UVC
> > > > > +extension [1_], but other vendors might have different formats.
> > > > > +
> > > > > +Applications might use information from the Hardware Database (hwdb)[2_] to
> > > > > +process the camera's metadata accordingly.
> > > >
> > > > Having something like that at the userspace API shouldn't be handled
> > > > lightly. This sounds to me that passing a blank check for vendors to stream
> > > > whatever they want without any requirements to provide and sort of
> > > > documentation for the usersace to decode it.
> > >
> > > As HdG previously mentioned, all the processing is done in the camera
> > > so the metadata is not going to hide highly secret required for
> > > processing:
> > > https://lore.kernel.org/linux-media/67c1a857-7656-421f-994c-751709b6ae01@redhat.com/
> >
> > Without judging whether or not such an undocumented format should be
> > supported by the driver, a correction is needed here: the issue is not
> > "secrets required for processing", but giving closed-source application
> > an unfair advantage.
>
> We could argue that vendors already have the possibility to pass
> secrets to userspace:
> - A camera could add proprietary information inside the frame, only
> parseable by a closed-source application
> - They can use undocumented UVC controls
> - They can exploit documented controls to do something else that they
> are designed for.
> Given these existing possibilities, I question whether "evil metadata"
>  offers any fundamentally new capabilities that cannot be achieved
> through these established methods.
>
> If we have to talk about unfair advantage, Linux is at an unfair
> advantage right now: there is no way to use the *documented*
> information provided by the metadata. Other OSs can use it.
> The way I see it, with this artificial limitation we are not blocking
> evil vendors but punishing good users.
>
> if it makes you feel more comfortable we can start enabling only
> V4L2_META_FMT_UVC_CUSTOM (or V4L2_META_FMT_MSXU_UVC_1_5 as proposed by
> Mauro) to devices that support MSXU_CONTROL_METADATA, the future
> ChromeOS XU, or quirks. But that artificial limitation will hurt a lot
> of current cameras for no real reason.

I guess this mail has fallen through the cracks....

I have just sent a new revision that limits the new metadata format to
devices that support MSXU_CONTROL_METADATA, so it can bump the
discussion.


>
> >
> > > > Also, it would be hard for userspace to distinguish what metatata
> > > > is contained for a random UVC camera. Please let's not do that.
> > >
> > > Userspace will use hwdb info to properly parse the metadata.
> >
> > I don't have experience with that, so I would like to see the effort
> > being started on hwdb support to see how it will look like before we
> > merge this patch. A few cameras should be added as examples, and a
> > stategy to ensure the hwdb will be properly populated should be
> > proposed.
>
> We can start by mapping the D4XX cameras. D4XX format follows
> Microsoft standard.
>
> Would that work for you?
>
> >
> > > > As the specific issue here is to support an already known extension,
> > > > which is already documented, Just add an specific format for it, e.g.
> > > > you could add something like that at the documentation:
> > >
> > > The problem here is how do we know from the driver if a device
> > > supports V4L2_META_FMT_MSXU_UVC_1_5 or not.
> > >
> > > In Windows it seems that vendors add that information to the device
> > > .inf file. That is equivalent to the hwdb proposal.
> > > In ChromeOS we are trying to push vendors to use an extension saying
> > > if there is metadata or not. But that will take some time to land and
> > > there are thousands of modules out there not ChromeOS compliant.
> > >
> > > >
> > > >         V4L2_META_FMT_MSXU_UVC_1_5
> > > >            Microsoft extensions to USB Video Class 1.5 specification.
> > > >
> > > >            For more details, see: https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5
> > > >
> > > > And then add the corresponding format to V4L2 API.
> >
> > --
> > Regards,
> >
> > Laurent Pinchart
>
>
>
> --
> Ricardo Ribalda



-- 
Ricardo Ribalda