[libcamera-devel,v1,6/6] ipa: rpi: agc: Make AGC controls affect all channels
diff mbox series

Message ID 20231020084002.30665-7-naush@raspberrypi.com
State Accepted
Commit 02eea043f2a4d976c2c40e5bc382096c3a536632
Headers show
Series
  • Raspberry Pi: Preliminary PiSP support (round 2)
Related show

Commit Message

Naushir Patuck Oct. 20, 2023, 8:40 a.m. UTC
From: David Plowman <david.plowman@raspberrypi.com>

We need to be able to do things like enable/disable AGC for all the
channels, so most of the AGC controls are updated to be applied to all
channels. There are a couple of exceptions, such as setting explicit
shutter/gain values, which apply only to channel 0.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
---
 src/ipa/rpi/common/ipa_base.cpp        | 14 +++----
 src/ipa/rpi/controller/agc_algorithm.h | 11 +++---
 src/ipa/rpi/controller/rpi/agc.cpp     | 51 +++++++++++++-------------
 src/ipa/rpi/controller/rpi/agc.h       | 13 +++----
 4 files changed, 42 insertions(+), 47 deletions(-)

Patch
diff mbox series

diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp
index 2583c6226f44..a1fec3aa3dd1 100644
--- a/src/ipa/rpi/common/ipa_base.cpp
+++ b/src/ipa/rpi/common/ipa_base.cpp
@@ -702,9 +702,9 @@  void IpaBase::applyControls(const ControlList &controls)
 			}
 
 			if (ctrl.second.get<bool>() == false)
-				agc->disableAuto(0);
+				agc->disableAuto();
 			else
-				agc->enableAuto(0);
+				agc->enableAuto();
 
 			libcameraMetadata_.set(controls::AeEnable, ctrl.second.get<bool>());
 			break;
@@ -773,7 +773,7 @@  void IpaBase::applyControls(const ControlList &controls)
 
 			int32_t idx = ctrl.second.get<int32_t>();
 			if (ConstraintModeTable.count(idx)) {
-				agc->setConstraintMode(0, ConstraintModeTable.at(idx));
+				agc->setConstraintMode(ConstraintModeTable.at(idx));
 				libcameraMetadata_.set(controls::AeConstraintMode, idx);
 			} else {
 				LOG(IPARPI, Error) << "Constraint mode " << idx
@@ -793,7 +793,7 @@  void IpaBase::applyControls(const ControlList &controls)
 
 			int32_t idx = ctrl.second.get<int32_t>();
 			if (ExposureModeTable.count(idx)) {
-				agc->setExposureMode(0, ExposureModeTable.at(idx));
+				agc->setExposureMode(ExposureModeTable.at(idx));
 				libcameraMetadata_.set(controls::AeExposureMode, idx);
 			} else {
 				LOG(IPARPI, Error) << "Exposure mode " << idx
@@ -836,12 +836,12 @@  void IpaBase::applyControls(const ControlList &controls)
 
 			switch (mode) {
 			case controls::FlickerOff:
-				agc->setFlickerPeriod(0, 0us);
+				agc->setFlickerPeriod(0us);
 
 				break;
 
 			case controls::FlickerManual:
-				agc->setFlickerPeriod(0, flickerState_.manualPeriod);
+				agc->setFlickerPeriod(flickerState_.manualPeriod);
 
 				break;
 
@@ -875,7 +875,7 @@  void IpaBase::applyControls(const ControlList &controls)
 			 * first, and the period updated after, or vice versa.
 			 */
 			if (flickerState_.mode == controls::FlickerManual)
-				agc->setFlickerPeriod(0, flickerState_.manualPeriod);
+				agc->setFlickerPeriod(flickerState_.manualPeriod);
 
 			break;
 		}
diff --git a/src/ipa/rpi/controller/agc_algorithm.h b/src/ipa/rpi/controller/agc_algorithm.h
index b8986560ac88..534e38e2b5b7 100644
--- a/src/ipa/rpi/controller/agc_algorithm.h
+++ b/src/ipa/rpi/controller/agc_algorithm.h
@@ -22,17 +22,16 @@  public:
 	virtual unsigned int getConvergenceFrames() const = 0;
 	virtual std::vector<double> const &getWeights() const = 0;
 	virtual void setEv(unsigned int channel, double ev) = 0;
-	virtual void setFlickerPeriod(unsigned int channel,
-				      libcamera::utils::Duration flickerPeriod) = 0;
+	virtual void setFlickerPeriod(libcamera::utils::Duration flickerPeriod) = 0;
 	virtual void setFixedShutter(unsigned int channel,
 				     libcamera::utils::Duration fixedShutter) = 0;
 	virtual void setMaxShutter(libcamera::utils::Duration maxShutter) = 0;
 	virtual void setFixedAnalogueGain(unsigned int channel, double fixedAnalogueGain) = 0;
 	virtual void setMeteringMode(std::string const &meteringModeName) = 0;
-	virtual void setExposureMode(unsigned int channel, std::string const &exposureModeName) = 0;
-	virtual void setConstraintMode(unsigned int channel, std::string const &contraintModeName) = 0;
-	virtual void enableAuto(unsigned int channel) = 0;
-	virtual void disableAuto(unsigned int channel) = 0;
+	virtual void setExposureMode(std::string const &exposureModeName) = 0;
+	virtual void setConstraintMode(std::string const &contraintModeName) = 0;
+	virtual void enableAuto() = 0;
+	virtual void disableAuto() = 0;
 	virtual void setActiveChannels(const std::vector<unsigned int> &activeChannels) = 0;
 };
 
diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp
index 32eb36242268..6549dedd0a07 100644
--- a/src/ipa/rpi/controller/rpi/agc.cpp
+++ b/src/ipa/rpi/controller/rpi/agc.cpp
@@ -74,22 +74,22 @@  int Agc::checkChannel(unsigned int channelIndex) const
 	return 0;
 }
 
