From patchwork Mon Nov 2 10:15:53 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 28742 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 97A3CC3302 for ; Mon, 2 Nov 2026 10:16:37 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BD1EB691FF; Mon, 2 Nov 2026 11:16:29 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="RgLb5nAW"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 94992691F6 for ; Mon, 2 Nov 2026 11:16:16 +0100 (CET) Received: from pb-laptop.local (185.221.143.99.nat.pool.zt.hu [185.221.143.99]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 492F02082; Mon, 2 Nov 2026 11:14:00 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1793614440; bh=Haze+6atIRpYx8Dt5N9mAQc2FqT/NLgO1iCOACr10O8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=RgLb5nAWPnH3bgvSb6I4VI+IAQQQ6tKzmsLxP0Kb+LPa5xAwDNZbmsPiFoGvSzoz8 6l5U3I6vIZxTVill+qmMr3RKRxWkGPrcoUmgt0xcUKVGDp0R9H8+W725mVkH2BzNEW bfgMx96mrQiH/K+wz1Xdjgo3K7TnrV0X8rPAnpXM= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Jacopo Mondi Subject: [PATCH v6 07/24] ipa: libipa: agc: Split tuning data fallback handling Date: Mon, 2 Nov 2026 11:15:53 +0100 Message-ID: <20261102101610.877390-8-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.56.0 In-Reply-To: <20261102101610.877390-1-barnabas.pocze@ideasonboard.com> References: <20261102101610.877390-1-barnabas.pocze@ideasonboard.com> MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 Reviewed-by: Jacopo Mondi --- src/ipa/libipa/agc.cpp | 245 +++++++++++++++++++++-------------------- 1 file changed, 125 insertions(+), 120 deletions(-) diff --git a/src/ipa/libipa/agc.cpp b/src/ipa/libipa/agc.cpp index 5c566a82fb..578ee12a51 100644 --- a/src/ipa/libipa/agc.cpp +++ b/src/ipa/libipa/agc.cpp @@ -237,13 +237,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(); if (!pwl) { @@ -293,7 +290,7 @@ 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) { static constexpr std::tuple compat[] = { { "ConstraintNormal", "Normal", controls::AeConstraintModeNormal }, @@ -302,67 +299,50 @@ int parseConstraintModes(AgcMeanLuminance::Configuration &config, const ValueNod { "ConstraintCustom", "Custom", controls::AeConstraintModeCustom }, }; - const ValueNode &constraintModes = tuningData[controls::AeConstraintMode.name()]; - if (constraintModes.isDictionary()) { - for (const auto &[modeName, modeDict] : constraintModes.asDict()) { - std::optional modeValue; - - auto it = controls::AeConstraintModeNameValueMap.find(modeName); - if (it == controls::AeConstraintModeNameValueMap.end()) { - for (const auto &[oldName, newName, v] : compat) { - if (modeName != oldName) - continue; - - modeValue = v; - - LOG(Agc, Warning) - << "Constraint mode '" << oldName << "' has been renamed to '" - << newName << "', please fix the tuning file"; - break; - } - } else { - modeValue = static_cast(it->second); - } + if (!constraintModes.isDictionary()) + return 0; - if (!modeValue) { - LOG(Agc, Warning) - << "Skipping unknown constraint mode '" << modeName << "'"; - continue; - } + for (const auto &[modeName, modeDict] : constraintModes.asDict()) { + std::optional modeValue; + + auto it = controls::AeConstraintModeNameValueMap.find(modeName); + if (it == controls::AeConstraintModeNameValueMap.end()) { + for (const auto &[oldName, newName, v] : compat) { + if (modeName != oldName) + continue; - if (!modeDict.isDictionary()) { - LOG(Agc, Error) - << "Invalid constraint mode '" << modeName << "'"; - return -EINVAL; + modeValue = v; + + LOG(Agc, Warning) + << "Constraint mode '" << oldName << "' has been renamed to '" + << newName << "', please fix the tuning file"; + break; } + } else { + modeValue = static_cast(it->second); + } - int ret = parseConstraint(config, modeDict, *modeValue); - if (ret) - return ret; + if (!modeValue) { + LOG(Agc, Warning) + << "Skipping unknown constraint mode '" << modeName << "'"; + continue; } - } - /* - * If the tuning data file contains no constraints then we use the - * default constraint that the IPU3/RkISP1 Agc algorithms were adhering - * to anyway before centralisation; this constraint forces the top 2% of - * the histogram to be at least 0.5. - */ - if (config.constraintModes.empty()) { - AgcMeanLuminance::AgcConstraint constraint = { - AgcMeanLuminance::AgcConstraint::Bound::Lower, - 0.98, - 1.0, - Pwl({ { { 0.0, 0.5 } } }), - }; + if (!modeDict.isDictionary()) { + LOG(Agc, Error) + << "Invalid constraint mode '" << modeName << "'"; + return -EINVAL; + } - config.constraintModes[controls::AeConstraintModeNormal].push_back(std::move(constraint)); + int ret = parseConstraint(config, modeDict, *modeValue); + if (ret) + return ret; } return 0; } -int parseExposureModes(AgcMeanLuminance::Configuration &config, const ValueNode &tuningData) +int parseExposureModes(AgcMeanLuminance::Configuration &config, const ValueNode &exposureModes) { static constexpr std::tuple compat[] = { { "ExposureNormal", "Normal", controls::AeExposureModeNormal }, @@ -371,79 +351,68 @@ int parseExposureModes(AgcMeanLuminance::Configuration &config, const ValueNode { "ExposureCustom", "Custom", controls::AeExposureModeCustom }, }; - const ValueNode &exposureModes = tuningData[controls::AeExposureMode.name()]; - if (exposureModes.isDictionary()) { - for (const auto &[modeName, modeValues] : exposureModes.asDict()) { - std::optional modeValue; - - auto it = controls::AeExposureModeNameValueMap.find(modeName); - if (it == controls::AeExposureModeNameValueMap.end()) { - for (const auto &[oldName, newName, v] : compat) { - if (modeName != oldName) - continue; - - modeValue = v; - - LOG(Agc, Warning) - << "Exposure mode '" << oldName << "' has been renamed to '" - << newName << "', please fix the tuning file"; - break; - } - } else { - modeValue = static_cast(it->second); - } + if (!exposureModes.isDictionary()) + return 0; + + for (const auto &[modeName, modeValues] : exposureModes.asDict()) { + std::optional modeValue; + + auto it = controls::AeExposureModeNameValueMap.find(modeName); + if (it == controls::AeExposureModeNameValueMap.end()) { + for (const auto &[oldName, newName, v] : compat) { + if (modeName != oldName) + continue; + + modeValue = v; - if (!modeValue) { LOG(Agc, Warning) - << "Skipping unknown exposure mode '" << modeName << "'"; - continue; + << "Exposure mode '" << oldName << "' has been renamed to '" + << newName << "', please fix the tuning file"; + break; } + } else { + modeValue = static_cast(it->second); + } - if (!modeValues.isDictionary()) { - LOG(Agc, Error) - << "Invalid exposure mode '" << modeName << "'"; - return -EINVAL; - } + if (!modeValue) { + LOG(Agc, Warning) + << "Skipping unknown exposure mode '" << modeName << "'"; + continue; + } - std::vector exposureTimes = - modeValues["exposureTime"].get>().value_or(utils::defopt); - std::vector gains = - modeValues["gain"].get>().value_or(utils::defopt); + if (!modeValues.isDictionary()) { + LOG(Agc, Error) + << "Invalid exposure mode '" << modeName << "'"; + return -EINVAL; + } - if (exposureTimes.size() != gains.size()) { - LOG(Agc, Error) - << "Exposure time and gain array sizes unequal"; - return -EINVAL; - } + std::vector exposureTimes = + modeValues["exposureTime"].get>().value_or(utils::defopt); + std::vector gains = + modeValues["gain"].get>().value_or(utils::defopt); - if (exposureTimes.empty()) { - LOG(Agc, Error) - << "Exposure time and gain arrays are empty"; - return -EINVAL; - } + if (exposureTimes.size() != gains.size()) { + LOG(Agc, Error) + << "Exposure time and gain array sizes unequal"; + return -EINVAL; + } - std::vector stages; - for (size_t i = 0; i < exposureTimes.size(); i++) { - stages.push_back({ - .exposureTime = std::chrono::microseconds(exposureTimes[i]), - .gain = gains[i], - }); - } + if (exposureTimes.empty()) { + LOG(Agc, Error) + << "Exposure time and gain arrays are empty"; + return -EINVAL; + } - config.exposureModeHelpers.try_emplace(*modeValue, std::move(stages)); + std::vector stages; + for (size_t i = 0; i < exposureTimes.size(); i++) { + stages.push_back({ + .exposureTime = std::chrono::microseconds(exposureTimes[i]), + .gain = gains[i], + }); } - } - /* - * 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 - * in the ExposureModeHelper simply driving the exposure time as high as - * possible before touching gain. - */ - if (config.exposureModeHelpers.empty()) - config.exposureModeHelpers.try_emplace( - controls::AeExposureModeNormal, - std::vector{}); + config.exposureModeHelpers.try_emplace(*modeValue, std::move(stages)); + } return 0; } @@ -452,15 +421,49 @@ int parseTuningData(AgcMeanLuminance::Configuration &config, const ValueNode &tu { int ret; - ret = parseRelativeLuminanceTarget(config, tuningData); + ret = parseRelativeLuminanceTarget(config, tuningData["relativeLuminanceTarget"]); if (ret) return ret; - ret = parseConstraintModes(config, tuningData); + ret = parseConstraintModes(config, tuningData[controls::AeConstraintMode.name()]); if (ret) return ret; - return parseExposureModes(config, tuningData); + 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 + * to anyway before centralisation; this constraint forces the top 2% of + * the histogram to be at least 0.5. + */ + if (config.constraintModes.empty()) { + AgcMeanLuminance::AgcConstraint constraint = { + AgcMeanLuminance::AgcConstraint::Bound::Lower, + 0.98, + 1.0, + Pwl({ { { 0.0, 0.5 } } }), + }; + + config.constraintModes[controls::AeConstraintModeNormal].push_back(std::move(constraint)); + } + + /* + * 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 + * in the ExposureModeHelper simply driving the exposure time as high as + * possible before touching gain. + */ + if (config.exposureModeHelpers.empty()) + config.exposureModeHelpers.try_emplace( + controls::AeExposureModeNormal, + std::vector{}); } } /* namespace */ @@ -608,6 +611,8 @@ int AgcAlgorithm::init(const ValueNode &tuningData, CameraSensorHelper *sensor, if (ret) return ret; + addFallbacks(agcConfig); + impl_.emplace(std::move(agcConfig)); } else { impl_.emplace();