From 4b5541b0862587bb099d127cb6e941b8568d7599 Mon Sep 17 00:00:00 2001 From: David Plowman Date: Mon, 21 Jul 2025 08:47:23 +0100 Subject: [PATCH] ipa: rpi: agc: Rename "analogue gain" to "gain" where appropriate 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 Signed-off-by: Kieran Bingham --- 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 a5bdcbb5..8b4eb75e 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 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 52851ee1..564beaf1 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 */