ipa: rpi: agc: Split AgcStatus into AgcStatus and AgcPrepareStatus

The Agc::process() function returns an AgcStatus object in the
metadata as before, but Agc::prepare() is changed to return the values
it computes in a separate AgcPrepareStatus object (under the new tag
"agc.prepare_status").

The "digitalGain" and "locked" fields are moved from AgcStatus to
AgcPrepareStatus.

This will be useful going forward as we can be more flexible about the
order in which prepare() and process() are called, without them
trampling on each other's results.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

src/ipa/rpi/common/ipa_base.cpp

@@ -1216,10 +1216,10 @@ void IpaBase::reportMetadata(unsigned int ipaContext)
 			libcameraMetadata_.set(controls::LensPosition, *deviceStatus->lensPosition);
 	}
 
-	AgcStatus *agcStatus = rpiMetadata.getLocked<AgcStatus>("agc.status");
+	AgcPrepareStatus *agcPrepareStatus = rpiMetadata.getLocked<AgcPrepareStatus>("agc.prepare_status");
-	if (agcStatus) {
+	if (agcPrepareStatus) {
-		libcameraMetadata_.set(controls::AeLocked, agcStatus->locked);
+		libcameraMetadata_.set(controls::AeLocked, agcPrepareStatus->locked);
-		libcameraMetadata_.set(controls::DigitalGain, agcStatus->digitalGain);
+		libcameraMetadata_.set(controls::DigitalGain, agcPrepareStatus->digitalGain);
 	}
 
 	LuxStatus *luxStatus = rpiMetadata.getLocked<LuxStatus>("lux.status");

src/ipa/rpi/controller/agc_status.h

@@ -11,8 +11,10 @@
 #include <libcamera/base/utils.h>
 
 /*
- * The AGC algorithm should post the following structure into the image's
+ * The AGC algorithm process method should post an AgcStatus into the image
- * "agc.status" metadata.
+ * metadata under the tag "agc.status".
+ * The AGC algorithm prepare method should post an AgcPrepareStatus instead
+ * under "agc.prepare_status".
  */
 
 /*
@@ -34,6 +36,9 @@ struct AgcStatus {
 	int floatingRegionEnable;
 	libcamera::utils::Duration fixedShutter;
 	double fixedAnalogueGain;
+};
+
+struct AgcPrepareStatus {
 	double digitalGain;
 	int locked;
 };

src/ipa/rpi/controller/rpi/agc.cpp

@@ -419,11 +419,13 @@ void Agc::prepare(Metadata *imageMetadata)
 {
 	Duration totalExposureValue = status_.totalExposureValue;
 	AgcStatus delayedStatus;
+	AgcPrepareStatus prepareStatus;
 
 	if (!imageMetadata->get("agc.delayed_status", delayedStatus))
 		totalExposureValue = delayedStatus.totalExposureValue;
 
-	status_.digitalGain = 1.0;
+	prepareStatus.digitalGain = 1.0;
+	prepareStatus.locked = false;
 
 	if (status_.totalExposureValue) {
 		/* Process has run, so we have meaningful values. */
@@ -432,23 +434,23 @@ void Agc::prepare(Metadata *imageMetadata)
 			Duration actualExposure = deviceStatus.shutterSpeed *
 						  deviceStatus.analogueGain;
 			if (actualExposure) {
-				status_.digitalGain = totalExposureValue / 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?
 				 */
-				status_.digitalGain = std::max(1.0, std::min(status_.digitalGain, 4.0));
+				prepareStatus.digitalGain = std::max(1.0, std::min(digitalGain, 4.0));
 				LOG(RPiAgc, Debug) << "Actual exposure " << actualExposure;
-				LOG(RPiAgc, Debug) << "Use digitalGain " << status_.digitalGain;
+				LOG(RPiAgc, Debug) << "Use digitalGain " << prepareStatus.digitalGain;
 				LOG(RPiAgc, Debug) << "Effective exposure "
-						   << actualExposure * status_.digitalGain;
+						   << actualExposure * prepareStatus.digitalGain;
 				/* Decide whether AEC/AGC has converged. */
-				updateLockStatus(deviceStatus);
+				prepareStatus.locked = updateLockStatus(deviceStatus);
 			}
 		} else
 			LOG(RPiAgc, Warning) << name() << ": no device metadata";
-		imageMetadata->set("agc.status", status_);
+		imageMetadata->set("agc.prepare_status", prepareStatus);
 	}
 }
 
@@ -486,7 +488,7 @@ void Agc::process(StatisticsPtr &stats, Metadata *imageMetadata)
 	writeAndFinish(imageMetadata, desaturate);
 }
 
-void Agc::updateLockStatus(DeviceStatus const &deviceStatus)
+bool Agc::updateLockStatus(DeviceStatus const &deviceStatus)
 {
 	const double errorFactor = 0.10; /* make these customisable? */
 	const int maxLockCount = 5;
@@ -522,7 +524,7 @@ void Agc::updateLockStatus(DeviceStatus const &deviceStatus)
 	lastTargetExposure_ = status_.targetExposureValue;
 
 	LOG(RPiAgc, Debug) << "Lock count updated to " << lockCount_;
-	status_.locked = lockCount_ == maxLockCount;
+	return lockCount_ == maxLockCount;
 }
 
 void Agc::housekeepConfig()

src/ipa/rpi/controller/rpi/agc.h

@@ -85,7 +85,7 @@ public:
 	void process(StatisticsPtr &stats, Metadata *imageMetadata) override;
 
 private:
-	void updateLockStatus(DeviceStatus const &deviceStatus);
+	bool updateLockStatus(DeviceStatus const &deviceStatus);
 	AgcConfig config_;
 	void housekeepConfig();
 	void fetchCurrentExposure(Metadata *imageMetadata);

src/ipa/rpi/vc4/vc4.cpp

@@ -60,7 +60,7 @@ private:
 	bool validateIspControls();
 
 	void applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls);
-	void applyDG(const struct AgcStatus *dgStatus, ControlList &ctrls);
+	void applyDG(const struct AgcPrepareStatus *dgStatus, 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);
@@ -142,7 +142,7 @@ void IpaVc4::platformPrepareIsp([[maybe_unused]] const PrepareParams &params,
 	if (ccmStatus)
 		applyCCM(ccmStatus, ctrls);
 
-	AgcStatus *dgStatus = rpiMetadata.getLocked<AgcStatus>("agc.status");
+	AgcPrepareStatus *dgStatus = rpiMetadata.getLocked<AgcPrepareStatus>("agc.prepare_status");
 	if (dgStatus)
 		applyDG(dgStatus, ctrls);
 
@@ -284,7 +284,7 @@ void IpaVc4::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)
 		  static_cast<int32_t>(awbStatus->gainB * 1000));
 }
 
-void IpaVc4::applyDG(const struct AgcStatus *dgStatus, ControlList &ctrls)
+void IpaVc4::applyDG(const struct AgcPrepareStatus *dgStatus, ControlList &ctrls)
 {
 	ctrls.set(V4L2_CID_DIGITAL_GAIN,
 		  static_cast<int32_t>(dgStatus->digitalGain * 1000));