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