[libcamera-devel] [RFC PATCH v1] libcamera: device_enumerator_udev: remove devnum from dependency map

[Barnabás Pőcze]

Hi


2023. április 17., hétfő 8:30 keltezéssel, Jacopo Mondi <jacopo.mondi at ideasonboard.com> írta:

> [...]
> > diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
> > index 5317afbd..a63cd360 100644
> > --- a/src/libcamera/device_enumerator_udev.cpp
> > +++ b/src/libcamera/device_enumerator_udev.cpp
> > @@ -315,6 +315,7 @@ int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum)
> >  	 * enumerator.
> >  	 */
> >  	deps->deps_.erase(devnum);
> > +	devMap_.erase(it);
> 
> Shouldn't this be done only when all the dependencies are actually
> satisfied ?
> 

Every dependency has its own entry in `devMap_` with a pointer to
the corresponding `MediaDeviceDeps` object. I think when one dependency
appears it should be removed from the map, just like it is removed
from `MediaDeviceDeps::deps_` (in the previous line).


> >
> >  	if (deps->deps_.empty()) {
> 
> Likely inside this if() clause ?

`deps->deps_` is empty at this point, so we cannot efficiently find all
entries in `devMap_` that belong to `deps`.


> 
> Otherwise, if there still are dependencies to meet for the devnum at
> hand, the next time addV4L2Device(devnum) is called it won't find any item
> on the devMap_ for that key.

My reading of the code suggests that devnum is unique and each v4l2 device
can be a dependency of at most 1 media device. Is that not correct?


> 
> >  		LOG(DeviceEnumerator, Debug)
> > --
> > 2.40.0
> >
> >
> 


Regards,
Barnabás Pőcze