From patchwork Thu Sep 25 22:17:03 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: 24463 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 1BB42BDB1C for ; Thu, 25 Sep 2025 22:17:21 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 9A4936B606; Fri, 26 Sep 2025 00:17:20 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="NfQqyKeq"; dkim-atps=neutral Received: from sea.source.kernel.org (sea.source.kernel.org [IPv6:2600:3c0a:e001:78e:0:1991:8:25]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 434886B5F8 for ; Fri, 26 Sep 2025 00:17:14 +0200 (CEST) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sea.source.kernel.org (Postfix) with ESMTP id D1C8F4057B; Thu, 25 Sep 2025 22:17:12 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 73F90C4CEF0; Thu, 25 Sep 2025 22:17:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1758838632; bh=i8luKV32QotAPxo62xELugb2IsfDO0Szq6/oYQ9c35A=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=NfQqyKeqd+FzWj7BNiwZp8wmQVOZUeALTuFjTblv2XY0iF6VNa9uWlsRbe7vuGw70 ywNLgC1CA7TbFBf+jN7CWLpsLLjYHxLIEthoZWauVPhuf64ifTJCWn15H3+so4X4RX z//uCiWFmuxTW9qZthoyfgQXT54nklrW5r3KuZ89i4u0ma6PQhVh7+tJD3IsDOkYbF Gj49+0z6YNhp8enknfTDUPGTPAyBQWPtgOkz5Ndc32s/2h/bvwGFGj1vqTL4cidn5l Iy9ylay4sIq14luizIyEuqJGIj396aZbyouyxj5D59iJxpO/A/Ch7nhF8y7wPsXrpx kdHiUkN6r0CGQ== From: Hans de Goede To: libcamera-devel@lists.libcamera.org Cc: Hans de Goede , Milan Zamazal , Isaac Scott , Kieran Bingham Subject: [PATCH v2 1/6] ipa: software_isp: Fix context_.configuration.agc.againMin init Date: Fri, 26 Sep 2025 00:17:03 +0200 Message-ID: <20250925221708.7471-2-hansg@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20250925221708.7471-1-hansg@kernel.org> References: <20250925221708.7471-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" Currently context_.configuration.agc.againMin is not initialized when the control reports a non 0 minimum gain value. So far only the againMin == 0 case was handled and context_.configuration.agc.againMin was left unset otherwise. Reviewed-by: Milan Zamazal Reviewed-by: Isaac Scott Tested-by: Milan Zamazal Tested-by: Kieran Bingham Signed-off-by: Hans de Goede --- src/ipa/simple/soft_simple.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index c94c4cd5..e70439ee 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -246,7 +246,9 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo) * other) we limit the range of the gain values used. */ context_.configuration.agc.againMax = againMax; - if (!againMin) { + if (againMin) { + context_.configuration.agc.againMin = againMin; + } else { LOG(IPASoft, Warning) << "Minimum gain is zero, that can't be linear"; context_.configuration.agc.againMin = From patchwork Thu Sep 25 22:17:04 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: 24464 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 55C2FC32A9 for ; Thu, 25 Sep 2025 22:17:24 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id EFCCB6B5FE; Fri, 26 Sep 2025 00:17:22 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="b5TJOizj"; dkim-atps=neutral Received: from sea.source.kernel.org (sea.source.kernel.org [IPv6:2600:3c0a:e001:78e:0:1991:8:25]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8F6BF6B5AA for ; Fri, 26 Sep 2025 00:17:15 +0200 (CEST) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sea.source.kernel.org (Postfix) with ESMTP id 5AEEC455BE; Thu, 25 Sep 2025 22:17:14 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 2C88DC4CEF0; Thu, 25 Sep 2025 22:17:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1758838634; bh=GsN3qabD8oF/CaS4DCismqZlNWjuYJsUJrbdjorqqsM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=b5TJOizjxR+t+odZfyENHJxvzsa+cvVXzIM45WnzZjJRIJPHLElzwlwiZZ2wV61kN ufUEEGpz5c6ppaRoYMAnnk//U2F3d0BlDMxXaFhtGQ2Z/bzMVpNtbhe33v0WQ++84W FH1Pu+NQKyK72+N/BnLjVHqvXrtUJIIz8WYmPinw8c97Gu6uvaVlpsl084gc9nxZ2Z ONiwZXvbV/VU/40zXqZyCVUchdt9xsLz9Qy1WaEXakMmt2x0G34cDE4mJ10bGEvY/P Yy7xQjimCTi7E+qUXf2pxU3BQKLlWalTMHWQMX3aKVDpcvdtm6R6lRPLqEoC5no+OZ S89yTSjRIN6ew== From: Hans de Goede To: libcamera-devel@lists.libcamera.org Cc: Hans de Goede , Milan Zamazal , Kieran Bingham Subject: [PATCH v2 2/6] ipa: software_isp: AGC: Do not lower gain below 1.0 Date: Fri, 26 Sep 2025 00:17:04 +0200 Message-ID: <20250925221708.7471-3-hansg@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20250925221708.7471-1-hansg@kernel.org> References: <20250925221708.7471-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: Isaac Scott --- 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 { From patchwork Thu Sep 25 22:17:05 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: 24465 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 6B4D8BDB1C for ; Thu, 25 Sep 2025 22:17:27 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id EE3A66B601; Fri, 26 Sep 2025 00:17:25 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="lhjpssgI"; dkim-atps=neutral Received: from sea.source.kernel.org (sea.source.kernel.org [IPv6:2600:3c0a:e001:78e:0:1991:8:25]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2522E69367 for ; Fri, 26 Sep 2025 00:17:17 +0200 (CEST) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sea.source.kernel.org (Postfix) with ESMTP id D96A24057B; Thu, 25 Sep 2025 22:17:15 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id AA2ECC4CEF7; Thu, 25 Sep 2025 22:17:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1758838635; bh=doDwnVpI3Y2qcYJ1FHiqEpl0+kdSx6c6kK2Df8Avxew=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=lhjpssgI4D/q9AHqyV7zwPlVUKL4X3nodn1athKhuy+G0z3hnOv/SZw421jeEqpVk rY659GEaXu8Xe/SAHM5hKmyv94sMs3aG9MGRiq8SuWzAGnTb/ZQ0eZcliYDbgCs2II vbz0LSLzP53BEOWmNXj/o8FYF66c7pZznuvJAb0zPCW5XM+PcKdZdxCuCP8cW/jTAA nm5v4pfy/0/94OQ1fErYInJrqVg2NhPpeHxuY1MuzeinpjODhkI1FmoybEAFw2grP4 gfWx+JNDDooXooA6GVkUauRDB/uoYoywdU4i48GFMkZaeuhxDcMhErEYHNj3cZz4f1 bSNxI8sE+EUAw== From: Hans de Goede To: libcamera-devel@lists.libcamera.org Cc: Hans de Goede , Milan Zamazal , Kieran Bingham Subject: [PATCH v2 3/6] ipa: software_isp: AGC: Raise exposure or gain not both at the same time Date: Fri, 26 Sep 2025 00:17:05 +0200 Message-ID: <20250925221708.7471-4-hansg@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20250925221708.7471-1-hansg@kernel.org> References: <20250925221708.7471-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" Raise either exposure or gain; not both at the same time. Before this change when the last exposure raise is done, hitting exposure-max, gain would be increased at the same time. Reviewed-by: Milan Zamazal Tested-by: Milan Zamazal Tested-by: Kieran Bingham Signed-off-by: Hans de Goede --- src/ipa/simple/algorithms/agc.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp index 1fc8d7f4..f7f73451 100644 --- a/src/ipa/simple/algorithms/agc.cpp +++ b/src/ipa/simple/algorithms/agc.cpp @@ -56,12 +56,13 @@ void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, dou double &again = frameContext.sensor.gain; if (exposureMSV < kExposureOptimal - kExposureSatisfactory) { - next = exposure * kExpNumeratorUp / kExpDenominator; - if (next - exposure < 1) - exposure += 1; - else - exposure = next; - if (exposure >= context.configuration.agc.exposureMax) { + if (exposure < context.configuration.agc.exposureMax) { + next = exposure * kExpNumeratorUp / kExpDenominator; + if (next - exposure < 1) + exposure += 1; + else + exposure = next; + } else { next = again * kExpNumeratorUp / kExpDenominator; if (next - again < context.configuration.agc.againMinStep) again += context.configuration.agc.againMinStep; From patchwork Thu Sep 25 22:17:06 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: 24466 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 3F85BC3331 for ; Thu, 25 Sep 2025 22:17:29 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BEA506B614; Fri, 26 Sep 2025 00:17:28 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="DwdaPalY"; dkim-atps=neutral Received: from tor.source.kernel.org (tor.source.kernel.org [IPv6:2600:3c04:e001:324:0:1991:8:25]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 85B736B5C4 for ; Fri, 26 Sep 2025 00:17:18 +0200 (CEST) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by tor.source.kernel.org (Postfix) with ESMTP id C152D61294; Thu, 25 Sep 2025 22:17:17 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 34431C4CEF0; Thu, 25 Sep 2025 22:17:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1758838637; bh=h29K2J8OhyTE/tswAlMiK3Ct6DvEdLi83NVOXcbxAmo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=DwdaPalYC74YKNtg7kaUr274eqYci7KnNspFecQGGroecJds6MeRjAqBDrI2ARGH9 pprj7q1tIoZG23HHNO0tpv37OrASvqZVNdcb1Ge9SsJ/NgG2U34sqe9wphm+0vN1Fm pXj7Y/ksnc/oSnbM4vQyIUKHFfFbYyqZIIeRHM0xMu0hanw8jY3G+LXs87HGiOzn// jDiRtdRUN+4DgeTZ7cKCW+sWSxUy/D65snl4cguEaG8GrYenszubbuA1R+s0ITBcyA kP4cSwdvPCJdie43Xy3i2SnGRII1uyolautKalMHp6ro1sm1Uxu+7XdTM1OF9YEs49 gc9L32UwuLPFg== From: Hans de Goede To: libcamera-devel@lists.libcamera.org Cc: Hans de Goede , Milan Zamazal , Isaac Scott , Kieran Bingham Subject: [PATCH v2 4/6] ipa: software_isp: AGC: Only use integers for exposure calculations Date: Fri, 26 Sep 2025 00:17:06 +0200 Message-ID: <20250925221708.7471-5-hansg@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20250925221708.7471-1-hansg@kernel.org> References: <20250925221708.7471-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" Exposure is an integer, instead of re-using the "double next" used for again calculations, doing intermediate calculations with double precision, use a local next variable of an integer type. Reviewed-by: Milan Zamazal Reviewed-by: Isaac Scott Tested-by: Milan Zamazal Tested-by: Kieran Bingham Signed-off-by: Hans de Goede --- src/ipa/simple/algorithms/agc.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp index f7f73451..3471cc88 100644 --- a/src/ipa/simple/algorithms/agc.cpp +++ b/src/ipa/simple/algorithms/agc.cpp @@ -51,19 +51,18 @@ void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, dou static constexpr uint8_t kExpNumeratorUp = kExpDenominator + 1; static constexpr uint8_t kExpNumeratorDown = kExpDenominator - 1; - double next; int32_t &exposure = frameContext.sensor.exposure; double &again = frameContext.sensor.gain; if (exposureMSV < kExposureOptimal - kExposureSatisfactory) { if (exposure < context.configuration.agc.exposureMax) { - next = exposure * kExpNumeratorUp / kExpDenominator; + int32_t next = exposure * kExpNumeratorUp / kExpDenominator; if (next - exposure < 1) exposure += 1; else exposure = next; } else { - next = again * kExpNumeratorUp / kExpDenominator; + double next = again * kExpNumeratorUp / kExpDenominator; if (next - again < context.configuration.agc.againMinStep) again += context.configuration.agc.againMinStep; else @@ -73,13 +72,13 @@ void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, dou if (exposureMSV > kExposureOptimal + kExposureSatisfactory) { if (again > context.configuration.agc.again10) { - next = again * kExpNumeratorDown / kExpDenominator; + double next = again * kExpNumeratorDown / kExpDenominator; if (again - next < context.configuration.agc.againMinStep) again -= context.configuration.agc.againMinStep; else again = next; } else { - next = exposure * kExpNumeratorDown / kExpDenominator; + int32_t next = exposure * kExpNumeratorDown / kExpDenominator; if (exposure - next < 1) exposure -= 1; else From patchwork Thu Sep 25 22:17:07 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: 24467 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 84113C3332 for ; Thu, 25 Sep 2025 22:17:31 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 77DCA6B601; Fri, 26 Sep 2025 00:17:30 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="P+rX+Bha"; dkim-atps=neutral Received: from tor.source.kernel.org (tor.source.kernel.org [IPv6:2600:3c04:e001:324:0:1991:8:25]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id CB93969367 for ; Fri, 26 Sep 2025 00:17:19 +0200 (CEST) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by tor.source.kernel.org (Postfix) with ESMTP id E616961296 for ; Thu, 25 Sep 2025 22:17:18 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E19C6C4CEF7; Thu, 25 Sep 2025 22:17:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1758838638; bh=6EgO+Cwy6VnHEnTNl8HJyuH0z9NTyUOpzOrpIFPwbFk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=P+rX+BhaV9IJwHLMT0JMpauaNJLkaA0b2W60dX9XJ/hCHOgsFLZdtsbZ/JfjR47JB O9qctxvth9pNMyMactv4VnjTArn9kEXpYku6SWbEJl42MSBd3qeY05OIO2CI3RikYR SrDiORrNUJwmabU1AfbdspaluPAChBmNkk2wWJJt6coMEk84q+0CXCxzACY/ebD5w/ DVLIkxSO+k0U5Flk64X/Kbh7obXn41nDY1c/KuA2Hd5IUdCMIHR35/QpXOvTPSmm02 y4nTSs9uGkZE5vCk8tAuQlO5Wmj1R2/LNyu4Knp3sWUn5MbbTZpHeRaunw3ISRQIU/ hlOzLofQN6BEQ== From: Hans de Goede To: libcamera-devel@lists.libcamera.org Cc: Hans de Goede Subject: [PATCH v2 5/6] libcamera: software_isp: Add valid flag to struct SwIspStats Date: Fri, 26 Sep 2025 00:17:07 +0200 Message-ID: <20250925221708.7471-6-hansg@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20250925221708.7471-1-hansg@kernel.org> References: <20250925221708.7471-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" Generating statistics for every single frame is not really necessary. However a roundtrip through ipa_->processStats() still need to be done every frame, even if there are no stats to make the IPA generate metadata for every frame. Add a valid flag to the statistics struct to let the IPA know when there are no statistics for the frame being processed and modify the IPA to only generate metadata for frames without valid statistics. This is a preparation patch for skipping statistics generation for some frames. Signed-off-by: Hans de Goede --- include/libcamera/internal/software_isp/swisp_stats.h | 4 ++++ src/ipa/simple/algorithms/agc.cpp | 3 +++ src/ipa/simple/algorithms/awb.cpp | 3 +++ src/ipa/simple/algorithms/blc.cpp | 3 +++ src/ipa/simple/soft_simple.cpp | 3 +++ src/libcamera/software_isp/debayer_cpu.cpp | 2 +- src/libcamera/software_isp/swstats_cpu.cpp | 4 +++- src/libcamera/software_isp/swstats_cpu.h | 2 +- 8 files changed, 21 insertions(+), 3 deletions(-) diff --git a/include/libcamera/internal/software_isp/swisp_stats.h b/include/libcamera/internal/software_isp/swisp_stats.h index ae11f112..cbcfd351 100644 --- a/include/libcamera/internal/software_isp/swisp_stats.h +++ b/include/libcamera/internal/software_isp/swisp_stats.h @@ -20,6 +20,10 @@ namespace libcamera { * wrap around. */ struct SwIspStats { + /** + * \brief Indicates if the statistics buffer contains valid data + */ + bool valid; /** * \brief Holds the sum of all sampled red pixels */ diff --git a/src/ipa/simple/algorithms/agc.cpp b/src/ipa/simple/algorithms/agc.cpp index 3471cc88..e4e24113 100644 --- a/src/ipa/simple/algorithms/agc.cpp +++ b/src/ipa/simple/algorithms/agc.cpp @@ -107,6 +107,9 @@ void Agc::process(IPAContext &context, metadata.set(controls::ExposureTime, exposureTime.get()); metadata.set(controls::AnalogueGain, frameContext.sensor.gain); + if (!stats->valid) + return; + /* * Calculate Mean Sample Value (MSV) according to formula from: * https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp index cf567e89..ddd0b791 100644 --- a/src/ipa/simple/algorithms/awb.cpp +++ b/src/ipa/simple/algorithms/awb.cpp @@ -61,6 +61,9 @@ void Awb::process(IPAContext &context, }; metadata.set(controls::ColourGains, mdGains); + if (!stats->valid) + return; + /* * Black level must be subtracted to get the correct AWB ratios, they * would be off if they were computed from the whole brightness range diff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp index 8c1e9ed0..616da0ee 100644 --- a/src/ipa/simple/algorithms/blc.cpp +++ b/src/ipa/simple/algorithms/blc.cpp @@ -60,6 +60,9 @@ void BlackLevel::process(IPAContext &context, }; metadata.set(controls::SensorBlackLevels, blackLevels); + if (!stats->valid) + return; + if (context.configuration.black.level.has_value()) return; diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index b147aca2..e1c8e21a 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -318,6 +318,9 @@ void IPASoftSimple::processStats(const uint32_t frame, algo->process(context_, frame, frameContext, stats_, metadata); metadataReady.emit(frame, metadata); + if (!stats_->valid) + return; + /* Sanity check */ if (!sensorControls.contains(V4L2_CID_EXPOSURE) || !sensorControls.contains(V4L2_CID_ANALOGUE_GAIN)) { diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp index 2dc85e5e..bfa60888 100644 --- a/src/libcamera/software_isp/debayer_cpu.cpp +++ b/src/libcamera/software_isp/debayer_cpu.cpp @@ -851,7 +851,7 @@ void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output * * \todo Pass real bufferId once stats buffer passing is changed. */ - stats_->finishFrame(frame, 0); + stats_->finishFrame(frame, 0, true); outputBufferReady.emit(output); inputBufferReady.emit(input); } diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp index 4b77b360..da91f912 100644 --- a/src/libcamera/software_isp/swstats_cpu.cpp +++ b/src/libcamera/software_isp/swstats_cpu.cpp @@ -313,11 +313,13 @@ void SwStatsCpu::startFrame(void) * \brief Finish statistics calculation for the current frame * \param[in] frame The frame number * \param[in] bufferId ID of the statistics buffer + * \param[in] valid Indicates if the statistics for the current frame are valid * * This may only be called after a successful setWindow() call. */ -void SwStatsCpu::finishFrame(uint32_t frame, uint32_t bufferId) +void SwStatsCpu::finishFrame(uint32_t frame, uint32_t bufferId, bool valid) { + stats_.valid = valid; *sharedStats_ = stats_; statsReady.emit(frame, bufferId); } diff --git a/src/libcamera/software_isp/swstats_cpu.h b/src/libcamera/software_isp/swstats_cpu.h index 26a2f462..6ac3c4de 100644 --- a/src/libcamera/software_isp/swstats_cpu.h +++ b/src/libcamera/software_isp/swstats_cpu.h @@ -41,7 +41,7 @@ public: int configure(const StreamConfiguration &inputCfg); void setWindow(const Rectangle &window); void startFrame(); - void finishFrame(uint32_t frame, uint32_t bufferId); + void finishFrame(uint32_t frame, uint32_t bufferId, bool valid); void processLine0(unsigned int y, const uint8_t *src[]) { From patchwork Thu Sep 25 22:17:08 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: 24468 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 C502DC3333 for ; Thu, 25 Sep 2025 22:17:33 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id F145F6B60E; Fri, 26 Sep 2025 00:17:32 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="uRc9mwiv"; dkim-atps=neutral Received: from sea.source.kernel.org (sea.source.kernel.org [172.234.252.31]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 007EF6B60F for ; Fri, 26 Sep 2025 00:17:20 +0200 (CEST) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sea.source.kernel.org (Postfix) with ESMTP id DD8264561F for ; Thu, 25 Sep 2025 22:17:19 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 12A01C4CEF7; Thu, 25 Sep 2025 22:17:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1758838639; bh=RH0Hcg13gyC2TYAJCCRxUwCSNWWjHlPrrMNTnoh9sgQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=uRc9mwivHLDvzHvjUesUfLu6s1RjT0ZHWsGxRvjyH8KLFrmDqZWXuWXnecwGzXw5I ev8hvSRH0iBhGfyR7OJQVS7XJUaXUo3E5GW93cywTc52ZX02fWJGestegIE53qQWpg 7i60zURpp2cg4N2ChgeYczGzqLBI0xg47F5mQZVtGsJbFidPmvy6jwQWNepnDzNbY1 xzFl+oFuGoLdm8AyI16OLfaEyS/lpWeMCbdKkqXq+YzOIS9gOqJpeXL62w7RR9h2kX I9NI48ReGgXKDsyT4nCe+ZjIvntbTbMN8VzD4TLvCrffD5vW4Bh/K4094Mz2X/ZRt+ hAnvPSmMOwrqA== From: Hans de Goede To: libcamera-devel@lists.libcamera.org Cc: Hans de Goede Subject: [PATCH v2 6/6] libcamera: software_isp: Run sw-statistics once every 4th frame Date: Fri, 26 Sep 2025 00:17:08 +0200 Message-ID: <20250925221708.7471-7-hansg@kernel.org> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20250925221708.7471-1-hansg@kernel.org> References: <20250925221708.7471-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" Run sw-statistics once every 4th frame, instead of every frame. There are 2 reasons for this: 1. There really is no need to have statistics for every frame and only doing this every 4th frame helps save some CPU time. 2. The generic nature of the simple pipeline-handler, so no information about possible CSI receiver frame-delays. In combination with the software ISP often being used with sensors without sensor info in the sensor-helper code, so no reliable control-delay information means that the software ISP is prone to AGC oscillation. Skipping statistics gathering also means skipping running the AGC algorithm slowing it down, avoiding this oscillation. Note ideally the AGC oscillation problem would be fixed by adding sensor metadata support all through the stack so that the exact gain and exposure used for a specific frame are reliably provided by the sensor metadata. Signed-off-by: Hans de Goede --- src/libcamera/software_isp/debayer_cpu.cpp | 25 +++++++++++++--------- src/libcamera/software_isp/debayer_cpu.h | 4 ++-- src/libcamera/software_isp/swstats_cpu.cpp | 5 +++++ src/libcamera/software_isp/swstats_cpu.h | 3 +++ 4 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp index bfa60888..9010333e 100644 --- a/src/libcamera/software_isp/debayer_cpu.cpp +++ b/src/libcamera/software_isp/debayer_cpu.cpp @@ -655,7 +655,7 @@ void DebayerCpu::memcpyNextLine(const uint8_t *linePointers[]) lineBufferIndex_ = (lineBufferIndex_ + 1) % (patternHeight + 1); } -void DebayerCpu::process2(const uint8_t *src, uint8_t *dst) +void DebayerCpu::process2(uint32_t frame, const uint8_t *src, uint8_t *dst) { unsigned int yEnd = window_.y + window_.height; /* Holds [0] previous- [1] current- [2] next-line */ @@ -681,7 +681,8 @@ void DebayerCpu::process2(const uint8_t *src, uint8_t *dst) for (unsigned int y = window_.y; y < yEnd; y += 2) { shiftLinePointers(linePointers, src); memcpyNextLine(linePointers); - stats_->processLine0(y, linePointers); + if (frame % SwStatsCpu::kStatPerNumFrames == 0) + stats_->processLine0(y, linePointers); (this->*debayer0_)(dst, linePointers); src += inputConfig_.stride; dst += outputConfig_.stride; @@ -696,7 +697,8 @@ void DebayerCpu::process2(const uint8_t *src, uint8_t *dst) if (window_.y == 0) { shiftLinePointers(linePointers, src); memcpyNextLine(linePointers); - stats_->processLine0(yEnd, linePointers); + if (frame % SwStatsCpu::kStatPerNumFrames == 0) + stats_->processLine0(yEnd, linePointers); (this->*debayer0_)(dst, linePointers); src += inputConfig_.stride; dst += outputConfig_.stride; @@ -710,7 +712,7 @@ void DebayerCpu::process2(const uint8_t *src, uint8_t *dst) } } -void DebayerCpu::process4(const uint8_t *src, uint8_t *dst) +void DebayerCpu::process4(uint32_t frame, const uint8_t *src, uint8_t *dst) { const unsigned int yEnd = window_.y + window_.height; /* @@ -733,7 +735,8 @@ void DebayerCpu::process4(const uint8_t *src, uint8_t *dst) for (unsigned int y = window_.y; y < yEnd; y += 4) { shiftLinePointers(linePointers, src); memcpyNextLine(linePointers); - stats_->processLine0(y, linePointers); + if (frame % SwStatsCpu::kStatPerNumFrames == 0) + stats_->processLine0(y, linePointers); (this->*debayer0_)(dst, linePointers); src += inputConfig_.stride; dst += outputConfig_.stride; @@ -746,7 +749,8 @@ void DebayerCpu::process4(const uint8_t *src, uint8_t *dst) shiftLinePointers(linePointers, src); memcpyNextLine(linePointers); - stats_->processLine2(y, linePointers); + if (frame % SwStatsCpu::kStatPerNumFrames == 0) + stats_->processLine2(y, linePointers); (this->*debayer2_)(dst, linePointers); src += inputConfig_.stride; dst += outputConfig_.stride; @@ -821,12 +825,13 @@ void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output return; } - stats_->startFrame(); + if (frame % SwStatsCpu::kStatPerNumFrames == 0) + stats_->startFrame(); if (inputConfig_.patternSize.height == 2) - process2(in.planes()[0].data(), out.planes()[0].data()); + process2(frame, in.planes()[0].data(), out.planes()[0].data()); else - process4(in.planes()[0].data(), out.planes()[0].data()); + process4(frame, in.planes()[0].data(), out.planes()[0].data()); metadata.planes()[0].bytesused = out.planes()[0].size(); @@ -851,7 +856,7 @@ void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output * * \todo Pass real bufferId once stats buffer passing is changed. */ - stats_->finishFrame(frame, 0, true); + stats_->finishFrame(frame, 0, frame % SwStatsCpu::kStatPerNumFrames == 0); outputBufferReady.emit(output); inputBufferReady.emit(input); } diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h index 9d343e46..03e0d784 100644 --- a/src/libcamera/software_isp/debayer_cpu.h +++ b/src/libcamera/software_isp/debayer_cpu.h @@ -133,8 +133,8 @@ private: void setupInputMemcpy(const uint8_t *linePointers[]); void shiftLinePointers(const uint8_t *linePointers[], const uint8_t *src); void memcpyNextLine(const uint8_t *linePointers[]); - void process2(const uint8_t *src, uint8_t *dst); - void process4(const uint8_t *src, uint8_t *dst); + void process2(uint32_t frame, const uint8_t *src, uint8_t *dst); + void process4(uint32_t frame, const uint8_t *src, uint8_t *dst); /* Max. supported Bayer pattern height is 4, debayering this requires 5 lines */ static constexpr unsigned int kMaxLineBuffers = 5; diff --git a/src/libcamera/software_isp/swstats_cpu.cpp b/src/libcamera/software_isp/swstats_cpu.cpp index da91f912..35ba0a46 100644 --- a/src/libcamera/software_isp/swstats_cpu.cpp +++ b/src/libcamera/software_isp/swstats_cpu.cpp @@ -89,6 +89,11 @@ namespace libcamera { * \brief Signals that the statistics are ready */ +/** + * \var SwStatsCpu::kStatPerNumFrames + * \brief Run stats once every kStatPerNumFrames frames + */ + /** * \typedef SwStatsCpu::statsProcessFn * \brief Called when there is data to get statistics from diff --git a/src/libcamera/software_isp/swstats_cpu.h b/src/libcamera/software_isp/swstats_cpu.h index 6ac3c4de..ea0e6d5a 100644 --- a/src/libcamera/software_isp/swstats_cpu.h +++ b/src/libcamera/software_isp/swstats_cpu.h @@ -32,6 +32,9 @@ public: SwStatsCpu(); ~SwStatsCpu() = default; + /* Run stats once every 4 frames */ + static constexpr uint32_t kStatPerNumFrames = 4; + bool isValid() const { return sharedStats_.fd().isValid(); } const SharedFD &getStatsFD() { return sharedStats_.fd(); }