[libcamera-devel] [libcamera-devel 1/5] pipeline: rkisp1: Support sensor tuning file

[Laurent Pinchart]

Hi Florian,

Thank you for the patch.

Nitpicking a bit on the naming, I'd write "Support IPA tuning file" in
the subject line, as the tuning isn't just about the camera sensor.

On Mon, May 23, 2022 at 11:24:31AM +0200, Florian Sylvestre via libcamera-devel wrote:
> Allow the usage of a Yaml configuration file that is either defined by

Likewise, here, you can write "YAML IPA tuning file".

> the sensor name (sensor_name.yaml) or defined by
> LIBCAMERA_RKISP1_TUNING_FILE environment variable.
> 
> Signed-off-by: Florian Sylvestre <fsylvestre at baylibre.com>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 4b3d2cf7..865c4ab0 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -322,7 +322,18 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)
>  	ipa_->paramsBufferReady.connect(this, &RkISP1CameraData::paramFilled);
>  	ipa_->metadataReady.connect(this, &RkISP1CameraData::metadataReady);
>  
> -	int ret = ipa_->init(IPASettings{ "", sensor_->model() }, hwRevision);
> +	/*
> +	 * The configuration (tuning file) is made from the sensor name unless

And "The IPA tuning file is made ...".

> +	 * the environment variable overrides it.
> +	 */
> +	std::string configurationFile;

I'll let you continue :-)

With this small issue addressed,

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

> +	char const *configFromEnv = utils::secure_getenv("LIBCAMERA_RKISP1_TUNING_FILE");
> +	if (!configFromEnv || *configFromEnv == '\0')
> +		configurationFile = ipa_->configurationFile(sensor_->model() + ".yaml");
> +	else
> +		configurationFile = std::string(configFromEnv);
> +
> +	int ret = ipa_->init({ configurationFile, sensor_->model() }, hwRevision);
>  	if (ret < 0) {
>  		LOG(RkISP1, Error) << "IPA initialization failure";
>  		return ret;

-- 
Regards,

Laurent Pinchart