[libcamera-devel] [PATCH v2 1/8] libcamera: delayed_controls: Add helper for controls that applies with a delay
Niklas Söderlund
niklas.soderlund at ragnatech.se
Wed Nov 25 20:58:03 CET 2020
Hi Naushir,
Thanks for your feedback.
On 2020-11-25 17:54:53 +0000, Naushir Patuck wrote:
> Hi Niklas,
>
> On Tue, 10 Nov 2020 at 00:27, Niklas Söderlund <
> niklas.soderlund at ragnatech.se> wrote:
>
> > Some sensor controls take effect with a delay as the sensor needs time
> > to adjust, for example exposure. Add a optional helper DelayedControls
> > to help pipelines deal with such controls.
> >
> > The idea is to provide a queue of controls towards the V4L2 device and
> > apply individual controls with the specified delay with the aim to get
> > predictable and retrievable control values for any given frame. To do
> > this the queue of controls needs to be at least as deep as the control
> > with the largest delay.
> >
> > The DelayedControls needs to be informed of every start of exposure.
> > This can be emulated but the helper is designed to be used with this
> > event being provide by the kernel thru V4L2 events.
> >
> > This helper is based on StaggeredCtrl from the Raspberry Pi pipeline
> > handler but expands on its API. This helpers aims to replace the
> > Raspberry Pi implementations and mimics it behavior perfectly.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> > * Changes since v1
> > - Correct copyright to reflect work is derived from Raspberry Pi
> > pipeline handler. This was always the intention and was wrong in v1.
> > - Rewrite large parts of the documentation.
> > - Join two loops to one in DelayedControls::DelayedControls()
> > ---
> > include/libcamera/internal/delayed_controls.h | 87 ++++++
> > src/libcamera/delayed_controls.cpp | 260 ++++++++++++++++++
> > src/libcamera/meson.build | 1 +
> > 3 files changed, 348 insertions(+)
> > create mode 100644 include/libcamera/internal/delayed_controls.h
> > create mode 100644 src/libcamera/delayed_controls.cpp
> >
> > diff --git a/include/libcamera/internal/delayed_controls.h
> > b/include/libcamera/internal/delayed_controls.h
> > new file mode 100644
> > index 0000000000000000..52bfba876e00e081
> > --- /dev/null
> > +++ b/include/libcamera/internal/delayed_controls.h
> > @@ -0,0 +1,87 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2020, Raspberry Pi (Trading) Ltd.
> > + *
> > + * delayed_controls.h - Helper to deal with controls that are applied
> > with a delay
> > + */
> > +#ifndef __LIBCAMERA_INTERNAL_DELAYED_CONTROLS_H__
> > +#define __LIBCAMERA_INTERNAL_DELAYED_CONTROLS_H__
> > +
> > +#include <mutex>
> > +#include <stdint.h>
> > +#include <unordered_map>
> > +
> > +#include <libcamera/controls.h>
> > +
> > +namespace libcamera {
> > +
> > +class V4L2Device;
> > +
> > +class DelayedControls
> > +{
> > +public:
> > + DelayedControls(V4L2Device *device,
> > + const std::unordered_map<uint32_t, unsigned int>
> > &delays);
> > +
> > + void reset(ControlList *controls = nullptr);
> > +
> > + bool push(const ControlList &controls);
> > + ControlList get(uint32_t sequence);
> > +
> > + void frameStart(uint32_t sequence);
> > +
> > +private:
> > + class ControlInfo
> > + {
> > + public:
> > + ControlInfo()
> > + : updated(false)
> > + {
> > + }
> > +
> > + ControlInfo(const ControlValue &v)
> > + : value(v), updated(true)
> > + {
> > + }
> > +
> > + ControlValue value;
> > + bool updated;
> > + };
> > +
> > + static constexpr int listSize = 16;
> > + class ControlArray : public std::array<ControlInfo, listSize>
> > + {
> > + public:
> > + ControlInfo &operator[](unsigned int index)
> > + {
> > + return std::array<ControlInfo,
> > listSize>::operator[](index % listSize);
> > + }
> > +
> > + const ControlInfo &operator[](unsigned int index) const
> > + {
> > + return std::array<ControlInfo,
> > listSize>::operator[](index % listSize);
> > + }
> > + };
> > +
> > + using ControlsDelays = std::unordered_map<const ControlId *,
> > unsigned int>;
> > + using ControlsValues = std::unordered_map<const ControlId *,
> > ControlArray>;
> > +
> > + bool queue(const ControlList &controls);
> > +
> > + std::mutex lock_;
> > +
> > + V4L2Device *device_;
> > + ControlsDelays delays_;
> > + unsigned int maxDelay_;
> > +
> > + bool running_;
> > + uint32_t fistSequence_;
> > +
> > + uint32_t queueCount_;
> > + uint32_t writeCount_;
> > + ControlsValues ctrls_;
> > +};
> > +
> > +} /* namespace libcamera */
> > +
> > +#endif /* __LIBCAMERA_INTERNAL_DELAYED_CONTROLS_H__ */
> > diff --git a/src/libcamera/delayed_controls.cpp
> > b/src/libcamera/delayed_controls.cpp
> > new file mode 100644
> > index 0000000000000000..f1c5f890957ec040
> > --- /dev/null
> > +++ b/src/libcamera/delayed_controls.cpp
> > @@ -0,0 +1,260 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2020, Raspberry Pi (Trading) Ltd.
> > + *
> > + * delayed_controls.h - Helper to deal with controls that are applied
> > with a delay
> > + */
> > +
> > +#include "libcamera/internal/delayed_controls.h"
> > +#include "libcamera/internal/v4l2_device.h"
> > +
> > +#include <libcamera/controls.h>
> > +
> > +#include "libcamera/internal/log.h"
> > +
> > +/**
> > + * \file delayed_controls.h
> > + * \brief Helper to deal with controls that are applied with a delay
> > + */
> > +
> > +namespace libcamera {
> > +
> > +LOG_DEFINE_CATEGORY(DelayedControls)
> > +
> > +/**
> > + * \class DelayedControls
> > + * \brief Helper to deal with controls that take effect with a delay
> > + *
> > + * Some sensor controls take effect with a delay as the sensor needs time
> > to
> > + * adjust, for example exposure and focus. This is an optional helper
> > class to
> > + * deal with such controls and the intended users are pipeline handlers.
> > + *
> > + * The idea is to extend the concept of the buffer depth of a pipeline the
> > + * application need to maintain to also cover controls. Just as with
> > buffer
> > + * depth if the application keeps the number of requests queued above the
> > + * control depth the controls are guaranteed to take effect for the
> > correct
> > + * request. The control depth is determined by the control with the grates
> > + * delay.
> > + */
> > +
> > +/**
> > + * \brief Construct a DelayedControls
> > + * \param[in] device The V4L2 device the controls have to be applied to
> > + * \param[in] delays Map of the numerical V4L2 control ids to their
> > associated
> > + * delays (in frames)
> > + *
> > + * Only controls specified in \a delays are handled. If it's desired to
> > mix
> > + * delayed controls and controls that take effect immediately the
> > immediate
> > + * controls must be listed in the \a delays map with a delay value of 0.
> > + */
> > +DelayedControls::DelayedControls(V4L2Device *device,
> > + const std::unordered_map<uint32_t,
> > unsigned int> &delays)
> > + : device_(device), maxDelay_(0)
> > +{
> > + const ControlInfoMap &controls = device_->controls();
> > +
> > + /*
> > + * Create a map of control ids to delays for controls exposed by
> > the
> > + * device.
> > + */
> > + for (auto const &delay : delays) {
> > + auto itControl = controls.find(delay.first);
> > + if (itControl == controls.end()) {
> > + LOG(DelayedControls, Error)
> > + << "Delay request for control id "
> > + << utils::hex(delay.first)
> > + << " but control is not exposed by device "
> > + << device_->deviceNode();
> > + continue;
> > + }
> > +
> > + const ControlId *id = itControl->first;
> > +
> > + delays_[id] = delay.second;
> > +
> > + LOG(DelayedControls, Debug)
> > + << "Set a delay of " << delays_[id]
> > + << " for " << id->name();
> > +
> > + maxDelay_ = std::max(maxDelay_, delays_[id]);
> > + }
> > +
> > + reset();
> > +}
> > +
> > +/**
> > + * \brief Reset state machine and controls
> > + * \param[in] controls Optional list of controls to apply to the device
> > + *
> > + * Resets the state machine to a starting position based on control values
> > + * retrieved from the device. Controls may optionally be set before they
> > are
> > + * read back using \a controls.
> > + */
> > +void DelayedControls::reset(ControlList *controls)
> > +{
> > + std::lock_guard<std::mutex> lock(lock_);
> > +
> > + running_ = false;
> > + fistSequence_ = 0;
> > + queueCount_ = 0;
> > + writeCount_ = 0;
> > +
> > + /* Set the controls on the device if requested. */
> > + if (controls)
> > + device_->setControls(controls);
> > +
> > + /* Retrieve current control values reported by the device. */
> > + std::vector<uint32_t> ids;
> > + for (auto const &delay : delays_)
> > + ids.push_back(delay.first->id());
> > +
> > + ControlList devCtrls = device_->getControls(ids);
> > +
> > + /* Seed the control queue with the controls reported by the
> > device. */
> > + ctrls_.clear();
> > + for (const auto &ctrl : devCtrls) {
> > + const ControlId *id =
> > devCtrls.infoMap()->idmap().at(ctrl.first);
> > + ctrls_[id][queueCount_] = ControlInfo(ctrl.second);
> > + }
> > +
> > + queueCount_++;
> > +}
> > +
> > +/**
> > + * \brief Push a set of controls on the queue
> > + * \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.
> > + *
> > + * \returns true if \a controls are accepted, or false otherwise
> > + */
> > +bool DelayedControls::push(const ControlList &controls)
> > +{
> > + std::lock_guard<std::mutex> lock(lock_);
> > +
> > + return queue(controls);
> > +}
> > +
> > +bool DelayedControls::queue(const ControlList &controls)
> > +{
> > + /* Copy state from previous frame. */
> > + for (auto &ctrl : ctrls_) {
> > + ControlInfo &info = ctrl.second[queueCount_];
> > + info.value = ctrls_[ctrl.first][queueCount_ - 1].value;
> > + info.updated = false;
> > + }
> > +
> > + /* Update with new controls. */
> > + for (const auto &control : controls) {
> > + const ControlId *id =
> > device_->controls().idmap().at(control.first);
> > +
> > + if (delays_.find(id) == delays_.end())
> > + return false;
> > +
> > + ControlInfo &info = ctrls_[id][queueCount_];
> > +
> > + info.value = control.second;
> > + info.updated = true;
> > +
> > + LOG(DelayedControls, Debug)
> > + << "Queuing " << id->name()
> > + << " to " << info.value.toString()
> > + << " at index " << queueCount_;
> > + }
> > +
> > + queueCount_++;
> > +
> > + return true;
> > +}
> > +
> > +/**
> > + * \brief Read back controls in effect at a sequence number
> > + * \param[in] sequence The sequence number to get controls for
> > + *
> > + * Read back what controls where in effect at a specific sequence number.
> > The
> > + * history is a ring buffer of 16 entries where new and old values
> > coexist. It's
> > + * the callers responsibility to not read to old sequence numbers that
> > have been
> > + * pushed out of the history.
> > + *
> > + * Historic values are evicted by pushing new values onto the queue using
> > + * 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
> > + */
> > +ControlList DelayedControls::get(uint32_t sequence)
> > +{
> > + std::lock_guard<std::mutex> lock(lock_);
> > +
> > + uint32_t adjustedSeq = sequence - fistSequence_ + 1;
> > + unsigned int index = std::max<int>(0, adjustedSeq - maxDelay_);
> > +
> > + ControlList out(device_->controls());
> > + for (const auto &ctrl : ctrls_) {
> > + const ControlId *id = ctrl.first;
> > + const ControlInfo &info = ctrl.second[index];
> > +
> > + out.set(id->id(), info.value);
> > +
> > + LOG(DelayedControls, Debug)
> > + << "Reading " << id->name()
> > + << " to " << info.value.toString()
> > + << " at index " << index;
> > + }
> > +
> > + return out;
> > +}
> > +
> > +/**
> > + * \brief Inform DelayedControls of the start of a new frame
> > + * \param[in] sequence Sequence number of the frame that started
> > + *
> > + * Inform the state machine that a new frame has started and of its
> > sequence
> > + * number. Any user of these helpers is responsible to inform the helper
> > about
> > + * the start of any frame.This can be connected with ease to the start of
> > a
> > + * exposure (SOE) V4L2 event.
> > + */
> > +void DelayedControls::frameStart(uint32_t sequence)
> > +{
> > + LOG(DelayedControls, Debug) << "frame " << sequence << " started";
> > +
> > + std::lock_guard<std::mutex> lock(lock_);
> > +
> > + if (!running_) {
> > + fistSequence_ = sequence;
> > + running_ = true;
> > + }
> > +
> > + /*
> > + * Create control list peaking ahead in the value queue to ensure
> > + * values are set in time to satisfy the sensor delay.
> > + */
> > + ControlList out(device_->controls());
> > + for (const auto &ctrl : ctrls_) {
> > + const ControlId *id = ctrl.first;
> > + unsigned int delayDiff = maxDelay_ - delays_[id];
> > + unsigned int index = std::max<int>(0, writeCount_ -
> > delayDiff);
> > + const ControlInfo &info = ctrl.second[index];
> > +
> > + if (info.updated) {
> > + out.set(id->id(), info.value);
> > + LOG(DelayedControls, Debug)
> > + << "Setting " << id->name()
> > + << " to " << info.value.toString()
> > + << " at index " << index;
> > + }
> > + }
> > +
> > + writeCount_++;
> > +
> > + while (writeCount_ >= queueCount_) {
> > + LOG(DelayedControls, Debug)
> > + << "Queue is empty, auto queue no-op.";
> > + queue({});
> > + }
> > +
> > + device_->setControls(&out);
> > +}
> >
>
> There is one thing I've been meaning to do in the StaggeredWriter for
> framerate / exposure control that will be superseded with this work. I
> wonder if you could add that functionality here?
> The issue is that some controls may need to be set before others. For
> example, VBLANK must be set separately before EXPOSURE. If this does not
> happen, the EXPOSURE value may be artificially constrained by the old
> VBLANK limits, and the device driver may erroneously reject the control.
>
> To work around this, I was imagining that we have an "immediate update"
> flag associated with a control. If that flag is set,
> DelayedControls::frameStart() will set that control straight away, i.e.
> without passing a list of controls to the device. All other controls
> without that flag set will work as above, i.e. get added to the
> ControlList::out map and set at the end of the function. With this
> implementation, we can guarantee VBLANK is set separately and before
> EXPOSURE, hence the new EXPOSURE control will be correctly validated by the
> limits of the new VBLANK.
>
> Hope that makes sense?
I think the idea make sens but I think it should be added as part of a
series that makes use of if so it can be evaluated in a working context.
For now I would like to focus on getting this series merged as it's been
floating around the ML for far too long IMHO and then we can build new
features on top.
>
> Regards,
> Naush
>
>
>
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > index 387d5d88ecae11ad..5a4bf0d7ba4fd231 100644
> > --- a/src/libcamera/meson.build
> > +++ b/src/libcamera/meson.build
> > @@ -12,6 +12,7 @@ libcamera_sources = files([
> > 'controls.cpp',
> > 'control_serializer.cpp',
> > 'control_validator.cpp',
> > + 'delayed_controls.cpp',
> > 'device_enumerator.cpp',
> > 'device_enumerator_sysfs.cpp',
> > 'event_dispatcher.cpp',
> > --
> > 2.29.2
> >
> >
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list