From patchwork Sat Sep 27 17:59:59 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: 24479 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 C7187BDB1C for ; Sat, 27 Sep 2025 18:00:17 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3CA636B605; Sat, 27 Sep 2025 20:00:16 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="A2vHpYFe"; 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 AC7306B5A2 for ; Sat, 27 Sep 2025 20:00:10 +0200 (CEST) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by tor.source.kernel.org (Postfix) with ESMTP id 5350C620A9; Sat, 27 Sep 2025 18:00:09 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id BAB93C113CF; Sat, 27 Sep 2025 18:00:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1758996009; bh=i8luKV32QotAPxo62xELugb2IsfDO0Szq6/oYQ9c35A=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=A2vHpYFe7icjYEALGq4q4bxiK6uzhhUIGI0y9oNs2hpwbj2v94C0pB/KwBN7BWgeS xv4Jbu/ybtZEKwpMIZ6i7C7BuUjdMeW701WbhijIrxCRtRwlys1n2PRNqgKkInn6cQ oIx5c0X5o+/+H94QScxvP3vdirzq2Kewj1S7iSI9HXwnI0eZS/Z2R2+rtlIRiCxW/b EJxnlL2xhL9v+NtuqgIVJ64fU7Qzavn5l+pXBQe4xgI8rA67xg45bs0g7ZxaQIbZos 7vQ+iL35vFEu8jQdfXyv4JereAzeyhQC80PWPxlao3AviGTUpSXQEj4SrC5X0164Xz vcNjuSbeAjcpg== From: Hans de Goede To: libcamera-devel@lists.libcamera.org Cc: Hans de Goede , Milan Zamazal , Isaac Scott , Kieran Bingham Subject: [PATCH v3 1/6] ipa: software_isp: Fix context_.configuration.agc.againMin init Date: Sat, 27 Sep 2025 19:59:59 +0200 Message-ID: <20250927180004.84620-2-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" 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 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 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 Sat Sep 27 18:00:01 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: 24481 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 0976AC32A9 for ; Sat, 27 Sep 2025 18:00:23 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 914116B5AA; Sat, 27 Sep 2025 20:00:22 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="hZqT/+IA"; 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 918046B5A2 for ; Sat, 27 Sep 2025 20:00:13 +0200 (CEST) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sea.source.kernel.org (Postfix) with ESMTP id 3E03544A46; Sat, 27 Sep 2025 18:00:12 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 053CCC4CEF8; Sat, 27 Sep 2025 18:00:10 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1758996012; bh=doDwnVpI3Y2qcYJ1FHiqEpl0+kdSx6c6kK2Df8Avxew=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=hZqT/+IAAb3h75Kk5r2L21NihEnhsLK4iLvC0/YlkclKxo3NwIfNANCpddnEdWxAc +Pd5QF5UFI6OY7VLCOL4Sr/hpNKlsZvjuxZG0fsDo0Hgx1OlV4FQLNCtxpwnix8+Qb vCV8zOzzgcDvgZCTQUbEB4VQP0qYAXuYXsHfw6PWIwC8BGj/9QVDMr5Ou56mIs/wvt Gw6t/6IiLAL2A8UyuUGCPHeRZY4/QOP46ro91US7ueVqbbcwDJFzA7kCSWk+iMa9oe Y4qAgO6exNFbBNiodv2n/2bjpSF85qnmFxX3zHBO+uLFNyY3b+1lIkwpVumM6TX7LO U9W8OHK9VaAoQ== From: Hans de Goede To: libcamera-devel@lists.libcamera.org Cc: Hans de Goede , Milan Zamazal , Kieran Bingham Subject: [PATCH v3 3/6] ipa: software_isp: AGC: Raise exposure or gain not both at the same time Date: Sat, 27 Sep 2025 20:00:01 +0200 Message-ID: <20250927180004.84620-4-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" 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 Sat Sep 27 18:00:02 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: 24482 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 6D848BDB1C for ; Sat, 27 Sep 2025 18:00:25 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 164B46B5F8; Sat, 27 Sep 2025 20:00:24 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="W+C/jSi5"; 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 E53A16B5F0 for ; Sat, 27 Sep 2025 20:00:14 +0200 (CEST) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by tor.source.kernel.org (Postfix) with ESMTP id 27545620A9; Sat, 27 Sep 2025 18:00:14 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 870C4C4CEF8; Sat, 27 Sep 2025 18:00:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1758996013; bh=h29K2J8OhyTE/tswAlMiK3Ct6DvEdLi83NVOXcbxAmo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=W+C/jSi5I7XQwHXCN077GIWicHNt4U+7rUf4cXgkcRNp697gu+oagJwUUMQWg1lIj KBcecKiDpwRtrO+M74Z7B1PoJL14GDfCXoAWjPw3upV3JuCZjuijERdGcQxeNL0SE/ CN7iLw7qZVCY4T4M7DeZq1mOC5ZBiiC+THHY8b+qhviuHGO4BMV8/yZH+mSwai8o7s 7/gdTyShzg0H8OQWQN4zDOyHVWSJPh82Nur9QYPGcxUKKdFLuPdlrdmBRQfqWzh6ga cWd3VwDkH42t7ViEyNFRoixruT/Lt/blzj7To3TN6sfLEGRZPu2urFYC9ZBVWJWa9+ KqH1b1IeVXQFA== From: Hans de Goede To: libcamera-devel@lists.libcamera.org Cc: Hans de Goede , Milan Zamazal , Isaac Scott , Kieran Bingham Subject: [PATCH v3 4/6] ipa: software_isp: AGC: Only use integers for exposure calculations Date: Sat, 27 Sep 2025 20:00:02 +0200 Message-ID: <20250927180004.84620-5-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" 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 Sat Sep 27 18:00: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: 24483 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 11A13BDB1C for ; Sat, 27 Sep 2025 18:00:27 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 68D786B5FE; Sat, 27 Sep 2025 20:00:26 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="HKi+qpa4"; 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 3FF5A6B606 for ; Sat, 27 Sep 2025 20:00:16 +0200 (CEST) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sea.source.kernel.org (Postfix) with ESMTP id 1BDF3429C2 for ; Sat, 27 Sep 2025 18:00:15 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 42299C4CEF8; Sat, 27 Sep 2025 18:00:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1758996015; bh=QalXbPH57usXHlRd899QCqINP1MdGetO/KuHHwL0sJQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=HKi+qpa4sYLoZM3ncLktxgc/HXuseYBekA5ik9MisiWtT6uoVk8LqY+Qkh3wBVjdc auqJpmP7YVyELqTxURp4jX2ThBIAZy4meP5dxtlp0W6W1MKoaqcDVzrUxmPmG3hSHW C2yvVmgLCqoQuX8TNWyxM4f0SV6lpfV76/RRiCCWp4hDLTzg8eo3fb+vfc3VmBkZAs t6bFZ4gMO3zrI424Ff2wRnQRZhYuZ0BO/vieKXoqGzSaYZmDtUGXmtmFMwUv6gRRlh QupYylwADzDYEut9TKdAwbCjY8bCs2dPUiffo6AOmAuEmlqVDR0h16mzFDy61WGN/k O+qBZht9eg2Kw== From: Hans de Goede To: libcamera-devel@lists.libcamera.org Cc: Hans de Goede Subject: [PATCH v3 5/6] libcamera: software_isp: Add valid flag to struct SwIspStats Date: Sat, 27 Sep 2025 20:00:03 +0200 Message-ID: <20250927180004.84620-6-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" 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 Reviewed-by: Kieran Bingham Reviewed-by: Milan Zamazal Tested-by: Milan Zamazal --- Changes in v3: - Drop if (!stats_->valid) early exit from IPASoftSimple::processStats() - Save last AGC calculated new gain and exposure values and re-use these for frames without statistics, this avoids delayedCtrl history underruns - Improve SwIspStats.valid documentation Changes in v2: - This is a new patch in v2 of this series --- .../libcamera/internal/software_isp/swisp_stats.h | 5 +++++ src/ipa/simple/algorithms/agc.cpp | 13 +++++++++++++ src/ipa/simple/algorithms/awb.cpp | 3 +++ src/ipa/simple/algorithms/blc.cpp | 3 +++ src/ipa/simple/ipa_context.h | 4 ++++ 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, 33 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..3c986018 100644 --- a/include/libcamera/internal/software_isp/swisp_stats.h +++ b/include/libcamera/internal/software_isp/swisp_stats.h @@ -20,6 +20,11 @@ namespace libcamera { * wrap around. */ struct SwIspStats { + /** + * \brief True if the statistics buffer contains valid data, false if + * no statistics were generated for this frame + */ + 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..af660a03 100644 --- a/src/ipa/simple/algorithms/agc.cpp +++ b/src/ipa/simple/algorithms/agc.cpp @@ -91,6 +91,9 @@ void Agc::updateExposure(IPAContext &context, IPAFrameContext &frameContext, dou again = std::clamp(again, context.configuration.agc.againMin, context.configuration.agc.againMax); + context.activeState.agc.exposure = exposure; + context.activeState.agc.again = again; + LOG(IPASoftExposure, Debug) << "exposureMSV " << exposureMSV << " exp " << exposure << " again " << again; @@ -107,6 +110,16 @@ void Agc::process(IPAContext &context, metadata.set(controls::ExposureTime, exposureTime.get()); metadata.set(controls::AnalogueGain, frameContext.sensor.gain); + if (!stats->valid) { + /* + * Use the new exposure and gain values calculated the last time + * there were valid stats. + */ + frameContext.sensor.exposure = context.activeState.agc.exposure; + frameContext.sensor.gain = context.activeState.agc.again; + 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/ipa_context.h b/src/ipa/simple/ipa_context.h index 468fccab..441f2a73 100644 --- a/src/ipa/simple/ipa_context.h +++ b/src/ipa/simple/ipa_context.h @@ -37,6 +37,10 @@ struct IPASessionConfiguration { }; struct IPAActiveState { + struct { + int32_t exposure; + double again; + } agc; struct { uint8_t level; int32_t lastExposure; 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 Sat Sep 27 18:00: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: 24484 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 418DFC328C for ; Sat, 27 Sep 2025 18:00:29 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id B4E9C6B605; Sat, 27 Sep 2025 20:00:28 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=kernel.org header.i=@kernel.org header.b="gJwGsTak"; 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 507D26B611 for ; Sat, 27 Sep 2025 20:00:17 +0200 (CEST) Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by tor.source.kernel.org (Postfix) with ESMTP id 70383620A9 for ; Sat, 27 Sep 2025 18:00:16 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6995DC113CF; Sat, 27 Sep 2025 18:00:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1758996016; bh=k+ifv+/UE1SUwzkMWeT6njsoiq5gwpUmHDTnph+s+dc=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=gJwGsTaka1f7d22tB5lcaxELWiE7TWC+Zj8NcCgBeMJxXm6ffhCZjDbw93R4gnYGM tWqy8f5gkHoxJ9zzf15+57biTfF7I2CumujWrH5hYQmTxuoUGC9dk2B76Zqz8k0D/I CWt/wwpkrVyNPtl8WBA4OE/Y7GJ97cv6muO/daKSrYjCkweM8/WBGWdPnY+bSwVFaz KsbOjxoO9DKGvU1uaZs+KvJhmdGDnpu+4QytvXu3HizjYrex2MLaQ1NJf/hcUB9Jbv 7dhXQ3BmB441ah/fyjavXTueU6zFeb02ob8nXi2RDcRgAQesFgT+fm/GuZ3CFvZDpk fYIF81p7wrwjQ== From: Hans de Goede To: libcamera-devel@lists.libcamera.org Cc: Hans de Goede Subject: [PATCH v3 6/6] libcamera: software_isp: Run sw-statistics once every 4th frame Date: Sat, 27 Sep 2025 20:00:04 +0200 Message-ID: <20250927180004.84620-7-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" 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 Reviewed-by: Kieran Bingham Tested-by: Milan Zamazal --- Changes in v2: - This is a new patch in v2 of this series --- 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(); }