[PATCH v1 05/11] libipa: Add grey world AWB algorithm

[Dan Scally]

Hi Stefan

On 20/01/2025 13:52, Stefan Klug wrote:
> Hi Dan,
>
> Thank you for the review.
>
> On Mon, Jan 20, 2025 at 10:37:18AM +0000, Dan Scally wrote:
>> Hi Stefan
>>
>> On 09/01/2025 11:53, Stefan Klug wrote:
>>> Add the grey world algorithm that is currently used in rkisp1 to libipa.
>>> No changes in functionality were made.
>>>
>>> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
>>> ---
>>>    src/ipa/libipa/awb_grey.cpp | 114 ++++++++++++++++++++++++++++++++++++
>>>    src/ipa/libipa/awb_grey.h   |  35 +++++++++++
>>>    src/ipa/libipa/meson.build  |   2 +
>>>    3 files changed, 151 insertions(+)
>>>    create mode 100644 src/ipa/libipa/awb_grey.cpp
>>>    create mode 100644 src/ipa/libipa/awb_grey.h
>>>
>>> diff --git a/src/ipa/libipa/awb_grey.cpp b/src/ipa/libipa/awb_grey.cpp
>>> new file mode 100644
>>> index 000000000000..192a7cf3834a
>>> --- /dev/null
>>> +++ b/src/ipa/libipa/awb_grey.cpp
>>> @@ -0,0 +1,114 @@
>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>>> +/*
>>> + * Copyright (C) 2024 Ideas on Board Oy
>>> + *
>>> + * Base class for bayesian AWB algorithm
>>> + */
>>> +
>>> +#include "awb_grey.h"
>>> +
>>> +#include <cmath>
>>> +
>>> +#include <libcamera/base/log.h>
>>> +#include <libcamera/control_ids.h>
>>> +
>>> +#include "colours.h"
>>> +
>>> +using namespace libcamera::controls;
>>> +
>>> +/**
>>> + * \file awb_grey.h
>>> + * \brief Implementation of a grey world AWB algorithm
>>> + */
>>> +
>>> +namespace libcamera {
>>> +
>>> +LOG_DECLARE_CATEGORY(Awb)
>> Is it worth differentiating between this log category and that in awb.cpp -
>> which is also just "Awb"? The logs will include the file path and line
>> number of course so they can be distinguished anyway so probably it doesn't
>> matter, but I thought I'd mention it just to bring it to mind - I'm happy
>> either way.
> I was also quite undecided here. The reason I went for the short one, is
> that there will always be only one AWB algorithm active per camera. So
> there seems to be no need to further differentiate that at runtime.
That seems like a solid argument to me - also makes it easier to remember which to turn on I suppose.
>   But
> both options are valid. I think I'd like to stick to the current
> (simple) solution for now.
Alright - fine by me.
>
>>> +namespace ipa {
>>> +
>>> +/**
>>> + * \class AwbGrey
>>> + * \brief A Grey world auto white balance algorithm
>>> + */
>>> +
>>> +/**
>>> + * \brief Initialize the algorithm with the given tuning data
>>> + * \param[in] tuningData The tuning data for the algorithm
>>> + *
>>> + * Load the colour temperature curve from the tuning data. If there is no tuning
>>> + * data available, continue with a warning. Manual colour temperature will not
>>> + * work in that case.
>>> + *
>>> + * \return 0 on success, a negative error code otherwise
>>> + */
>>> +int AwbGrey::init(const YamlObject &tuningData)
>>> +{
>>> +	Interpolator<Vector<double, 2>> gains;
>>> +	int ret = gains.readYaml(tuningData["colourGains"], "ct", "gains");
>>> +	if (ret < 0)
>>> +		LOG(Awb, Warning)
>>> +			<< "Failed to parse 'colourGains' "
>>> +			<< "parameter from tuning file; "
>>> +			<< "manual colour temperature will not work properly";
>>> +	else
>>> +		colourGainCurve_ = gains;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +/**
>>> + * \brief Calculate awb data from the given statistics
>>> + * \param[in] stats The statistics to use for the calculation
>>> + * \param[in] lux The lux value of the scene
>>> + *
>>> + * Estimates the colour temperature based on the coulours::estimateCCT function.
>> s/coulours/colours.
>>> + * The gains are calculated purely based on the RGB means provided by the \a
>>> + * stats. The colour temperature is not taken into account when calculating the
>>> + * gains.
>>> + *
>>> + * The \a lux parameter is not used in this algorithm.
>>> + *
>>> + * \return The awb result
>>> + */
>>> +AwbResult AwbGrey::calculateAwb(const AwbStats &stats, [[maybe_unused]] int lux)
>>> +{
>>> +	AwbResult result;
>>> +	auto means = stats.getRGBMeans();
>>> +	result.colourTemperature = estimateCCT(means);
>>> +
>>> +	/*
>>> +	 * Estimate the red and blue gains to apply in a grey world. The green
>>> +	 * gain is hardcoded to 1.0. Avoid divisions by zero by clamping the
>>> +	 * divisor to a minimum value of 1.0.
>>> +	 */
>>> +	result.gains.r() = means.g() / std::max(means.r(), 1.0);
>>> +	result.gains.g() = 1.0;
>>> +	result.gains.b() = means.g() / std::max(means.b(), 1.0);
>>> +	return result;
>>> +}
>>> +
>>> +/**
>>> + * \brief Compute white balance gains from a colour temperature
>>> + * \param[in] colourTemperature The colour temperature in Kelvin
>>> + *
>>> + * Compute the white balance gains from a \a colourTemperature. This function
>>> + * does not take any statistics into account. It simply interpolates the colour
>>> + * gains configured in the colour temperature curve.
>>> + *
>>> + * \return The colour gains if a colour temperature curve is available,
>>> + * [1, 1, 1] otherwise.
>>> + */
>>> +RGB<double> AwbGrey::gainsFromColourTemperature(double colourTemperature)
>>> +{
>>> +	if (!colourGainCurve_) {
>>> +		LOG(Awb, Error) << "No gains defined";
>>> +		return RGB<double>({ 1.0, 1.0, 1.0 });
>>> +	}
>>> +
>>> +	auto gains = colourGainCurve_->getInterpolated(colourTemperature);
>>> +	return { { gains[0], 1.0, gains[0] } };
>>> +}
>>> +
>>> +} /* namespace ipa */
>>> +
>>> +} /* namespace libcamera */
>>> diff --git a/src/ipa/libipa/awb_grey.h b/src/ipa/libipa/awb_grey.h
>>> new file mode 100644
>>> index 000000000000..6eda8e5498fb
>>> --- /dev/null
>>> +++ b/src/ipa/libipa/awb_grey.h
>>> @@ -0,0 +1,35 @@
>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>>> +/*
>>> + * Copyright (C) 2024 Ideas on Board Oy
>>> + *
>>> + * AWB grey world algorithm
>>> + */
>>> +
>>> +#pragma once
>>> +
>>> +#include "libcamera/internal/yaml_parser.h"
>>> +
>>> +#include "awb.h"
>>> +#include "interpolator.h"
>>> +#include "vector.h"
>>> +
>>> +namespace libcamera {
>>> +
>>> +namespace ipa {
>>> +
>>> +class AwbGrey : public AwbAlgorithm
>>> +{
>>> +public:
>>> +	AwbGrey() = default;
>>> +
>>> +	int init(const YamlObject &tuningData) override;
>>> +	AwbResult calculateAwb(const AwbStats &stats, int lux) override;
>>> +	RGB<double> gainsFromColourTemperature(double coulourTemperature) override;
>> s/coulourTemperature/colourTemperature
> hmpf..
>
>>
>> Otherwise looks good I think:
>>
>>
>> Reviewed-by: Daniel Scally <dan.scally at ideasonboard.com>
> Thanks,
> Stefan
>
>>> +
>>> +private:
>>> +	std::optional<Interpolator<Vector<double, 2>>> colourGainCurve_;
>>> +};
>>> +
>>> +} /* namespace ipa */
>>> +
>>> +} /* namespace libcamera */
>>> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build
>>> index 03e879c5834f..c550a6eb45b6 100644
>>> --- a/src/ipa/libipa/meson.build
>>> +++ b/src/ipa/libipa/meson.build
>>> @@ -3,6 +3,7 @@
>>>    libipa_headers = files([
>>>        'agc_mean_luminance.h',
>>>        'algorithm.h',
>>> +    'awb_grey.h',
>>>        'awb.h',
>>>        'camera_sensor_helper.h',
>>>        'colours.h',
>>> @@ -21,6 +22,7 @@ libipa_headers = files([
>>>    libipa_sources = files([
>>>        'agc_mean_luminance.cpp',
>>>        'algorithm.cpp',
>>> +    'awb_grey.cpp',
>>>        'awb.cpp',
>>>        'camera_sensor_helper.cpp',
>>>        'colours.cpp',