[libcamera-devel] [PATCH v3 3/6] libcamera: pipeline_handler: uvcvideo: register all Cameras along with a devnum

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Mon, Dec 23, 2019 at 01:26:17AM -0600, Paul Elder wrote:
> Register all UVC Cameras along with its device number, to eventually

s/its device number/their device numbers/

> allow the V4L2 compatibility layer to match against it.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> 
> ---
> New in v3
> ---
>  src/libcamera/pipeline/uvcvideo.cpp | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 3043366b..096c1c09 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -7,6 +7,7 @@
>  
>  #include <algorithm>
>  #include <iomanip>
> +#include <sys/sysmacros.h>
>  #include <tuple>
>  
>  #include <libcamera/camera.h>
> @@ -294,17 +295,19 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
>  		return false;
>  
>  	std::unique_ptr<UVCCameraData> data = utils::make_unique<UVCCameraData>(this);
> +	dev_t devnum = 0;
>  
>  	/* Locate and initialise the camera data with the default video node. */
>  	for (MediaEntity *entity : media->entities()) {
>  		if (entity->flags() & MEDIA_ENT_FL_DEFAULT) {
>  			if (data->init(entity))
>  				return false;
> +			devnum = makedev(entity->deviceMajor(), entity->deviceMinor());

You can move this line after the loop, and merge it with the devnum
variable declaration. I would actually move it after the "Could not find
a default video device" check and make it a separate check with a
dedicated error message. Or even drop the check completely as it really
shouldn't happen (in which case you can move the devnum calculation to
the registration below).

With this fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

>  			break;
>  		}
>  	}
>  
> -	if (!data) {
> +	if (!data || !devnum) {
>  		LOG(UVC, Error) << "Could not find a default video device";
>  		return false;
>  	}
> @@ -312,7 +315,7 @@ bool PipelineHandlerUVC::match(DeviceEnumerator *enumerator)
>  	/* Create and register the camera. */
>  	std::set<Stream *> streams{ &data->stream_ };
>  	std::shared_ptr<Camera> camera = Camera::create(this, media->model(), streams);
> -	registerCamera(std::move(camera), std::move(data));
> +	registerCamera(std::move(camera), std::move(data), devnum);
>  
>  	/* Enable hot-unplug notifications. */
>  	hotplugMediaDevice(media);

-- 
Regards,

Laurent Pinchart