[libcamera-devel,v6,4/8] pipeline: raspberrypi: delayed_controls: Add user cookie to DelayedControls

17796 diff mbox series

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>
---
 .../pipeline/raspberrypi/delayed_controls.cpp      | 14 ++++++++------
 .../pipeline/raspberrypi/delayed_controls.h        |  8 +++++---
 src/libcamera/pipeline/raspberrypi/raspberrypi.cpp |  6 +++---
 3 files changed, 16 insertions(+), 12 deletions(-)

Hi Naush

Thanks for this patch.

On Tue, 15 Nov 2022 at 09:08, Naushir Patuck via libcamera-devel
<libcamera-devel@lists.libcamera.org> wrote:
>
> 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>

I guess the only thing I wonder very slightly, given that these
chances no longer affect the world at large, is whether we should call
"cookies" something more meaningful. But that's just me thinking out
loud, and I would honestly not suggest doing anything at this point!

Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

Thanks!
David

> ---
>  .../pipeline/raspberrypi/delayed_controls.cpp      | 14 ++++++++------
>  .../pipeline/raspberrypi/delayed_controls.h        |  8 +++++---
>  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp |  6 +++---
>  3 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/delayed_controls.cpp b/src/libcamera/pipeline/raspberrypi/delayed_controls.cpp
> index 867e3866cc46..3db92e7d24fb 100644
> --- a/src/libcamera/pipeline/raspberrypi/delayed_controls.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/delayed_controls.cpp
> @@ -108,7 +108,7 @@ DelayedControls::DelayedControls(V4L2Device *device,
>                 maxDelay_ = std::max(maxDelay_, controlParams_[id].delay);
>         }
>
> -       reset();
> +       reset(0);
>  }
>
>  /**
> @@ -117,10 +117,11 @@ DelayedControls::DelayedControls(V4L2Device *device,
>   * 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;
> @@ -150,7 +151,7 @@ void DelayedControls::reset()
>   *
>   * \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_) {
> @@ -184,6 +185,7 @@ bool DelayedControls::push(const ControlList &controls)
>                         << " at index " << queueCount_;
>         }
>
> +       cookies_[queueCount_] = cookie;
>         queueCount_++;
>
>         return true;
> @@ -204,7 +206,7 @@ bool DelayedControls::push(const ControlList &controls)
>   *
>   * \return The controls at \a sequence number
>   */
> -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_);
>
> @@ -221,7 +223,7 @@ ControlList DelayedControls::get(uint32_t sequence)
>                         << " at index " << index;
>         }
>
> -       return out;
> +       return { out, cookies_[index] };
>  }
>
>  /**
> @@ -280,7 +282,7 @@ void DelayedControls::applyControls(uint32_t sequence)
>         while (writeCount_ > queueCount_) {
>                 LOG(RPiDelayedControls, Debug)
>                         << "Queue is empty, auto queue no-op.";
> -               push({});
> +               push({}, cookies_[queueCount_ - 1]);
>         }
>
>         device_->setControls(&out);
> diff --git a/src/libcamera/pipeline/raspberrypi/delayed_controls.h b/src/libcamera/pipeline/raspberrypi/delayed_controls.h
> index 238b86ab6cb4..61f755f0fddd 100644
> --- a/src/libcamera/pipeline/raspberrypi/delayed_controls.h
> +++ b/src/libcamera/pipeline/raspberrypi/delayed_controls.h
> @@ -11,6 +11,7 @@
>
>  #include <stdint.h>
>  #include <unordered_map>
> +#include <utility>
>
>  #include <libcamera/controls.h>
>
> @@ -31,10 +32,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);
>
> @@ -78,6 +79,7 @@ private:
>         uint32_t queueCount_;
>         uint32_t writeCount_;
>         std::unordered_map<const ControlId *, RingBuffer<Info>> values_;
> +       RingBuffer<unsigned int> cookies_;
>  };
>
>  } /* namespace RPi */
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index f3be4ee3b730..2c12ed1fba95 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.
> --
> 2.25.1
>