From patchwork Mon Nov 9 09:49:40 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: 28803 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 189FBC335B for ; Mon, 9 Nov 2026 09:50:19 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A52DB6936B; Mon, 9 Nov 2026 10:50:07 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="m1t+FGUr"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B82446918C for ; Mon, 9 Nov 2026 10:49:59 +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 2382B802; Mon, 9 Nov 2026 10:47:38 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1794217658; bh=MoIA5uzgc9bwwTwK/n3n4LY7ODuBkpJhykB9Srlh1nU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=m1t+FGUrZF8GAgCH6XFeLu0Cy2FvCw3ce6xQG6vVN4UePLM3EMXZyz1Y4k1R6OUqA ydxD6kOt6BMsbugxAhI84CsuX9SfEINTzNNY+uEe8IR1YEYHmNexz4TVN5Gp0KWpfA uOv+4MRzp93VnOUVp2+o2VyuvCEgcDmAPHU0cRAs= From: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= To: libcamera-devel@lists.libcamera.org Cc: Jacopo Mondi Subject: [PATCH v7 05/19] ipa: libipa: agc: Split tuning data fallback handling Date: Mon, 9 Nov 2026 10:49:40 +0100 Message-ID: <20261109094954.341480-6-barnabas.pocze@ideasonboard.com> X-Mailer: git-send-email 2.56.0 In-Reply-To: <20261109094954.341480-1-barnabas.pocze@ideasonboard.com> References: <20261109094954.341480-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 Reviewed-by: Stefan Klug --- 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 38bf90879d..6127859d0f 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();