[v1,01/35] libcamera: delayed_controls: Add push() function that accepts a sequence number
diff mbox series

Message ID 176907326823.3882822.2987303171422721170@neptunite.rasen.tech
State New
Headers show
Series
  • [v1,01/35] libcamera: delayed_controls: Add push() function that accepts a sequence number
Related show

Commit Message

Stefan Klug Jan. 22, 2026, 9:14 a.m. UTC 
The push function is asymmetric 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 parameter 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>

Looks good to me.

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

---
 include/libcamera/internal/delayed_controls.h |  1 +
 src/libcamera/delayed_controls.cpp            | 32 ++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)

Patch
diff mbox series 

diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h
index b64d8bba7cf7..c650e672d964 100644
--- a/include/libcamera/internal/delayed_controls.h
+++ b/include/libcamera/internal/delayed_controls.h
@@ -32,6 +32,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..246e7cefa4e5 100644
--- a/src/libcamera/delayed_controls.cpp
+++ b/src/libcamera/delayed_controls.cpp
@@ -144,10 +144,40 @@  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)
 {
+       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 +306,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);