linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v8 0/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5 + other meta fixes
@ 2025-07-07 18:34 Ricardo Ribalda
  2025-07-07 18:34 ` [PATCH v8 1/5] media: uvcvideo: Do not mark valid metadata as invalid Ricardo Ribalda
                   ` (5 more replies)
  0 siblings, 6 replies; 21+ messages in thread
From: Ricardo Ribalda @ 2025-07-07 18:34 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab, Guennadi Liakhovetski,
	Greg Kroah-Hartman, Hans de Goede, Hans de Goede
  Cc: linux-media, linux-kernel, linux-usb, Ricardo Ribalda, stable

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

The new metadata format can be enabled in runtime with quirks.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Changes in v8:
- Dynamically fill dev->meta_formats instead of be const.
- Link to v7: https://lore.kernel.org/r/20250617-uvc-meta-v7-0-9c50623e2286@chromium.org

Changes in v7:
- Add patch: Introduce dev->meta_formats
- Link to v6: https://lore.kernel.org/r/20250604-uvc-meta-v6-0-7141d48c322c@chromium.org

Changes in v6 (Thanks Laurent):
- Fix typo in metafmt-uvc.rst
- Improve metafmt-uvc-msxu-1-5.rst
- uvc_meta_v4l2_try_format() block MSXU format unless the quirk is
  active
- Refactor uvc_enable_msxu
- Document uvc_meta_detect_msxu
- Rebase series
- Add R-b
- Link to v5: https://lore.kernel.org/r/20250404-uvc-meta-v5-0-f79974fc2d20@chromium.org

Changes in v5:
- Fix codestyle and kerneldoc warnings reported by media-ci
- Link to v4: https://lore.kernel.org/r/20250403-uvc-meta-v4-0-877aa6475975@chromium.org

Changes in v4:
- Rename format to V4L2_META_FMT_UVC_MSXU_1_5 (Thanks Mauro)
- Flag the new format with a quirk.
- Autodetect MSXU devices.
- Link to v3: https://lore.kernel.org/linux-media/20250313-uvc-metadata-v3-0-c467af869c60@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
- Introduce 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 (5):
      media: uvcvideo: Do not mark valid metadata as invalid
      media: Documentation: Add note about UVCH length field
      media: uvcvideo: Introduce dev->meta_formats
      media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5
      media: uvcvideo: Auto-set UVC_QUIRK_MSXU_META

 .../userspace-api/media/v4l/meta-formats.rst       |   1 +
 .../media/v4l/metafmt-uvc-msxu-1-5.rst             |  23 +++++
 .../userspace-api/media/v4l/metafmt-uvc.rst        |   4 +-
 MAINTAINERS                                        |   1 +
 drivers/media/usb/uvc/uvc_driver.c                 |   7 ++
 drivers/media/usb/uvc/uvc_metadata.c               | 115 +++++++++++++++++++--
 drivers/media/usb/uvc/uvc_video.c                  |  12 +--
 drivers/media/usb/uvc/uvcvideo.h                   |   7 ++
 drivers/media/v4l2-core/v4l2-ioctl.c               |   1 +
 include/linux/usb/uvc.h                            |   3 +
 include/uapi/linux/videodev2.h                     |   1 +
 11 files changed, 161 insertions(+), 14 deletions(-)
---
base-commit: a8598c7de1bcd94461ca54c972efa9b4ea501fb9
change-id: 20250403-uvc-meta-e556773d12ae

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


^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH v8 1/5] media: uvcvideo: Do not mark valid metadata as invalid
  2025-07-07 18:34 [PATCH v8 0/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5 + other meta fixes Ricardo Ribalda
@ 2025-07-07 18:34 ` Ricardo Ribalda
  2025-07-07 18:34 ` [PATCH v8 2/5] media: Documentation: Add note about UVCH length field Ricardo Ribalda
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 21+ messages in thread
From: Ricardo Ribalda @ 2025-07-07 18:34 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab, Guennadi Liakhovetski,
	Greg Kroah-Hartman, Hans de Goede, Hans de Goede
  Cc: linux-media, linux-kernel, linux-usb, Ricardo Ribalda, stable

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")
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Hans de Goede <hansg@kernel.org>
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 11769a1832d2ba9b3f9a50bcb10b0c4cdff71f09..2e377e7b9e81599aca19b800a171cc16a09c1e8a 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1442,12 +1442,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;
 
@@ -1468,6 +1462,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.50.0.727.gbf7dc18ff4-goog


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v8 2/5] media: Documentation: Add note about UVCH length field
  2025-07-07 18:34 [PATCH v8 0/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5 + other meta fixes Ricardo Ribalda
  2025-07-07 18:34 ` [PATCH v8 1/5] media: uvcvideo: Do not mark valid metadata as invalid Ricardo Ribalda
@ 2025-07-07 18:34 ` Ricardo Ribalda
  2025-07-07 18:34 ` [PATCH v8 3/5] media: uvcvideo: Introduce dev->meta_formats Ricardo Ribalda
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 21+ messages in thread
From: Ricardo Ribalda @ 2025-07-07 18:34 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab, Guennadi Liakhovetski,
	Greg Kroah-Hartman, Hans de Goede, Hans de Goede
  Cc: linux-media, linux-kernel, linux-usb, 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.

Reviewed-by: Hans de Goede <hansg@kernel.org>
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..4c05e9e54683a2bf844ddf26f99d0d9713ef05de 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 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.50.0.727.gbf7dc18ff4-goog


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v8 3/5] media: uvcvideo: Introduce dev->meta_formats
  2025-07-07 18:34 [PATCH v8 0/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5 + other meta fixes Ricardo Ribalda
  2025-07-07 18:34 ` [PATCH v8 1/5] media: uvcvideo: Do not mark valid metadata as invalid Ricardo Ribalda
  2025-07-07 18:34 ` [PATCH v8 2/5] media: Documentation: Add note about UVCH length field Ricardo Ribalda
@ 2025-07-07 18:34 ` Ricardo Ribalda
  2025-07-07 20:55   ` Hans de Goede
  2025-07-07 18:34 ` [PATCH v8 4/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5 Ricardo Ribalda
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 21+ messages in thread
From: Ricardo Ribalda @ 2025-07-07 18:34 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab, Guennadi Liakhovetski,
	Greg Kroah-Hartman, Hans de Goede, Hans de Goede
  Cc: linux-media, linux-kernel, linux-usb, Ricardo Ribalda

Right now, there driver supports devices with one or two metadata
formats. Prepare it to support more than two metadata formats.

This is achieved with the introduction of a new field `meta_formats`,
that contains the array of metadata formats supported by the device, in
the order expected by userspace.

Suggested-by: Hans de Goede <hansg@kernel.org>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_driver.c   |  2 ++
 drivers/media/usb/uvc/uvc_metadata.c | 38 +++++++++++++++++++++++++++++-------
 drivers/media/usb/uvc/uvcvideo.h     |  6 ++++++
 3 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 62eb45435d8bec5c955720ecb46fb8936871e6cc..56ea20eeb7b9d5d92f3d837c15bdf11d536e9f2d 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -2315,6 +2315,8 @@ static int uvc_probe(struct usb_interface *intf,
 		goto error;
 	}
 
+	uvc_meta_init(dev);
+
 	if (dev->quirks & UVC_QUIRK_NO_RESET_RESUME)
 		udev->quirks &= ~USB_QUIRK_RESET_RESUME;
 
diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
index 82de7781f5b6b70c5ba16bcba9e0741231231904..4bcbc22f47e67c52baf6e133f240131ff3d32a03 100644
--- a/drivers/media/usb/uvc/uvc_metadata.c
+++ b/drivers/media/usb/uvc/uvc_metadata.c
@@ -64,14 +64,20 @@ static int uvc_meta_v4l2_try_format(struct file *file, void *fh,
 	struct uvc_device *dev = stream->dev;
 	struct v4l2_meta_format *fmt = &format->fmt.meta;
 	u32 fmeta = fmt->dataformat;
+	u32 i;
 
 	if (format->type != vfh->vdev->queue->type)
 		return -EINVAL;
 
+	for (i = 0; (fmeta != dev->meta_formats[i]) && dev->meta_formats[i];
+	     i++)
+		;
+	if (!dev->meta_formats[i])
+		fmeta = V4L2_META_FMT_UVC;
+
 	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;
@@ -112,17 +118,21 @@ static int uvc_meta_v4l2_enum_formats(struct file *file, void *fh,
 	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;
+	u32 i;
+
+	if (fdesc->type != vfh->vdev->queue->type)
+		return -EINVAL;
 
-	if (fdesc->type != vfh->vdev->queue->type ||
-	    index > 1U || (index && !dev->info->meta_format))
+	for (i = 0; (i < fdesc->index) && dev->meta_formats[i]; i++)
+		;
+	if (!dev->meta_formats[i])
 		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->index = i;
+	fdesc->pixelformat = dev->meta_formats[i];
 
 	return 0;
 }
@@ -174,3 +184,17 @@ int uvc_meta_register(struct uvc_streaming *stream)
 					 V4L2_BUF_TYPE_META_CAPTURE,
 					 &uvc_meta_fops, &uvc_meta_ioctl_ops);
 }
+
+void uvc_meta_init(struct uvc_device *dev)
+{
+	unsigned int i = 0;
+
+	dev->meta_formats[i++] = V4L2_META_FMT_UVC;
+
+	if (dev->info->meta_format &&
+	    !WARN_ON(dev->info->meta_format == V4L2_META_FMT_UVC))
+		dev->meta_formats[i++] = dev->info->meta_format;
+
+	 /* IMPORTANT: for new meta-formats update UVC_MAX_META_DATA_FORMATS. */
+	dev->meta_formats[i++] = 0;
+}
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 11d6e3c2ebdfbabd7bbe5722f88ff85f406d9bb6..b3c094c6591e7a71fc00e1096bcf493a83f330ad 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -572,6 +572,8 @@ struct uvc_status {
 	};
 } __packed;
 
