[RFC,1/7] libcamera: delayed_controls: Add push() function that accepts a sequence number
diff mbox series

Message ID 20241220162724.756494-2-stefan.klug@ideasonboard.com
State New
Headers show
Series
  • Improve regulation on rkisp1
Related show

Commit Message

Stefan Klug Dec. 20, 2024, 4:26 p.m. UTC 
The push function is assymetric to the get() and applyControls()
function in that it doesn't allow to specify a frame number. This leads
to the unfortunate situation that it is very difficult to detect if
anything goes out of sync. Add a version of the push() function that
takes a sequence paramater and warns when the sequence provided differs
from the expected sequence. Don't take any further actions for now to
see where issues pop up.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 include/libcamera/internal/delayed_controls.h |  1 +
 src/libcamera/delayed_controls.cpp            | 33 +++++++++++++++++--
 2 files changed, 32 insertions(+), 2 deletions(-)

Patch
diff mbox series 

diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h
index e8d3014d92cb..f1467fd79814 100644
--- a/include/libcamera/internal/delayed_controls.h
+++ b/include/libcamera/internal/delayed_controls.h
@@ -30,6 +30,7 @@  public:
 	void reset();
 
 	bool push(const ControlList &controls);
+	bool push(uint32_t sequence, const ControlList &controls);
 	ControlList get(uint32_t sequence);
 
 	void applyControls(uint32_t sequence);
diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp
index 94d0a575b01b..6c82d5058ae8 100644
--- a/src/libcamera/delayed_controls.cpp
+++ b/src/libcamera/delayed_controls.cpp
@@ -144,10 +144,39 @@  void DelayedControls::reset()
  * Push a set of controls to the control queue. This increases the control queue
  * depth by one.
  *
+ * \note The usage of this function is discouraged as it does not provide a way
+ * to detect double pushes for the same sequence. Better
+ * DelayedControls::push(uint32_t sequence, const ControlList &controls)
+ * instead.
+ *
  * \returns true if \a controls are accepted, or false otherwise
  */
-bool DelayedControls::push(const ControlList &controls)
+bool DelayedControls::push(const ControlList &controls) {
+	LOG(DelayedControls, Debug) << "Deprecated: Push without sequence number";
+	return push(queueCount_, controls);
+}
+
+/**
+ * \brief Push a set of controls on the queue
+ * \param[in] sequence The sequence number to push for
+ * \param[in] controls List of controls to add to the device queue
+ *
+ * Push a set of controls to the control queue. This increases the control queue
+ * depth by one.
+ *
+ * The \a sequence number is used to do some sanity checks to detect double
+ * pushes for the same sequence (either due to a bug or a request underrun).
+ *
+ * \returns true if \a controls are accepted, or false otherwise
+ */
+bool DelayedControls::push(uint32_t sequence, const ControlList &controls)
 {
+	if(sequence != queueCount_) {
+		LOG(DelayedControls, Warning)
+			<< "Double push for sequence " << sequence
+			<< " current queue index: " << queueCount_;
+	}
+
 	/* Copy state from previous frame. */
 	for (auto &ctrl : values_) {
 		Info &info = ctrl.second[queueCount_];
@@ -276,7 +305,7 @@  void DelayedControls::applyControls(uint32_t sequence)
 	while (writeCount_ > queueCount_) {
 		LOG(DelayedControls, Debug)
 			<< "Queue is empty, auto queue no-op.";
-		push({});
+		push(queueCount_, {});
 	}
 
 	device_->setControls(&out);