Re: [PATCH] media: uvcvideo: Enable full UVC metadata for all devices

[Hans de Goede]

Hi,

On 5-Mar-25 2:11 PM, Laurent Pinchart wrote:
> On Wed, Mar 05, 2025 at 02:03:58PM +0100, Hans de Goede wrote:
>> On 3-Mar-25 5:22 PM, Ricardo Ribalda wrote:
>>> On Mon, 3 Mar 2025 at 17:11, Laurent Pinchart wrote:
>>>> On Mon, Mar 03, 2025 at 04:43:50PM +0100, Hans de Goede wrote:
>>>>> On 3-Mar-25 16:13, Laurent Pinchart wrote:
>>>>>> On Mon, Mar 03, 2025 at 03:47:51PM +0100, Hans de Goede wrote:
>>>>>>> On 26-Feb-25 14:00, Ricardo Ribalda wrote:
>>>>>>>> The UVC driver provides two metadata types V4L2_META_FMT_UVC, and
>>>>>>>> V4L2_META_FMT_D4XX. The only difference between the two of them is that
>>>>>>>> V4L2_META_FMT_UVC only copies PTS, SCR, size and flags, and
>>>>>>>> V4L2_META_FMT_D4XX copies the whole metadata section.
>>>>>>>>
>>>>>>>> Now we only enable V4L2_META_FMT_D4XX for the Intel D4xx family of
>>>>>>>> devices, but it is useful for any device where vendors include other
>>>>>>>> metadata, such as the one described by Microsoft:
>>>>>>>> - https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/mf-capture-metadata
>>>>>>>>
>>>>>>>> This patch removes the UVC_INFO_META macro and enables
>>>>>>>> V4L2_META_FMT_D4XX for every device. It also updates the documentation
>>>>>>>> to reflect the change.
>>>>>>>>
>>>>>>>> Signed-off-by: Ricardo Ribalda <ribalda@xxxxxxxxxxxx>
>>>>>>>
>>>>>>> Thanks, patch looks good to me:
>>>>>>>
>>>>>>> Reviewed-by: Hans de Goede <hdegoede@xxxxxxxxxx>
>>>>>>
>>>>>> I don't quite agree, sorry.
>>>>>>
>>>>>> The reason why the current mechanism has been implemented this way is to
>>>>>> ensure we have documentation for the metadata format of devices that
>>>>>> expose metadata to userspace.
>>>>>>
>>>>>> If you want to expose the MS metadata, you can create a new metadata
>>>>>> format for that, and enable it on devices that implement it.
>>>>>
>>>>> Looking at the long list of quirks this removes just for the D4xx
>>>>> cameras I do not believe that having quirked based relaying of
>>>>> which metadata fmt is used to userspace is something which scales
>>>>> on the long term. Given the large amount of different UVC cameras
>>>>> I really think we should move the USB VID:PID -> metadata format
>>>>> mapping out of the kernel.
>>>>
>>>> If we can find a solution to ensure the metadata format gets documented,
>>>> sure.
>>>
>>> MS default metadata is already documented:
>>> https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/mf-capture-metadata
>>>
>>> I would not worry too much about vendors abusing the metadata for
>>> custom magic if that is your concern.
>>> This would not work with Windows default driver, and that is what most
>>> camera modules are tested against.
>>>
>>>> When it comes to the MS metadata format, if I recall correctly, Ricardo
>>>> said there's an XU with a known GUID to detect the metadata format. We
>>>> therefore wouldn't need quirks.
>>>
>>> There is MXSU control 	
>>> https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5
>>> But not all the vendors use it.
>>
>> Right so I actually already checked:
>>
>> https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5
>>
>> yesterday before my udev hwdb suggestion I was wondering if there was a way
>> to just get if MSmetadata was used from the camera itself. What I found was this:
>>
>> "UVC metadata is opt-in. Every IHV/OEM that needs metadata support must be enabled through a custom INF file."
>>
>> which lead me to the udev + hwdb suggestion.
>>
>> It is good to know that some cameras have a a way to fet this from a known XU GUID,
>> but the official way seems to be to have per camera info in .inf files. So for Linux
>> that would translate to having a list of vid:pid combinations somewhere.
>>
>> The question then becomes where do we put the vid:pid list and IMHO hwdb is much
>> better (easier to maintain / update) then hardcoding this in the kernel.
> 
> Additional questions: where do we store documentation for the metadata
> format, 

hwdb config files typically have a comment block with which key:value pairs
that hwdb file will set. We can add known/supported formats + links to
the documentation per format there.

> how do we convey what format is supported to applications

This would be a udev property on the /dev/video# node.

> , and
> how do we enable metadata capture when a device is listed in the DB ?

The idea would be to at the kernel side just add a single new CUSTOM metadata
fmt and when that is set copy the entire length of the received metadata
to userspace like we are already doing for the D4xx format.

Yes this will also possibly send extra metadata in other formats to
userspace, without us having documented the format but I don't really think
this is a showstopper here. It is not like this metadata is going to hide
some highly secret processing info which we need to know on the Linux side,
since all the processing is already done on the camera and we get a ready
to use image out of the camera.

Regards,

Hans