[PATCH v3 08/16] libcamera: delayed_controls: Add ctrls list parameter to reset() function
Jacopo Mondi
jacopo.mondi at ideasonboard.com
Thu Mar 21 12:36:50 CET 2024
Hi Stefan
On Tue, Mar 19, 2024 at 01:05:09PM +0100, Stefan Klug wrote:
> This makes it easier for the caller. There is often this pattern
>
> sensor_->setControls(controls);
> delayedControls_->reset();
>
> which can then be reduced to
>
> delayedControls_->reset(controls);
>
Nit: I would then feel more confortable having the constructor accept
a CameraSensor &sensor and initialize device_ with sensor->device()
so that we guarantee the device_ is the same.
However, I wonder, to have controls applied immediately the sensor
should not be streaming and nothing enforces delayed controls to be
reset only when the sensor is not streaming.
I guess that's not different than what we have right now
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> ---
> include/libcamera/internal/delayed_controls.h | 2 +-
> src/libcamera/delayed_controls.cpp | 22 +++++++++++++++----
> 2 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h
> index ccbe7239..224c1f7e 100644
> --- a/include/libcamera/internal/delayed_controls.h
> +++ b/include/libcamera/internal/delayed_controls.h
> @@ -27,7 +27,7 @@ public:
> DelayedControls(V4L2Device *device,
> const std::unordered_map<uint32_t, ControlParams> &controlParams);
>
> - void reset();
> + void reset(ControlList *controls = nullptr);
>
> bool push(const ControlList &controls);
> ControlList get(uint32_t sequence);
> diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp
> index 6c766ede..6f06950e 100644
> --- a/src/libcamera/delayed_controls.cpp
> +++ b/src/libcamera/delayed_controls.cpp
> @@ -109,11 +109,13 @@ DelayedControls::DelayedControls(V4L2Device *device,
>
> /**
> * \brief Reset state machine
> + * \param[in,out] controls The controls to apply to the device
> *
> * Resets the state machine to a starting position based on control values
> - * retrieved from the device.
> + * retrieved from the device. If \a controls is given, these controls are set
> + * on the device before retrieving the reset values.
> */
> -void DelayedControls::reset()
> +void DelayedControls::reset(ControlList *controls)
> {
> queueIndex_ = 1;
> writeIndex_ = 0;
> @@ -123,11 +125,23 @@ void DelayedControls::reset()
> for (auto const ¶m : controlParams_)
> ids.push_back(param.first->id());
>
> - ControlList controls = device_->getControls(ids);
> + if (controls) {
> + device_->setControls(controls);
> +
> + LOG(DelayedControls, Debug) << "reset:";
> + auto idMap = controls->idMap();
> + if (idMap) {
> + for (const auto &[id, value] : *controls)
> + LOG(DelayedControls, Debug) << " " << idMap->at(id)->name()
> + << " : " << value.toString();
> + }
> + }
> +
> + ControlList ctrls = device_->getControls(ids);
>
> /* Seed the control queue with the controls reported by the device. */
> values_.clear();
> - for (const auto &ctrl : controls) {
> + for (const auto &ctrl : ctrls) {
> const ControlId *id = device_->controls().idmap().at(ctrl.first);
> /*
> * Do not mark this control value as updated, it does not need
> --
> 2.40.1
>
More information about the libcamera-devel
mailing list