[libcamera-devel] [PATCH 2/2] libcamera: device_enumerator_udev: Don't add media device twice

[Niklas Söderlund]

Hi Laurent,

Thanks for your patch.

On 2020-03-20 23:04:35 +0200, Laurent Pinchart wrote:
> Commit 68daa9302f90 ("libcamera: device_enumerator: Don't stop if one
> device fails") introduced a bug in device enumeration. Previously, a
> media device would only be added when all its dependencies were present,
> as indicated by populateMediaDevice() returning 0. The commit changed
> the logic to propagate the populateMediaDevice() return code up, but
> mistakenly added the media device to the enumerator even when
> dependencies were missing. This causes media devices enumerated with
> missing dependencies to be added twice, once when enumerating the media
> device itself, and a second time when all the dependencies are found.
> 
> Fix it by deferring addition of the media device when dependencies are
> missing, and add debug messages to track this process.
> 
> Fixes: 68daa9302f90 ("libcamera: device_enumerator: Don't stop if one device fails")
> Reported-by: Andrey Konovalov <andrey.konovalov at linaro.org>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

> ---
>  src/libcamera/device_enumerator_udev.cpp | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
> index 87638c59761c..e55350544feb 100644
> --- a/src/libcamera/device_enumerator_udev.cpp
> +++ b/src/libcamera/device_enumerator_udev.cpp
> @@ -90,6 +90,13 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
>  			return ret;
>  		}
>  
> +		if (ret) {
> +			LOG(DeviceEnumerator, Debug)
> +				<< "Defer media device " << media->deviceNode()
> +				<< " due to " << ret << " missing dependencies";
> +			return 0;
> +		}
> +
>  		addDevice(media);
>  		return 0;
>  	}
> @@ -313,6 +320,9 @@ int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum)
>  	deps->deps_.erase(devnum);
>  
>  	if (deps->deps_.empty()) {
> +		LOG(DeviceEnumerator, Debug)
> +			<< "All dependencies for media device "
> +			<< deps->media_->deviceNode() << " found";
>  		addDevice(deps->media_);
>  		pending_.remove(*deps);
>  	}
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund