[PATCH v1 03/11] libipa: Add AWB algorithm base class

[Stefan Klug]

Hi Dan,

Thank you for the review. 

On Fri, Jan 17, 2025 at 02:30:29PM +0000, Dan Scally wrote:
> Hi Stefan, thanks for the cool set
> 
> On 09/01/2025 11:53, Stefan Klug wrote:
> > Add a class to provide a generic interface for auto white balance
> > algorithms. Concrete AWB algorithms are expected to subclass the
> > AwbAlgorithm class to implement their functionality.
> > 
> > Pipeline handlers are expected to subclass the AwbStats class and
> > implement the necessary function to give the algorithm access to the
> > hardware specific statistics data.
> 
> 
> That sounds strange; I'd expect the IPA to handle that rather than the PipelineHandler
> 
> > 
> > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> > ---
> >   src/ipa/libipa/awb.cpp     | 137 +++++++++++++++++++++++++++++++++++++
> >   src/ipa/libipa/awb.h       |  51 ++++++++++++++
> >   src/ipa/libipa/meson.build |   2 +
> >   3 files changed, 190 insertions(+)
> >   create mode 100644 src/ipa/libipa/awb.cpp
> >   create mode 100644 src/ipa/libipa/awb.h
> > 
> > diff --git a/src/ipa/libipa/awb.cpp b/src/ipa/libipa/awb.cpp
> > new file mode 100644
> > index 000000000000..74e88d513b27
> > --- /dev/null
> > +++ b/src/ipa/libipa/awb.cpp
> > @@ -0,0 +1,137 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2024 Ideas on Board Oy
> > + *
> > + * Generic AWB algorithms
> > + */
> > +
> > +#include "awb.h"
> > +
> > +#include <libcamera/base/log.h>
> > +
> > +/**
> > + * \file awb.h
> > + * \brief Base classes for AWB algorithms
> > + */
> > +
> > +namespace libcamera {
> > +
> > +LOG_DEFINE_CATEGORY(Awb)
> > +
> > +namespace ipa {
> > +
> > +/**
> > + * \class AwbResult
> > + * \brief The result of an awb calculation
> > + *
> > + * This class holds the result of an auto white balance calculation.
> > + */
> > +
> > +/**
> > + * \var AwbResult::gains
> > + * \brief The calculated white balance gains
> > + */
> > +
> > +/**
> > + * \var AwbResult::colourTemperature
> > + * \brief The calculated colour temperature in Kelvin
> > + */
> > +
> > +/**
> > + * \class AwbStats
> > + * \brief A abstract class wrapping system specific AWB statistics
> s/A/An here, and I think I'd say "An abstraction class wrapping hardware
> specific..." but that's just bikeshedding so feel free to ignore it.

Oh thanks, that sounds better.

> > + *
> > + * Pipline handlers using an AWB algorithm based on the AwbAlgorithm class need
> > + * to implement this class to give the algorithm access to the hardware specific
> > + * statics data.
> s/statics/statistics
> > + */
> > +
> > +/**
> > + * \fn AwbStats::computeColourError
> > + * \brief Compute an error value for when the given gains would be applied
> > + * \param[in] gains The gains to apply
> > + *
> > + * Compute an error value (non-greyness) assuming the given \a gains would be
> > + * applied. To keep the actual implementations computationally inexpensive,
> > + * the squared colour error shall be returned.
> > + *
> > + * If the awb statistics provide multiple zones, the sum over all zones needs to
> > + * calculated.
> > + *
> > + * \return The computed error value
> > + */
> > +
> > +/**
> > + * \fn AwbStats::getRGBMeans
> > + * \brief Get RGB means of the statistics
> > + *
> > + * Fetch the RGB means from the statistics. The values of each channel are
> > + * dimensionless and only the ratios are used for further calculations. This is
> > + * used by the simple gray world model to calculate the gains to apply.
> > + *
> > + * \return The RGB means
> > + */
> > +
> > +/**
> > + * \class AwbAlgorithm
> > + * \brief A base class for auto white balance algorithms
> > + *
> > + * This class is a base class for auto white balance algorithms. It provides an
> > + * interface for the algorithms to implement, and is used by the pipeline
> > + * handler to interact with the concrete implementation.
> 
> I think s/provides/defines, and it should be used by the IPA rather than the pipeline handler.

definitely :-)

> 
> 
> Otherwise, looks good:
> 
> 
> Reviewed-by: Daniel Scally <dan.scally at ideasonboard.com>

Thanks,
Stefan

> 
> > + */
> > +
> > +/**
> > + * \fn AwbAlgorithm::init
> > + * \brief Initialize the algorithm with the given tuning data
> > + * \param[in] tuningData The tuning data to use for the algorithm
> > + *
> > + * \return 0 on success, a negative error code otherwise
> > + */
> > +
> > +/**
> > + * \fn AwbAlgorithm::calculateAwb
> > + * \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
> > + *
> > + * Calculate a AwbResult object from the given statistics and lux value. A \a
> > + * lux value of 0 means it is unknown or invalid and the algorithm shall ignore
> > + * it.
> > + *
> > + * \return The awb result
> > + */
> > +
> > +/**
> > + * \fn AwbAlgorithm::gainsFromColourTemperature
> > + * \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 is used to compute the colour
> > + * gains when the user manually specifies a colour temperature.
> > + *
> > + * \return The colour gains
> > + */
> > +
> > +/**
> > + * \fn AwbAlgorithm::controls
> > + * \brief Get the controls info map for this algorithm
> > + *
> > + * \return The controls info map
> > + */
> > +
> > +/**
> > + * \fn AwbAlgorithm::handleControls
> > + * \param[in] controls The controls to handle
> > + * \brief Handle the controls supplied in a request
> > + */
> > +
> > +/**
> > + * \var AwbAlgorithm::controls_
> > + * \brief Controls info map for the controls provided by the algorithm
> > + */
> > +
> > +} /* namespace ipa */
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/ipa/libipa/awb.h b/src/ipa/libipa/awb.h
> > new file mode 100644
> > index 000000000000..2dd471606ec4
> > --- /dev/null
> > +++ b/src/ipa/libipa/awb.h
> > @@ -0,0 +1,51 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2024 Ideas on Board Oy
> > + *
> > + * Generic AWB algorithms
> > + */
> > +
> > +#pragma once
> > +
> > +#include <libcamera/controls.h>
> > +#include "libcamera/internal/yaml_parser.h"
> > +
> > +#include "vector.h"
> > +
> > +namespace libcamera {
> > +
> > +namespace ipa {
> > +
> > +struct AwbResult {
> > +	RGB<double> gains;
> > +	double colourTemperature;
> > +};
> > +
> > +struct AwbStats {
> > +	virtual double computeColourError(const RGB<double> &gains) const = 0;
> > +	virtual RGB<double> getRGBMeans() const = 0;
> > +};
> > +
> > +class AwbAlgorithm
> > +{
> > +public:
> > +	virtual ~AwbAlgorithm() = default;
> > +
> > +	virtual int init(const YamlObject &tuningData) = 0;
> > +	virtual AwbResult calculateAwb(const AwbStats &stats, int lux) = 0;
> > +	virtual RGB<double> gainsFromColourTemperature(double colourTemperature) = 0;
> > +
> > +	const ControlInfoMap::Map &controls() const
> > +	{
> > +		return controls_;
> > +	}
> > +
> > +	virtual void handleControls([[maybe_unused]] const ControlList &controls) {}
> > +
> > +protected:
> > +	ControlInfoMap::Map controls_;
> > +};
> > +
> > +} /* namespace ipa */
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build
> > index f2b2f4be50db..03e879c5834f 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.h',
> >       'camera_sensor_helper.h',
> >       'colours.h',
> >       'exposure_mode_helper.h',
> > @@ -20,6 +21,7 @@ libipa_headers = files([
> >   libipa_sources = files([
> >       'agc_mean_luminance.cpp',
> >       'algorithm.cpp',
> > +    'awb.cpp',
> >       'camera_sensor_helper.cpp',
> >       'colours.cpp',
> >       'exposure_mode_helper.cpp',