From patchwork Tue Sep 23 19:06:57 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 24444 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 31361C32A9 for ; Tue, 23 Sep 2025 19:07:19 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 4B3A66B61A; Tue, 23 Sep 2025 21:07:18 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="ZLwO21v8"; dkim-atps=neutral Received: from tor.source.kernel.org (tor.source.kernel.org [172.105.4.254]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 336CC6B608 for ; Tue, 23 Sep 2025 21:07:08 +0200 (CEST) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by tor.source.kernel.org (Postfix) with ESMTP id 6693C60278 for ; Tue, 23 Sep 2025 19:07:07 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 616BDC113D0; Tue, 23 Sep 2025 19:07:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1758654427; bh=CCZwD+CJ73bAxaZILpNf1Z+g+VTllZSU3Kfzx+PICJo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ZLwO21v8l2nuSKTSwlU0e0HrtbJyTcgYoTmRGq01ImSZWGWgnjd93fRlk6HDp5fk+ GtZl7I+0n+Cripchh3UwOXgrTfIIaakb3Snxm1rmjGSSVGjO++pFPAMTTLJlqDXcBA iKsSulbjwy078SMWeAdREZ7ch5x40iJ65+PFTrgxPXAGNLmfCypz+ueoGc3x519h9s mg6Z2IIf1Wp9dPhViwl+xWvDbepS4GcDCd6Cj9zPpXSsCAVJ//VzDXzuiTdmSb0huG mV/3JLhYTf/hIhZ7gJorxnf9kviBkfzESIN9SzOaVInGbXbpE6qKT+ahTk50zXDOAM rcNP4QO++YhFA== From: Hans de Goede To: libcamera-devel@lists.libcamera.org Cc: Hans de Goede Subject: [PATCH 5/5] ipa: software_isp: AGC: Stop using delayed control for previous values Date: Tue, 23 Sep 2025 21:06:57 +0200 Message-ID: <20250923190657.21453-6-hansg@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20250923190657.21453-1-hansg@kernel.org> References: <20250923190657.21453-1-hansg@kernel.org> 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" Due to a combination of not having correct control-delay information for various sensors as well as the generic nature of the simple-pipeline + software-ISP meaning that any pipeline delays are unknown it is impossible to get reliable control-delay values. Wrong control-delay values can lead to pretty bad oscilation. Since atm it is not possible to fix the wrong control-delay values, stop using the old sensor values reported by the delayed-controlled mechanism. Instead remember the gain and exposures set the last time the algorithm runs (initializing the cached values with the actual sensor values on the first frame), combined with skipping a fixed number of frames after changing values. Signed-off-by: Hans de Goede --- src/ipa/simple/algorithms/agc.cpp | 30 +++++++++++++++++++++++++----- src/ipa/simple/algorithms/agc.h | 3 ++- src/ipa/simple/ipa_context.h | 6 ++++++ src/ipa/simple/soft_simple.cpp | 4 ++-- 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp index cdde56ba2..3c0e20ddc 100644 --- a/src/ipa/simple/algorithms/agc.cpp +++ b/src/ipa/simple/algorithms/agc.cpp @@ -41,7 +41,15 @@ Agc::Agc() { } -void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, double exposureMSV) +int Agc::configure(IPAContext &context, + [[maybe_unused]] const IPAConfigInfo &configInfo) +{ + context.activeState.agc.skipFrames = 0; + + return 0; +} + +void Agc::updateExposure(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, double exposureMSV) { /* * kExpDenominator of 10 gives ~10% increment/decrement; @@ -51,8 +59,18 @@ void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, dou static constexpr uint8_t kExpNumeratorUp = kExpDenominator + 1; static constexpr uint8_t kExpNumeratorDown = kExpDenominator - 1; - int32_t &exposure = frameContext.sensor.exposure; - double &again = frameContext.sensor.gain; + int32_t &skipFrames = context.activeState.agc.skipFrames; + int32_t &exposure = context.activeState.agc.exposure; + double &again = context.activeState.agc.again; + + /* Set initial-gain values from sensor on first frame */ + if (frame == 0) { + exposure = frameContext.sensor.exposure; + again = frameContext.sensor.gain; + } + + if (skipFrames && --skipFrames) + return; if (exposureMSV < kExposureOptimal - kExposureSatisfactory) { if (exposure < context.configuration.agc.exposureMax) { @@ -68,6 +86,7 @@ void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, dou else again = next; } + skipFrames = 3; } if (exposureMSV > kExposureOptimal + kExposureSatisfactory) { @@ -84,6 +103,7 @@ void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, dou else exposure = next; } + skipFrames = 3; } exposure = std::clamp(exposure, context.configuration.agc.exposureMin, @@ -97,7 +117,7 @@ void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, dou } void Agc::process(IPAContext &context, - [[maybe_unused]] const uint32_t frame, + const uint32_t frame, IPAFrameContext &frameContext, const SwIspStats *stats, ControlList &metadata) @@ -135,7 +155,7 @@ void Agc::process(IPAContext &context, } float exposureMSV = (denom == 0 ? 0 : static_cast(num) / denom); - updateExposure(context, frameContext, exposureMSV); + updateExposure(context, frame, frameContext, exposureMSV); } REGISTER_IPA_ALGORITHM(Agc, "Agc") diff --git a/src/ipa/simple/algorithms/agc.h b/src/ipa/simple/algorithms/agc.h index 112d9f5a1..ef387664f 100644 --- a/src/ipa/simple/algorithms/agc.h +++ b/src/ipa/simple/algorithms/agc.h @@ -19,13 +19,14 @@ public: Agc(); ~Agc() = default; + int configure(IPAContext &context, const IPAConfigInfo &configInfo) override; void process(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, const SwIspStats *stats, ControlList &metadata) override; private: - void updateExposure(IPAContext &context, IPAFrameContext &frameContext, double exposureMSV); + void updateExposure(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, double exposureMSV); }; } /* namespace ipa::soft::algorithms */ diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h index ba525a881..cdab980a1 100644 --- a/src/ipa/simple/ipa_context.h +++ b/src/ipa/simple/ipa_context.h @@ -37,6 +37,12 @@ struct IPASessionConfiguration { }; struct IPAActiveState { + struct { + int32_t skipFrames; + int32_t exposure; + double again; + } agc; + struct { uint8_t level; int32_t lastExposure; diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index f0764ef46..f8c6291e2 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -327,8 +327,8 @@ void IPASoftSimple::processStats(const uint32_t frame, ControlList ctrls(sensorInfoMap_); - auto &againNew = frameContext.sensor.gain; - ctrls.set(V4L2_CID_EXPOSURE, frameContext.sensor.exposure); + auto &againNew = context_.activeState.agc.again; + ctrls.set(V4L2_CID_EXPOSURE, context_.activeState.agc.exposure); ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast(camHelper_ ? camHelper_->gainCode(againNew) : againNew));