| Message ID | 20260327144726.7983-2-david.plowman@raspberrypi.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi David, Thank you for the update. On Fri, 27 Mar 2026 at 14:47, David Plowman <david.plowman@raspberrypi.com> wrote: > > The queueCount field in the DelayedControls is actually > redundant. Remove it. > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> Reviewed-by: Naushir Patuck <naush@raspberrypi.com> > --- > .../pipeline/rpi/common/delayed_controls.cpp | 32 +++++++++++-------- > .../pipeline/rpi/common/delayed_controls.h | 1 - > 2 files changed, 18 insertions(+), 15 deletions(-) > > diff --git a/src/libcamera/pipeline/rpi/common/delayed_controls.cpp b/src/libcamera/pipeline/rpi/common/delayed_controls.cpp > index 19c71946..93145d30 100644 > --- a/src/libcamera/pipeline/rpi/common/delayed_controls.cpp > +++ b/src/libcamera/pipeline/rpi/common/delayed_controls.cpp > @@ -119,7 +119,6 @@ DelayedControls::DelayedControls(V4L2Device *device, > */ > void DelayedControls::reset(unsigned int cookie) > { > - queueCount_ = 1; > writeCount_ = 0; > cookies_[0] = cookie; > > @@ -154,10 +153,16 @@ void DelayedControls::reset(unsigned int cookie) > bool DelayedControls::push(const ControlList &controls, const unsigned int cookie) > { > /* Copy state from previous frame. */ > - for (auto &ctrl : values_) { > - Info &info = ctrl.second[queueCount_]; > - info = values_[ctrl.first][queueCount_ - 1]; > - info.updated = false; > + if (writeCount_ > 0) { > + for (auto &ctrl : values_) { > + Info &info = ctrl.second[writeCount_]; > + info = values_[ctrl.first][writeCount_ - 1]; > + info.updated = false; > + } > + } else { > + /* Although it works, we don't expect this. */ > + LOG(RPiDelayedControls, Warning) > + << "push called before applyControls"; > } > > /* Update with new controls. */ > @@ -175,18 +180,17 @@ bool DelayedControls::push(const ControlList &controls, const unsigned int cooki > if (controlParams_.find(id) == controlParams_.end()) > return false; > > - Info &info = values_[id][queueCount_]; > + Info &info = values_[id][writeCount_]; > > info = Info(control.second); > > LOG(RPiDelayedControls, Debug) > << "Queuing " << id->name() > << " to " << info.toString() > - << " at index " << queueCount_; > + << " at index " << writeCount_; > } > > - cookies_[queueCount_] = cookie; > - queueCount_++; > + cookies_[writeCount_] = cookie; > > return true; > } > @@ -277,12 +281,12 @@ void DelayedControls::applyControls(uint32_t sequence) > } > } > > - writeCount_ = sequence + 1; > - > - while (writeCount_ > queueCount_) { > + while (writeCount_ < sequence + 1) { > + writeCount_++; > LOG(RPiDelayedControls, Debug) > - << "Queue is empty, auto queue no-op."; > - push({}, cookies_[queueCount_ - 1]); > + << "Pushing noop with for index " << writeCount_ > + << " with cookie " << cookies_[writeCount_ - 1]; > + push({}, cookies_[writeCount_ - 1]); > } > > device_->setControls(&out); > diff --git a/src/libcamera/pipeline/rpi/common/delayed_controls.h b/src/libcamera/pipeline/rpi/common/delayed_controls.h > index 487b0057..48ad5a0f 100644 > --- a/src/libcamera/pipeline/rpi/common/delayed_controls.h > +++ b/src/libcamera/pipeline/rpi/common/delayed_controls.h > @@ -76,7 +76,6 @@ private: > std::unordered_map<const ControlId *, ControlParams> controlParams_; > unsigned int maxDelay_; > > - uint32_t queueCount_; > uint32_t writeCount_; > std::unordered_map<const ControlId *, RingBuffer<Info>> values_; > RingBuffer<unsigned int> cookies_; > -- > 2.47.3 >
diff --git a/src/libcamera/pipeline/rpi/common/delayed_controls.cpp b/src/libcamera/pipeline/rpi/common/delayed_controls.cpp index 19c71946..93145d30 100644 --- a/src/libcamera/pipeline/rpi/common/delayed_controls.cpp +++ b/src/libcamera/pipeline/rpi/common/delayed_controls.cpp @@ -119,7 +119,6 @@ DelayedControls::DelayedControls(V4L2Device *device, */ void DelayedControls::reset(unsigned int cookie) { - queueCount_ = 1; writeCount_ = 0; cookies_[0] = cookie; @@ -154,10 +153,16 @@ void DelayedControls::reset(unsigned int cookie) bool DelayedControls::push(const ControlList &controls, const unsigned int cookie) { /* Copy state from previous frame. */ - for (auto &ctrl : values_) { - Info &info = ctrl.second[queueCount_]; - info = values_[ctrl.first][queueCount_ - 1]; - info.updated = false; + if (writeCount_ > 0) { + for (auto &ctrl : values_) { + Info &info = ctrl.second[writeCount_]; + info = values_[ctrl.first][writeCount_ - 1]; + info.updated = false; + } + } else { + /* Although it works, we don't expect this. */ + LOG(RPiDelayedControls, Warning) + << "push called before applyControls"; } /* Update with new controls. */ @@ -175,18 +180,17 @@ bool DelayedControls::push(const ControlList &controls, const unsigned int cooki if (controlParams_.find(id) == controlParams_.end()) return false; - Info &info = values_[id][queueCount_]; + Info &info = values_[id][writeCount_]; info = Info(control.second); LOG(RPiDelayedControls, Debug) << "Queuing " << id->name() << " to " << info.toString() - << " at index " << queueCount_; + << " at index " << writeCount_; } - cookies_[queueCount_] = cookie; - queueCount_++; + cookies_[writeCount_] = cookie; return true; } @@ -277,12 +281,12 @@ void DelayedControls::applyControls(uint32_t sequence) } } - writeCount_ = sequence + 1; - - while (writeCount_ > queueCount_) { + while (writeCount_ < sequence + 1) { + writeCount_++; LOG(RPiDelayedControls, Debug) - << "Queue is empty, auto queue no-op."; - push({}, cookies_[queueCount_ - 1]); + << "Pushing noop with for index " << writeCount_ + << " with cookie " << cookies_[writeCount_ - 1]; + push({}, cookies_[writeCount_ - 1]); } device_->setControls(&out); diff --git a/src/libcamera/pipeline/rpi/common/delayed_controls.h b/src/libcamera/pipeline/rpi/common/delayed_controls.h index 487b0057..48ad5a0f 100644 --- a/src/libcamera/pipeline/rpi/common/delayed_controls.h +++ b/src/libcamera/pipeline/rpi/common/delayed_controls.h @@ -76,7 +76,6 @@ private: std::unordered_map<const ControlId *, ControlParams> controlParams_; unsigned int maxDelay_; - uint32_t queueCount_; uint32_t writeCount_; std::unordered_map<const ControlId *, RingBuffer<Info>> values_; RingBuffer<unsigned int> cookies_;
The queueCount field in the DelayedControls is actually redundant. Remove it. Signed-off-by: David Plowman <david.plowman@raspberrypi.com> --- .../pipeline/rpi/common/delayed_controls.cpp | 32 +++++++++++-------- .../pipeline/rpi/common/delayed_controls.h | 1 - 2 files changed, 18 insertions(+), 15 deletions(-)