-void Agc::disableAuto(unsigned int channelIndex)
+void Agc::disableAuto()
 {
-	if (checkChannel(channelIndex))
-		return;
+	LOG(RPiAgc, Debug) << "disableAuto";
 
-	LOG(RPiAgc, Debug) << "disableAuto for channel " << channelIndex;
-	channelData_[channelIndex].channel.disableAuto();
+	/* All channels are enabled/disabled together. */
+	for (auto &data : channelData_)
+		data.channel.disableAuto();
 }
 
-void Agc::enableAuto(unsigned int channelIndex)
+void Agc::enableAuto()
 {
-	if (checkChannel(channelIndex))
-		return;
+	LOG(RPiAgc, Debug) << "enableAuto";
 
-	LOG(RPiAgc, Debug) << "enableAuto for channel " << channelIndex;
-	channelData_[channelIndex].channel.enableAuto();
+	/* All channels are enabled/disabled together. */
+	for (auto &data : channelData_)
+		data.channel.enableAuto();
 }
 
 unsigned int Agc::getConvergenceFrames() const
@@ -118,14 +118,13 @@  void Agc::setEv(unsigned int channelIndex, double ev)
 	channelData_[channelIndex].channel.setEv(ev);
 }
 
-void Agc::setFlickerPeriod(unsigned int channelIndex, Duration flickerPeriod)
+void Agc::setFlickerPeriod(Duration flickerPeriod)
 {
-	if (checkChannel(channelIndex))
-		return;
+	LOG(RPiAgc, Debug) << "setFlickerPeriod " << flickerPeriod;
 
-	LOG(RPiAgc, Debug) << "setFlickerPeriod " << flickerPeriod
-			   << " for channel " << channelIndex;
-	channelData_[channelIndex].channel.setFlickerPeriod(flickerPeriod);
+	/* Flicker period will be the same across all channels. */
+	for (auto &data : channelData_)
+		data.channel.setFlickerPeriod(flickerPeriod);
 }
 
 void Agc::setMaxShutter(Duration maxShutter)
@@ -162,22 +161,22 @@  void Agc::setMeteringMode(std::string const &meteringModeName)
 		data.channel.setMeteringMode(meteringModeName);
 }
 
-void Agc::setExposureMode(unsigned int channelIndex, std::string const &exposureModeName)
+void Agc::setExposureMode(std::string const &exposureModeName)
 {
-	if (checkChannel(channelIndex))
-		return;
+	LOG(RPiAgc, Debug) << "setExposureMode " << exposureModeName;
 
-	LOG(RPiAgc, Debug) << "setExposureMode " << exposureModeName
-			   << " for channel " << channelIndex;
-	channelData_[channelIndex].channel.setExposureMode(exposureModeName);
+	/* Exposure mode will be the same across all channels. */
+	for (auto &data : channelData_)
+		data.channel.setExposureMode(exposureModeName);
 }
 
-void Agc::setConstraintMode(unsigned int channelIndex, std::string const &constraintModeName)
+void Agc::setConstraintMode(std::string const &constraintModeName)
 {
-	if (checkChannel(channelIndex))
-		return;
+	LOG(RPiAgc, Debug) << "setConstraintMode " << constraintModeName;
 
-	channelData_[channelIndex].channel.setConstraintMode(constraintModeName);
+	/* Constraint mode will be the same across all channels. */
+	for (auto &data : channelData_)
+		data.channel.setConstraintMode(constraintModeName);
 }
 
 template<typename T>
diff --git a/src/ipa/rpi/controller/rpi/agc.h b/src/ipa/rpi/controller/rpi/agc.h
index 908904393fbf..7d26bdf6df45 100644
--- a/src/ipa/rpi/controller/rpi/agc.h
+++ b/src/ipa/rpi/controller/rpi/agc.h
@@ -31,20 +31,17 @@  public:
 	unsigned int getConvergenceFrames() const override;
 	std::vector<double> const &getWeights() const override;
 	void setEv(unsigned int channel, double ev) override;
-	void setFlickerPeriod(unsigned int channelIndex,
-			      libcamera::utils::Duration flickerPeriod) override;
+	void setFlickerPeriod(libcamera::utils::Duration flickerPeriod) override;
 	void setMaxShutter(libcamera::utils::Duration maxShutter) override;
 	void setFixedShutter(unsigned int channelIndex,
 			     libcamera::utils::Duration fixedShutter) override;
 	void setFixedAnalogueGain(unsigned int channelIndex,
 				  double fixedAnalogueGain) override;
 	void setMeteringMode(std::string const &meteringModeName) override;
-	void setExposureMode(unsigned int channelIndex,
-			     std::string const &exposureModeName) override;
-	void setConstraintMode(unsigned int channelIndex,
-			       std::string const &contraintModeName) override;
-	void enableAuto(unsigned int channelIndex) override;
-	void disableAuto(unsigned int channelIndex) override;
+	void setExposureMode(std::string const &exposureModeName) override;
+	void setConstraintMode(std::string const &contraintModeName) override;
+	void enableAuto() override;
+	void disableAuto() override;
 	void switchMode(CameraMode const &cameraMode, Metadata *metadata) override;
 	void prepare(Metadata *imageMetadata) override;
 	void process(StatisticsPtr &stats, Metadata *imageMetadata) override;