5 files changed, 62 insertions, 27 deletions

diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
index 10958a7e..04062a36 100644
--- a/src/ipa/rkisp1/algorithms/agc.cpp
+++ b/src/ipa/rkisp1/algorithms/agc.cpp
@@ -144,17 +144,19 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue)
 /**
  * \brief Estimate the new exposure and gain values
  * \param[inout] context The shared IPA Context
+ * \param[in] frameContext The FrameContext for this frame
  * \param[in] yGain The gain calculated on the current brightness level
  * \param[in] iqMeanGain The gain calculated based on the relative luminance target
  */
-void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain)
+void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,
+			  double yGain, double iqMeanGain)
 {
 	IPASessionConfiguration &configuration = context.configuration;
 	IPAActiveState &activeState = context.activeState;
 
 	/* Get the effective exposure and gain applied on the sensor. */
-	uint32_t exposure = activeState.sensor.exposure;
-	double analogueGain = activeState.sensor.gain;
+	uint32_t exposure = frameContext.sensor.exposure;
+	double analogueGain = frameContext.sensor.gain;
 
 	/* Use the highest of the two gain estimates. */
 	double evGain = std::max(yGain, iqMeanGain);
@@ -286,9 +288,16 @@ double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const
  * new exposure and gain for the scene.
  */
 void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
-		  [[maybe_unused]] IPAFrameContext &frameContext,
-		  const rkisp1_stat_buffer *stats)
+		  IPAFrameContext &frameContext, const rkisp1_stat_buffer *stats)
 {
+	/*
+	 * \todo Verify that the exposure and gain applied by the sensor for
+	 * this frame match what has been requested. This isn't a hard
+	 * requirement for stability of the AGC (the guarantee we need in
+	 * automatic mode is a perfect match between the frame and the values
+	 * we receive), but is important in manual mode.
+	 */
+
 	const rkisp1_cif_isp_stat *params = &stats->params;
 	ASSERT(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP);
 
@@ -320,7 +329,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
 			break;
 	}
 
-	computeExposure(context, yGain, iqMeanGain);
+	computeExposure(context, frameContext, yGain, iqMeanGain);
 	frameCount_++;
 }
 
@@ -328,9 +337,11 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
  * \copydoc libcamera::ipa::Algorithm::prepare
  */
 void Agc::prepare(IPAContext &context, const uint32_t frame,
-		  [[maybe_unused]] IPAFrameContext &frameContext,
-		  rkisp1_params_cfg *params)
+		  IPAFrameContext &frameContext, rkisp1_params_cfg *params)
 {
+	frameContext.agc.exposure = context.activeState.agc.exposure;
+	frameContext.agc.gain = context.activeState.agc.gain;
+
 	if (frame > 0)
 		return;
 
diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h
index f115ba2e..9ad5c32f 100644
--- a/src/ipa/rkisp1/algorithms/agc.h
+++ b/src/ipa/rkisp1/algorithms/agc.h
@@ -34,7 +34,8 @@ public:
 		     const rkisp1_stat_buffer *stats) override;
 
 private:
-	void computeExposure(IPAContext &Context, double yGain, double iqMeanGain);
+	void computeExposure(IPAContext &Context, IPAFrameContext &frameContext,
+			     double yGain, double iqMeanGain);
 	utils::Duration filterExposure(utils::Duration exposureValue);
 	double estimateLuminance(const rkisp1_cif_isp_ae_stat *ae, double gain);
 	double measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const;
diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
index 78a785f5..c7d5b1b6 100644
--- a/src/ipa/rkisp1/ipa_context.cpp
+++ b/src/ipa/rkisp1/ipa_context.cpp
@@ -104,16 +104,13 @@ namespace libcamera::ipa::rkisp1 {
  * \var IPAActiveState::agc
  * \brief State for the Automatic Gain Control algorithm
  *
- * The exposure and gain determined are expected to be applied to the sensor
- * at the earliest opportunity.
+ * The exposure and gain are the latest values computed by the AGC algorithm.
  *
  * \var IPAActiveState::agc.exposure
  * \brief Exposure time expressed as a number of lines
  *
  * \var IPAActiveState::agc.gain
  * \brief Analogue gain multiplier
- *
- * The gain should be adapted to the sensor specific gain code before applying.
  */
 
 /**
@@ -182,21 +179,37 @@ namespace libcamera::ipa::rkisp1 {
  */
 
 /**
- * \var IPAActiveState::sensor
- * \brief Effective sensor values
+ * \struct IPAFrameContext
+ * \brief Per-frame context for algorithms
+ *
+ * \todo Populate the frame context for all algorithms
+ */
+
+/**
+ * \var IPAFrameContext::agc
+ * \brief Automatic Gain Control parameters for this frame
  *
- * \var IPAActiveState::sensor.exposure
+ * The exposure and gain are provided by the AGC algorithm, and are to be
+ * applied to the sensor in order to take effect for this frame.
+ *
+ * \var IPAFrameContext::agc.exposure
  * \brief Exposure time expressed as a number of lines
  *
- * \var IPAActiveState::sensor.gain
+ * \var IPAFrameContext::agc.gain
  * \brief Analogue gain multiplier
+ *
+ * The gain should be adapted to the sensor specific gain code before applying.
  */
 
 /**
- * \struct IPAFrameContext
- * \brief Per-frame context for algorithms
+ * \var IPAFrameContext::sensor
+ * \brief Sensor configuration that used been used for this frame
  *
- * \todo Populate the frame context for all algorithms
+ * \var IPAFrameContext::sensor.exposure
+ * \brief Exposure time expressed as a number of lines
+ *
+ * \var IPAFrameContext::sensor.gain
+ * \brief Analogue gain multiplier
  */
 
 /**
diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
index 1390f803..3fbd6b18 100644
--- a/src/ipa/rkisp1/ipa_context.h
+++ b/src/ipa/rkisp1/ipa_context.h
@@ -82,14 +82,18 @@ struct IPAActiveState {
 		uint8_t sharpness;
 		bool updateParams;
 	} filter;
+};
 
+struct IPAFrameContext : public FrameContext {
 	struct {
 		uint32_t exposure;
 		double gain;
-	} sensor;
-};
+	} agc;
 
-struct IPAFrameContext : public FrameContext {
+	struct {
+		uint32_t exposure;
+		double gain;
+	} sensor;
 };
 
 struct IPAContext {
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index 11fd8606..32feb168 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -333,9 +333,9 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId
 		reinterpret_cast<rkisp1_stat_buffer *>(
 			mappedBuffers_.at(bufferId).planes()[0].data());
 
-	context_.activeState.sensor.exposure =
+	frameContext.sensor.exposure =
 		sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
-	context_.activeState.sensor.gain =
+	frameContext.sensor.gain =
 		camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
 
 	unsigned int aeState = 0;
@@ -350,8 +350,14 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId
 
 void IPARkISP1::setControls(unsigned int frame)
 {
-	uint32_t exposure = context_.activeState.agc.exposure;
-	uint32_t gain = camHelper_->gainCode(context_.activeState.agc.gain);
+	/*
+	 * \todo The frame number is most likely wrong here, we need to take
+	 * internal sensor delays and other timing parameters into account.
+	 */
+
+	IPAFrameContext &frameContext = context_.frameContexts.get(frame);
+	uint32_t exposure = frameContext.agc.exposure;
+	uint32_t gain = camHelper_->gainCode(frameContext.agc.gain);
 
 	ControlList ctrls(ctrls_);
 	ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure));