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

[Jacopo Mondi]

Hi Laurent,

On Wed, Oct 21, 2020 at 03:24:16AM +0300, 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>
> ---
>  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;
> +

Intentional empty line ?

>  	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;

So the actual camera matching happens in data->init() here ?

Anyway, looks good
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

>  	}
>
> -	return true;
> +	return registered;


>  }
>
>  V4L2VideoDevice *SimplePipelineHandler::video(const MediaEntity *entity)
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel