[libcamera-devel,v5,2/7] delayed_controls: Add user cookie to DelayedControls
diff mbox series

Message ID 20221031114522.14215-3-naush@raspberrypi.com
State Superseded
Headers show
Series
  • Raspberry Pi AGC digital gain fixes
Related show

Commit Message

Naushir Patuck Oct. 31, 2022, 11:45 a.m. UTC 
Allow the caller to provide a cookie value to DelayedControls::reset and
DelayedControls::push. This cookie value is returned from DelayedControls::get
for the frame that has the control values applied to it.

The cookie value is useful in tracking when a set of controls has been applied
to a frame. In a subsequent commit, it will be used by the Raspberry Pi IPA to
track the IPA context used when setting the control values.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Tested-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 include/libcamera/internal/delayed_controls.h |  8 ++++---
 src/libcamera/delayed_controls.cpp            | 22 ++++++++++-------
 src/libcamera/pipeline/ipu3/ipu3.cpp          |  9 +++----
 .../pipeline/raspberrypi/raspberrypi.cpp      |  6 ++---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp      |  7 +++---
 test/delayed_controls.cpp                     | 24 +++++++++----------
 6 files changed, 43 insertions(+), 33 deletions(-)

Patch
diff mbox series 

diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h
index de8026e3e4f0..14470f3966ac 100644
--- a/include/libcamera/internal/delayed_controls.h
+++ b/include/libcamera/internal/delayed_controls.h
@@ -9,6 +9,7 @@ 
 
 #include <stdint.h>
 #include <unordered_map>
+#include <utility>
 
 #include <libcamera/controls.h>
 
@@ -27,10 +28,10 @@  public:
 	DelayedControls(V4L2Device *device,
 			const std::unordered_map<uint32_t, ControlParams> &controlParams);
 
-	void reset();
+	void reset(unsigned int cookie);
 
-	bool push(const ControlList &controls);
-	ControlList get(uint32_t sequence);
+	bool push(const ControlList &controls, unsigned int cookie);
+	std::pair<ControlList, unsigned int> get(uint32_t sequence);
 
 	void applyControls(uint32_t sequence);
 
@@ -77,6 +78,7 @@  private:
 	uint32_t writeCount_;
 	/* \todo Evaluate if we should index on ControlId * or unsigned int */
 	std::unordered_map<const ControlId *, RingBuffer<Info>> values_;
+	RingBuffer<unsigned int> cookies_;
 };
 
 } /* namespace libcamera */
diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp
index 777441e8222a..26243dc9e939 100644
--- a/src/libcamera/delayed_controls.cpp
+++ b/src/libcamera/delayed_controls.cpp
@@ -104,19 +104,21 @@  DelayedControls::DelayedControls(V4L2Device *device,
 		maxDelay_ = std::max(maxDelay_, controlParams_[id].delay);
 	}
 
-	reset();
+	reset(0);
 }
 
 /**
  * \brief Reset state machine
+ * \param[in] cookie User supplied reset cookie value
  *
  * Resets the state machine to a starting position based on control values
  * retrieved from the device.
  */
-void DelayedControls::reset()
+void DelayedControls::reset(unsigned int cookie)
 {
 	queueCount_ = 1;
 	writeCount_ = 0;
+	cookies_[0] = cookie;
 
 	/* Retrieve control as reported by the device. */
 	std::vector<uint32_t> ids;
@@ -140,13 +142,15 @@  void DelayedControls::reset()
 /**
  * \brief Push a set of controls on the queue
  * \param[in] controls List of controls to add to the device queue
+ * \param[in] cookie User supplied cookie value for \a controls
  *
  * Push a set of controls to the control queue. This increases the control queue
- * depth by one.
+ * depth by one. The \a cookie value will be subsequently returned from
+ * \a get() for the frame with all controls applied.
  *
  * \returns true if \a controls are accepted, or false otherwise
  */
-bool DelayedControls::push(const ControlList &controls)
+bool DelayedControls::push(const ControlList &controls, const unsigned int cookie)
 {
 	/* Copy state from previous frame. */
 	for (auto &ctrl : values_) {
@@ -180,6 +184,7 @@  bool DelayedControls::push(const ControlList &controls)
 			<< " at index " << queueCount_;
 	}
 
+	cookies_[queueCount_] = cookie;
 	queueCount_++;
 
 	return true;
@@ -198,9 +203,10 @@  bool DelayedControls::push(const ControlList &controls)
  * push(). The max history from the current sequence number that yields valid
  * values are thus 16 minus number of controls pushed.
  *
- * \return The controls at \a sequence number
+ * \return The controls at \a sequence number and associated user supplied
+ * cookie value.
  */
-ControlList DelayedControls::get(uint32_t sequence)
+std::pair<ControlList, unsigned int> DelayedControls::get(uint32_t sequence)
 {
 	unsigned int index = std::max<int>(0, sequence - maxDelay_);
 
@@ -217,7 +223,7 @@  ControlList DelayedControls::get(uint32_t sequence)
 			<< " at index " << index;
 	}
 
-	return out;
+	return { out, cookies_[index] };
 }
 
 /**
@@ -276,7 +282,7 @@  void DelayedControls::applyControls(uint32_t sequence)
 	while (writeCount_ > queueCount_) {
 		LOG(DelayedControls, Debug)
 			<< "Queue is empty, auto queue no-op.";
-		push({});
+		push({}, cookies_[queueCount_ - 1]);
 	}
 
 	device_->setControls(&out);
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 3b892d9671c5..8a5de76aa1b8 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -789,7 +789,7 @@  int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis
 	if (ret)
 		goto error;
 
-	data->delayedCtrls_->reset();
+	data->delayedCtrls_->reset(0);
 
 	/*
 	 * Start the ImgU video devices, buffers will be queued to the
@@ -1265,11 +1265,11 @@  int IPU3CameraData::loadIPA()
 	return 0;
 }
 
-void IPU3CameraData::setSensorControls([[maybe_unused]] unsigned int id,
+void IPU3CameraData::setSensorControls(unsigned int id,
 				       const ControlList &sensorControls,
 				       const ControlList &lensControls)
 {
-	delayedCtrls_->push(sensorControls);
+	delayedCtrls_->push(sensorControls, id);
 
 	CameraLens *focusLens = cio2_.sensor()->focusLens();
 	if (!focusLens)
@@ -1389,7 +1389,8 @@  void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
 	request->metadata().set(controls::SensorTimestamp,
 				buffer->metadata().timestamp);
 
-	info->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence);
+	auto [controls, cookie] = delayedCtrls_->get(buffer->metadata().sequence);
+	info->effectiveSensorControls = std::move(controls);
 
 	if (request->findBuffer(&rawStream_))
 		pipe()->completeBuffer(request, buffer);
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index 343f8cb2c7ed..29626c0ef14e 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -1064,7 +1064,7 @@  int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)
 	 * Reset the delayed controls with the gain and exposure values set by
 	 * the IPA.
 	 */
-	data->delayedCtrls_->reset();
+	data->delayedCtrls_->reset(0);
 
 	data->state_ = RPiCameraData::State::Idle;
 
@@ -1794,7 +1794,7 @@  void RPiCameraData::setIspControls(const ControlList &controls)
 
 void RPiCameraData::setDelayedControls(const ControlList &controls)
 {
-	if (!delayedCtrls_->push(controls))
+	if (!delayedCtrls_->push(controls, 0))
 		LOG(RPI, Error) << "V4L2 DelayedControl set failed";
 	handleState();
 }
@@ -1867,7 +1867,7 @@  void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)
 		 * Lookup the sensor controls used for this frame sequence from
 		 * DelayedControl and queue them along with the frame buffer.
 		 */
