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

[Andrey Konovalov]

Hi Laurent,

Thanks for looking into this!

On 21.10.2020 03:24, 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.

I've faced this endless loop once when in the libcamera sources I commented out
all the formats supported by the sensor - thus making data->init() to fail.

Your patch does fix this issue - has tested it with simple pipeline handler.

Tested-by: Andrey Konovalov <andrey.konovalov at linaro.org>

Thanks,
Andrey

> Signed-off-by: Laurent Pinchart <laurent.pinchart 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)
>