[PATCH v2 2/5] libcamera: software_isp: Honor black level in AWB

[Andrei Konovalov]

Hi Milan,

Thank you for the patch!

Reviewed-by: Andrei Konovalov <andrey.konovalov.ynk at gmail.com>

On 30.04.2024 20:34, Milan Zamazal wrote:
> The white balance computation didn't consider black level.  This is
> wrong because then the computed ratios are off when they are computed
> from the whole brightness range rather than the sensor range.
> 
> This patch adjusts white balance computation for the black level.  There
> is no need to change white balance application in debayering as this is
> already applied after black level correction.
> 
> Exposure computation already subtracts black level, no changes are
> needed there.
> 
> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
> ---
>   src/ipa/simple/soft_simple.cpp | 36 ++++++++++++++++++++++------------
>   1 file changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
> index b9fb58b5..f595abc2 100644
> --- a/src/ipa/simple/soft_simple.cpp
> +++ b/src/ipa/simple/soft_simple.cpp
> @@ -5,6 +5,8 @@
>    * soft_simple.cpp - Simple Software Image Processing Algorithm module
>    */
>   
> +#include <numeric>
> +#include <stdint.h>
>   #include <sys/mman.h>
>   
>   #include <linux/v4l2-controls.h>
> @@ -240,28 +242,36 @@ void IPASoftSimple::stop()
>   
>   void IPASoftSimple::processStats(const ControlList &sensorControls)
>   {
> +	SwIspStats::Histogram histogram = stats_->yHistogram;
> +	if (ignoreUpdates_ > 0)
> +		blackLevel_.update(histogram);
> +	const uint8_t blackLevel = blackLevel_.get();
> +	params_->blackLevel = blackLevel;
> +
>   	/*
>   	 * Calculate red and blue gains for AWB.
>   	 * Clamp max gain at 4.0, this also avoids 0 division.
> +	 * Black level must be subtracted to get the correct AWB ratios,
> +	 * from the sensor range.
>   	 */
> -	if (stats_->sumR_ <= stats_->sumG_ / 4)
> -		params_->gainR = 1024;
> -	else
> -		params_->gainR = 256 * stats_->sumG_ / stats_->sumR_;
> -
> -	if (stats_->sumB_ <= stats_->sumG_ / 4)
> -		params_->gainB = 1024;
> -	else
> -		params_->gainB = 256 * stats_->sumG_ / stats_->sumB_;
> +	const uint64_t nPixels = std::accumulate(
> +		histogram.begin(), histogram.end(), 0);
> +	auto subtractBlackLevel = [blackLevel, nPixels](
> +					  uint64_t sum, uint64_t div)
> +		-> uint64_t {
> +		return sum - blackLevel * nPixels / div;
> +	};
> +	const uint64_t sumR = subtractBlackLevel(stats_->sumR_, 4);
> +	const uint64_t sumG = subtractBlackLevel(stats_->sumG_, 2);
> +	const uint64_t sumB = subtractBlackLevel(stats_->sumB_, 4);
> +
> +	params_->gainR = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR;
> +	params_->gainB = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB;
>   
>   	/* Green gain and gamma values are fixed */
>   	params_->gainG = 256;
>   	params_->gamma = 0.5;
>   
> -	if (ignoreUpdates_ > 0)
> -		blackLevel_.update(stats_->yHistogram);
> -	params_->blackLevel = blackLevel_.get();
> -
>   	setIspParams.emit();
>   
>   	/* \todo Switch to the libipa/algorithm.h API someday. */