-		ControlList ctrl = delayedCtrls_->get(buffer->metadata().sequence);
+		auto [ctrl, cookie] = delayedCtrls_->get(buffer->metadata().sequence);
 		/*
 		 * Add the frame timestamp to the ControlList for the IPA to use
 		 * as it does not receive the FrameBuffer object.
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 455ee2a0a711..6f3c34e764fd 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -376,10 +376,10 @@  void RkISP1CameraData::paramFilled(unsigned int frame)
 		selfPath_->queueBuffer(info->selfPathBuffer);
 }
 
-void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame,
+void RkISP1CameraData::setSensorControls(unsigned int frame,
 					 const ControlList &sensorControls)
 {
-	delayedCtrls_->push(sensorControls);
+	delayedCtrls_->push(sensorControls, frame);
 }
 
 void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata)
@@ -1191,8 +1191,9 @@  void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
 	if (data->frame_ <= buffer->metadata().sequence)
 		data->frame_ = buffer->metadata().sequence + 1;
 
+	auto [controls, cookie] = data->delayedCtrls_->get(buffer->metadata().sequence);
 	data->ipa_->processStatsBuffer(info->frame, info->statBuffer->cookie(),
-				       data->delayedCtrls_->get(buffer->metadata().sequence));
+				       controls);
 }
 
 REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1)
diff --git a/test/delayed_controls.cpp b/test/delayed_controls.cpp
index a8ce9828d73d..26037268f245 100644
--- a/test/delayed_controls.cpp
+++ b/test/delayed_controls.cpp
@@ -82,7 +82,7 @@  protected:
 		/* Reset control to value not used in test. */
 		ctrls.set(V4L2_CID_BRIGHTNESS, 1);
 		dev_->setControls(&ctrls);
-		delayed->reset();
+		delayed->reset(0);
 
 		/* Trigger the first frame start event */
 		delayed->applyControls(0);
@@ -92,11 +92,11 @@  protected:
 			int32_t value = 100 + i;
 
 			ctrls.set(V4L2_CID_BRIGHTNESS, value);
-			delayed->push(ctrls);
+			delayed->push(ctrls, i);
 
 			delayed->applyControls(i);
 
-			ControlList result = delayed->get(i);
+			auto [result, cookie] = delayed->get(i);
 			int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
 			if (brightness != value) {
 				cerr << "Failed single control without delay"
@@ -124,7 +124,7 @@  protected:
 		int32_t expected = 4;
 		ctrls.set(V4L2_CID_BRIGHTNESS, expected);
 		dev_->setControls(&ctrls);
-		delayed->reset();
+		delayed->reset(0);
 
 		/* Trigger the first frame start event */
 		delayed->applyControls(0);
@@ -134,11 +134,11 @@  protected:
 			int32_t value = 10 + i;
 
 			ctrls.set(V4L2_CID_BRIGHTNESS, value);
-			delayed->push(ctrls);
+			delayed->push(ctrls, i);
 
 			delayed->applyControls(i);
 
-			ControlList result = delayed->get(i);
+			auto [result, cookie] = delayed->get(i);
 			int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
 			if (brightness != expected) {
 				cerr << "Failed single control with delay"
@@ -172,7 +172,7 @@  protected:
 		ctrls.set(V4L2_CID_BRIGHTNESS, expected);
 		ctrls.set(V4L2_CID_CONTRAST, expected + 1);
 		dev_->setControls(&ctrls);
-		delayed->reset();
+		delayed->reset(0);
 
 		/* Trigger the first frame start event */
 		delayed->applyControls(0);
@@ -183,11 +183,11 @@  protected:
 
 			ctrls.set(V4L2_CID_BRIGHTNESS, value);
 			ctrls.set(V4L2_CID_CONTRAST, value + 1);
-			delayed->push(ctrls);
+			delayed->push(ctrls, i);
 
 			delayed->applyControls(i);
 
-			ControlList result = delayed->get(i);
+			auto [result, cookie] = delayed->get(i);
 			int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
 			int32_t contrast = result.get(V4L2_CID_CONTRAST).get<int32_t>();
 			if (brightness != expected || contrast != expected + 1) {
@@ -223,7 +223,7 @@  protected:
 		ctrls.set(V4L2_CID_BRIGHTNESS, expected);
 		ctrls.set(V4L2_CID_CONTRAST, expected);
 		dev_->setControls(&ctrls);
-		delayed->reset();
+		delayed->reset(0);
 
 		/* Trigger the first frame start event */
 		delayed->applyControls(0);
@@ -238,7 +238,7 @@  protected:
 
 			ctrls.set(V4L2_CID_BRIGHTNESS, value);
 			ctrls.set(V4L2_CID_CONTRAST, value);
-			delayed->push(ctrls);
+			delayed->push(ctrls, i);
 		}
 
 		/* Process all queued controls. */
@@ -247,7 +247,7 @@  protected:
 
 			delayed->applyControls(i);
 
-			ControlList result = delayed->get(i);
+			auto [result, cookie] = delayed->get(i);
 
 			int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get<int32_t>();
 			int32_t contrast = result.get(V4L2_CID_CONTRAST).get<int32_t>();