[libcamera-devel] [PATCH v1 3/6] ipa: rkisp1: Introduce Black Level Correction

[Laurent Pinchart]

Hi Jean-Michel,

Thank you for the patch.

On Thu, Dec 02, 2021 at 07:04:07PM +0100, Jean-Michel Hautbois wrote:
> In order to have the proper pixel levels, apply a fixed black level
> correction, based on the imx219 tuning file in RPi. The value is 4096 on
> 16 bits, and the pipeline for RkISP1 is on 12 bits, scale it.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> ---
>  src/ipa/rkisp1/algorithms/blc.cpp     | 55 +++++++++++++++++++++++++++
>  src/ipa/rkisp1/algorithms/blc.h       | 30 +++++++++++++++
>  src/ipa/rkisp1/algorithms/meson.build |  1 +
>  src/ipa/rkisp1/rkisp1.cpp             |  2 +
>  4 files changed, 88 insertions(+)
>  create mode 100644 src/ipa/rkisp1/algorithms/blc.cpp
>  create mode 100644 src/ipa/rkisp1/algorithms/blc.h
> 
> diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp
> new file mode 100644
> index 000000000..f27a8e439
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/blc.cpp
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Ideas On Board
> + *
> + * blc.cpp - RkISP1 Black Level Correction control
> + */
> +
> +#include "blc.h"
> +
> +/**
> + * \file blc.h
> + */
> +
> +namespace libcamera {
> +
> +namespace ipa::rkisp1::algorithms {
> +
> +/**
> + * \class BlackLevelCorrection
> + * \brief RkISP1 Black Level Correction control
> + *
> + * The pixels output by the camera normally include a black level, because
> + * sensors do not always report a signal level of '0' for black. Pixels at or
> + * below this level should be considered black. To achieve that, the RkISP BLC
> + * algorithm subtracts a configurable offset from all pixels.
> + *
> + * The black level can be measured at runtime from an optical dark region of the
> + * camera sensor, or measured during the camera tuning process. The first option
> + * isn't currently supported.
> + */
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::prepare
> + */
> +void BlackLevelCorrection::prepare([[maybe_unused]] IPAContext &context,
> +				   rkisp1_params_cfg *params)
> +{
> +	/*
> +	 * Substract fixed values taken from imx219 tuning file.
> +	 * \todo Use a configuration file for it ?
> +	 */
> +	params->others.bls_config.enable_auto = 0;
> +	params->others.bls_config.fixed_val.r = 256;
> +	params->others.bls_config.fixed_val.gr = 256;
> +	params->others.bls_config.fixed_val.gb = 256;
> +	params->others.bls_config.fixed_val.b = 256;
> +
> +	params->module_en_update |= RKISP1_CIF_ISP_MODULE_BLS;
> +	params->module_ens |= RKISP1_CIF_ISP_MODULE_BLS;
> +	params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_BLS;

Same as previously, if the values are fixed, we shouldn't change them
for every frame.

Apart from that, this looks good.

> +}
> +
> +} /* namespace ipa::rkisp1::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/algorithms/blc.h b/src/ipa/rkisp1/algorithms/blc.h
> new file mode 100644
> index 000000000..331a22097
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/blc.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Ideas On Board
> + *
> + * blc.h - RkISP1 Black Level Correction control
> + */
> +
> +#pragma once
> +
> +#include <linux/rkisp1-config.h>
> +
> +#include "algorithm.h"
> +
> +namespace libcamera {
> +
> +struct IPACameraSensorInfo;
> +
> +namespace ipa::rkisp1::algorithms {
> +
> +class BlackLevelCorrection : public Algorithm
> +{
> +public:
> +	BlackLevelCorrection() = default;
> +	~BlackLevelCorrection() = default;
> +
> +	void prepare(IPAContext &context, rkisp1_params_cfg *params) override;
> +};
> +
> +} /* namespace ipa::rkisp1::algorithms */
> +} /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build
> index 0678518d8..e6767aa5f 100644
> --- a/src/ipa/rkisp1/algorithms/meson.build
> +++ b/src/ipa/rkisp1/algorithms/meson.build
> @@ -2,5 +2,6 @@
>  
>  rkisp1_ipa_algorithms = files([
>      'agc.cpp',
> +    'blc.cpp',
>      'sdg.cpp',
>  ])
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index cd425a2e1..ec879ba37 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -27,6 +27,7 @@
>  
>  #include "algorithms/agc.h"
>  #include "algorithms/algorithm.h"
> +#include "algorithms/blc.h"
>  #include "algorithms/sdg.h"
>  #include "libipa/camera_sensor_helper.h"
>  
> @@ -126,6 +127,7 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)
>  
>  	/* Construct our Algorithms */
>  	algorithms_.push_back(std::make_unique<algorithms::Agc>());
> +	algorithms_.push_back(std::make_unique<algorithms::BlackLevelCorrection>());
>  	algorithms_.push_back(std::make_unique<algorithms::SensorDeGamma>());
>  
>  	return 0;

-- 
Regards,

Laurent Pinchart