| Message ID | 20261029125844.170324-8-barnabas.pocze@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Barnabás On Thu, Oct 29, 2026 at 01:58:27PM +0100, Barnabás Pőcze wrote: > Add a separate function whose job is to add the fallback values > for the various configuration parameters that were not specified > in the tuning file. > > This allows some simplification of the code by making use of early > returns, so do that as well. And additionally, pass only the > relevant object to the "sub" parsers instead of the whole thing. > > This is needed in order to be able to determine what exactly the tuning > file contains, before adding the fallback values. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > src/ipa/libipa/agc.cpp | 189 +++++++++++++++++++++-------------------- > 1 file changed, 97 insertions(+), 92 deletions(-) > > diff --git a/src/ipa/libipa/agc.cpp b/src/ipa/libipa/agc.cpp > index 54aa8207d4..229c37b0ea 100644 > --- a/src/ipa/libipa/agc.cpp > +++ b/src/ipa/libipa/agc.cpp > @@ -235,13 +235,10 @@ namespace { > */ > static constexpr double kDefaultRelativeLuminanceTarget = 0.16; > > -int parseRelativeLuminanceTarget(AgcMeanLuminance::Configuration &config, const ValueNode &tuningData) > +int parseRelativeLuminanceTarget(AgcMeanLuminance::Configuration &config, const ValueNode &target) > { > - auto &target = tuningData["relativeLuminanceTarget"]; > - if (!target) { > - config.relativeLuminanceTarget = { { { { 0.0, kDefaultRelativeLuminanceTarget } } } }; > + if (!target) > return 0; > - } > > auto pwl = target.get<Pwl>(); > if (!pwl) { > @@ -291,30 +288,103 @@ int parseConstraint(AgcMeanLuminance::Configuration &config, const ValueNode &mo > return 0; > } > > -int parseConstraintModes(AgcMeanLuminance::Configuration &config, const ValueNode &tuningData) > +int parseConstraintModes(AgcMeanLuminance::Configuration &config, const ValueNode &constraintModes) > { > - const ValueNode &constraintModes = tuningData[controls::AeConstraintMode.name()]; > - if (constraintModes.isDictionary()) { > - for (const auto &[modeName, modeDict] : constraintModes.asDict()) { > - auto it = controls::AeConstraintModeNameValueMap.find(modeName); > - if (it == controls::AeConstraintModeNameValueMap.end()) { > - LOG(Agc, Warning) > - << "Skipping unknown constraint mode '" << modeName << "'"; > - continue; > - } > - > - if (!modeDict.isDictionary()) { > - LOG(Agc, Error) > - << "Invalid constraint mode '" << modeName << "'"; > - return -EINVAL; > - } > - > - int ret = parseConstraint(config, modeDict, it->second); > - if (ret) > - return ret; > + if (!constraintModes.isDictionary()) > + return 0; > + > + for (const auto &[modeName, modeDict] : constraintModes.asDict()) { > + auto it = controls::AeConstraintModeNameValueMap.find(modeName); > + if (it == controls::AeConstraintModeNameValueMap.end()) { > + LOG(Agc, Warning) > + << "Skipping unknown constraint mode '" << modeName << "'"; > + continue; > } > + > + if (!modeDict.isDictionary()) { > + LOG(Agc, Error) > + << "Invalid constraint mode '" << modeName << "'"; > + return -EINVAL; > + } > + > + int ret = parseConstraint(config, modeDict, it->second); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +int parseExposureModes(AgcMeanLuminance::Configuration &config, const ValueNode &exposureModes) > +{ > + if (!exposureModes.isDictionary()) > + return 0; > + > + for (const auto &[modeName, modeValues] : exposureModes.asDict()) { > + auto it = controls::AeExposureModeNameValueMap.find(modeName); > + if (it == controls::AeExposureModeNameValueMap.end()) { > + LOG(Agc, Warning) > + << "Skipping unknown exposure mode '" << modeName << "'"; > + continue; > + } > + > + if (!modeValues.isDictionary()) { > + LOG(Agc, Error) > + << "Invalid exposure mode '" << modeName << "'"; > + return -EINVAL; > + } > + > + std::vector<uint32_t> exposureTimes = > + modeValues["exposureTime"].get<std::vector<uint32_t>>().value_or(utils::defopt); > + std::vector<double> gains = > + modeValues["gain"].get<std::vector<double>>().value_or(utils::defopt); > + > + if (exposureTimes.size() != gains.size()) { > + LOG(Agc, Error) > + << "Exposure time and gain array sizes unequal"; > + return -EINVAL; > + } > + > + if (exposureTimes.empty()) { > + LOG(Agc, Error) > + << "Exposure time and gain arrays are empty"; > + return -EINVAL; > + } > + > + std::vector<ExposureModeHelper::Stage> stages; > + for (size_t i = 0; i < exposureTimes.size(); i++) { > + stages.push_back({ > + .exposureTime = std::chrono::microseconds(exposureTimes[i]), > + .gain = gains[i], > + }); > + } > + > + config.exposureModeHelpers.try_emplace(it->second, std::move(stages)); > } > > + return 0; > +} > + > +int parseTuningData(AgcMeanLuminance::Configuration &config, const ValueNode &tuningData) > +{ > + int ret; > + > + ret = parseRelativeLuminanceTarget(config, tuningData["relativeLuminanceTarget"]); > + if (ret) > + return ret; > + > + ret = parseConstraintModes(config, tuningData[controls::AeConstraintMode.name()]); > + if (ret) > + return ret; > + > + return parseExposureModes(config, tuningData[controls::AeExposureMode.name()]); > +} > + > +void addFallbacks(AgcMeanLuminance::Configuration &config) > +{ > + if (config.relativeLuminanceTarget.empty()) > + config.relativeLuminanceTarget = { { { { 0.0, kDefaultRelativeLuminanceTarget } } } }; > + > /* > * If the tuning data file contains no constraints then we use the > * default constraint that the IPU3/RkISP1 Agc algorithms were adhering > @@ -333,56 +403,6 @@ int parseConstraintModes(AgcMeanLuminance::Configuration &config, const ValueNod > std::move(constraint)); > } > > - return 0; > -} > - > -int parseExposureModes(AgcMeanLuminance::Configuration &config, const ValueNode &tuningData) > -{ > - const ValueNode &exposureModes = tuningData[controls::AeExposureMode.name()]; > - if (exposureModes.isDictionary()) { > - for (const auto &[modeName, modeValues] : exposureModes.asDict()) { > - auto it = controls::AeExposureModeNameValueMap.find(modeName); > - if (it == controls::AeExposureModeNameValueMap.end()) { > - LOG(Agc, Warning) > - << "Skipping unknown exposure mode '" << modeName << "'"; > - continue; > - } > - > - if (!modeValues.isDictionary()) { > - LOG(Agc, Error) > - << "Invalid exposure mode '" << modeName << "'"; > - return -EINVAL; > - } > - > - std::vector<uint32_t> exposureTimes = > - modeValues["exposureTime"].get<std::vector<uint32_t>>().value_or(utils::defopt); > - std::vector<double> gains = > - modeValues["gain"].get<std::vector<double>>().value_or(utils::defopt); > - > - if (exposureTimes.size() != gains.size()) { > - LOG(Agc, Error) > - << "Exposure time and gain array sizes unequal"; > - return -EINVAL; > - } > - > - if (exposureTimes.empty()) { > - LOG(Agc, Error) > - << "Exposure time and gain arrays are empty"; > - return -EINVAL; > - } > - > - std::vector<ExposureModeHelper::Stage> stages; > - for (size_t i = 0; i < exposureTimes.size(); i++) { > - stages.push_back({ > - .exposureTime = std::chrono::microseconds(exposureTimes[i]), > - .gain = gains[i], > - }); > - } > - > - config.exposureModeHelpers.try_emplace(it->second, std::move(stages)); > - } > - } > - > /* > * If we don't have any exposure modes in the tuning data we create an > * ExposureModeHelper using an empty vector of stages. This will result > @@ -393,23 +413,6 @@ int parseExposureModes(AgcMeanLuminance::Configuration &config, const ValueNode > config.exposureModeHelpers.try_emplace( > controls::AeExposureModeNormal, > std::vector<ExposureModeHelper::Stage>{}); > - > - return 0; > -} > - > -int parseTuningData(AgcMeanLuminance::Configuration &config, const ValueNode &tuningData) > -{ > - int ret; > - > - ret = parseRelativeLuminanceTarget(config, tuningData); > - if (ret) > - return ret; > - > - ret = parseConstraintModes(config, tuningData); > - if (ret) > - return ret; > - > - return parseExposureModes(config, tuningData); > } > > } /* namespace */ > @@ -557,6 +560,8 @@ int AgcAlgorithm::init(const ValueNode &tuningData, CameraSensorHelper *sensor, > if (ret) > return ret; > > + addFallbacks(agcConfig); > + ack, let's do the fixups after parsing then Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> > impl_.emplace<AgcMeanLuminance>(std::move(agcConfig)); > } else { > impl_.emplace<AgcMSV>(); > -- > 2.56.0 >
diff --git a/src/ipa/libipa/agc.cpp b/src/ipa/libipa/agc.cpp index 54aa8207d4..229c37b0ea 100644 --- a/src/ipa/libipa/agc.cpp +++ b/src/ipa/libipa/agc.cpp @@ -235,13 +235,10 @@ namespace { */ static constexpr double kDefaultRelativeLuminanceTarget = 0.16; -int parseRelativeLuminanceTarget(AgcMeanLuminance::Configuration &config, const ValueNode &tuningData) +int parseRelativeLuminanceTarget(AgcMeanLuminance::Configuration &config, const ValueNode &target) { - auto &target = tuningData["relativeLuminanceTarget"]; - if (!target) { - config.relativeLuminanceTarget = { { { { 0.0, kDefaultRelativeLuminanceTarget } } } }; + if (!target) return 0; - } auto pwl = target.get<Pwl>(); if (!pwl) { @@ -291,30 +288,103 @@ int parseConstraint(AgcMeanLuminance::Configuration &config, const ValueNode &mo return 0; } -int parseConstraintModes(AgcMeanLuminance::Configuration &config, const ValueNode &tuningData) +int parseConstraintModes(AgcMeanLuminance::Configuration &config, const ValueNode &constraintModes) { - const ValueNode &constraintModes = tuningData[controls::AeConstraintMode.name()]; - if (constraintModes.isDictionary()) { - for (const auto &[modeName, modeDict] : constraintModes.asDict()) { - auto it = controls::AeConstraintModeNameValueMap.find(modeName); - if (it == controls::AeConstraintModeNameValueMap.end()) { - LOG(Agc, Warning) - << "Skipping unknown constraint mode '" << modeName << "'"; - continue; - } - - if (!modeDict.isDictionary()) { - LOG(Agc, Error) - << "Invalid constraint mode '" << modeName << "'"; - return -EINVAL; - } - - int ret = parseConstraint(config, modeDict, it->second); - if (ret) - return ret; + if (!constraintModes.isDictionary()) + return 0; + + for (const auto &[modeName, modeDict] : constraintModes.asDict()) { + auto it = controls::AeConstraintModeNameValueMap.find(modeName); + if (it == controls::AeConstraintModeNameValueMap.end()) { + LOG(Agc, Warning) + << "Skipping unknown constraint mode '" << modeName << "'"; + continue; } + + if (!modeDict.isDictionary()) { + LOG(Agc, Error) + << "Invalid constraint mode '" << modeName << "'"; + return -EINVAL; + } + + int ret = parseConstraint(config, modeDict, it->second); + if (ret) + return ret; + } + + return 0; +} + +int parseExposureModes(AgcMeanLuminance::Configuration &config, const ValueNode &exposureModes) +{ + if (!exposureModes.isDictionary()) + return 0; + + for (const auto &[modeName, modeValues] : exposureModes.asDict()) { + auto it = controls::AeExposureModeNameValueMap.find(modeName); + if (it == controls::AeExposureModeNameValueMap.end()) { + LOG(Agc, Warning) + << "Skipping unknown exposure mode '" << modeName << "'"; + continue; + } + + if (!modeValues.isDictionary()) { + LOG(Agc, Error) + << "Invalid exposure mode '" << modeName << "'"; + return -EINVAL; + } + + std::vector<uint32_t> exposureTimes = + modeValues["exposureTime"].get<std::vector<uint32_t>>().value_or(utils::defopt); + std::vector<double> gains = + modeValues["gain"].get<std::vector<double>>().value_or(utils::defopt); + + if (exposureTimes.size() != gains.size()) { + LOG(Agc, Error) + << "Exposure time and gain array sizes unequal"; + return -EINVAL; + } + + if (exposureTimes.empty()) { + LOG(Agc, Error) + << "Exposure time and gain arrays are empty"; + return -EINVAL; + } + + std::vector<ExposureModeHelper::Stage> stages; + for (size_t i = 0; i < exposureTimes.size(); i++) { + stages.push_back({ + .exposureTime = std::chrono::microseconds(exposureTimes[i]), + .gain = gains[i], + }); + } + + config.exposureModeHelpers.try_emplace(it->second, std::move(stages)); } + return 0; +} + +int parseTuningData(AgcMeanLuminance::Configuration &config, const ValueNode &tuningData) +{ + int ret; + + ret = parseRelativeLuminanceTarget(config, tuningData["relativeLuminanceTarget"]); + if (ret) + return ret; + + ret = parseConstraintModes(config, tuningData[controls::AeConstraintMode.name()]); + if (ret) + return ret; + + return parseExposureModes(config, tuningData[controls::AeExposureMode.name()]); +} + +void addFallbacks(AgcMeanLuminance::Configuration &config) +{ + if (config.relativeLuminanceTarget.empty()) + config.relativeLuminanceTarget = { { { { 0.0, kDefaultRelativeLuminanceTarget } } } }; + /* * If the tuning data file contains no constraints then we use the * default constraint that the IPU3/RkISP1 Agc algorithms were adhering @@ -333,56 +403,6 @@ int parseConstraintModes(AgcMeanLuminance::Configuration &config, const ValueNod std::move(constraint)); } - return 0; -} - -int parseExposureModes(AgcMeanLuminance::Configuration &config, const ValueNode &tuningData) -{ - const ValueNode &exposureModes = tuningData[controls::AeExposureMode.name()]; - if (exposureModes.isDictionary()) { - for (const auto &[modeName, modeValues] : exposureModes.asDict()) { - auto it = controls::AeExposureModeNameValueMap.find(modeName); - if (it == controls::AeExposureModeNameValueMap.end()) { - LOG(Agc, Warning) - << "Skipping unknown exposure mode '" << modeName << "'"; - continue; - } - - if (!modeValues.isDictionary()) { - LOG(Agc, Error) - << "Invalid exposure mode '" << modeName << "'"; - return -EINVAL; - } - - std::vector<uint32_t> exposureTimes = - modeValues["exposureTime"].get<std::vector<uint32_t>>().value_or(utils::defopt); - std::vector<double> gains = - modeValues["gain"].get<std::vector<double>>().value_or(utils::defopt); - - if (exposureTimes.size() != gains.size()) { - LOG(Agc, Error) - << "Exposure time and gain array sizes unequal"; - return -EINVAL; - } - - if (exposureTimes.empty()) { - LOG(Agc, Error) - << "Exposure time and gain arrays are empty"; - return -EINVAL; - } - - std::vector<ExposureModeHelper::Stage> stages; - for (size_t i = 0; i < exposureTimes.size(); i++) { - stages.push_back({ - .exposureTime = std::chrono::microseconds(exposureTimes[i]), - .gain = gains[i], - }); - } - - config.exposureModeHelpers.try_emplace(it->second, std::move(stages)); - } - } - /* * If we don't have any exposure modes in the tuning data we create an * ExposureModeHelper using an empty vector of stages. This will result @@ -393,23 +413,6 @@ int parseExposureModes(AgcMeanLuminance::Configuration &config, const ValueNode config.exposureModeHelpers.try_emplace( controls::AeExposureModeNormal, std::vector<ExposureModeHelper::Stage>{}); - - return 0; -} - -int parseTuningData(AgcMeanLuminance::Configuration &config, const ValueNode &tuningData) -{ - int ret; - - ret = parseRelativeLuminanceTarget(config, tuningData); - if (ret) - return ret; - - ret = parseConstraintModes(config, tuningData); - if (ret) - return ret; - - return parseExposureModes(config, tuningData); } } /* namespace */ @@ -557,6 +560,8 @@ int AgcAlgorithm::init(const ValueNode &tuningData, CameraSensorHelper *sensor, if (ret) return ret; + addFallbacks(agcConfig); + impl_.emplace<AgcMeanLuminance>(std::move(agcConfig)); } else { impl_.emplace<AgcMSV>();
Add a separate function whose job is to add the fallback values for the various configuration parameters that were not specified in the tuning file. This allows some simplification of the code by making use of early returns, so do that as well. And additionally, pass only the relevant object to the "sub" parsers instead of the whole thing. This is needed in order to be able to determine what exactly the tuning file contains, before adding the fallback values. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- src/ipa/libipa/agc.cpp | 189 +++++++++++++++++++++-------------------- 1 file changed, 97 insertions(+), 92 deletions(-)