| Message ID | 20221019090107.19975-3-naush@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting Naushir Patuck via libcamera-devel (2022-10-19 10:01:02) > 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> > --- > include/libcamera/internal/delayed_controls.h | 8 +++++--- > src/libcamera/delayed_controls.cpp | 20 ++++++++++++------- > src/libcamera/pipeline/ipu3/ipu3.cpp | 3 ++- > .../pipeline/raspberrypi/raspberrypi.cpp | 2 +- > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++- > test/delayed_controls.cpp | 8 ++++---- > 6 files changed, 27 insertions(+), 17 deletions(-) > > diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h > index de8026e3e4f0..c7e79150afb8 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 = 0); > > - bool push(const ControlList &controls); > - ControlList get(uint32_t sequence); > + bool push(const ControlList &controls, unsigned int cookie = 0); The part that worries me here is that we /require/ always getting the cookie, but not setting it... I would feel safer if this wasn't defaulted to 0... > + 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..3e7f60f9c421 100644 > --- a/src/libcamera/delayed_controls.cpp > +++ b/src/libcamera/delayed_controls.cpp > @@ -109,14 +109,16 @@ DelayedControls::DelayedControls(V4L2Device *device, > > /** > * \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..bf612089fdcb 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -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); Which means this cookie is always zero. Could you also update the push call to set the cookie? That looks like it happens at void IPU3CameraData::setSensorControls([[maybe_unused]] unsigned int id, const ControlList &sensorControls, const ControlList &lensControls) { delayedCtrls_->push(sensorControls); ... Where the 'id' is actually the request sequence number in the current implementation. So we can remove [[maybe_unused]] from unsigned int id, and pass it to push(). Ultimately, this then means that the cookie is the request sequence number of the request which is supposed to reflect these controls. Which is a nice way of tieing these together. > 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..23f2460190f4 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -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..20049d089472 100644 > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > @@ -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); Same for rkisp1: void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame, const ControlList &sensorControls) { delayedCtrls_->push(sensorControls); } ... 'frame' should be a suitable/expected cookie here. > 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..322c545998b2 100644 > --- a/test/delayed_controls.cpp > +++ b/test/delayed_controls.cpp And in here, testing locally - then the pushes here need a cookie - and 'i' seems suitable. With that, Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > @@ -96,7 +96,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>(); > if (brightness != value) { > cerr << "Failed single control without delay" > @@ -138,7 +138,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>(); > if (brightness != expected) { > cerr << "Failed single control with delay" > @@ -187,7 +187,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>(); > if (brightness != expected || contrast != expected + 1) { > @@ -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>(); > -- > 2.25.1 >
Hi Kieran, Thank you for the review! On Wed, 26 Oct 2022 at 17:38, Kieran Bingham < kieran.bingham@ideasonboard.com> wrote: > Quoting Naushir Patuck via libcamera-devel (2022-10-19 10:01:02) > > 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> > > --- > > include/libcamera/internal/delayed_controls.h | 8 +++++--- > > src/libcamera/delayed_controls.cpp | 20 ++++++++++++------- > > src/libcamera/pipeline/ipu3/ipu3.cpp | 3 ++- > > .../pipeline/raspberrypi/raspberrypi.cpp | 2 +- > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++- > > test/delayed_controls.cpp | 8 ++++---- > > 6 files changed, 27 insertions(+), 17 deletions(-) > > > > diff --git a/include/libcamera/internal/delayed_controls.h > b/include/libcamera/internal/delayed_controls.h > > index de8026e3e4f0..c7e79150afb8 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 = 0); > > > > - bool push(const ControlList &controls); > > - ControlList get(uint32_t sequence); > > + bool push(const ControlList &controls, unsigned int cookie = 0); > > The part that worries me here is that we /require/ always getting the > cookie, but not setting it... > > I would feel safer if this wasn't defaulted to 0... > Sure, I'll remove the default values from push() and reset(), and make the below suggested changes to the ipu3 and rkisp pipeline handlers. Regards, Naush > > > + 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..3e7f60f9c421 100644 > > --- a/src/libcamera/delayed_controls.cpp > > +++ b/src/libcamera/delayed_controls.cpp > > @@ -109,14 +109,16 @@ DelayedControls::DelayedControls(V4L2Device > *device, > > > > /** > > * \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..bf612089fdcb 100644 > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > @@ -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); > > Which means this cookie is always zero. > > Could you also update the push call to set the cookie? > > That looks like it happens at > > void IPU3CameraData::setSensorControls([[maybe_unused]] unsigned int id, > const ControlList &sensorControls, > const ControlList &lensControls) > { > delayedCtrls_->push(sensorControls); > ... > > Where the 'id' is actually the request sequence number in the current > implementation. > > So we can remove [[maybe_unused]] from unsigned int id, and pass it to > push(). > > Ultimately, this then means that the cookie is the request sequence > number of the request which is supposed to reflect these controls. Which > is a nice way of tieing these together. > > > > 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..23f2460190f4 100644 > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > @@ -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..20049d089472 100644 > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp > > @@ -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); > > Same for rkisp1: > > void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int > frame, > const ControlList &sensorControls) > { > delayedCtrls_->push(sensorControls); > } > > ... 'frame' should be a suitable/expected cookie here. > > > > 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..322c545998b2 100644 > > --- a/test/delayed_controls.cpp > > +++ b/test/delayed_controls.cpp > > And in here, testing locally - then the pushes here need a cookie - and > 'i' seems suitable. > > With that, > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > @@ -96,7 +96,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>(); > > if (brightness != value) { > > cerr << "Failed single control without > delay" > > @@ -138,7 +138,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>(); > > if (brightness != expected) { > > cerr << "Failed single control with > delay" > > @@ -187,7 +187,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>(); > > if (brightness != expected || contrast != > expected + 1) { > > @@ -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>(); > > -- > > 2.25.1 > > >
diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h index de8026e3e4f0..c7e79150afb8 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 = 0); - bool push(const ControlList &controls); - ControlList get(uint32_t sequence); + bool push(const ControlList &controls, unsigned int cookie = 0); + 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..3e7f60f9c421 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -109,14 +109,16 @@ DelayedControls::DelayedControls(V4L2Device *device, /** * \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..bf612089fdcb 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -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..23f2460190f4 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -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..20049d089472 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -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..322c545998b2 100644 --- a/test/delayed_controls.cpp +++ b/test/delayed_controls.cpp @@ -96,7 +96,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>(); if (brightness != value) { cerr << "Failed single control without delay" @@ -138,7 +138,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>(); if (brightness != expected) { cerr << "Failed single control with delay" @@ -187,7 +187,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>(); if (brightness != expected || contrast != expected + 1) { @@ -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>();