From patchwork Wed Mar 13 12:12:17 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 19708 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 CACCDC32A3 for ; Wed, 13 Mar 2024 12:12:58 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3570A62C9B; Wed, 13 Mar 2024 13:12:58 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="f75teChM"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 77E2162C95 for ; Wed, 13 Mar 2024 13:12:45 +0100 (CET) Received: from jasper.fritz.box (unknown [IPv6:2a00:6020:448c:6c00:9b07:31b5:38e1:e957]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D6EFFA8F; Wed, 13 Mar 2024 13:12:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1710331943; bh=KrTXobubEcNjNHEuv7yp0VY2NjrjW5FghfhtaFzy0pY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=f75teChM2Q9Cn66jNPd0/9CZEZmgt4peSi6n9HGSHZb2beds/fM2KsOlJYzHCEIMZ RwKdAXT37y8e1JSJd7hg7jkRxqiljscyc58AqPWlh2Cb36mE5aYA6+cyCsDPrJjF9p CrMaNAj5MO6EcQBZfPQtJE+YyaFQsBe8o1KJL/kA= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Subject: [PATCH v2 06/12] libcamera: delayed_controls: Rework delayed controls implementation Date: Wed, 13 Mar 2024 13:12:17 +0100 Message-Id: <20240313121223.138150-7-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.40.1 In-Reply-To: <20240313121223.138150-1-stefan.klug@ideasonboard.com> References: <20240313121223.138150-1-stefan.klug@ideasonboard.com> MIME-Version: 1.0 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: , Cc: Stefan Klug Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Functional changes: - Requests need to be queued for every frame - The startup phase is no longer treated as special case - Requests carry a sequence number, so that we can detect when the system gets out of sync - Controls for a given sequence number can be updated multiple times as long as they are not yet sent out - If it's too late, controls get delayed but they are not lost - Requests attached to frame 0 don't get lost Technical notes: A sourceSequence_ replaces the updated flag to be able to track from which frame the update stems. This is needed for the following use case: Assume e.g. On frame 10 an ExposureTime=42 is queued because the user wants manual exposure. Now the agc gets the stats for frame 8 (where auto regulation is still active) and pushes a new ExposureTime for frame 9. Frame 9 was already sent out, so it gets delayed to frame 11 (assuming a delay of 2 on ExposureTime). This would revert the request from frame 10. Taking the sourceSequence into account, the delayed request from frame 9 will be discarded, which is correct. Signed-off-by: Stefan Klug --- include/libcamera/internal/delayed_controls.h | 12 +- src/libcamera/delayed_controls.cpp | 207 ++++++++++++++---- 2 files changed, 174 insertions(+), 45 deletions(-) diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h index 4f8d2424..2738e8bf 100644 --- a/include/libcamera/internal/delayed_controls.h +++ b/include/libcamera/internal/delayed_controls.h @@ -30,25 +30,29 @@ public: void reset(); bool push(const ControlList &controls); + bool pushForFrame(uint32_t sequence, const ControlList &controls); ControlList get(uint32_t sequence); void applyControls(uint32_t sequence); private: + bool controlsAreQueuedForFrame(unsigned int frame, const ControlList &controls); + class Info : public ControlValue { public: Info() - : updated(false) + : sourceSequence_(0) { } - Info(const ControlValue &v, bool updated_ = true) - : ControlValue(v), updated(updated_) + Info(const ControlValue &v, std::optional sourceSequence = std::nullopt) + : ControlValue(v), sourceSequence_(sourceSequence) { } - bool updated; + /* The sequence id, this info stems from*/ + std::optional sourceSequence_; }; /* \todo Make the listSize configurable at instance creation time. */ diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index 86571cd4..59314388 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -115,8 +115,9 @@ DelayedControls::DelayedControls(V4L2Device *device, */ void DelayedControls::reset() { - queueIndex_ = 1; - writeIndex_ = 0; + queueIndex_ = 0; + /* Frames up to maxDelay_ will be based on sensor init values. */ + writeIndex_ = maxDelay_; /* Retrieve control as reported by the device. */ std::vector ids; @@ -133,26 +134,129 @@ void DelayedControls::reset() * Do not mark this control value as updated, it does not need * to be written to to device on startup. */ - values_[id][0] = Info(ctrl.second, false); + values_[id][0] = Info(ctrl.second, 0); } + + /* Copy state from previous frames. */ + for (auto &ctrl : values_) { + for (auto i = queueIndex_; i < writeIndex_; i++) { + Info &info = ctrl.second[i + 1]; + info = ctrl.second[i]; + info.sourceSequence_.reset(); + } + } +} + +/** + * \brief Helper function to check if controls are queued already + * \param[in] sequence Sequence number to check + * \param[in] controls List of controls to compare against + * + * This function checks if the controls queued for frame \a sequence + * are equal to \a controls. This is helpful in cases where a control algorithm + * unconditionally queues controls for every frame, but always too late. + * In that case this can be used to check if incoming controls are already queued + * or need to be queued for a later frame. + * + * \returns true if \a controls are queued for the given sequence + */ +bool DelayedControls::controlsAreQueuedForFrame(unsigned int sequence, const ControlList &controls) +{ + auto idMap = controls.idMap(); + if (!idMap) { + LOG(DelayedControls, Warning) << " idmap is null"; + return false; + } else { + for (const auto &[id, value] : controls) { + if (values_[idMap->at(id)][sequence] != value) { + return false; + } + } + } + return true; } /** * \brief Push a set of controls on the queue * \param[in] controls List of controls to add to the device queue + * \deprecated This function is deprecated in favour of pushForFrame * * 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 + * \returns See return value of DelayedControls::pushForFrame */ bool DelayedControls::push(const ControlList &controls) { - /* Copy state from previous frame. */ - for (auto &ctrl : values_) { - Info &info = ctrl.second[queueIndex_]; - info = values_[ctrl.first][queueIndex_ - 1]; - info.updated = false; + LOG(DelayedControls, Debug) << "Using deprecated function push(controls): " << queueIndex_; + return pushForFrame(queueIndex_, controls); +} + +/** + * \brief Push a set of controls for a given frame + * \param[in] sequence Sequence to push the controls for + * \param[in] controls List of controls to add to the device queue + * + * Pushes the controls given by \a controls, to be applied at frame \a sequence. + * + * If there are controls already queued for that frame, these get updated. + * + * If it's too late for frame \a sequence (controls are already sent to the sensor), + * the system checks if the controls that where written out for frame \a sequence + * are the same as the requested ones. In this case, nothing is done. + * If they differ, the controls get queued for the earliest frame possible + * if no other controls with a higher sequence number are queued for that frame already. + * + * \returns true if \a controls are accepted, or false otherwise + */ + +bool DelayedControls::pushForFrame(uint32_t sequence, const ControlList &controls) +{ + if (sequence < queueIndex_) { + LOG(DelayedControls, Debug) << "Got updated data for frame:" << sequence; + } + + if (sequence > queueIndex_) { + LOG(DelayedControls, Warning) << "Hole in queue sequence. This should not happen. Expected: " + << queueIndex_ << " got " << sequence; + } + + uint32_t updateIndex = sequence; + /* check if its too late for the request */ + if (sequence < writeIndex_) { + /* Check if we can safely ignore the request */ + if (controls.empty() || controlsAreQueuedForFrame(sequence, controls)) { + if (sequence >= queueIndex_) { + queueIndex_ = sequence + 1; + return true; + } + } else { + LOG(DelayedControls, Debug) << "Controls for frame " << sequence + << " are already in flight. Will be queued for frame " << writeIndex_; + updateIndex = writeIndex_; + } + } + + if (static_cast(updateIndex) - static_cast(writeIndex_) >= listSize - static_cast(maxDelay_)) { + /* The system is in an undefined state now. This will heal itself, as soon as all controls where rewritten */ + LOG(DelayedControls, Error) << "Queue length exceeded. The system is out of sync. Index to update:" + << updateIndex << " Next Index to apply: " << writeIndex_; + } + + /** + * Prepare the ringbuffer entries with previous data. + * Data up to [writeIndex_] gets prepared in applyControls. + */ + if (updateIndex > writeIndex_ && updateIndex >= queueIndex_) { + LOG(DelayedControls, Debug) << "Copy from previous " << queueIndex_; + for (auto i = queueIndex_; i < updateIndex; i++) { + /* Copy state from previous queue. */ + for (auto &ctrl : values_) { + auto &ringBuffer = ctrl.second; + ringBuffer[i] = ringBuffer[i - 1]; + ringBuffer[i].sourceSequence_.reset(); + } + } } /* Update with new controls. */ @@ -167,20 +271,34 @@ bool DelayedControls::push(const ControlList &controls) const ControlId *id = it->second; - if (controlParams_.find(id) == controlParams_.end()) - return false; - - Info &info = values_[id][queueIndex_]; + if (controlParams_.find(id) == controlParams_.end()) { + LOG(DelayedControls, Error) << "Could not find params for control " << id << " ignored"; + continue; + } - info = Info(control.second); + Info &info = values_[id][updateIndex]; + /* + * Update the control only, if the already existing value stems from a request + * with a sequence number smaller or equal to the current one + */ + if (info.sourceSequence_.value_or(0) <= sequence) { + info = Info(control.second, sequence); - LOG(DelayedControls, Debug) - << "Queuing " << id->name() - << " to " << info.toString() - << " at index " << queueIndex_; + LOG(DelayedControls, Debug) + << "Queuing " << id->name() + << " to " << info.toString() + << " at index " << updateIndex; + } else { + LOG(DelayedControls, Warning) + << "Skipped update " << id->name() + << " at index " << updateIndex; + } } - queueIndex_++; + LOG(DelayedControls, Debug) << "Queued frame: " << queueIndex_ << " it will be active in " << updateIndex; + + if (sequence >= queueIndex_) + queueIndex_ = sequence + 1; return true; } @@ -202,19 +320,17 @@ bool DelayedControls::push(const ControlList &controls) */ ControlList DelayedControls::get(uint32_t sequence) { - unsigned int index = std::max(0, sequence - maxDelay_); - ControlList out(device_->controls()); for (const auto &ctrl : values_) { const ControlId *id = ctrl.first; - const Info &info = ctrl.second[index]; + const Info &info = ctrl.second[sequence]; out.set(id->id(), info); LOG(DelayedControls, Debug) << "Reading " << id->name() << " to " << info.toString() - << " at index " << index; + << " at index " << sequence; } return out; @@ -222,16 +338,21 @@ ControlList DelayedControls::get(uint32_t sequence) /** * \brief Inform DelayedControls of the start of a new frame - * \param[in] sequence Sequence number of the frame that started + * \param[in] sequence Sequence number of the frame that started (0-based) * - * 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. + * Inform the state machine that a new frame has started to arrive at the receiver + * (e.g. the sensor started to clock out pixels) and of its sequence + * number. This is usually the earliest point in time to update registers in the + * sensor for upcoming frames. 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"; + LOG(DelayedControls, Debug) << "Apply controls " << sequence; + if (sequence != writeIndex_ - maxDelay_) { + LOG(DelayedControls, Warning) << "Sequence and writeIndex are out of sync. Expected seq: " << writeIndex_ - maxDelay_ << " got " << sequence; + } /* * Create control list peeking ahead in the value queue to ensure @@ -241,10 +362,10 @@ void DelayedControls::applyControls(uint32_t sequence) for (auto &ctrl : values_) { const ControlId *id = ctrl.first; unsigned int delayDiff = maxDelay_ - controlParams_[id].delay; - unsigned int index = std::max(0, writeIndex_ - delayDiff); + unsigned int index = writeIndex_ - delayDiff; Info &info = ctrl.second[index]; - if (info.updated) { + if (info.sourceSequence_.has_value()) { if (controlParams_[id].priorityWrite) { /* * This control must be written now, it could @@ -262,21 +383,25 @@ void DelayedControls::applyControls(uint32_t sequence) } LOG(DelayedControls, Debug) - << "Setting " << id->name() - << " to " << info.toString() - << " at index " << index; - - /* Done with this update, so mark as completed. */ - info.updated = false; + << "Writing " << id->name() + << " (" << info.toString() << ") " + << " for frame " << index; } } - writeIndex_ = sequence + 1; + auto oldWriteIndex = writeIndex_; + writeIndex_ = sequence + maxDelay_ + 1; - while (writeIndex_ > queueIndex_) { + if (writeIndex_ >= queueIndex_ && writeIndex_ > oldWriteIndex) { LOG(DelayedControls, Debug) - << "Queue is empty, auto queue no-op."; - push({}); + << "Index " << writeIndex_ << " is not yet queued. Prepare with old state"; + /* Copy state from previous frames without resetting the sourceSequence */ + for (auto &ctrl : values_) { + for (auto i = oldWriteIndex; i < writeIndex_; i++) { + Info &info = ctrl.second[i + 1]; + info = values_[ctrl.first][i]; + } + } } device_->setControls(&out);