+#define UVC_MAX_META_DATA_FORMATS 3
+
 struct uvc_device {
 	struct usb_device *udev;
 	struct usb_interface *intf;
@@ -582,6 +584,9 @@ struct uvc_device {
 
 	const struct uvc_device_info *info;
 
+	/* Zero-ended list of meta formats */
+	u32 meta_formats[UVC_MAX_META_DATA_FORMATS + 1];
+
 	atomic_t nmappings;
 
 	/* Video control interface */
@@ -751,6 +756,7 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
 void uvc_video_clock_update(struct uvc_streaming *stream,
 			    struct vb2_v4l2_buffer *vbuf,
 			    struct uvc_buffer *buf);
+void uvc_meta_init(struct uvc_device *dev);
 int uvc_meta_register(struct uvc_streaming *stream);
 
 int uvc_register_video_device(struct uvc_device *dev,

-- 
2.50.0.727.gbf7dc18ff4-goog


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v8 4/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5
  2025-07-07 18:34 [PATCH v8 0/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5 + other meta fixes Ricardo Ribalda
                   ` (2 preceding siblings ...)
  2025-07-07 18:34 ` [PATCH v8 3/5] media: uvcvideo: Introduce dev->meta_formats Ricardo Ribalda
@ 2025-07-07 18:34 ` Ricardo Ribalda
  2025-07-11 20:13   ` Laurent Pinchart
  2025-07-14 14:59   ` Laurent Pinchart
  2025-07-07 18:34 ` [PATCH v8 5/5] media: uvcvideo: Auto-set UVC_QUIRK_MSXU_META Ricardo Ribalda
  2025-07-07 21:00 ` [PATCH v8 0/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5 + other meta fixes Hans de Goede
  5 siblings, 2 replies; 21+ messages in thread
From: Ricardo Ribalda @ 2025-07-07 18:34 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab, Guennadi Liakhovetski,
	Greg Kroah-Hartman, Hans de Goede, Hans de Goede
  Cc: linux-media, linux-kernel, linux-usb, 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 payload 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_MSXU_1_5, that is
identical to V4L2_META_FMT_D4XX.

Let the user enable this format with a quirk for now. This way they can
test if their devices provide useful metadata without rebuilding the
kernel. They can later contribute patches to auto-quirk their devices.
We will also work in methods to auto-detect devices compatible with this
new metadata format.

Suggested-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Hans de Goede <hansg@kernel.org>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 .../userspace-api/media/v4l/meta-formats.rst       |  1 +
 .../media/v4l/metafmt-uvc-msxu-1-5.rst             | 23 ++++++++++++++++++++++
 MAINTAINERS                                        |  1 +
 drivers/media/usb/uvc/uvc_metadata.c               |  4 ++++
 drivers/media/usb/uvc/uvcvideo.h                   |  1 +
 drivers/media/v4l2-core/v4l2-ioctl.c               |  1 +
 include/uapi/linux/videodev2.h                     |  1 +
 7 files changed, 32 insertions(+)

diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst
index bb6876cfc271e1a0543eee4209d6251e1a6a73cc..0de80328c36bf148051a19abe9e5241234ddfe5c 100644
--- a/Documentation/userspace-api/media/v4l/meta-formats.rst
+++ b/Documentation/userspace-api/media/v4l/meta-formats.rst
@@ -20,6 +20,7 @@ These formats are used for the :ref:`metadata` interface only.
     metafmt-pisp-fe
     metafmt-rkisp1
     metafmt-uvc
+    metafmt-uvc-msxu-1-5
     metafmt-vivid
     metafmt-vsp1-hgo
     metafmt-vsp1-hgt
diff --git a/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst b/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst
new file mode 100644
index 0000000000000000000000000000000000000000..dd1c3076df243d770a13e7f6d07c3296a269e16a
--- /dev/null
+++ b/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst
@@ -0,0 +1,23 @@
+.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
+
+.. _v4l2-meta-fmt-uvc-msxu-1-5:
+
+***********************************
+V4L2_META_FMT_UVC_MSXU_1_5 ('UVCM')
+***********************************
+
+Microsoft(R)'s UVC Payload Metadata.
+
+
+Description
+===========
+
+V4L2_META_FMT_UVC_MSXU_1_5 buffers follow the metadata buffer layout of
+V4L2_META_FMT_UVC with the only difference that it includes all the UVC
+metadata in the `buffer[]` field, not just the first 2-12 bytes.
+
+The metadata format follows the specification from Microsoft(R) [1].
+
+.. _1:
+
+[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5
diff --git a/MAINTAINERS b/MAINTAINERS
index 658543062bba3b7e600699d7271ffc89250ba7e5..fdde1d37ed2ef9058e3ea3417bec25afe454dfc0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -25827,6 +25827,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-msxu-1-5.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 4bcbc22f47e67c52baf6e133f240131ff3d32a03..77e03273d3cf6b00cac6ebb9b29b941f1cbfd9f7 100644
--- a/drivers/media/usb/uvc/uvc_metadata.c
+++ b/drivers/media/usb/uvc/uvc_metadata.c
@@ -195,6 +195,10 @@ void uvc_meta_init(struct uvc_device *dev)
 	    !WARN_ON(dev->info->meta_format == V4L2_META_FMT_UVC))
 		dev->meta_formats[i++] = dev->info->meta_format;
 
+	if (dev->quirks & UVC_QUIRK_MSXU_META &&
+	    !WARN_ON(dev->info->meta_format == V4L2_META_FMT_UVC_MSXU_1_5))
+		dev->meta_formats[i++] = V4L2_META_FMT_UVC_MSXU_1_5;
+
 	 /* IMPORTANT: for new meta-formats update UVC_MAX_META_DATA_FORMATS. */
 	dev->meta_formats[i++] = 0;
 }
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index b3c094c6591e7a71fc00e1096bcf493a83f330ad..616adc417c62a58686beccbc440a5dfac0a2d588 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -77,6 +77,7 @@
 #define UVC_QUIRK_DISABLE_AUTOSUSPEND	0x00008000
 #define UVC_QUIRK_INVALID_DEVICE_SOF	0x00010000
 #define UVC_QUIRK_MJPEG_NO_EOF		0x00020000
+#define UVC_QUIRK_MSXU_META		0x00040000
 
 /* Format flags */
 #define UVC_FMT_FLAG_COMPRESSED		0x00000001
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
index be94a79b976e3de4eb957f5d2584ec6d4230469e..993b36417b4655456ce545cb42a41b55b98e4d6c 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -1463,6 +1463,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_MSXU_1_5:	descr = "UVC MSXU 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 6f7bd38dd5aa4b1b2084685512512a380d76a5e4..863bc5b7dec32303e852d7e9c3891011ce5a3d71 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -867,6 +867,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_MSXU_1_5  v4l2_fourcc('U', 'V', 'C', 'M') /* UVC MSXU metadata */
 #define V4L2_META_FMT_VIVID	  v4l2_fourcc('V', 'I', 'V', 'D') /* Vivid Metadata */
 
 /* Vendor specific - used for RK_ISP1 camera sub-system */

-- 
2.50.0.727.gbf7dc18ff4-goog


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH v8 5/5] media: uvcvideo: Auto-set UVC_QUIRK_MSXU_META
  2025-07-07 18:34 [PATCH v8 0/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5 + other meta fixes Ricardo Ribalda
                   ` (3 preceding siblings ...)
  2025-07-07 18:34 ` [PATCH v8 4/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5 Ricardo Ribalda
@ 2025-07-07 18:34 ` Ricardo Ribalda
  2025-07-11 19:58   ` Laurent Pinchart
  2025-07-07 21:00 ` [PATCH v8 0/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5 + other meta fixes Hans de Goede
  5 siblings, 1 reply; 21+ messages in thread
From: Ricardo Ribalda @ 2025-07-07 18:34 UTC (permalink / raw)
  To: Laurent Pinchart, Mauro Carvalho Chehab, Guennadi Liakhovetski,
	Greg Kroah-Hartman, Hans de Goede, Hans de Goede
  Cc: linux-media, linux-kernel, linux-usb, Ricardo Ribalda

If the camera supports the MSXU_CONTROL_METADATA control, auto set the
MSXU_META quirk.

Reviewed-by: Hans de Goede <hansg@kernel.org>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_driver.c   |  7 +++-
 drivers/media/usb/uvc/uvc_metadata.c | 75 +++++++++++++++++++++++++++++++++++-
 drivers/media/usb/uvc/uvcvideo.h     |  2 +-
 include/linux/usb/uvc.h              |  3 ++
 4 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index 56ea20eeb7b9d5d92f3d837c15bdf11d536e9f2d..9de5abb43e19d9e876cddc5d7124592953db89ac 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -2315,7 +2315,12 @@ static int uvc_probe(struct usb_interface *intf,
 		goto error;
 	}
 
-	uvc_meta_init(dev);
+	ret = uvc_meta_init(dev);
+	if (ret < 0) {
+		dev_err(&dev->udev->dev,
+			"Error initializing the metadata formats (%d)\n", ret);
+		goto error;
+	}
 
 	if (dev->quirks & UVC_QUIRK_NO_RESET_RESUME)
 		udev->quirks &= ~USB_QUIRK_RESET_RESUME;
diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
index 77e03273d3cf6b00cac6ebb9b29b941f1cbfd9f7..59bb133baf9a73ef6a30fa8ead85aa90653d60f4 100644
--- a/drivers/media/usb/uvc/uvc_metadata.c
+++ b/drivers/media/usb/uvc/uvc_metadata.c
@@ -10,6 +10,7 @@
 #include <linux/list.h>
 #include <linux/module.h>
 #include <linux/usb.h>
+#include <linux/usb/uvc.h>
 #include <linux/videodev2.h>
 
 #include <media/v4l2-ioctl.h>
@@ -166,6 +167,71 @@ static const struct v4l2_file_operations uvc_meta_fops = {
 	.mmap = vb2_fop_mmap,
 };
 
+static struct uvc_entity *uvc_meta_find_msxu(struct uvc_device *dev)
+{
+	static const u8 uvc_msxu_guid[16] = UVC_GUID_MSXU_1_5;
+	struct uvc_entity *entity;
+
+	list_for_each_entry(entity, &dev->entities, list) {
+		if (!memcmp(entity->guid, uvc_msxu_guid, sizeof(entity->guid)))
+			return entity;
+	}
+
+	return NULL;
+}
+
+#define MSXU_CONTROL_METADATA 0x9
+static int uvc_meta_detect_msxu(struct uvc_device *dev)
+{
+	u32 *data __free(kfree) = NULL;
+	struct uvc_entity *entity;
+	int ret;
+
+	entity = uvc_meta_find_msxu(dev);
+	if (!entity)
+		return 0;
+
+	/*
+	 * USB requires buffers aligned in a special way, simplest way is to
+	 * make sure that query_ctrl will work is to kmalloc() them.
+	 */
+	data = kmalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	/* Check if the metadata is already enabled. */
+	ret = uvc_query_ctrl(dev, UVC_GET_CUR, entity->id, dev->intfnum,
+			     MSXU_CONTROL_METADATA, data, sizeof(*data));
+	if (ret)
+		return 0;
+
+	if (*data) {
+		dev->quirks |= UVC_QUIRK_MSXU_META;
+		return 0;
+	}
+
+	/*
+	 * We have seen devices that require 1 to enable the metadata, others
+	 * requiring a value != 1 and others requiring a value >1. Luckily for
+	 * us, the value from GET_MAX seems to work all the time.
+	 */
+	ret = uvc_query_ctrl(dev, UVC_GET_MAX, entity->id, dev->intfnum,
+			     MSXU_CONTROL_METADATA, data, sizeof(*data));
+	if (ret || !*data)
+		return 0;
+
+	/*
+	 * If we can set MSXU_CONTROL_METADATA, the device will report
+	 * metadata.
+	 */
+	ret = uvc_query_ctrl(dev, UVC_SET_CUR, entity->id, dev->intfnum,
+			     MSXU_CONTROL_METADATA, data, sizeof(*data));
+	if (!ret)
+		dev->quirks |= UVC_QUIRK_MSXU_META;
+
+	return 0;
+}
+
 int uvc_meta_register(struct uvc_streaming *stream)
 {
 	struct uvc_device *dev = stream->dev;
@@ -185,9 +251,14 @@ int uvc_meta_register(struct uvc_streaming *stream)
 					 &uvc_meta_fops, &uvc_meta_ioctl_ops);
 }
 
-void uvc_meta_init(struct uvc_device *dev)
+int uvc_meta_init(struct uvc_device *dev)
 {
 	unsigned int i = 0;
+	int ret;
+
+	ret = uvc_meta_detect_msxu(dev);
+	if (ret)
+		return ret;
 
 	dev->meta_formats[i++] = V4L2_META_FMT_UVC;
 
@@ -201,4 +272,6 @@ void uvc_meta_init(struct uvc_device *dev)
 
 	 /* IMPORTANT: for new meta-formats update UVC_MAX_META_DATA_FORMATS. */
 	dev->meta_formats[i++] = 0;
+
+	return 0;
 }
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 616adc417c62a58686beccbc440a5dfac0a2d588..a4c064c5e046f2a4adba742c8777a10619569606 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -757,7 +757,7 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
 void uvc_video_clock_update(struct uvc_streaming *stream,
 			    struct vb2_v4l2_buffer *vbuf,
 			    struct uvc_buffer *buf);
-void uvc_meta_init(struct uvc_device *dev);
+int uvc_meta_init(struct uvc_device *dev);
 int uvc_meta_register(struct uvc_streaming *stream);
 
 int uvc_register_video_device(struct uvc_device *dev,
diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h
index bce95153e5a65613a710d7316fc17cf5462b5bce..ee19e9f915b8370c333c426dc1ee4202c7b75c5b 100644
--- a/include/linux/usb/uvc.h
+++ b/include/linux/usb/uvc.h
@@ -29,6 +29,9 @@
 #define UVC_GUID_EXT_GPIO_CONTROLLER \
 	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
 	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x03}
+#define UVC_GUID_MSXU_1_5 \
+	{0xdc, 0x95, 0x3f, 0x0f, 0x32, 0x26, 0x4e, 0x4c, \
+	 0x92, 0xc9, 0xa0, 0x47, 0x82, 0xf4, 0x3b, 0xc8}
 
 #define UVC_GUID_FORMAT_MJPEG \
 	{ 'M',  'J',  'P',  'G', 0x00, 0x00, 0x10, 0x00, \

-- 
2.50.0.727.gbf7dc18ff4-goog


^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 3/5] media: uvcvideo: Introduce dev->meta_formats
  2025-07-07 18:34 ` [PATCH v8 3/5] media: uvcvideo: Introduce dev->meta_formats Ricardo Ribalda
@ 2025-07-07 20:55   ` Hans de Goede
  0 siblings, 0 replies; 21+ messages in thread
From: Hans de Goede @ 2025-07-07 20:55 UTC (permalink / raw)
  To: Ricardo Ribalda, Laurent Pinchart, Mauro Carvalho Chehab,
	Guennadi Liakhovetski, Greg Kroah-Hartman
  Cc: linux-media, linux-kernel, linux-usb

Hi,

On 7-Jul-25 8:34 PM, Ricardo Ribalda wrote:
> Right now, there driver supports devices with one or two metadata
> formats. Prepare it to support more than two metadata formats.
> 
> This is achieved with the introduction of a new field `meta_formats`,
> that contains the array of metadata formats supported by the device, in
> the order expected by userspace.
> 
> Suggested-by: Hans de Goede <hansg@kernel.org>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hansg@kernel.org>

Regards,

Hans



> ---
>  drivers/media/usb/uvc/uvc_driver.c   |  2 ++
>  drivers/media/usb/uvc/uvc_metadata.c | 38 +++++++++++++++++++++++++++++-------
>  drivers/media/usb/uvc/uvcvideo.h     |  6 ++++++
>  3 files changed, 39 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 62eb45435d8bec5c955720ecb46fb8936871e6cc..56ea20eeb7b9d5d92f3d837c15bdf11d536e9f2d 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -2315,6 +2315,8 @@ static int uvc_probe(struct usb_interface *intf,
>  		goto error;
>  	}
>  
> +	uvc_meta_init(dev);
> +
>  	if (dev->quirks & UVC_QUIRK_NO_RESET_RESUME)
>  		udev->quirks &= ~USB_QUIRK_RESET_RESUME;
>  
> diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
> index 82de7781f5b6b70c5ba16bcba9e0741231231904..4bcbc22f47e67c52baf6e133f240131ff3d32a03 100644
> --- a/drivers/media/usb/uvc/uvc_metadata.c
> +++ b/drivers/media/usb/uvc/uvc_metadata.c
> @@ -64,14 +64,20 @@ static int uvc_meta_v4l2_try_format(struct file *file, void *fh,
>  	struct uvc_device *dev = stream->dev;
>  	struct v4l2_meta_format *fmt = &format->fmt.meta;
>  	u32 fmeta = fmt->dataformat;
> +	u32 i;
>  
>  	if (format->type != vfh->vdev->queue->type)
>  		return -EINVAL;
>  
> +	for (i = 0; (fmeta != dev->meta_formats[i]) && dev->meta_formats[i];
> +	     i++)
> +		;
> +	if (!dev->meta_formats[i])
> +		fmeta = V4L2_META_FMT_UVC;
> +
>  	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;
> @@ -112,17 +118,21 @@ static int uvc_meta_v4l2_enum_formats(struct file *file, void *fh,
>  	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;
> +	u32 i;
> +
> +	if (fdesc->type != vfh->vdev->queue->type)
> +		return -EINVAL;
>  
> -	if (fdesc->type != vfh->vdev->queue->type ||
> -	    index > 1U || (index && !dev->info->meta_format))
> +	for (i = 0; (i < fdesc->index) && dev->meta_formats[i]; i++)
> +		;
> +	if (!dev->meta_formats[i])
>  		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->index = i;
> +	fdesc->pixelformat = dev->meta_formats[i];
>  
>  	return 0;
>  }
> @@ -174,3 +184,17 @@ int uvc_meta_register(struct uvc_streaming *stream)
>  					 V4L2_BUF_TYPE_META_CAPTURE,
>  					 &uvc_meta_fops, &uvc_meta_ioctl_ops);
>  }
> +
> +void uvc_meta_init(struct uvc_device *dev)
> +{
> +	unsigned int i = 0;
> +
> +	dev->meta_formats[i++] = V4L2_META_FMT_UVC;
> +
> +	if (dev->info->meta_format &&
> +	    !WARN_ON(dev->info->meta_format == V4L2_META_FMT_UVC))
> +		dev->meta_formats[i++] = dev->info->meta_format;
> +
> +	 /* IMPORTANT: for new meta-formats update UVC_MAX_META_DATA_FORMATS. */
> +	dev->meta_formats[i++] = 0;
> +}
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 11d6e3c2ebdfbabd7bbe5722f88ff85f406d9bb6..b3c094c6591e7a71fc00e1096bcf493a83f330ad 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -572,6 +572,8 @@ struct uvc_status {
>  	};
>  } __packed;
>  
> +#define UVC_MAX_META_DATA_FORMATS 3
> +
>  struct uvc_device {
>  	struct usb_device *udev;
>  	struct usb_interface *intf;
> @@ -582,6 +584,9 @@ struct uvc_device {
>  
>  	const struct uvc_device_info *info;
>  
> +	/* Zero-ended list of meta formats */
> +	u32 meta_formats[UVC_MAX_META_DATA_FORMATS + 1];
> +
>  	atomic_t nmappings;
>  
>  	/* Video control interface */
> @@ -751,6 +756,7 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
>  void uvc_video_clock_update(struct uvc_streaming *stream,
>  			    struct vb2_v4l2_buffer *vbuf,
>  			    struct uvc_buffer *buf);
> +void uvc_meta_init(struct uvc_device *dev);
>  int uvc_meta_register(struct uvc_streaming *stream);
>  
>  int uvc_register_video_device(struct uvc_device *dev,
> 


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 0/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5 + other meta fixes
  2025-07-07 18:34 [PATCH v8 0/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5 + other meta fixes Ricardo Ribalda
                   ` (4 preceding siblings ...)
  2025-07-07 18:34 ` [PATCH v8 5/5] media: uvcvideo: Auto-set UVC_QUIRK_MSXU_META Ricardo Ribalda
@ 2025-07-07 21:00 ` Hans de Goede
  5 siblings, 0 replies; 21+ messages in thread
From: Hans de Goede @ 2025-07-07 21:00 UTC (permalink / raw)
  To: Ricardo Ribalda, Laurent Pinchart, Mauro Carvalho Chehab,
	Guennadi Liakhovetski, Greg Kroah-Hartman
  Cc: linux-media, linux-kernel, linux-usb, stable

Hi Ricardo,

On 7-Jul-25 8:34 PM, Ricardo Ribalda wrote:
> This series introduces a new metadata format for UVC cameras and adds a
> couple of improvements to the UVC metadata handling.
> 
> The new metadata format can be enabled in runtime with quirks.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> Changes in v8:
> - Dynamically fill dev->meta_formats instead of be const.
> - Link to v7: https://lore.kernel.org/r/20250617-uvc-meta-v7-0-9c50623e2286@chromium.org

Thank you for the new version. I've merged this series
and pushed this to uvc/for-next now.

Regards,

Hans




> Changes in v7:
> - Add patch: Introduce dev->meta_formats
> - Link to v6: https://lore.kernel.org/r/20250604-uvc-meta-v6-0-7141d48c322c@chromium.org
> 
> Changes in v6 (Thanks Laurent):
> - Fix typo in metafmt-uvc.rst
> - Improve metafmt-uvc-msxu-1-5.rst
> - uvc_meta_v4l2_try_format() block MSXU format unless the quirk is
>   active
> - Refactor uvc_enable_msxu
> - Document uvc_meta_detect_msxu
> - Rebase series
> - Add R-b
> - Link to v5: https://lore.kernel.org/r/20250404-uvc-meta-v5-0-f79974fc2d20@chromium.org
> 
> Changes in v5:
> - Fix codestyle and kerneldoc warnings reported by media-ci
> - Link to v4: https://lore.kernel.org/r/20250403-uvc-meta-v4-0-877aa6475975@chromium.org
> 
> Changes in v4:
> - Rename format to V4L2_META_FMT_UVC_MSXU_1_5 (Thanks Mauro)
> - Flag the new format with a quirk.
> - Autodetect MSXU devices.
> - Link to v3: https://lore.kernel.org/linux-media/20250313-uvc-metadata-v3-0-c467af869c60@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
> - Introduce 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 (5):
>       media: uvcvideo: Do not mark valid metadata as invalid
>       media: Documentation: Add note about UVCH length field
>       media: uvcvideo: Introduce dev->meta_formats
>       media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5
>       media: uvcvideo: Auto-set UVC_QUIRK_MSXU_META
> 
>  .../userspace-api/media/v4l/meta-formats.rst       |   1 +
>  .../media/v4l/metafmt-uvc-msxu-1-5.rst             |  23 +++++
>  .../userspace-api/media/v4l/metafmt-uvc.rst        |   4 +-
>  MAINTAINERS                                        |   1 +
>  drivers/media/usb/uvc/uvc_driver.c                 |   7 ++
>  drivers/media/usb/uvc/uvc_metadata.c               | 115 +++++++++++++++++++--
>  drivers/media/usb/uvc/uvc_video.c                  |  12 +--
>  drivers/media/usb/uvc/uvcvideo.h                   |   7 ++
>  drivers/media/v4l2-core/v4l2-ioctl.c               |   1 +
>  include/linux/usb/uvc.h                            |   3 +
>  include/uapi/linux/videodev2.h                     |   1 +
>  11 files changed, 161 insertions(+), 14 deletions(-)
> ---
> base-commit: a8598c7de1bcd94461ca54c972efa9b4ea501fb9
> change-id: 20250403-uvc-meta-e556773d12ae
> 
> Best regards,


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 5/5] media: uvcvideo: Auto-set UVC_QUIRK_MSXU_META
  2025-07-07 18:34 ` [PATCH v8 5/5] media: uvcvideo: Auto-set UVC_QUIRK_MSXU_META Ricardo Ribalda
@ 2025-07-11 19:58   ` Laurent Pinchart
  2025-07-14  7:46     ` Ricardo Ribalda
  0 siblings, 1 reply; 21+ messages in thread
From: Laurent Pinchart @ 2025-07-11 19:58 UTC (permalink / raw)
  To: Ricardo Ribalda
  Cc: Mauro Carvalho Chehab, Guennadi Liakhovetski, Greg Kroah-Hartman,
	Hans de Goede, linux-media, linux-kernel, linux-usb

On Mon, Jul 07, 2025 at 06:34:05PM +0000, Ricardo Ribalda wrote:
> If the camera supports the MSXU_CONTROL_METADATA control, auto set the
> MSXU_META quirk.
> 
> Reviewed-by: Hans de Goede <hansg@kernel.org>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/usb/uvc/uvc_driver.c   |  7 +++-
>  drivers/media/usb/uvc/uvc_metadata.c | 75 +++++++++++++++++++++++++++++++++++-
>  drivers/media/usb/uvc/uvcvideo.h     |  2 +-
>  include/linux/usb/uvc.h              |  3 ++
>  4 files changed, 84 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> index 56ea20eeb7b9d5d92f3d837c15bdf11d536e9f2d..9de5abb43e19d9e876cddc5d7124592953db89ac 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -2315,7 +2315,12 @@ static int uvc_probe(struct usb_interface *intf,
>  		goto error;
>  	}
>  
> -	uvc_meta_init(dev);
> +	ret = uvc_meta_init(dev);
> +	if (ret < 0) {
> +		dev_err(&dev->udev->dev,
> +			"Error initializing the metadata formats (%d)\n", ret);
> +		goto error;
> +	}
>  
>  	if (dev->quirks & UVC_QUIRK_NO_RESET_RESUME)
>  		udev->quirks &= ~USB_QUIRK_RESET_RESUME;
> diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
> index 77e03273d3cf6b00cac6ebb9b29b941f1cbfd9f7..59bb133baf9a73ef6a30fa8ead85aa90653d60f4 100644
> --- a/drivers/media/usb/uvc/uvc_metadata.c
> +++ b/drivers/media/usb/uvc/uvc_metadata.c
> @@ -10,6 +10,7 @@
>  #include <linux/list.h>
>  #include <linux/module.h>
>  #include <linux/usb.h>
> +#include <linux/usb/uvc.h>
>  #include <linux/videodev2.h>
>  
>  #include <media/v4l2-ioctl.h>
> @@ -166,6 +167,71 @@ static const struct v4l2_file_operations uvc_meta_fops = {
>  	.mmap = vb2_fop_mmap,
>  };
>  
> +static struct uvc_entity *uvc_meta_find_msxu(struct uvc_device *dev)
> +{
> +	static const u8 uvc_msxu_guid[16] = UVC_GUID_MSXU_1_5;
> +	struct uvc_entity *entity;
> +
> +	list_for_each_entry(entity, &dev->entities, list) {
> +		if (!memcmp(entity->guid, uvc_msxu_guid, sizeof(entity->guid)))
> +			return entity;
> +	}
> +
> +	return NULL;
> +}
> +
> +#define MSXU_CONTROL_METADATA 0x9
> +static int uvc_meta_detect_msxu(struct uvc_device *dev)
> +{
> +	u32 *data __free(kfree) = NULL;
> +	struct uvc_entity *entity;
> +	int ret;
> +
> +	entity = uvc_meta_find_msxu(dev);
> +	if (!entity)
> +		return 0;
> +
> +	/*
> +	 * USB requires buffers aligned in a special way, simplest way is to
> +	 * make sure that query_ctrl will work is to kmalloc() them.
> +	 */
> +	data = kmalloc(sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	/* Check if the metadata is already enabled. */
> +	ret = uvc_query_ctrl(dev, UVC_GET_CUR, entity->id, dev->intfnum,
> +			     MSXU_CONTROL_METADATA, data, sizeof(*data));
> +	if (ret)
> +		return 0;
> +
> +	if (*data) {
> +		dev->quirks |= UVC_QUIRK_MSXU_META;
> +		return 0;
> +	}
> +
> +	/*
> +	 * We have seen devices that require 1 to enable the metadata, others
> +	 * requiring a value != 1 and others requiring a value >1. Luckily for

I'm confused here. If those are three different behaviours, then value
!= 1 would be value == 0 (as the third behaviour is value > 1). You test
for !*data below, so 0 is not accepted as a valid value for this
purpose. What am I missing ?

> +	 * us, the value from GET_MAX seems to work all the time.
> +	 */
> +	ret = uvc_query_ctrl(dev, UVC_GET_MAX, entity->id, dev->intfnum,
> +			     MSXU_CONTROL_METADATA, data, sizeof(*data));
> +	if (ret || !*data)
> +		return 0;
> +
> +	/*
> +	 * If we can set MSXU_CONTROL_METADATA, the device will report
> +	 * metadata.
> +	 */
> +	ret = uvc_query_ctrl(dev, UVC_SET_CUR, entity->id, dev->intfnum,
> +			     MSXU_CONTROL_METADATA, data, sizeof(*data));
> +	if (!ret)
> +		dev->quirks |= UVC_QUIRK_MSXU_META;
> +
> +	return 0;
> +}
> +
>  int uvc_meta_register(struct uvc_streaming *stream)
>  {
>  	struct uvc_device *dev = stream->dev;
> @@ -185,9 +251,14 @@ int uvc_meta_register(struct uvc_streaming *stream)
>  					 &uvc_meta_fops, &uvc_meta_ioctl_ops);
>  }
>  
> -void uvc_meta_init(struct uvc_device *dev)
> +int uvc_meta_init(struct uvc_device *dev)
>  {
>  	unsigned int i = 0;
> +	int ret;
> +
> +	ret = uvc_meta_detect_msxu(dev);
> +	if (ret)
> +		return ret;
>  
>  	dev->meta_formats[i++] = V4L2_META_FMT_UVC;
>  
> @@ -201,4 +272,6 @@ void uvc_meta_init(struct uvc_device *dev)
>  
>  	 /* IMPORTANT: for new meta-formats update UVC_MAX_META_DATA_FORMATS. */
>  	dev->meta_formats[i++] = 0;
> +
> +	return 0;
>  }
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 616adc417c62a58686beccbc440a5dfac0a2d588..a4c064c5e046f2a4adba742c8777a10619569606 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -757,7 +757,7 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
>  void uvc_video_clock_update(struct uvc_streaming *stream,
>  			    struct vb2_v4l2_buffer *vbuf,
>  			    struct uvc_buffer *buf);
> -void uvc_meta_init(struct uvc_device *dev);
> +int uvc_meta_init(struct uvc_device *dev);
>  int uvc_meta_register(struct uvc_streaming *stream);
>  
>  int uvc_register_video_device(struct uvc_device *dev,
> diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h
> index bce95153e5a65613a710d7316fc17cf5462b5bce..ee19e9f915b8370c333c426dc1ee4202c7b75c5b 100644
> --- a/include/linux/usb/uvc.h
> +++ b/include/linux/usb/uvc.h
> @@ -29,6 +29,9 @@
>  #define UVC_GUID_EXT_GPIO_CONTROLLER \
>  	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
>  	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x03}
> +#define UVC_GUID_MSXU_1_5 \
> +	{0xdc, 0x95, 0x3f, 0x0f, 0x32, 0x26, 0x4e, 0x4c, \
> +	 0x92, 0xc9, 0xa0, 0x47, 0x82, 0xf4, 0x3b, 0xc8}
>  
>  #define UVC_GUID_FORMAT_MJPEG \
>  	{ 'M',  'J',  'P',  'G', 0x00, 0x00, 0x10, 0x00, \
> 

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 4/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5
  2025-07-07 18:34 ` [PATCH v8 4/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5 Ricardo Ribalda
@ 2025-07-11 20:13   ` Laurent Pinchart
  2025-07-14  7:44     ` Ricardo Ribalda
  2025-07-14 14:59   ` Laurent Pinchart
  1 sibling, 1 reply; 21+ messages in thread
From: Laurent Pinchart @ 2025-07-11 20:13 UTC (permalink / raw)
  To: Ricardo Ribalda
  Cc: Mauro Carvalho Chehab, Guennadi Liakhovetski, Greg Kroah-Hartman,
	Hans de Goede, linux-media, linux-kernel, linux-usb

On Mon, Jul 07, 2025 at 06:34:04PM +0000, 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 to have the whole metadata payload 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_MSXU_1_5, that is
> identical to V4L2_META_FMT_D4XX.
> 
> Let the user enable this format with a quirk for now. This way they can
> test if their devices provide useful metadata without rebuilding the
> kernel. They can later contribute patches to auto-quirk their devices.
> We will also work in methods to auto-detect devices compatible with this
> new metadata format.
> 
> Suggested-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Hans de Goede <hansg@kernel.org>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  .../userspace-api/media/v4l/meta-formats.rst       |  1 +
>  .../media/v4l/metafmt-uvc-msxu-1-5.rst             | 23 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  drivers/media/usb/uvc/uvc_metadata.c               |  4 ++++
>  drivers/media/usb/uvc/uvcvideo.h                   |  1 +
>  drivers/media/v4l2-core/v4l2-ioctl.c               |  1 +
>  include/uapi/linux/videodev2.h                     |  1 +
>  7 files changed, 32 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst
> index bb6876cfc271e1a0543eee4209d6251e1a6a73cc..0de80328c36bf148051a19abe9e5241234ddfe5c 100644
> --- a/Documentation/userspace-api/media/v4l/meta-formats.rst
> +++ b/Documentation/userspace-api/media/v4l/meta-formats.rst
> @@ -20,6 +20,7 @@ These formats are used for the :ref:`metadata` interface only.
>      metafmt-pisp-fe
>      metafmt-rkisp1
>      metafmt-uvc
> +    metafmt-uvc-msxu-1-5
>      metafmt-vivid
>      metafmt-vsp1-hgo
>      metafmt-vsp1-hgt
> diff --git a/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst b/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst
> new file mode 100644
> index 0000000000000000000000000000000000000000..dd1c3076df243d770a13e7f6d07c3296a269e16a
> --- /dev/null
> +++ b/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst
> @@ -0,0 +1,23 @@
> +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
> +
> +.. _v4l2-meta-fmt-uvc-msxu-1-5:
> +
> +***********************************
> +V4L2_META_FMT_UVC_MSXU_1_5 ('UVCM')
> +***********************************
> +
> +Microsoft(R)'s UVC Payload Metadata.
> +
> +
> +Description
> +===========
> +
> +V4L2_META_FMT_UVC_MSXU_1_5 buffers follow the metadata buffer layout of
> +V4L2_META_FMT_UVC with the only difference that it includes all the UVC
> +metadata in the `buffer[]` field, not just the first 2-12 bytes.
> +
> +The metadata format follows the specification from Microsoft(R) [1].
> +
> +.. _1:
> +
> +[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 658543062bba3b7e600699d7271ffc89250ba7e5..fdde1d37ed2ef9058e3ea3417bec25afe454dfc0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -25827,6 +25827,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-msxu-1-5.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 4bcbc22f47e67c52baf6e133f240131ff3d32a03..77e03273d3cf6b00cac6ebb9b29b941f1cbfd9f7 100644
> --- a/drivers/media/usb/uvc/uvc_metadata.c
> +++ b/drivers/media/usb/uvc/uvc_metadata.c
> @@ -195,6 +195,10 @@ void uvc_meta_init(struct uvc_device *dev)
>  	    !WARN_ON(dev->info->meta_format == V4L2_META_FMT_UVC))
>  		dev->meta_formats[i++] = dev->info->meta_format;
>  
> +	if (dev->quirks & UVC_QUIRK_MSXU_META &&
> +	    !WARN_ON(dev->info->meta_format == V4L2_META_FMT_UVC_MSXU_1_5))
> +		dev->meta_formats[i++] = V4L2_META_FMT_UVC_MSXU_1_5;

Just to clarify, does this mean that your goal is to set
dev->info->meta_format to V4L2_META_FMT_UVC_MSXU_1_5 as devices are
reported to support the format ?

> +
>  	 /* IMPORTANT: for new meta-formats update UVC_MAX_META_DATA_FORMATS. */
>  	dev->meta_formats[i++] = 0;
>  }
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index b3c094c6591e7a71fc00e1096bcf493a83f330ad..616adc417c62a58686beccbc440a5dfac0a2d588 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -77,6 +77,7 @@
>  #define UVC_QUIRK_DISABLE_AUTOSUSPEND	0x00008000
>  #define UVC_QUIRK_INVALID_DEVICE_SOF	0x00010000
>  #define UVC_QUIRK_MJPEG_NO_EOF		0x00020000
> +#define UVC_QUIRK_MSXU_META		0x00040000
>  
>  /* Format flags */
>  #define UVC_FMT_FLAG_COMPRESSED		0x00000001
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index be94a79b976e3de4eb957f5d2584ec6d4230469e..993b36417b4655456ce545cb42a41b55b98e4d6c 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1463,6 +1463,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_MSXU_1_5:	descr = "UVC MSXU 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 6f7bd38dd5aa4b1b2084685512512a380d76a5e4..863bc5b7dec32303e852d7e9c3891011ce5a3d71 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -867,6 +867,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_MSXU_1_5  v4l2_fourcc('U', 'V', 'C', 'M') /* UVC MSXU metadata */
>  #define V4L2_META_FMT_VIVID	  v4l2_fourcc('V', 'I', 'V', 'D') /* Vivid Metadata */
>  
>  /* Vendor specific - used for RK_ISP1 camera sub-system */

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 4/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5
  2025-07-11 20:13   ` Laurent Pinchart
@ 2025-07-14  7:44     ` Ricardo Ribalda
  0 siblings, 0 replies; 21+ messages in thread
From: Ricardo Ribalda @ 2025-07-14  7:44 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Mauro Carvalho Chehab, Guennadi Liakhovetski, Greg Kroah-Hartman,
	Hans de Goede, linux-media, linux-kernel, linux-usb

Hi Laurent

On Fri, 11 Jul 2025 at 22:13, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Mon, Jul 07, 2025 at 06:34:04PM +0000, 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 to have the whole metadata payload 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_MSXU_1_5, that is
> > identical to V4L2_META_FMT_D4XX.
> >
> > Let the user enable this format with a quirk for now. This way they can
> > test if their devices provide useful metadata without rebuilding the
> > kernel. They can later contribute patches to auto-quirk their devices.
> > We will also work in methods to auto-detect devices compatible with this
> > new metadata format.
> >
> > Suggested-by: Hans de Goede <hdegoede@redhat.com>
> > Reviewed-by: Hans de Goede <hansg@kernel.org>
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> >  .../userspace-api/media/v4l/meta-formats.rst       |  1 +
> >  .../media/v4l/metafmt-uvc-msxu-1-5.rst             | 23 ++++++++++++++++++++++
> >  MAINTAINERS                                        |  1 +
> >  drivers/media/usb/uvc/uvc_metadata.c               |  4 ++++
> >  drivers/media/usb/uvc/uvcvideo.h                   |  1 +
> >  drivers/media/v4l2-core/v4l2-ioctl.c               |  1 +
> >  include/uapi/linux/videodev2.h                     |  1 +
> >  7 files changed, 32 insertions(+)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > index bb6876cfc271e1a0543eee4209d6251e1a6a73cc..0de80328c36bf148051a19abe9e5241234ddfe5c 100644
> > --- a/Documentation/userspace-api/media/v4l/meta-formats.rst
> > +++ b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > @@ -20,6 +20,7 @@ These formats are used for the :ref:`metadata` interface only.
> >      metafmt-pisp-fe
> >      metafmt-rkisp1
> >      metafmt-uvc
> > +    metafmt-uvc-msxu-1-5
> >      metafmt-vivid
> >      metafmt-vsp1-hgo
> >      metafmt-vsp1-hgt
> > diff --git a/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst b/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..dd1c3076df243d770a13e7f6d07c3296a269e16a
> > --- /dev/null
> > +++ b/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst
> > @@ -0,0 +1,23 @@
> > +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
> > +
> > +.. _v4l2-meta-fmt-uvc-msxu-1-5:
> > +
> > +***********************************
> > +V4L2_META_FMT_UVC_MSXU_1_5 ('UVCM')
> > +***********************************
> > +
> > +Microsoft(R)'s UVC Payload Metadata.
> > +
> > +
> > +Description
> > +===========
> > +
> > +V4L2_META_FMT_UVC_MSXU_1_5 buffers follow the metadata buffer layout of
> > +V4L2_META_FMT_UVC with the only difference that it includes all the UVC
> > +metadata in the `buffer[]` field, not just the first 2-12 bytes.
> > +
> > +The metadata format follows the specification from Microsoft(R) [1].
> > +
> > +.. _1:
> > +
> > +[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 658543062bba3b7e600699d7271ffc89250ba7e5..fdde1d37ed2ef9058e3ea3417bec25afe454dfc0 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -25827,6 +25827,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-msxu-1-5.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 4bcbc22f47e67c52baf6e133f240131ff3d32a03..77e03273d3cf6b00cac6ebb9b29b941f1cbfd9f7 100644
> > --- a/drivers/media/usb/uvc/uvc_metadata.c
> > +++ b/drivers/media/usb/uvc/uvc_metadata.c
> > @@ -195,6 +195,10 @@ void uvc_meta_init(struct uvc_device *dev)
> >           !WARN_ON(dev->info->meta_format == V4L2_META_FMT_UVC))
> >               dev->meta_formats[i++] = dev->info->meta_format;
> >
> > +     if (dev->quirks & UVC_QUIRK_MSXU_META &&
> > +         !WARN_ON(dev->info->meta_format == V4L2_META_FMT_UVC_MSXU_1_5))
> > +             dev->meta_formats[i++] = V4L2_META_FMT_UVC_MSXU_1_5;
>
> Just to clarify, does this mean that your goal is to set
> dev->info->meta_format to V4L2_META_FMT_UVC_MSXU_1_5 as devices are
> reported to support the format ?

That is correct. Compatible devices shall either be autodetected via
the special control or manually added to the quirk list.

Quirking via module parameter should be just used for development purposes.


>
> > +
> >        /* IMPORTANT: for new meta-formats update UVC_MAX_META_DATA_FORMATS. */
> >       dev->meta_formats[i++] = 0;
> >  }
> > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > index b3c094c6591e7a71fc00e1096bcf493a83f330ad..616adc417c62a58686beccbc440a5dfac0a2d588 100644
> > --- a/drivers/media/usb/uvc/uvcvideo.h
> > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > @@ -77,6 +77,7 @@
> >  #define UVC_QUIRK_DISABLE_AUTOSUSPEND        0x00008000
> >  #define UVC_QUIRK_INVALID_DEVICE_SOF 0x00010000
> >  #define UVC_QUIRK_MJPEG_NO_EOF               0x00020000
> > +#define UVC_QUIRK_MSXU_META          0x00040000
> >
> >  /* Format flags */
> >  #define UVC_FMT_FLAG_COMPRESSED              0x00000001
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> > index be94a79b976e3de4eb957f5d2584ec6d4230469e..993b36417b4655456ce545cb42a41b55b98e4d6c 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -1463,6 +1463,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_MSXU_1_5:        descr = "UVC MSXU 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 6f7bd38dd5aa4b1b2084685512512a380d76a5e4..863bc5b7dec32303e852d7e9c3891011ce5a3d71 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -867,6 +867,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_MSXU_1_5  v4l2_fourcc('U', 'V', 'C', 'M') /* UVC MSXU metadata */
> >  #define V4L2_META_FMT_VIVID    v4l2_fourcc('V', 'I', 'V', 'D') /* Vivid Metadata */
> >
> >  /* Vendor specific - used for RK_ISP1 camera sub-system */
>
> --
> Regards,
>
> Laurent Pinchart



-- 
Ricardo Ribalda

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 5/5] media: uvcvideo: Auto-set UVC_QUIRK_MSXU_META
  2025-07-11 19:58   ` Laurent Pinchart
@ 2025-07-14  7:46     ` Ricardo Ribalda
  2025-07-14  8:06       ` Laurent Pinchart
  0 siblings, 1 reply; 21+ messages in thread
From: Ricardo Ribalda @ 2025-07-14  7:46 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Mauro Carvalho Chehab, Guennadi Liakhovetski, Greg Kroah-Hartman,
	Hans de Goede, linux-media, linux-kernel, linux-usb

Hi Laurent

On Fri, 11 Jul 2025 at 21:58, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Mon, Jul 07, 2025 at 06:34:05PM +0000, Ricardo Ribalda wrote:
> > If the camera supports the MSXU_CONTROL_METADATA control, auto set the
> > MSXU_META quirk.
> >
> > Reviewed-by: Hans de Goede <hansg@kernel.org>
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> >  drivers/media/usb/uvc/uvc_driver.c   |  7 +++-
> >  drivers/media/usb/uvc/uvc_metadata.c | 75 +++++++++++++++++++++++++++++++++++-
> >  drivers/media/usb/uvc/uvcvideo.h     |  2 +-
> >  include/linux/usb/uvc.h              |  3 ++
> >  4 files changed, 84 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > index 56ea20eeb7b9d5d92f3d837c15bdf11d536e9f2d..9de5abb43e19d9e876cddc5d7124592953db89ac 100644
> > --- a/drivers/media/usb/uvc/uvc_driver.c
> > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > @@ -2315,7 +2315,12 @@ static int uvc_probe(struct usb_interface *intf,
> >               goto error;
> >       }
> >
> > -     uvc_meta_init(dev);
> > +     ret = uvc_meta_init(dev);
> > +     if (ret < 0) {
> > +             dev_err(&dev->udev->dev,
> > +                     "Error initializing the metadata formats (%d)\n", ret);
> > +             goto error;
> > +     }
> >
> >       if (dev->quirks & UVC_QUIRK_NO_RESET_RESUME)
> >               udev->quirks &= ~USB_QUIRK_RESET_RESUME;
> > diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
> > index 77e03273d3cf6b00cac6ebb9b29b941f1cbfd9f7..59bb133baf9a73ef6a30fa8ead85aa90653d60f4 100644
> > --- a/drivers/media/usb/uvc/uvc_metadata.c
> > +++ b/drivers/media/usb/uvc/uvc_metadata.c
> > @@ -10,6 +10,7 @@
> >  #include <linux/list.h>
> >  #include <linux/module.h>
> >  #include <linux/usb.h>
> > +#include <linux/usb/uvc.h>
> >  #include <linux/videodev2.h>
> >
> >  #include <media/v4l2-ioctl.h>
> > @@ -166,6 +167,71 @@ static const struct v4l2_file_operations uvc_meta_fops = {
> >       .mmap = vb2_fop_mmap,
> >  };
> >
> > +static struct uvc_entity *uvc_meta_find_msxu(struct uvc_device *dev)
> > +{
> > +     static const u8 uvc_msxu_guid[16] = UVC_GUID_MSXU_1_5;
> > +     struct uvc_entity *entity;
> > +
> > +     list_for_each_entry(entity, &dev->entities, list) {
> > +             if (!memcmp(entity->guid, uvc_msxu_guid, sizeof(entity->guid)))
> > +                     return entity;
> > +     }
> > +
> > +     return NULL;
> > +}
> > +
> > +#define MSXU_CONTROL_METADATA 0x9
> > +static int uvc_meta_detect_msxu(struct uvc_device *dev)
> > +{
> > +     u32 *data __free(kfree) = NULL;
> > +     struct uvc_entity *entity;
> > +     int ret;
> > +
> > +     entity = uvc_meta_find_msxu(dev);
> > +     if (!entity)
> > +             return 0;
> > +
> > +     /*
> > +      * USB requires buffers aligned in a special way, simplest way is to
> > +      * make sure that query_ctrl will work is to kmalloc() them.
> > +      */
> > +     data = kmalloc(sizeof(*data), GFP_KERNEL);
> > +     if (!data)
> > +             return -ENOMEM;
> > +
> > +     /* Check if the metadata is already enabled. */
> > +     ret = uvc_query_ctrl(dev, UVC_GET_CUR, entity->id, dev->intfnum,
> > +                          MSXU_CONTROL_METADATA, data, sizeof(*data));
> > +     if (ret)
> > +             return 0;
> > +
> > +     if (*data) {
> > +             dev->quirks |= UVC_QUIRK_MSXU_META;
> > +             return 0;
> > +     }
> > +
> > +     /*
> > +      * We have seen devices that require 1 to enable the metadata, others
> > +      * requiring a value != 1 and others requiring a value >1. Luckily for
>
> I'm confused here. If those are three different behaviours, then value
> != 1 would be value == 0 (as the third behaviour is value > 1). You test
> for !*data below, so 0 is not accepted as a valid value for this
> purpose. What am I missing ?

There is a typo in the comment.

Some devices require 1, some devices any value !=0, and other value=MAX.
I will fix it in a follow-up patch.


>
> > +      * us, the value from GET_MAX seems to work all the time.
> > +      */
> > +     ret = uvc_query_ctrl(dev, UVC_GET_MAX, entity->id, dev->intfnum,
> > +                          MSXU_CONTROL_METADATA, data, sizeof(*data));
> > +     if (ret || !*data)
> > +             return 0;
> > +
> > +     /*
> > +      * If we can set MSXU_CONTROL_METADATA, the device will report
> > +      * metadata.
> > +      */
> > +     ret = uvc_query_ctrl(dev, UVC_SET_CUR, entity->id, dev->intfnum,
> > +                          MSXU_CONTROL_METADATA, data, sizeof(*data));
> > +     if (!ret)
> > +             dev->quirks |= UVC_QUIRK_MSXU_META;
> > +
> > +     return 0;
> > +}
> > +
> >  int uvc_meta_register(struct uvc_streaming *stream)
> >  {
> >       struct uvc_device *dev = stream->dev;
> > @@ -185,9 +251,14 @@ int uvc_meta_register(struct uvc_streaming *stream)
> >                                        &uvc_meta_fops, &uvc_meta_ioctl_ops);
> >  }
> >
> > -void uvc_meta_init(struct uvc_device *dev)
> > +int uvc_meta_init(struct uvc_device *dev)
> >  {
> >       unsigned int i = 0;
> > +     int ret;
> > +
> > +     ret = uvc_meta_detect_msxu(dev);
> > +     if (ret)
> > +             return ret;
> >
> >       dev->meta_formats[i++] = V4L2_META_FMT_UVC;
> >
> > @@ -201,4 +272,6 @@ void uvc_meta_init(struct uvc_device *dev)
> >
> >        /* IMPORTANT: for new meta-formats update UVC_MAX_META_DATA_FORMATS. */
> >       dev->meta_formats[i++] = 0;
> > +
> > +     return 0;
> >  }
> > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > index 616adc417c62a58686beccbc440a5dfac0a2d588..a4c064c5e046f2a4adba742c8777a10619569606 100644
> > --- a/drivers/media/usb/uvc/uvcvideo.h
> > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > @@ -757,7 +757,7 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
> >  void uvc_video_clock_update(struct uvc_streaming *stream,
> >                           struct vb2_v4l2_buffer *vbuf,
> >                           struct uvc_buffer *buf);
> > -void uvc_meta_init(struct uvc_device *dev);
> > +int uvc_meta_init(struct uvc_device *dev);
> >  int uvc_meta_register(struct uvc_streaming *stream);
> >
> >  int uvc_register_video_device(struct uvc_device *dev,
> > diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h
> > index bce95153e5a65613a710d7316fc17cf5462b5bce..ee19e9f915b8370c333c426dc1ee4202c7b75c5b 100644
> > --- a/include/linux/usb/uvc.h
> > +++ b/include/linux/usb/uvc.h
> > @@ -29,6 +29,9 @@
> >  #define UVC_GUID_EXT_GPIO_CONTROLLER \
> >       {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> >        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x03}
> > +#define UVC_GUID_MSXU_1_5 \
> > +     {0xdc, 0x95, 0x3f, 0x0f, 0x32, 0x26, 0x4e, 0x4c, \
> > +      0x92, 0xc9, 0xa0, 0x47, 0x82, 0xf4, 0x3b, 0xc8}
> >
> >  #define UVC_GUID_FORMAT_MJPEG \
> >       { 'M',  'J',  'P',  'G', 0x00, 0x00, 0x10, 0x00, \
> >
>
> --
> Regards,
>
> Laurent Pinchart



-- 
Ricardo Ribalda

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 5/5] media: uvcvideo: Auto-set UVC_QUIRK_MSXU_META
  2025-07-14  7:46     ` Ricardo Ribalda
@ 2025-07-14  8:06       ` Laurent Pinchart
  2025-07-14  8:21         ` Ricardo Ribalda
  0 siblings, 1 reply; 21+ messages in thread
From: Laurent Pinchart @ 2025-07-14  8:06 UTC (permalink / raw)
  To: Ricardo Ribalda
  Cc: Mauro Carvalho Chehab, Guennadi Liakhovetski, Greg Kroah-Hartman,
	Hans de Goede, linux-media, linux-kernel, linux-usb

On Mon, Jul 14, 2025 at 09:46:45AM +0200, Ricardo Ribalda wrote:
> On Fri, 11 Jul 2025 at 21:58, Laurent Pinchart wrote:
> > On Mon, Jul 07, 2025 at 06:34:05PM +0000, Ricardo Ribalda wrote:
> > > If the camera supports the MSXU_CONTROL_METADATA control, auto set the
> > > MSXU_META quirk.
> > >
> > > Reviewed-by: Hans de Goede <hansg@kernel.org>
> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > ---
> > >  drivers/media/usb/uvc/uvc_driver.c   |  7 +++-
> > >  drivers/media/usb/uvc/uvc_metadata.c | 75 +++++++++++++++++++++++++++++++++++-
> > >  drivers/media/usb/uvc/uvcvideo.h     |  2 +-
> > >  include/linux/usb/uvc.h              |  3 ++
> > >  4 files changed, 84 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > > index 56ea20eeb7b9d5d92f3d837c15bdf11d536e9f2d..9de5abb43e19d9e876cddc5d7124592953db89ac 100644
> > > --- a/drivers/media/usb/uvc/uvc_driver.c
> > > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > > @@ -2315,7 +2315,12 @@ static int uvc_probe(struct usb_interface *intf,
> > >               goto error;
> > >       }
> > >
> > > -     uvc_meta_init(dev);
> > > +     ret = uvc_meta_init(dev);
> > > +     if (ret < 0) {
> > > +             dev_err(&dev->udev->dev,
> > > +                     "Error initializing the metadata formats (%d)\n", ret);
> > > +             goto error;
> > > +     }
> > >
> > >       if (dev->quirks & UVC_QUIRK_NO_RESET_RESUME)
> > >               udev->quirks &= ~USB_QUIRK_RESET_RESUME;
> > > diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
> > > index 77e03273d3cf6b00cac6ebb9b29b941f1cbfd9f7..59bb133baf9a73ef6a30fa8ead85aa90653d60f4 100644
> > > --- a/drivers/media/usb/uvc/uvc_metadata.c
> > > +++ b/drivers/media/usb/uvc/uvc_metadata.c
> > > @@ -10,6 +10,7 @@
> > >  #include <linux/list.h>
> > >  #include <linux/module.h>
> > >  #include <linux/usb.h>
> > > +#include <linux/usb/uvc.h>
> > >  #include <linux/videodev2.h>
> > >
> > >  #include <media/v4l2-ioctl.h>
> > > @@ -166,6 +167,71 @@ static const struct v4l2_file_operations uvc_meta_fops = {
> > >       .mmap = vb2_fop_mmap,
> > >  };
> > >
> > > +static struct uvc_entity *uvc_meta_find_msxu(struct uvc_device *dev)
> > > +{
> > > +     static const u8 uvc_msxu_guid[16] = UVC_GUID_MSXU_1_5;
> > > +     struct uvc_entity *entity;
> > > +
> > > +     list_for_each_entry(entity, &dev->entities, list) {
> > > +             if (!memcmp(entity->guid, uvc_msxu_guid, sizeof(entity->guid)))
> > > +                     return entity;
> > > +     }
> > > +
> > > +     return NULL;
> > > +}
> > > +
> > > +#define MSXU_CONTROL_METADATA 0x9
> > > +static int uvc_meta_detect_msxu(struct uvc_device *dev)
> > > +{
> > > +     u32 *data __free(kfree) = NULL;
> > > +     struct uvc_entity *entity;
> > > +     int ret;
> > > +
> > > +     entity = uvc_meta_find_msxu(dev);
> > > +     if (!entity)
> > > +             return 0;
> > > +
> > > +     /*
> > > +      * USB requires buffers aligned in a special way, simplest way is to
> > > +      * make sure that query_ctrl will work is to kmalloc() them.
> > > +      */
> > > +     data = kmalloc(sizeof(*data), GFP_KERNEL);
> > > +     if (!data)
> > > +             return -ENOMEM;
> > > +
> > > +     /* Check if the metadata is already enabled. */
> > > +     ret = uvc_query_ctrl(dev, UVC_GET_CUR, entity->id, dev->intfnum,
> > > +                          MSXU_CONTROL_METADATA, data, sizeof(*data));
> > > +     if (ret)
> > > +             return 0;
> > > +
> > > +     if (*data) {
> > > +             dev->quirks |= UVC_QUIRK_MSXU_META;
> > > +             return 0;
> > > +     }
> > > +
> > > +     /*
> > > +      * We have seen devices that require 1 to enable the metadata, others
> > > +      * requiring a value != 1 and others requiring a value >1. Luckily for
> >
> > I'm confused here. If those are three different behaviours, then value
> > != 1 would be value == 0 (as the third behaviour is value > 1). You test
> > for !*data below, so 0 is not accepted as a valid value for this
> > purpose. What am I missing ?
> 
> There is a typo in the comment.
> 
> Some devices require 1, some devices any value !=0, and other value=MAX.
> I will fix it in a follow-up patch.

The documentation of the control states that MSXU_CONTROL_METADATA
reports the maximum size of the MS metadata generated by the device in
kB, and the control should be set to the value reported by GET_MAX to
enable metadata generation. That's what you're doing in this patch, so
you can update the comment to document there.

Some devices also don't support SET_CUR, in which case they should
report GET_MIN == GET_DEF == GET_MAX. I assume GET_CUR should then also
report the same value. Please also update the previous comment to
explain this, the GET_CUR value check above is more about handling
devices that always produce metadata than devices for which a driver has
enabled metadata production.

This leads to another question: should we enable metadata generation
only when the metadata capture device is streaming ?

> > > +      * us, the value from GET_MAX seems to work all the time.
> > > +      */
> > > +     ret = uvc_query_ctrl(dev, UVC_GET_MAX, entity->id, dev->intfnum,
> > > +                          MSXU_CONTROL_METADATA, data, sizeof(*data));
> > > +     if (ret || !*data)
> > > +             return 0;
> > > +
> > > +     /*
> > > +      * If we can set MSXU_CONTROL_METADATA, the device will report
> > > +      * metadata.
> > > +      */
> > > +     ret = uvc_query_ctrl(dev, UVC_SET_CUR, entity->id, dev->intfnum,
> > > +                          MSXU_CONTROL_METADATA, data, sizeof(*data));
> > > +     if (!ret)
> > > +             dev->quirks |= UVC_QUIRK_MSXU_META;
> > > +
> > > +     return 0;
> > > +}
> > > +
> > >  int uvc_meta_register(struct uvc_streaming *stream)
> > >  {
> > >       struct uvc_device *dev = stream->dev;
> > > @@ -185,9 +251,14 @@ int uvc_meta_register(struct uvc_streaming *stream)
> > >                                        &uvc_meta_fops, &uvc_meta_ioctl_ops);
> > >  }
> > >
> > > -void uvc_meta_init(struct uvc_device *dev)
> > > +int uvc_meta_init(struct uvc_device *dev)
> > >  {
> > >       unsigned int i = 0;
> > > +     int ret;
> > > +
> > > +     ret = uvc_meta_detect_msxu(dev);
> > > +     if (ret)
> > > +             return ret;
> > >
> > >       dev->meta_formats[i++] = V4L2_META_FMT_UVC;
> > >
> > > @@ -201,4 +272,6 @@ void uvc_meta_init(struct uvc_device *dev)
> > >
> > >        /* IMPORTANT: for new meta-formats update UVC_MAX_META_DATA_FORMATS. */
> > >       dev->meta_formats[i++] = 0;
> > > +
> > > +     return 0;
> > >  }
> > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > > index 616adc417c62a58686beccbc440a5dfac0a2d588..a4c064c5e046f2a4adba742c8777a10619569606 100644
> > > --- a/drivers/media/usb/uvc/uvcvideo.h
> > > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > > @@ -757,7 +757,7 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
> > >  void uvc_video_clock_update(struct uvc_streaming *stream,
> > >                           struct vb2_v4l2_buffer *vbuf,
> > >                           struct uvc_buffer *buf);
> > > -void uvc_meta_init(struct uvc_device *dev);
> > > +int uvc_meta_init(struct uvc_device *dev);
> > >  int uvc_meta_register(struct uvc_streaming *stream);
> > >
> > >  int uvc_register_video_device(struct uvc_device *dev,
> > > diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h
> > > index bce95153e5a65613a710d7316fc17cf5462b5bce..ee19e9f915b8370c333c426dc1ee4202c7b75c5b 100644
> > > --- a/include/linux/usb/uvc.h
> > > +++ b/include/linux/usb/uvc.h
> > > @@ -29,6 +29,9 @@
> > >  #define UVC_GUID_EXT_GPIO_CONTROLLER \
> > >       {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> > >        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x03}
> > > +#define UVC_GUID_MSXU_1_5 \
> > > +     {0xdc, 0x95, 0x3f, 0x0f, 0x32, 0x26, 0x4e, 0x4c, \
> > > +      0x92, 0xc9, 0xa0, 0x47, 0x82, 0xf4, 0x3b, 0xc8}
> > >
> > >  #define UVC_GUID_FORMAT_MJPEG \
> > >       { 'M',  'J',  'P',  'G', 0x00, 0x00, 0x10, 0x00, \
> > >

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 5/5] media: uvcvideo: Auto-set UVC_QUIRK_MSXU_META
  2025-07-14  8:06       ` Laurent Pinchart
@ 2025-07-14  8:21         ` Ricardo Ribalda
  2025-07-14  8:34           ` Laurent Pinchart
  0 siblings, 1 reply; 21+ messages in thread
From: Ricardo Ribalda @ 2025-07-14  8:21 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Mauro Carvalho Chehab, Guennadi Liakhovetski, Greg Kroah-Hartman,
	Hans de Goede, linux-media, linux-kernel, linux-usb

On Mon, 14 Jul 2025 at 10:06, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Mon, Jul 14, 2025 at 09:46:45AM +0200, Ricardo Ribalda wrote:
> > On Fri, 11 Jul 2025 at 21:58, Laurent Pinchart wrote:
> > > On Mon, Jul 07, 2025 at 06:34:05PM +0000, Ricardo Ribalda wrote:
> > > > If the camera supports the MSXU_CONTROL_METADATA control, auto set the
> > > > MSXU_META quirk.
> > > >
> > > > Reviewed-by: Hans de Goede <hansg@kernel.org>
> > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > ---
> > > >  drivers/media/usb/uvc/uvc_driver.c   |  7 +++-
> > > >  drivers/media/usb/uvc/uvc_metadata.c | 75 +++++++++++++++++++++++++++++++++++-
> > > >  drivers/media/usb/uvc/uvcvideo.h     |  2 +-
> > > >  include/linux/usb/uvc.h              |  3 ++
> > > >  4 files changed, 84 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > > > index 56ea20eeb7b9d5d92f3d837c15bdf11d536e9f2d..9de5abb43e19d9e876cddc5d7124592953db89ac 100644
> > > > --- a/drivers/media/usb/uvc/uvc_driver.c
> > > > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > > > @@ -2315,7 +2315,12 @@ static int uvc_probe(struct usb_interface *intf,
> > > >               goto error;
> > > >       }
> > > >
> > > > -     uvc_meta_init(dev);
> > > > +     ret = uvc_meta_init(dev);
> > > > +     if (ret < 0) {
> > > > +             dev_err(&dev->udev->dev,
> > > > +                     "Error initializing the metadata formats (%d)\n", ret);
> > > > +             goto error;
> > > > +     }
> > > >
> > > >       if (dev->quirks & UVC_QUIRK_NO_RESET_RESUME)
> > > >               udev->quirks &= ~USB_QUIRK_RESET_RESUME;
> > > > diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
> > > > index 77e03273d3cf6b00cac6ebb9b29b941f1cbfd9f7..59bb133baf9a73ef6a30fa8ead85aa90653d60f4 100644
> > > > --- a/drivers/media/usb/uvc/uvc_metadata.c
> > > > +++ b/drivers/media/usb/uvc/uvc_metadata.c
> > > > @@ -10,6 +10,7 @@
> > > >  #include <linux/list.h>
> > > >  #include <linux/module.h>
> > > >  #include <linux/usb.h>
> > > > +#include <linux/usb/uvc.h>
> > > >  #include <linux/videodev2.h>
> > > >
> > > >  #include <media/v4l2-ioctl.h>
> > > > @@ -166,6 +167,71 @@ static const struct v4l2_file_operations uvc_meta_fops = {
> > > >       .mmap = vb2_fop_mmap,
> > > >  };
> > > >
> > > > +static struct uvc_entity *uvc_meta_find_msxu(struct uvc_device *dev)
> > > > +{
> > > > +     static const u8 uvc_msxu_guid[16] = UVC_GUID_MSXU_1_5;
> > > > +     struct uvc_entity *entity;
> > > > +
> > > > +     list_for_each_entry(entity, &dev->entities, list) {
> > > > +             if (!memcmp(entity->guid, uvc_msxu_guid, sizeof(entity->guid)))
> > > > +                     return entity;
> > > > +     }
> > > > +
> > > > +     return NULL;
> > > > +}
> > > > +
> > > > +#define MSXU_CONTROL_METADATA 0x9
> > > > +static int uvc_meta_detect_msxu(struct uvc_device *dev)
> > > > +{
> > > > +     u32 *data __free(kfree) = NULL;
> > > > +     struct uvc_entity *entity;
> > > > +     int ret;
> > > > +
> > > > +     entity = uvc_meta_find_msxu(dev);
> > > > +     if (!entity)
> > > > +             return 0;
> > > > +
> > > > +     /*
> > > > +      * USB requires buffers aligned in a special way, simplest way is to
> > > > +      * make sure that query_ctrl will work is to kmalloc() them.
> > > > +      */
> > > > +     data = kmalloc(sizeof(*data), GFP_KERNEL);
> > > > +     if (!data)
> > > > +             return -ENOMEM;
> > > > +
> > > > +     /* Check if the metadata is already enabled. */
> > > > +     ret = uvc_query_ctrl(dev, UVC_GET_CUR, entity->id, dev->intfnum,
> > > > +                          MSXU_CONTROL_METADATA, data, sizeof(*data));
> > > > +     if (ret)
> > > > +             return 0;
> > > > +
> > > > +     if (*data) {
> > > > +             dev->quirks |= UVC_QUIRK_MSXU_META;
> > > > +             return 0;
> > > > +     }
> > > > +
> > > > +     /*
> > > > +      * We have seen devices that require 1 to enable the metadata, others
> > > > +      * requiring a value != 1 and others requiring a value >1. Luckily for
> > >
> > > I'm confused here. If those are three different behaviours, then value
> > > != 1 would be value == 0 (as the third behaviour is value > 1). You test
> > > for !*data below, so 0 is not accepted as a valid value for this
> > > purpose. What am I missing ?
> >
> > There is a typo in the comment.
> >
> > Some devices require 1, some devices any value !=0, and other value=MAX.
> > I will fix it in a follow-up patch.
>
> The documentation of the control states that MSXU_CONTROL_METADATA
> reports the maximum size of the MS metadata generated by the device in
> kB, and the control should be set to the value reported by GET_MAX to
> enable metadata generation. That's what you're doing in this patch, so
> you can update the comment to document there.

Some vendors assumed the kB was a typo in MS spec and used bytes instead :)
In this favor I must say that kB here sounds like a mistake, I haven't
seen more than 300 bytes of metadata.

Will send a patch with the updated documentation.


>
> Some devices also don't support SET_CUR, in which case they should
> report GET_MIN == GET_DEF == GET_MAX. I assume GET_CUR should then also
> report the same value. Please also update the previous comment to
> explain this, the GET_CUR value check above is more about handling
> devices that always produce metadata than devices for which a driver has
> enabled metadata production.
>
> This leads to another question: should we enable metadata generation
> only when the metadata capture device is streaming ?

This will lead to a lot of issues with very little benefit.

We have no control over the order that the user will enable streamon
in the metadata and "normal" video devices. And the device will not
like that MSXU_CONTROL_METADATA changes during streamon.

I have not seen any performance penalty by the 300 new bytes
transferred when the metadata is on.


>
> > > > +      * us, the value from GET_MAX seems to work all the time.
> > > > +      */
> > > > +     ret = uvc_query_ctrl(dev, UVC_GET_MAX, entity->id, dev->intfnum,
> > > > +                          MSXU_CONTROL_METADATA, data, sizeof(*data));
> > > > +     if (ret || !*data)
> > > > +             return 0;
> > > > +
> > > > +     /*
> > > > +      * If we can set MSXU_CONTROL_METADATA, the device will report
> > > > +      * metadata.
> > > > +      */
> > > > +     ret = uvc_query_ctrl(dev, UVC_SET_CUR, entity->id, dev->intfnum,
> > > > +                          MSXU_CONTROL_METADATA, data, sizeof(*data));
> > > > +     if (!ret)
> > > > +             dev->quirks |= UVC_QUIRK_MSXU_META;
> > > > +
> > > > +     return 0;
> > > > +}
> > > > +
> > > >  int uvc_meta_register(struct uvc_streaming *stream)
> > > >  {
> > > >       struct uvc_device *dev = stream->dev;
> > > > @@ -185,9 +251,14 @@ int uvc_meta_register(struct uvc_streaming *stream)
> > > >                                        &uvc_meta_fops, &uvc_meta_ioctl_ops);
> > > >  }
> > > >
> > > > -void uvc_meta_init(struct uvc_device *dev)
> > > > +int uvc_meta_init(struct uvc_device *dev)
> > > >  {
> > > >       unsigned int i = 0;
> > > > +     int ret;
> > > > +
> > > > +     ret = uvc_meta_detect_msxu(dev);
> > > > +     if (ret)
> > > > +             return ret;
> > > >
> > > >       dev->meta_formats[i++] = V4L2_META_FMT_UVC;
> > > >
> > > > @@ -201,4 +272,6 @@ void uvc_meta_init(struct uvc_device *dev)
> > > >
> > > >        /* IMPORTANT: for new meta-formats update UVC_MAX_META_DATA_FORMATS. */
> > > >       dev->meta_formats[i++] = 0;
> > > > +
> > > > +     return 0;
> > > >  }
> > > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > > > index 616adc417c62a58686beccbc440a5dfac0a2d588..a4c064c5e046f2a4adba742c8777a10619569606 100644
> > > > --- a/drivers/media/usb/uvc/uvcvideo.h
> > > > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > > > @@ -757,7 +757,7 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
> > > >  void uvc_video_clock_update(struct uvc_streaming *stream,
> > > >                           struct vb2_v4l2_buffer *vbuf,
> > > >                           struct uvc_buffer *buf);
> > > > -void uvc_meta_init(struct uvc_device *dev);
> > > > +int uvc_meta_init(struct uvc_device *dev);
> > > >  int uvc_meta_register(struct uvc_streaming *stream);
> > > >
> > > >  int uvc_register_video_device(struct uvc_device *dev,
> > > > diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h
> > > > index bce95153e5a65613a710d7316fc17cf5462b5bce..ee19e9f915b8370c333c426dc1ee4202c7b75c5b 100644
> > > > --- a/include/linux/usb/uvc.h
> > > > +++ b/include/linux/usb/uvc.h
> > > > @@ -29,6 +29,9 @@
> > > >  #define UVC_GUID_EXT_GPIO_CONTROLLER \
> > > >       {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> > > >        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x03}
> > > > +#define UVC_GUID_MSXU_1_5 \
> > > > +     {0xdc, 0x95, 0x3f, 0x0f, 0x32, 0x26, 0x4e, 0x4c, \
> > > > +      0x92, 0xc9, 0xa0, 0x47, 0x82, 0xf4, 0x3b, 0xc8}
> > > >
> > > >  #define UVC_GUID_FORMAT_MJPEG \
> > > >       { 'M',  'J',  'P',  'G', 0x00, 0x00, 0x10, 0x00, \
> > > >
>
> --
> Regards,
>
> Laurent Pinchart



-- 
Ricardo Ribalda

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 5/5] media: uvcvideo: Auto-set UVC_QUIRK_MSXU_META
  2025-07-14  8:21         ` Ricardo Ribalda
@ 2025-07-14  8:34           ` Laurent Pinchart
  0 siblings, 0 replies; 21+ messages in thread
From: Laurent Pinchart @ 2025-07-14  8:34 UTC (permalink / raw)
  To: Ricardo Ribalda
  Cc: Mauro Carvalho Chehab, Guennadi Liakhovetski, Greg Kroah-Hartman,
	Hans de Goede, linux-media, linux-kernel, linux-usb

On Mon, Jul 14, 2025 at 10:21:20AM +0200, Ricardo Ribalda wrote:
> On Mon, 14 Jul 2025 at 10:06, Laurent Pinchart wrote:
> > On Mon, Jul 14, 2025 at 09:46:45AM +0200, Ricardo Ribalda wrote:
> > > On Fri, 11 Jul 2025 at 21:58, Laurent Pinchart wrote:
> > > > On Mon, Jul 07, 2025 at 06:34:05PM +0000, Ricardo Ribalda wrote:
> > > > > If the camera supports the MSXU_CONTROL_METADATA control, auto set the
> > > > > MSXU_META quirk.
> > > > >
> > > > > Reviewed-by: Hans de Goede <hansg@kernel.org>
> > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > > ---
> > > > >  drivers/media/usb/uvc/uvc_driver.c   |  7 +++-
> > > > >  drivers/media/usb/uvc/uvc_metadata.c | 75 +++++++++++++++++++++++++++++++++++-
> > > > >  drivers/media/usb/uvc/uvcvideo.h     |  2 +-
> > > > >  include/linux/usb/uvc.h              |  3 ++
> > > > >  4 files changed, 84 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
> > > > > index 56ea20eeb7b9d5d92f3d837c15bdf11d536e9f2d..9de5abb43e19d9e876cddc5d7124592953db89ac 100644
> > > > > --- a/drivers/media/usb/uvc/uvc_driver.c
> > > > > +++ b/drivers/media/usb/uvc/uvc_driver.c
> > > > > @@ -2315,7 +2315,12 @@ static int uvc_probe(struct usb_interface *intf,
> > > > >               goto error;
> > > > >       }
> > > > >
> > > > > -     uvc_meta_init(dev);
> > > > > +     ret = uvc_meta_init(dev);
> > > > > +     if (ret < 0) {
> > > > > +             dev_err(&dev->udev->dev,
> > > > > +                     "Error initializing the metadata formats (%d)\n", ret);
> > > > > +             goto error;
> > > > > +     }
> > > > >
> > > > >       if (dev->quirks & UVC_QUIRK_NO_RESET_RESUME)
> > > > >               udev->quirks &= ~USB_QUIRK_RESET_RESUME;
> > > > > diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
> > > > > index 77e03273d3cf6b00cac6ebb9b29b941f1cbfd9f7..59bb133baf9a73ef6a30fa8ead85aa90653d60f4 100644
> > > > > --- a/drivers/media/usb/uvc/uvc_metadata.c
> > > > > +++ b/drivers/media/usb/uvc/uvc_metadata.c
> > > > > @@ -10,6 +10,7 @@
> > > > >  #include <linux/list.h>
> > > > >  #include <linux/module.h>
> > > > >  #include <linux/usb.h>
> > > > > +#include <linux/usb/uvc.h>
> > > > >  #include <linux/videodev2.h>
> > > > >
> > > > >  #include <media/v4l2-ioctl.h>
> > > > > @@ -166,6 +167,71 @@ static const struct v4l2_file_operations uvc_meta_fops = {
> > > > >       .mmap = vb2_fop_mmap,
> > > > >  };
> > > > >
> > > > > +static struct uvc_entity *uvc_meta_find_msxu(struct uvc_device *dev)
> > > > > +{
> > > > > +     static const u8 uvc_msxu_guid[16] = UVC_GUID_MSXU_1_5;
> > > > > +     struct uvc_entity *entity;
> > > > > +
> > > > > +     list_for_each_entry(entity, &dev->entities, list) {
> > > > > +             if (!memcmp(entity->guid, uvc_msxu_guid, sizeof(entity->guid)))
> > > > > +                     return entity;
> > > > > +     }
> > > > > +
> > > > > +     return NULL;
> > > > > +}
> > > > > +
> > > > > +#define MSXU_CONTROL_METADATA 0x9
> > > > > +static int uvc_meta_detect_msxu(struct uvc_device *dev)
> > > > > +{
> > > > > +     u32 *data __free(kfree) = NULL;
> > > > > +     struct uvc_entity *entity;
> > > > > +     int ret;
> > > > > +
> > > > > +     entity = uvc_meta_find_msxu(dev);
> > > > > +     if (!entity)
> > > > > +             return 0;
> > > > > +
> > > > > +     /*
> > > > > +      * USB requires buffers aligned in a special way, simplest way is to
> > > > > +      * make sure that query_ctrl will work is to kmalloc() them.
> > > > > +      */
> > > > > +     data = kmalloc(sizeof(*data), GFP_KERNEL);
> > > > > +     if (!data)
> > > > > +             return -ENOMEM;
> > > > > +
> > > > > +     /* Check if the metadata is already enabled. */
> > > > > +     ret = uvc_query_ctrl(dev, UVC_GET_CUR, entity->id, dev->intfnum,
> > > > > +                          MSXU_CONTROL_METADATA, data, sizeof(*data));
> > > > > +     if (ret)
> > > > > +             return 0;
> > > > > +
> > > > > +     if (*data) {
> > > > > +             dev->quirks |= UVC_QUIRK_MSXU_META;
> > > > > +             return 0;
> > > > > +     }
> > > > > +
> > > > > +     /*
> > > > > +      * We have seen devices that require 1 to enable the metadata, others
> > > > > +      * requiring a value != 1 and others requiring a value >1. Luckily for
> > > >
> > > > I'm confused here. If those are three different behaviours, then value
> > > > != 1 would be value == 0 (as the third behaviour is value > 1). You test
> > > > for !*data below, so 0 is not accepted as a valid value for this
> > > > purpose. What am I missing ?
> > >
> > > There is a typo in the comment.
> > >
> > > Some devices require 1, some devices any value !=0, and other value=MAX.
> > > I will fix it in a follow-up patch.
> >
> > The documentation of the control states that MSXU_CONTROL_METADATA
> > reports the maximum size of the MS metadata generated by the device in
> > kB, and the control should be set to the value reported by GET_MAX to
> > enable metadata generation. That's what you're doing in this patch, so
> > you can update the comment to document there.
> 
> Some vendors assumed the kB was a typo in MS spec and used bytes instead :)
> In this favor I must say that kB here sounds like a mistake, I haven't
> seen more than 300 bytes of metadata.

It doesn't seem to be a typo, as the documentation states

  The camera can produce metadata and the total size of such metadata
  can't exceed the dwValue - as reported by GET_MAX request – times 1024
  bytes less the size of a UsbVideoHeader metadata payload, for any
  frame.

It looks like a design mistake though.

> Will send a patch with the updated documentation.
> 
> > Some devices also don't support SET_CUR, in which case they should
> > report GET_MIN == GET_DEF == GET_MAX. I assume GET_CUR should then also
> > report the same value. Please also update the previous comment to
> > explain this, the GET_CUR value check above is more about handling
> > devices that always produce metadata than devices for which a driver has
> > enabled metadata production.
> >
> > This leads to another question: should we enable metadata generation
> > only when the metadata capture device is streaming ?
> 
> This will lead to a lot of issues with very little benefit.
> 
> We have no control over the order that the user will enable streamon
> in the metadata and "normal" video devices. And the device will not
> like that MSXU_CONTROL_METADATA changes during streamon.
> 
> I have not seen any performance penalty by the 300 new bytes
> transferred when the metadata is on.
> 
> > > > > +      * us, the value from GET_MAX seems to work all the time.
> > > > > +      */
> > > > > +     ret = uvc_query_ctrl(dev, UVC_GET_MAX, entity->id, dev->intfnum,
> > > > > +                          MSXU_CONTROL_METADATA, data, sizeof(*data));
> > > > > +     if (ret || !*data)
> > > > > +             return 0;
> > > > > +
> > > > > +     /*
> > > > > +      * If we can set MSXU_CONTROL_METADATA, the device will report
> > > > > +      * metadata.
> > > > > +      */
> > > > > +     ret = uvc_query_ctrl(dev, UVC_SET_CUR, entity->id, dev->intfnum,
> > > > > +                          MSXU_CONTROL_METADATA, data, sizeof(*data));
> > > > > +     if (!ret)
> > > > > +             dev->quirks |= UVC_QUIRK_MSXU_META;
> > > > > +
> > > > > +     return 0;
> > > > > +}
> > > > > +
> > > > >  int uvc_meta_register(struct uvc_streaming *stream)
> > > > >  {
> > > > >       struct uvc_device *dev = stream->dev;
> > > > > @@ -185,9 +251,14 @@ int uvc_meta_register(struct uvc_streaming *stream)
> > > > >                                        &uvc_meta_fops, &uvc_meta_ioctl_ops);
> > > > >  }
> > > > >
> > > > > -void uvc_meta_init(struct uvc_device *dev)
> > > > > +int uvc_meta_init(struct uvc_device *dev)
> > > > >  {
> > > > >       unsigned int i = 0;
> > > > > +     int ret;
> > > > > +
> > > > > +     ret = uvc_meta_detect_msxu(dev);
> > > > > +     if (ret)
> > > > > +             return ret;
> > > > >
> > > > >       dev->meta_formats[i++] = V4L2_META_FMT_UVC;
> > > > >
> > > > > @@ -201,4 +272,6 @@ void uvc_meta_init(struct uvc_device *dev)
> > > > >
> > > > >        /* IMPORTANT: for new meta-formats update UVC_MAX_META_DATA_FORMATS. */
> > > > >       dev->meta_formats[i++] = 0;
> > > > > +
> > > > > +     return 0;
> > > > >  }
> > > > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > > > > index 616adc417c62a58686beccbc440a5dfac0a2d588..a4c064c5e046f2a4adba742c8777a10619569606 100644
> > > > > --- a/drivers/media/usb/uvc/uvcvideo.h
> > > > > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > > > > @@ -757,7 +757,7 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
> > > > >  void uvc_video_clock_update(struct uvc_streaming *stream,
> > > > >                           struct vb2_v4l2_buffer *vbuf,
> > > > >                           struct uvc_buffer *buf);
> > > > > -void uvc_meta_init(struct uvc_device *dev);
> > > > > +int uvc_meta_init(struct uvc_device *dev);
> > > > >  int uvc_meta_register(struct uvc_streaming *stream);
> > > > >
> > > > >  int uvc_register_video_device(struct uvc_device *dev,
> > > > > diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h
> > > > > index bce95153e5a65613a710d7316fc17cf5462b5bce..ee19e9f915b8370c333c426dc1ee4202c7b75c5b 100644
> > > > > --- a/include/linux/usb/uvc.h
> > > > > +++ b/include/linux/usb/uvc.h
> > > > > @@ -29,6 +29,9 @@
> > > > >  #define UVC_GUID_EXT_GPIO_CONTROLLER \
> > > > >       {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> > > > >        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x03}
> > > > > +#define UVC_GUID_MSXU_1_5 \
> > > > > +     {0xdc, 0x95, 0x3f, 0x0f, 0x32, 0x26, 0x4e, 0x4c, \
> > > > > +      0x92, 0xc9, 0xa0, 0x47, 0x82, 0xf4, 0x3b, 0xc8}
> > > > >
> > > > >  #define UVC_GUID_FORMAT_MJPEG \
> > > > >       { 'M',  'J',  'P',  'G', 0x00, 0x00, 0x10, 0x00, \
> > > > >

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 4/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5
  2025-07-07 18:34 ` [PATCH v8 4/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5 Ricardo Ribalda
  2025-07-11 20:13   ` Laurent Pinchart
@ 2025-07-14 14:59   ` Laurent Pinchart
  2025-07-14 16:21     ` Ricardo Ribalda
  1 sibling, 1 reply; 21+ messages in thread
From: Laurent Pinchart @ 2025-07-14 14:59 UTC (permalink / raw)
  To: Ricardo Ribalda
  Cc: Mauro Carvalho Chehab, Guennadi Liakhovetski, Greg Kroah-Hartman,
	Hans de Goede, linux-media, linux-kernel, linux-usb

Hi Ricardo,

A bit of a stupid question, or rather a question that I wonder why I
didn't think of before.

On Mon, Jul 07, 2025 at 06:34:04PM +0000, 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 to have the whole metadata payload 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_MSXU_1_5, that is
> identical to V4L2_META_FMT_D4XX.

Wouldn't it be simpler for everybody to just

#define V4L2_META_FMT_UVC_MSXU_1_5  v4l2_fourcc('D', '4', 'X', 'X') /* UVC MSXU metadata */
#define V4L2_META_FMT_D4XX	V4L2_META_FMT_UVC_MSXU_1_5 /* For backward compatibility */

? I'm a bit uncomfortable with committing to a UABI with two different
4CCs for the exact same format.

> Let the user enable this format with a quirk for now. This way they can
> test if their devices provide useful metadata without rebuilding the
> kernel. They can later contribute patches to auto-quirk their devices.
> We will also work in methods to auto-detect devices compatible with this
> new metadata format.
> 
> Suggested-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Hans de Goede <hansg@kernel.org>
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  .../userspace-api/media/v4l/meta-formats.rst       |  1 +
>  .../media/v4l/metafmt-uvc-msxu-1-5.rst             | 23 ++++++++++++++++++++++
>  MAINTAINERS                                        |  1 +
>  drivers/media/usb/uvc/uvc_metadata.c               |  4 ++++
>  drivers/media/usb/uvc/uvcvideo.h                   |  1 +
>  drivers/media/v4l2-core/v4l2-ioctl.c               |  1 +
>  include/uapi/linux/videodev2.h                     |  1 +
>  7 files changed, 32 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst
> index bb6876cfc271e1a0543eee4209d6251e1a6a73cc..0de80328c36bf148051a19abe9e5241234ddfe5c 100644
> --- a/Documentation/userspace-api/media/v4l/meta-formats.rst
> +++ b/Documentation/userspace-api/media/v4l/meta-formats.rst
> @@ -20,6 +20,7 @@ These formats are used for the :ref:`metadata` interface only.
>      metafmt-pisp-fe
>      metafmt-rkisp1
>      metafmt-uvc
> +    metafmt-uvc-msxu-1-5
>      metafmt-vivid
>      metafmt-vsp1-hgo
>      metafmt-vsp1-hgt
> diff --git a/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst b/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst
> new file mode 100644
> index 0000000000000000000000000000000000000000..dd1c3076df243d770a13e7f6d07c3296a269e16a
> --- /dev/null
> +++ b/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst
> @@ -0,0 +1,23 @@
> +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
> +
> +.. _v4l2-meta-fmt-uvc-msxu-1-5:
> +
> +***********************************
> +V4L2_META_FMT_UVC_MSXU_1_5 ('UVCM')
> +***********************************
> +
> +Microsoft(R)'s UVC Payload Metadata.
> +
> +
> +Description
> +===========
> +
> +V4L2_META_FMT_UVC_MSXU_1_5 buffers follow the metadata buffer layout of
> +V4L2_META_FMT_UVC with the only difference that it includes all the UVC
> +metadata in the `buffer[]` field, not just the first 2-12 bytes.
> +
> +The metadata format follows the specification from Microsoft(R) [1].
> +
> +.. _1:
> +
> +[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 658543062bba3b7e600699d7271ffc89250ba7e5..fdde1d37ed2ef9058e3ea3417bec25afe454dfc0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -25827,6 +25827,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-msxu-1-5.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 4bcbc22f47e67c52baf6e133f240131ff3d32a03..77e03273d3cf6b00cac6ebb9b29b941f1cbfd9f7 100644
> --- a/drivers/media/usb/uvc/uvc_metadata.c
> +++ b/drivers/media/usb/uvc/uvc_metadata.c
> @@ -195,6 +195,10 @@ void uvc_meta_init(struct uvc_device *dev)
>  	    !WARN_ON(dev->info->meta_format == V4L2_META_FMT_UVC))
>  		dev->meta_formats[i++] = dev->info->meta_format;
>  
> +	if (dev->quirks & UVC_QUIRK_MSXU_META &&
> +	    !WARN_ON(dev->info->meta_format == V4L2_META_FMT_UVC_MSXU_1_5))
> +		dev->meta_formats[i++] = V4L2_META_FMT_UVC_MSXU_1_5;
> +
>  	 /* IMPORTANT: for new meta-formats update UVC_MAX_META_DATA_FORMATS. */
>  	dev->meta_formats[i++] = 0;
>  }
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index b3c094c6591e7a71fc00e1096bcf493a83f330ad..616adc417c62a58686beccbc440a5dfac0a2d588 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -77,6 +77,7 @@
>  #define UVC_QUIRK_DISABLE_AUTOSUSPEND	0x00008000
>  #define UVC_QUIRK_INVALID_DEVICE_SOF	0x00010000
>  #define UVC_QUIRK_MJPEG_NO_EOF		0x00020000
> +#define UVC_QUIRK_MSXU_META		0x00040000
>  
>  /* Format flags */
>  #define UVC_FMT_FLAG_COMPRESSED		0x00000001
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index be94a79b976e3de4eb957f5d2584ec6d4230469e..993b36417b4655456ce545cb42a41b55b98e4d6c 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1463,6 +1463,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_MSXU_1_5:	descr = "UVC MSXU 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 6f7bd38dd5aa4b1b2084685512512a380d76a5e4..863bc5b7dec32303e852d7e9c3891011ce5a3d71 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -867,6 +867,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_MSXU_1_5  v4l2_fourcc('U', 'V', 'C', 'M') /* UVC MSXU metadata */
>  #define V4L2_META_FMT_VIVID	  v4l2_fourcc('V', 'I', 'V', 'D') /* Vivid Metadata */
>  
>  /* Vendor specific - used for RK_ISP1 camera sub-system */

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 4/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5
  2025-07-14 14:59   ` Laurent Pinchart
@ 2025-07-14 16:21     ` Ricardo Ribalda
  2025-07-14 16:27       ` Laurent Pinchart
  0 siblings, 1 reply; 21+ messages in thread
From: Ricardo Ribalda @ 2025-07-14 16:21 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Mauro Carvalho Chehab, Guennadi Liakhovetski, Greg Kroah-Hartman,
	Hans de Goede, linux-media, linux-kernel, linux-usb

Hi Laurent

On Mon, 14 Jul 2025 at 17:00, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Ricardo,
>
> A bit of a stupid question, or rather a question that I wonder why I
> didn't think of before.

I believe we discussed this in the very beginning, when I just enabled
V4L2_META_FMT_D4XX for all the devices.

We thought that it could break applications. Imagine an APP that can
work with D4XX but not with other formats: if it tries to parse MSXU
format it might crash.

Regards

>
> On Mon, Jul 07, 2025 at 06:34:04PM +0000, 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 to have the whole metadata payload 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_MSXU_1_5, that is
> > identical to V4L2_META_FMT_D4XX.
>
> Wouldn't it be simpler for everybody to just
>
> #define V4L2_META_FMT_UVC_MSXU_1_5  v4l2_fourcc('D', '4', 'X', 'X') /* UVC MSXU metadata */
> #define V4L2_META_FMT_D4XX      V4L2_META_FMT_UVC_MSXU_1_5 /* For backward compatibility */
>
> ? I'm a bit uncomfortable with committing to a UABI with two different
> 4CCs for the exact same format.
>
> > Let the user enable this format with a quirk for now. This way they can
> > test if their devices provide useful metadata without rebuilding the
> > kernel. They can later contribute patches to auto-quirk their devices.
> > We will also work in methods to auto-detect devices compatible with this
> > new metadata format.
> >
> > Suggested-by: Hans de Goede <hdegoede@redhat.com>
> > Reviewed-by: Hans de Goede <hansg@kernel.org>
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> >  .../userspace-api/media/v4l/meta-formats.rst       |  1 +
> >  .../media/v4l/metafmt-uvc-msxu-1-5.rst             | 23 ++++++++++++++++++++++
> >  MAINTAINERS                                        |  1 +
> >  drivers/media/usb/uvc/uvc_metadata.c               |  4 ++++
> >  drivers/media/usb/uvc/uvcvideo.h                   |  1 +
> >  drivers/media/v4l2-core/v4l2-ioctl.c               |  1 +
> >  include/uapi/linux/videodev2.h                     |  1 +
> >  7 files changed, 32 insertions(+)
> >
> > diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > index bb6876cfc271e1a0543eee4209d6251e1a6a73cc..0de80328c36bf148051a19abe9e5241234ddfe5c 100644
> > --- a/Documentation/userspace-api/media/v4l/meta-formats.rst
> > +++ b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > @@ -20,6 +20,7 @@ These formats are used for the :ref:`metadata` interface only.
> >      metafmt-pisp-fe
> >      metafmt-rkisp1
> >      metafmt-uvc
> > +    metafmt-uvc-msxu-1-5
> >      metafmt-vivid
> >      metafmt-vsp1-hgo
> >      metafmt-vsp1-hgt
> > diff --git a/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst b/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..dd1c3076df243d770a13e7f6d07c3296a269e16a
> > --- /dev/null
> > +++ b/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst
> > @@ -0,0 +1,23 @@
> > +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
> > +
> > +.. _v4l2-meta-fmt-uvc-msxu-1-5:
> > +
> > +***********************************
> > +V4L2_META_FMT_UVC_MSXU_1_5 ('UVCM')
> > +***********************************
> > +
> > +Microsoft(R)'s UVC Payload Metadata.
> > +
> > +
> > +Description
> > +===========
> > +
> > +V4L2_META_FMT_UVC_MSXU_1_5 buffers follow the metadata buffer layout of
> > +V4L2_META_FMT_UVC with the only difference that it includes all the UVC
> > +metadata in the `buffer[]` field, not just the first 2-12 bytes.
> > +
> > +The metadata format follows the specification from Microsoft(R) [1].
> > +
> > +.. _1:
> > +
> > +[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 658543062bba3b7e600699d7271ffc89250ba7e5..fdde1d37ed2ef9058e3ea3417bec25afe454dfc0 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -25827,6 +25827,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-msxu-1-5.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 4bcbc22f47e67c52baf6e133f240131ff3d32a03..77e03273d3cf6b00cac6ebb9b29b941f1cbfd9f7 100644
> > --- a/drivers/media/usb/uvc/uvc_metadata.c
> > +++ b/drivers/media/usb/uvc/uvc_metadata.c
> > @@ -195,6 +195,10 @@ void uvc_meta_init(struct uvc_device *dev)
> >           !WARN_ON(dev->info->meta_format == V4L2_META_FMT_UVC))
> >               dev->meta_formats[i++] = dev->info->meta_format;
> >
> > +     if (dev->quirks & UVC_QUIRK_MSXU_META &&
> > +         !WARN_ON(dev->info->meta_format == V4L2_META_FMT_UVC_MSXU_1_5))
> > +             dev->meta_formats[i++] = V4L2_META_FMT_UVC_MSXU_1_5;
> > +
> >        /* IMPORTANT: for new meta-formats update UVC_MAX_META_DATA_FORMATS. */
> >       dev->meta_formats[i++] = 0;
> >  }
> > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > index b3c094c6591e7a71fc00e1096bcf493a83f330ad..616adc417c62a58686beccbc440a5dfac0a2d588 100644
> > --- a/drivers/media/usb/uvc/uvcvideo.h
> > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > @@ -77,6 +77,7 @@
> >  #define UVC_QUIRK_DISABLE_AUTOSUSPEND        0x00008000
> >  #define UVC_QUIRK_INVALID_DEVICE_SOF 0x00010000
> >  #define UVC_QUIRK_MJPEG_NO_EOF               0x00020000
> > +#define UVC_QUIRK_MSXU_META          0x00040000
> >
> >  /* Format flags */
> >  #define UVC_FMT_FLAG_COMPRESSED              0x00000001
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> > index be94a79b976e3de4eb957f5d2584ec6d4230469e..993b36417b4655456ce545cb42a41b55b98e4d6c 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -1463,6 +1463,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_MSXU_1_5:        descr = "UVC MSXU 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 6f7bd38dd5aa4b1b2084685512512a380d76a5e4..863bc5b7dec32303e852d7e9c3891011ce5a3d71 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -867,6 +867,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_MSXU_1_5  v4l2_fourcc('U', 'V', 'C', 'M') /* UVC MSXU metadata */
> >  #define V4L2_META_FMT_VIVID    v4l2_fourcc('V', 'I', 'V', 'D') /* Vivid Metadata */
> >
> >  /* Vendor specific - used for RK_ISP1 camera sub-system */
>
> --
> Regards,
>
> Laurent Pinchart



-- 
Ricardo Ribalda

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 4/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5
  2025-07-14 16:21     ` Ricardo Ribalda
@ 2025-07-14 16:27       ` Laurent Pinchart
  2025-07-14 16:42         ` Ricardo Ribalda
  0 siblings, 1 reply; 21+ messages in thread
From: Laurent Pinchart @ 2025-07-14 16:27 UTC (permalink / raw)
  To: Ricardo Ribalda
  Cc: Mauro Carvalho Chehab, Guennadi Liakhovetski, Greg Kroah-Hartman,
	Hans de Goede, linux-media, linux-kernel, linux-usb

On Mon, Jul 14, 2025 at 06:21:05PM +0200, Ricardo Ribalda wrote:
> On Mon, 14 Jul 2025 at 17:00, Laurent Pinchart wrote:
> >
> > Hi Ricardo,
> >
> > A bit of a stupid question, or rather a question that I wonder why I
> > didn't think of before.
> 
> I believe we discussed this in the very beginning, when I just enabled
> V4L2_META_FMT_D4XX for all the devices.

Sorry if that was the case, it was a while ago.

> We thought that it could break applications. Imagine an APP that can
> work with D4XX but not with other formats: if it tries to parse MSXU
> format it might crash.

How so, if V4L2_META_FMT_D4XX and V4L2_META_FMT_UVC_MSXU_1_5 identify
the same format ?

> > On Mon, Jul 07, 2025 at 06:34:04PM +0000, 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 to have the whole metadata payload 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_MSXU_1_5, that is
> > > identical to V4L2_META_FMT_D4XX.
> >
> > Wouldn't it be simpler for everybody to just
> >
> > #define V4L2_META_FMT_UVC_MSXU_1_5  v4l2_fourcc('D', '4', 'X', 'X') /* UVC MSXU metadata */
> > #define V4L2_META_FMT_D4XX      V4L2_META_FMT_UVC_MSXU_1_5 /* For backward compatibility */
> >
> > ? I'm a bit uncomfortable with committing to a UABI with two different
> > 4CCs for the exact same format.
> >
> > > Let the user enable this format with a quirk for now. This way they can
> > > test if their devices provide useful metadata without rebuilding the
> > > kernel. They can later contribute patches to auto-quirk their devices.
> > > We will also work in methods to auto-detect devices compatible with this
> > > new metadata format.
> > >
> > > Suggested-by: Hans de Goede <hdegoede@redhat.com>
> > > Reviewed-by: Hans de Goede <hansg@kernel.org>
> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > ---
> > >  .../userspace-api/media/v4l/meta-formats.rst       |  1 +
> > >  .../media/v4l/metafmt-uvc-msxu-1-5.rst             | 23 ++++++++++++++++++++++
> > >  MAINTAINERS                                        |  1 +
> > >  drivers/media/usb/uvc/uvc_metadata.c               |  4 ++++
> > >  drivers/media/usb/uvc/uvcvideo.h                   |  1 +
> > >  drivers/media/v4l2-core/v4l2-ioctl.c               |  1 +
> > >  include/uapi/linux/videodev2.h                     |  1 +
> > >  7 files changed, 32 insertions(+)
> > >
> > > diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > > index bb6876cfc271e1a0543eee4209d6251e1a6a73cc..0de80328c36bf148051a19abe9e5241234ddfe5c 100644
> > > --- a/Documentation/userspace-api/media/v4l/meta-formats.rst
> > > +++ b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > > @@ -20,6 +20,7 @@ These formats are used for the :ref:`metadata` interface only.
> > >      metafmt-pisp-fe
> > >      metafmt-rkisp1
> > >      metafmt-uvc
> > > +    metafmt-uvc-msxu-1-5
> > >      metafmt-vivid
> > >      metafmt-vsp1-hgo
> > >      metafmt-vsp1-hgt
> > > diff --git a/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst b/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst
> > > new file mode 100644
> > > index 0000000000000000000000000000000000000000..dd1c3076df243d770a13e7f6d07c3296a269e16a
> > > --- /dev/null
> > > +++ b/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst
> > > @@ -0,0 +1,23 @@
> > > +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
> > > +
> > > +.. _v4l2-meta-fmt-uvc-msxu-1-5:
> > > +
> > > +***********************************
> > > +V4L2_META_FMT_UVC_MSXU_1_5 ('UVCM')
> > > +***********************************
> > > +
> > > +Microsoft(R)'s UVC Payload Metadata.
> > > +
> > > +
> > > +Description
> > > +===========
> > > +
> > > +V4L2_META_FMT_UVC_MSXU_1_5 buffers follow the metadata buffer layout of
> > > +V4L2_META_FMT_UVC with the only difference that it includes all the UVC
> > > +metadata in the `buffer[]` field, not just the first 2-12 bytes.
> > > +
> > > +The metadata format follows the specification from Microsoft(R) [1].
> > > +
> > > +.. _1:
> > > +
> > > +[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index 658543062bba3b7e600699d7271ffc89250ba7e5..fdde1d37ed2ef9058e3ea3417bec25afe454dfc0 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -25827,6 +25827,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-msxu-1-5.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 4bcbc22f47e67c52baf6e133f240131ff3d32a03..77e03273d3cf6b00cac6ebb9b29b941f1cbfd9f7 100644
> > > --- a/drivers/media/usb/uvc/uvc_metadata.c
> > > +++ b/drivers/media/usb/uvc/uvc_metadata.c
> > > @@ -195,6 +195,10 @@ void uvc_meta_init(struct uvc_device *dev)
> > >           !WARN_ON(dev->info->meta_format == V4L2_META_FMT_UVC))
> > >               dev->meta_formats[i++] = dev->info->meta_format;
> > >
> > > +     if (dev->quirks & UVC_QUIRK_MSXU_META &&
> > > +         !WARN_ON(dev->info->meta_format == V4L2_META_FMT_UVC_MSXU_1_5))
> > > +             dev->meta_formats[i++] = V4L2_META_FMT_UVC_MSXU_1_5;
> > > +
> > >        /* IMPORTANT: for new meta-formats update UVC_MAX_META_DATA_FORMATS. */
> > >       dev->meta_formats[i++] = 0;
> > >  }
> > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > > index b3c094c6591e7a71fc00e1096bcf493a83f330ad..616adc417c62a58686beccbc440a5dfac0a2d588 100644
> > > --- a/drivers/media/usb/uvc/uvcvideo.h
> > > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > > @@ -77,6 +77,7 @@
> > >  #define UVC_QUIRK_DISABLE_AUTOSUSPEND        0x00008000
> > >  #define UVC_QUIRK_INVALID_DEVICE_SOF 0x00010000
> > >  #define UVC_QUIRK_MJPEG_NO_EOF               0x00020000
> > > +#define UVC_QUIRK_MSXU_META          0x00040000
> > >
> > >  /* Format flags */
> > >  #define UVC_FMT_FLAG_COMPRESSED              0x00000001
> > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> > > index be94a79b976e3de4eb957f5d2584ec6d4230469e..993b36417b4655456ce545cb42a41b55b98e4d6c 100644
> > > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > > @@ -1463,6 +1463,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_MSXU_1_5:        descr = "UVC MSXU 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 6f7bd38dd5aa4b1b2084685512512a380d76a5e4..863bc5b7dec32303e852d7e9c3891011ce5a3d71 100644
> > > --- a/include/uapi/linux/videodev2.h
> > > +++ b/include/uapi/linux/videodev2.h
> > > @@ -867,6 +867,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_MSXU_1_5  v4l2_fourcc('U', 'V', 'C', 'M') /* UVC MSXU metadata */
> > >  #define V4L2_META_FMT_VIVID    v4l2_fourcc('V', 'I', 'V', 'D') /* Vivid Metadata */
> > >
> > >  /* Vendor specific - used for RK_ISP1 camera sub-system */

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 4/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5
  2025-07-14 16:27       ` Laurent Pinchart
@ 2025-07-14 16:42         ` Ricardo Ribalda
  2025-07-14 17:07           ` Laurent Pinchart
  0 siblings, 1 reply; 21+ messages in thread
From: Ricardo Ribalda @ 2025-07-14 16:42 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Mauro Carvalho Chehab, Guennadi Liakhovetski, Greg Kroah-Hartman,
	Hans de Goede, linux-media, linux-kernel, linux-usb

Hi Laurent

On Mon, 14 Jul 2025 at 18:27, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Mon, Jul 14, 2025 at 06:21:05PM +0200, Ricardo Ribalda wrote:
> > On Mon, 14 Jul 2025 at 17:00, Laurent Pinchart wrote:
> > >
> > > Hi Ricardo,
> > >
> > > A bit of a stupid question, or rather a question that I wonder why I
> > > didn't think of before.
> >
> > I believe we discussed this in the very beginning, when I just enabled
> > V4L2_META_FMT_D4XX for all the devices.
>
> Sorry if that was the case, it was a while ago.
>
> > We thought that it could break applications. Imagine an APP that can
> > work with D4XX but not with other formats: if it tries to parse MSXU
> > format it might crash.
>
> How so, if V4L2_META_FMT_D4XX and V4L2_META_FMT_UVC_MSXU_1_5 identify
> the same format ?

D4XX uses vendor IDs from MSXU (0x80000000-2) [1]. There is no
guarantee that other vendors will collide with that ID.

Also, we do not know how apps will behave with IDs that they do not
know or with sizes that they have not been tested with.

[1] https://www.kernel.org/doc/html/v5.8/userspace-api/media/v4l/pixfmt-meta-d4xx.html#id8
>
> > > On Mon, Jul 07, 2025 at 06:34:04PM +0000, 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 to have the whole metadata payload 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_MSXU_1_5, that is
> > > > identical to V4L2_META_FMT_D4XX.
> > >
> > > Wouldn't it be simpler for everybody to just
> > >
> > > #define V4L2_META_FMT_UVC_MSXU_1_5  v4l2_fourcc('D', '4', 'X', 'X') /* UVC MSXU metadata */
> > > #define V4L2_META_FMT_D4XX      V4L2_META_FMT_UVC_MSXU_1_5 /* For backward compatibility */
> > >
> > > ? I'm a bit uncomfortable with committing to a UABI with two different
> > > 4CCs for the exact same format.
> > >
> > > > Let the user enable this format with a quirk for now. This way they can
> > > > test if their devices provide useful metadata without rebuilding the
> > > > kernel. They can later contribute patches to auto-quirk their devices.
> > > > We will also work in methods to auto-detect devices compatible with this
> > > > new metadata format.
> > > >
> > > > Suggested-by: Hans de Goede <hdegoede@redhat.com>
> > > > Reviewed-by: Hans de Goede <hansg@kernel.org>
> > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > ---
> > > >  .../userspace-api/media/v4l/meta-formats.rst       |  1 +
> > > >  .../media/v4l/metafmt-uvc-msxu-1-5.rst             | 23 ++++++++++++++++++++++
> > > >  MAINTAINERS                                        |  1 +
> > > >  drivers/media/usb/uvc/uvc_metadata.c               |  4 ++++
> > > >  drivers/media/usb/uvc/uvcvideo.h                   |  1 +
> > > >  drivers/media/v4l2-core/v4l2-ioctl.c               |  1 +
> > > >  include/uapi/linux/videodev2.h                     |  1 +
> > > >  7 files changed, 32 insertions(+)
> > > >
> > > > diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > > > index bb6876cfc271e1a0543eee4209d6251e1a6a73cc..0de80328c36bf148051a19abe9e5241234ddfe5c 100644
> > > > --- a/Documentation/userspace-api/media/v4l/meta-formats.rst
> > > > +++ b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > > > @@ -20,6 +20,7 @@ These formats are used for the :ref:`metadata` interface only.
> > > >      metafmt-pisp-fe
> > > >      metafmt-rkisp1
> > > >      metafmt-uvc
> > > > +    metafmt-uvc-msxu-1-5
> > > >      metafmt-vivid
> > > >      metafmt-vsp1-hgo
> > > >      metafmt-vsp1-hgt
> > > > diff --git a/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst b/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst
> > > > new file mode 100644
> > > > index 0000000000000000000000000000000000000000..dd1c3076df243d770a13e7f6d07c3296a269e16a
> > > > --- /dev/null
> > > > +++ b/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst
> > > > @@ -0,0 +1,23 @@
> > > > +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
> > > > +
> > > > +.. _v4l2-meta-fmt-uvc-msxu-1-5:
> > > > +
> > > > +***********************************
> > > > +V4L2_META_FMT_UVC_MSXU_1_5 ('UVCM')
> > > > +***********************************
> > > > +
> > > > +Microsoft(R)'s UVC Payload Metadata.
> > > > +
> > > > +
> > > > +Description
> > > > +===========
> > > > +
> > > > +V4L2_META_FMT_UVC_MSXU_1_5 buffers follow the metadata buffer layout of
> > > > +V4L2_META_FMT_UVC with the only difference that it includes all the UVC
> > > > +metadata in the `buffer[]` field, not just the first 2-12 bytes.
> > > > +
> > > > +The metadata format follows the specification from Microsoft(R) [1].
> > > > +
> > > > +.. _1:
> > > > +
> > > > +[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index 658543062bba3b7e600699d7271ffc89250ba7e5..fdde1d37ed2ef9058e3ea3417bec25afe454dfc0 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -25827,6 +25827,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-msxu-1-5.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 4bcbc22f47e67c52baf6e133f240131ff3d32a03..77e03273d3cf6b00cac6ebb9b29b941f1cbfd9f7 100644
> > > > --- a/drivers/media/usb/uvc/uvc_metadata.c
> > > > +++ b/drivers/media/usb/uvc/uvc_metadata.c
> > > > @@ -195,6 +195,10 @@ void uvc_meta_init(struct uvc_device *dev)
> > > >           !WARN_ON(dev->info->meta_format == V4L2_META_FMT_UVC))
> > > >               dev->meta_formats[i++] = dev->info->meta_format;
> > > >
> > > > +     if (dev->quirks & UVC_QUIRK_MSXU_META &&
> > > > +         !WARN_ON(dev->info->meta_format == V4L2_META_FMT_UVC_MSXU_1_5))
> > > > +             dev->meta_formats[i++] = V4L2_META_FMT_UVC_MSXU_1_5;
> > > > +
> > > >        /* IMPORTANT: for new meta-formats update UVC_MAX_META_DATA_FORMATS. */
> > > >       dev->meta_formats[i++] = 0;
> > > >  }
> > > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > > > index b3c094c6591e7a71fc00e1096bcf493a83f330ad..616adc417c62a58686beccbc440a5dfac0a2d588 100644
> > > > --- a/drivers/media/usb/uvc/uvcvideo.h
> > > > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > > > @@ -77,6 +77,7 @@
> > > >  #define UVC_QUIRK_DISABLE_AUTOSUSPEND        0x00008000
> > > >  #define UVC_QUIRK_INVALID_DEVICE_SOF 0x00010000
> > > >  #define UVC_QUIRK_MJPEG_NO_EOF               0x00020000
> > > > +#define UVC_QUIRK_MSXU_META          0x00040000
> > > >
> > > >  /* Format flags */
> > > >  #define UVC_FMT_FLAG_COMPRESSED              0x00000001
> > > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> > > > index be94a79b976e3de4eb957f5d2584ec6d4230469e..993b36417b4655456ce545cb42a41b55b98e4d6c 100644
> > > > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > > > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > > > @@ -1463,6 +1463,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_MSXU_1_5:        descr = "UVC MSXU 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 6f7bd38dd5aa4b1b2084685512512a380d76a5e4..863bc5b7dec32303e852d7e9c3891011ce5a3d71 100644
> > > > --- a/include/uapi/linux/videodev2.h
> > > > +++ b/include/uapi/linux/videodev2.h
> > > > @@ -867,6 +867,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_MSXU_1_5  v4l2_fourcc('U', 'V', 'C', 'M') /* UVC MSXU metadata */
> > > >  #define V4L2_META_FMT_VIVID    v4l2_fourcc('V', 'I', 'V', 'D') /* Vivid Metadata */
> > > >
> > > >  /* Vendor specific - used for RK_ISP1 camera sub-system */
>
> --
> Regards,
>
> Laurent Pinchart



--
Ricardo Ribalda

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 4/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5
  2025-07-14 16:42         ` Ricardo Ribalda
@ 2025-07-14 17:07           ` Laurent Pinchart
  2025-07-14 17:32             ` Ricardo Ribalda
  0 siblings, 1 reply; 21+ messages in thread
From: Laurent Pinchart @ 2025-07-14 17:07 UTC (permalink / raw)
  To: Ricardo Ribalda
  Cc: Mauro Carvalho Chehab, Guennadi Liakhovetski, Greg Kroah-Hartman,
	Hans de Goede, linux-media, linux-kernel, linux-usb

On Mon, Jul 14, 2025 at 06:42:14PM +0200, Ricardo Ribalda wrote:
> On Mon, 14 Jul 2025 at 18:27, Laurent Pinchart wrote:
> > On Mon, Jul 14, 2025 at 06:21:05PM +0200, Ricardo Ribalda wrote:
> > > On Mon, 14 Jul 2025 at 17:00, Laurent Pinchart wrote:
> > > >
> > > > Hi Ricardo,
> > > >
> > > > A bit of a stupid question, or rather a question that I wonder why I
> > > > didn't think of before.
> > >
> > > I believe we discussed this in the very beginning, when I just enabled
> > > V4L2_META_FMT_D4XX for all the devices.
> >
> > Sorry if that was the case, it was a while ago.
> >
> > > We thought that it could break applications. Imagine an APP that can
> > > work with D4XX but not with other formats: if it tries to parse MSXU
> > > format it might crash.
> >
> > How so, if V4L2_META_FMT_D4XX and V4L2_META_FMT_UVC_MSXU_1_5 identify
> > the same format ?
> 
> D4XX uses vendor IDs from MSXU (0x80000000-2) [1]. There is no
> guarantee that other vendors will collide with that ID.

I assume you mean "will not collide" ? There's also no guarantee that
different vendors implementing the MSXU won't use vendor-specific
metadata with colliding IDs, is there ?

> Also, we do not know how apps will behave with IDs that they do not
> know or with sizes that they have not been tested with.

The change won't cause any regression for those apps when using Intel
D4xx devices.

> [1] https://www.kernel.org/doc/html/v5.8/userspace-api/media/v4l/pixfmt-meta-d4xx.html#id8
>
> > > > On Mon, Jul 07, 2025 at 06:34:04PM +0000, 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 to have the whole metadata payload 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_MSXU_1_5, that is
> > > > > identical to V4L2_META_FMT_D4XX.
> > > >
> > > > Wouldn't it be simpler for everybody to just
> > > >
> > > > #define V4L2_META_FMT_UVC_MSXU_1_5  v4l2_fourcc('D', '4', 'X', 'X') /* UVC MSXU metadata */
> > > > #define V4L2_META_FMT_D4XX      V4L2_META_FMT_UVC_MSXU_1_5 /* For backward compatibility */
> > > >
> > > > ? I'm a bit uncomfortable with committing to a UABI with two different
> > > > 4CCs for the exact same format.
> > > >
> > > > > Let the user enable this format with a quirk for now. This way they can
> > > > > test if their devices provide useful metadata without rebuilding the
> > > > > kernel. They can later contribute patches to auto-quirk their devices.
> > > > > We will also work in methods to auto-detect devices compatible with this
> > > > > new metadata format.
> > > > >
> > > > > Suggested-by: Hans de Goede <hdegoede@redhat.com>
> > > > > Reviewed-by: Hans de Goede <hansg@kernel.org>
> > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > > ---
> > > > >  .../userspace-api/media/v4l/meta-formats.rst       |  1 +
> > > > >  .../media/v4l/metafmt-uvc-msxu-1-5.rst             | 23 ++++++++++++++++++++++
> > > > >  MAINTAINERS                                        |  1 +
> > > > >  drivers/media/usb/uvc/uvc_metadata.c               |  4 ++++
> > > > >  drivers/media/usb/uvc/uvcvideo.h                   |  1 +
> > > > >  drivers/media/v4l2-core/v4l2-ioctl.c               |  1 +
> > > > >  include/uapi/linux/videodev2.h                     |  1 +
> > > > >  7 files changed, 32 insertions(+)
> > > > >
> > > > > diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > > > > index bb6876cfc271e1a0543eee4209d6251e1a6a73cc..0de80328c36bf148051a19abe9e5241234ddfe5c 100644
> > > > > --- a/Documentation/userspace-api/media/v4l/meta-formats.rst
> > > > > +++ b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > > > > @@ -20,6 +20,7 @@ These formats are used for the :ref:`metadata` interface only.
> > > > >      metafmt-pisp-fe
> > > > >      metafmt-rkisp1
> > > > >      metafmt-uvc
> > > > > +    metafmt-uvc-msxu-1-5
> > > > >      metafmt-vivid
> > > > >      metafmt-vsp1-hgo
> > > > >      metafmt-vsp1-hgt
> > > > > diff --git a/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst b/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst
> > > > > new file mode 100644
> > > > > index 0000000000000000000000000000000000000000..dd1c3076df243d770a13e7f6d07c3296a269e16a
> > > > > --- /dev/null
> > > > > +++ b/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst
> > > > > @@ -0,0 +1,23 @@
> > > > > +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
> > > > > +
> > > > > +.. _v4l2-meta-fmt-uvc-msxu-1-5:
> > > > > +
> > > > > +***********************************
> > > > > +V4L2_META_FMT_UVC_MSXU_1_5 ('UVCM')
> > > > > +***********************************
> > > > > +
> > > > > +Microsoft(R)'s UVC Payload Metadata.
> > > > > +
> > > > > +
> > > > > +Description
> > > > > +===========
> > > > > +
> > > > > +V4L2_META_FMT_UVC_MSXU_1_5 buffers follow the metadata buffer layout of
> > > > > +V4L2_META_FMT_UVC with the only difference that it includes all the UVC
> > > > > +metadata in the `buffer[]` field, not just the first 2-12 bytes.
> > > > > +
> > > > > +The metadata format follows the specification from Microsoft(R) [1].
> > > > > +
> > > > > +.. _1:
> > > > > +
> > > > > +[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5
> > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > index 658543062bba3b7e600699d7271ffc89250ba7e5..fdde1d37ed2ef9058e3ea3417bec25afe454dfc0 100644
> > > > > --- a/MAINTAINERS
> > > > > +++ b/MAINTAINERS
> > > > > @@ -25827,6 +25827,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-msxu-1-5.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 4bcbc22f47e67c52baf6e133f240131ff3d32a03..77e03273d3cf6b00cac6ebb9b29b941f1cbfd9f7 100644
> > > > > --- a/drivers/media/usb/uvc/uvc_metadata.c
> > > > > +++ b/drivers/media/usb/uvc/uvc_metadata.c
> > > > > @@ -195,6 +195,10 @@ void uvc_meta_init(struct uvc_device *dev)
> > > > >           !WARN_ON(dev->info->meta_format == V4L2_META_FMT_UVC))
> > > > >               dev->meta_formats[i++] = dev->info->meta_format;
> > > > >
> > > > > +     if (dev->quirks & UVC_QUIRK_MSXU_META &&
> > > > > +         !WARN_ON(dev->info->meta_format == V4L2_META_FMT_UVC_MSXU_1_5))
> > > > > +             dev->meta_formats[i++] = V4L2_META_FMT_UVC_MSXU_1_5;
> > > > > +
> > > > >        /* IMPORTANT: for new meta-formats update UVC_MAX_META_DATA_FORMATS. */
> > > > >       dev->meta_formats[i++] = 0;
> > > > >  }
> > > > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > > > > index b3c094c6591e7a71fc00e1096bcf493a83f330ad..616adc417c62a58686beccbc440a5dfac0a2d588 100644
> > > > > --- a/drivers/media/usb/uvc/uvcvideo.h
> > > > > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > > > > @@ -77,6 +77,7 @@
> > > > >  #define UVC_QUIRK_DISABLE_AUTOSUSPEND        0x00008000
> > > > >  #define UVC_QUIRK_INVALID_DEVICE_SOF 0x00010000
> > > > >  #define UVC_QUIRK_MJPEG_NO_EOF               0x00020000
> > > > > +#define UVC_QUIRK_MSXU_META          0x00040000
> > > > >
> > > > >  /* Format flags */
> > > > >  #define UVC_FMT_FLAG_COMPRESSED              0x00000001
> > > > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> > > > > index be94a79b976e3de4eb957f5d2584ec6d4230469e..993b36417b4655456ce545cb42a41b55b98e4d6c 100644
> > > > > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > > > > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > > > > @@ -1463,6 +1463,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_MSXU_1_5:        descr = "UVC MSXU 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 6f7bd38dd5aa4b1b2084685512512a380d76a5e4..863bc5b7dec32303e852d7e9c3891011ce5a3d71 100644
> > > > > --- a/include/uapi/linux/videodev2.h
> > > > > +++ b/include/uapi/linux/videodev2.h
> > > > > @@ -867,6 +867,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_MSXU_1_5  v4l2_fourcc('U', 'V', 'C', 'M') /* UVC MSXU metadata */
> > > > >  #define V4L2_META_FMT_VIVID    v4l2_fourcc('V', 'I', 'V', 'D') /* Vivid Metadata */
> > > > >
> > > > >  /* Vendor specific - used for RK_ISP1 camera sub-system */

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH v8 4/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5
  2025-07-14 17:07           ` Laurent Pinchart
@ 2025-07-14 17:32             ` Ricardo Ribalda
  0 siblings, 0 replies; 21+ messages in thread
From: Ricardo Ribalda @ 2025-07-14 17:32 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Mauro Carvalho Chehab, Guennadi Liakhovetski, Greg Kroah-Hartman,
	Hans de Goede, linux-media, linux-kernel, linux-usb

On Mon, 14 Jul 2025 at 19:08, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Mon, Jul 14, 2025 at 06:42:14PM +0200, Ricardo Ribalda wrote:
> > On Mon, 14 Jul 2025 at 18:27, Laurent Pinchart wrote:
> > > On Mon, Jul 14, 2025 at 06:21:05PM +0200, Ricardo Ribalda wrote:
> > > > On Mon, 14 Jul 2025 at 17:00, Laurent Pinchart wrote:
> > > > >
> > > > > Hi Ricardo,
> > > > >
> > > > > A bit of a stupid question, or rather a question that I wonder why I
> > > > > didn't think of before.
> > > >
> > > > I believe we discussed this in the very beginning, when I just enabled
> > > > V4L2_META_FMT_D4XX for all the devices.
> > >
> > > Sorry if that was the case, it was a while ago.
> > >
> > > > We thought that it could break applications. Imagine an APP that can
> > > > work with D4XX but not with other formats: if it tries to parse MSXU
> > > > format it might crash.
> > >
> > > How so, if V4L2_META_FMT_D4XX and V4L2_META_FMT_UVC_MSXU_1_5 identify
> > > the same format ?
> >
> > D4XX uses vendor IDs from MSXU (0x80000000-2) [1]. There is no
> > guarantee that other vendors will collide with that ID.
>
> I assume you mean "will not collide" ? There's also no guarantee that
> different vendors implementing the MSXU won't use vendor-specific
> metadata with colliding IDs, is there ?

New apps will have to implement mechanisms to make sure that the data
is valid for them, but old apps did not have to care about id
collisions or unknown IDs.
They might even use the output of VIDIOC_ENUM_FMT to decide if a camera is D4XX.


>
> > Also, we do not know how apps will behave with IDs that they do not
> > know or with sizes that they have not been tested with.
>
> The change won't cause any regression for those apps when using Intel
> D4xx devices.

You are correct, but they might crash with notebooks internal cameras
and those cannot be easily disconnected.

At this stage I believe the current approach is the safest.


>
> > [1] https://www.kernel.org/doc/html/v5.8/userspace-api/media/v4l/pixfmt-meta-d4xx.html#id8
> >
> > > > > On Mon, Jul 07, 2025 at 06:34:04PM +0000, 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 to have the whole metadata payload 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_MSXU_1_5, that is
> > > > > > identical to V4L2_META_FMT_D4XX.
> > > > >
> > > > > Wouldn't it be simpler for everybody to just
> > > > >
> > > > > #define V4L2_META_FMT_UVC_MSXU_1_5  v4l2_fourcc('D', '4', 'X', 'X') /* UVC MSXU metadata */
> > > > > #define V4L2_META_FMT_D4XX      V4L2_META_FMT_UVC_MSXU_1_5 /* For backward compatibility */
> > > > >
> > > > > ? I'm a bit uncomfortable with committing to a UABI with two different
> > > > > 4CCs for the exact same format.
> > > > >
> > > > > > Let the user enable this format with a quirk for now. This way they can
> > > > > > test if their devices provide useful metadata without rebuilding the
> > > > > > kernel. They can later contribute patches to auto-quirk their devices.
> > > > > > We will also work in methods to auto-detect devices compatible with this
> > > > > > new metadata format.
> > > > > >
> > > > > > Suggested-by: Hans de Goede <hdegoede@redhat.com>
> > > > > > Reviewed-by: Hans de Goede <hansg@kernel.org>
> > > > > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > > > > ---
> > > > > >  .../userspace-api/media/v4l/meta-formats.rst       |  1 +
> > > > > >  .../media/v4l/metafmt-uvc-msxu-1-5.rst             | 23 ++++++++++++++++++++++
> > > > > >  MAINTAINERS                                        |  1 +
> > > > > >  drivers/media/usb/uvc/uvc_metadata.c               |  4 ++++
> > > > > >  drivers/media/usb/uvc/uvcvideo.h                   |  1 +
> > > > > >  drivers/media/v4l2-core/v4l2-ioctl.c               |  1 +
> > > > > >  include/uapi/linux/videodev2.h                     |  1 +
> > > > > >  7 files changed, 32 insertions(+)
> > > > > >
> > > > > > diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > > > > > index bb6876cfc271e1a0543eee4209d6251e1a6a73cc..0de80328c36bf148051a19abe9e5241234ddfe5c 100644
> > > > > > --- a/Documentation/userspace-api/media/v4l/meta-formats.rst
> > > > > > +++ b/Documentation/userspace-api/media/v4l/meta-formats.rst
> > > > > > @@ -20,6 +20,7 @@ These formats are used for the :ref:`metadata` interface only.
> > > > > >      metafmt-pisp-fe
> > > > > >      metafmt-rkisp1
> > > > > >      metafmt-uvc
> > > > > > +    metafmt-uvc-msxu-1-5
> > > > > >      metafmt-vivid
> > > > > >      metafmt-vsp1-hgo
> > > > > >      metafmt-vsp1-hgt
> > > > > > diff --git a/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst b/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst
> > > > > > new file mode 100644
> > > > > > index 0000000000000000000000000000000000000000..dd1c3076df243d770a13e7f6d07c3296a269e16a
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/userspace-api/media/v4l/metafmt-uvc-msxu-1-5.rst
> > > > > > @@ -0,0 +1,23 @@
> > > > > > +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later
> > > > > > +
> > > > > > +.. _v4l2-meta-fmt-uvc-msxu-1-5:
> > > > > > +
> > > > > > +***********************************
> > > > > > +V4L2_META_FMT_UVC_MSXU_1_5 ('UVCM')
> > > > > > +***********************************
> > > > > > +
> > > > > > +Microsoft(R)'s UVC Payload Metadata.
> > > > > > +
> > > > > > +
> > > > > > +Description
> > > > > > +===========
> > > > > > +
> > > > > > +V4L2_META_FMT_UVC_MSXU_1_5 buffers follow the metadata buffer layout of
> > > > > > +V4L2_META_FMT_UVC with the only difference that it includes all the UVC
> > > > > > +metadata in the `buffer[]` field, not just the first 2-12 bytes.
> > > > > > +
> > > > > > +The metadata format follows the specification from Microsoft(R) [1].
> > > > > > +
> > > > > > +.. _1:
> > > > > > +
> > > > > > +[1] https://docs.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5
> > > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > > index 658543062bba3b7e600699d7271ffc89250ba7e5..fdde1d37ed2ef9058e3ea3417bec25afe454dfc0 100644
> > > > > > --- a/MAINTAINERS
> > > > > > +++ b/MAINTAINERS
> > > > > > @@ -25827,6 +25827,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-msxu-1-5.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 4bcbc22f47e67c52baf6e133f240131ff3d32a03..77e03273d3cf6b00cac6ebb9b29b941f1cbfd9f7 100644
> > > > > > --- a/drivers/media/usb/uvc/uvc_metadata.c
> > > > > > +++ b/drivers/media/usb/uvc/uvc_metadata.c
> > > > > > @@ -195,6 +195,10 @@ void uvc_meta_init(struct uvc_device *dev)
> > > > > >           !WARN_ON(dev->info->meta_format == V4L2_META_FMT_UVC))
> > > > > >               dev->meta_formats[i++] = dev->info->meta_format;
> > > > > >
> > > > > > +     if (dev->quirks & UVC_QUIRK_MSXU_META &&
> > > > > > +         !WARN_ON(dev->info->meta_format == V4L2_META_FMT_UVC_MSXU_1_5))
> > > > > > +             dev->meta_formats[i++] = V4L2_META_FMT_UVC_MSXU_1_5;
> > > > > > +
> > > > > >        /* IMPORTANT: for new meta-formats update UVC_MAX_META_DATA_FORMATS. */
> > > > > >       dev->meta_formats[i++] = 0;
> > > > > >  }
> > > > > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> > > > > > index b3c094c6591e7a71fc00e1096bcf493a83f330ad..616adc417c62a58686beccbc440a5dfac0a2d588 100644
> > > > > > --- a/drivers/media/usb/uvc/uvcvideo.h
> > > > > > +++ b/drivers/media/usb/uvc/uvcvideo.h
> > > > > > @@ -77,6 +77,7 @@
> > > > > >  #define UVC_QUIRK_DISABLE_AUTOSUSPEND        0x00008000
> > > > > >  #define UVC_QUIRK_INVALID_DEVICE_SOF 0x00010000
> > > > > >  #define UVC_QUIRK_MJPEG_NO_EOF               0x00020000
> > > > > > +#define UVC_QUIRK_MSXU_META          0x00040000
> > > > > >
> > > > > >  /* Format flags */
> > > > > >  #define UVC_FMT_FLAG_COMPRESSED              0x00000001
> > > > > > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> > > > > > index be94a79b976e3de4eb957f5d2584ec6d4230469e..993b36417b4655456ce545cb42a41b55b98e4d6c 100644
> > > > > > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > > > > > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > > > > > @@ -1463,6 +1463,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_MSXU_1_5:        descr = "UVC MSXU 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 6f7bd38dd5aa4b1b2084685512512a380d76a5e4..863bc5b7dec32303e852d7e9c3891011ce5a3d71 100644
> > > > > > --- a/include/uapi/linux/videodev2.h
> > > > > > +++ b/include/uapi/linux/videodev2.h
> > > > > > @@ -867,6 +867,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_MSXU_1_5  v4l2_fourcc('U', 'V', 'C', 'M') /* UVC MSXU metadata */
> > > > > >  #define V4L2_META_FMT_VIVID    v4l2_fourcc('V', 'I', 'V', 'D') /* Vivid Metadata */
> > > > > >
> > > > > >  /* Vendor specific - used for RK_ISP1 camera sub-system */
>
> --
> Regards,
>
> Laurent Pinchart



--
Ricardo Ribalda

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2025-07-14 17:33 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-07 18:34 [PATCH v8 0/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5 + other meta fixes Ricardo Ribalda
2025-07-07 18:34 ` [PATCH v8 1/5] media: uvcvideo: Do not mark valid metadata as invalid Ricardo Ribalda
2025-07-07 18:34 ` [PATCH v8 2/5] media: Documentation: Add note about UVCH length field Ricardo Ribalda
2025-07-07 18:34 ` [PATCH v8 3/5] media: uvcvideo: Introduce dev->meta_formats Ricardo Ribalda
2025-07-07 20:55   ` Hans de Goede
2025-07-07 18:34 ` [PATCH v8 4/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5 Ricardo Ribalda
2025-07-11 20:13   ` Laurent Pinchart
2025-07-14  7:44     ` Ricardo Ribalda
2025-07-14 14:59   ` Laurent Pinchart
2025-07-14 16:21     ` Ricardo Ribalda
2025-07-14 16:27       ` Laurent Pinchart
2025-07-14 16:42         ` Ricardo Ribalda
2025-07-14 17:07           ` Laurent Pinchart
2025-07-14 17:32             ` Ricardo Ribalda
2025-07-07 18:34 ` [PATCH v8 5/5] media: uvcvideo: Auto-set UVC_QUIRK_MSXU_META Ricardo Ribalda
2025-07-11 19:58   ` Laurent Pinchart
2025-07-14  7:46     ` Ricardo Ribalda
2025-07-14  8:06       ` Laurent Pinchart
2025-07-14  8:21         ` Ricardo Ribalda
2025-07-14  8:34           ` Laurent Pinchart
2025-07-07 21:00 ` [PATCH v8 0/5] media: uvcvideo: Introduce V4L2_META_FMT_UVC_MSXU_1_5 + other meta fixes Hans de Goede

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).