From patchwork Sat Jan 16 11:47:58 2021 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: 10875 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 E7A46BD808 for ; Sat, 16 Jan 2021 11:48:18 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C1C0268112; Sat, 16 Jan 2021 12:48:18 +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 C3ABD68105 for ; Sat, 16 Jan 2021 12:48:16 +0100 (CET) X-Halon-ID: b76b9fc0-57f0-11eb-a076-005056917f90 Authorized-sender: niklas.soderlund@fsdn.se Received: from bismarck.berto.se (p4fca2458.dip0.t-ipconnect.de [79.202.36.88]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id b76b9fc0-57f0-11eb-a076-005056917f90; Sat, 16 Jan 2021 12:48:15 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Sat, 16 Jan 2021 12:47:58 +0100 Message-Id: <20210116114805.905397-2-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.30.0 In-Reply-To: <20210116114805.905397-1-niklas.soderlund@ragnatech.se> References: <20210116114805.905397-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v5 1/8] libcamera: delayed_controls: Add helper for controls that apply 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 an 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 through 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 Reviewed-by: Jacopo Mondi Reviewed-by: Naushir Patuck --- * Changes since v4 - Fold queue() into push() as the mutex was dropped in v3 having a public accessors that takes the mutex is no longer needed. - Add delayed_controls.h to meson.build. - Update patch subject and commit message. - Have class Info inherit from ControlValue. - Add todo to reevaluate if maps should be indexed on ControlID or unsigned int. - Update documentation. * Changes since v3 - Drop optional argument to reset(). - Update commit message. - Remove usage of Mutex. - Rename frameStart() to applyControls)_. - Rename ControlInfo to Into. - Rename ControlArray to ControlRingBuffer. - Drop ControlsDelays and ControlsValues. - Sort headers. - Rename iterators. - Simplify queueCount_ handeling in reset(). - Add more warnings. - Update documentation. * Changes since v2 - Improve error logic in queue() as suggested by Jean-Michel Hautbois. - s/fistSequence_/firstSequence_/ * 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 | 81 ++++++ include/libcamera/internal/meson.build | 1 + src/libcamera/delayed_controls.cpp | 247 ++++++++++++++++++ src/libcamera/meson.build | 1 + 4 files changed, 330 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..dc447a882514182f --- /dev/null +++ b/include/libcamera/internal/delayed_controls.h @@ -0,0 +1,81 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2020, Raspberry Pi (Trading) Ltd. + * + * delayed_controls.h - Helper to deal with controls that take effect with a delay + */ +#ifndef __LIBCAMERA_INTERNAL_DELAYED_CONTROLS_H__ +#define __LIBCAMERA_INTERNAL_DELAYED_CONTROLS_H__ + +#include +#include + +#include + +namespace libcamera { + +class V4L2Device; + +class DelayedControls +{ +public: + DelayedControls(V4L2Device *device, + const std::unordered_map &delays); + + void reset(); + + bool push(const ControlList &controls); + ControlList get(uint32_t sequence); + + void applyControls(uint32_t sequence); + +private: + class Info : public ControlValue + { + public: + Info() + : updated(false) + { + } + + Info(const ControlValue &v) + : ControlValue(v), updated(true) + { + } + + bool updated; + }; + + /* \todo: Make the listSize configurable at instance creation time. */ + static constexpr int listSize = 16; + class ControlRingBuffer : public std::array + { + public: + Info &operator[](unsigned int index) + { + return std::array::operator[](index % listSize); + } + + const Info &operator[](unsigned int index) const + { + return std::array::operator[](index % listSize); + } + }; + + V4L2Device *device_; + /* \todo Evaluate if we should index on ControlId * or unsigned int */ + std::unordered_map delays_; + unsigned int maxDelay_; + + bool running_; + uint32_t firstSequence_; + + uint32_t queueCount_; + uint32_t writeCount_; + /* \todo Evaluate if we should index on ControlId * or unsigned int */ + std::unordered_map values_; +}; + +} /* namespace libcamera */ + +#endif /* __LIBCAMERA_INTERNAL_DELAYED_CONTROLS_H__ */ diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build index 1b1bdc77951235a5..e67a359fb8656f71 100644 --- a/include/libcamera/internal/meson.build +++ b/include/libcamera/internal/meson.build @@ -17,6 +17,7 @@ libcamera_internal_headers = files([ 'camera_sensor.h', 'control_serializer.h', 'control_validator.h', + 'delayed_controls.h', 'device_enumerator.h', 'device_enumerator_sysfs.h', 'device_enumerator_udev.h', diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp new file mode 100644 index 0000000000000000..f24db43d7f1e357d --- /dev/null +++ b/src/libcamera/delayed_controls.cpp @@ -0,0 +1,247 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2020, Raspberry Pi (Trading) Ltd. + * + * delayed_controls.h - Helper to deal with controls that take effect with a delay + */ + +#include "libcamera/internal/delayed_controls.h" + +#include + +#include "libcamera/internal/log.h" +#include "libcamera/internal/v4l2_device.h" + +/** + * \file delayed_controls.h + * \brief Helper to deal with controls that take effect 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 analog gain. This is a 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 needs 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 greatest + * delay. + */ + +/** + * \brief Construct a DelayedControls instance + * \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 &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 it = controls.find(delay.first); + if (it == 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 = it->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 + * + * Resets the state machine to a starting position based on control values + * retrieved from the device. + */ +void DelayedControls::reset() +{ + running_ = false; + firstSequence_ = 0; + queueCount_ = 1; + writeCount_ = 0; + + /* Retrieve control as reported by the device. */ + std::vector ids; + for (auto const &delay : delays_) + ids.push_back(delay.first->id()); + + ControlList controls = device_->getControls(ids); + + /* Seed the control queue with the controls reported by the device. */ + values_.clear(); + for (const auto &ctrl : controls) { + const ControlId *id = device_->controls().idmap().at(ctrl.first); + values_[id][0] = Info(ctrl.second); + } +} + +/** + * \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) +{ + /* Copy state from previous frame. */ + for (auto &ctrl : values_) { + Info &info = ctrl.second[queueCount_]; + info = values_[ctrl.first][queueCount_ - 1]; + info.updated = false; + } + + /* Update with new controls. */ + const ControlIdMap &idmap = device_->controls().idmap(); + for (const auto &control : controls) { + const auto &it = idmap.find(control.first); + if (it == idmap.end()) { + LOG(DelayedControls, Warning) + << "Unknown control " << control.first; + return false; + } + + const ControlId *id = it->second; + + if (delays_.find(id) == delays_.end()) + return false; + + Info &info = values_[id][queueCount_]; + + info = control.second; + info.updated = true; + + LOG(DelayedControls, Debug) + << "Queuing " << id->name() + << " to " << info.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 too 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) +{ + uint32_t adjustedSeq = sequence - firstSequence_ + 1; + unsigned int index = std::max(0, adjustedSeq - maxDelay_); + + ControlList out(device_->controls()); + for (const auto &ctrl : values_) { + const ControlId *id = ctrl.first; + const Info &info = ctrl.second[index]; + + out.set(id->id(), info); + + LOG(DelayedControls, Debug) + << "Reading " << id->name() + << " to " << info.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::applyControls(uint32_t sequence) +{ + LOG(DelayedControls, Debug) << "frame " << sequence << " started"; + + if (!running_) { + firstSequence_ = 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 : values_) { + const ControlId *id = ctrl.first; + unsigned int delayDiff = maxDelay_ - delays_[id]; + unsigned int index = std::max(0, writeCount_ - delayDiff); + const Info &info = ctrl.second[index]; + + if (info.updated) { + out.set(id->id(), info); + LOG(DelayedControls, Debug) + << "Setting " << id->name() + << " to " << info.toString() + << " at index " << index; + } + } + + writeCount_++; + + while (writeCount_ >= queueCount_) { + LOG(DelayedControls, Debug) + << "Queue is empty, auto queue no-op."; + push({}); + } + + device_->setControls(&out); +} + +} /* 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', From patchwork Sat Jan 16 11:47:59 2021 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: 10876 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 58B8EBD808 for ; Sat, 16 Jan 2021 11:48:20 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 31B6568107; Sat, 16 Jan 2021 12:48: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 DA80D68105 for ; Sat, 16 Jan 2021 12:48:17 +0100 (CET) X-Halon-ID: b881d676-57f0-11eb-a076-005056917f90 Authorized-sender: niklas.soderlund@fsdn.se Received: from bismarck.berto.se (p4fca2458.dip0.t-ipconnect.de [79.202.36.88]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id b881d676-57f0-11eb-a076-005056917f90; Sat, 16 Jan 2021 12:48:16 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Sat, 16 Jan 2021 12:47:59 +0100 Message-Id: <20210116114805.905397-3-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.30.0 In-Reply-To: <20210116114805.905397-1-niklas.soderlund@ragnatech.se> References: <20210116114805.905397-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v5 2/8] 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 exercises the setting of controls and reading back what controls were used for a particular frame. Also exercise corner cases such as a V4L2 device that does not reset its sequence number to 0 at stream on and sequence number wrapping around the uint32_t value space. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- * Changes since v4 - Store dev_ as std::unique_ptr<>. - Update comments. - Switch from uint32_t to unsigned int - Step brightness and contrast test 1 step apart. - s/int32_t/unsigned int/ for for loops in singleControlNoDelay(). * Changes since v3 - Update commit message. * Changes since v3 - Update commit message. - Update header documentation. - Add protected and override. - Use unsigned int instead of int32_t in loops. * Changes since v2 - Remove a unused LoC. - Add and remove blank lines. - Align number of loop iterations. --- test/delayed_contols.cpp | 297 +++++++++++++++++++++++++++++++++++++++ test/meson.build | 1 + 2 files changed, 298 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..50169b12e5664020 --- /dev/null +++ b/test/delayed_contols.cpp @@ -0,0 +1,297 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2020, Google Inc. + * + * delayed_controls.cpp - libcamera delayed controls test + */ + +#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() + { + } + +protected: + int init() override + { + 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; + } + + dev_ = V4L2VideoDevice::fromEntityName(media_.get(), "vivid-000-vid-cap"); + if (dev_->open()) { + cerr << "Failed to open video device" << endl; + return TestFail; + } + + const ControlInfoMap &infoMap = dev_->controls(); + + /* Make sure the controls we require are present. */ + 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; + } + + return TestPass; + } + + int singleControlNoDelay() + { + std::unordered_map delays = { + { V4L2_CID_BRIGHTNESS, 0 }, + }; + std::unique_ptr delayed = + std::make_unique(dev_.get(), delays); + ControlList ctrls; + + /* Reset control to value not used in test. */ + ctrls.set(V4L2_CID_BRIGHTNESS, 1); + dev_->setControls(&ctrls); + + /* Test control without delay are set at once. */ + for (unsigned int i = 0; i < 100; i++) { + int32_t value = 100 + i; + + ctrls.set(V4L2_CID_BRIGHTNESS, value); + delayed->push(ctrls); + + delayed->applyControls(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_.get(), delays); + ControlList ctrls; + + /* Reset control to value that will be first in test. */ + int32_t expected = 4; + ctrls.set(V4L2_CID_BRIGHTNESS, expected); + dev_->setControls(&ctrls); + delayed->reset(); + + /* Test single control with delay. */ + for (unsigned int i = 0; i < 100; i++) { + int32_t value = 10 + i; + + ctrls.set(V4L2_CID_BRIGHTNESS, value); + delayed->push(ctrls); + + delayed->applyControls(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_.get(), 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 + 1); + dev_->setControls(&ctrls); + delayed->reset(); + + /* Test dual control with delay. */ + for (unsigned int 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 + 1); + delayed->push(ctrls); + + delayed->applyControls(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 + 1) { + 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_.get(), 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); + dev_->setControls(&ctrls); + delayed->reset(); + + /* + * 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 (unsigned int 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 (unsigned int i = 0; i < 16; i++) { + int32_t value = 10 + i; + + delayed->applyControls(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() override + { + 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 than consumed. */ + ret = dualControlsMultiQueue(); + if (ret) + return ret; + + return TestPass; + } + +private: + std::unique_ptr enumerator_; + std::shared_ptr media_; + std::unique_ptr dev_; +}; + +TEST_REGISTER(DelayedControlsTest) diff --git a/test/meson.build b/test/meson.build index 7f0682ad9efed8f4..065c55866356ff1c 100644 --- a/test/meson.build +++ b/test/meson.build @@ -29,6 +29,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 Sat Jan 16 11:48:00 2021 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: 10877 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 E06B4BD808 for ; Sat, 16 Jan 2021 11:48:20 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 585106811A; Sat, 16 Jan 2021 12:48: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 5182960109 for ; Sat, 16 Jan 2021 12:48:19 +0100 (CET) X-Halon-ID: b93207ca-57f0-11eb-a076-005056917f90 Authorized-sender: niklas.soderlund@fsdn.se Received: from bismarck.berto.se (p4fca2458.dip0.t-ipconnect.de [79.202.36.88]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id b93207ca-57f0-11eb-a076-005056917f90; Sat, 16 Jan 2021 12:48:18 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Sat, 16 Jan 2021 12:48:00 +0100 Message-Id: <20210116114805.905397-4-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.30.0 In-Reply-To: <20210116114805.905397-1-niklas.soderlund@ragnatech.se> References: <20210116114805.905397-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v5 3/8] 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 Reviewed-by: Jacopo Mondi Reviewed-by: Naushir Patuck Reviewed-by: Laurent Pinchart --- * Changes since v4 - Rename IPA_ACTION_V4L2_SET_STAGGERED to IPA_ACTION_SET_DELAYED_CTRLS. - Optimize copy of controls in PipelineHandlerRPi::start(). - Update comments and an error message. --- include/libcamera/ipa/raspberrypi.h | 2 +- src/ipa/raspberrypi/raspberrypi.cpp | 2 +- .../pipeline/raspberrypi/raspberrypi.cpp | 57 ++++++++----------- 3 files changed, 27 insertions(+), 34 deletions(-) diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h index 01fe5abce9a6072c..c0516cbe786a1636 100644 --- a/include/libcamera/ipa/raspberrypi.h +++ b/include/libcamera/ipa/raspberrypi.h @@ -26,7 +26,7 @@ enum ConfigParameters { }; enum Operations { - IPA_ACTION_V4L2_SET_STAGGERED = 1, + IPA_ACTION_SET_DELAYED_CTRLS = 1, IPA_ACTION_V4L2_SET_ISP, IPA_ACTION_STATS_METADATA_COMPLETE, IPA_ACTION_RUN_ISP, diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index 8af3d603a3ff64b9..2d8fd793654e402c 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -944,7 +944,7 @@ void IPARPi::processStats(unsigned int bufferId) applyAGC(&agcStatus, ctrls); IPAOperationData op; - op.operation = RPi::IPA_ACTION_V4L2_SET_STAGGERED; + op.operation = RPi::IPA_ACTION_SET_DELAYED_CTRLS; op.controls.emplace_back(ctrls); queueFrameAction.emit(0, op); } diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index f121328ee9a9b6f2..3613671aded0a8fd 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -27,6 +27,7 @@ #include "libcamera/internal/bayer_format.h" #include "libcamera/internal/buffer.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" @@ -37,7 +38,6 @@ #include "dma_heaps.h" #include "rpi_stream.h" -#include "staggered_ctrl.h" namespace libcamera { @@ -178,8 +178,7 @@ public: RPi::DmaHeap dmaHeap_; FileDescriptor lsTable_; - RPi::StaggeredCtrl staggeredCtrl_; - uint32_t expectedSequence_; + std::unique_ptr delayedCtrls_; bool sensorMetadata_; /* @@ -766,11 +765,9 @@ int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont } /* Apply any gain/exposure settings that the IPA may have passed back. */ - ASSERT(data->staggeredCtrl_); if (result.operation & RPi::IPA_CONFIG_SENSOR) { - const ControlList &ctrls = result.controls[0]; - if (!data->staggeredCtrl_.set(ctrls)) - LOG(RPI, Error) << "V4L2 staggered set failed"; + ControlList &ctrls = result.controls[0]; + data->unicam_[Unicam::Image].dev()->setControls(&ctrls); } if (result.operation & RPi::IPA_CONFIG_DROP_FRAMES) { @@ -802,13 +799,10 @@ int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(true); /* - * Write the last set of gain and exposure values to the camera before - * starting. First check that the staggered ctrl has been initialised - * by configure(). + * Reset the delayed controls with the gain and exposure values set by + * the IPA. */ - data->staggeredCtrl_.reset(); - data->staggeredCtrl_.write(); - data->expectedSequence_ = 0; + data->delayedCtrls_->reset(); data->state_ = RPiCameraData::State::Idle; @@ -1147,7 +1141,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_->applyControls(sequence); } int RPiCameraData::loadIPA() @@ -1227,21 +1221,24 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config) unsigned int resultIdx = 0; if (result.operation & RPi::IPA_CONFIG_STAGGERED_WRITE) { /* - * Setup our staggered control writer with the sensor default + * Setup our delayed 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]; + unicam_[Unicam::Image].dev()->setControls(&ctrls); } /* @@ -1268,10 +1265,10 @@ void RPiCameraData::queueFrameAction([[maybe_unused]] unsigned int frame, * a stopped state. */ switch (action.operation) { - case RPi::IPA_ACTION_V4L2_SET_STAGGERED: { + case RPi::IPA_ACTION_SET_DELAYED_CTRLS: { const ControlList &controls = action.controls[0]; - if (!staggeredCtrl_.set(controls)) - LOG(RPI, Error) << "V4L2 staggered set failed"; + if (!delayedCtrls_->push(controls)) + LOG(RPI, Error) << "Failed to set delayed controls"; goto done; } @@ -1375,11 +1372,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 @@ -1391,8 +1384,8 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer) auto it = mappedEmbeddedBuffers_.find(bufferId); if (it != mappedEmbeddedBuffers_.end()) { uint32_t *mem = reinterpret_cast(it->second.maps()[0].data()); - 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(); } else { LOG(RPI, Warning) << "Failed to find embedded buffer " << bufferId; From patchwork Sat Jan 16 11:48:02 2021 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: 10878 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 DD747C0F1D for ; Sat, 16 Jan 2021 11:48:22 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id AAF2168123; Sat, 16 Jan 2021 12:48:22 +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 514706810C for ; Sat, 16 Jan 2021 12:48:21 +0100 (CET) X-Halon-ID: bac20099-57f0-11eb-a076-005056917f90 Authorized-sender: niklas.soderlund@fsdn.se Received: from bismarck.berto.se (p4fca2458.dip0.t-ipconnect.de [79.202.36.88]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id bac20099-57f0-11eb-a076-005056917f90; Sat, 16 Jan 2021 12:48:20 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Sat, 16 Jan 2021 12:48:02 +0100 Message-Id: <20210116114805.905397-6-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.30.0 In-Reply-To: <20210116114805.905397-1-niklas.soderlund@ragnatech.se> References: <20210116114805.905397-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v5 5/8] libcamera: camera_sensor: Expose the camera device 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 device backing the CameraSensor instance. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- * Changes since v4 - Add a \todo * Changes since v3 - New, needed to be able to move the DelayedControls handling out of CameraSensor. --- include/libcamera/internal/camera_sensor.h | 2 ++ src/libcamera/camera_sensor.cpp | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h index f80d836161a564e7..a70e024aa327f69b 100644 --- a/include/libcamera/internal/camera_sensor.h +++ b/include/libcamera/internal/camera_sensor.h @@ -61,6 +61,8 @@ public: ControlList getControls(const std::vector &ids); int setControls(ControlList *ctrls); + V4L2Subdevice *device() { return subdev_.get(); } + const ControlList &properties() const { return properties_; } int sensorInfo(CameraSensorInfo *info) const; diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp index e786821d4ba22fb8..8c00e1f8289b88fe 100644 --- a/src/libcamera/camera_sensor.cpp +++ b/src/libcamera/camera_sensor.cpp @@ -514,6 +514,13 @@ ControlList CameraSensor::getControls(const std::vector &ids) return subdev_->getControls(ids); } +/** + * \fn CameraSensor::device() + * \brief Retrieve the camera sensor device + * \todo Remove this function by integrating DelayedControl with CameraSensor + * \return The camera sensor device + */ + /** * \fn CameraSensor::properties() * \brief Retrieve the camera sensor properties From patchwork Sat Jan 16 11:48:03 2021 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: 10879 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 47709BD808 for ; Sat, 16 Jan 2021 11:48:25 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1AD2468115; Sat, 16 Jan 2021 12:48:25 +0100 (CET) Received: from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net [195.74.38.227]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2F06D6810C for ; Sat, 16 Jan 2021 12:48:22 +0100 (CET) X-Halon-ID: bb3f8c42-57f0-11eb-a076-005056917f90 Authorized-sender: niklas.soderlund@fsdn.se Received: from bismarck.berto.se (p4fca2458.dip0.t-ipconnect.de [79.202.36.88]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id bb3f8c42-57f0-11eb-a076-005056917f90; Sat, 16 Jan 2021 12:48:21 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Sat, 16 Jan 2021 12:48:03 +0100 Message-Id: <20210116114805.905397-7-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.30.0 In-Reply-To: <20210116114805.905397-1-niklas.soderlund@ragnatech.se> References: <20210116114805.905397-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v5 6/8] libcamera: pipeline: rkisp1: Use 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. The result is 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 were 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. Signed-off-by: Niklas Söderlund Reviewed-by: Jacopo Mondi Reviewed-by: Laurent Pinchart --- * Changes since v3 - Rewrite to not use CameraSensor. --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 43 +++++++++++++----------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 5ce37febc1d7c11c..116d205b85cc146c 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -23,6 +23,7 @@ #include #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/log.h" @@ -131,6 +132,7 @@ public: Stream mainPathStream_; Stream selfPathStream_; std::unique_ptr sensor_; + std::unique_ptr delayedCtrls_; unsigned int frame_; std::vector ipaBuffers_; RkISP1Frames frameInfo_; @@ -340,23 +342,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: @@ -424,9 +409,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_.get(), - controls)); + delayedCtrls_->push(controls); break; } case RKISP1_IPA_ACTION_PARAM_FILLED: { @@ -893,6 +876,8 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c }; } + isp_->setFrameStartEnabled(true); + activeCamera_ = camera; /* Inform IPA of stream configuration and sensor controls. */ @@ -920,6 +905,8 @@ void PipelineHandlerRkISP1::stop(Camera *camera) RkISP1CameraData *data = cameraData(camera); int ret; + isp_->setFrameStartEnabled(false); + selfPath_.stop(); mainPath_.stop(); @@ -1038,6 +1025,22 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) /* Initialize the camera properties. */ data->properties_ = data->sensor_->properties(); + /* + * \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 }, + }; + + data->delayedCtrls_ = + std::make_unique(data->sensor_->device(), + delays); + isp_->frameStart.connect(data->delayedCtrls_.get(), + &DelayedControls::applyControls); + ret = data->loadIPA(); if (ret) return ret; From patchwork Sat Jan 16 11:48:04 2021 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: 10880 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 8A70EC0F1D for ; Sat, 16 Jan 2021 11:48:25 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5D60268129; Sat, 16 Jan 2021 12:48:25 +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 12D7E6810C for ; Sat, 16 Jan 2021 12:48:23 +0100 (CET) X-Halon-ID: bbc2f532-57f0-11eb-a076-005056917f90 Authorized-sender: niklas.soderlund@fsdn.se Received: from bismarck.berto.se (p4fca2458.dip0.t-ipconnect.de [79.202.36.88]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id bbc2f532-57f0-11eb-a076-005056917f90; Sat, 16 Jan 2021 12:48:22 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Sat, 16 Jan 2021 12:48:04 +0100 Message-Id: <20210116114805.905397-8-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.30.0 In-Reply-To: <20210116114805.905397-1-niklas.soderlund@ragnatech.se> References: <20210116114805.905397-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v5 7/8] 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 it's 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 Reviewed-by: Jacopo Mondi --- 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 116d205b85cc146c..ab555cf23a7b564b 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -41,7 +41,6 @@ namespace libcamera { LOG_DEFINE_CATEGORY(RkISP1) class PipelineHandlerRkISP1; -class RkISP1ActionQueueBuffers; class RkISP1CameraData; enum RkISP1ActionType { @@ -197,7 +196,6 @@ private: PipelineHandler::cameraData(camera)); } - friend RkISP1ActionQueueBuffers; friend RkISP1CameraData; friend RkISP1Frames; @@ -208,6 +206,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); @@ -342,53 +341,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); @@ -945,9 +897,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_++; @@ -1097,6 +1054,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); /* @@ -1175,8 +1133,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; @@ -1186,6 +1142,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 */