<div dir="ltr"><div dir="ltr">Thanks Jacopo for forwarding this to me!</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, May 15, 2023 at 5:29 PM Jacopo Mondi <<a href="mailto:jacopo.mondi@ideasonboard.com">jacopo.mondi@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Sophie,<br>
sorry for the very long delay in reviewing this patch<br>
<br>
let me cc Harvey which, as you might know, is working on adding a<br>
pipeline handler implementation that does not use the media device<br>
abstraction.<br>
<br>
Harvey: Sophie is instead introducing a generalized form of device<br>
matching that can be made to use different identifiers than the media<br>
entities names. I wonder if this is something that could help with<br>
virtual pipeline (see below)<br>
<br>
On Thu, Mar 02, 2023 at 01:54:14AM +0100, Sophie Friedrich via libcamera-devel wrote:<br>
> In preparation for introducing libusb support into libcamera, we need<br>
> to generalize the matching functionality. The `MediaEntity` class was<br>
> never meant to handle the cases required for e.g. libusb. Additionally<br>
> the current way matching is handled is insufficient for (future)<br>
> `MediaDevices` in which a specific key/value relationship exists<br>
> for properties (e.g. USB vid and pid).<br>
<br>
Do I get it right that you plan to use the vid as key and the pid as<br>
value ?<br>
<br></blockquote><div><br></div><div>I don't really understand the difference of the key and the value in</div><div>`DeviceMatchEntityInterface`: it seems that `DeviceMatch::match`</div><div>needs to compare them both to match an entity, so why not merging</div><div>them into one (say id or key)? For USB devices, we can come up with an encoder</div><div>that takes vid and pid into one single id.</div><div><br></div><div>Unless there are further plans for the key and the value that I'm not aware of :)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
><br>
> To solve these two problems I've abstracted away the requirements for<br>
> entities that `DeviceMatch` requires in `DeviceMatchEntityInterface`<br>
> and extended `DeviceMatch` to handle key/value pairs. Using "*" as a<br>
> key is currently a catch all for the presumably keyless property of<br>
> V4L2 `MediaEntity`.<br>
<br>
Harvey: my understanding is that your plan is to define the number of<br>
virtual cameras in a configuration file. I wonder if by introducing a<br>
device enumerator that parses the configuration file we could create<br>
some form of matching for the Virtual pipeline handler.<br>
<br></blockquote><div><br></div><div>Yes, that makes a lot of sense to me. I'll update <a href="https://patchwork.libcamera.org/patch/18532/">https://patchwork.libcamera.org/patch/18532/</a> when this patch is merged.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
The "ConfigurationFileDeviceEnumerator" would define a number of<br>
"virtual instances" and the virtual pipeline handler will be matches<br>
against each of them. This would allow to create multiple instances of<br>
the virtual pipeline handler to test it, and in future, if this would<br>
ever be useful, to match different version of the virtual pipeline<br>
handler against different 'tags' specified in the configuration. Do<br>
you think it's something that might prove useful ?<br>
<br></blockquote><div><br></div><div>Yes, that sounds very applicable to me. I don't have a clear view of the</div><div>benefit of multiple VirtualPipelineHandlers though. I guess that's why</div><div>we need a DeviceEnumerator though :)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
><br>
> Sophie Friedrich (1):<br>
> libcamera: enumeration: Generalize DeviceMatch<br>
><br>
> .../libcamera/internal/device_enumerator.h | 16 +--<br>
> include/libcamera/internal/device_match.h | 46 +++++++<br>
> include/libcamera/internal/media_device.h | 1 +<br>
> include/libcamera/internal/media_object.h | 7 +-<br>
> include/libcamera/internal/meson.build | 1 +<br>
> src/libcamera/device_enumerator.cpp | 73 ----------<br>
> src/libcamera/device_match.cpp | 126 ++++++++++++++++++<br>
> src/libcamera/meson.build | 1 +<br>
> 8 files changed, 183 insertions(+), 88 deletions(-)<br>
> create mode 100644 include/libcamera/internal/device_match.h<br>
> create mode 100644 src/libcamera/device_match.cpp<br>
><br>
> --<br>
> 2.34.1<br>
><br>
</blockquote></div></div>