[libcamera-devel] [PATCH] libcamera: pipeline: vimc: Fail without an IPA

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Wed, Jun 16, 2021 at 10:56:10AM +0100, Kieran Bingham wrote:
> Registering a camera for VIMC without an IPA will fail later when
> attempting to configure.
> 
> The IPA is required for VIMC so fail early if it can't be loaded.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

Assuming this doesn't break any unit test, the change looks good.

> ---
>  src/libcamera/pipeline/vimc/vimc.cpp | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp
> index 9ebd723be171..8af0e92012e6 100644
> --- a/src/libcamera/pipeline/vimc/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc/vimc.cpp
> @@ -431,6 +431,7 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)
>  		data->ipa_->init(IPASettings{ conf, data->sensor_->model() });
>  	} else {
>  		LOG(VIMC, Warning) << "no matching IPA found";
> +		return false;
>  	}

I would have rewritten the code as

	if (!data->ipa_) {
		LOG(VIMC, Warning) << "no matching IPA found";
		return false;
	}

	std::string conf = data->ipa_->configurationFile("vimc.conf");
	data->ipa_->init(IPASettings{ conf, data->sensor_->model() });

and while at it, turned the message to an Error.

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

>  
>  	/* Create and register the camera. */

-- 
Regards,

Laurent Pinchart