[libcamera-devel] [PATCH 0/1] Generalized DeviceMatch

[Sophie Friedrich]

Hi Harvey and Jacopo,

On 22/05/2023 15:37, Jacopo Mondi wrote:
> Hi Harvey
> 
> On Mon, May 22, 2023 at 08:22:52PM +0800, Cheng-Hao Yang via libcamera-devel wrote:
>> Thanks Jacopo for forwarding this to me!
>>
>> On Mon, May 15, 2023 at 5:29 PM Jacopo Mondi <jacopo.mondi at ideasonboard.com>
>> wrote:
>>
>>> Hi Sophie,
>>>    sorry for the very long delay in reviewing this patch
>>>
>>> let me cc Harvey which, as you might know, is working on adding a
>>> pipeline handler implementation that does not use the media device
>>> abstraction.

Thank you this sounds like a good idea.

>>> Harvey: Sophie is instead introducing a generalized form of device
>>> matching that can be made to use different identifiers than the media
>>> entities names. I wonder if this is something that could help with
>>> virtual pipeline (see below)
>>>
>>> On Thu, Mar 02, 2023 at 01:54:14AM +0100, Sophie Friedrich via
>>> libcamera-devel wrote:
>>>> In preparation for introducing libusb support into libcamera, we need
>>>> to generalize the matching functionality. The `MediaEntity` class was
>>>> never meant to handle the cases required for e.g. libusb. Additionally
>>>> the current way matching is handled is insufficient for (future)
>>>> `MediaDevices` in which a specific key/value relationship exists
>>>> for properties (e.g. USB vid and pid).
>>>
>>> Do I get it right that you plan to use the vid as key and the pid as
>>> value ?
>>>
>>>
>> I don't really understand the difference of the key and the value in
>> `DeviceMatchEntityInterface`: it seems that `DeviceMatch::match`
>> needs to compare them both to match an entity, so why not merging
>> them into one (say id or key)? For USB devices, we can come up with an
>> encoder
>> that takes vid and pid into one single id.
>>
>> Unless there are further plans for the key and the value that I'm not aware
>> of :)
>>
> 
> Let's hear from Sophie how she plans to use them

Maybe using VID and PID as an example was a bit misleading. I wanted to
generalize the interface in a way where we won't run into issues with
potential requirements for other device matching in the future.

In the case of matching USB devices this would be more than just VID and
PID. The Linux kernel finds the drivers for a new USB device by using
the `usb_device_id` struct which can contain more than just VID or PID
and is even able to look at these separately. Just using VID together
with PID could proof to be to limiting.

See the Linux kernel docs:
https://www.kernel.org/doc/html/v6.1/driver-api/basics.html#c.usb_device_id

For example, I imagine it to be used with the following keys
(for e.g. USB)

|        key       | value |
|------------------|-------|
| usb_id_product   | 1d6b  |
| usb_id_vendor    | 0003  |
| usb_bDeviceClass | 3     |


>>>>
>>>> Sophie Friedrich (1):
>>>>    libcamera: enumeration: Generalize DeviceMatch
>>>>
>>>>   .../libcamera/internal/device_enumerator.h    |  16 +--
>>>>   include/libcamera/internal/device_match.h     |  46 +++++++
>>>>   include/libcamera/internal/media_device.h     |   1 +
>>>>   include/libcamera/internal/media_object.h     |   7 +-
>>>>   include/libcamera/internal/meson.build        |   1 +
>>>>   src/libcamera/device_enumerator.cpp           |  73 ----------
>>>>   src/libcamera/device_match.cpp                | 126 ++++++++++++++++++
>>>>   src/libcamera/meson.build                     |   1 +
>>>>   8 files changed, 183 insertions(+), 88 deletions(-)
>>>>   create mode 100644 include/libcamera/internal/device_match.h
>>>>   create mode 100644 src/libcamera/device_match.cpp
>>>>
>>>> --
>>>> 2.34.1
>>>>
>>>