[libcamera-devel] [PATCH v2 05/14] libcamera: pipeline: Add callback to initialize IPA

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Fri, Aug 30, 2019 at 01:26:44AM +0200, Niklas Söderlund wrote:
> Add a callback so the IPA can be initialized before a camera is exposed
> to an application.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/include/pipeline_handler.h |  2 ++
>  src/libcamera/pipeline_handler.cpp       | 20 ++++++++++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> index 91d40ef40a465c4e..ca40e7ef2b85f372 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -43,6 +43,8 @@ public:
>  	}
>  	virtual ~CameraData() {}
>  
> +	virtual int initIPA() { return 0; };
> +
>  	Camera *camera_;
>  	PipelineHandler *pipe_;
>  	std::list<Request *> queuedRequests_;
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 846272485c7d2fc0..613751619a398968 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -66,6 +66,20 @@ LOG_DEFINE_CATEGORY(Pipeline)
>   * is needed for the camera both parameters should be set to 0.
>   */
>  
> +/**
> + * \fn CameraData::initIPA()
> + * \brief Callback to initialize the IPA
> + *
> + * This callback is called once a IPA have been associated with the camera.
> + * It allows the CameraData to preform initialization of the IPA before the
> + * camera is registered in the system.
> + *
> + * If this callback returns an error, the camera it represents will not be
> + * registered in the system.
> + *
> + * \return 0 on success or a negative error code otherwise
> + */

This calls for an explicit loadIPA() method as proposed in the review of
03/14, to let the pipeline handler initialise the IPA before registering
the camera. Furthermore, looking at the patch that uses this (14/14),
it's as much about initialising the IPA than the pipeline handler, so
the method isn't well named. Dropping it will avoid having to find a
good name :-)

> +
>  /**
>   * \var CameraData::camera_
>   * \brief The camera related to this CameraData instance
> @@ -461,6 +475,12 @@ void PipelineHandler::registerCamera(std::shared_ptr<Camera> camera,
>  					       << " no IPA found";
>  			return;
>  		}
> +
> +		if (data->initIPA()) {
> +			LOG(Pipeline, Warning) << "Skipping " << camera->name()
> +				<< " initialization of camera data failed";
> +			return;
> +		}
>  	}
>  
>  	cameraData_[camera.get()] = std::move(data);

-- 
Regards,

Laurent Pinchart