On Fri, 14 Mar 2025 at 10:09, Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote: > > Hi Ricardo, > > On Fri, Mar 14, 2025 at 09:28:34AM +0100, Ricardo Ribalda wrote: > > On Fri, 14 Mar 2025 at 07:35, Mauro Carvalho Chehab wrote: > > > Em Thu, 13 Mar 2025 12:06:27 +0000 Ricardo Ribalda escreveu: > > > > > > > The UVC driver provides two metadata types V4L2_META_FMT_UVC, and > > > > V4L2_META_FMT_D4XX. The only difference between the two of them is that > > > > V4L2_META_FMT_UVC only copies PTS, SCR, size and flags, and > > > > V4L2_META_FMT_D4XX copies the whole metadata section. > > > > > > > > Now we only enable V4L2_META_FMT_D4XX for the Intel D4xx family of > > > > devices, but it is useful to have the whole metadata section for any > > > > device where vendors include other metadata, such as the one described by > > > > Microsoft: > > > > https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/mf-capture-metadata > > > > > > > > This patch introduces a new format V4L2_META_FMT_UVC_CUSTOM, that is > > > > identical to V4L2_META_FMT_D4XX but it is available to all the UVC > > > > devices. > > > > > > > > Suggested-by: Hans de Goede <hdegoede@xxxxxxxxxx> > > > > Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx> > > > > --- > > > > .../userspace-api/media/v4l/meta-formats.rst | 1 + > > > > .../userspace-api/media/v4l/metafmt-uvc-custom.rst | 31 +++++++++++++++++ > > > > MAINTAINERS | 1 + > > > > drivers/media/usb/uvc/uvc_metadata.c | 40 ++++++++++++++++++---- > > > > drivers/media/v4l2-core/v4l2-ioctl.c | 1 + > > > > include/uapi/linux/videodev2.h | 1 + > > > > 6 files changed, 69 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/Documentation/userspace-api/media/v4l/meta-formats.rst b/Documentation/userspace-api/media/v4l/meta-formats.rst > > > > index 86ffb3bc8ade2e0c563dd84441572ecea1a571a6..9fd83f4a3cc8509702a2a9f032fdc04bf6c6d1bc 100644 > > > > --- a/Documentation/userspace-api/media/v4l/meta-formats.rst > > > > +++ b/Documentation/userspace-api/media/v4l/meta-formats.rst > > > > @@ -19,6 +19,7 @@ These formats are used for the :ref:`metadata` interface only. > > > > metafmt-pisp-fe > > > > metafmt-rkisp1 > > > > metafmt-uvc > > > > + metafmt-uvc-custom > > > > metafmt-vivid > > > > metafmt-vsp1-hgo > > > > metafmt-vsp1-hgt > > > > diff --git a/Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst b/Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst > > > > new file mode 100644 > > > > index 0000000000000000000000000000000000000000..9f150fc2b6f379cc4707ff45041dd014956ae11a > > > > --- /dev/null > > > > +++ b/Documentation/userspace-api/media/v4l/metafmt-uvc-custom.rst > > > > @@ -0,0 +1,31 @@ > > > > +.. SPDX-License-Identifier: GFDL-1.1-no-invariants-or-later > > > > + > > > > +.. _v4l2-meta-fmt-uvc-custom: > > > > + > > > > +********************************* > > > > +V4L2_META_FMT_UVC_CUSTOM ('UVCC') > > > > +********************************* > > > > + > > > > +UVC Custom Payload Metadata. > > > > + > > > > + > > > > +Description > > > > +=========== > > > > + > > > > +V4L2_META_FMT_UVC_CUSTOM buffers follow the metadata buffer layout of > > > > +V4L2_META_FMT_UVC with the only difference that it includes all the UVC > > > > +metadata, not just the first 2-12 bytes. > > > > + > > > > +The most common metadata format is the one proposed by Microsoft(R)'s UVC > > > > +extension [1_], but other vendors might have different formats. > > > > + > > > > +Applications might use information from the Hardware Database (hwdb)[2_] to > > > > +process the camera's metadata accordingly. > > > > > > Having something like that at the userspace API shouldn't be handled > > > lightly. This sounds to me that passing a blank check for vendors to stream > > > whatever they want without any requirements to provide and sort of > > > documentation for the usersace to decode it. > > > > As HdG previously mentioned, all the processing is done in the camera > > so the metadata is not going to hide highly secret required for > > processing: > > https://lore.kernel.org/linux-media/67c1a857-7656-421f-994c-751709b6ae01@xxxxxxxxxx/ > > Without judging whether or not such an undocumented format should be > supported by the driver, a correction is needed here: the issue is not > "secrets required for processing", but giving closed-source application > an unfair advantage. We could argue that vendors already have the possibility to pass secrets to userspace: - A camera could add proprietary information inside the frame, only parseable by a closed-source application - They can use undocumented UVC controls - They can exploit documented controls to do something else that they are designed for. Given these existing possibilities, I question whether "evil metadata" offers any fundamentally new capabilities that cannot be achieved through these established methods. If we have to talk about unfair advantage, Linux is at an unfair advantage right now: there is no way to use the *documented* information provided by the metadata. Other OSs can use it. The way I see it, with this artificial limitation we are not blocking evil vendors but punishing good users. if it makes you feel more comfortable we can start enabling only V4L2_META_FMT_UVC_CUSTOM (or V4L2_META_FMT_MSXU_UVC_1_5 as proposed by Mauro) to devices that support MSXU_CONTROL_METADATA, the future ChromeOS XU, or quirks. But that artificial limitation will hurt a lot of current cameras for no real reason. > > > > Also, it would be hard for userspace to distinguish what metatata > > > is contained for a random UVC camera. Please let's not do that. > > > > Userspace will use hwdb info to properly parse the metadata. > > I don't have experience with that, so I would like to see the effort > being started on hwdb support to see how it will look like before we > merge this patch. A few cameras should be added as examples, and a > stategy to ensure the hwdb will be properly populated should be > proposed. We can start by mapping the D4XX cameras. D4XX format follows Microsoft standard. Would that work for you? > > > > As the specific issue here is to support an already known extension, > > > which is already documented, Just add an specific format for it, e.g. > > > you could add something like that at the documentation: > > > > The problem here is how do we know from the driver if a device > > supports V4L2_META_FMT_MSXU_UVC_1_5 or not. > > > > In Windows it seems that vendors add that information to the device > > .inf file. That is equivalent to the hwdb proposal. > > In ChromeOS we are trying to push vendors to use an extension saying > > if there is metadata or not. But that will take some time to land and > > there are thousands of modules out there not ChromeOS compliant. > > > > > > > > V4L2_META_FMT_MSXU_UVC_1_5 > > > Microsoft extensions to USB Video Class 1.5 specification. > > > > > > For more details, see: https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5 > > > > > > And then add the corresponding format to V4L2 API. > > -- > Regards, > > Laurent Pinchart -- Ricardo Ribalda
