[libcamera-devel] [PATCH v2 10/12] ipa: ipu3: awb: Introduce Black Level Correction

[Laurent Pinchart]

Hi Jean-Michel,

Thank you for the patch.

On Thu, Sep 30, 2021 at 11:37:13AM +0200, Jean-Michel Hautbois wrote:
> 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 and to achieve that, we
> need to substract an offset to all the pixels. This can be taken into
> account by reading the lowest value of a special region on sensors which
> is not exposed to light. This provides a substracting factor to be
> able to adjust the expected black levels in the resultant images.

s/resultant/resulting/

> For a camera outputting 10-bit pixel values (in the range 0 to 1023) a
> typical black level might be 64. It is a fixed value, obtained by
> capturing a raw frame with minimum exposure and gain fixed to 1.0 while
> covering the sensor (the darker the better). We consider it good enough
> as a very first approximation, until we measure it during a tuning
> process and include it in a configuration file
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  src/ipa/ipu3/algorithms/black_correction.cpp | 67 ++++++++++++++++++++
>  src/ipa/ipu3/algorithms/black_correction.h   | 28 ++++++++

As you've correctly mentioned in the subject, this is called black level
correction (or sometimes compensation). The file name would get a bit
long, so we could name it blc.cpp and blc.h. Same for the class, but I
wouldn't mind a BlackLevelCorrection class.

>  src/ipa/ipu3/algorithms/meson.build          |  1 +
>  src/ipa/ipu3/ipu3.cpp                        |  2 +
>  4 files changed, 98 insertions(+)
>  create mode 100644 src/ipa/ipu3/algorithms/black_correction.cpp
>  create mode 100644 src/ipa/ipu3/algorithms/black_correction.h
> 
> diff --git a/src/ipa/ipu3/algorithms/black_correction.cpp b/src/ipa/ipu3/algorithms/black_correction.cpp
> new file mode 100644
> index 00000000..5b5b5d31
> --- /dev/null
> +++ b/src/ipa/ipu3/algorithms/black_correction.cpp
> @@ -0,0 +1,67 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google inc.
> + *
> + * black_correction.cpp - IPU3 Optical Black Correction control

s/Optical Black Correction/Black Level Correction/

Same below. "optical black" (or optical dark) is the name of the sensor
region not exposed to light, but the algorithm is named "black level
correction".

> + */
> +
> +#include "black_correction.h"
> +
> +#include <string.h>
> +
> +/**
> + * \file black_correction.h
> + */
> +
> +namespace libcamera {
> +
> +namespace ipa::ipu3::algorithms {
> +
> +/**
> + * \class BlackCorrection
> + * \brief A class to handle optical black correction
> + *
> + * 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 and to achieve that, we need to
> + * substract an offset to all the pixels.

 * below this level should be considered black. To achieve that, the ImgU BLC
 * algorithm subtracts a configurable offset from all pixels.

> + *
> + * This can be taken into account by reading the lowest value of a special
> + * region on sensors which is not exposed to light. This provides a substracting
> + * factor to be able to adjust the expected black levels in the resultant
> + * images.

 * 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.

> + */
> +
> +BlackCorrection::BlackCorrection()
> +{
> +}
> +
> +/**
> + * \brief Fill in the parameter structure, and enable optical black correction
> + * \param context The shared IPA context
> + * \param params The IPU3 parameters
> + *
> + * Populate the IPU3 parameter structure with the correction values for each
> + * channel and enable the corresponding ImgU block processing.
> + */
> +void BlackCorrection::prepare([[maybe_unused]] IPAContext &context,
> +			      ipu3_uapi_params *params)
> +{
> +	/*
> +	 * The Optical Black Level correction values
> +	 * \todo The correction values should come from sensor specific
> +	 * tuning processes. This is a first approximation

s/approximation/rough approximation./

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

> +	 */
> +	params->obgrid_param.gr = 64;
> +	params->obgrid_param.r  = 64;
> +	params->obgrid_param.b  = 64;
> +	params->obgrid_param.gb = 64;
> +
> +	/* Enable the custom optical black correction processing */
> +	params->use.obgrid = 1;
> +	params->use.obgrid_param = 1;
> +}
> +
> +} /* namespace ipa::ipu3::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/ipu3/algorithms/black_correction.h b/src/ipa/ipu3/algorithms/black_correction.h
> new file mode 100644
> index 00000000..6486ac5b
> --- /dev/null
> +++ b/src/ipa/ipu3/algorithms/black_correction.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google inc.
> + *
> + * black_correction.h - IPU3 Optical Black Correction control
> + */
> +#ifndef __LIBCAMERA_IPU3_ALGORITHMS_BLACK_CORRECTION_H__
> +#define __LIBCAMERA_IPU3_ALGORITHMS_BLACK_CORRECTION_H__
> +
> +#include "algorithm.h"
> +
> +namespace libcamera {
> +
> +namespace ipa::ipu3::algorithms {
> +
> +class BlackCorrection : public Algorithm
> +{
> +public:
> +	BlackCorrection();
> +
> +	void prepare(IPAContext &context, ipu3_uapi_params *params) override;
> +};
> +
> +} /* namespace ipa::ipu3::algorithms */
> +
> +} /* namespace libcamera */
> +
> +#endif /* __LIBCAMERA_IPU3_ALGORITHMS_BLACK_CORRECTION_H__ */
> diff --git a/src/ipa/ipu3/algorithms/meson.build b/src/ipa/ipu3/algorithms/meson.build
> index deae225b..66877c1b 100644
> --- a/src/ipa/ipu3/algorithms/meson.build
> +++ b/src/ipa/ipu3/algorithms/meson.build
> @@ -4,5 +4,6 @@ ipu3_ipa_algorithms = files([
>      'agc.cpp',
>      'algorithm.cpp',
>      'awb.cpp',
> +    'black_correction.cpp',
>      'tone_mapping.cpp',
>  ])
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index b33e8c77..7cc0fb89 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -33,6 +33,7 @@
>  #include "algorithms/agc.h"
>  #include "algorithms/algorithm.h"
>  #include "algorithms/awb.h"
> +#include "algorithms/black_correction.h"
>  #include "algorithms/tone_mapping.h"
>  #include "libipa/camera_sensor_helper.h"
>  
> @@ -279,6 +280,7 @@ int IPAIPU3::init(const IPASettings &settings,
>  	/* Construct our Algorithms */
>  	algorithms_.push_back(std::make_unique<algorithms::Agc>());
>  	algorithms_.push_back(std::make_unique<algorithms::Awb>());
> +	algorithms_.push_back(std::make_unique<algorithms::BlackCorrection>());
>  	algorithms_.push_back(std::make_unique<algorithms::ToneMapping>());
>  
>  	return 0;

-- 
Regards,

Laurent Pinchart