From patchwork Mon Aug 18 08:28:39 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 24151 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 F0C42BEFBE for ; Mon, 18 Aug 2025 08:29:59 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id AC0BC6925F; Mon, 18 Aug 2025 10:29:59 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="EXOVCIfT"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 86FA76924E for ; Mon, 18 Aug 2025 10:29:57 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:766d:a405:f64e:fe3a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 901812416; Mon, 18 Aug 2025 10:29:00 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1755505740; bh=s5Oen/RpoxgdECRxB6R8dYCZKQYyFROrCS3k7Wxu1os=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=EXOVCIfTkRoHNEejE1V6SwLQXswKFBVcsadbdMeeDK5G4Gguwny/xZQQncAXUXE7S XVZ0usS9KUXVRcokzZT9lrIKxMjxICIWVjy/9GqK7oMgVvYZ9EVWflre0DapdcHHC7 f04FeJCQLPnl4x87izEpCvHRuYm6XT1V5mjBhz5w= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [RFC PATCH 1/4] libipa: agc_mean_luminance: pass estimateLuminance() as parameter Date: Mon, 18 Aug 2025 10:28:39 +0200 Message-ID: <20250818082909.2001635-2-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.48.1 In-Reply-To: <20250818082909.2001635-1-stefan.klug@ideasonboard.com> References: <20250818082909.2001635-1-stefan.klug@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" To use AgcMeanLuminance an algorithm derives from it and overrides the estimateLuminance() function. This override is the only reason to derive from this class. It has however two downsides: - It is more difficult for the reader to know which functionality comes from where. - It is necessary to keep references to data needed in the estimateLuminance() function inside the class, even though that date is only accessed inside AgcMenaLuminance::calculateNewEv(). Remove the need to derive from AgcMeanLuminance by passing estimateLuminance as function object into calculateNewEv(). Signed-off-by: Stefan Klug --- src/ipa/ipu3/algorithms/agc.cpp | 4 +- src/ipa/ipu3/algorithms/agc.h | 2 +- src/ipa/libipa/agc_mean_luminance.cpp | 57 ++++++++++++++------------- src/ipa/libipa/agc_mean_luminance.h | 9 +++-- src/ipa/mali-c55/algorithms/agc.cpp | 5 ++- src/ipa/mali-c55/algorithms/agc.h | 2 +- src/ipa/rkisp1/algorithms/agc.cpp | 23 ++++++----- src/ipa/rkisp1/algorithms/agc.h | 5 +-- 8 files changed, 60 insertions(+), 47 deletions(-) diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp index 39d0aebb0838..98a034a47625 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -27,6 +27,7 @@ namespace libcamera { using namespace std::literals::chrono_literals; +using namespace std::placeholders; namespace ipa::ipu3::algorithms { @@ -224,7 +225,8 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, utils::Duration newExposureTime; double aGain, dGain; std::tie(newExposureTime, aGain, dGain) = - calculateNewEv(context.activeState.agc.constraintMode, + calculateNewEv(std::bind(&Agc::estimateLuminance, this, _1), + context.activeState.agc.constraintMode, context.activeState.agc.exposureMode, hist, effectiveExposureValue); diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h index 890c271b4462..8e182cd7cff3 100644 --- a/src/ipa/ipu3/algorithms/agc.h +++ b/src/ipa/ipu3/algorithms/agc.h @@ -38,7 +38,7 @@ public: ControlList &metadata) override; private: - double estimateLuminance(double gain) const override; + double estimateLuminance(double gain) const; Histogram parseStatistics(const ipu3_uapi_stats_3a *stats, const ipu3_uapi_grid_config &grid); diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp index ff96a381ffce..df7efb284a89 100644 --- a/src/ipa/libipa/agc_mean_luminance.cpp +++ b/src/ipa/libipa/agc_mean_luminance.cpp @@ -18,7 +18,7 @@ using namespace libcamera::controls; /** * \file agc_mean_luminance.h - * \brief Base class implementing mean luminance AEGC + * \brief Class implementing mean luminance AEGC */ namespace libcamera { @@ -53,6 +53,21 @@ static constexpr double kDefaultRelativeLuminanceTarget = 0.16; */ static constexpr double kMaxRelativeLuminanceTarget = 0.95; +/** + * \fn AgcMeanLuminance::EstimateLuminanceFn + * \brief Function to estimate the mean luminance given a gain + * \param[in] gain The gain with which to adjust the luminance estimate + * + * Callback functions of this type are used within \a calculateNewEv to estimate + * the average relative luminance of the frame that would be output by the + * sensor if an additional \a gain was applied. It is a is implemented as + * callback function because estimation of luminance is a hardware-specific + * operation, which depends wholly on the format of the stats that are delivered + * to libcamera from the ISP. + * + * \return The normalised relative luminance of the image + */ + /** * \struct AgcMeanLuminance::AgcConstraint * \brief The boundaries and target for an AeConstraintMode constraint @@ -133,13 +148,11 @@ static constexpr double kMaxRelativeLuminanceTarget = 0.95; * will determine the supportable precision of the constraints. * * IPA modules that want to use this class to implement their AEGC algorithm - * should derive it and provide an overriding estimateLuminance() function for - * this class to use. They must call parseTuningData() in init(), and must also - * call setLimits() and resetFrameCounter() in configure(). They may then use - * calculateNewEv() in process(). If the limits passed to setLimits() change for - * any reason (for example, in response to a FrameDurationLimit control being - * passed in queueRequest()) then setLimits() must be called again with the new - * values. + * must call parseTuningData() in init(), and must also call setLimits() and + * resetFrameCounter() in configure(). They may then use calculateNewEv() in + * process(). If the limits passed to setLimits() change for any reason (for + * example, in response to a FrameDurationLimit control being passed in + * queueRequest()) then setLimits() must be called again with the new values. */ AgcMeanLuminance::AgcMeanLuminance() @@ -420,28 +433,12 @@ void AgcMeanLuminance::setLimits(utils::Duration minExposureTime, * \brief Get the controls that have been generated after parsing tuning data */ -/** - * \fn AgcMeanLuminance::estimateLuminance(const double gain) - * \brief Estimate the luminance of an image, adjusted by a given gain - * \param[in] gain The gain with which to adjust the luminance estimate - * - * This function estimates the average relative luminance of the frame that - * would be output by the sensor if an additional \a gain was applied. It is a - * pure virtual function because estimation of luminance is a hardware-specific - * operation, which depends wholly on the format of the stats that are delivered - * to libcamera from the ISP. Derived classes must override this function with - * one that calculates the normalised mean luminance value across the entire - * image. - * - * \return The normalised relative luminance of the image - */ - /** * \brief Estimate the initial gain needed to achieve a relative luminance * target * \return The calculated initial gain */ -double AgcMeanLuminance::estimateInitialGain() const +double AgcMeanLuminance::estimateInitialGain(EstimateLuminanceFn estimateLuminance) const { double yTarget = std::min(relativeLuminanceTarget_ * exposureCompensation_, kMaxRelativeLuminanceTarget); @@ -542,6 +539,7 @@ utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue) /** * \brief Calculate the new exposure value and splut it between exposure time * and gain + * \param[in] estimateLuminance A function to get luminance estimates * \param[in] constraintModeIndex The index of the current constraint mode * \param[in] exposureModeIndex The index of the current exposure mode * \param[in] yHist A Histogram from the ISP statistics to use in constraining @@ -553,10 +551,15 @@ utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue) * exposure value is filtered to prevent rapid changes from frame to frame, and * divided into exposure time, analogue and digital gain. * + * The \a estimateLuminance shall estimate the average relative luminance of the + * frame that would be output by the sensor if an additional \a gain was + * applied. + * * \return Tuple of exposure time, analogue gain, and digital gain */ std::tuple -AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex, +AgcMeanLuminance::calculateNewEv(EstimateLuminanceFn estimateLuminance, + uint32_t constraintModeIndex, uint32_t exposureModeIndex, const Histogram &yHist, utils::Duration effectiveExposureValue) @@ -580,7 +583,7 @@ AgcMeanLuminance::calculateNewEv(uint32_t constraintModeIndex, return exposureModeHelper->splitExposure(10ms); } - double gain = estimateInitialGain(); + double gain = estimateInitialGain(estimateLuminance); gain = constraintClampGain(constraintModeIndex, yHist, gain); /* diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h index cad7ef845487..5e4c598c8ab9 100644 --- a/src/ipa/libipa/agc_mean_luminance.h +++ b/src/ipa/libipa/agc_mean_luminance.h @@ -31,6 +31,8 @@ public: AgcMeanLuminance(); virtual ~AgcMeanLuminance(); + using EstimateLuminanceFn = std::function; + struct AgcConstraint { enum class Bound { Lower = 0, @@ -68,7 +70,8 @@ public: } std::tuple - calculateNewEv(uint32_t constraintModeIndex, uint32_t exposureModeIndex, + calculateNewEv(EstimateLuminanceFn estimateLuminance, + uint32_t constraintModeIndex, uint32_t exposureModeIndex, const Histogram &yHist, utils::Duration effectiveExposureValue); void resetFrameCount() @@ -77,13 +80,11 @@ public: } private: - virtual double estimateLuminance(const double gain) const = 0; - void parseRelativeLuminanceTarget(const YamlObject &tuningData); void parseConstraint(const YamlObject &modeDict, int32_t id); int parseConstraintModes(const YamlObject &tuningData); int parseExposureModes(const YamlObject &tuningData); - double estimateInitialGain() const; + double estimateInitialGain(EstimateLuminanceFn estimateLuminance) const; double constraintClampGain(uint32_t constraintModeIndex, const Histogram &hist, double gain); diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp index 15963994b2d6..4bfb4aaeedf0 100644 --- a/src/ipa/mali-c55/algorithms/agc.cpp +++ b/src/ipa/mali-c55/algorithms/agc.cpp @@ -8,6 +8,7 @@ #include "agc.h" #include +#include #include #include @@ -21,6 +22,7 @@ namespace libcamera { using namespace std::literals::chrono_literals; +using namespace std::placeholders; namespace ipa::mali_c55::algorithms { @@ -383,7 +385,8 @@ void Agc::process(IPAContext &context, utils::Duration shutterTime; double aGain, dGain; std::tie(shutterTime, aGain, dGain) = - calculateNewEv(activeState.agc.constraintMode, + calculateNewEv(std::bind(&Agc::estimateLuminance, this, _1), + activeState.agc.constraintMode, activeState.agc.exposureMode, statistics_.yHist, effectiveExposureValue); diff --git a/src/ipa/mali-c55/algorithms/agc.h b/src/ipa/mali-c55/algorithms/agc.h index 0b4bf7eda1c2..698de57e1ba8 100644 --- a/src/ipa/mali-c55/algorithms/agc.h +++ b/src/ipa/mali-c55/algorithms/agc.h @@ -64,7 +64,7 @@ public: ControlList &metadata) override; private: - double estimateLuminance(const double gain) const override; + double estimateLuminance(const double gain) const; size_t fillGainParamBlock(IPAContext &context, IPAFrameContext &frameContext, mali_c55_params_block block); diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp index 35440b67e999..3a078f9753f6 100644 --- a/src/ipa/rkisp1/algorithms/agc.cpp +++ b/src/ipa/rkisp1/algorithms/agc.cpp @@ -440,16 +440,17 @@ void Agc::fillMetadata(IPAContext &context, IPAFrameContext &frameContext, * * \return The relative luminance */ -double Agc::estimateLuminance(double gain) const +double Agc::estimateLuminance(Span expMeans, + Span weights, double gain) { - ASSERT(expMeans_.size() == weights_.size()); + ASSERT(expMeans.size() == weights.size()); double ySum = 0.0; double wSum = 0.0; /* Sum the averages, saturated to 255. */ - for (unsigned i = 0; i < expMeans_.size(); i++) { - double w = weights_[i]; - ySum += std::min(expMeans_[i] * gain, 255.0) * w; + for (unsigned i = 0; i < expMeans.size(); i++) { + double w = weights[i]; + ySum += std::min(expMeans[i] * gain, 255.0) * w; wSum += w; } @@ -522,9 +523,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, /* The lower 4 bits are fractional and meant to be discarded. */ Histogram hist({ params->hist.hist_bins, context.hw->numHistogramBins }, [](uint32_t x) { return x >> 4; }); - expMeans_ = { params->ae.exp_mean, context.hw->numAeCells }; std::vector &modeWeights = meteringModes_.at(frameContext.agc.meteringMode); - weights_ = { modeWeights.data(), modeWeights.size() }; /* * Set the AGC limits using the fixed exposure time and/or gain in @@ -566,10 +565,17 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, setExposureCompensation(pow(2.0, frameContext.agc.exposureValue)); + AgcMeanLuminance::EstimateLuminanceFn estimateLuminanceFn = std::bind( + &Agc::estimateLuminance, this, + Span(params->ae.exp_mean, context.hw->numAeCells), + Span(modeWeights.data(), modeWeights.size()), + std::placeholders::_1); + utils::Duration newExposureTime; double aGain, dGain; std::tie(newExposureTime, aGain, dGain) = - calculateNewEv(frameContext.agc.constraintMode, + calculateNewEv(estimateLuminanceFn, + frameContext.agc.constraintMode, frameContext.agc.exposureMode, hist, effectiveExposureValue); @@ -590,7 +596,6 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, std::max(frameContext.agc.minFrameDuration, newExposureTime)); fillMetadata(context, frameContext, metadata); - expMeans_ = {}; } REGISTER_IPA_ALGORITHM(Agc, "Agc") diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h index 7867eed9c4e3..742c8f7371f3 100644 --- a/src/ipa/rkisp1/algorithms/agc.h +++ b/src/ipa/rkisp1/algorithms/agc.h @@ -43,19 +43,18 @@ public: ControlList &metadata) override; private: + double estimateLuminance(Span expMeans, + Span weights, double gain); int parseMeteringModes(IPAContext &context, const YamlObject &tuningData); uint8_t computeHistogramPredivider(const Size &size, enum rkisp1_cif_isp_histogram_mode mode); void fillMetadata(IPAContext &context, IPAFrameContext &frameContext, ControlList &metadata); - double estimateLuminance(double gain) const override; void processFrameDuration(IPAContext &context, IPAFrameContext &frameContext, utils::Duration frameDuration); - Span expMeans_; - Span weights_; std::map> meteringModes_; };