[PATCH v1 04/11] libipa: awb: Add helper functions for AWB mode support

[Dan Scally]

Hi Stefan


On 09/01/2025 11:53, Stefan Klug wrote:
> The AWB modes are specified in the libcamera core controls. It is
> therefore quite likely that every AWB algorithm will implement them. Add
> helper functions for parsing and storing the configured modes and the
> currently selected mode to the AwbAlgorithm base class.


I think that this can just be squashed into the last patch.

> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> ---
>   src/ipa/libipa/awb.cpp | 100 +++++++++++++++++++++++++++++++++++++++++
>   src/ipa/libipa/awb.h   |  12 +++++
>   2 files changed, 112 insertions(+)
>
> diff --git a/src/ipa/libipa/awb.cpp b/src/ipa/libipa/awb.cpp
> index 74e88d513b27..07502da66f73 100644
> --- a/src/ipa/libipa/awb.cpp
> +++ b/src/ipa/libipa/awb.cpp
> @@ -9,6 +9,8 @@
>   
>   #include <libcamera/base/log.h>
>   
> +#include <libcamera/control_ids.h>
> +
>   /**
>    * \file awb.h
>    * \brief Base classes for AWB algorithms
> @@ -132,6 +134,104 @@ namespace ipa {
>    * \brief Controls info map for the controls provided by the algorithm
>    */
>   
> +/**
> + * \var AwbAlgorithm::modes_
> + * \brief Map of all configured modes
> + * \sa AwbAlgorithm::parseModeConfigs
> + */
> +
> +/**
> + * \class AwbAlgorithm::ModeConfig
> + * \brief Holds the configuration of a single AWB mode
> + *
> + * Awb modes limit the regulation of the AWB algorithm to a specific range of
> + * colour temperatures.
> + */
> +
> +/**
> + * \var AwbAlgorithm::ModeConfig::ctLo
> + * \brief The lowest valid colour temperature of that mode
> + */
> +
> +/**
> + * \var AwbAlgorithm::ModeConfig::ctHi
> + * \brief The highest valid colour temperature of that mode
> + */
> +
> +/**
> + * \brief Parse the mode configurations from the tuning data
> + * \param[in] tuningData the YamlObject representing the tuning data
> + *
> + * Utility function to parse the tuning data for a AwbMode entry and read all
> + * provided modes. It populetes AwbAlgorithm::controls_, AwbAlgorithm::modes_
s/populetes/populates
> + * and sets the current mode to AwbAuto.

What if AwbAuto isn't one of the modes defined? Ah, I see it fails. Perhaps that is worth a mention. 
I think it's also probably worth an example of the expected tuning file format like in 
AgcMeanLuminance::parseTuningData().


Reviewed-by: Daniel Scally <dan.scally at ideasonboard.com>

> + *
> + * \return Zero on success, negative error code otherwise
> + */
> +int AwbAlgorithm::parseModeConfigs(const YamlObject &tuningData)
> +{
> +	std::vector<ControlValue> availableModes;
> +
> +	const YamlObject &yamlModes = tuningData[controls::AwbMode.name()];
> +	if (!yamlModes.isDictionary()) {
> +		LOG(Awb, Error)
> +			<< "AwbModes must be a dictionary.";
> +		return -EINVAL;
> +	}
> +
> +	for (const auto &[modeName, modeDict] : yamlModes.asDict()) {
> +		if (controls::AwbModeNameValueMap.find(modeName) ==
> +		    controls::AwbModeNameValueMap.end()) {
> +			LOG(Awb, Warning)
> +				<< "Skipping unknown awb mode '"
> +				<< modeName << "'";
> +			continue;
> +		}
> +
> +		if (!modeDict.isDictionary()) {
> +			LOG(Awb, Error)
> +				<< "Invalid awb mode '" << modeName << "'";
> +			return -EINVAL;
> +		}
> +
> +		const auto &modeValue = static_cast<controls::AwbModeEnum>(
> +			controls::AwbModeNameValueMap.at(modeName));
> +
> +		auto &config = modes_[modeValue];
> +		auto hi = modeDict["hi"].get<double>();
> +		if (!hi) {
> +			LOG(Awb, Error) << "Failed to read hi param of mode "
> +					<< modeName;
> +			return -EINVAL;
> +		}
> +		config.ctHi = *hi;
> +
> +		auto lo = modeDict["lo"].get<double>();
> +		if (!lo) {
> +			LOG(Awb, Error) << "Failed to read low param of mode "
> +					<< modeName;
> +			return -EINVAL;
> +		}
> +		config.ctLo = *lo;
> +
> +		availableModes.push_back(modeValue);
> +	}
> +
> +	if (modes_.empty()) {
> +		LOG(Awb, Error) << "No AWB modes configured";
> +		return -EINVAL;
> +	}
> +
> +	if (modes_.find(controls::AwbAuto) == modes_.end()) {
> +		LOG(Awb, Error) << "AwbAuto mode is missing in the configuration.";
> +		return -EINVAL;
> +	}
> +
> +	controls_[&controls::AwbMode] = ControlInfo(availableModes);
> +
> +	return 0;
> +}
> +
>   } /* namespace ipa */
>   
>   } /* namespace libcamera */
> diff --git a/src/ipa/libipa/awb.h b/src/ipa/libipa/awb.h
> index 2dd471606ec4..426fdd6dae77 100644
> --- a/src/ipa/libipa/awb.h
> +++ b/src/ipa/libipa/awb.h
> @@ -7,7 +7,11 @@
>   
>   #pragma once
>   
> +#include <map>
> +
> +#include <libcamera/control_ids.h>
>   #include <libcamera/controls.h>
> +
>   #include "libcamera/internal/yaml_parser.h"
>   
>   #include "vector.h"
> @@ -43,7 +47,15 @@ public:
>   	virtual void handleControls([[maybe_unused]] const ControlList &controls) {}
>   
>   protected:
> +	int parseModeConfigs(const YamlObject &tuningData);
> +
> +	struct ModeConfig {
> +		double ctHi;
> +		double ctLo;
> +	};
> +
>   	ControlInfoMap::Map controls_;
> +	std::map<controls::AwbModeEnum, AwbAlgorithm::ModeConfig> modes_;
>   };
>   
>   } /* namespace ipa */