From patchwork Mon Jul 21 07:47:21 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 23860 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 6AB78BDCC1 for ; Mon, 21 Jul 2025 07:49:04 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3660B68FD0; Mon, 21 Jul 2025 09:49:00 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="NnkI+N6W"; dkim-atps=neutral Received: from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com [IPv6:2a00:1450:4864:20::32b]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D7BB968FC0 for ; Mon, 21 Jul 2025 09:48:56 +0200 (CEST) Received: by mail-wm1-x32b.google.com with SMTP id 5b1f17b1804b1-456382f1a56so1394735e9.1 for ; Mon, 21 Jul 2025 00:48:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; t=1753084136; x=1753688936; darn=lists.libcamera.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=+/xwyzCEXzzigrIhr+7k3LnH+V+nCWBfygRA7NkkKPA=; b=NnkI+N6WBmtqM9WcZudIlRzwYn/ZFUNwaz6VADb+wD3v6s+c3EGvmPF0VEhbP+mBT6 WjUhhrcuy609dnhTtqMSIIqL3Kpr0yTMz9ZSR+f+Q/x5WSLUPcfeJJxzDzYXzilPKac7 fSdHw3e0f5NDshlqVw17eRoibiDhNIKdQ58wPX+bIHNGquiWxLrmkehnXPyAiDpA30fw 3H2C+MQyGrlOG/VcBAM/WqHOwOxSGOoOdZ1FQuBxQLTWaI/XJSqpwjyjPywef6vvpSTz S1WY7NauHc57WFuBn3ebog+LqPN8tPo7QZMFWPcikadpfPx40d4mQp9XoAQqAfe+XHAE TKNg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1753084136; x=1753688936; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=+/xwyzCEXzzigrIhr+7k3LnH+V+nCWBfygRA7NkkKPA=; b=WidVpswDzZq+BI0++hOGzYeNqFxYsyT/dYCsJ/kUxKDpK12gHPYXfTzIxpnXxsZw3K FVm9slXePoC4p27cABIkk8cWxh/71zZZNj4BBOMdkYxFlBVf2SyY/XA/Jdavhkgif0qy kH7Lm454KADsouMMk165dVd+4l1XEshc1wc6r8izlVf9QEXalg7I59TaZ6paaqHU8QJx +fO9jq3wVLwmdfl6ixEq3IYon8c+BM/UuN1dYPdvkn0ZAwuDQGN6TAXm/gi6PuesMzXD dMf8fFcKT3dEm4udvTMMSM9OxvvoHQDqyi9PwTVMhVFHzVX+szrxUpAhUJH4OG1QDmcM emOg== X-Gm-Message-State: AOJu0YwixMWn0Fa/ZhabudmavmYyvLPcb5ZGmFW5OyIGRlNRgm2xOQxa o5r95JK/9EY4ej3CGYk0CdCf5LsJp8ELYjB2B1AXzmcgvwssG3h4xHzw6QqEUSnI6zVkJAP6tse yK+MZPdY= X-Gm-Gg: ASbGnctfOZORmVV7uKstnw4PmE8A/+7/KQ+JYrPaJFJvTXLCXI91UjUWsgBsTr4aPCp Zktqxzcl2M5WHu3F0HgURE/PIi+kmquKLYW1naOuSgpsrnQA5lH/1ccr8Pt8hLTfpMPGGkNDVYh XAgHzVximg96EaK8QrE0TwlchLUT7MCZAeCG7JpaffUWFg9xhcsv5ELqm69XMCC2zM/CgQE9D3A uojnNjmGYABYkBvDBWHe1tTgQjqigsgJdDK7ffgb0eJbEfK4YMpms8JMa9Jp7s3QhjJRVA5vdIb UDCCjORGMVXOLhIY3wkjc5S4Tf1NfOc9da1rnQnWiK30l67VPzyU3QwhTdlSctA8zw22icq62dZ 8Hiw8ZHVXqTraiQlO3Aa1JSDHfzrtQggm4xTke2P98V5mSaZL9dIi X-Google-Smtp-Source: AGHT+IF+KYEVDQyvG12+oDz9H810iKfvPuRdOaGdm7eZJ1iGVZ6UoOq95YZmuYnE/G+z6/MbZxzcbQ== X-Received: by 2002:adf:b601:0:b0:3a4:eb46:7258 with SMTP id ffacd0b85a97d-3b60dd892c5mr5164280f8f.15.1753084135957; Mon, 21 Jul 2025 00:48:55 -0700 (PDT) Received: from NAUSH-P-DELL.pitowers.org ([93.93.133.154]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4562e802afasm151101765e9.12.2025.07.21.00.48.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Jul 2025 00:48:55 -0700 (PDT) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Cc: David Plowman , Naushir Patuck Subject: [PATCH v2 1/7] ipa: rpi: agc: Change handling of colour gains less than 1 Date: Mon, 21 Jul 2025 08:47:21 +0100 Message-ID: <20250721074853.1463358-2-naush@raspberrypi.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250721074853.1463358-1-naush@raspberrypi.com> References: <20250721074853.1463358-1-naush@raspberrypi.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" From: David Plowman Previously these were handled in the AGC/AEC exposure update calculations by explicitly driving a higher digital gain to "cancel out" any colour gains that were less than 1. Now we're ignoring this in the AGC and leaving it to the IPA code to normalise all the gains so that the smallest is 1. We don't regard this as a "real" increase because one of the colour channels (just not necessarily the green one) still gets the minimum gain possible. We do, however, update the statistics calculations so that they reflect any such digital gain increase, so that images are driven to the correct level. Signed-off-by: David Plowman Signed-off-by: Naushir Patuck Reviewed-by: Naushir Patuck --- src/ipa/rpi/controller/rpi/agc_channel.cpp | 41 +++++++--------------- src/ipa/rpi/pisp/pisp.cpp | 26 ++++++++++---- src/ipa/rpi/vc4/vc4.cpp | 26 +++++++++++--- 3 files changed, 54 insertions(+), 39 deletions(-) diff --git a/src/ipa/rpi/controller/rpi/agc_channel.cpp b/src/ipa/rpi/controller/rpi/agc_channel.cpp index b2999364bf27..ae0cb148893a 100644 --- a/src/ipa/rpi/controller/rpi/agc_channel.cpp +++ b/src/ipa/rpi/controller/rpi/agc_channel.cpp @@ -435,14 +435,9 @@ void AgcChannel::switchMode(CameraMode const &cameraMode, Duration fixedExposureTime = limitExposureTime(fixedExposureTime_); if (fixedExposureTime && fixedAnalogueGain_) { - /* We're going to reset the algorithm here with these fixed values. */ - fetchAwbStatus(metadata); - double minColourGain = std::min({ awb_.gainR, awb_.gainG, awb_.gainB, 1.0 }); - ASSERT(minColourGain != 0.0); - /* This is the equivalent of computeTargetExposure and applyDigitalGain. */ target_.totalExposureNoDG = fixedExposureTime_ * fixedAnalogueGain_; - target_.totalExposure = target_.totalExposureNoDG / minColourGain; + target_.totalExposure = target_.totalExposureNoDG; /* Equivalent of filterExposure. This resets any "history". */ filtered_ = target_; @@ -462,10 +457,10 @@ void AgcChannel::switchMode(CameraMode const &cameraMode, */ double ratio = lastSensitivity / cameraMode.sensitivity; - target_.totalExposureNoDG *= ratio; target_.totalExposure *= ratio; - filtered_.totalExposureNoDG *= ratio; + target_.totalExposureNoDG = target_.totalExposure; filtered_.totalExposure *= ratio; + filtered_.totalExposureNoDG = filtered_.totalExposure; divideUpExposure(); } else { @@ -716,8 +711,13 @@ static double computeInitialY(StatisticsPtr &stats, AwbStatus const &awb, } /* Factor in the AWB correction if needed. */ - if (stats->agcStatsPos == Statistics::AgcStatsPos::PreWb) - sum *= RGB{ { awb.gainR, awb.gainG, awb.gainB } }; + if (stats->agcStatsPos == Statistics::AgcStatsPos::PreWb) { + double minColourGain = std::min({ awb.gainR, awb.gainG, awb.gainB, 1.0 }); + minColourGain = std::max(minColourGain, 1.0); + RGB colourGains{ { awb.gainR, awb.gainG, awb.gainB } }; + colourGains /= minColourGain; + sum *= colourGains; + } double ySum = ipa::rec601LuminanceFromRGB(sum); @@ -797,16 +797,8 @@ void AgcChannel::computeGain(StatisticsPtr &statistics, Metadata *imageMetadata, void AgcChannel::computeTargetExposure(double gain) { if (status_.fixedExposureTime && status_.fixedAnalogueGain) { - /* - * When analogue gain and exposure time are both fixed, we need - * to drive the total exposure so that we end up with a digital - * gain of at least 1/minColourGain. Otherwise we'd desaturate - * channels causing white to go cyan or magenta. - */ - double minColourGain = std::min({ awb_.gainR, awb_.gainG, awb_.gainB, 1.0 }); - ASSERT(minColourGain != 0.0); target_.totalExposure = - status_.fixedExposureTime * status_.fixedAnalogueGain / minColourGain; + status_.fixedExposureTime * status_.fixedAnalogueGain; } else { /* * The statistics reflect the image without digital gain, so the final @@ -867,15 +859,8 @@ bool AgcChannel::applyChannelConstraints(const AgcChannelTotalExposures &channel bool AgcChannel::applyDigitalGain(double gain, double targetY, bool channelBound) { - double minColourGain = std::min({ awb_.gainR, awb_.gainG, awb_.gainB, 1.0 }); - ASSERT(minColourGain != 0.0); - double dg = 1.0 / minColourGain; - /* - * I think this pipeline subtracts black level and rescales before we - * get the stats, so no need to worry about it. - */ - LOG(RPiAgc, Debug) << "after AWB, target dg " << dg << " gain " << gain - << " target_Y " << targetY; + double dg = 1.0; + /* * Finally, if we're trying to reduce exposure but the target_Y is * "close" to 1.0, then the gain computed for that constraint will be diff --git a/src/ipa/rpi/pisp/pisp.cpp b/src/ipa/rpi/pisp/pisp.cpp index bb50a9e05904..e1a804f533bb 100644 --- a/src/ipa/rpi/pisp/pisp.cpp +++ b/src/ipa/rpi/pisp/pisp.cpp @@ -521,10 +521,24 @@ void IpaPiSP::applyWBG(const AwbStatus *awbStatus, const AgcPrepareStatus *agcPr pisp_wbg_config wbg; pisp_fe_rgby_config rgby = {}; double dg = agcPrepareStatus ? agcPrepareStatus->digitalGain : 1.0; + double minColourGain = std::min({ awbStatus->gainR, awbStatus->gainG, awbStatus->gainB, 1.0 }); + /* The 0.1 here doesn't mean much, but just stops arithmetic errors and extreme behaviour. */ + double extraGain = 1.0 / std::max({ minColourGain, 0.1 }); - wbg.gain_r = clampField(dg * awbStatus->gainR, 14, 10); - wbg.gain_g = clampField(dg * awbStatus->gainG, 14, 10); - wbg.gain_b = clampField(dg * awbStatus->gainB, 14, 10); + /* + * Apply an extra gain of 1 / minColourGain so as not to apply < 1 gains to any + * channels (which would cause saturated pixels to go cyan or magenta). + * Doing this doesn't really apply more gain than necessary, because one of the + * channels is always getting the minimum gain possible. For this reason we also + * don't change the values that we report externally. + */ + double gainR = awbStatus->gainR * extraGain; + double gainG = awbStatus->gainG * extraGain; + double gainB = awbStatus->gainB * extraGain; + + wbg.gain_r = clampField(dg * gainR, 14, 10); + wbg.gain_g = clampField(dg * gainG, 14, 10); + wbg.gain_b = clampField(dg * gainB, 14, 10); /* * The YCbCr conversion block should contain the appropriate YCbCr @@ -535,9 +549,9 @@ void IpaPiSP::applyWBG(const AwbStatus *awbStatus, const AgcPrepareStatus *agcPr be_->GetYcbcr(csc); /* The CSC coefficients already have the << 10 scaling applied. */ - rgby.gain_r = clampField(csc.coeffs[0] * awbStatus->gainR, 14); - rgby.gain_g = clampField(csc.coeffs[1] * awbStatus->gainG, 14); - rgby.gain_b = clampField(csc.coeffs[2] * awbStatus->gainB, 14); + rgby.gain_r = clampField(csc.coeffs[0] * gainR, 14); + rgby.gain_g = clampField(csc.coeffs[1] * gainG, 14); + rgby.gain_b = clampField(csc.coeffs[2] * gainB, 14); LOG(IPARPI, Debug) << "Applying WB R: " << awbStatus->gainR << " B: " << awbStatus->gainB; diff --git a/src/ipa/rpi/vc4/vc4.cpp b/src/ipa/rpi/vc4/vc4.cpp index ba43e4741584..8a7a37c870ed 100644 --- a/src/ipa/rpi/vc4/vc4.cpp +++ b/src/ipa/rpi/vc4/vc4.cpp @@ -63,7 +63,8 @@ private: bool validateIspControls(); void applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls); - void applyDG(const struct AgcPrepareStatus *dgStatus, ControlList &ctrls); + void applyDG(const struct AgcPrepareStatus *dgStatus, + const struct AwbStatus *awbStatus, ControlList &ctrls); void applyCCM(const struct CcmStatus *ccmStatus, ControlList &ctrls); void applyBlackLevel(const struct BlackLevelStatus *blackLevelStatus, ControlList &ctrls); void applyGamma(const struct ContrastStatus *contrastStatus, ControlList &ctrls); @@ -152,8 +153,7 @@ void IpaVc4::platformPrepareIsp([[maybe_unused]] const PrepareParams ¶ms, applyCCM(ccmStatus, ctrls); AgcPrepareStatus *dgStatus = rpiMetadata.getLocked("agc.prepare_status"); - if (dgStatus) - applyDG(dgStatus, ctrls); + applyDG(dgStatus, awbStatus, ctrls); AlscStatus *lsStatus = rpiMetadata.getLocked("alsc.status"); if (lsStatus) @@ -329,10 +329,26 @@ void IpaVc4::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls) static_cast(awbStatus->gainB * 1000)); } -void IpaVc4::applyDG(const struct AgcPrepareStatus *dgStatus, ControlList &ctrls) +void IpaVc4::applyDG(const struct AgcPrepareStatus *dgStatus, + const struct AwbStatus *awbStatus, ControlList &ctrls) { + double digitalGain = dgStatus ? dgStatus->digitalGain : 1.0; + + if (awbStatus) { + /* + * We must apply sufficient extra digital gain to stop any of the channel gains being + * less than 1, which would cause saturation artifacts. Note that one of the colour + * channels is still getting the minimum possible gain, so it's not a "real" gain + * increase. + */ + double minColourGain = std::min({ awbStatus->gainR, awbStatus->gainG, awbStatus->gainB, 1.0 }); + /* The 0.1 here doesn't mean much, but just stops arithmetic errors and extreme behaviour. */ + double extraGain = 1.0 / std::max({ minColourGain, 0.1 }); + digitalGain *= extraGain; + } + ctrls.set(V4L2_CID_DIGITAL_GAIN, - static_cast(dgStatus->digitalGain * 1000)); + static_cast(digitalGain * 1000)); } void IpaVc4::applyCCM(const struct CcmStatus *ccmStatus, ControlList &ctrls) From patchwork Mon Jul 21 07:47:22 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 23861 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 3A83FBDCC1 for ; Mon, 21 Jul 2025 07:49:06 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E0F1B68FD3; Mon, 21 Jul 2025 09:49:00 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="VaGnwwfu"; dkim-atps=neutral Received: from mail-wr1-x436.google.com (mail-wr1-x436.google.com [IPv6:2a00:1450:4864:20::436]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5EC8F61509 for ; Mon, 21 Jul 2025 09:48:57 +0200 (CEST) Received: by mail-wr1-x436.google.com with SMTP id ffacd0b85a97d-3a4eb4dfd8eso606720f8f.2 for ; Mon, 21 Jul 2025 00:48:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; t=1753084137; x=1753688937; darn=lists.libcamera.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=jhk5zx5hcc30h8hdBHPGHevEhGEI2Gxx5qESeEfLWJU=; b=VaGnwwfukRKO6LLp7Oyo0MO+lP7hjHxgGgiYZ/VCaDUkT4/foqn+9ger4+v/k8tluI Q/K2DhUYvjXxn9RAVk9f134/wgNiO1TKDesg+qvgeIw+tKM/WpPf2hVU/osQJNlytFC4 7tmpyrdD0RbClTVF3W0wAxO2g9S8IXlJqIMxqDC+XZIFkFGHDgFXVbPju/m3k42wgfxL LXooOpG5cwM7cB2LChpJZXGvzkqw5b3/ldFu0zmz0UebdLfs+S1ETaqN9pvAyZp1Ehic FP24xd3kRJe/6CoyydNfhyjosvlWM5eRGWALr9x6NUOFUVOJuhzqc37KdSMWojW8rhqI paGQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1753084137; x=1753688937; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=jhk5zx5hcc30h8hdBHPGHevEhGEI2Gxx5qESeEfLWJU=; b=Ggcmm6g+xwUg7Vl7w6xwBJY5wDc3oggpyCfQUo/QPmdPOTV7pRGkmPZPxlVGBZnYjP GJcWTGzEHUxMGILwz7GwcS323H1zRWzpjbNFtnKKwtaV/NX43g5m4ZMX38GlXU6ZZTtP G/C5UM4eONm/nhj4n9K35zlMZG5L/ghNBAKKaAeRINBQyG0/RitL93dh4KwooReJu2jv nR/xH1tnTZClYnJtrHwSlglBVCuA+/D1O2BqEqxPazK1TtxycXC4ZUo8g2i98k0nTK2S ryOWX1lfwDkQ0iTBE+NZ94lEXwU+7KYwRKGV095RJhUOqE1aRCJQIlRlVYjv6yS6oV30 oSHw== X-Gm-Message-State: AOJu0YzrnGZfxIyIK48bhYJ3nQN3KhJSn/mCEM+4DpGzOQyf3UuynGby vLbpXWhyZvFZx6BdznwH3cSecbqDegTIEpPHq+BGmyWcq5HB2Fsn83O9ZUGPUkJoOhVnQLOkKKn Cq4fXfts= X-Gm-Gg: ASbGncvHMiPGAasszyjRSa+A/Q5ZK2bGac/rVNCQGCKiGsSD6aq25pO0fAEr5Olb5mU hrp+r9fbsN53i1GxuWqk5rFq3vsPxZecVJWLmdtYg64bNBn6ZL2yih/wiBa9Kf59doYZAvchmFJ DAubz/vnfWci5TV4fLAC8gKL3Hp3dEo3KDXDYkS1UezIU4st3tZjYafn0KkQzIm4vEju69TaZ6R 48CcpxV+DSMT80QVSO6DG3tvOoRVF9mOZKhbGxkHOwTjmxhikeKX9gyiGkdvEfqalAu3oIWFG2s AmEiuuserMMP0G5X62sIZSs9txk85JVq0C/EwU6QDwLb6rw1LEda9hftC/AkDp/RRV4cd1SUP8S MptzmwhRSnv21yXznt79Ua9f/4qkGDCE2+MbFC1Fjwg== X-Google-Smtp-Source: AGHT+IFhGtJfVtjw1Xtd9wyET9sjGcnPqFD1ytezRO3V5OZZn0qB8sFFOP3U0snozOZVH1AG8D8wYw== X-Received: by 2002:adf:b642:0:b0:3a5:8abe:a265 with SMTP id ffacd0b85a97d-3b60dd78984mr4999714f8f.10.1753084136572; Mon, 21 Jul 2025 00:48:56 -0700 (PDT) Received: from NAUSH-P-DELL.pitowers.org ([93.93.133.154]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4562e802afasm151101765e9.12.2025.07.21.00.48.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Jul 2025 00:48:56 -0700 (PDT) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Cc: David Plowman , Naushir Patuck Subject: [PATCH v2 2/7] ipa: rpi: agc: Make the maximum digital gain configurable Date: Mon, 21 Jul 2025 08:47:22 +0100 Message-ID: <20250721074853.1463358-3-naush@raspberrypi.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250721074853.1463358-1-naush@raspberrypi.com> References: <20250721074853.1463358-1-naush@raspberrypi.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" From: David Plowman The maximum allowed digital gain is hard-coded to 4. Make it a configurable parameter. Signed-off-by: David Plowman Signed-off-by: Naushir Patuck Reviewed-by: Naushir Patuck --- src/ipa/rpi/controller/rpi/agc_channel.cpp | 5 ++++- src/ipa/rpi/controller/rpi/agc_channel.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/ipa/rpi/controller/rpi/agc_channel.cpp b/src/ipa/rpi/controller/rpi/agc_channel.cpp index ae0cb148893a..f6a191d50fb3 100644 --- a/src/ipa/rpi/controller/rpi/agc_channel.cpp +++ b/src/ipa/rpi/controller/rpi/agc_channel.cpp @@ -260,6 +260,8 @@ int AgcConfig::read(const libcamera::YamlObject ¶ms) desaturate = params["desaturate"].get(1); + maxDigitalGain = params["max_digital_gain"].get(4.0); + return 0; } @@ -508,7 +510,8 @@ void AgcChannel::prepare(Metadata *imageMetadata) * Never ask for a gain < 1.0, and also impose * some upper limit. Make it customisable? */ - prepareStatus.digitalGain = std::max(1.0, std::min(digitalGain, 4.0)); + prepareStatus.digitalGain = std::max(1.0, std::min(digitalGain, + config_.maxDigitalGain)); LOG(RPiAgc, Debug) << "Actual exposure " << actualExposure; LOG(RPiAgc, Debug) << "Use digitalGain " << prepareStatus.digitalGain; LOG(RPiAgc, Debug) << "Effective exposure " diff --git a/src/ipa/rpi/controller/rpi/agc_channel.h b/src/ipa/rpi/controller/rpi/agc_channel.h index fa697e6fa57d..e3475d864b0b 100644 --- a/src/ipa/rpi/controller/rpi/agc_channel.h +++ b/src/ipa/rpi/controller/rpi/agc_channel.h @@ -78,6 +78,7 @@ struct AgcConfig { double defaultAnalogueGain; double stableRegion; bool desaturate; + double maxDigitalGain; }; class AgcChannel From patchwork Mon Jul 21 07:47:23 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 23862 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 DA6AAC3323 for ; Mon, 21 Jul 2025 07:49:07 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1E66468FD7; Mon, 21 Jul 2025 09:49:05 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="mYnA71VF"; dkim-atps=neutral Received: from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com [IPv6:2a00:1450:4864:20::32b]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 0E8A968FC0 for ; Mon, 21 Jul 2025 09:48:58 +0200 (CEST) Received: by mail-wm1-x32b.google.com with SMTP id 5b1f17b1804b1-456053b5b8cso1734675e9.3 for ; Mon, 21 Jul 2025 00:48:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; t=1753084137; x=1753688937; darn=lists.libcamera.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=zocCf5y6pEDwCbRf0ZnxWaN++n1LhwhYJvqr+OSo1EI=; b=mYnA71VFpCzY90V5e0ce5f/OFpsWvInkJvimxmohgfzMkm/ceOAOkEdjXio2FjSGlD +Wed/EuooJvVEdOQ4RzqyGFhXjiSKiEg0pZ1BPL1BKAigPLloNeNKqVTroVyZIK2ks6A 58HngNEA8fvyIggtGwI+7kXaxplwz326XXo5YqcQfE3hmZGkQ9EbKM/ekt78rHifcsFD GohQOAZGG7L5e3/REAig2g6AbUm5oNEUiMiMQ2tXZcLqMXeHjqwxR8y8IBuYw2rJV9aR qbLRMr/JyRW/DQ+VcUfwSvsZuZVyTC4QoBRRAuDJjTELRoTc1D1w3OiRCiCkFtahfFgs obgQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1753084137; x=1753688937; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=zocCf5y6pEDwCbRf0ZnxWaN++n1LhwhYJvqr+OSo1EI=; b=XKifTdMKUtTzgK5jVbe9e1TKpfruFIXvOBR+eIvWtS4XUxUVurxUhKEA291luS4M2B Cmi4xQDGCT3dABSsGOD7mSG+BwjPqs4D7Sx1J5GICMEm8eVHvo5rwlLbNt9Uxk8Xrthj 9TP0yHXmuYGDn2hcSfuU6M/T+I/dPuUaVmKtO0JiWumhw12R4QZUR++p6e45DAeFpuaq QaV+EySigth+o78F18r5TCHfb0Tt73O5VGFNFqcc71oEtcZJ4hJhesJIPoanzqU9AHSH 4xXO7pyj9N0tDdsZipEDzf6SSgRiqBbMrMBuw2/iv4f+T9E/c4m6mSqHtf7sVc0V24gU SigA== X-Gm-Message-State: AOJu0YxX8H+QqZXVmxg7dZE0EvMEigDkRTlgZ3SlRYA0UW4f+9o9wzGV qeP72T0Pvu6YB9qrdJxJS2IuNkM/10rXagKc0s7gXljiaojvDUlYBuIPFDEEFD7CDBSYXmfGQCV OGUUMUlw= X-Gm-Gg: ASbGnctPHwDXabAGSUc18qm/GZeRrTrhquP9KT9Mwez4xJP1huAoDXt0b/Z7pbEmlrK tnRrsWZgplHVNKXExk0LdH78vJEAaTR9j5pE0+J+gv7zf50+fIzDxuydRTgtQYfMIgiQZ0CbjT5 FtUfNJKbSlH29/reZuALIpnQ4P8XNrMrXm8I5WGcnxQdjWEQfLAa3krxnNYQVPNucsJ9UkFIBpj ULxwh5kHHzHkBIC/jc7ok+fiJjEm63UMaUZkOtDX5l9hDoskvG2op9RgpqImnfn/vtu1h4f2mbi wIsYydHZg1gD1vEESxjV3sp+zIyeS29Oc5LyZDM4bvxLuJKbMmttwyecRXmmFQlNC8Jq2zOipat pCCpHXatJkKqrS5pYgm1TrmV5S3hcTAhLFqCt4LN6KA== X-Google-Smtp-Source: AGHT+IH+dvz9E4Wr3yabDL+um9sPjYqyvgTX627dCnnmbH5oAEcrvW/XAb6Zc4uZ4yJheV50S34Tkw== X-Received: by 2002:a05:600c:1e20:b0:456:11a6:a50a with SMTP id 5b1f17b1804b1-4562dff98c2mr87505875e9.2.1753084137194; Mon, 21 Jul 2025 00:48:57 -0700 (PDT) Received: from NAUSH-P-DELL.pitowers.org ([93.93.133.154]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4562e802afasm151101765e9.12.2025.07.21.00.48.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Jul 2025 00:48:56 -0700 (PDT) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Cc: David Plowman , Naushir Patuck Subject: [PATCH v2 3/7] ipa: rpi: agc: Rename "analogue gain" to "gain" where appropriate Date: Mon, 21 Jul 2025 08:47:23 +0100 Message-ID: <20250721074853.1463358-4-naush@raspberrypi.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250721074853.1463358-1-naush@raspberrypi.com> References: <20250721074853.1463358-1-naush@raspberrypi.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" From: David Plowman Much of the time we use the term "analogue gain" where we really mean the combined analogue and digital gain (because the digital gain will make up whatever the analogue gain can't deliver). This commit replaces the use of "analogue gain" by just "gain" in places where we really mean the combined gain. There are a couple of principle areas: 1. Where we previously talked about the "fixedAnalaogueGain" (including setting it "manually") this is now just the "fixedGain" (because it always encompassed both analogue and digital gain). Along with this, the setfixedExposureTime/Gain functions no longer update the output status directly. Applications should wait in the usual way for AGC/AEC changes to take effect, and this "shortcut" actually doesn't fit well with the gain being the combined gain. 2. The divideUpExposure method is adjusted to be clearer that it's setting the combined gain, and it's prepare() that will discover later what the analogue gain actually delivered. Signed-off-by: David Plowman Signed-off-by: Naushir Patuck Reviewed-by: Naushir Patuck --- src/ipa/rpi/common/ipa_base.cpp | 2 +- src/ipa/rpi/controller/agc_algorithm.h | 2 +- src/ipa/rpi/controller/agc_status.h | 2 +- src/ipa/rpi/controller/rpi/agc.cpp | 6 +- src/ipa/rpi/controller/rpi/agc.h | 4 +- src/ipa/rpi/controller/rpi/agc_channel.cpp | 77 +++++++++++----------- src/ipa/rpi/controller/rpi/agc_channel.h | 4 +- 7 files changed, 48 insertions(+), 49 deletions(-) diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp index a5bdcbb5838c..8b4eb75e7e6b 100644 --- a/src/ipa/rpi/common/ipa_base.cpp +++ b/src/ipa/rpi/common/ipa_base.cpp @@ -868,7 +868,7 @@ void IpaBase::applyControls(const ControlList &controls) if (agc->autoGainEnabled()) break; - agc->setFixedAnalogueGain(0, ctrl.second.get()); + agc->setFixedGain(0, ctrl.second.get()); libcameraMetadata_.set(controls::AnalogueGain, ctrl.second.get()); diff --git a/src/ipa/rpi/controller/agc_algorithm.h b/src/ipa/rpi/controller/agc_algorithm.h index fdaa10e6c176..9fa2bd205160 100644 --- a/src/ipa/rpi/controller/agc_algorithm.h +++ b/src/ipa/rpi/controller/agc_algorithm.h @@ -26,7 +26,7 @@ public: virtual void setFixedExposureTime(unsigned int channel, libcamera::utils::Duration fixedExposureTime) = 0; virtual void setMaxExposureTime(libcamera::utils::Duration maxExposureTime) = 0; - virtual void setFixedAnalogueGain(unsigned int channel, double fixedAnalogueGain) = 0; + virtual void setFixedGain(unsigned int channel, double fixedGain) = 0; virtual void setMeteringMode(std::string const &meteringModeName) = 0; virtual void setExposureMode(std::string const &exposureModeName) = 0; virtual void setConstraintMode(std::string const &contraintModeName) = 0; diff --git a/src/ipa/rpi/controller/agc_status.h b/src/ipa/rpi/controller/agc_status.h index 9308b156051c..d4cedcf49c3c 100644 --- a/src/ipa/rpi/controller/agc_status.h +++ b/src/ipa/rpi/controller/agc_status.h @@ -37,7 +37,7 @@ struct AgcStatus { libcamera::utils::Duration flickerPeriod; int floatingRegionEnable; libcamera::utils::Duration fixedExposureTime; - double fixedAnalogueGain; + double fixedGain; unsigned int channel; HdrStatus hdr; }; diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp index 02bfdb4a5e22..afda2e364f64 100644 --- a/src/ipa/rpi/controller/rpi/agc.cpp +++ b/src/ipa/rpi/controller/rpi/agc.cpp @@ -184,14 +184,14 @@ void Agc::setFixedExposureTime(unsigned int channelIndex, Duration fixedExposure channelData_[channelIndex].channel.setFixedExposureTime(fixedExposureTime); } -void Agc::setFixedAnalogueGain(unsigned int channelIndex, double fixedAnalogueGain) +void Agc::setFixedGain(unsigned int channelIndex, double fixedGain) { if (checkChannel(channelIndex)) return; - LOG(RPiAgc, Debug) << "setFixedAnalogueGain " << fixedAnalogueGain + LOG(RPiAgc, Debug) << "setFixedGain " << fixedGain << " for channel " << channelIndex; - channelData_[channelIndex].channel.setFixedAnalogueGain(fixedAnalogueGain); + channelData_[channelIndex].channel.setFixedGain(fixedGain); } void Agc::setMeteringMode(std::string const &meteringModeName) diff --git a/src/ipa/rpi/controller/rpi/agc.h b/src/ipa/rpi/controller/rpi/agc.h index c3a940bf697a..966630a26303 100644 --- a/src/ipa/rpi/controller/rpi/agc.h +++ b/src/ipa/rpi/controller/rpi/agc.h @@ -35,8 +35,8 @@ public: void setMaxExposureTime(libcamera::utils::Duration maxExposureTime) override; void setFixedExposureTime(unsigned int channelIndex, libcamera::utils::Duration fixedExposureTime) override; - void setFixedAnalogueGain(unsigned int channelIndex, - double fixedAnalogueGain) override; + void setFixedGain(unsigned int channelIndex, + double fixedGain) override; void setMeteringMode(std::string const &meteringModeName) override; void setExposureMode(std::string const &exposureModeName) override; void setConstraintMode(std::string const &contraintModeName) override; diff --git a/src/ipa/rpi/controller/rpi/agc_channel.cpp b/src/ipa/rpi/controller/rpi/agc_channel.cpp index f6a191d50fb3..9e4661616051 100644 --- a/src/ipa/rpi/controller/rpi/agc_channel.cpp +++ b/src/ipa/rpi/controller/rpi/agc_channel.cpp @@ -275,7 +275,7 @@ AgcChannel::AgcChannel() : meteringMode_(nullptr), exposureMode_(nullptr), constraintMode_(nullptr), frameCount_(0), lockCount_(0), lastTargetExposure_(0s), ev_(1.0), flickerPeriod_(0s), - maxExposureTime_(0s), fixedExposureTime_(0s), fixedAnalogueGain_(0.0) + maxExposureTime_(0s), fixedExposureTime_(0s), fixedGain_(0.0) { /* Set AWB default values in case early frames have no updates in metadata. */ awb_.gainR = 1.0; @@ -339,17 +339,17 @@ bool AgcChannel::autoExposureEnabled() const void AgcChannel::disableAutoGain() { - fixedAnalogueGain_ = status_.analogueGain; + fixedGain_ = status_.analogueGain; } void AgcChannel::enableAutoGain() { - fixedAnalogueGain_ = 0; + fixedGain_ = 0; } bool AgcChannel::autoGainEnabled() const { - return fixedAnalogueGain_ == 0; + return fixedGain_ == 0; } unsigned int AgcChannel::getConvergenceFrames() const @@ -358,7 +358,7 @@ unsigned int AgcChannel::getConvergenceFrames() const * If exposure time and gain have been explicitly set, there is no * convergence to happen, so no need to drop any frames - return zero. */ - if (fixedExposureTime_ && fixedAnalogueGain_) + if (fixedExposureTime_ && fixedGain_) return 0; else return config_.convergenceFrames; @@ -398,11 +398,9 @@ void AgcChannel::setFixedExposureTime(Duration fixedExposureTime) status_.exposureTime = limitExposureTime(fixedExposureTime_); } -void AgcChannel::setFixedAnalogueGain(double fixedAnalogueGain) +void AgcChannel::setFixedGain(double fixedGain) { - fixedAnalogueGain_ = fixedAnalogueGain; - /* Set this in case someone calls disableAuto() straight after. */ - status_.analogueGain = limitGain(fixedAnalogueGain); + fixedGain_ = fixedGain; } void AgcChannel::setMeteringMode(std::string const &meteringModeName) @@ -436,9 +434,9 @@ void AgcChannel::switchMode(CameraMode const &cameraMode, mode_ = cameraMode; Duration fixedExposureTime = limitExposureTime(fixedExposureTime_); - if (fixedExposureTime && fixedAnalogueGain_) { + if (fixedExposureTime && fixedGain_) { /* This is the equivalent of computeTargetExposure and applyDigitalGain. */ - target_.totalExposureNoDG = fixedExposureTime_ * fixedAnalogueGain_; + target_.totalExposureNoDG = fixedExposureTime_ * fixedGain_; target_.totalExposure = target_.totalExposureNoDG; /* Equivalent of filterExposure. This resets any "history". */ @@ -446,7 +444,7 @@ void AgcChannel::switchMode(CameraMode const &cameraMode, /* Equivalent of divideUpExposure. */ filtered_.exposureTime = fixedExposureTime; - filtered_.analogueGain = fixedAnalogueGain_; + filtered_.analogueGain = fixedGain_; } else if (status_.totalExposureValue) { /* * On a mode switch, various things could happen: @@ -476,7 +474,7 @@ void AgcChannel::switchMode(CameraMode const &cameraMode, /* Equivalent of divideUpExposure. */ filtered_.exposureTime = fixedExposureTime ? fixedExposureTime : config_.defaultExposureTime; - filtered_.analogueGain = fixedAnalogueGain_ ? fixedAnalogueGain_ : config_.defaultAnalogueGain; + filtered_.analogueGain = fixedGain_ ? fixedGain_ : config_.defaultAnalogueGain; } writeAndFinish(metadata, false); @@ -608,11 +606,11 @@ void AgcChannel::housekeepConfig() /* First fetch all the up-to-date settings, so no one else has to do it. */ status_.ev = ev_; status_.fixedExposureTime = limitExposureTime(fixedExposureTime_); - status_.fixedAnalogueGain = fixedAnalogueGain_; + status_.fixedGain = fixedGain_; status_.flickerPeriod = flickerPeriod_; LOG(RPiAgc, Debug) << "ev " << status_.ev << " fixedExposureTime " - << status_.fixedExposureTime << " fixedAnalogueGain " - << status_.fixedAnalogueGain; + << status_.fixedExposureTime << " fixedGain " + << status_.fixedGain; /* * Make sure the "mode" pointers point to the up-to-date things, if * they've changed. @@ -799,9 +797,9 @@ void AgcChannel::computeGain(StatisticsPtr &statistics, Metadata *imageMetadata, void AgcChannel::computeTargetExposure(double gain) { - if (status_.fixedExposureTime && status_.fixedAnalogueGain) { + if (status_.fixedExposureTime && status_.fixedGain) { target_.totalExposure = - status_.fixedExposureTime * status_.fixedAnalogueGain; + status_.fixedExposureTime * status_.fixedGain; } else { /* * The statistics reflect the image without digital gain, so the final @@ -815,8 +813,8 @@ void AgcChannel::computeTargetExposure(double gain) maxExposureTime = limitExposureTime(maxExposureTime); Duration maxTotalExposure = maxExposureTime * - (status_.fixedAnalogueGain != 0.0 - ? status_.fixedAnalogueGain + (status_.fixedGain != 0.0 + ? status_.fixedGain : exposureMode_->gain.back()); target_.totalExposure = std::min(target_.totalExposure, maxTotalExposure); } @@ -896,7 +894,7 @@ void AgcChannel::filterExposure() * region, because we want to reflect any user exposure/gain updates, * however small. */ - if ((status_.fixedExposureTime && status_.fixedAnalogueGain) || + if ((status_.fixedExposureTime && status_.fixedGain) || frameCount_ <= config_.startupFrames) { speed = 1.0; stableRegion = 0.0; @@ -930,63 +928,64 @@ void AgcChannel::divideUpExposure() */ Duration exposureValue = filtered_.totalExposureNoDG; Duration exposureTime; - double analogueGain; + double gain; exposureTime = status_.fixedExposureTime ? status_.fixedExposureTime : exposureMode_->exposureTime[0]; exposureTime = limitExposureTime(exposureTime); - analogueGain = status_.fixedAnalogueGain != 0.0 ? status_.fixedAnalogueGain - : exposureMode_->gain[0]; - analogueGain = limitGain(analogueGain); - if (exposureTime * analogueGain < exposureValue) { + gain = status_.fixedGain != 0.0 ? status_.fixedGain + : exposureMode_->gain[0]; + gain = limitGain(gain); + if (exposureTime * gain < exposureValue) { for (unsigned int stage = 1; stage < exposureMode_->gain.size(); stage++) { if (!status_.fixedExposureTime) { Duration stageExposureTime = limitExposureTime(exposureMode_->exposureTime[stage]); - if (stageExposureTime * analogueGain >= exposureValue) { - exposureTime = exposureValue / analogueGain; + if (stageExposureTime * gain >= exposureValue) { + exposureTime = exposureValue / gain; break; } exposureTime = stageExposureTime; } - if (status_.fixedAnalogueGain == 0.0) { + if (status_.fixedGain == 0.0) { if (exposureMode_->gain[stage] * exposureTime >= exposureValue) { - analogueGain = exposureValue / exposureTime; + gain = exposureValue / exposureTime; break; } - analogueGain = exposureMode_->gain[stage]; - analogueGain = limitGain(analogueGain); + gain = exposureMode_->gain[stage]; + gain = limitGain(gain); } } } LOG(RPiAgc, Debug) << "Divided up exposure time and gain are " << exposureTime - << " and " << analogueGain; + << " and " << gain; /* * Finally adjust exposure time for flicker avoidance (require both * exposure time and gain not to be fixed). */ - if (!status_.fixedExposureTime && !status_.fixedAnalogueGain && + if (!status_.fixedExposureTime && !status_.fixedGain && status_.flickerPeriod) { int flickerPeriods = exposureTime / status_.flickerPeriod; if (flickerPeriods) { Duration newExposureTime = flickerPeriods * status_.flickerPeriod; - analogueGain *= exposureTime / newExposureTime; + gain *= exposureTime / newExposureTime; /* * We should still not allow the ag to go over the * largest value in the exposure mode. Note that this * may force more of the total exposure into the digital * gain as a side-effect. */ - analogueGain = std::min(analogueGain, exposureMode_->gain.back()); - analogueGain = limitGain(analogueGain); + gain = std::min(gain, exposureMode_->gain.back()); + gain = limitGain(gain); exposureTime = newExposureTime; } LOG(RPiAgc, Debug) << "After flicker avoidance, exposure time " - << exposureTime << " gain " << analogueGain; + << exposureTime << " gain " << gain; } filtered_.exposureTime = exposureTime; - filtered_.analogueGain = analogueGain; + /* We ask for all the gain as analogue gain; prepare() will be told what we got. */ + filtered_.analogueGain = gain; } void AgcChannel::writeAndFinish(Metadata *imageMetadata, bool desaturate) diff --git a/src/ipa/rpi/controller/rpi/agc_channel.h b/src/ipa/rpi/controller/rpi/agc_channel.h index e3475d864b0b..93229128abf1 100644 --- a/src/ipa/rpi/controller/rpi/agc_channel.h +++ b/src/ipa/rpi/controller/rpi/agc_channel.h @@ -93,7 +93,7 @@ public: void setFlickerPeriod(libcamera::utils::Duration flickerPeriod); void setMaxExposureTime(libcamera::utils::Duration maxExposureTime); void setFixedExposureTime(libcamera::utils::Duration fixedExposureTime); - void setFixedAnalogueGain(double fixedAnalogueGain); + void setFixedGain(double fixedGain); void setMeteringMode(std::string const &meteringModeName); void setExposureMode(std::string const &exposureModeName); void setConstraintMode(std::string const &contraintModeName); @@ -153,7 +153,7 @@ private: libcamera::utils::Duration flickerPeriod_; libcamera::utils::Duration maxExposureTime_; libcamera::utils::Duration fixedExposureTime_; - double fixedAnalogueGain_; + double fixedGain_; }; } /* namespace RPiController */ From patchwork Mon Jul 21 07:47:24 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 23863 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 4B366BDCC1 for ; Mon, 21 Jul 2025 07:49:09 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 795C168FD9; Mon, 21 Jul 2025 09:49:07 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="f3Tda5Du"; dkim-atps=neutral Received: from mail-wr1-x432.google.com (mail-wr1-x432.google.com [IPv6:2a00:1450:4864:20::432]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 85BE768FCA for ; Mon, 21 Jul 2025 09:48:58 +0200 (CEST) Received: by mail-wr1-x432.google.com with SMTP id ffacd0b85a97d-3a4eed70f24so639362f8f.0 for ; Mon, 21 Jul 2025 00:48:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; t=1753084138; x=1753688938; darn=lists.libcamera.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=pJO3YyGYylLhkVR5hB/KZwH6SyR+HghyeB4hoxooCns=; b=f3Tda5Du4Dpi3pUZ2zgYvIgyooPUisHc8chubC6k1iYuSJ2BfrdhbY8O3Ur6sYpar+ DFTpMZMwxutNOQp/z8ux9IAfNzMNefEME73QcwIbm22nXPNZ22j6gsx/tQdoAB/kWPJq 56gma7riuX7WhuxV2p0ejOwpRoH18LAHxv7k7yWCTgF6T+Y6/7gSlj//gZGUwUJ5c7KI r4P85yE6Sj2HbKrd4jDqTdT0aBzixjhKxVarJP9W7LMwx4T60iqBnPb+p1/OhbDpn8yZ XdbCwPpoCAXsiwIZSVWPGonPPFK5L/88OBLkuss/+nZP5FJPFFopKIcOZH4bekzr2Rq2 /23w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1753084138; x=1753688938; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=pJO3YyGYylLhkVR5hB/KZwH6SyR+HghyeB4hoxooCns=; b=DFVaBDvb78IWcIvojDIw2BNJHCRbAZ7os+Xz4ExDa2npkpe5jG2f+pwGAsUfz8fBN7 5f8aJxG9xcutWI/iL+Sa7UupK0djorqlHB54bD3ydl90s4aIp+maBJqgHVovFuv8OcQy Cu9ueniy3SkBRJ7fRfTX1DG/QS+8f5Hc8xYxawZ+We7pRYLbqvAy5DxOzKl5nkn5qfkQ o9DUjFMWdPt1n0YtIq91TWDaU+HnFWftD7qVz4drdJGz0lB8MOkRwESjz2ZG5s4vyllR lK7AeDLhtk6e7si06/OebvAZGmTTTIbdQkISI5YKLeg1cpzZObESlsafI47g1B8qiNed 2udA== X-Gm-Message-State: AOJu0YwcvjYFPs9QkRLdzncV3AEQscWSQgmdVPrWCvEL327tPt0gDu0F J8wCx9x66HVgtrvQU9PFSIaJ8azPVyBbqojC5pYJaRPI0ykkwsiSzRpjLoErNzzQVNg3GsbfZg3 vUa9ncXI= X-Gm-Gg: ASbGncvBXlR3QKRaDwX0LG+qhf7NyCQI0uoAu0cQ7R3DNAeFHlFCN+9HH0Y4wNqxJOb W9/6IrRJaMX0BCevcYQZQy0X2gkTZI9cgU9mWj7FU3TJD2kDUIOHROhobAR44+hzcXtejNcNnPE N4NZvj7tLoJFTor9rDVKYR6+hb6rk8QN87F+8eZpHRi7LNcTZ1qXEPNdZs+YbcvZGLRcCv6S1CZ 3coKNz2wZu18adOHQDIh7SBgK1fwjVlUs4freNekyiiH1yxJGqCUYJCYxh3zOwD5WojLXYjVpsw wEGCTZ9Shd7nB5nFwTL9X9G52CJQNFUq1GJMrr/e63zpEIU3SnoKIr18EEMLyinTSJ6fVosOZkc Qr5sBbMeBDOdXKtijTok3kc/L/iXTpsEKjbHmeE/9GQ== X-Google-Smtp-Source: AGHT+IEHcjDWNX8N0MkqJ2yMUlOKWR6zeSeibUBHGiURmFAaLEh2Hhstw85jAdVQJaHC0RENYs/M9Q== X-Received: by 2002:a5d:5f87:0:b0:3a3:63d3:368e with SMTP id ffacd0b85a97d-3b60dd14839mr6562931f8f.0.1753084137779; Mon, 21 Jul 2025 00:48:57 -0700 (PDT) Received: from NAUSH-P-DELL.pitowers.org ([93.93.133.154]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4562e802afasm151101765e9.12.2025.07.21.00.48.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Jul 2025 00:48:57 -0700 (PDT) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Cc: David Plowman , Naushir Patuck Subject: [PATCH v2 4/7] ipa: rpi: Advance the delay context counter even when IPAs don't run Date: Mon, 21 Jul 2025 08:47:24 +0100 Message-ID: <20250721074853.1463358-5-naush@raspberrypi.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250721074853.1463358-1-naush@raspberrypi.com> References: <20250721074853.1463358-1-naush@raspberrypi.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" From: David Plowman The delay context counter must be advanced even when we decide not to run prepare/process. Otherwise, when we skip them at higher framerates, the current IPA context counter will catch up and overwrite the delay context. It's safe to advance the counter because the metadata is always copied forward a slot when we decide not to run the IPAs. Signed-off-by: David Plowman Signed-off-by: Naushir Patuck Reviewed-by: Naushir Patuck --- src/ipa/rpi/common/ipa_base.cpp | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp index 8b4eb75e7e6b..98690b80d5d3 100644 --- a/src/ipa/rpi/common/ipa_base.cpp +++ b/src/ipa/rpi/common/ipa_base.cpp @@ -501,10 +501,11 @@ void IpaBase::prepareIsp(const PrepareParams ¶ms) void IpaBase::processStats(const ProcessParams ¶ms) { unsigned int ipaContext = params.ipaContext % rpiMetadata_.size(); + RPiController::Metadata &rpiMetadata = rpiMetadata_[ipaContext]; - if (processPending_ && frameCount_ >= mistrustCount_) { - RPiController::Metadata &rpiMetadata = rpiMetadata_[ipaContext]; + Duration offset(0s); + if (processPending_ && frameCount_ >= mistrustCount_) { auto it = buffers_.find(params.buffers.stats); if (it == buffers_.end()) { LOG(IPARPI, Error) << "Could not find stats buffer!"; @@ -518,14 +519,14 @@ void IpaBase::processStats(const ProcessParams ¶ms) helper_->process(statistics, rpiMetadata); controller_.process(statistics, &rpiMetadata); + } - struct AgcStatus agcStatus; - if (rpiMetadata.get("agc.status", agcStatus) == 0) { - ControlList ctrls(sensorCtrls_); - applyAGC(&agcStatus, ctrls); - setDelayedControls.emit(ctrls, ipaContext); - setCameraTimeoutValue(); - } + struct AgcStatus agcStatus; + if (rpiMetadata.get("agc.status", agcStatus) == 0) { + ControlList ctrls(sensorCtrls_); + applyAGC(&agcStatus, ctrls); + setDelayedControls.emit(ctrls, ipaContext); + setCameraTimeoutValue(); } /* From patchwork Mon Jul 21 07:47:25 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 23864 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 A02B8C3323 for ; Mon, 21 Jul 2025 07:49:10 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 18C4468FDE; Mon, 21 Jul 2025 09:49:09 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="Wu/Nv8zC"; dkim-atps=neutral Received: from mail-wr1-x42a.google.com (mail-wr1-x42a.google.com [IPv6:2a00:1450:4864:20::42a]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 61E2768FCE for ; Mon, 21 Jul 2025 09:48:59 +0200 (CEST) Received: by mail-wr1-x42a.google.com with SMTP id ffacd0b85a97d-3a4e749d7b2so660864f8f.0 for ; Mon, 21 Jul 2025 00:48:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; t=1753084138; x=1753688938; darn=lists.libcamera.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=BGJsTePNOrgIui+pOXQicmAtP5q9l21Oj649LL5oFl8=; b=Wu/Nv8zCOgGamHseKgJDBKNiNolkId9fAU7UrQAMoOVa3dn8m+NE9UZ/BkIIHMCfLH x17wAEmmL4jnd/f0B0iJsYgOkm0DdI/ZcjPy7CApNx+nHvpkaIkF6SOXcq0RyHx9d/O7 y1z33Rv2qVehiSF3dcBovdecFVBFdUwtkETXbTCznETJECcJmfpIe228IFWUlHJpSEJK Z/e7Elk4xoA8rSRB9+lVpVFrZo7NQk6uRfIa3NdaVn8e+bsbKob20lFg7PgBC6Nwcna4 ZeSKjOhNzh30uPEIVdx3OrxW9Df72f7O6ZjXxxQRCBkwzlOPKpZu0P5V+lm4xV3SO3Yd OYvA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1753084138; x=1753688938; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=BGJsTePNOrgIui+pOXQicmAtP5q9l21Oj649LL5oFl8=; b=b9XbiK1ydYbK4lixVeCF9YSpiyR7p+sRaN2HsjzigGAC3Ej/omy1geLl2EAjpW712b RlNPhP7eNW2y1WvdnlPrjZGOocge8wBYwbBFYwL2r8UWeIyfSUZtkq/FbYcVLA7a7elA HY2gk32K2EWqBTP5KYdZX7NgbYHphdrxJjHihsyAYGP0jC0um+FrItDghcVVwQuk1toQ c9T/UjpBkzReIyotGj6yVL517lsKisCXMatir6ydVKaRZvRgw/49+v874LjtqJ+YRcZX Oncl+M2bdApGSTP5S3wfpgJvfLYmWgIHNEPr6/8UYcuJo7N657v65aHFwJmE7yLPxcDj lhGA== X-Gm-Message-State: AOJu0YyFRiW409MWaGM4jptQ1MZ7F2f8rSWil/3GYza2keoOqgtVRsdI gJbiGuRFQ945N3PZ1Bknsz5IpdrhtxPKi7u0tLpvIzMFTzoe890anZfcTNYgLQRycwjdv57z1wl vm5wl2VE= X-Gm-Gg: ASbGncuxnoO65IuJeDT//XiWZedqUSVXLy/32qkJ7TUhyfUm8VJrc8+qCSZgcOUA4aG p5dYxxtibC6d6J9ZIPBu+c2bAOS6cPQajTK7JcN8SW3oC2RcHI5aoEd0XqVEtqNu/nCtowpzYAX oq3i3TZvdYE9e0BJOmc/8rxTR8xzSd6G3mTFd4szktl/MixwkzPy96yZFEj68IpT633osAPri/H 1hkLsZQJJ69MoKPzPgaZgVgwjYFBCm+jTSBG01PQIUQlNm3WHb3LRGSZ+8a9iUBib6KB8Xoxq2H 2fzsXxkwyHcRGKNu4RamxoyZAIfzOIfiLMJiTjDtP8miCX8RZmyubF6ueWuupo+q14izmITzPjd qXP7BJc01coM5KVb0cUADtkOzn3Mh56Wfd+ltCsGEDw== X-Google-Smtp-Source: AGHT+IHSjNsYTMgVxEuC/RBdJ23Pfo9VGVH4d83MFxJ1R/fuA+VxwgGT1hXql0gklPeNPavgW+kv1Q== X-Received: by 2002:adf:eec7:0:b0:3a4:dbdf:7152 with SMTP id ffacd0b85a97d-3b60dd87cc9mr5469220f8f.14.1753084138335; Mon, 21 Jul 2025 00:48:58 -0700 (PDT) Received: from NAUSH-P-DELL.pitowers.org ([93.93.133.154]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4562e802afasm151101765e9.12.2025.07.21.00.48.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Jul 2025 00:48:58 -0700 (PDT) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Cc: David Plowman , Naushir Patuck Subject: [PATCH v2 5/7] ipa: rpi: agc: Calculate digital gain in process() Date: Mon, 21 Jul 2025 08:47:25 +0100 Message-ID: <20250721074853.1463358-6-naush@raspberrypi.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250721074853.1463358-1-naush@raspberrypi.com> References: <20250721074853.1463358-1-naush@raspberrypi.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" From: David Plowman Previously we let prepare() do the work by comparing the desired total exposure against the shutter time and analogue gain. This can cause the image to "wink" at high framerates because we may skip running prepare() to get the new digital gain even when the delayed AGC status (which came out of an earlier call to process()) shows that a change was required. Now we're taking explicit control of the digital gain by calculating it ourselves so that we can output it in the standard AgcStatus object. This means that whenever the delayed AGC status changes, we have the correct digital gain to go with it. Signed-off-by: David Plowman Signed-off-by: Naushir Patuck Reviewed-by: Naushir Patuck --- src/ipa/rpi/controller/agc_status.h | 1 + src/ipa/rpi/controller/rpi/agc_channel.cpp | 163 +++++++++------------ src/ipa/rpi/controller/rpi/agc_channel.h | 1 + 3 files changed, 75 insertions(+), 90 deletions(-) diff --git a/src/ipa/rpi/controller/agc_status.h b/src/ipa/rpi/controller/agc_status.h index d4cedcf49c3c..956d6abf398c 100644 --- a/src/ipa/rpi/controller/agc_status.h +++ b/src/ipa/rpi/controller/agc_status.h @@ -30,6 +30,7 @@ struct AgcStatus { libcamera::utils::Duration targetExposureValue; /* (unfiltered) target total exposure AGC is aiming for */ libcamera::utils::Duration exposureTime; double analogueGain; + double digitalGain; std::string exposureMode; std::string constraintMode; std::string meteringMode; diff --git a/src/ipa/rpi/controller/rpi/agc_channel.cpp b/src/ipa/rpi/controller/rpi/agc_channel.cpp index 9e4661616051..7da1c6dbbec7 100644 --- a/src/ipa/rpi/controller/rpi/agc_channel.cpp +++ b/src/ipa/rpi/controller/rpi/agc_channel.cpp @@ -266,7 +266,7 @@ int AgcConfig::read(const libcamera::YamlObject ¶ms) } AgcChannel::ExposureValues::ExposureValues() - : exposureTime(0s), analogueGain(0), + : exposureTime(0s), analogueGain(0), digitalGain(0), totalExposure(0s), totalExposureNoDG(0s) { } @@ -434,17 +434,10 @@ void AgcChannel::switchMode(CameraMode const &cameraMode, mode_ = cameraMode; Duration fixedExposureTime = limitExposureTime(fixedExposureTime_); + double fixedGain = limitGain(fixedGain_); if (fixedExposureTime && fixedGain_) { - /* This is the equivalent of computeTargetExposure and applyDigitalGain. */ - target_.totalExposureNoDG = fixedExposureTime_ * fixedGain_; - target_.totalExposure = target_.totalExposureNoDG; - - /* Equivalent of filterExposure. This resets any "history". */ - filtered_ = target_; - - /* Equivalent of divideUpExposure. */ - filtered_.exposureTime = fixedExposureTime; - filtered_.analogueGain = fixedGain_; + filtered_.totalExposureNoDG = fixedExposureTime * fixedGain; + filtered_.totalExposure = filtered_.totalExposureNoDG; } else if (status_.totalExposureValue) { /* * On a mode switch, various things could happen: @@ -457,12 +450,8 @@ void AgcChannel::switchMode(CameraMode const &cameraMode, */ double ratio = lastSensitivity / cameraMode.sensitivity; - target_.totalExposure *= ratio; - target_.totalExposureNoDG = target_.totalExposure; filtered_.totalExposure *= ratio; filtered_.totalExposureNoDG = filtered_.totalExposure; - - divideUpExposure(); } else { /* * We come through here on startup, when at least one of the @@ -472,55 +461,46 @@ void AgcChannel::switchMode(CameraMode const &cameraMode, * weren't set. */ - /* Equivalent of divideUpExposure. */ - filtered_.exposureTime = fixedExposureTime ? fixedExposureTime : config_.defaultExposureTime; - filtered_.analogueGain = fixedGain_ ? fixedGain_ : config_.defaultAnalogueGain; + Duration exposureTime = fixedExposureTime ? fixedExposureTime : config_.defaultExposureTime; + double gain = fixedGain ? fixedGain : config_.defaultAnalogueGain; + filtered_.totalExposure = exposureTime * gain; + filtered_.totalExposureNoDG = filtered_.totalExposure; } + /* Setting target_ to filtered_ removes any history from before the mode switch. */ + target_ = filtered_; + divideUpExposure(); + writeAndFinish(metadata, false); } void AgcChannel::prepare(Metadata *imageMetadata) { - Duration totalExposureValue = status_.totalExposureValue; - AgcStatus delayedStatus; + DeviceStatus deviceStatus; AgcPrepareStatus prepareStatus; - /* Fetch the AWB status now because AWB also sets it in the prepare method. */ - fetchAwbStatus(imageMetadata); - - if (!imageMetadata->get("agc.delayed_status", delayedStatus)) - totalExposureValue = delayedStatus.totalExposureValue; - - prepareStatus.digitalGain = 1.0; prepareStatus.locked = false; + prepareStatus.digitalGain = 1.0; - if (status_.totalExposureValue) { - /* Process has run, so we have meaningful values. */ - DeviceStatus deviceStatus; - if (imageMetadata->get("device.status", deviceStatus) == 0) { - Duration actualExposure = deviceStatus.exposureTime * - deviceStatus.analogueGain; - if (actualExposure) { - double digitalGain = totalExposureValue / actualExposure; - LOG(RPiAgc, Debug) << "Want total exposure " << totalExposureValue; - /* - * Never ask for a gain < 1.0, and also impose - * some upper limit. Make it customisable? - */ - prepareStatus.digitalGain = std::max(1.0, std::min(digitalGain, - config_.maxDigitalGain)); - LOG(RPiAgc, Debug) << "Actual exposure " << actualExposure; - LOG(RPiAgc, Debug) << "Use digitalGain " << prepareStatus.digitalGain; - LOG(RPiAgc, Debug) << "Effective exposure " - << actualExposure * prepareStatus.digitalGain; - /* Decide whether AEC/AGC has converged. */ - prepareStatus.locked = updateLockStatus(deviceStatus); - } - } else - LOG(RPiAgc, Warning) << "AgcChannel: no device metadata"; - imageMetadata->set("agc.prepare_status", prepareStatus); + if (!imageMetadata->get("device.status", deviceStatus)) { + prepareStatus.locked = updateLockStatus(deviceStatus); + + /* + * For now, the IPA code is still expecting the digital gain to come back in + * the prepare_status. To keep things happy, we'll just fill in the value that + * we calculated previously and put in the AgcStatus (which comes back as the + * "delayed" status). Once the rest of the IPA code is updated, we'll be able + * to remove this, and indeed remove the digitalGain from the AgcPrepareStatus. + */ + AgcStatus delayedStatus; + if (!imageMetadata->get("agc.delayed_status", delayedStatus)) + prepareStatus.digitalGain = delayedStatus.digitalGain; + else + /* After a mode switch, this must be correct until new values come through. */ + prepareStatus.digitalGain = status_.digitalGain; } + + imageMetadata->set("agc.prepare_status", prepareStatus); } void AgcChannel::process(StatisticsPtr &stats, DeviceStatus const &deviceStatus, @@ -606,7 +586,7 @@ void AgcChannel::housekeepConfig() /* First fetch all the up-to-date settings, so no one else has to do it. */ status_.ev = ev_; status_.fixedExposureTime = limitExposureTime(fixedExposureTime_); - status_.fixedGain = fixedGain_; + status_.fixedGain = limitGain(fixedGain_); status_.flickerPeriod = flickerPeriod_; LOG(RPiAgc, Debug) << "ev " << status_.ev << " fixedExposureTime " << status_.fixedExposureTime << " fixedGain " @@ -657,6 +637,9 @@ void AgcChannel::fetchCurrentExposure(DeviceStatus const &deviceStatus) current_.analogueGain = deviceStatus.analogueGain; current_.totalExposure = 0s; /* this value is unused */ current_.totalExposureNoDG = current_.exposureTime * current_.analogueGain; + LOG(RPiAgc, Debug) << "Current frame: exposure time " << current_.exposureTime + << " ag " << current_.analogueGain + << " (total " << current_.totalExposureNoDG << ")"; } void AgcChannel::fetchAwbStatus(Metadata *imageMetadata) @@ -808,14 +791,12 @@ void AgcChannel::computeTargetExposure(double gain) target_.totalExposure = current_.totalExposureNoDG * gain; /* The final target exposure is also limited to what the exposure mode allows. */ Duration maxExposureTime = status_.fixedExposureTime - ? status_.fixedExposureTime - : exposureMode_->exposureTime.back(); + ? status_.fixedExposureTime + : exposureMode_->exposureTime.back(); maxExposureTime = limitExposureTime(maxExposureTime); - Duration maxTotalExposure = - maxExposureTime * - (status_.fixedGain != 0.0 - ? status_.fixedGain - : exposureMode_->gain.back()); + double maxGain = status_.fixedGain ? status_.fixedGain : exposureMode_->gain.back(); + maxGain = limitGain(maxGain); + Duration maxTotalExposure = maxExposureTime * maxGain; target_.totalExposure = std::min(target_.totalExposure, maxTotalExposure); } LOG(RPiAgc, Debug) << "Target totalExposure " << target_.totalExposure; @@ -824,8 +805,6 @@ void AgcChannel::computeTargetExposure(double gain) bool AgcChannel::applyChannelConstraints(const AgcChannelTotalExposures &channelTotalExposures) { bool channelBound = false; - LOG(RPiAgc, Debug) - << "Total exposure before channel constraints " << filtered_.totalExposure; for (const auto &constraint : config_.channelConstraints) { LOG(RPiAgc, Debug) @@ -860,7 +839,7 @@ bool AgcChannel::applyChannelConstraints(const AgcChannelTotalExposures &channel bool AgcChannel::applyDigitalGain(double gain, double targetY, bool channelBound) { - double dg = 1.0; + filtered_.totalExposureNoDG = filtered_.totalExposure; /* * Finally, if we're trying to reduce exposure but the target_Y is @@ -871,15 +850,14 @@ bool AgcChannel::applyDigitalGain(double gain, double targetY, bool channelBound * quickly (and we then approach the correct value more quickly from * below). */ - bool desaturate = false; - if (config_.desaturate) - desaturate = !channelBound && - targetY > config_.fastReduceThreshold && gain < sqrt(targetY); - if (desaturate) - dg /= config_.fastReduceThreshold; - LOG(RPiAgc, Debug) << "Digital gain " << dg << " desaturate? " << desaturate; - filtered_.totalExposureNoDG = filtered_.totalExposure / dg; - LOG(RPiAgc, Debug) << "Target totalExposureNoDG " << filtered_.totalExposureNoDG; + bool desaturate = config_.desaturate && !channelBound && + targetY > config_.fastReduceThreshold && gain < sqrt(targetY); + + if (desaturate) { + filtered_.totalExposureNoDG *= config_.fastReduceThreshold; + LOG(RPiAgc, Debug) << "Desaturating, exposure no dg " << filtered_.totalExposureNoDG; + } + return desaturate; } @@ -915,8 +893,7 @@ void AgcChannel::filterExposure() filtered_.totalExposure = speed * target_.totalExposure + filtered_.totalExposure * (1.0 - speed); } - LOG(RPiAgc, Debug) << "After filtering, totalExposure " << filtered_.totalExposure - << " no dg " << filtered_.totalExposureNoDG; + LOG(RPiAgc, Debug) << "After filtering, totalExposure " << filtered_.totalExposure; } void AgcChannel::divideUpExposure() @@ -957,9 +934,7 @@ void AgcChannel::divideUpExposure() } } } - LOG(RPiAgc, Debug) - << "Divided up exposure time and gain are " << exposureTime - << " and " << gain; + /* * Finally adjust exposure time for flicker avoidance (require both * exposure time and gain not to be fixed). @@ -970,22 +945,30 @@ void AgcChannel::divideUpExposure() if (flickerPeriods) { Duration newExposureTime = flickerPeriods * status_.flickerPeriod; gain *= exposureTime / newExposureTime; - /* - * We should still not allow the ag to go over the - * largest value in the exposure mode. Note that this - * may force more of the total exposure into the digital - * gain as a side-effect. - */ - gain = std::min(gain, exposureMode_->gain.back()); - gain = limitGain(gain); exposureTime = newExposureTime; } LOG(RPiAgc, Debug) << "After flicker avoidance, exposure time " << exposureTime << " gain " << gain; } + + /* Limit analogue gain to maximum allowed. */ + double analogueGain = std::min(gain, mode_.maxAnalogueGain); + + /* Finally work out the digital gain that we will need. */ + filtered_.totalExposureNoDG = analogueGain * exposureTime; + double digitalGain = filtered_.totalExposure / filtered_.totalExposureNoDG; + /* Limit dg by what is allowed. */ + digitalGain = std::min(digitalGain, config_.maxDigitalGain); + /* Update total exposure, in case the dg went down. */ + filtered_.totalExposure = filtered_.totalExposureNoDG * digitalGain; + filtered_.exposureTime = exposureTime; - /* We ask for all the gain as analogue gain; prepare() will be told what we got. */ - filtered_.analogueGain = gain; + filtered_.analogueGain = analogueGain; + filtered_.digitalGain = digitalGain; + LOG(RPiAgc, Debug) << "DivideUpExposure: total " << filtered_.totalExposure + << " no dg " << filtered_.totalExposureNoDG; + LOG(RPiAgc, Debug) << "DivideUpExposure: exp " << exposureTime + << " ag " << gain << " dg " << digitalGain; } void AgcChannel::writeAndFinish(Metadata *imageMetadata, bool desaturate) @@ -994,6 +977,7 @@ void AgcChannel::writeAndFinish(Metadata *imageMetadata, bool desaturate) status_.targetExposureValue = desaturate ? 0s : target_.totalExposure; status_.exposureTime = filtered_.exposureTime; status_.analogueGain = filtered_.analogueGain; + status_.digitalGain = filtered_.digitalGain; /* * Write to metadata as well, in case anyone wants to update the camera * immediately. @@ -1001,8 +985,6 @@ void AgcChannel::writeAndFinish(Metadata *imageMetadata, bool desaturate) imageMetadata->set("agc.status", status_); LOG(RPiAgc, Debug) << "Output written, total exposure requested is " << filtered_.totalExposure; - LOG(RPiAgc, Debug) << "Camera exposure update: exposure time " << filtered_.exposureTime - << " analogue gain " << filtered_.analogueGain; } Duration AgcChannel::limitExposureTime(Duration exposureTime) @@ -1031,6 +1013,7 @@ double AgcChannel::limitGain(double gain) const if (!gain) return gain; - gain = std::max(gain, mode_.minAnalogueGain); + gain = std::clamp(gain, mode_.minAnalogueGain, + mode_.maxAnalogueGain * config_.maxDigitalGain); return gain; } diff --git a/src/ipa/rpi/controller/rpi/agc_channel.h b/src/ipa/rpi/controller/rpi/agc_channel.h index 93229128abf1..42d85ec15e8d 100644 --- a/src/ipa/rpi/controller/rpi/agc_channel.h +++ b/src/ipa/rpi/controller/rpi/agc_channel.h @@ -135,6 +135,7 @@ private: libcamera::utils::Duration exposureTime; double analogueGain; + double digitalGain; libcamera::utils::Duration totalExposure; libcamera::utils::Duration totalExposureNoDG; /* without digital gain */ }; From patchwork Mon Jul 21 07:47:26 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 23865 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 F13C6BDCC1 for ; Mon, 21 Jul 2025 07:49:11 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 08B9468FD6; Mon, 21 Jul 2025 09:49:11 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="ZNP3hrSy"; dkim-atps=neutral Received: from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com [IPv6:2a00:1450:4864:20::32b]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E970368FCF for ; Mon, 21 Jul 2025 09:48:59 +0200 (CEST) Received: by mail-wm1-x32b.google.com with SMTP id 5b1f17b1804b1-4535fc0485dso7198475e9.0 for ; Mon, 21 Jul 2025 00:48:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; t=1753084139; x=1753688939; darn=lists.libcamera.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=lRuNQgN56gu4G18be4zT6O0Mp0CBKGWcJG4krGOuu/I=; b=ZNP3hrSyqb8qw7BH0oER+pc/4FPuInWNUGSr9l5D7tQP75RcHhfKLr9DEhH0czTGYq bWVoGNXnSyzgu3SKzhAwltrup66dCuUNJVyshd+jB7qWHA+dKqz0yY90jCecFpu/PBhR OXWfd+O92l9/jumoJ9dEFo15O3Zdsnn07mZBzozfUdZhFCD4ilBpN3SlugEWlV2tJLkK 8S5rbS7n9zGrsvyguNUdh9qnFm04wVEY+HQE6T/2o2KAZmhiNy7DTKDv4xT6HHbPUISi zfyWRfvH1Gl5PXy0cB6N59+dTXfOEW223snihE4YJd1ebFJGMoLuUzJx7hcnz9+f+dLL OTRw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1753084139; x=1753688939; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=lRuNQgN56gu4G18be4zT6O0Mp0CBKGWcJG4krGOuu/I=; b=otgPlnuP/sCFkjJkJWkuFRKvnvAdnieLyFNu3MQ/OP21PQNZAI8+r+BoV4DShN9BEH n0APZUO4QTjbi7DJ4n9gQBhYoONmcEak4Hl6vg2aLsLAOifywRJ+/AzZLIb3z+eRP8TZ 6QFHHLvxMoAJRKRaeIrZ1ySWe0O8BJ9oJWVBFvj9hoX9Qf3B59PFE+SHPm4rhLpu64Hg w85/KmLpDcUWt9F3Z6c7eSxnPs8GlVM/JOLlzbbU+L00HViNlfNbroY+X9KeNtJHf1xy aHrWl0XTDcDxNkVXBYtFKi8agbCdh4+3gUhpCscnq+bk/hFoiTf3P0xcz0LTrrVKthrL QXmw== X-Gm-Message-State: AOJu0YwVYjj2M64QHaf8/C1ePOGNKH8X+nv5pR1vhjovLO7KBcQZMmSB ee3VDKST7kUnMNKYvPh3c82WViQLLyOPGTPQXiw5Deh/8lZbq0IUw01eyWQwhAHQAgvT502wu2z rG+EFZ0A= X-Gm-Gg: ASbGncuz5rIbeeXIEjYmRS3AtWGgN4elZtsKRLm3TYBF12ILsGaD5iExY6tsFxOd4o8 ujRzJ7eISF/KJjH5OBVHQrFWtPc7uq6g0tlzgW8m9c4etCFeczF7Y73JspqT3jj1bfUhEMlDxI4 etR3o/9oMFq20x492HM+/eIdQbvKgeyH32mQW1zKV7wUwgtWUk7HtAU52utn9WnhH3FuX3CJMhV i/2RW6MnyfZJ2ewQFOdzhLgIjDx9Gzj4N8ZAIsAY6PZTPgsy7/CSIIjH3i7/h4in3YlJWIe0I2f icqwB15FQm0DbEQENUmpfEyr3fBgsmpOijbi4KwhFQvd16/hSCoTNX6JaG0AbivQ0TBF1Nvdy+y egQcdYQgvwPWYE9t9tXiiM8vMGiezeznroNyFQgroxA== X-Google-Smtp-Source: AGHT+IFepQHqYX8SHoX0hOEQLEsBO3hROqmRv/PqYYRQoGPJDWcGFDA/TvTD2c0Kj6NQfd7y0UKezw== X-Received: by 2002:a05:600c:a306:b0:456:7cf:5268 with SMTP id 5b1f17b1804b1-4562e277c74mr55034845e9.4.1753084138935; Mon, 21 Jul 2025 00:48:58 -0700 (PDT) Received: from NAUSH-P-DELL.pitowers.org ([93.93.133.154]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4562e802afasm151101765e9.12.2025.07.21.00.48.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Jul 2025 00:48:58 -0700 (PDT) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Cc: David Plowman , Naushir Patuck Subject: [PATCH v2 6/7] ipa: rpi: Update digital gain handling in IPA base and derived classes Date: Mon, 21 Jul 2025 08:47:26 +0100 Message-ID: <20250721074853.1463358-7-naush@raspberrypi.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250721074853.1463358-1-naush@raspberrypi.com> References: <20250721074853.1463358-1-naush@raspberrypi.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" From: David Plowman Here we update the digital gain handling to use the value computed by process() in the AgcStatus, not the version that was previously in the AgcPrepareStatus. Because we apply this digital gain directly with no further modification, we have to update it to reflect any exposure/gain quantisation that happens (in IpaBase::applyAGC). We must also run the new platformPrepareAgc() even when we're skipping platformPrepareIsp(), which has been split out of the previous platformPrepareIsp() implementation. Signed-off-by: David Plowman Signed-off-by: Naushir Patuck Reviewed-by: Naushir Patuck --- src/ipa/rpi/common/ipa_base.cpp | 50 +++++++++++++++++++-------- src/ipa/rpi/common/ipa_base.h | 6 +++- src/ipa/rpi/pisp/pisp.cpp | 61 +++++++++++++++++++++------------ src/ipa/rpi/vc4/vc4.cpp | 28 +++++++++------ 4 files changed, 98 insertions(+), 47 deletions(-) diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp index 98690b80d5d3..1408c5e32a69 100644 --- a/src/ipa/rpi/common/ipa_base.cpp +++ b/src/ipa/rpi/common/ipa_base.cpp @@ -298,20 +298,23 @@ void IpaBase::start(const ControlList &controls, StartResult *result) frameLengths_.clear(); frameLengths_.resize(FrameLengthsQueueSize, 0s); - /* SwitchMode may supply updated exposure/gain values to use. */ - AgcStatus agcStatus; - agcStatus.exposureTime = 0.0s; - agcStatus.analogueGain = 0.0; + /* + * SwitchMode may supply updated exposure/gain values to use. + * agcStatus_ will store these values for us to use until delayed_status values + * start to appear. + */ + agcStatus_.exposureTime = 0.0s; + agcStatus_.analogueGain = 0.0; - metadata.get("agc.status", agcStatus); - if (agcStatus.exposureTime && agcStatus.analogueGain) { + metadata.get("agc.status", agcStatus_); + if (agcStatus_.exposureTime && agcStatus_.analogueGain) { ControlList ctrls(sensorCtrls_); - applyAGC(&agcStatus, ctrls); + applyAGC(&agcStatus_, ctrls); result->controls = std::move(ctrls); setCameraTimeoutValue(); } /* Make a note of this as it tells us the HDR status of the first few frames. */ - hdrStatus_ = agcStatus.hdr; + hdrStatus_ = agcStatus_.hdr; /* * AF: If no lens position was specified, drive lens to a default position. @@ -486,7 +489,9 @@ void IpaBase::prepareIsp(const PrepareParams ¶ms) controller_.prepare(&rpiMetadata); /* Actually prepare the ISP parameters for the frame. */ platformPrepareIsp(params, rpiMetadata); - } + platformPrepareAgc(rpiMetadata); + } else + platformPrepareAgc(rpiMetadata); frameCount_++; @@ -525,6 +530,7 @@ void IpaBase::processStats(const ProcessParams ¶ms) if (rpiMetadata.get("agc.status", agcStatus) == 0) { ControlList ctrls(sensorCtrls_); applyAGC(&agcStatus, ctrls); + rpiMetadata.set("agc.status", agcStatus); setDelayedControls.emit(ctrls, ipaContext); setCameraTimeoutValue(); } @@ -1422,9 +1428,6 @@ void IpaBase::reportMetadata(unsigned int ipaContext) } AgcPrepareStatus *agcPrepareStatus = rpiMetadata.getLocked("agc.prepare_status"); - if (agcPrepareStatus) - libcameraMetadata_.set(controls::DigitalGain, agcPrepareStatus->digitalGain); - RPiController::AgcAlgorithm *agc = dynamic_cast( controller_.getAlgorithm("agc")); if (agc) { @@ -1437,6 +1440,13 @@ void IpaBase::reportMetadata(unsigned int ipaContext) : controls::AeStateSearching); } + const AgcStatus *agcStatus = rpiMetadata.getLocked("agc.delayed_status"); + if (agcStatus) + libcameraMetadata_.set(controls::DigitalGain, agcStatus->digitalGain); + else + libcameraMetadata_.set(controls::DigitalGain, agcStatus_.digitalGain); + /* The HDR metadata reporting will use this agcStatus too. */ + LuxStatus *luxStatus = rpiMetadata.getLocked("lux.status"); if (luxStatus) libcameraMetadata_.set(controls::Lux, luxStatus->lux); @@ -1526,7 +1536,6 @@ void IpaBase::reportMetadata(unsigned int ipaContext) * delayed_status to be available, we use the HDR status that came out of the * switchMode call. */ - const AgcStatus *agcStatus = rpiMetadata.getLocked("agc.delayed_status"); const HdrStatus &hdrStatus = agcStatus ? agcStatus->hdr : hdrStatus_; if (!hdrStatus.mode.empty() && hdrStatus.mode != "Off") { int32_t hdrMode = controls::HdrModeOff; @@ -1584,7 +1593,7 @@ void IpaBase::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDu agc->setMaxExposureTime(maxExposureTime); } -void IpaBase::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls) +void IpaBase::applyAGC(struct AgcStatus *agcStatus, ControlList &ctrls) { const int32_t minGainCode = helper_->gainCode(mode_.minAnalogueGain); const int32_t maxGainCode = helper_->gainCode(mode_.maxAnalogueGain); @@ -1613,6 +1622,19 @@ void IpaBase::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls) ctrls.set(V4L2_CID_EXPOSURE, exposureLines); ctrls.set(V4L2_CID_ANALOGUE_GAIN, gainCode); + /* + * We must update the digital gain to make up for any quantisation that happens, and + * communicate that back into the metadata so that it will appear as the "delayed" status. + * (Note that "exposure" is already the "actual" exposure.) + */ + double actualGain = helper_->gain(gainCode); + double ratio = agcStatus->analogueGain / actualGain; + ratio *= agcStatus->exposureTime / exposure; + double newDigitalGain = agcStatus->digitalGain * ratio; + agcStatus->digitalGain = newDigitalGain; + agcStatus->analogueGain = actualGain; + agcStatus->exposureTime = exposure; + /* * At present, there is no way of knowing if a control is read-only. * As a workaround, assume that if the minimum and maximum values of diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h index e818104ba633..e2f6e330b2ab 100644 --- a/src/ipa/rpi/common/ipa_base.h +++ b/src/ipa/rpi/common/ipa_base.h @@ -73,6 +73,9 @@ protected: /* Remember the HDR status after a mode switch. */ HdrStatus hdrStatus_; + /* Remember the AGC status after a mode switch. */ + AgcStatus agcStatus_; + /* Whether the stitch block (if available) needs to swap buffers. */ bool stitchSwapBuffers_; @@ -86,6 +89,7 @@ private: virtual void platformPrepareIsp(const PrepareParams ¶ms, RPiController::Metadata &rpiMetadata) = 0; + virtual void platformPrepareAgc(RPiController::Metadata &rpiMetadata) = 0; virtual RPiController::StatisticsPtr platformProcessStats(Span mem) = 0; void setMode(const IPACameraSensorInfo &sensorInfo); @@ -97,7 +101,7 @@ private: void fillDeviceStatus(const ControlList &sensorControls, unsigned int ipaContext); void reportMetadata(unsigned int ipaContext); void applyFrameDurations(utils::Duration minFrameDuration, utils::Duration maxFrameDuration); - void applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls); + void applyAGC(struct AgcStatus *agcStatus, ControlList &ctrls); std::map buffers_; diff --git a/src/ipa/rpi/pisp/pisp.cpp b/src/ipa/rpi/pisp/pisp.cpp index e1a804f533bb..ab70d8f42636 100644 --- a/src/ipa/rpi/pisp/pisp.cpp +++ b/src/ipa/rpi/pisp/pisp.cpp @@ -218,13 +218,14 @@ private: void platformPrepareIsp(const PrepareParams ¶ms, RPiController::Metadata &rpiMetadata) override; + void platformPrepareAgc(RPiController::Metadata &rpiMetadata) override; RPiController::StatisticsPtr platformProcessStats(Span mem) override; void handleControls(const ControlList &controls) override; - void applyWBG(const AwbStatus *awbStatus, const AgcPrepareStatus *agcStatus, + void applyWBG(const AwbStatus *awbStatus, double digitalGain, pisp_be_global_config &global); - void applyDgOnly(const AgcPrepareStatus *agcPrepareStatus, pisp_be_global_config &global); + void applyDgOnly(double digitalGain, pisp_be_global_config &global); void applyCAC(const CacStatus *cacStatus, pisp_be_global_config &global); void applyContrast(const ContrastStatus *contrastStatus, pisp_be_global_config &global); @@ -341,7 +342,6 @@ void IpaPiSP::platformPrepareIsp([[maybe_unused]] const PrepareParams ¶ms, PISP_BE_RGB_ENABLE_SHARPEN + PISP_BE_RGB_ENABLE_SAT_CONTROL); NoiseStatus *noiseStatus = rpiMetadata.getLocked("noise.status"); - AgcPrepareStatus *agcPrepareStatus = rpiMetadata.getLocked("agc.prepare_status"); { /* All Frontend config goes first, we do not want to hold the FE lock for long! */ @@ -355,14 +355,6 @@ void IpaPiSP::platformPrepareIsp([[maybe_unused]] const PrepareParams ¶ms, if (blackLevelStatus) applyBlackLevel(blackLevelStatus, global); - AwbStatus *awbStatus = rpiMetadata.getLocked("awb.status"); - if (awbStatus && agcPrepareStatus) { - /* Applies digital gain as well. */ - applyWBG(awbStatus, agcPrepareStatus, global); - } else if (agcPrepareStatus) { - /* Mono sensor fallback for digital gain. */ - applyDgOnly(agcPrepareStatus, global); - } } CacStatus *cacStatus = rpiMetadata.getLocked("cac.status"); @@ -443,6 +435,34 @@ void IpaPiSP::platformPrepareIsp([[maybe_unused]] const PrepareParams ¶ms, } } +void IpaPiSP::platformPrepareAgc(RPiController::Metadata &rpiMetadata) +{ + std::scoped_lock l(rpiMetadata); + + AgcStatus *delayedAgcStatus = rpiMetadata.getLocked("agc.delayed_status"); + /* If no delayed status, use the gain from the last mode switch. */ + double digitalGain = delayedAgcStatus ? delayedAgcStatus->digitalGain : agcStatus_.digitalGain; + AwbStatus *awbStatus = rpiMetadata.getLocked("awb.status"); + + pisp_be_global_config global; + be_->GetGlobal(global); + + { + /* All Frontend config goes first, we do not want to hold the FE lock for long! */ + std::scoped_lock lf(*fe_); + + if (awbStatus) { + /* Applies digital gain as well. */ + applyWBG(awbStatus, digitalGain, global); + } else { + /* Mono sensor fallback for digital gain. */ + applyDgOnly(digitalGain, global); + } + } + + be_->SetGlobal(global); +} + RPiController::StatisticsPtr IpaPiSP::platformProcessStats(Span mem) { using namespace RPiController; @@ -515,12 +535,11 @@ void IpaPiSP::handleControls(const ControlList &controls) } } -void IpaPiSP::applyWBG(const AwbStatus *awbStatus, const AgcPrepareStatus *agcPrepareStatus, +void IpaPiSP::applyWBG(const AwbStatus *awbStatus, double digitalGain, pisp_be_global_config &global) { pisp_wbg_config wbg; pisp_fe_rgby_config rgby = {}; - double dg = agcPrepareStatus ? agcPrepareStatus->digitalGain : 1.0; double minColourGain = std::min({ awbStatus->gainR, awbStatus->gainG, awbStatus->gainB, 1.0 }); /* The 0.1 here doesn't mean much, but just stops arithmetic errors and extreme behaviour. */ double extraGain = 1.0 / std::max({ minColourGain, 0.1 }); @@ -536,9 +555,9 @@ void IpaPiSP::applyWBG(const AwbStatus *awbStatus, const AgcPrepareStatus *agcPr double gainG = awbStatus->gainG * extraGain; double gainB = awbStatus->gainB * extraGain; - wbg.gain_r = clampField(dg * gainR, 14, 10); - wbg.gain_g = clampField(dg * gainG, 14, 10); - wbg.gain_b = clampField(dg * gainB, 14, 10); + wbg.gain_r = clampField(digitalGain * gainR, 14, 10); + wbg.gain_g = clampField(digitalGain * gainG, 14, 10); + wbg.gain_b = clampField(digitalGain * gainB, 14, 10); /* * The YCbCr conversion block should contain the appropriate YCbCr @@ -561,15 +580,15 @@ void IpaPiSP::applyWBG(const AwbStatus *awbStatus, const AgcPrepareStatus *agcPr global.bayer_enables |= PISP_BE_BAYER_ENABLE_WBG; } -void IpaPiSP::applyDgOnly(const AgcPrepareStatus *agcPrepareStatus, pisp_be_global_config &global) +void IpaPiSP::applyDgOnly(double digitalGain, pisp_be_global_config &global) { pisp_wbg_config wbg; - wbg.gain_r = clampField(agcPrepareStatus->digitalGain, 14, 10); - wbg.gain_g = clampField(agcPrepareStatus->digitalGain, 14, 10); - wbg.gain_b = clampField(agcPrepareStatus->digitalGain, 14, 10); + wbg.gain_r = clampField(digitalGain, 14, 10); + wbg.gain_g = clampField(digitalGain, 14, 10); + wbg.gain_b = clampField(digitalGain, 14, 10); - LOG(IPARPI, Debug) << "Applying DG (only) : " << agcPrepareStatus->digitalGain; + LOG(IPARPI, Debug) << "Applying DG (only) : " << digitalGain; be_->SetWbg(wbg); global.bayer_enables |= PISP_BE_BAYER_ENABLE_WBG; diff --git a/src/ipa/rpi/vc4/vc4.cpp b/src/ipa/rpi/vc4/vc4.cpp index 8a7a37c870ed..b2fec9344804 100644 --- a/src/ipa/rpi/vc4/vc4.cpp +++ b/src/ipa/rpi/vc4/vc4.cpp @@ -57,14 +57,14 @@ private: int32_t platformConfigure(const ConfigParams ¶ms, ConfigResult *result) override; void platformPrepareIsp(const PrepareParams ¶ms, RPiController::Metadata &rpiMetadata) override; + void platformPrepareAgc([[maybe_unused]] RPiController::Metadata &rpiMetadata) override; RPiController::StatisticsPtr platformProcessStats(Span mem) override; void handleControls(const ControlList &controls) override; bool validateIspControls(); void applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls); - void applyDG(const struct AgcPrepareStatus *dgStatus, - const struct AwbStatus *awbStatus, ControlList &ctrls); + void applyDG(double digitalGain, const struct AwbStatus *awbStatus, ControlList &ctrls); void applyCCM(const struct CcmStatus *ccmStatus, ControlList &ctrls); void applyBlackLevel(const struct BlackLevelStatus *blackLevelStatus, ControlList &ctrls); void applyGamma(const struct ContrastStatus *contrastStatus, ControlList &ctrls); @@ -78,6 +78,7 @@ private: /* VC4 ISP controls. */ ControlInfoMap ispCtrls_; + ControlList ctrls_; /* LS table allocation passed in from the pipeline handler. */ SharedFD lsTableHandle_; @@ -107,6 +108,7 @@ int32_t IpaVc4::platformStart([[maybe_unused]] const ControlList &controls, int32_t IpaVc4::platformConfigure(const ConfigParams ¶ms, [[maybe_unused]] ConfigResult *result) { ispCtrls_ = params.ispControls; + ctrls_ = ControlList(ispCtrls_); if (!validateIspControls()) { LOG(IPARPI, Error) << "ISP control validation failed."; return -1; @@ -139,7 +141,7 @@ int32_t IpaVc4::platformConfigure(const ConfigParams ¶ms, [[maybe_unused]] C void IpaVc4::platformPrepareIsp([[maybe_unused]] const PrepareParams ¶ms, RPiController::Metadata &rpiMetadata) { - ControlList ctrls(ispCtrls_); + ControlList &ctrls = ctrls_; /* Lock the metadata buffer to avoid constant locks/unlocks. */ std::unique_lock lock(rpiMetadata); @@ -152,9 +154,6 @@ void IpaVc4::platformPrepareIsp([[maybe_unused]] const PrepareParams ¶ms, if (ccmStatus) applyCCM(ccmStatus, ctrls); - AgcPrepareStatus *dgStatus = rpiMetadata.getLocked("agc.prepare_status"); - applyDG(dgStatus, awbStatus, ctrls); - AlscStatus *lsStatus = rpiMetadata.getLocked("alsc.status"); if (lsStatus) applyLS(lsStatus, ctrls); @@ -190,9 +189,18 @@ void IpaVc4::platformPrepareIsp([[maybe_unused]] const PrepareParams ¶ms, if (!lensctrls.empty()) setLensControls.emit(lensctrls); } +} - if (!ctrls.empty()) - setIspControls.emit(ctrls); +void IpaVc4::platformPrepareAgc(RPiController::Metadata &rpiMetadata) +{ + AgcStatus *delayedAgcStatus = rpiMetadata.getLocked("agc.delayed_status"); + double digitalGain = delayedAgcStatus ? delayedAgcStatus->digitalGain : agcStatus_.digitalGain; + AwbStatus *awbStatus = rpiMetadata.getLocked("awb.status"); + + applyDG(digitalGain, awbStatus, ctrls_); + + setIspControls.emit(ctrls_); + ctrls_ = ControlList(ispCtrls_); } RPiController::StatisticsPtr IpaVc4::platformProcessStats(Span mem) @@ -329,11 +337,9 @@ void IpaVc4::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls) static_cast(awbStatus->gainB * 1000)); } -void IpaVc4::applyDG(const struct AgcPrepareStatus *dgStatus, +void IpaVc4::applyDG(double digitalGain, const struct AwbStatus *awbStatus, ControlList &ctrls) { - double digitalGain = dgStatus ? dgStatus->digitalGain : 1.0; - if (awbStatus) { /* * We must apply sufficient extra digital gain to stop any of the channel gains being From patchwork Mon Jul 21 07:47:27 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 23866 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 F3BD9C3323 for ; Mon, 21 Jul 2025 07:49:12 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id DE24868FE1; Mon, 21 Jul 2025 09:49:11 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="RvU4LSMX"; dkim-atps=neutral Received: from mail-wr1-x42c.google.com (mail-wr1-x42c.google.com [IPv6:2a00:1450:4864:20::42c]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3F08868FD1 for ; Mon, 21 Jul 2025 09:49:00 +0200 (CEST) Received: by mail-wr1-x42c.google.com with SMTP id ffacd0b85a97d-3a4e749d7b2so660866f8f.0 for ; Mon, 21 Jul 2025 00:49:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; t=1753084139; x=1753688939; darn=lists.libcamera.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=yCqWjxhzUe3uZSUifzeDiS7Du7OgdYFrmNRNGtduVSU=; b=RvU4LSMXHxQwp1YcW0K7d2gjQTCvuaDEEskQ8D5EkkxFOO93j0OtuxanIAoaFw0d21 +W8hQqUq3t8WEnExgW3CWaOQhWjORQk5UB8X5HUBoPT6cKy0RK+7y0Zu86uCckPesP6e 8kR9wcGIrFU+KKVqWBLvByXwqd75qv2tjO7+b8MiV9trCUehO9z8Gc1aohUdt9ObELaq Tml3ML88NbQ9gjQ5+FxJvxFKJXV6E27Ybkg609DWd5HrQD7yzUk1Xj8qvFecbNx+0BI4 SjTsrzT94gCTULC9XcCzN60WwKlQCdAaWn9PDpHp4CufvhyBnd7VpHyzXa00OG1zw+l5 wDLA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1753084139; x=1753688939; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=yCqWjxhzUe3uZSUifzeDiS7Du7OgdYFrmNRNGtduVSU=; b=iutCqxsxrgablf+25ALnsV51eloWAJVgXYHSBbNIY7iPiO/+ZRZUlyoGtdnUVVt+NV ixRMcVG3mdXipsuniad/A4BW2lPNkZxuBwSHas1goFKoFkJV2w+qBzOgr5GS/6NuIqeA J+cyDSW382qGzExXVvU//Suwi0VKKK/uIvXV8ElnC47+zT2yk5WT4E/KTQrP/Bf7NYgp HX0UegzqghGaLauCTVFN3UZtFAyk9WhAsvKJQKjlq1Awdtr1FOCsPuPES4Qbgmp/xu7Q hTBjb0MS06VBxEduT3FVWTzCJTLtu9o/T1BbZ0gJ4jwF/oj5qOYMwKOFcBI5/NxRWjDj UFoA== X-Gm-Message-State: AOJu0YxekZf4jHM5q7tAKiujcaAbuxNrx3ddRWDXpQZJjzo6T+lmKdrb yKvCni+3u5ItMr8XSteaiEQZ4xL8W4Pscek3gYlorWEW3zynWH2Yp/jBtNzBeKFFTSGD3HhQwxL edBhKK4U= X-Gm-Gg: ASbGncu04ppl+wVSWzXC7ThDHTNbwKeeh5QAHt5gPOKjwngQ3HVrxjvvwrrKvzx/Ay3 /Oh0c8gw4nM0Ptg5BVwxnPpYpsuKo06xaFKYz5aan6zE6/05saaUwrglHoAtsm5X+DrMJI82izR UUZy7hk0o2G8QhFVqnLxvpd506rGdMzZ8GdSJqIj1ytmkqMkouRpBh7LlMpHLmhQSrAng53zyfu Uj1JDjPK2TUQaAHtmfno4BICz2+pdGuJqmYOk0SnwmGy/hOYERtqqrUpQkmFpT14dNRwvXEsL8i mXI9L+nvICpLRWja4oVpMoFQ+mbCKJQyiw9xY7ANIqiaZk7ZaLQLGVbTDXEd5B4tCyDf1fP6Qxt m8eiteQqM8A+Kzwvcbl62eRV6uf3llKIJUo+NvEgSVw== X-Google-Smtp-Source: AGHT+IHFvQrr5G0b0Ek2oDufRoXndAb4rhnfIcSZTooXgba9jnsdFbTpa4uIcb/wX7rTa62IJt6wQA== X-Received: by 2002:a05:6000:40c7:b0:3b6:18c4:e5af with SMTP id ffacd0b85a97d-3b618c4e70bmr4213953f8f.10.1753084139480; Mon, 21 Jul 2025 00:48:59 -0700 (PDT) Received: from NAUSH-P-DELL.pitowers.org ([93.93.133.154]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4562e802afasm151101765e9.12.2025.07.21.00.48.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 21 Jul 2025 00:48:59 -0700 (PDT) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Cc: David Plowman , Naushir Patuck Subject: [PATCH v2 7/7] ipa: rpi: agc: Remove digital gain from AgcPrepareStatus Date: Mon, 21 Jul 2025 08:47:27 +0100 Message-ID: <20250721074853.1463358-8-naush@raspberrypi.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250721074853.1463358-1-naush@raspberrypi.com> References: <20250721074853.1463358-1-naush@raspberrypi.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" From: David Plowman All platforms are now using the digital gain from the AgcStatus, so the AgcPrepareStatus and prepare() methods can be tidied up. Signed-off-by: David Plowman Signed-off-by: Naushir Patuck Reviewed-by: Naushir Patuck --- src/ipa/rpi/controller/agc_status.h | 1 - src/ipa/rpi/controller/rpi/agc_channel.cpp | 18 +----------------- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/src/ipa/rpi/controller/agc_status.h b/src/ipa/rpi/controller/agc_status.h index 956d6abf398c..1ae069f2ee8a 100644 --- a/src/ipa/rpi/controller/agc_status.h +++ b/src/ipa/rpi/controller/agc_status.h @@ -44,6 +44,5 @@ struct AgcStatus { }; struct AgcPrepareStatus { - double digitalGain; int locked; }; diff --git a/src/ipa/rpi/controller/rpi/agc_channel.cpp b/src/ipa/rpi/controller/rpi/agc_channel.cpp index 7da1c6dbbec7..cc944e6dd54e 100644 --- a/src/ipa/rpi/controller/rpi/agc_channel.cpp +++ b/src/ipa/rpi/controller/rpi/agc_channel.cpp @@ -480,26 +480,10 @@ void AgcChannel::prepare(Metadata *imageMetadata) AgcPrepareStatus prepareStatus; prepareStatus.locked = false; - prepareStatus.digitalGain = 1.0; - if (!imageMetadata->get("device.status", deviceStatus)) { + if (!imageMetadata->get("device.status", deviceStatus)) prepareStatus.locked = updateLockStatus(deviceStatus); - /* - * For now, the IPA code is still expecting the digital gain to come back in - * the prepare_status. To keep things happy, we'll just fill in the value that - * we calculated previously and put in the AgcStatus (which comes back as the - * "delayed" status). Once the rest of the IPA code is updated, we'll be able - * to remove this, and indeed remove the digitalGain from the AgcPrepareStatus. - */ - AgcStatus delayedStatus; - if (!imageMetadata->get("agc.delayed_status", delayedStatus)) - prepareStatus.digitalGain = delayedStatus.digitalGain; - else - /* After a mode switch, this must be correct until new values come through. */ - prepareStatus.digitalGain = status_.digitalGain; - } - imageMetadata->set("agc.prepare_status", prepareStatus); }