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

[Dan Scally]

Hi Stefan

On 17/01/2025 14:41, Dan Scally wrote:
> 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().
>
Actually, this doesn't happen here. The currentMode_ is set to AwbAuto by AwbBayes::init() later in 
the series...so either that (and the member that stores the current mode) needs moving to this class 
or the comment needs correcting.


Thanks

Dan

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