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

[Paul Elder]

On Thu, Jan 09, 2025 at 12:53:54PM +0100, 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.
> 
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>

The code/design looks great. Just some english comments...

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

I think "An abstract class that wraps system-specific AWB statistics"
might be better.

> + *
> + * Pipline handlers using an AWB algorithm based on the AwbAlgorithm class need

s/Pipline/Pipeline/

> + * to implement this class to give the algorithm access to the hardware specific

s/hardware specific/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 thought the IPAs interact with the algos and not the pipeline
handler... (unless there's a surprise in later patches that I haven't
seen yet).

> + */
> +
> +/**
> + * \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

s/ a / an /

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

A bit of bikeshedding perhaps but personally I'd prefer "processControls". Up to you though :)


Thanks,

Paul

> + * \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',
> -- 
> 2.43.0
>