[libcamera-devel] [PATCH] libcamera: pipeline: Fail match() when no camera is registered

[Kieran Bingham]

Hi Laurent,

On 20/10/2020 20:51, Laurent Pinchart wrote:
> The rkisp1 and simple pipeline handlers can fail to register any camera,
> if initialization of all the detected cameras fail. In that case, they
> still return success from their match function. As no camera gets
> registered, the pipeline handler is immediately destroyed, releasing the
> acquired media devices, and the camera manager immediately tries to
> match the same pipeline handler with the same media device, causing an
> endless loop.
> 
> Fix it by returning false from the match function if no camera gets
> registered.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 ++++++---
>  src/libcamera/pipeline/simple/simple.cpp | 5 ++++-
>  2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 2352ab3b234a..c74a2e9bd548 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -1107,10 +1107,13 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
>  	if (!pad)
>  		return false;
>  
> -	for (MediaLink *link : pad->links())
> -		createCamera(link->source()->entity());
> +	bool registered = false;
> +	for (MediaLink *link : pad->links()) {
> +		if (!createCamera(link->source()->entity()))
> +			registered = true;
> +	}
>  
> -	return true;
> +	return registered;
>  }
>  
>  /* -----------------------------------------------------------------------------
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 8c00c0ffc121..8868a43beeb4 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -821,6 +821,8 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
>  	}
>  
>  	/* Initialize each pipeline and register a corresponding camera. */
> +	bool registered = false;
> +
>  	for (std::unique_ptr<SimpleCameraData> &data : pipelines) {
>  		int ret = data->init();
>  		if (ret < 0)
> @@ -830,9 +832,10 @@ bool SimplePipelineHandler::match(DeviceEnumerator *enumerator)
>  			Camera::create(this, data->sensor_->id(),
>  				       data->streams());
>  		registerCamera(std::move(camera), std::move(data));
> +		registered = true;
>  	}
>  
> -	return true;
> +	return registered;
>  }
>  
>  V4L2VideoDevice *SimplePipelineHandler::video(const MediaEntity *entity)
> 

-- 
Regards
--
Kieran