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

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Mon, Dec 30, 2019 at 11:33:13PM -0600, Paul Elder wrote:
> Register all UVC Cameras along with their device numbers, to eventually
> allow the V4L2 compatibility layer to match against it.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> ---
> No change in v4
> 
> 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());
>  			break;
>  		}
>  	}
>  
> -	if (!data) {
> +	if (!data || !devnum) {

This actually fixes a bug in the UVC pipeline handler. I've just sent a
fix for that ("[PATCH] libcamera: pipeline: uvcvideo: Fix crash when
default entity is not found"), could you review it and rebase this
series on top ?

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

>  		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