From patchwork Tue Jun 17 08:29:51 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Plowman X-Patchwork-Id: 23586 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 335CDBDE6B for ; Tue, 17 Jun 2025 08:30:15 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BACE968DC1; Tue, 17 Jun 2025 10:30:10 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="Yd2oYbvF"; dkim-atps=neutral Received: from mail-wm1-x32c.google.com (mail-wm1-x32c.google.com [IPv6:2a00:1450:4864:20::32c]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3897768DCF for ; Tue, 17 Jun 2025 10:30:04 +0200 (CEST) Received: by mail-wm1-x32c.google.com with SMTP id 5b1f17b1804b1-450ccda1a6eso48181355e9.2 for ; Tue, 17 Jun 2025 01:30:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; t=1750149003; x=1750753803; 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=qOB8nBAG4kp9rTIZ7GZcgT3x3VEbWhGE6/qVbsYOuEk=; b=Yd2oYbvFmcg4dBIQMlrzR2FS5tiYDGbcPSDUZhvL73VQuei6LyU3Aw4g3HsIN0FTDb TPLvUhfB/UL2JeWM5izQ27HcG1GwrpQX5WHZswX/17IUGZxh85ZuNPULmlnlf/QGXy3c HmbRWUmhryswzdtfjU4j03VHwDYKQ1j/avEoE9zTHth1eAGux+hKY4Vi5DTYU20kdxkD y/jv15cCZhZQNuzd1zJZgZUGyy9FW5ch0NEOc6ahZ7N7SxJHPm60dUKFIxsie/vjARSW PdsTReswMnjqNr5X3yJHQbMBlBjRZKxZ/6wSSTVEYszIpyd+eJn3fSJgnerfNcfrJ6zF pJzA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1750149003; x=1750753803; 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=qOB8nBAG4kp9rTIZ7GZcgT3x3VEbWhGE6/qVbsYOuEk=; b=M/mi0B8Rdm+f7TH8h/hKD6sNgBky7UgNyP6nrX+Uu9ymvg6jHxxusdT1w3XpAtlNiH jPE52S1ZsRmfLR4Y4bufrwP9SnA2TGvP4P+TNtmcUM29Su/DhGx25HSzrsiXT/bx0h4r ExVgLhiM+Dm1fYQu2jxzux07jUNBylMOKyb44C9vBTZyQ7jZJluVtthjGBnM913Q4AO8 iPSk5OIEzp9wCoQm75PYpBf3hsN3R0T2C1BWCCHjM7w6nler/beSZHDSh4sK9nV9/xha ERvUbgaG8P59v11mVI+TBTDmUR4jPDz2LxFhCKY7FzkvVK3Z4HwuQV8px8O0V46hJcbV lonw== X-Gm-Message-State: AOJu0Yyd2OHeuC52el3issbJNOMfK66Q0WX6zgme74xNE+bJYAzCO06u utcnUUogMij90y9GtcUpqV/h+ZSabuhGRD2HJ7VDDRcarezffxZm/fu06KfKl+L+lh/slWJYm4H T93lN X-Gm-Gg: ASbGncs5ahmO2+OYBVlYRmtEv/s6mrZ/ysAn7T21rRJfQFu8DkWATT+qEp4L6P9c6/0 U62cQ0JHfkKn1Vg42DYbOp63GAEIRohXmOeeM/O8BOhNIAFWH3V340DDleuvH2TN4sasf4w/Cmb kdkpBIqBf/vpRdL3MnIdcALY7Le7xtuWjvO01/k95R5L47bXD3vg0o6z1WYp2dIy7HwdOCq+q3p elanuMxbh4wGRc0KTa33lz5n030IfuwTIS0JMJTSQtNORVY9RcsRrOfNGTi2fjc5DcdTh4psYS7 3v0dHATcDaL6djgUOrGUxPBpE/n5SCu72+ScTN4M2nR3RTzsCprXas00zKIzEGBYJAcZiehHyhx L6sgUa9CbWr70khlCZA== X-Google-Smtp-Source: AGHT+IF36OMgQNa4YXaSjfrqmWm4p2UXIoYFc9LsF/dUsx+UU812IziUoZctEEhl1DCTZ4qQkh6LSQ== X-Received: by 2002:a05:600c:3513:b0:453:a88:d509 with SMTP id 5b1f17b1804b1-4533ca7443cmr150359325e9.10.1750149003046; Tue, 17 Jun 2025 01:30:03 -0700 (PDT) Received: from raspberrypi.pitowers.org ([2a00:1098:3142:1f:ffc9:aff6:7f7f:893b]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4532e259108sm166062955e9.32.2025.06.17.01.30.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 17 Jun 2025 01:30:02 -0700 (PDT) From: David Plowman To: libcamera-devel@lists.libcamera.org Cc: David Plowman Subject: [PATCH 3/7] ipa: rpi: agc: Rename "analogue gain" to "gain" where appropriate Date: Tue, 17 Jun 2025 09:29:51 +0100 Message-Id: <20250617082956.5699-4-david.plowman@raspberrypi.com> X-Mailer: git-send-email 2.39.5 In-Reply-To: <20250617082956.5699-1-david.plowman@raspberrypi.com> References: <20250617082956.5699-1-david.plowman@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" 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 --- 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 80c17588..05699228 100644 --- a/src/ipa/rpi/common/ipa_base.cpp +++ b/src/ipa/rpi/common/ipa_base.cpp @@ -873,7 +873,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 fdaa10e6..9fa2bd20 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 9308b156..d4cedcf4 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 02bfdb4a..afda2e36 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 c3a940bf..966630a2 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 d7fff2fc..1028d82e 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 e3475d86..93229128 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 */