From patchwork Wed Oct 28 01:00:43 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 10274 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id D8B5BC3B5C for ; Wed, 28 Oct 2020 01:01:12 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id B76B362244; Wed, 28 Oct 2020 02:01:12 +0100 (CET) Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D57B56220D for ; Wed, 28 Oct 2020 02:01:10 +0100 (CET) X-Halon-ID: 106f6308-18b9-11eb-954c-0050569116f7 Authorized-sender: niklas.soderlund@fsdn.se Received: from bismarck.berto.se (p4fca2458.dip0.t-ipconnect.de [79.202.36.88]) by bin-vsp-out-03.atm.binero.net (Halon) with ESMTPA id 106f6308-18b9-11eb-954c-0050569116f7; Wed, 28 Oct 2020 02:01:09 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org, david.plowman@raspberrypi.com, naush@raspberrypi.com Date: Wed, 28 Oct 2020 02:00:43 +0100 Message-Id: <20201028010051.3830668-2-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.29.1 In-Reply-To: <20201028010051.3830668-1-niklas.soderlund@ragnatech.se> References: <20201028010051.3830668-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 1/9] libcamera: v4l2_device: Move start of frame detection to V4L2Device X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The V4L2_EVENT_FRAME_SYNC event may occur on both V4L2 video-devices (V4L2VideoDevice) and sub-devices (V4L2Subdevice). Move the start of frame detection to the common base class of the two, V4L2Device. There is no functional change. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- * Changes since v1 - Mark eventAvailable() private instead of protected. - Remove a blank line. - Call setFd() instead of duplicating it. --- include/libcamera/internal/v4l2_device.h | 13 +++- include/libcamera/internal/v4l2_videodevice.h | 8 -- src/libcamera/v4l2_device.cpp | 76 ++++++++++++++++++- src/libcamera/v4l2_videodevice.cpp | 75 +----------------- 4 files changed, 87 insertions(+), 85 deletions(-) diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h index 722fb72207d74111..295f08f0be6f1980 100644 --- a/include/libcamera/internal/v4l2_device.h +++ b/include/libcamera/internal/v4l2_device.h @@ -13,11 +13,15 @@ #include +#include + #include "libcamera/internal/log.h" #include "libcamera/internal/v4l2_controls.h" namespace libcamera { +class EventNotifier; + class V4L2Device : protected Loggable { public: @@ -34,6 +38,9 @@ public: const std::string &deviceNode() const { return deviceNode_; } std::string devicePath() const; + int setFrameStartEnabled(bool enable); + Signal frameStart; + protected: V4L2Device(const std::string &deviceNode); ~V4L2Device(); @@ -44,18 +51,22 @@ protected: int ioctl(unsigned long request, void *argp); int fd() { return fd_; } - private: void listControls(); void updateControls(ControlList *ctrls, const struct v4l2_ext_control *v4l2Ctrls, unsigned int count); + void eventAvailable(EventNotifier *notifier); + std::map controlInfo_; std::vector> controlIds_; ControlInfoMap controls_; std::string deviceNode_; int fd_; + + EventNotifier *fdEventNotifier_; + bool frameStartEnabled_; }; } /* namespace libcamera */ diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h index 53f6a2d5515b9bb3..cf705ec9cd6ad118 100644 --- a/include/libcamera/internal/v4l2_videodevice.h +++ b/include/libcamera/internal/v4l2_videodevice.h @@ -204,9 +204,6 @@ public: int queueBuffer(FrameBuffer *buffer); Signal bufferReady; - int setFrameStartEnabled(bool enable); - Signal frameStart; - int streamOn(); int streamOff(); @@ -240,8 +237,6 @@ private: void bufferAvailable(EventNotifier *notifier); FrameBuffer *dequeueBuffer(); - void eventAvailable(EventNotifier *notifier); - V4L2Capability caps_; enum v4l2_buf_type bufferType_; @@ -251,9 +246,6 @@ private: std::map queuedBuffers_; EventNotifier *fdBufferNotifier_; - EventNotifier *fdEventNotifier_; - - bool frameStartEnabled_; }; class V4L2M2MDevice diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 31d4dad0ee47b734..36fe62b04003551e 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -16,6 +16,8 @@ #include #include +#include + #include "libcamera/internal/log.h" #include "libcamera/internal/sysfs.h" #include "libcamera/internal/utils.h" @@ -52,7 +54,8 @@ LOG_DEFINE_CATEGORY(V4L2) * at open() time, and the \a logTag to prefix log messages with. */ V4L2Device::V4L2Device(const std::string &deviceNode) - : deviceNode_(deviceNode), fd_(-1) + : deviceNode_(deviceNode), fd_(-1), fdEventNotifier_(nullptr), + frameStartEnabled_(false) { } @@ -87,7 +90,7 @@ int V4L2Device::open(unsigned int flags) return ret; } - fd_ = ret; + setFd(ret); listControls(); @@ -117,6 +120,10 @@ int V4L2Device::setFd(int fd) fd_ = fd; + fdEventNotifier_ = new EventNotifier(fd_, EventNotifier::Exception); + fdEventNotifier_->activated.connect(this, &V4L2Device::eventAvailable); + fdEventNotifier_->setEnabled(false); + return 0; } @@ -130,6 +137,8 @@ void V4L2Device::close() if (!isOpen()) return; + delete fdEventNotifier_; + if (::close(fd_) < 0) LOG(V4L2, Error) << "Failed to close V4L2 device: " << strerror(errno); @@ -396,6 +405,40 @@ std::string V4L2Device::devicePath() const return path; } +/** + * \brief Enable or disable frame start event notification + * \param[in] enable True to enable frame start events, false to disable them + * + * This function enables or disables generation of frame start events. Once + * enabled, the events are signalled through the frameStart signal. + * + * \return 0 on success, a negative error code otherwise + */ +int V4L2Device::setFrameStartEnabled(bool enable) +{ + if (frameStartEnabled_ == enable) + return 0; + + struct v4l2_event_subscription event{}; + event.type = V4L2_EVENT_FRAME_SYNC; + + unsigned long request = enable ? VIDIOC_SUBSCRIBE_EVENT + : VIDIOC_UNSUBSCRIBE_EVENT; + int ret = ioctl(request, &event); + if (enable && ret) + return ret; + + fdEventNotifier_->setEnabled(enable); + frameStartEnabled_ = enable; + + return ret; +} + +/** + * \var V4L2Device::frameStart + * \brief A Signal emitted when capture of a frame has started + */ + /** * \brief Perform an IOCTL system call on the device node * \param[in] request The IOCTL request code @@ -519,4 +562,33 @@ void V4L2Device::updateControls(ControlList *ctrls, } } +/** + * \brief Slot to handle V4L2 events from the V4L2 device + * \param[in] notifier The event notifier + * + * When this slot is called, a V4L2 event is available to be dequeued from the + * device. + */ +void V4L2Device::eventAvailable([[maybe_unused]] EventNotifier *notifier) +{ + struct v4l2_event event{}; + int ret = ioctl(VIDIOC_DQEVENT, &event); + if (ret < 0) { + LOG(V4L2, Error) + << "Failed to dequeue event, disabling event notifier"; + fdEventNotifier_->setEnabled(false); + return; + } + + if (event.type != V4L2_EVENT_FRAME_SYNC) { + LOG(V4L2, Error) + << "Spurious event (" << event.type + << "), disabling event notifier"; + fdEventNotifier_->setEnabled(false); + return; + } + + frameStart.emit(event.u.frame_sync.frame_sequence); +} + } /* namespace libcamera */ diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index 36d7d9a0f27a103f..012d9bc73f30ad09 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -472,8 +472,7 @@ const std::string V4L2DeviceFormat::toString() const * \param[in] deviceNode The file-system path to the video device node */ V4L2VideoDevice::V4L2VideoDevice(const std::string &deviceNode) - : V4L2Device(deviceNode), cache_(nullptr), fdBufferNotifier_(nullptr), - fdEventNotifier_(nullptr), frameStartEnabled_(false) + : V4L2Device(deviceNode), cache_(nullptr), fdBufferNotifier_(nullptr) { /* * We default to an MMAP based CAPTURE video device, however this will @@ -566,10 +565,6 @@ int V4L2VideoDevice::open() fdBufferNotifier_->activated.connect(this, &V4L2VideoDevice::bufferAvailable); fdBufferNotifier_->setEnabled(false); - fdEventNotifier_ = new EventNotifier(fd(), EventNotifier::Exception); - fdEventNotifier_->activated.connect(this, &V4L2VideoDevice::eventAvailable); - fdEventNotifier_->setEnabled(false); - LOG(V4L2, Debug) << "Opened device " << caps_.bus_info() << ": " << caps_.driver() << ": " << caps_.card(); @@ -659,10 +654,6 @@ int V4L2VideoDevice::open(int handle, enum v4l2_buf_type type) fdBufferNotifier_->activated.connect(this, &V4L2VideoDevice::bufferAvailable); fdBufferNotifier_->setEnabled(false); - fdEventNotifier_ = new EventNotifier(fd(), EventNotifier::Exception); - fdEventNotifier_->activated.connect(this, &V4L2VideoDevice::eventAvailable); - fdEventNotifier_->setEnabled(false); - LOG(V4L2, Debug) << "Opened device " << caps_.bus_info() << ": " << caps_.driver() << ": " << caps_.card(); @@ -680,7 +671,6 @@ void V4L2VideoDevice::close() releaseBuffers(); delete fdBufferNotifier_; - delete fdEventNotifier_; V4L2Device::close(); } @@ -1533,74 +1523,11 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer() return buffer; } -/** - * \brief Slot to handle V4L2 events from the V4L2 video device - * \param[in] notifier The event notifier - * - * When this slot is called, a V4L2 event is available to be dequeued from the - * device. - */ -void V4L2VideoDevice::eventAvailable([[maybe_unused]] EventNotifier *notifier) -{ - struct v4l2_event event{}; - int ret = ioctl(VIDIOC_DQEVENT, &event); - if (ret < 0) { - LOG(V4L2, Error) - << "Failed to dequeue event, disabling event notifier"; - fdEventNotifier_->setEnabled(false); - return; - } - - if (event.type != V4L2_EVENT_FRAME_SYNC) { - LOG(V4L2, Error) - << "Spurious event (" << event.type - << "), disabling event notifier"; - fdEventNotifier_->setEnabled(false); - return; - } - - frameStart.emit(event.u.frame_sync.frame_sequence); -} - /** * \var V4L2VideoDevice::bufferReady * \brief A Signal emitted when a framebuffer completes */ -/** - * \brief Enable or disable frame start event notification - * \param[in] enable True to enable frame start events, false to disable them - * - * This function enables or disables generation of frame start events. Once - * enabled, the events are signalled through the frameStart signal. - * - * \return 0 on success, a negative error code otherwise - */ -int V4L2VideoDevice::setFrameStartEnabled(bool enable) -{ - if (frameStartEnabled_ == enable) - return 0; - - struct v4l2_event_subscription event{}; - event.type = V4L2_EVENT_FRAME_SYNC; - - unsigned long request = enable ? VIDIOC_SUBSCRIBE_EVENT - : VIDIOC_UNSUBSCRIBE_EVENT; - int ret = ioctl(request, &event); - if (enable && ret) - return ret; - - fdEventNotifier_->setEnabled(enable); - frameStartEnabled_ = enable; - - return ret; -} - -/** - * \var V4L2VideoDevice::frameStart - * \brief A Signal emitted when capture of a frame has started - */ - /** * \brief Start the video stream * \return 0 on success or a negative error code otherwise From patchwork Wed Oct 28 01:00:44 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 10275 X-Patchwork-Delegate: niklas.soderlund@ragnatech.se Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 0B3F7C3B5C for ; Wed, 28 Oct 2020 01:01:16 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D91DE6225D; Wed, 28 Oct 2020 02:01:15 +0100 (CET) Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 444F06220D for ; Wed, 28 Oct 2020 02:01:12 +0100 (CET) X-Halon-ID: 1163a54f-18b9-11eb-954c-0050569116f7 Authorized-sender: niklas.soderlund@fsdn.se Received: from bismarck.berto.se (p4fca2458.dip0.t-ipconnect.de [79.202.36.88]) by bin-vsp-out-03.atm.binero.net (Halon) with ESMTPA id 1163a54f-18b9-11eb-954c-0050569116f7; Wed, 28 Oct 2020 02:01:10 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org, david.plowman@raspberrypi.com, naush@raspberrypi.com Date: Wed, 28 Oct 2020 02:00:44 +0100 Message-Id: <20201028010051.3830668-3-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.29.1 In-Reply-To: <20201028010051.3830668-1-niklas.soderlund@ragnatech.se> References: <20201028010051.3830668-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 2/9] libcamera: delayed_controls: Add helper for controls that applies with a delay X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 --- include/libcamera/internal/delayed_controls.h | 87 ++++++ src/libcamera/delayed_controls.cpp | 282 ++++++++++++++++++ src/libcamera/meson.build | 1 + 3 files changed, 370 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..df5520d240a54e4b --- /dev/null +++ b/include/libcamera/internal/delayed_controls.h @@ -0,0 +1,87 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2020, Google Inc. + * + * 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 +#include +#include + +#include + +namespace libcamera { + +class V4L2Device; + +class DelayedControls +{ +public: + DelayedControls(V4L2Device *device, + const std::unordered_map &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 + { + public: + ControlInfo &operator[](unsigned int index) + { + return std::array::operator[](index % listSize); + } + + const ControlInfo &operator[](unsigned int index) const + { + return std::array::operator[](index % listSize); + } + }; + + using ControlsDelays = std::unordered_map; + using ControlsValues = std::unordered_map; + + 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..0e32f417c5cc68b7 --- /dev/null +++ b/src/libcamera/delayed_controls.cpp @@ -0,0 +1,282 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2020, Google Inc. + * + * 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 + +#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 takes 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 user is pipeline handlers. + * + * The idea is to extend the concept of the pipeline depth the users needs to + * maintain for buffers to controls. The depth is determined with by the control + * with the grates delay. As long as the pipeline keeps the control queue above + * this level the control values are guaranteed to be in effect at the specified + * point in time. + * + * The downside is of course that the pipeline needs to know what controls to + * set control depth frames in advance. But there really is no way around this + * as the delay is a consequence of the physical world. Compare this with + * parameter buffers where the buffer may be queued to the device while it's + * still being written to as long as it's ready before it's consumed, this is + * because the parameter buffer (usually) does not contain controls that + * requires time to take effect. + */ + + +/** + * \brief Construct a DelayedControls + * \param[in] device The V4L2 device containing the delayed controls + * \param[in] delays Map of numerical V4L2 control id to its delay to take + * effect in frames + * + * Only controls specified in \a delays are handled by the DelayedControls + * instance. If it's desired to mix delayed controls and controls that takes + * 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 &delays) + : device_(device), maxDelay_(0) +{ + const ControlInfoMap &controls = device_->controls(); + + /* + * Sanity check that all controls where delays are requested are + * exposed byt the device. + */ + for (auto const &delay : delays) { + unsigned int id = delay.first; + + if (controls.find(id) == controls.end()) + LOG(DelayedControls, Error) + << "Delay request for control id " + << utils::hex(id) + << " but control is not exposed by device " + << device_->deviceNode(); + } + + /* + * Create a map of control to delay for all controls exposed by the + * device. If no delay is specified assume the control applies directly. + */ + for (auto const &control : controls) { + const ControlId *id = control.first; + + auto it = delays.find(id->id()); + if (it == delays.end()) + continue; + + delays_[id] = it->second; + + LOG(DelayedControls, Debug) + << "Set a delay of " << delays_[id] + << " for " << id->name(); + + maxDelay_ = std::max(maxDelay_, delays_[id]); + } + + reset(); +} + +/** + * \brief Reset the V4L2 device + * \param[in] controls List of controls to reset the device to or nullptr + * + * Resets the delayed controls state machine to its starting state. All controls + * are fetched from the V4L2 device to provide a good starting point for the + * first frames (length of control depth). + * + * Optionally \a controls can be specified to set some or all of the handled + * V4L2 controls prior to reading them back. If no controls needs to be set + * nullptr may be used. + */ +void DelayedControls::reset(ControlList *controls) +{ + std::lock_guard 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 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 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 lock(lock_); + + return queue(controls); +} + +bool DelayedControls::queue(const ControlList &controls) +{ + /* Copy state from previous frame. */ + for (auto &ctrl : ctrls_) { + ControlInfo &info = ctrls_[ctrl.first][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 specific sequence number + * \param[in] sequence Sequence number to get read back 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. + * + * \returns List of controls in effect at \a sequence + */ +ControlList DelayedControls::get(uint32_t sequence) +{ + std::lock_guard lock(lock_); + + uint32_t adjustedSeq = sequence - fistSequence_ + 1; + unsigned int index = std::max(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 a start of a new frame + * \param[in] sequence Sequence number of the frame that started + * + * Inform the state machine that a new frame have started and it's sequence + * number. It's user of this helpers responsibility to inform the helper + * at the start of every frame. This can with ease be connected to the start + * of exposure (SOE) V4L2 event. + */ +void DelayedControls::frameStart(uint32_t sequence) +{ + LOG(DelayedControls, Debug) << "frame " << sequence << " started"; + + std::lock_guard 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(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); +} + +} /* namespace libcamera */ diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index 07711b5f93bcc921..19f22f9c94e1d64d 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', From patchwork Wed Oct 28 01:00:45 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 10276 X-Patchwork-Delegate: niklas.soderlund@ragnatech.se Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 65AD4C3B60 for ; Wed, 28 Oct 2020 01:01:16 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 27AB662261; Wed, 28 Oct 2020 02:01:16 +0100 (CET) Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7BB5E6220D for ; Wed, 28 Oct 2020 02:01:13 +0100 (CET) X-Halon-ID: 1241bec0-18b9-11eb-954c-0050569116f7 Authorized-sender: niklas.soderlund@fsdn.se Received: from bismarck.berto.se (p4fca2458.dip0.t-ipconnect.de [79.202.36.88]) by bin-vsp-out-03.atm.binero.net (Halon) with ESMTPA id 1241bec0-18b9-11eb-954c-0050569116f7; Wed, 28 Oct 2020 02:01:12 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org, david.plowman@raspberrypi.com, naush@raspberrypi.com Date: Wed, 28 Oct 2020 02:00:45 +0100 Message-Id: <20201028010051.3830668-4-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.29.1 In-Reply-To: <20201028010051.3830668-1-niklas.soderlund@ragnatech.se> References: <20201028010051.3830668-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 3/9] test: delayed_controls: Add test case for DelayedControls X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Add a test-case for DelayedControls that exercise the setting of controls and reading back what controls where used for a particular frame. Also exercise corner case such as a V4L2 devices that do not reset it sequence number to 0 at stream on and sequence number wrapping around the uint32_t value space. Signed-off-by: Niklas Söderlund --- test/delayed_contols.cpp | 307 +++++++++++++++++++++++++++++++++++++++ test/meson.build | 1 + 2 files changed, 308 insertions(+) create mode 100644 test/delayed_contols.cpp diff --git a/test/delayed_contols.cpp b/test/delayed_contols.cpp new file mode 100644 index 0000000000000000..e71da6662c30bbdc --- /dev/null +++ b/test/delayed_contols.cpp @@ -0,0 +1,307 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2020, Google Inc. + * + * libcamera delayed controls tests + */ + +#include + +#include "libcamera/internal/delayed_controls.h" +#include "libcamera/internal/device_enumerator.h" +#include "libcamera/internal/media_device.h" +#include "libcamera/internal/v4l2_videodevice.h" + +#include "test.h" + +using namespace std; +using namespace libcamera; + +class DelayedControlsTest : public Test +{ +public: + DelayedControlsTest() + : dev_(nullptr) + { + } + + int init() + { + enumerator_ = DeviceEnumerator::create(); + if (!enumerator_) { + cerr << "Failed to create device enumerator" << endl; + return TestFail; + } + + if (enumerator_->enumerate()) { + cerr << "Failed to enumerate media devices" << endl; + return TestFail; + } + + DeviceMatch dm("vivid"); + dm.add("vivid-000-vid-cap"); + + media_ = enumerator_->search(dm); + if (!media_) { + cerr << "vivid video device found" << endl; + return TestSkip; + } + + MediaEntity *entity = media_->getEntityByName("vivid-000-vid-cap"); + dev_ = new V4L2VideoDevice(entity->deviceNode()); + if (dev_->open()) { + cerr << "Failed to open video device" << endl; + return TestFail; + } + + const ControlInfoMap &infoMap = dev_->controls(); + + /* Test control enumeration. */ + if (infoMap.empty()) { + cerr << "Failed to enumerate controls" << endl; + return TestFail; + } + + if (infoMap.find(V4L2_CID_BRIGHTNESS) == infoMap.end() || + infoMap.find(V4L2_CID_CONTRAST) == infoMap.end()) { + cerr << "Missing controls" << endl; + return TestFail; + } + + ControlList ctrls = dev_->getControls({ V4L2_CID_BRIGHTNESS }); + + return TestPass; + } + + int singleControlNoDelay() + { + std::unordered_map delays = { + { V4L2_CID_BRIGHTNESS, 0 }, + }; + std::unique_ptr delayed = + std::make_unique(dev_, delays); + + ControlList ctrls; + + /* Reset control to value not used in test. */ + ctrls.set(V4L2_CID_BRIGHTNESS, 1); + delayed->reset(&ctrls); + + /* Test control without delay are set at once. */ + for (int32_t i = 0; i < 10; i++) { + int32_t value = 100 + i; + + ctrls.set(V4L2_CID_BRIGHTNESS, value); + delayed->push(ctrls); + + delayed->frameStart(i); + + ControlList result = delayed->get(i); + int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get(); + if (brightness != value) { + cerr << "Failed single control without delay" + << " frame " << i + << " expected " << value + << " got " << brightness + << endl; + return TestFail; + } + } + + return TestPass; + } + + int singleControlWithDelay() + { + std::unordered_map delays = { + { V4L2_CID_BRIGHTNESS, 1 }, + }; + std::unique_ptr delayed = + std::make_unique(dev_, delays); + + ControlList ctrls; + + /* Reset control to value that will be first in test. */ + int32_t expected = 4; + ctrls.set(V4L2_CID_BRIGHTNESS, expected); + delayed->reset(&ctrls); + + /* Test single control with delay. */ + for (int32_t i = 0; i < 100; i++) { + int32_t value = 10 + i; + + ctrls.set(V4L2_CID_BRIGHTNESS, value); + delayed->push(ctrls); + + delayed->frameStart(i); + + ControlList result = delayed->get(i); + int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get(); + if (brightness != expected) { + cerr << "Failed single control with delay" + << " frame " << i + << " expected " << expected + << " got " << brightness + << endl; + return TestFail; + } + + expected = value; + } + + return TestPass; + } + + int dualControlsWithDelay(uint32_t startOffset) + { + std::unordered_map delays = { + { V4L2_CID_BRIGHTNESS, 1 }, + { V4L2_CID_CONTRAST, 2 }, + }; + std::unique_ptr delayed = + std::make_unique(dev_, delays); + + ControlList ctrls; + + /* Reset control to value that will be first two frames in test. */ + int32_t expected = 200; + ctrls.set(V4L2_CID_BRIGHTNESS, expected); + ctrls.set(V4L2_CID_CONTRAST, expected); + delayed->reset(&ctrls); + + /* Test dual control with delay. */ + for (int32_t i = 0; i < 100; i++) { + uint32_t frame = startOffset + i; + + int32_t value = 10 + i; + + ctrls.set(V4L2_CID_BRIGHTNESS, value); + ctrls.set(V4L2_CID_CONTRAST, value); + delayed->push(ctrls); + + delayed->frameStart(frame); + + ControlList result = delayed->get(frame); + + int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get(); + int32_t contrast = result.get(V4L2_CID_CONTRAST).get(); + if (brightness != expected || contrast != expected) { + cerr << "Failed dual controls" + << " frame " << frame + << " brightness " << brightness + << " contrast " << contrast + << " expected " << expected + << endl; + return TestFail; + } + + expected = i < 1 ? expected : value - 1; + } + + return TestPass; + } + + int dualControlsMultiQueue() + { + std::unordered_map delays = { + { V4L2_CID_BRIGHTNESS, 1 }, + { V4L2_CID_CONTRAST, 2 }, + }; + std::unique_ptr delayed = + std::make_unique(dev_, delays); + + ControlList ctrls; + + /* Reset control to value that will be first two frames in test. */ + int32_t expected = 100; + ctrls.set(V4L2_CID_BRIGHTNESS, expected); + ctrls.set(V4L2_CID_CONTRAST, expected); + delayed->reset(&ctrls); + + /* + * Queue all controls before any fake frame start. Note we + * can't queue up more then the delayed controls history size + * which is 16. Where one spot is used by the reset control. + */ + for (int32_t i = 0; i < 15; i++) { + int32_t value = 10 + i; + + ctrls.set(V4L2_CID_BRIGHTNESS, value); + ctrls.set(V4L2_CID_CONTRAST, value); + delayed->push(ctrls); + } + + /* Process all queued controls. */ + for (int32_t i = 0; i < 16; i++) { + int32_t value = 10 + i; + + delayed->frameStart(i); + + ControlList result = delayed->get(i); + + int32_t brightness = result.get(V4L2_CID_BRIGHTNESS).get(); + int32_t contrast = result.get(V4L2_CID_CONTRAST).get(); + if (brightness != expected || contrast != expected) { + cerr << "Failed multi queue" + << " frame " << i + << " brightness " << brightness + << " contrast " << contrast + << " expected " << expected + << endl; + return TestFail; + } + + expected = i < 1 ? expected : value - 1; + } + + return TestPass; + } + + int run() + { + int ret; + + /* Test single control without delay. */ + ret = singleControlNoDelay(); + if (ret) + return ret; + + /* Test single control with delay. */ + ret = singleControlWithDelay(); + if (ret) + return ret; + + /* Test dual controls with different delays. */ + ret = dualControlsWithDelay(0); + if (ret) + return ret; + + /* Test dual controls with non-zero sequence start. */ + ret = dualControlsWithDelay(10000); + if (ret) + return ret; + + /* Test dual controls with sequence number wraparound. */ + ret = dualControlsWithDelay(UINT32_MAX - 50); + if (ret) + return ret; + + /* Test control values produced faster then consumed. */ + ret = dualControlsMultiQueue(); + if (ret) + return ret; + return TestPass; + } + + void cleanup() + { + delete dev_; + } + +private: + std::unique_ptr enumerator_; + std::shared_ptr media_; + V4L2VideoDevice *dev_; +}; + +TEST_REGISTER(DelayedControlsTest) diff --git a/test/meson.build b/test/meson.build index 0a1d434e399641bb..a683a657a439b4ff 100644 --- a/test/meson.build +++ b/test/meson.build @@ -25,6 +25,7 @@ public_tests = [ internal_tests = [ ['byte-stream-buffer', 'byte-stream-buffer.cpp'], ['camera-sensor', 'camera-sensor.cpp'], + ['delayed_contols', 'delayed_contols.cpp'], ['event', 'event.cpp'], ['event-dispatcher', 'event-dispatcher.cpp'], ['event-thread', 'event-thread.cpp'], From patchwork Wed Oct 28 01:00:46 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 10277 X-Patchwork-Delegate: niklas.soderlund@ragnatech.se Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 00F37C3B5C for ; Wed, 28 Oct 2020 01:01:17 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 73E5E62265; Wed, 28 Oct 2020 02:01:16 +0100 (CET) Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5B7956220D for ; Wed, 28 Oct 2020 02:01:14 +0100 (CET) X-Halon-ID: 12f73b83-18b9-11eb-954c-0050569116f7 Authorized-sender: niklas.soderlund@fsdn.se Received: from bismarck.berto.se (p4fca2458.dip0.t-ipconnect.de [79.202.36.88]) by bin-vsp-out-03.atm.binero.net (Halon) with ESMTPA id 12f73b83-18b9-11eb-954c-0050569116f7; Wed, 28 Oct 2020 02:01:13 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org, david.plowman@raspberrypi.com, naush@raspberrypi.com Date: Wed, 28 Oct 2020 02:00:46 +0100 Message-Id: <20201028010051.3830668-5-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.29.1 In-Reply-To: <20201028010051.3830668-1-niklas.soderlund@ragnatech.se> References: <20201028010051.3830668-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 4/9] libcamera: raspberrypi: Switch to DelayedControls X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Use the libcamera core helper DelayedControls instead of the local StaggeredCtrl. The new helper is modeled after the StaggeredCtrl implementation and behaves the same. Signed-off-by: Niklas Söderlund --- .../pipeline/raspberrypi/raspberrypi.cpp | 44 +++++++++---------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index a48013d8c27b98eb..4087985f1e66c940 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -25,6 +25,7 @@ #include "libcamera/internal/bayer_format.h" #include "libcamera/internal/camera_sensor.h" +#include "libcamera/internal/delayed_controls.h" #include "libcamera/internal/device_enumerator.h" #include "libcamera/internal/ipa_manager.h" #include "libcamera/internal/media_device.h" @@ -35,7 +36,6 @@ #include "dma_heaps.h" #include "rpi_stream.h" -#include "staggered_ctrl.h" namespace libcamera { @@ -169,8 +169,7 @@ public: RPi::DmaHeap dmaHeap_; FileDescriptor lsTable_; - RPi::StaggeredCtrl staggeredCtrl_; - uint32_t expectedSequence_; + std::unique_ptr delayedCtrls_; bool sensorMetadata_; /* @@ -773,10 +772,7 @@ int PipelineHandlerRPi::start(Camera *camera) * starting. First check that the staggered ctrl has been initialised * by configure(). */ - ASSERT(data->staggeredCtrl_); - data->staggeredCtrl_.reset(); - data->staggeredCtrl_.write(); - data->expectedSequence_ = 0; + data->delayedCtrls_->reset(); data->state_ = RPiCameraData::State::Idle; @@ -1107,7 +1103,7 @@ void RPiCameraData::frameStarted(uint32_t sequence) LOG(RPI, Debug) << "frame start " << sequence; /* Write any controls for the next frame as soon as we can. */ - staggeredCtrl_.write(); + delayedCtrls_->frameStart(sequence); } int RPiCameraData::loadIPA() @@ -1185,18 +1181,22 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config) * Setup our staggered control writer with the sensor default * gain and exposure delays. */ - if (!staggeredCtrl_) { - staggeredCtrl_.init(unicam_[Unicam::Image].dev(), - { { V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] }, - { V4L2_CID_EXPOSURE, result.data[resultIdx++] } }); + + if (!delayedCtrls_) { + std::unordered_map delays = { + { V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] }, + { V4L2_CID_EXPOSURE, result.data[resultIdx++] } + }; + + delayedCtrls_ = std::make_unique(unicam_[Unicam::Image].dev(), delays); + sensorMetadata_ = result.data[resultIdx++]; } } if (result.operation & RPi::IPA_CONFIG_SENSOR) { - const ControlList &ctrls = result.controls[0]; - if (!staggeredCtrl_.set(ctrls)) - LOG(RPI, Error) << "V4L2 staggered set failed"; + ControlList ctrls = result.controls[0]; + delayedCtrls_->reset(&ctrls); } if (result.operation & RPi::IPA_CONFIG_DROP_FRAMES) { @@ -1230,8 +1230,8 @@ void RPiCameraData::queueFrameAction([[maybe_unused]] unsigned int frame, switch (action.operation) { case RPi::IPA_ACTION_V4L2_SET_STAGGERED: { const ControlList &controls = action.controls[0]; - if (!staggeredCtrl_.set(controls)) - LOG(RPI, Error) << "V4L2 staggered set failed"; + if (!delayedCtrls_->push(controls)) + LOG(RPI, Error) << "V4L2 delay set failed"; goto done; } @@ -1335,11 +1335,7 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer) } else { embeddedQueue_.push(buffer); - std::unordered_map ctrl; - int offset = buffer->metadata().sequence - expectedSequence_; - staggeredCtrl_.get(ctrl, offset); - - expectedSequence_ = buffer->metadata().sequence + 1; + ControlList ctrl = delayedCtrls_->get(buffer->metadata().sequence); /* * Sensor metadata is unavailable, so put the expected ctrl @@ -1352,8 +1348,8 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer) PROT_READ | PROT_WRITE, MAP_SHARED, fb.planes()[0].fd.fd(), 0)); - mem[0] = ctrl[V4L2_CID_EXPOSURE]; - mem[1] = ctrl[V4L2_CID_ANALOGUE_GAIN]; + mem[0] = ctrl.get(V4L2_CID_EXPOSURE).get(); + mem[1] = ctrl.get(V4L2_CID_ANALOGUE_GAIN).get(); munmap(mem, fb.planes()[0].length); } } From patchwork Wed Oct 28 01:00:47 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 10278 X-Patchwork-Delegate: niklas.soderlund@ragnatech.se Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 7E1ABC3B5C for ; Wed, 28 Oct 2020 01:01:19 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5658562271; Wed, 28 Oct 2020 02:01:19 +0100 (CET) Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7B30862237 for ; Wed, 28 Oct 2020 02:01:15 +0100 (CET) X-Halon-ID: 1380cbf4-18b9-11eb-954c-0050569116f7 Authorized-sender: niklas.soderlund@fsdn.se Received: from bismarck.berto.se (p4fca2458.dip0.t-ipconnect.de [79.202.36.88]) by bin-vsp-out-03.atm.binero.net (Halon) with ESMTPA id 1380cbf4-18b9-11eb-954c-0050569116f7; Wed, 28 Oct 2020 02:01:14 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org, david.plowman@raspberrypi.com, naush@raspberrypi.com Date: Wed, 28 Oct 2020 02:00:47 +0100 Message-Id: <20201028010051.3830668-6-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.29.1 In-Reply-To: <20201028010051.3830668-1-niklas.soderlund@ragnatech.se> References: <20201028010051.3830668-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 5/9] libcamera: raspberrypi: Remove StaggeredCtrl X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" There are no users left, remove it. Signed-off-by: Niklas Söderlund --- .../pipeline/raspberrypi/meson.build | 1 - .../pipeline/raspberrypi/staggered_ctrl.cpp | 174 ------------------ .../pipeline/raspberrypi/staggered_ctrl.h | 96 ---------- 3 files changed, 271 deletions(-) delete mode 100644 src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp delete mode 100644 src/libcamera/pipeline/raspberrypi/staggered_ctrl.h diff --git a/src/libcamera/pipeline/raspberrypi/meson.build b/src/libcamera/pipeline/raspberrypi/meson.build index 7c5b6ff73741c968..f1a2f5ee72c2a0dd 100644 --- a/src/libcamera/pipeline/raspberrypi/meson.build +++ b/src/libcamera/pipeline/raspberrypi/meson.build @@ -4,5 +4,4 @@ libcamera_sources += files([ 'dma_heaps.cpp', 'raspberrypi.cpp', 'rpi_stream.cpp', - 'staggered_ctrl.cpp', ]) diff --git a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp b/src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp deleted file mode 100644 index 62605c0fceee7d11..0000000000000000 --- a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp +++ /dev/null @@ -1,174 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -/* - * Copyright (C) 2020, Raspberry Pi (Trading) Ltd. - * - * staggered_ctrl.cpp - Helper for writing staggered ctrls to a V4L2 device. - */ - -#include "staggered_ctrl.h" - -#include - -#include - -#include "libcamera/internal/log.h" -#include "libcamera/internal/utils.h" -#include "libcamera/internal/v4l2_videodevice.h" - -namespace libcamera { - -LOG_DEFINE_CATEGORY(RPI_S_W) - -namespace RPi { - -void StaggeredCtrl::init(V4L2VideoDevice *dev, - std::initializer_list> delayList) -{ - std::lock_guard lock(lock_); - - dev_ = dev; - delay_ = delayList; - ctrl_.clear(); - - /* Find the largest delay across all controls. */ - maxDelay_ = 0; - for (auto const &p : delay_) { - LOG(RPI_S_W, Info) << "Init ctrl " - << utils::hex(p.first) << " with delay " - << static_cast(p.second); - maxDelay_ = std::max(maxDelay_, p.second); - } - - init_ = true; -} - -void StaggeredCtrl::reset() -{ - std::lock_guard lock(lock_); - - int lastSetCount = setCount_; - std::unordered_map lastVal; - - /* Reset the counters. */ - setCount_ = getCount_ = 0; - - /* Look for the last set values. */ - for (auto const &c : ctrl_) - lastVal[c.first] = c.second[lastSetCount].value; - - /* Apply the last set values as the next to be applied. */ - ctrl_.clear(); - for (auto &c : lastVal) - ctrl_[c.first][setCount_] = CtrlInfo(c.second); -} - -bool StaggeredCtrl::set(uint32_t ctrl, int32_t value) -{ - std::lock_guard lock(lock_); - - /* Can we find this ctrl as one that is registered? */ - if (delay_.find(ctrl) == delay_.end()) - return false; - - ctrl_[ctrl][setCount_].value = value; - ctrl_[ctrl][setCount_].updated = true; - - return true; -} - -bool StaggeredCtrl::set(std::initializer_list> ctrlList) -{ - std::lock_guard lock(lock_); - - for (auto const &p : ctrlList) { - /* Can we find this ctrl? */ - if (delay_.find(p.first) == delay_.end()) - return false; - - ctrl_[p.first][setCount_] = CtrlInfo(p.second); - } - - return true; -} - -bool StaggeredCtrl::set(const ControlList &controls) -{ - std::lock_guard lock(lock_); - - for (auto const &p : controls) { - /* Can we find this ctrl? */ - if (delay_.find(p.first) == delay_.end()) - return false; - - ctrl_[p.first][setCount_] = CtrlInfo(p.second.get()); - LOG(RPI_S_W, Debug) << "Setting ctrl " - << utils::hex(p.first) << " to " - << ctrl_[p.first][setCount_].value - << " at index " - << setCount_; - } - - return true; -} - -int StaggeredCtrl::write() -{ - std::lock_guard lock(lock_); - ControlList controls(dev_->controls()); - - for (auto &p : ctrl_) { - int delayDiff = maxDelay_ - delay_[p.first]; - int index = std::max(0, setCount_ - delayDiff); - - if (p.second[index].updated) { - /* We need to write this value out. */ - controls.set(p.first, p.second[index].value); - p.second[index].updated = false; - LOG(RPI_S_W, Debug) << "Writing ctrl " - << utils::hex(p.first) << " to " - << p.second[index].value - << " at index " - << index; - } - } - - nextFrame(); - return dev_->setControls(&controls); -} - -void StaggeredCtrl::get(std::unordered_map &ctrl, uint8_t offset) -{ - std::lock_guard lock(lock_); - - /* Account for the offset to reset the getCounter. */ - getCount_ += offset + 1; - - ctrl.clear(); - for (auto &p : ctrl_) { - int index = std::max(0, getCount_ - maxDelay_); - ctrl[p.first] = p.second[index].value; - LOG(RPI_S_W, Debug) << "Getting ctrl " - << utils::hex(p.first) << " to " - << p.second[index].value - << " at index " - << index; - } -} - -void StaggeredCtrl::nextFrame() -{ - /* Advance the control history to the next frame */ - int prevCount = setCount_; - setCount_++; - - LOG(RPI_S_W, Debug) << "Next frame, set index is " << setCount_; - - for (auto &p : ctrl_) { - p.second[setCount_].value = p.second[prevCount].value; - p.second[setCount_].updated = false; - } -} - -} /* namespace RPi */ - -} /* namespace libcamera */ diff --git a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h b/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h deleted file mode 100644 index 382fa31a685357f2..0000000000000000 --- a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h +++ /dev/null @@ -1,96 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -/* - * Copyright (C) 2020, Raspberry Pi (Trading) Ltd. - * - * staggered_ctrl.h - Helper for writing staggered ctrls to a V4L2 device. - */ -#ifndef __LIBCAMERA_PIPELINE_RASPBERRYPI_STAGGERED_CTRL_H__ -#define __LIBCAMERA_PIPELINE_RASPBERRYPI_STAGGERED_CTRL_H__ - -#include -#include -#include -#include -#include - -namespace libcamera { - -class ControlList; -class V4L2VideoDevice; - -namespace RPi { - -class StaggeredCtrl -{ -public: - StaggeredCtrl() - : init_(false), setCount_(0), getCount_(0), maxDelay_(0) - { - } - - operator bool() const - { - return init_; - } - - void init(V4L2VideoDevice *dev, - std::initializer_list> delayList); - void reset(); - - void get(std::unordered_map &ctrl, uint8_t offset = 0); - - bool set(uint32_t ctrl, int32_t value); - bool set(std::initializer_list> ctrlList); - bool set(const ControlList &controls); - - int write(); - -private: - void nextFrame(); - - /* listSize must be a power of 2. */ - static constexpr int listSize = (1 << 4); - struct CtrlInfo { - CtrlInfo() - : value(0), updated(false) - { - } - - CtrlInfo(int32_t value_) - : value(value_), updated(true) - { - } - - int32_t value; - bool updated; - }; - - class CircularArray : public std::array - { - public: - CtrlInfo &operator[](int index) - { - return std::array::operator[](index & (listSize - 1)); - } - - const CtrlInfo &operator[](int index) const - { - return std::array::operator[](index & (listSize - 1)); - } - }; - - bool init_; - uint32_t setCount_; - uint32_t getCount_; - uint8_t maxDelay_; - V4L2VideoDevice *dev_; - std::unordered_map delay_; - std::unordered_map ctrl_; - std::mutex lock_; -}; - -} /* namespace RPi */ - -} /* namespace libcamera */ - -#endif /* __LIBCAMERA_PIPELINE_RASPBERRYPI_STAGGERED_CTRL_H__ */ From patchwork Wed Oct 28 01:00:48 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 10279 X-Patchwork-Delegate: niklas.soderlund@ragnatech.se Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id F26AEC3B5C for ; Wed, 28 Oct 2020 01:01:19 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id AD71862279; Wed, 28 Oct 2020 02:01:19 +0100 (CET) Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6996462264 for ; Wed, 28 Oct 2020 02:01:16 +0100 (CET) X-Halon-ID: 142dafb7-18b9-11eb-954c-0050569116f7 Authorized-sender: niklas.soderlund@fsdn.se Received: from bismarck.berto.se (p4fca2458.dip0.t-ipconnect.de [79.202.36.88]) by bin-vsp-out-03.atm.binero.net (Halon) with ESMTPA id 142dafb7-18b9-11eb-954c-0050569116f7; Wed, 28 Oct 2020 02:01:15 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org, david.plowman@raspberrypi.com, naush@raspberrypi.com Date: Wed, 28 Oct 2020 02:00:48 +0100 Message-Id: <20201028010051.3830668-7-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.29.1 In-Reply-To: <20201028010051.3830668-1-niklas.soderlund@ragnatech.se> References: <20201028010051.3830668-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 6/9] libcamera: camera_sensor: Expose a DelayedControls interface X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Expose the optional DelayedControls interface to pipeline handlers. If used by a pipeline generic delays are used. In the future the delay values should be fetched to match the specific sensor module, either from a new kernel interface or worst case a sensors database. Signed-off-by: Niklas Söderlund --- include/libcamera/internal/camera_sensor.h | 5 ++++ src/libcamera/camera_sensor.cpp | 31 ++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h index b9eba89f00fba882..6be1cfd49134c534 100644 --- a/include/libcamera/internal/camera_sensor.h +++ b/include/libcamera/internal/camera_sensor.h @@ -14,6 +14,7 @@ #include #include +#include "libcamera/internal/delayed_controls.h" #include "libcamera/internal/formats.h" #include "libcamera/internal/log.h" #include "libcamera/internal/v4l2_subdevice.h" @@ -61,6 +62,8 @@ public: ControlList getControls(const std::vector &ids); int setControls(ControlList *ctrls); + DelayedControls *delayedContols(); + const ControlList &properties() const { return properties_; } int sensorInfo(CameraSensorInfo *info) const; @@ -83,6 +86,8 @@ private: std::vector sizes_; ControlList properties_; + + std::unique_ptr delayedControls_; }; } /* namespace libcamera */ diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index 935de528c4963453..6c92c97f4cc2547a 100644 --- a/src/libcamera/camera_sensor.cpp +++ b/src/libcamera/camera_sensor.cpp @@ -493,6 +493,37 @@ int CameraSensor::setControls(ControlList *ctrls) return subdev_->setControls(ctrls); } +/** + * \brief Get the sensors delayed controls interface + * + * Access the sensors delayed controls interface. This interface aids pipelines + * keeping tack of controls that needs time to take effect. The interface is + * optional to use and does not interact with setControls() and getControls() + * that operates directly on the sensor device. + * + * \sa DelayedControls + * \return The delayed controls interface + */ +DelayedControls *CameraSensor::delayedContols() +{ + if (!delayedControls_) { + /* + * \todo Read dealy values from the sensor itself or from a + * a sensor database. For now use generic values taken from + * the Raspberry Pi and listed as generic values. + */ + std::unordered_map delays = { + { V4L2_CID_ANALOGUE_GAIN, 1 }, + { V4L2_CID_EXPOSURE, 2 }, + }; + + delayedControls_ = + std::make_unique(subdev_.get(), delays); + } + + return delayedControls_.get(); +} + /** * \brief Assemble and return the camera sensor info * \param[out] info The camera sensor info From patchwork Wed Oct 28 01:00:49 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 10280 X-Patchwork-Delegate: niklas.soderlund@ragnatech.se Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 7DE19C3B5C for ; Wed, 28 Oct 2020 01:01:20 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3E9AD6226D; Wed, 28 Oct 2020 02:01:20 +0100 (CET) Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3BAB96225C for ; Wed, 28 Oct 2020 02:01:17 +0100 (CET) X-Halon-ID: 14add907-18b9-11eb-954c-0050569116f7 Authorized-sender: niklas.soderlund@fsdn.se Received: from bismarck.berto.se (p4fca2458.dip0.t-ipconnect.de [79.202.36.88]) by bin-vsp-out-03.atm.binero.net (Halon) with ESMTPA id 14add907-18b9-11eb-954c-0050569116f7; Wed, 28 Oct 2020 02:01:16 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org, david.plowman@raspberrypi.com, naush@raspberrypi.com Date: Wed, 28 Oct 2020 02:00:49 +0100 Message-Id: <20201028010051.3830668-8-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.29.1 In-Reply-To: <20201028010051.3830668-1-niklas.soderlund@ragnatech.se> References: <20201028010051.3830668-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 7/9] libcamera: pipeline: rkisp1: Use CameraSensor and delayed controls X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Instead of setting controls using the RkISP1 local Timeline helper use the DelayedControls interface provided by the CameraSensor. The result are the same, the controls are applied with a delay. The values of the delays are however different between the two methods. The values used in the Timeline solution was chosen after some experimentation and the values used in DelayedControls are taken from a generic sensor. None of the two are a perfect match as the delays can be different for different sensors used with the pipeline. However using the interface provided by CameraSensor we prepare for the future where sensor specific delays will provided by the CameraSensor and used without any change in the pipeline. Signed-off-by: Niklas Söderlund --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 35 +++++++++--------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index c74a2e9bd5482057..d0a8be06656535a3 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -121,8 +121,9 @@ class RkISP1CameraData : public CameraData public: RkISP1CameraData(PipelineHandler *pipe, RkISP1MainPath *mainPath, RkISP1SelfPath *selfPath) - : CameraData(pipe), sensor_(nullptr), frame_(0), - frameInfo_(pipe), mainPath_(mainPath), selfPath_(selfPath) + : CameraData(pipe), sensor_(nullptr), delayedCtrls_(nullptr), + frame_(0), frameInfo_(pipe), mainPath_(mainPath), + selfPath_(selfPath) { } @@ -136,6 +137,7 @@ public: Stream mainPathStream_; Stream selfPathStream_; CameraSensor *sensor_; + DelayedControls *delayedCtrls_; unsigned int frame_; std::vector ipaBuffers_; RkISP1Frames frameInfo_; @@ -346,23 +348,6 @@ RkISP1FrameInfo *RkISP1Frames::find(Request *request) return nullptr; } -class RkISP1ActionSetSensor : public FrameAction -{ -public: - RkISP1ActionSetSensor(unsigned int frame, CameraSensor *sensor, const ControlList &controls) - : FrameAction(frame, SetSensor), sensor_(sensor), controls_(controls) {} - -protected: - void run() override - { - sensor_->setControls(&controls_); - } - -private: - CameraSensor *sensor_; - ControlList controls_; -}; - class RkISP1ActionQueueBuffers : public FrameAction { public: @@ -430,9 +415,7 @@ void RkISP1CameraData::queueFrameAction(unsigned int frame, switch (action.operation) { case RKISP1_IPA_ACTION_V4L2_SET: { const ControlList &controls = action.controls[0]; - timeline_.scheduleAction(std::make_unique(frame, - sensor_, - controls)); + delayedCtrls_->push(controls); break; } case RKISP1_IPA_ACTION_PARAM_FILLED: { @@ -896,6 +879,8 @@ int PipelineHandlerRkISP1::start(Camera *camera) }; } + isp_->setFrameStartEnabled(true); + activeCamera_ = camera; /* Inform IPA of stream configuration and sensor controls. */ @@ -923,6 +908,8 @@ void PipelineHandlerRkISP1::stop(Camera *camera) RkISP1CameraData *data = cameraData(camera); int ret; + isp_->setFrameStartEnabled(false); + selfPath_.stop(); mainPath_.stop(); @@ -1041,6 +1028,10 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) /* Initialize the camera properties. */ data->properties_ = data->sensor_->properties(); + data->delayedCtrls_ = data->sensor_->delayedContols(); + isp_->frameStart.connect(data->delayedCtrls_, + &DelayedControls::frameStart); + ret = data->loadIPA(); if (ret) return ret; From patchwork Wed Oct 28 01:00:50 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 10281 X-Patchwork-Delegate: niklas.soderlund@ragnatech.se Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id CC4F7C3B60 for ; Wed, 28 Oct 2020 01:01:20 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 823F462284; Wed, 28 Oct 2020 02:01:20 +0100 (CET) Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 34DB26225C for ; Wed, 28 Oct 2020 02:01:18 +0100 (CET) X-Halon-ID: 15350d2e-18b9-11eb-954c-0050569116f7 Authorized-sender: niklas.soderlund@fsdn.se Received: from bismarck.berto.se (p4fca2458.dip0.t-ipconnect.de [79.202.36.88]) by bin-vsp-out-03.atm.binero.net (Halon) with ESMTPA id 15350d2e-18b9-11eb-954c-0050569116f7; Wed, 28 Oct 2020 02:01:17 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org, david.plowman@raspberrypi.com, naush@raspberrypi.com Date: Wed, 28 Oct 2020 02:00:50 +0100 Message-Id: <20201028010051.3830668-9-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.29.1 In-Reply-To: <20201028010051.3830668-1-niklas.soderlund@ragnatech.se> References: <20201028010051.3830668-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 8/9] libcamera: pipeline: rkisp1: Use SOF event to warn about late parameters X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" In the Timeline approach the idea was to delay queuing buffers to the device until the IPA had a chance to prepare the parameters buffer. A check was still added to warn if the IPA queued buffers before the parameters buffer was filled in. This check happened much sooner then needed as the parameter buffers does not have to be ready when the buffer is queued but just before its consumed. As the pipeline now has true knowledge of each SOF we can move the check there and remove the delaying of queuing of buffers. This change really speeds up the IPA reactions as the delays used in the Timeline where approximated while with this change they are driven by events reported by the device. Signed-off-by: Niklas Söderlund --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 78 ++++++++---------------- 1 file changed, 24 insertions(+), 54 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index d0a8be06656535a3..552c9ebbd5617e59 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -40,7 +40,6 @@ namespace libcamera { LOG_DEFINE_CATEGORY(RkISP1) class PipelineHandlerRkISP1; -class RkISP1ActionQueueBuffers; class RkISP1CameraData; enum RkISP1ActionType { @@ -203,7 +202,6 @@ private: PipelineHandler::cameraData(camera)); } - friend RkISP1ActionQueueBuffers; friend RkISP1CameraData; friend RkISP1Frames; @@ -214,6 +212,7 @@ private: void bufferReady(FrameBuffer *buffer); void paramReady(FrameBuffer *buffer); void statReady(FrameBuffer *buffer); + void frameStart(uint32_t sequence); int allocateBuffers(Camera *camera); int freeBuffers(Camera *camera); @@ -348,53 +347,6 @@ RkISP1FrameInfo *RkISP1Frames::find(Request *request) return nullptr; } -class RkISP1ActionQueueBuffers : public FrameAction -{ -public: - RkISP1ActionQueueBuffers(unsigned int frame, RkISP1CameraData *data, - PipelineHandlerRkISP1 *pipe) - : FrameAction(frame, QueueBuffers), data_(data), pipe_(pipe) - { - } - -protected: - void run() override - { - RkISP1FrameInfo *info = data_->frameInfo_.find(frame()); - if (!info) - LOG(RkISP1, Fatal) << "Frame not known"; - - /* - * \todo: If parameters are not filled a better method to handle - * the situation than queuing a buffer with unknown content - * should be used. - * - * It seems excessive to keep an internal zeroed scratch - * parameters buffer around as this should not happen unless the - * devices is under too much load. Perhaps failing the request - * and returning it to the application with an error code is - * better than queue it to hardware? - */ - if (!info->paramFilled) - LOG(RkISP1, Error) - << "Parameters not ready on time for frame " - << frame(); - - pipe_->param_->queueBuffer(info->paramBuffer); - pipe_->stat_->queueBuffer(info->statBuffer); - - if (info->mainPathBuffer) - pipe_->mainPath_.queueBuffer(info->mainPathBuffer); - - if (info->selfPathBuffer) - pipe_->selfPath_.queueBuffer(info->selfPathBuffer); - } - -private: - RkISP1CameraData *data_; - PipelineHandlerRkISP1 *pipe_; -}; - int RkISP1CameraData::loadIPA() { ipa_ = IPAManager::createIPA(pipe_, 1, 1); @@ -948,9 +900,14 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request) op.controls = { request->controls() }; data->ipa_->processEvent(op); - data->timeline_.scheduleAction(std::make_unique(data->frame_, - data, - this)); + param_->queueBuffer(info->paramBuffer); + stat_->queueBuffer(info->statBuffer); + + if (info->mainPathBuffer) + mainPath_.queueBuffer(info->mainPathBuffer); + + if (info->selfPathBuffer) + selfPath_.queueBuffer(info->selfPathBuffer); data->frame_++; @@ -1088,6 +1045,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) mainPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::bufferReady); selfPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::bufferReady); stat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady); + isp_->frameStart.connect(this, &PipelineHandlerRkISP1::frameStart); param_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady); /* @@ -1167,8 +1125,6 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer) if (!info) return; - data->timeline_.bufferReady(buffer); - if (data->frame_ <= buffer->metadata().sequence) data->frame_ = buffer->metadata().sequence + 1; @@ -1178,6 +1134,20 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer) data->ipa_->processEvent(op); } +void PipelineHandlerRkISP1::frameStart(uint32_t sequence) +{ + if (!activeCamera_) + return; + + RkISP1CameraData *data = cameraData(activeCamera_); + + RkISP1FrameInfo *info = data->frameInfo_.find(sequence); + if (!info || !info->paramFilled) + LOG(RkISP1, Info) + << "Parameters not ready on time for frame " + << sequence; +} + REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1) } /* namespace libcamera */ From patchwork Wed Oct 28 01:00:51 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 10282 X-Patchwork-Delegate: niklas.soderlund@ragnatech.se Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 226C7C3B5C for ; Wed, 28 Oct 2020 01:01:24 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id EE00E6225B; Wed, 28 Oct 2020 02:01:23 +0100 (CET) Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8233B62277 for ; Wed, 28 Oct 2020 02:01:19 +0100 (CET) X-Halon-ID: 15c9d570-18b9-11eb-954c-0050569116f7 Authorized-sender: niklas.soderlund@fsdn.se Received: from bismarck.berto.se (p4fca2458.dip0.t-ipconnect.de [79.202.36.88]) by bin-vsp-out-03.atm.binero.net (Halon) with ESMTPA id 15c9d570-18b9-11eb-954c-0050569116f7; Wed, 28 Oct 2020 02:01:18 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org, david.plowman@raspberrypi.com, naush@raspberrypi.com Date: Wed, 28 Oct 2020 02:00:51 +0100 Message-Id: <20201028010051.3830668-10-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.29.1 In-Reply-To: <20201028010051.3830668-1-niklas.soderlund@ragnatech.se> References: <20201028010051.3830668-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 9/9] libcamera: pipeline: rkisp1: Remove Timeline X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" There are no users left of the Timeline and there is no longer a need to keep emulating a start of exposure event as the CSI-2 resciver reports it. Remove the Timeline helper and what's left of it's integration in the pipeline. There is no functional change as nothing i the pipeline uses the Timeline. Signed-off-by: Niklas Söderlund --- src/libcamera/pipeline/rkisp1/meson.build | 1 - src/libcamera/pipeline/rkisp1/rkisp1.cpp | 45 ---- src/libcamera/pipeline/rkisp1/timeline.cpp | 227 --------------------- src/libcamera/pipeline/rkisp1/timeline.h | 72 ------- 4 files changed, 345 deletions(-) delete mode 100644 src/libcamera/pipeline/rkisp1/timeline.cpp delete mode 100644 src/libcamera/pipeline/rkisp1/timeline.h diff --git a/src/libcamera/pipeline/rkisp1/meson.build b/src/libcamera/pipeline/rkisp1/meson.build index 5cd40d949d543fa9..cad66535c25f8cc0 100644 --- a/src/libcamera/pipeline/rkisp1/meson.build +++ b/src/libcamera/pipeline/rkisp1/meson.build @@ -3,5 +3,4 @@ libcamera_sources += files([ 'rkisp1.cpp', 'rkisp1_path.cpp', - 'timeline.cpp', ]) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 552c9ebbd5617e59..424099d087528fb1 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -33,7 +33,6 @@ #include "libcamera/internal/v4l2_videodevice.h" #include "rkisp1_path.h" -#include "timeline.h" namespace libcamera { @@ -42,12 +41,6 @@ LOG_DEFINE_CATEGORY(RkISP1) class PipelineHandlerRkISP1; class RkISP1CameraData; -enum RkISP1ActionType { - SetSensor, - SOE, - QueueBuffers, -}; - struct RkISP1FrameInfo { unsigned int frame; Request *request; @@ -80,41 +73,6 @@ private: std::map frameInfo_; }; -class RkISP1Timeline : public Timeline -{ -public: - RkISP1Timeline() - : Timeline() - { - setDelay(SetSensor, -1, 5); - setDelay(SOE, 0, -1); - setDelay(QueueBuffers, -1, 10); - } - - void bufferReady(FrameBuffer *buffer) - { - /* - * Calculate SOE by taking the end of DMA set by the kernel and applying - * the time offsets provideprovided by the IPA to find the best estimate - * of SOE. - */ - - ASSERT(frameOffset(SOE) == 0); - - utils::time_point soe = std::chrono::time_point() - + std::chrono::nanoseconds(buffer->metadata().timestamp) - + timeOffset(SOE); - - notifyStartOfExposure(buffer->metadata().sequence, soe); - } - - void setDelay(unsigned int type, int frame, int msdelay) - { - utils::duration delay = std::chrono::milliseconds(msdelay); - setRawDelay(type, frame, delay); - } -}; - class RkISP1CameraData : public CameraData { public: @@ -140,7 +98,6 @@ public: unsigned int frame_; std::vector ipaBuffers_; RkISP1Frames frameInfo_; - RkISP1Timeline timeline_; RkISP1MainPath *mainPath_; RkISP1SelfPath *selfPath_; @@ -877,8 +834,6 @@ void PipelineHandlerRkISP1::stop(Camera *camera) data->ipa_->stop(); - data->timeline_.reset(); - data->frameInfo_.clear(); freeBuffers(camera); diff --git a/src/libcamera/pipeline/rkisp1/timeline.cpp b/src/libcamera/pipeline/rkisp1/timeline.cpp deleted file mode 100644 index 6b83bbe5b5892531..0000000000000000 --- a/src/libcamera/pipeline/rkisp1/timeline.cpp +++ /dev/null @@ -1,227 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -/* - * Copyright (C) 2019, Google Inc. - * - * timeline.cpp - Timeline for per-frame control - */ - -#include "timeline.h" - -#include "libcamera/internal/log.h" - -/** - * \file timeline.h - * \brief Timeline for per-frame control - */ - -namespace libcamera { - -LOG_DEFINE_CATEGORY(Timeline) - -/** - * \class FrameAction - * \brief Action that can be schedule on a Timeline - * - * A frame action is an event schedule to be executed on a Timeline. A frame - * action has two primal attributes a frame number and a type. - * - * The frame number describes the frame to which the action is associated. The - * type is a numerical ID which identifies the action within the pipeline and - * IPA protocol. - */ - -/** - * \class Timeline - * \brief Executor of FrameAction - * - * The timeline has three primary functions: - * - * 1. Keep track of the Start of Exposure (SOE) for every frame processed by - * the hardware. Using this information it shall keep an up-to-date estimate - * of the frame interval (time between two consecutive SOE events). - * - * The estimated frame interval together with recorded SOE events are the - * foundation for how the timeline schedule FrameAction at specific points - * in time. - * \todo Improve the frame interval estimation algorithm. - * - * 2. Keep track of current delays for different types of actions. The delays - * for different actions might differ during a capture session. Exposure time - * effects the over all FPS and different ISP parameters might impacts its - * processing time. - * - * The action type delays shall be updated by the IPA in conjunction with - * how it changes the capture parameters. - * - * 3. Schedule actions on the timeline. This is the process of taking a - * FrameAction which contains an abstract description of what frame and - * what type of action it contains and turning that into an time point - * and make sure the action is executed at that time. - */ - -Timeline::Timeline() - : frameInterval_(0) -{ - timer_.timeout.connect(this, &Timeline::timeout); -} - -/** - * \brief Reset and stop the timeline - * - * The timeline needs to be reset when the timeline should no longer execute - * actions. A timeline should be reset between two capture sessions to prevent - * the old capture session to effect the second one. - */ -void Timeline::reset() -{ - timer_.stop(); - - actions_.clear(); - history_.clear(); -} - -/** - * \brief Schedule an action on the timeline - * \param[in] action FrameAction to schedule - * - * The act of scheduling an action to the timeline is the process of taking - * the properties of the action (type, frame and time offsets) and translating - * that to a time point using the current values for the action type timings - * value recorded in the timeline. If an action is scheduled too late, execute - * it immediately. - */ -void Timeline::scheduleAction(std::unique_ptr action) -{ - unsigned int lastFrame; - utils::time_point lastTime; - - if (history_.empty()) { - lastFrame = 0; - lastTime = std::chrono::steady_clock::now(); - } else { - lastFrame = history_.back().first; - lastTime = history_.back().second; - } - - /* - * Calculate when the action shall be schedule by first finding out how - * many frames in the future the action acts on and then add the actions - * frame offset. After the spatial frame offset is found out translate - * that to a time point by using the last estimated start of exposure - * (SOE) as the fixed offset. Lastly add the action time offset to the - * time point. - */ - int frame = action->frame() - lastFrame + frameOffset(action->type()); - utils::time_point deadline = lastTime + frame * frameInterval_ - + timeOffset(action->type()); - - utils::time_point now = std::chrono::steady_clock::now(); - if (deadline < now) { - LOG(Timeline, Warning) - << "Action scheduled too late " - << utils::time_point_to_string(deadline) - << ", run now " << utils::time_point_to_string(now); - action->run(); - } else { - actions_.emplace(deadline, std::move(action)); - updateDeadline(); - } -} - -void Timeline::notifyStartOfExposure(unsigned int frame, utils::time_point time) -{ - history_.push_back(std::make_pair(frame, time)); - - if (history_.size() <= HISTORY_DEPTH / 2) - return; - - while (history_.size() > HISTORY_DEPTH) - history_.pop_front(); - - /* Update esitmated time between two start of exposures. */ - utils::duration sumExposures(0); - unsigned int numExposures = 0; - - utils::time_point lastTime; - for (auto it = history_.begin(); it != history_.end(); it++) { - if (it != history_.begin()) { - sumExposures += it->second - lastTime; - numExposures++; - } - - lastTime = it->second; - } - - frameInterval_ = sumExposures; - if (numExposures) - frameInterval_ /= numExposures; -} - -int Timeline::frameOffset(unsigned int type) const -{ - const auto it = delays_.find(type); - if (it == delays_.end()) { - LOG(Timeline, Error) - << "No frame offset set for action type " << type; - return 0; - } - - return it->second.first; -} - -utils::duration Timeline::timeOffset(unsigned int type) const -{ - const auto it = delays_.find(type); - if (it == delays_.end()) { - LOG(Timeline, Error) - << "No time offset set for action type " << type; - return utils::duration::zero(); - } - - return it->second.second; -} - -void Timeline::setRawDelay(unsigned int type, int frame, utils::duration time) -{ - delays_[type] = std::make_pair(frame, time); -} - -void Timeline::updateDeadline() -{ - if (actions_.empty()) - return; - - const utils::time_point &deadline = actions_.begin()->first; - - if (timer_.isRunning() && deadline >= timer_.deadline()) - return; - - if (deadline <= std::chrono::steady_clock::now()) { - timeout(&timer_); - return; - } - - timer_.start(deadline); -} - -void Timeline::timeout([[maybe_unused]] Timer *timer) -{ - utils::time_point now = std::chrono::steady_clock::now(); - - for (auto it = actions_.begin(); it != actions_.end();) { - const utils::time_point &sched = it->first; - - if (sched > now) - break; - - FrameAction *action = it->second.get(); - - action->run(); - - it = actions_.erase(it); - } - - updateDeadline(); -} - -} /* namespace libcamera */ diff --git a/src/libcamera/pipeline/rkisp1/timeline.h b/src/libcamera/pipeline/rkisp1/timeline.h deleted file mode 100644 index 0c37b06fa6dfe14c..0000000000000000 --- a/src/libcamera/pipeline/rkisp1/timeline.h +++ /dev/null @@ -1,72 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -/* - * Copyright (C) 2019, Google Inc. - * - * timeline.h - Timeline for per-frame controls - */ -#ifndef __LIBCAMERA_TIMELINE_H__ -#define __LIBCAMERA_TIMELINE_H__ - -#include -#include - -#include - -#include "libcamera/internal/utils.h" - -namespace libcamera { - -class FrameAction -{ -public: - FrameAction(unsigned int frame, unsigned int type) - : frame_(frame), type_(type) {} - - virtual ~FrameAction() = default; - - unsigned int frame() const { return frame_; } - unsigned int type() const { return type_; } - - virtual void run() = 0; - -private: - unsigned int frame_; - unsigned int type_; -}; - -class Timeline -{ -public: - Timeline(); - virtual ~Timeline() = default; - - virtual void reset(); - virtual void scheduleAction(std::unique_ptr action); - virtual void notifyStartOfExposure(unsigned int frame, utils::time_point time); - - utils::duration frameInterval() const { return frameInterval_; } - -protected: - int frameOffset(unsigned int type) const; - utils::duration timeOffset(unsigned int type) const; - - void setRawDelay(unsigned int type, int frame, utils::duration time); - - std::map> delays_; - -private: - static constexpr unsigned int HISTORY_DEPTH = 10; - - void timeout(Timer *timer); - void updateDeadline(); - - std::list> history_; - std::multimap> actions_; - utils::duration frameInterval_; - - Timer timer_; -}; - -} /* namespace libcamera */ - -#endif /* __LIBCAMERA_TIMELINE_H__ */