From patchwork Sat Sep 27 18:00:00 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: 24480 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 3BA97BDB1C for ; Sat, 27 Sep 2025 18:00:21 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 929F46B5F0; Sat, 27 Sep 2025 20:00:18 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="nN+Z3ZMk"; 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 C938E6B5FB for ; Sat, 27 Sep 2025 20:00:11 +0200 (CEST) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by tor.source.kernel.org (Postfix) with ESMTP id D6BFE601AA; Sat, 27 Sep 2025 18:00:10 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 758CDC4CEF8; Sat, 27 Sep 2025 18:00:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1758996010; bh=GsN3qabD8oF/CaS4DCismqZlNWjuYJsUJrbdjorqqsM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=nN+Z3ZMkgWIIiDG/4TZD3DJkAQIHnLhRJaFn7OnyLXIODoNemlDqcfPK3IHDs9kPM am2bZ4EmRru+NZTWMWor/R6hySXYdGqWz2iD5sTnaEjCTbVzMaY3tOwhHn1iMg7LDg FWCnAk4p4MeUUGfbmhJhNBvUx+hC7unh29GRxIuyTDRJxPUObPlh21QZOHBy9waChq INeJiJ33F1ETeTk0Oa6l57SzDoTtfS2VKxMvzMVYzlhxakh2lY7qw1LBHAJ2qWemOQ Ofcdw5242bKkrDvLUm9e1pR+YbwLo7ieGZujG5iF8XxXcrErUtX19fUUjrFpGndGkp W9Nk4d5tTnsQA== From: Hans de Goede To: libcamera-devel@lists.libcamera.org Cc: Hans de Goede , Milan Zamazal , Kieran Bingham Subject: [PATCH v3 2/6] ipa: software_isp: AGC: Do not lower gain below 1.0 Date: Sat, 27 Sep 2025 20:00:00 +0200 Message-ID: <20250927180004.84620-3-hansg@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20250927180004.84620-1-hansg@kernel.org> References: <20250927180004.84620-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" At the moment when the overall image brightness is considered too high the AGC code will lower the gain all the way down to againMin before considering lowering the exposure. What should happen instead is lower the gain no lower than 1.0 and after that lower the exposure instead of lowering the gain. Otherwise there might be a heavily overexposed image (e.g. all white) which then is made less white by a gain < 1.0 which is no good. When there is no sensor-helper, assume the driver reported default-gain value is close to a gain of 1.0 . While at it also remove the weird limitation to only lower the gain when exposure is set to the maximum. As long as the gain is higher than the default gain, the gain should be lowered first. Reviewed-by: Milan Zamazal Tested-by: Milan Zamazal Tested-by: Kieran Bingham Signed-off-by: Hans de Goede Reviewed-by: Kieran Bingham --- Changes in v2: - Use a gain of 1.0 as low-limit instead of always using the default-gain, falling back to the default if there is no sensor-helper for the sensor. --- src/ipa/simple/algorithms/agc.cpp | 3 +-- src/ipa/simple/ipa_context.h | 2 +- src/ipa/simple/soft_simple.cpp | 3 +++ 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp index c46bb0eb..1fc8d7f4 100644 --- a/src/ipa/simple/algorithms/agc.cpp +++ b/src/ipa/simple/algorithms/agc.cpp @@ -71,8 +71,7 @@ void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, dou } if (exposureMSV > kExposureOptimal + kExposureSatisfactory) { - if (exposure == context.configuration.agc.exposureMax && - again > context.configuration.agc.againMin) { + if (again > context.configuration.agc.again10) { next = again * kExpNumeratorDown / kExpDenominator; if (again - next < context.configuration.agc.againMinStep) again -= context.configuration.agc.againMinStep; diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h index a471b80a..468fccab 100644 --- a/src/ipa/simple/ipa_context.h +++ b/src/ipa/simple/ipa_context.h @@ -28,7 +28,7 @@ struct IPASessionConfiguration { float gamma; struct { int32_t exposureMin, exposureMax; - double againMin, againMax, againMinStep; + double againMin, againMax, again10, againMinStep; utils::Duration lineDuration; } agc; struct { diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index e70439ee..b147aca2 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -216,10 +216,12 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo) int32_t againMin = gainInfo.min().get(); int32_t againMax = gainInfo.max().get(); + int32_t againDef = gainInfo.def().get(); if (camHelper_) { context_.configuration.agc.againMin = camHelper_->gain(againMin); context_.configuration.agc.againMax = camHelper_->gain(againMax); + context_.configuration.agc.again10 = camHelper_->gain(1.0); context_.configuration.agc.againMinStep = (context_.configuration.agc.againMax - context_.configuration.agc.againMin) / @@ -246,6 +248,7 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo) * other) we limit the range of the gain values used. */ context_.configuration.agc.againMax = againMax; + context_.configuration.agc.again10 = againDef; if (againMin) { context_.configuration.agc.againMin = againMin; } else {