From patchwork Wed Mar 25 15:13:33 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26343 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 F4061BE086 for ; Wed, 25 Mar 2026 15:14:47 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id AD73C6283D; Wed, 25 Mar 2026 16:14:47 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="L/F8LakD"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 736B3627CE for ; Wed, 25 Mar 2026 16:14:45 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 80ED112BB; Wed, 25 Mar 2026 16:13:27 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451607; bh=OP4oUpfx8I/P6x0HjoKgs/XRVqyEtt46jOEPEqpxeTA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=L/F8LakD1Iz56QjvyvnS9lOXzk7Y64qpI/zObe3Y8BZQcDbky7bWAe5ogarR4811o kMkR85exvxAaE9bThinSxxA6OBYvL4ThcidQ/fXpqNNZZEhQ/7+qZ8HHpO1XaQmu1M 18Ll8XqVU5o2eHfaWMirxws/+k4Pi3Xz3qBoQhJI= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 01/32] libcamera: delayed_controls: Add push() function that accepts a sequence number Date: Wed, 25 Mar 2026 16:13:33 +0100 Message-ID: <20260325151416.2114564-2-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The push function is asymmetric to the get() and applyControls() function in that it doesn't allow to specify a frame number. This leads to the unfortunate situation that it is very difficult to detect if anything goes out of sync. Add a version of the push() function that takes a sequence parameter and warns when the sequence provided differs from the expected sequence. Don't take any further actions for now to see where issues pop up. Signed-off-by: Stefan Klug --- include/libcamera/internal/delayed_controls.h | 1 + src/libcamera/delayed_controls.cpp | 32 ++++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h index b64d8bba7cf7..c650e672d964 100644 --- a/include/libcamera/internal/delayed_controls.h +++ b/include/libcamera/internal/delayed_controls.h @@ -32,6 +32,7 @@ public: void reset(); bool push(const ControlList &controls); + bool push(uint32_t sequence, const ControlList &controls); ControlList get(uint32_t sequence); void applyControls(uint32_t sequence); diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index 5b3ced4e711d..36d1f20a3680 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -144,10 +144,40 @@ void DelayedControls::reset() * Push a set of controls to the control queue. This increases the control queue * depth by one. * + * \note The usage of this function is discouraged as it does not provide a way + * to detect double pushes for the same sequence. Better use + * DelayedControls::push(uint32_t sequence, const ControlList &controls) + * instead. + * * \returns true if \a controls are accepted, or false otherwise */ bool DelayedControls::push(const ControlList &controls) { + LOG(DelayedControls, Debug) << "Deprecated: Push without sequence number"; + return push(queueCount_, controls); +} + +/** + * \brief Push a set of controls on the queue + * \param[in] sequence The sequence number to push for + * \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. + * + * The \a sequence number is used to do some sanity checks to detect double + * pushes for the same sequence (either due to a bug or a request underrun). + * + * \returns true if \a controls are accepted, or false otherwise + */ +bool DelayedControls::push(uint32_t sequence, const ControlList &controls) +{ + if (sequence < queueCount_) { + LOG(DelayedControls, Warning) + << "Double push for sequence " << sequence + << " current queue index: " << queueCount_; + } + /* Copy state from previous frame. */ for (auto &ctrl : values_) { Info &info = ctrl.second[queueCount_]; @@ -276,7 +306,7 @@ void DelayedControls::applyControls(uint32_t sequence) while (writeCount_ > queueCount_) { LOG(DelayedControls, Debug) << "Queue is empty, auto queue no-op."; - push({}); + push(queueCount_, {}); } device_->setControls(&out); From patchwork Wed Mar 25 15:13:34 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26344 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 D60CAC32DE for ; Wed, 25 Mar 2026 15:14:50 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 34A5562841; Wed, 25 Mar 2026 16:14:50 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="sgRNSEwF"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1BB4A627D7 for ; Wed, 25 Mar 2026 16:14:48 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 0881412BB; Wed, 25 Mar 2026 16:13:30 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451610; bh=l3vpLe0XYWTCBdGP+/6jXvvI83rewD783FNSy/pnRwE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=sgRNSEwF2ugLbLSGNEDZlMKMX44vXkN0V94R/sEFkIdCJMKLFso7gl/Cp+jv3PTcf a5NHql5Sd5UfWq4q+GGR3YyT7M4+eUe3t9TPhqYI66a9EtXSfvev2FxQlSzpBgw/a/ pt82GCacjVi6r59bO/2eNeGrGji7hlne/k4OH+DA= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Paul Elder Subject: [PATCH v2 02/32] libcamera: delayed_controls: Handle missed pushes Date: Wed, 25 Mar 2026 16:13:34 +0100 Message-ID: <20260325151416.2114564-3-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" In unstable systems it can happen, that some sequence numbers are missed in the IPA. Gracefully handle that situation by queuing no-ops. Signed-off-by: Stefan Klug Reviewed-by: Paul Elder --- Changes in v2: - Collected tag --- src/libcamera/delayed_controls.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index 36d1f20a3680..dc3850c0885d 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -178,6 +178,15 @@ bool DelayedControls::push(uint32_t sequence, const ControlList &controls) << " current queue index: " << queueCount_; } + while (sequence > queueCount_) { + LOG(DelayedControls, Warning) + << "Missed push for sequence " << queueCount_ + << " Auto queue no-op."; + push(queueCount_, {}); + } + + ASSERT(sequence == queueCount_); + /* Copy state from previous frame. */ for (auto &ctrl : values_) { Info &info = ctrl.second[queueCount_]; From patchwork Wed Mar 25 15:13:35 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26345 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 695A5C32F5 for ; Wed, 25 Mar 2026 15:14:52 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BDF7062840; Wed, 25 Mar 2026 16:14:51 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="CV/Gh+r7"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B3D5C627D7 for ; Wed, 25 Mar 2026 16:14:50 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id BF1351943; Wed, 25 Mar 2026 16:13:32 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451612; bh=fMUczoutMt4SBDXR5wnNqFd3IPSgMRdL5wKSExrc7Ow=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=CV/Gh+r7nj2UVfqjrTHD0pD2SOEwDWuWLYJPWnFJG2pUJZPO8mJN19S1nEliA90v8 SaPQiOaKSzTeBpFxKQ17saT02l9Okx7WlLyPLtg1ycSD1+A1BsmXP/wvB6u4HdumYP JL0C1DG7kc8BOm2xLhef8VVR7YXVKoN1SIWy0cHo= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Paul Elder Subject: [PATCH v2 03/32] libcamera: delayed_controls: Increase log level for dummy pushes Date: Wed, 25 Mar 2026 16:13:35 +0100 Message-ID: <20260325151416.2114564-4-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" When an automatic no-op is queued a debug message is printed. Change the debug level to warning, because that situation is actually an indication of something going seriously out of sync. Signed-off-by: Stefan Klug Reviewed-by: Paul Elder --- Changes in v2: - Collected tag --- src/libcamera/delayed_controls.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index dc3850c0885d..2854016c6170 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -313,7 +313,7 @@ void DelayedControls::applyControls(uint32_t sequence) writeCount_ = sequence + 1; while (writeCount_ > queueCount_) { - LOG(DelayedControls, Debug) + LOG(DelayedControls, Warning) << "Queue is empty, auto queue no-op."; push(queueCount_, {}); } From patchwork Wed Mar 25 15:13:36 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26346 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 417EFC32F6 for ; Wed, 25 Mar 2026 15:14:55 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 80C7F627D7; Wed, 25 Mar 2026 16:14:54 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="j43NuIxs"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 96724627D7 for ; Wed, 25 Mar 2026 16:14:52 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 954E51992; Wed, 25 Mar 2026 16:13:34 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451614; bh=EbhtE94aXfZc4txsZOEuCZh+VEJl4RDHgjpWhMcla5Y=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=j43NuIxsyCbrc+yWLvf6DAZEzXAPI8R6NsQMUeakTz8t//6e8QdOuHRlI3Rgoq927 WpJPFgSoifRblUlMa/yRB75kroE7HoUynaBIBvtN7JdTUlkzkt0mYL1NNs7XpdO3MH XOaj7kapWgjif5/s9rs6g1syzA/Hh9Y4tWeqNgXc= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 04/32] libcamera: delayed_controls: Queue noop when needed, not before Date: Wed, 25 Mar 2026 16:13:36 +0100 Message-ID: <20260325151416.2114564-5-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" A no-op is queued when DelayedControls runs out of controls at the end of applyControls(). But these controls are only needed on the next call to applyControls(), so there is still time for a proper push. To fix that, move the no-op push to the beginning of applyContols(). Drop writeCount_ as it is no longer needed. Signed-off-by: Stefan Klug --- Changes in v2: - Improved a log message - Improved commit message - dropped writeCount_ member --- include/libcamera/internal/delayed_controls.h | 1 - src/libcamera/delayed_controls.cpp | 18 ++++++++---------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h index c650e672d964..2af572606d0f 100644 --- a/include/libcamera/internal/delayed_controls.h +++ b/include/libcamera/internal/delayed_controls.h @@ -76,7 +76,6 @@ private: unsigned int maxDelay_; uint32_t queueCount_; - uint32_t writeCount_; /* \todo Evaluate if we should index on ControlId * or unsigned int */ std::unordered_map values_; }; diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index 2854016c6170..123a37bdc887 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -116,7 +116,6 @@ DelayedControls::DelayedControls(V4L2Device *device, void DelayedControls::reset() { queueCount_ = 1; - writeCount_ = 0; /* Retrieve control as reported by the device. */ std::vector ids; @@ -272,6 +271,13 @@ void DelayedControls::applyControls(uint32_t sequence) { LOG(DelayedControls, Debug) << "frame " << sequence << " started"; + while (queueCount_ - 1 < sequence) { + LOG(DelayedControls, Warning) + << "Queue is empty, auto queue no-op for sequence " + << queueCount_; + push(queueCount_, {}); + } + /* * Create control list peeking ahead in the value queue to ensure * values are set in time to satisfy the sensor delay. @@ -280,7 +286,7 @@ 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, writeCount_ - delayDiff); + unsigned int index = std::max(0, sequence - delayDiff); Info &info = ctrl.second[index]; if (info.updated) { @@ -310,14 +316,6 @@ void DelayedControls::applyControls(uint32_t sequence) } } - writeCount_ = sequence + 1; - - while (writeCount_ > queueCount_) { - LOG(DelayedControls, Warning) - << "Queue is empty, auto queue no-op."; - push(queueCount_, {}); - } - device_->setControls(&out); } From patchwork Wed Mar 25 15:13:37 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26347 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 7F2A1C32F7 for ; Wed, 25 Mar 2026 15:14:57 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id F38176284B; Wed, 25 Mar 2026 16:14:56 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="jrELRHGK"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C244A62841 for ; Wed, 25 Mar 2026 16:14:55 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id C95A41862; Wed, 25 Mar 2026 16:13:37 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451617; bh=c36oqgLD16SYQZuTpMgniVBmMeCIkXxM3t8wZCYQlak=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=jrELRHGKVPlhkKtIPVItqwYGZkDRzpNtj7J/VxCV1GpDyodPd4T7w4qotaP7f5B05 U9UvnWiK2fDAp0dhEHMjdetQ40OJagFxlEDcdimEAQWv43XT80QxuZhvnQeSfZmctu O03m5IcEQa5UOaSq87JJ2sxa9sTABWyiINwyk6rY= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Paul Elder Subject: [PATCH v2 05/32] libcamera: delayed_controls: Add maxDelay() function Date: Wed, 25 Mar 2026 16:13:37 +0100 Message-ID: <20260325151416.2114564-6-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Add a maxDelay() function to be able to query the maximum delay of the sensor. Signed-off-by: Stefan Klug Reviewed-by: Paul Elder --- Changes in v2: - Collected tag --- include/libcamera/internal/delayed_controls.h | 2 ++ src/libcamera/delayed_controls.cpp | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/include/libcamera/internal/delayed_controls.h b/include/libcamera/internal/delayed_controls.h index 2af572606d0f..9ad2414a9e99 100644 --- a/include/libcamera/internal/delayed_controls.h +++ b/include/libcamera/internal/delayed_controls.h @@ -35,6 +35,8 @@ public: bool push(uint32_t sequence, const ControlList &controls); ControlList get(uint32_t sequence); + uint32_t maxDelay() const { return maxDelay_; } + void applyControls(uint32_t sequence); private: diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index 123a37bdc887..71071a0c670d 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -258,6 +258,13 @@ ControlList DelayedControls::get(uint32_t sequence) return out; } +/** + * \fn DelayedControls::maxDelay() + * \brief Get the maximum delay of the sensor + * + * \return The maximum delay of the sensor + */ + /** * \brief Inform DelayedControls of the start of a new frame * \param[in] sequence Sequence number of the frame that started From patchwork Wed Mar 25 15:13:38 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26348 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 B5817BE086 for ; Wed, 25 Mar 2026 15:15:01 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6FE2C6284B; Wed, 25 Mar 2026 16:15:00 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="T8AKW9/5"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 0A294627D7 for ; Wed, 25 Mar 2026 16:14:59 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 13D9C1943; Wed, 25 Mar 2026 16:13:41 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451621; bh=129Z8a31qMQqyivzB+cAoXFqgdyhkmvW0MDCa8PtwJQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=T8AKW9/5foFXfPB1lMKq1zSW2bhW5EsftFWc6gAMC+mAfbOekfJ2pUhPestQpZ80G bP0E79M9VYEY2eAwAIwq6TuS/TSC5eab9qeKk48AVKGQu7qxn948IbxCETC2LFE6Ih nYfsJkAMSH7HxgDcunj7JdNVRIb0eW7vfhLGEEhE= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Paul Elder Subject: [PATCH v2 06/32] pipeline: rkisp1: Include frame number when pushing to delayed controls Date: Wed, 25 Mar 2026 16:13:38 +0100 Message-ID: <20260325151416.2114564-7-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Pass the frame number when pushing to delayed controls, to ease further development. Signed-off-by: Stefan Klug Reviewed-by: Paul Elder --- Changes in v2: - Collected tag --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 320a4dc5a899..31e5ae103209 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -495,10 +495,10 @@ void RkISP1CameraData::paramsComputed(unsigned int frame, unsigned int bytesused selfPath_->queueBuffer(info->selfPathBuffer); } -void RkISP1CameraData::setSensorControls([[maybe_unused]] unsigned int frame, +void RkISP1CameraData::setSensorControls(unsigned int frame, const ControlList &sensorControls) { - delayedCtrls_->push(sensorControls); + delayedCtrls_->push(frame, sensorControls); } void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata) From patchwork Wed Mar 25 15:13:39 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26349 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 0B3D5C32F8 for ; Wed, 25 Mar 2026 15:15:03 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E898E62852; Wed, 25 Mar 2026 16:15:02 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="TgzmCRLS"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5D9D8627D7 for ; Wed, 25 Mar 2026 16:15:01 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 5A5F712BB; Wed, 25 Mar 2026 16:13:43 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451623; bh=VOqC7Xke0kB/p9Ixwo8tICQdTYGuG+M0+xf5hv/yUqw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=TgzmCRLSWInV3cFxyG+bmLWe2U7YcVC3KIisPhIs4xX+tiHO4Caz1OOItyJHNnU7z Dsv2VDNBcIwJQwkW/GvUr8eb0CilabihlkYWVvCU44bUqNkd+ACym2YvjD76bTEFeq AL+MaTmpEYgcNJ9X/PXupR+71nVXbYfKQR0VzX8A= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Paul Elder Subject: [PATCH v2 07/32] libipa: fc_queue: Rename template argument to FC Date: Wed, 25 Mar 2026 16:13:39 +0100 Message-ID: <20260325151416.2114564-8-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The FCQueue has a template argument called FrameContext which is easily confused with the class FrameContext defined in the same file. Reduce that confusion by renaming the template argument to FC. Signed-off-by: Stefan Klug Reviewed-by: Paul Elder --- Changes in v2: - Collected tag --- src/ipa/libipa/fc_queue.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h index a1d136521107..1f4f84c27fbc 100644 --- a/src/ipa/libipa/fc_queue.h +++ b/src/ipa/libipa/fc_queue.h @@ -28,7 +28,7 @@ private: bool initialised = false; }; -template +template class FCQueue { public: @@ -39,15 +39,15 @@ public: void clear() { - for (FrameContext &ctx : contexts_) { + for (FC &ctx : contexts_) { ctx.initialised = false; ctx.frame = 0; } } - FrameContext &alloc(const uint32_t frame) + FC &alloc(const uint32_t frame) { - FrameContext &frameContext = contexts_[frame % contexts_.size()]; + FC &frameContext = contexts_[frame % contexts_.size()]; /* * Do not re-initialise if a get() call has already fetched this @@ -69,9 +69,9 @@ public: return frameContext; } - FrameContext &get(uint32_t frame) + FC &get(uint32_t frame) { - FrameContext &frameContext = contexts_[frame % contexts_.size()]; + FC &frameContext = contexts_[frame % contexts_.size()]; /* * If the IPA algorithms try to access a frame context slot which @@ -122,14 +122,14 @@ public: } private: - void init(FrameContext &frameContext, const uint32_t frame) + void init(FC &frameContext, const uint32_t frame) { frameContext = {}; frameContext.frame = frame; frameContext.initialised = true; } - std::vector contexts_; + std::vector contexts_; }; } /* namespace ipa */ From patchwork Wed Mar 25 15:13:40 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26350 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 68B6BC32F9 for ; Wed, 25 Mar 2026 15:15:05 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id C20A162855; Wed, 25 Mar 2026 16:15:04 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="IbVd2FjY"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 20BD16284A for ; Wed, 25 Mar 2026 16:15:04 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 17B3C1943; Wed, 25 Mar 2026 16:13:46 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451626; bh=HAhuAcE+mhWcDbQtFIYbi59V755HCbcRTS4wIYxopBM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=IbVd2FjYPr+22M9A4P5yvBsL/6k+p5txmG22PnwyVpTutDvjw4v6zzD8hjmqFaHku fX0gZie8fEZtoCUOnLwVbSeVbEbacOE66/N5kELClmbUVoo+WZUUrVdWTfLl5j6PBf 5+sOmkdJbbymhkygciD4hQFIVvg+aAexDYMKwL54= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Paul Elder Subject: [PATCH v2 08/32] libipa: fc_queue: Add trailing underscore to private members of FrameContext Date: Wed, 25 Mar 2026 16:13:40 +0100 Message-ID: <20260325151416.2114564-9-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" It is not immediately obvious that FCQueue accesses private members of the FrameContext class with the help of the friend declaration. This gets even more confusing when such a member is shadowed by a declaration in the actual IPA FrameContext class. Improve that by accessing the FrameContext members via references to that type. Additionally add an underscore to the variable names like we do on other members and which allows us to create a frame() accessor without a name clash in an upcoming commit. Signed-off-by: Stefan Klug Reviewed-by: Paul Elder --- Changes in v2: - Collected tag --- src/ipa/libipa/fc_queue.cpp | 2 +- src/ipa/libipa/fc_queue.h | 51 ++++++++++++++++++++----------------- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp index 0365e9197748..39222c2ed204 100644 --- a/src/ipa/libipa/fc_queue.cpp +++ b/src/ipa/libipa/fc_queue.cpp @@ -34,7 +34,7 @@ namespace ipa { * update any specific action for this frame, and finally to update the metadata * control lists when the frame is fully completed. * - * \var FrameContext::frame + * \var FrameContext::frame_ * \brief The frame number */ diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h index 1f4f84c27fbc..1128e42f8ca6 100644 --- a/src/ipa/libipa/fc_queue.h +++ b/src/ipa/libipa/fc_queue.h @@ -24,8 +24,8 @@ class FCQueue; struct FrameContext { private: template friend class FCQueue; - uint32_t frame; - bool initialised = false; + uint32_t frame_; + bool initialised_ = false; }; template @@ -40,14 +40,15 @@ public: void clear() { for (FC &ctx : contexts_) { - ctx.initialised = false; - ctx.frame = 0; + ctx.initialised_ = false; + ctx.frame_ = 0; } } FC &alloc(const uint32_t frame) { - FC &frameContext = contexts_[frame % contexts_.size()]; + FC &fc = contexts_[frame % contexts_.size()]; + FrameContext &frameContext = fc; /* * Do not re-initialise if a get() call has already fetched this @@ -60,18 +61,19 @@ public: * time the application has queued a request. Does this deserve * an error condition ? */ - if (frame != 0 && frame <= frameContext.frame) + if (frame != 0 && frame <= frameContext.frame_) LOG(FCQueue, Warning) << "Frame " << frame << " already initialised"; else - init(frameContext, frame); + init(fc, frame); - return frameContext; + return fc; } FC &get(uint32_t frame) { - FC &frameContext = contexts_[frame % contexts_.size()]; + FC &fc = contexts_[frame % contexts_.size()]; + FrameContext &frameContext = fc; /* * If the IPA algorithms try to access a frame context slot which @@ -81,28 +83,28 @@ public: * queueing more requests to the IPA than the frame context * queue size. */ - if (frame < frameContext.frame) + if (frame < frameContext.frame_) LOG(FCQueue, Fatal) << "Frame context for " << frame << " has been overwritten by " - << frameContext.frame; + << frameContext.frame_; - if (frame == 0 && !frameContext.initialised) { + if (frame == 0 && !frameContext.initialised_) { /* * If the IPA calls get() at start() time it will get an * un-intialized FrameContext as the below "frame == - * frameContext.frame" check will return success because - * FrameContexts are zeroed at creation time. + * frameContext.frame_" check will return success + * because FrameContexts are zeroed at creation time. * * Make sure the FrameContext gets initialised if get() * is called before alloc() by the IPA for frame#0. */ - init(frameContext, frame); + init(fc, frame); - return frameContext; + return fc; } - if (frame == frameContext.frame) - return frameContext; + if (frame == frameContext.frame_) + return fc; /* * The frame context has been retrieved before it was @@ -116,17 +118,18 @@ public: LOG(FCQueue, Warning) << "Obtained an uninitialised FrameContext for " << frame; - init(frameContext, frame); + init(fc, frame); - return frameContext; + return fc; } private: - void init(FC &frameContext, const uint32_t frame) + void init(FC &fc, const uint32_t frame) { - frameContext = {}; - frameContext.frame = frame; - frameContext.initialised = true; + fc = {}; + FrameContext &frameContext = fc; + frameContext.frame_ = frame; + frameContext.initialised_ = true; } std::vector contexts_; From patchwork Wed Mar 25 15:13:41 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26351 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 4C8C0C32FA for ; Wed, 25 Mar 2026 15:15:09 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6DDA662857; Wed, 25 Mar 2026 16:15:08 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="DdHp5oLZ"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1AE426284A for ; Wed, 25 Mar 2026 16:15:07 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 1E9EA121A; Wed, 25 Mar 2026 16:13:49 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451629; bh=XT8qlJr/aApRTAQ0hbgKO428Qjo9spEdkl6DAVtevbA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=DdHp5oLZ4GXYDk+UfK9+SyrzIFS/0KoUErwj54+2e0WS4oKgyVhABqcMmvsgdbNvf u5yPpPAMlKckoyKJEgVxohRcut1f4Hi8tvLgwICmmCG4M8Jc95dQM/CDQJl3R3A62Y D1ftGzcXsxijg0cf2dVsN4Pflv7xJzEdkKXhFbhk= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Paul Elder Subject: [PATCH v2 09/32] ipa: rkisp1: Refactor setControls() Date: Wed, 25 Mar 2026 16:13:41 +0100 Message-ID: <20260325151416.2114564-10-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" IPARkISP1::setControls() is called when new sensor controls shall be queued to the pipeline handler. It constructs a list of sensor controls and then emits the setSensorControls signal. To be able to return initial sensor controls from the IPARkISP1::start() function, similar functionality will be needed. Prepare for that by changing the setControls() function to a getSensorControls() that is passed a frame context and by moving the setSensorControls.emit() out of the function. Signed-off-by: Stefan Klug Reviewed-by: Paul Elder --- Changes in v2: - Collected tag --- src/ipa/libipa/fc_queue.cpp | 6 ++++++ src/ipa/libipa/fc_queue.h | 2 ++ src/ipa/rkisp1/rkisp1.cpp | 26 +++++++++++++------------- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp index 39222c2ed204..7ba28ed21611 100644 --- a/src/ipa/libipa/fc_queue.cpp +++ b/src/ipa/libipa/fc_queue.cpp @@ -38,6 +38,12 @@ namespace ipa { * \brief The frame number */ +/** + * \fn FrameContext::frame() + * \brief Get the frame of that frame context + * \return THe frame number + */ + /** * \class FCQueue * \brief A support class for managing FrameContext instances in IPA modules diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h index 1128e42f8ca6..812022c496ed 100644 --- a/src/ipa/libipa/fc_queue.h +++ b/src/ipa/libipa/fc_queue.h @@ -22,6 +22,8 @@ template class FCQueue; struct FrameContext { + uint32_t frame() const { return frame_; } + private: template friend class FCQueue; uint32_t frame_; diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 373a343bddd0..991843e29a5d 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -77,7 +77,7 @@ private: void updateControls(const IPACameraSensorInfo &sensorInfo, const ControlInfoMap &sensorControls, ControlInfoMap *ipaControls); - void setControls(unsigned int frame); + ControlList getSensorControls(const IPAFrameContext &context); std::map buffers_; std::map mappedBuffers_; @@ -380,7 +380,12 @@ void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId, algo->process(context_, frame, frameContext, stats, metadata); } - setControls(frame); + /* + * \todo: Here we should do a lookahead that takes the sensor delays + * into account. + */ + ControlList ctrls = getSensorControls(frameContext); + setSensorControls.emit(frame, ctrls); context_.debugMetadata.moveEntries(metadata); metadataReady.emit(frame, metadata); @@ -446,28 +451,23 @@ void IPARkISP1::updateControls(const IPACameraSensorInfo &sensorInfo, *ipaControls = ControlInfoMap(std::move(ctrlMap), controls::controls); } -void IPARkISP1::setControls(unsigned int frame) +ControlList IPARkISP1::getSensorControls(const IPAFrameContext &frameContext) { - /* - * \todo The frame number is most likely wrong here, we need to take - * internal sensor delays and other timing parameters into account. - */ - - IPAFrameContext &frameContext = context_.frameContexts.get(frame); uint32_t exposure = frameContext.agc.exposure; uint32_t gain = context_.camHelper->gainCode(frameContext.agc.gain); uint32_t vblank = frameContext.agc.vblank; LOG(IPARkISP1, Debug) - << "Set controls for frame " << frame << ": exposure " << exposure - << ", gain " << frameContext.agc.gain << ", vblank " << vblank; + << "Set controls for frame " << frameContext.frame() + << ": exposure " << exposure + << ", gain " << frameContext.agc.gain + << ", vblank " << vblank; ControlList ctrls(sensorControls_); ctrls.set(V4L2_CID_EXPOSURE, static_cast(exposure)); ctrls.set(V4L2_CID_ANALOGUE_GAIN, static_cast(gain)); ctrls.set(V4L2_CID_VBLANK, static_cast(vblank)); - - setSensorControls.emit(frame, ctrls); + return ctrls; } } /* namespace ipa::rkisp1 */ From patchwork Wed Mar 25 15:13:42 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26352 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 A6735C32FB for ; Wed, 25 Mar 2026 15:15:11 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E49AC6285C; Wed, 25 Mar 2026 16:15:10 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="SjHtBU4E"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 17E416284A for ; Wed, 25 Mar 2026 16:15:10 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 293151943; Wed, 25 Mar 2026 16:13:52 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451632; bh=bWuAtp2490iH7v1FEummzNlFI1ouVcOvkqF6jlhzuYI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=SjHtBU4EMhMR3yKCcZyQ9GkX+B+B8pmbDdVZGJAI0gXx0ElZTd/iMP1hhEu1iIkml 2q7PT5HjKaAklbVTgEUHXPLTn+e4LOi50PxKzQvn9dxgtnmLrwA5vZBpWO7OnuRURV xr3Nd4iCdXtouQW2/7XWRjFSXO8s0BLIqZzOZx20= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 10/32] pipeline: rkisp1: Add a frameStart function to handle DelayedControls::applyControls Date: Wed, 25 Mar 2026 16:13:42 +0100 Message-ID: <20260325151416.2114564-11-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Move the call to applyControls into an intermediate function for upcoming modfications. As the frameStart handler checks for an activeCamera we can safely connect the signal in match() where all the other signals get connected. Signed-off-by: Stefan Klug --- Changes in v2: - Moved signal connection into match() function. --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 31e5ae103209..a3b78bf4dc6b 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -1470,8 +1470,6 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) data->delayedCtrls_ = std::make_unique(data->sensor_->device(), params); - isp_->frameStart.connect(data->delayedCtrls_.get(), - &DelayedControls::applyControls); uint32_t supportedBlocks = kDefaultExtParamsBlocks; @@ -1506,6 +1504,15 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor) return 0; } +void PipelineHandlerRkISP1::frameStart(uint32_t sequence) +{ + if (!activeCamera_) + return; + + RkISP1CameraData *data = cameraData(activeCamera_); + data->delayedCtrls_->applyControls(sequence); +} + bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) { DeviceMatch dm("rkisp1"); @@ -1548,6 +1555,7 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) if (hasSelfPath_ && !selfPath_.init(media_)) return false; + isp_->frameStart.connect(this, &PipelineHandlerRkISP1::frameStart); mainPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::imageBufferReady); if (hasSelfPath_) selfPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::imageBufferReady); From patchwork Wed Mar 25 15:13:43 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26353 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 CFBA3C32DE for ; Wed, 25 Mar 2026 15:15:13 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 86CD062854; Wed, 25 Mar 2026 16:15:13 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="U7o2P0YW"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 34DBB62854 for ; Wed, 25 Mar 2026 16:15:12 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 42D3512BB; Wed, 25 Mar 2026 16:13:54 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451634; bh=frnossdUElVa3lOkrWeRsZUq7113G56zV2qRkDwG0Wk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=U7o2P0YWEOxDog4YQJ63Mdw549zPp9DvL2Rsa13vwj+6kM4xVMVEaZDClnHBNm4DR Es4o4yqG/eenmVbet2jQ7MlxtZ6c19+jv2metJMxUd3+6ltoseZzjJ+iGqv0nK8p3G llDjEvGeeWHKWrVxPB/ofqvBos75dmw5fdLcB1vE= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Paul Elder Subject: [PATCH v2 11/32] ipa: rkisp1: Move setSensorControls signal to computeParams Date: Wed, 25 Mar 2026 16:13:43 +0100 Message-ID: <20260325151416.2114564-12-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The setSensorControls event is emitted in the processStats() function. On first sight this looks reasonable as in processStats() we got the latest statistics and can therefore calculate the most up to date sensor controls. In the light of per-frame-controls however it produces difficult to solve timing issues: - The frame context in processStats() is the frame context of the frame that produced the stats, not for the frame that should be prepared and sent to the sensor. - To synchronize digital gain applied in the ISP with the analog gain applied in the sensor the set of parameters prepared for sensor and ISP must also be synchronized, which is not the case. To fix that, move the calculation and setting of sensor controls into the computeParams(). This way the model is far more easy to understand. We loose a tiny option for optimizations in that (in theory) we could delay the calculation of ISP parameters by another frame (assuming the sensor has a typical 2-frame delay). But all discussions and tests showed that keeping all parameters in sync is more important than that possible optimization for one frame. Signed-off-by: Stefan Klug Reviewed-by: Paul Elder --- Changes in v2: - Collected tag --- src/ipa/rkisp1/rkisp1.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 991843e29a5d..81430d6532ac 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -349,6 +349,9 @@ void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId) for (const auto &algo : algorithms()) algo->prepare(context_, frame, frameContext, ¶ms); + ControlList ctrls = getSensorControls(frameContext); + setSensorControls.emit(frame, ctrls); + paramsComputed.emit(frame, params.bytesused()); } @@ -380,13 +383,6 @@ void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId, algo->process(context_, frame, frameContext, stats, metadata); } - /* - * \todo: Here we should do a lookahead that takes the sensor delays - * into account. - */ - ControlList ctrls = getSensorControls(frameContext); - setSensorControls.emit(frame, ctrls); - context_.debugMetadata.moveEntries(metadata); metadataReady.emit(frame, metadata); } From patchwork Wed Mar 25 15:13:44 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26354 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 B11EBC32FC for ; Wed, 25 Mar 2026 15:15:16 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1127762882; Wed, 25 Mar 2026 16:15:16 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="GIc2qa0t"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E1E8C62857 for ; Wed, 25 Mar 2026 16:15:14 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id ED8321943; Wed, 25 Mar 2026 16:13:56 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451637; bh=X6yvf1PwwFd+CBeirPNd09zNmApLC+ULARtP3wJL9iA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=GIc2qa0tRC1fv6oH+dOu/LQmuYz2ehhY7Qb9743sTEiSlwszdqCUvD96f/doseK+a 7t0IGhLobafESVvgDDTa4/ev53OuZqCDS8+A1bcQAusQtIQhvBoTdaNUQsJHdwhIEf U7zSEvqrITorqF28pjkImKs5VaeR9gujAZBdu3DY= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 12/32] pipeline: rkisp1: Fix controls in raw mode Date: Wed, 25 Mar 2026 16:13:44 +0100 Message-ID: <20260325151416.2114564-13-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" After the pipeline restructuring setSensorControls is no longer emitted within process() but within computeParams(). In raw mode computeParams is not called and therefore setSensorControls is never emitted. Fix that by allowing computeParams to be called with an empty bufferId. This strategy is also used for processStats() to ensure that metadata gets filled in raw mode. Then call computeParams within frameStart() when in raw mode. Signed-off-by: Stefan Klug --- Changes in v2: - Call computeParams() within queueRequestDevice for now to make the patch easier to follow. Moving computeParams to frameStart() will happen in a later patch. --- src/ipa/rkisp1/rkisp1.cpp | 12 ++++++++++++ src/libcamera/pipeline/rkisp1/rkisp1.cpp | 6 ++++++ 2 files changed, 18 insertions(+) diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 81430d6532ac..e06238a7abe9 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -343,6 +343,18 @@ void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId) { IPAFrameContext &frameContext = context_.frameContexts.get(frame); + /* + * \todo: This needs discussion. In raw mode, computeParams is + * called without a params buffer, to trigger the setSensorControls + * signal. Currently our algorithms don't support prepare calls with + * a nullptr. Do we need that or can we safely skip it? + */ + if (bufferId == 0) { + ControlList ctrls = getSensorControls(frameContext); + setSensorControls.emit(frame, ctrls); + return; + } + RkISP1Params params(context_.configuration.paramFormat, mappedBuffers_.at(bufferId).planes()[0]); diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index a3b78bf4dc6b..7352237dbb86 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -1350,6 +1350,12 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request) if (data->selfPath_ && info->selfPathBuffer) data->selfPath_->queueBuffer(info->selfPathBuffer); + + /* + * Call computeParams with an empty param buffer to trigger the + * setSensorControls signal. + */ + data->ipa_->computeParams(data->frame_, 0); } else { data->ipa_->computeParams(data->frame_, info->paramBuffer->cookie()); From patchwork Wed Mar 25 15:13:45 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26355 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 04A0BC32FE for ; Wed, 25 Mar 2026 15:15:19 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 849A7628CA; Wed, 25 Mar 2026 16:15:19 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="evULO233"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 639AC62857 for ; Wed, 25 Mar 2026 16:15:17 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 6C7FD12BB; Wed, 25 Mar 2026 16:13:59 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451639; bh=Sght2xYmJzdJeyr0Yb0PkBGrNhk5aQDMCUrVqwU1ee8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=evULO233fBOm+MqtcMeKD4q+q1MB7jthIJQe9byck3ZU1X4RDDw1IpXiyuALOu0Xb oHKDpO5C3Ff5hTqV1MvyLUU5BjkvMscGSq9Q+/OvfQ2VrkwumhChbcte4Ns5io/iLd mKEi4ekMkipXq798tmVnMsqbOEhD2p71Pl8+09W0= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 13/32] ipa: rkisp1: Add initializeFrameContext() function Date: Wed, 25 Mar 2026 16:13:45 +0100 Message-ID: <20260325151416.2114564-14-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" In preparation to handling startup controls, split the frame context initialization from queueRequest into a separate function. This patch contains no functional changes. Signed-off-by: Stefan Klug --- src/ipa/rkisp1/rkisp1.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index e06238a7abe9..98ed4a8ff16d 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -67,6 +67,9 @@ public: void queueRequest(const uint32_t frame, const ControlList &controls) override; void computeParams(const uint32_t frame, const uint32_t bufferId) override; + void initializeFrameContext(const uint32_t frame, + IPAFrameContext &frameContext, + const ControlList &controls); void processStats(const uint32_t frame, const uint32_t bufferId, const ControlList &sensorControls) override; @@ -331,6 +334,13 @@ void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls) IPAFrameContext &frameContext = context_.frameContexts.alloc(frame); context_.debugMetadata.enableByControl(controls); + initializeFrameContext(frame, frameContext, controls); +} + +void IPARkISP1::initializeFrameContext(const uint32_t frame, + IPAFrameContext &frameContext, + const ControlList &controls) +{ for (const auto &a : algorithms()) { Algorithm *algo = static_cast(a.get()); if (algo->disabled_) From patchwork Wed Mar 25 15:13:46 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26356 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 B8BBFC3300 for ; Wed, 25 Mar 2026 15:15:21 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 221DB62BDE; Wed, 25 Mar 2026 16:15:21 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="dMPTN9jd"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E859E62857 for ; Wed, 25 Mar 2026 16:15:19 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 021D81862; Wed, 25 Mar 2026 16:14:01 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451642; bh=UvrXME2nZvlUfV9R4/FUoLETP9kb1SrndT7JYlzUCws=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=dMPTN9jdFaLsJLhBhG+9Kg2Zz+GkSvV/63MUSPjCOE4nltgBKQR79Pys7JXsx2uaT wQvYm3+rn9UTHBWbW++0Z2zKekgsC8EC+l33TdiRTFDF3oXnbLcEFDMLNkWqEvKiBv 0641lbIRz+aV6ALrNqCVymqud5Dc60HUGv9Itbwo= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 14/32] pipeline: rkisp1: Apply initial controls Date: Wed, 25 Mar 2026 16:13:46 +0100 Message-ID: <20260325151416.2114564-15-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Controls passed to Camera::start() are not handled at the moment. Implement that by initializing a separate frame context and then returning the controls synchronously to the caller. In the pipeline the controls get passed to the sensor before the call to streamOn() to ensure the controls are applied right away. Passing the controls to the pipeline using the setSensorControls signal is not appropriate because it is asynchronous and would reach the sensor too late (initial controls need to be applied before the sensor starts and before delayed controls initializes it's start condition.) Signed-off-by: Stefan Klug --- include/libcamera/ipa/rkisp1.mojom | 7 ++++++- src/ipa/rkisp1/rkisp1.cpp | 10 ++++++---- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 11 +++++++++-- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom index 068e898848c4..4c29b53cd7f9 100644 --- a/include/libcamera/ipa/rkisp1.mojom +++ b/include/libcamera/ipa/rkisp1.mojom @@ -14,13 +14,18 @@ struct IPAConfigInfo { uint32 paramFormat; }; +struct StartResult { + libcamera.ControlList controls; + int32 code; +}; + interface IPARkISP1Interface { init(libcamera.IPASettings settings, uint32 hwRevision, uint32 supportedBlocks, libcamera.IPACameraSensorInfo sensorInfo, libcamera.ControlInfoMap sensorControls) => (int32 ret, libcamera.ControlInfoMap ipaControls); - start() => (int32 ret); + start(libcamera.ControlList controls) => (StartResult result); stop(); configure(IPAConfigInfo configInfo, diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 98ed4a8ff16d..3f943a08d011 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -56,7 +56,7 @@ public: const IPACameraSensorInfo &sensorInfo, const ControlInfoMap &sensorControls, ControlInfoMap *ipaControls) override; - int start() override; + void start(const ControlList &controls, StartResult *result) override; void stop() override; int configure(const IPAConfigInfo &ipaConfig, @@ -215,10 +215,12 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision, return 0; } -int IPARkISP1::start() +void IPARkISP1::start(const ControlList &controls, StartResult *result) { - /* \todo Properly handle startup controls. */ - return 0; + IPAFrameContext frameContext = {}; + initializeFrameContext(0, frameContext, controls); + result->controls = getSensorControls(frameContext); + result->code = 0; } void IPARkISP1::stop() diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 7352237dbb86..89e8ab0999f4 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -1238,13 +1238,20 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL return ret; actions += [&]() { freeBuffers(camera); }; - ret = data->ipa_->start(); - if (ret) { + ControlList ctrls; + if (!!controls) + ctrls = *controls; + + ipa::rkisp1::StartResult res; + data->ipa_->start(ctrls, &res); + if (res.code) { LOG(RkISP1, Error) << "Failed to start IPA " << camera->id(); return ret; } actions += [&]() { data->ipa_->stop(); }; + data->sensor_->setControls(&res.controls); + data->delayedCtrls_->reset(); data->frame_ = 0; From patchwork Wed Mar 25 15:13:47 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26357 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 1AF4BC32F5 for ; Wed, 25 Mar 2026 15:15:25 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id B595E62B9A; Wed, 25 Mar 2026 16:15:24 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="cjkXxnQH"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 4ACF6628CA for ; Wed, 25 Mar 2026 16:15:23 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 522521992; Wed, 25 Mar 2026 16:14:05 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451645; bh=XoB6H/0U6tGn+weK6nvbPi46XdQqpT3ry2fV8PvjQx4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=cjkXxnQHyghYkMXOaQXd9x7a+DbqDCqXVnUBw4WmRT1Fh0kaVLOc5TATLdliX2zkp 10UAJ1VBAE8FJ2MfSPQU/R+ShbnGfaYgHlNUDmBdm7BzpmTwa125SmGNV+iwdYnAFo WCOEZb+9e+Lz7aT6LDci0g/PChTZBH6bUGNc4mug= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Paul Elder Subject: [PATCH v2 15/32] ipa: rkisp1: Set frameContext.agc in queueRequest for auto mode also Date: Wed, 25 Mar 2026 16:13:47 +0100 Message-ID: <20260325151416.2114564-16-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" If the agc is in auto mode, exposure time and gain used to be set in the frame context within prepare(). As exposure time and gain are used by getSensorControls(0) from within start() that is too late (prepare() hasn't run yet). Also prepare() is documented as the place to initialize the params buffer, not the frame context. From the pipeline point of view, prepare() gets called immediately after queueRequest(). Therefore we can safely move setting the frame context into queueRequest() to fix the sensor controls for start(). Signed-off-by: Stefan Klug Reviewed-by: Paul Elder --- Changes in v2: - Collected tag --- src/ipa/rkisp1/algorithms/agc.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp index 1ecaff680978..fd227e2129ed 100644 --- a/src/ipa/rkisp1/algorithms/agc.cpp +++ b/src/ipa/rkisp1/algorithms/agc.cpp @@ -284,9 +284,13 @@ void Agc::queueRequest(IPAContext &context, frameContext.agc.autoExposureEnabled = agc.autoExposureEnabled; frameContext.agc.autoGainEnabled = agc.autoGainEnabled; - if (!frameContext.agc.autoExposureEnabled) + if (frameContext.agc.autoExposureEnabled) + frameContext.agc.exposure = context.activeState.agc.automatic.exposure; + else frameContext.agc.exposure = agc.manual.exposure; - if (!frameContext.agc.autoGainEnabled) + if (frameContext.agc.autoGainEnabled) + frameContext.agc.gain = context.activeState.agc.automatic.gain; + else frameContext.agc.gain = agc.manual.gain; if (!frameContext.agc.autoExposureEnabled && From patchwork Wed Mar 25 15:13:48 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26358 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 CF490C3301 for ; Wed, 25 Mar 2026 15:15:27 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5188162BDE; Wed, 25 Mar 2026 16:15:27 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="M0/DvBoK"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 165C462B91 for ; Wed, 25 Mar 2026 16:15:26 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 22B271AFB; Wed, 25 Mar 2026 16:14:08 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451648; bh=oAi/9LLTASwPFAIPya7rkzLto5JItlOEGT7r+u/Ulgk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=M0/DvBoKzdTZnsG9wTYfejBBrjN3UuzRjviN/QyW8BePmzd+bA2o5l7W0/folNx/8 462bhuRcmgXIKfwogqR23/eKI51hky4CUFzjZccZ5sU54j5FPHJ2HV2fzMe1FI0yaE ZpOq2cx+eOuVIEY1vNyFtsMaE8g859yTsPxhe7rs= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Paul Elder Subject: [PATCH v2 16/32] ipa: rkisp1: agc: Process frame duration at the right time Date: Wed, 25 Mar 2026 16:13:48 +0100 Message-ID: <20260325151416.2114564-17-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The frame duration and vblank should not be calculated during process() but within prepare(), where the data for that frame get's computed. In raw mode, process is not called, so also update it in queueRequest(). Signed-off-by: Stefan Klug Reviewed-by: Paul Elder --- Changes in v2: - Squashed with next patch - Collected tag --- src/ipa/rkisp1/algorithms/agc.cpp | 23 +++++++++++++---------- src/ipa/rkisp1/algorithms/agc.h | 3 +-- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp index fd227e2129ed..4a7b8988221d 100644 --- a/src/ipa/rkisp1/algorithms/agc.cpp +++ b/src/ipa/rkisp1/algorithms/agc.cpp @@ -340,6 +340,9 @@ void Agc::queueRequest(IPAContext &context, } frameContext.agc.minFrameDuration = agc.minFrameDuration; frameContext.agc.maxFrameDuration = agc.maxFrameDuration; + + /* V-blank needs to be valid for the start controls handling. Update it. */ + processFrameDuration(context, frameContext); } /** @@ -383,6 +386,12 @@ void Agc::prepare(IPAContext &context, const uint32_t frame, frameContext.agc.yTarget = context.activeState.agc.automatic.yTarget; + /* + * Expand the target frame duration so that we do not run faster than + * the minimum frame duration when we have short exposures. + */ + processFrameDuration(context, frameContext); + if (frame > 0 && !frameContext.agc.updateMetering) return; @@ -511,11 +520,13 @@ double Agc::estimateLuminance(double gain) const * Compute and populate vblank from the target frame duration. */ void Agc::processFrameDuration(IPAContext &context, - IPAFrameContext &frameContext, - utils::Duration frameDuration) + IPAFrameContext &frameContext) { IPACameraSensorInfo &sensorInfo = context.sensorInfo; utils::Duration lineDuration = context.configuration.sensor.lineDuration; + utils::Duration frameDuration = frameContext.agc.exposure * lineDuration; + + frameDuration = std::max(frameDuration, frameContext.agc.minFrameDuration); frameContext.agc.vblank = (frameDuration / lineDuration) - sensorInfo.outputSize.height; @@ -539,8 +550,6 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, ControlList &metadata) { if (!stats) { - processFrameDuration(context, frameContext, - frameContext.agc.minFrameDuration); fillMetadata(context, frameContext, metadata); return; } @@ -642,12 +651,6 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame, activeState.agc.automatic.gain = aGain; activeState.agc.automatic.quantizationGain = qGain; activeState.agc.automatic.yTarget = effectiveYTarget(); - /* - * Expand the target frame duration so that we do not run faster than - * the minimum frame duration when we have short exposures. - */ - processFrameDuration(context, frameContext, - std::max(frameContext.agc.minFrameDuration, newExposureTime)); fillMetadata(context, frameContext, metadata); expMeans_ = {}; diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h index 7867eed9c4e3..4432711f43af 100644 --- a/src/ipa/rkisp1/algorithms/agc.h +++ b/src/ipa/rkisp1/algorithms/agc.h @@ -51,8 +51,7 @@ private: ControlList &metadata); double estimateLuminance(double gain) const override; void processFrameDuration(IPAContext &context, - IPAFrameContext &frameContext, - utils::Duration frameDuration); + IPAFrameContext &frameContext); Span expMeans_; Span weights_; From patchwork Wed Mar 25 15:13:49 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26359 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 61131C32F6 for ; Wed, 25 Mar 2026 15:15:30 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 10F0A62C1A; Wed, 25 Mar 2026 16:15:30 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="BpXjWCLU"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1C76A62BC0 for ; Wed, 25 Mar 2026 16:15:29 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 1F8801B98; Wed, 25 Mar 2026 16:14:11 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451651; bh=K0nTSvVIRBMzsU2O5sVVts7K7edW3VA1pbvMF5UEXU8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=BpXjWCLUOG4K9B64zpECQkY+EGLH6XaJotMk1LwSmiUtRhDthrNjreoon5m9SB1Kf 1DGayPRYuOj9JJSSAlqHzaO55EigqU92lim5SYa1oVe8NLNGE7OkUmbwLRWIoEH0iv zpk/yUEeiDyFxVWnj5v1IyTDKDuNl2hEvnlGxwIo= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 17/32] libcamera: delayed_controls: Change semantics of sequence numbers Date: Wed, 25 Mar 2026 16:13:49 +0100 Message-ID: <20260325151416.2114564-18-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" In the context of per frame controls I believe it is easier to think about what needs to be done for a specific sequence number. So (assuming a max sensor delay of 2) the actions would be: - delayedControls.push(n) stores the controls that shall be active on frame n - delayedControls.get(n) returns these controls again - delayedControls.apply(n) applies the slowest control for frame n and does a look back on other controls. So when a frameStart for frame n occurs, it is time to call delayedControls.apply(n + maxDelay) Changing these semantics on delayed controls doesn't require much code change and has the added benefit that we don't run into clamping for get() on frames < maxDelay. ToDo: This breaks the delayed controls test at the moment. The tests need to be fixed. This is not straight forward as the tests need a deeper overhaul. Signed-off-by: Stefan Klug --- Changes in v2: - Applied the semantics change to other pipelines as well. --- src/libcamera/delayed_controls.cpp | 15 ++++++++------- src/libcamera/pipeline/ipu3/ipu3.cpp | 2 +- src/libcamera/pipeline/mali-c55/mali-c55.cpp | 6 ++++-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++- test/meson.build | 2 +- 5 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index 71071a0c670d..4efe3b39c3f6 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -240,7 +240,7 @@ bool DelayedControls::push(uint32_t sequence, const ControlList &controls) */ ControlList DelayedControls::get(uint32_t sequence) { - unsigned int index = std::max(0, sequence - maxDelay_); + unsigned int index = sequence; ControlList out(device_->controls()); for (const auto &ctrl : values_) { @@ -266,13 +266,14 @@ 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 + * \brief Apply controls for a frame + * \param[in] sequence Sequence number of the frame to apply * - * 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. + * Apply controls for the frame \a sequence. This applies the controls with the + * largest delay. For controls with a smaller delay it does a look back and + * applies the controls for the previous sequence. So usually this function is + * called in a start of exposure event as applyControls(startedSequence + + * maxDelay) */ void DelayedControls::applyControls(uint32_t sequence) { diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index bac6f1c2ef40..4e7e8cd49e13 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -1385,7 +1385,7 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer) */ void IPU3CameraData::frameStart(uint32_t sequence) { - delayedCtrls_->applyControls(sequence); + delayedCtrls_->applyControls(sequence + delayedCtrls_->maxDelay()); if (processingRequests_.empty()) return; diff --git a/src/libcamera/pipeline/mali-c55/mali-c55.cpp b/src/libcamera/pipeline/mali-c55/mali-c55.cpp index c209b0b070b1..2a8bbca098bb 100644 --- a/src/libcamera/pipeline/mali-c55/mali-c55.cpp +++ b/src/libcamera/pipeline/mali-c55/mali-c55.cpp @@ -1615,8 +1615,10 @@ bool PipelineHandlerMaliC55::registerSensorCamera(MediaLink *ispLink) data->delayedCtrls_ = std::make_unique(data->sensor_->device(), params); - isp_->frameStart.connect(data->delayedCtrls_.get(), - &DelayedControls::applyControls); + isp_->frameStart.connect(data->delayedCtrls_.get(), [&](uint32_t seq) { + uint32_t lookahead = data->delayedCtrls_->maxDelay(); + data->delayedCtrls_->applyControls(seq + lookahead); + }); /* \todo Init properties. */ diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 89e8ab0999f4..b70c551fc775 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -1523,7 +1523,8 @@ void PipelineHandlerRkISP1::frameStart(uint32_t sequence) return; RkISP1CameraData *data = cameraData(activeCamera_); - data->delayedCtrls_->applyControls(sequence); + uint32_t sequenceToApply = sequence + data->delayedCtrls_->maxDelay(); + data->delayedCtrls_->applyControls(sequenceToApply); } bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) diff --git a/test/meson.build b/test/meson.build index 52f04364e4fc..80fb543f2201 100644 --- a/test/meson.build +++ b/test/meson.build @@ -53,7 +53,7 @@ internal_tests = [ {'name': 'bayer-format', 'sources': ['bayer-format.cpp']}, {'name': 'byte-stream-buffer', 'sources': ['byte-stream-buffer.cpp']}, {'name': 'camera-sensor', 'sources': ['camera-sensor.cpp']}, - {'name': 'delayed_controls', 'sources': ['delayed_controls.cpp']}, + {'name': 'delayed_controls', 'sources': ['delayed_controls.cpp'], 'should_fail': true}, {'name': 'event', 'sources': ['event.cpp']}, {'name': 'event-dispatcher', 'sources': ['event-dispatcher.cpp']}, {'name': 'event-thread', 'sources': ['event-thread.cpp']}, From patchwork Wed Mar 25 15:13:50 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26360 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 20BCCC3302 for ; Wed, 25 Mar 2026 15:15:34 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 9B0C962C3D; Wed, 25 Mar 2026 16:15:33 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="BBL8VjlX"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A776F6274D for ; Wed, 25 Mar 2026 16:15:31 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id B0A901943; Wed, 25 Mar 2026 16:14:13 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451653; bh=EFewKCAt/7JAXms7yDaFl2tWUeG+Y0w4vVz6NT4aA/A=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=BBL8VjlX4prXE/fovZXaBsjasQ5dAc3cLXxHFW9twH6NxvdVg9umPPJCY/Rnsn48Y hrpP7CwfXNmSf711na/4v7uZCHDEp5yt9dPfU6c7NQEDTS96G6I6s8uLzmBjJYxS7O 3u/ga9VpvSB/xNhinNIr0LjJcH/y2xu5nLXQXGWM= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 18/32] libipa: algorithm: Update docs Date: Wed, 25 Mar 2026 16:13:50 +0100 Message-ID: <20260325151416.2114564-19-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Update the algorithm documentation to reflect the changed timing model. Signed-off-by: Stefan Klug --- Changes in v2: - Added more documentation --- src/ipa/libipa/algorithm.cpp | 41 ++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp index 201efdfdba25..e491cb241442 100644 --- a/src/ipa/libipa/algorithm.cpp +++ b/src/ipa/libipa/algorithm.cpp @@ -76,11 +76,13 @@ namespace ipa { * * This function is called for each request queued to the camera. It provides * the controls stored in the request to the algorithm. The \a frame number - * is the Request sequence number and identifies the desired corresponding + * is the sensor sequence number and identifies the desired corresponding * frame to target for the controls to take effect. * * Algorithms shall read the applicable controls and store their value for later - * use during frame processing. + * use during frame processing. All values that are already known (like exposure + * time/gain in manual mode) shall be updated in \a frameContext to support + * cases where prepare() is never called (like in raw mode). */ /** @@ -92,20 +94,29 @@ namespace ipa { * \param[out] params The ISP specific parameters * * This function is called for every frame when the camera is running before it - * is processed by the ISP to prepare the ISP processing parameters for that - * frame. + * is processed by the ISP to prepare the ISP processing parameters and the + * sensor parameters for that frame. * * Algorithms shall fill in the parameter structure fields appropriately to * configure the ISP processing blocks that they are responsible for. This * includes setting fields and flags that enable those processing blocks. + * + * Additionally \a frameContext shall be updated with the most up to date values + * necessary to configure the sensor. After prepare() the \a frameContext for + * this frame shall be treated read only. + * + * \todo: For offline ISPs there might be use cases where it is beneficial to + * separate the calculation of sensor parameters from the calculation of ISP + * paremeters. This is currently not supported. */ /** * \fn Algorithm::process() * \brief Process ISP statistics, and run algorithm operations * \param[in] context The shared IPA context - * \param[in] frame The frame context sequence number - * \param[in] frameContext The current frame's context + * \param[in] frame The frame sequence number that produces the stats + * \param[in] frameContext The frame context for the frame that produced the + * stats * \param[in] stats The IPA statistics and ISP results * \param[out] metadata Metadata for the frame, to be filled by the algorithm * @@ -118,19 +129,17 @@ namespace ipa { * computationally expensive calculations or operations must be handled * asynchronously in a separate thread. * - * Algorithms can store state in their respective IPAFrameContext structures, - * and reference state from the IPAFrameContext of other algorithms. - * - * \todo Historical data may be required as part of the processing. - * Either the previous frame, or the IPAFrameContext state of the frame - * that generated the statistics for this operation may be required for - * some advanced algorithms to prevent oscillations or support control - * loops correctly. Only a single IPAFrameContext is available currently, - * and so any data stored may represent the results of the previously - * completed operations. + * Care must be taken to ensure that the frameContext is only updated in cases + * where the frame was not processed yet. This usually differs between offline + * and inline ISPs. In an inline ISP the stats are received after processing the + * frame. In this case the frame context *must not* be updated. Algorithms + * typically update the active state which is then picked up in prepare(). * * Care shall be taken to ensure the ordering of access to the information * such that the algorithms use up to date state as required. + * + * The \a stats parameter can be null in which case only the frame metadata + * shall be filled with the data from frameContext. */ /** From patchwork Wed Mar 25 15:13:51 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26361 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 BF0BFC3303 for ; Wed, 25 Mar 2026 15:15:36 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 255E962C4E; Wed, 25 Mar 2026 16:15:36 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="FA2YZy+7"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8D2AA62BF0 for ; Wed, 25 Mar 2026 16:15:34 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 8A8571B98; Wed, 25 Mar 2026 16:14:16 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451656; bh=328G/l2Pz1R8U8ujQ2WBNyhtWw2/yBzz2aZhPPiXddU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=FA2YZy+79XnoE4YkiK/ZW4Jyq1Uaf1TlItSJF8j0TBGFp+6QTqYdkl6fbiAp/M99t iuXXbIt/Q1jjsMkLavDnndbNh8T8FYnaPyDoBzRE2WNaxKdkIIJPG+xgWLxf/fWyfy r/T1XsYHq9do+Q4bzsEFPwY6smOpMrWJqXrDRsCU= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Paul Elder Subject: [PATCH v2 19/32] libcamera: delayed_controls: Ignore double pushes for the same frame number Date: Wed, 25 Mar 2026 16:13:51 +0100 Message-ID: <20260325151416.2114564-20-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" For successful PFC a single sequence must only be pushed once to delayed controls. Such a situation can occur if no-ops were pushed in delayed controls due to a buffer underrun. Signed-off-by: Stefan Klug Reviewed-by: Paul Elder --- Changes in v2: - Collated double log message - Collected tag --- src/libcamera/delayed_controls.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libcamera/delayed_controls.cpp b/src/libcamera/delayed_controls.cpp index 4efe3b39c3f6..bcbce4413456 100644 --- a/src/libcamera/delayed_controls.cpp +++ b/src/libcamera/delayed_controls.cpp @@ -173,8 +173,9 @@ bool DelayedControls::push(uint32_t sequence, const ControlList &controls) { if (sequence < queueCount_) { LOG(DelayedControls, Warning) - << "Double push for sequence " << sequence - << " current queue index: " << queueCount_; + << "Ignored double push for sequence " << sequence + << ". Current queue index: " << queueCount_; + return true; } while (sequence > queueCount_) { @@ -277,7 +278,10 @@ ControlList DelayedControls::get(uint32_t sequence) */ void DelayedControls::applyControls(uint32_t sequence) { - LOG(DelayedControls, Debug) << "frame " << sequence << " started"; + LOG(DelayedControls, Debug) + << "Apply controls for: " << sequence + << " (instant controls for frame " + << (sequence - maxDelay_) << ")"; while (queueCount_ - 1 < sequence) { LOG(DelayedControls, Warning) From patchwork Wed Mar 25 15:13:52 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26362 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 36277C32F7 for ; Wed, 25 Mar 2026 15:15:39 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D02AF62C4E; Wed, 25 Mar 2026 16:15:38 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="qILCJV6h"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 37F2D62BF0 for ; Wed, 25 Mar 2026 16:15:37 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 2DD2F1ADE; Wed, 25 Mar 2026 16:14:19 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451659; bh=Nh+/HjRT8Ow0xkr2/Ni0Fr/5sJvQgPPeB+GRIqLevtM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=qILCJV6htDiLKZGrDUUTPSP3ce1t3k+y7+bOLVmxVLJA61worFRKpzmmiTr2lqQPP JFpBDcPzmHzdY4umYIoVVG4MH4mdvlnUCLSnZUYIfWPKO7LdqexIYTlg/PSA9JyxY2 OvPNF0bu3euZCzizAUs5NTapZkOseJWy0LpgvbpI= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Jacopo Mondi Subject: [PATCH v2 20/32] libcamera: v4l2_videodevice: Do not hide frame drops Date: Wed, 25 Mar 2026 16:13:52 +0100 Message-ID: <20260325151416.2114564-21-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Jacopo Mondi Autocorrecting the sequence number in case it doesn't start with 0 produces difficult to debug problems in cases where the first frame got lost. We need to handle these properly in the pipeline handler (and fix kernel drivers if needed). Signed-off-by: Jacopo Mondi Signed-off-by: Stefan Klug --- Changes in v2: - Rewrote commit message. --- include/libcamera/internal/v4l2_videodevice.h | 1 - src/libcamera/v4l2_videodevice.cpp | 15 --------------- 2 files changed, 16 deletions(-) diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h index 57db0036db6b..de91fbd0f8dc 100644 --- a/include/libcamera/internal/v4l2_videodevice.h +++ b/include/libcamera/internal/v4l2_videodevice.h @@ -286,7 +286,6 @@ private: EventNotifier *fdBufferNotifier_; State state_; - std::optional firstFrame_; Timer watchdog_; utils::Duration watchdogDuration_; diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp index d877df29ee6e..2c911a11dba7 100644 --- a/src/libcamera/v4l2_videodevice.cpp +++ b/src/libcamera/v4l2_videodevice.cpp @@ -1904,19 +1904,6 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer() if (V4L2_TYPE_IS_OUTPUT(buf.type)) return buffer; - /* - * Detect kernel drivers which do not reset the sequence number to zero - * on stream start. - */ - if (!firstFrame_.has_value()) { - if (buf.sequence) - LOG(V4L2, Info) - << "Zero sequence expected for first frame (got " - << buf.sequence << ")"; - firstFrame_ = buf.sequence; - } - metadata.sequence -= firstFrame_.value(); - Span framebufferPlanes = buffer->planes(); unsigned int numV4l2Planes = multiPlanar ? buf.length : 1; @@ -1993,8 +1980,6 @@ int V4L2VideoDevice::streamOn() { int ret; - firstFrame_.reset(); - ret = ioctl(VIDIOC_STREAMON, &bufferType_); if (ret < 0) { LOG(V4L2, Error) From patchwork Wed Mar 25 15:13:53 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26363 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 D22B9C3304 for ; Wed, 25 Mar 2026 15:15:41 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 631C762C7D; Wed, 25 Mar 2026 16:15:41 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="ZBVuDrcg"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8B01B62C44 for ; Wed, 25 Mar 2026 16:15:40 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 73CE71992; Wed, 25 Mar 2026 16:14:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451662; bh=ZvkKBAI6BWqkHPAtWOtsQMkeg8HSUtRWRmQVERiUozw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ZBVuDrcgyyGmNGJqBEAn6Ri8ooEEJ6hkVroYJ/jLLUbgFCviJGy+AwtgOInaaxQCX EvWp04t5igVCsxOp/fK/EkwZ8+GZut+n9pMUstDK052/son5c3LdCk+0jrZgFvK9LD gcn/gSxcvoxVizYUz7RZQSFs2ZdhXK4FgiDZi9CQ= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 21/32] ipa: rkisp1: Allow processStats() to be called without stats buffer Date: Wed, 25 Mar 2026 16:13:53 +0100 Message-ID: <20260325151416.2114564-22-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" When there are no stats available for a frame, it still makes sense to call processStats() to fill in the metadata of that frame. This mechanism is already used for the raw path. Allow it's use for non-raw also. The current code never produces buffers with id 0, but it is not enforced. Add a assert to enforce that. Signed-off-by: Stefan Klug --- Changes in v2: - Added an assert to ensure there is no buffer with id 0 --- src/ipa/rkisp1/rkisp1.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 3f943a08d011..88cf6afc219d 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -304,6 +304,9 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig, void IPARkISP1::mapBuffers(const std::vector &buffers) { for (const IPABuffer &buffer : buffers) { + /* A buffer id of 0 is considered invalid */ + ASSERT(buffer.id != 0); + auto elem = buffers_.emplace(std::piecewise_construct, std::forward_as_tuple(buffer.id), std::forward_as_tuple(buffer.planes)); @@ -389,7 +392,7 @@ void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId, * provided. */ const rkisp1_stat_buffer *stats = nullptr; - if (!context_.configuration.raw) + if (bufferId != 0) stats = reinterpret_cast( mappedBuffers_.at(bufferId).planes()[0].data()); From patchwork Wed Mar 25 15:13:54 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26364 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 768C1C3305 for ; Wed, 25 Mar 2026 15:15:45 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E932A62CB1; Wed, 25 Mar 2026 16:15:44 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Z3dVqU0S"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 4699162C44 for ; Wed, 25 Mar 2026 16:15:43 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 3005B1AFB; Wed, 25 Mar 2026 16:14:25 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451665; bh=ZXy9ncr79oa95/Dp9fjeSQ8CeGOzW7j447IMXY2MLuw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Z3dVqU0SMyeZ8oqvTxTbIO3sZIo3olKW+vCVjBfk4p0EYjsvqjz4xS+AqL3TlW9by 2fLQKCZ8Aq7dy6Q6UNk2UkYxs0q2x0655WfVZBB6JzebFxf2P5NmE95w1AOtWgiqaA 0qPe4iSsBBUwEAp5WWsD5ygeQI/LMQW/qq/4g2mo= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 22/32] ipa: rkisp1: Lazy initialise frame context Date: Wed, 25 Mar 2026 16:13:54 +0100 Message-ID: <20260325151416.2114564-23-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" For per frame control we want to tick the IPA by the sensor frame sequence instead of the request sequence. This has the side effect that the IPA must be able to cope with situations where a frame context is required for a frame that was not queued before (computeParams is called without a corresponding request) or processStats is called for an unexpected sequence number (because a scratch buffer was used on kernel side). With the current FCQueue implementation this is not easy to model, as it has distinct calls for alloc() and get(). Simplify that by passing the FCQueue a callback that it can call to initialize a frame context when needed. This has the added benefit that the FCQueue can collate controls for requests that were queued in too late. This simplifies the logic on the IPA side. As fetching an uninitialized frame context is no error anymore, demote the corresponding warnings to debug messages. Signed-off-by: Stefan Klug --- Changes in v2: - Rewrote the patch to use a callback based mechanism. --- src/ipa/ipu3/ipu3.cpp | 18 ++++-- src/ipa/libipa/fc_queue.h | 97 +++++++++++-------------------- src/ipa/mali-c55/mali-c55.cpp | 20 +++++-- src/ipa/rkisp1/algorithms/awb.cpp | 2 + src/ipa/rkisp1/rkisp1.cpp | 25 ++++---- src/ipa/simple/soft_simple.cpp | 18 ++++-- 6 files changed, 92 insertions(+), 88 deletions(-) diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp index 92f5bd072134..ed4ea8d31866 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -172,6 +172,8 @@ private: void setControls(unsigned int frame); void calculateBdsGrid(const Size &bdsOutputSize); + void initializeFrameContext(IPAFrameContext &frameContext, + const ControlList &controls); std::map buffers_; @@ -190,6 +192,10 @@ private: IPAIPU3::IPAIPU3() : context_(kMaxFrameContexts) { + context_.frameContexts.setInitCallback( + [this](IPAFrameContext &fc, const ControlList &c) { + this->initializeFrameContext(fc, c); + }); } std::string IPAIPU3::logPrefix() const @@ -561,7 +567,7 @@ void IPAIPU3::computeParams(const uint32_t frame, const uint32_t bufferId) */ params->use = {}; - IPAFrameContext &frameContext = context_.frameContexts.get(frame); + IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame); for (const auto &algo : algorithms()) algo->prepare(context_, frame, frameContext, params); @@ -594,7 +600,7 @@ void IPAIPU3::processStats(const uint32_t frame, const ipu3_uapi_stats_3a *stats = reinterpret_cast(mem.data()); - IPAFrameContext &frameContext = context_.frameContexts.get(frame); + IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame); frameContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get(); frameContext.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get()); @@ -627,10 +633,14 @@ void IPAIPU3::processStats(const uint32_t frame, */ void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls) { - IPAFrameContext &frameContext = context_.frameContexts.alloc(frame); + context_.frameContexts.getOrInitContext(frame, controls); +} +void IPAIPU3::initializeFrameContext(IPAFrameContext &frameContext, + const ControlList &controls) +{ for (const auto &algo : algorithms()) - algo->queueRequest(context_, frame, frameContext, controls); + algo->queueRequest(context_, frameContext.frame(), frameContext, controls); } /** diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h index 812022c496ed..633bf646d88d 100644 --- a/src/ipa/libipa/fc_queue.h +++ b/src/ipa/libipa/fc_queue.h @@ -11,6 +11,7 @@ #include #include +#include namespace libcamera { @@ -27,52 +28,33 @@ struct FrameContext { private: template friend class FCQueue; uint32_t frame_; - bool initialised_ = false; }; template class FCQueue { public: + using InitCallback = std::function; + FCQueue(unsigned int size) : contexts_(size) { } + void setInitCallback(const InitCallback &cb) + { + initCallback_ = cb; + } + void clear() { for (FC &ctx : contexts_) { - ctx.initialised_ = false; ctx.frame_ = 0; } + initialized_ = false; } - FC &alloc(const uint32_t frame) - { - FC &fc = contexts_[frame % contexts_.size()]; - FrameContext &frameContext = fc; - - /* - * Do not re-initialise if a get() call has already fetched this - * frame context to preseve the context. - * - * \todo If the the sequence number of the context to initialise - * is smaller than the sequence number of the queue slot to use, - * it means that we had a serious request underrun and more - * frames than the queue size has been produced since the last - * time the application has queued a request. Does this deserve - * an error condition ? - */ - if (frame != 0 && frame <= frameContext.frame_) - LOG(FCQueue, Warning) - << "Frame " << frame << " already initialised"; - else - init(fc, frame); - - return fc; - } - - FC &get(uint32_t frame) + FC &getOrInitContext(unsigned int frame, const ControlList &controls = {}) { FC &fc = contexts_[frame % contexts_.size()]; FrameContext &frameContext = fc; @@ -90,51 +72,42 @@ public: << " has been overwritten by " << frameContext.frame_; - if (frame == 0 && !frameContext.initialised_) { - /* - * If the IPA calls get() at start() time it will get an - * un-intialized FrameContext as the below "frame == - * frameContext.frame_" check will return success - * because FrameContexts are zeroed at creation time. - * - * Make sure the FrameContext gets initialised if get() - * is called before alloc() by the IPA for frame#0. - */ - init(fc, frame); - + if (initialized_ && frame == frameContext.frame_) { + if (!controls.empty()) { + /* Too late to apply the controls. Store them for later. */ + LOG(FCQueue, Warning) + << "Request underrun. Controls for frame " + << frame << " are delayed "; + controlsToApply_.merge(controls, + ControlList::MergePolicy::OverwriteExisting); + } + LOG(FCQueue, Debug) << "Got " << frame; return fc; } - if (frame == frameContext.frame_) - return fc; + const ControlList *controls2 = &controls; + if (!controlsToApply_.empty()) { + LOG(FCQueue, Debug) << "Applied late controls on frame" << frame; + controlsToApply_.merge(controls, ControlList::MergePolicy::OverwriteExisting); + controls2 = &controlsToApply_; + } - /* - * The frame context has been retrieved before it was - * initialised through the initialise() call. This indicates an - * algorithm attempted to access a Frame context before it was - * queued to the IPA. Controls applied for this request may be - * left unhandled. - * - * \todo Set an error flag for per-frame control errors. - */ - LOG(FCQueue, Warning) - << "Obtained an uninitialised FrameContext for " << frame; + LOG(FCQueue, Debug) << "Init " << frame; - init(fc, frame); + fc = {}; + frameContext.frame_ = frame; + initCallback_(fc, *controls2); + initialized_ = true; + controlsToApply_.clear(); return fc; } private: - void init(FC &fc, const uint32_t frame) - { - fc = {}; - FrameContext &frameContext = fc; - frameContext.frame_ = frame; - frameContext.initialised_ = true; - } - std::vector contexts_; + InitCallback initCallback_; + ControlList controlsToApply_; + bool initialized_; }; } /* namespace ipa */ diff --git a/src/ipa/mali-c55/mali-c55.cpp b/src/ipa/mali-c55/mali-c55.cpp index fd5c9563d6c3..5bd112b5f30a 100644 --- a/src/ipa/mali-c55/mali-c55.cpp +++ b/src/ipa/mali-c55/mali-c55.cpp @@ -71,6 +71,8 @@ private: void updateControls(const IPACameraSensorInfo &sensorInfo, const ControlInfoMap &sensorControls, ControlInfoMap *ipaControls); + void initializeFrameContext(IPAFrameContext &frameContext, + const ControlList &controls); void setControls(); std::map buffers_; @@ -91,6 +93,10 @@ namespace { IPAMaliC55::IPAMaliC55() : context_(kMaxFrameContexts) { + context_.frameContexts.setInitCallback( + [this](IPAFrameContext &fc, const ControlList &c) { + this->initializeFrameContext(fc, c); + }); } std::string IPAMaliC55::logPrefix() const @@ -320,21 +326,25 @@ void IPAMaliC55::unmapBuffers(const std::vector &buffers) } } -void IPAMaliC55::queueRequest(const uint32_t request, const ControlList &controls) +void IPAMaliC55::queueRequest(const uint32_t frame, const ControlList &controls) { - IPAFrameContext &frameContext = context_.frameContexts.alloc(request); + context_.frameContexts.getOrInitContext(frame, controls); +} +void IPAMaliC55::initializeFrameContext(IPAFrameContext &frameContext, + const ControlList &controls) +{ for (const auto &a : algorithms()) { Algorithm *algo = static_cast(a.get()); - algo->queueRequest(context_, request, frameContext, controls); + algo->queueRequest(context_, frameContext.frame(), frameContext, controls); } } void IPAMaliC55::fillParams(unsigned int request, [[maybe_unused]] uint32_t bufferId) { - IPAFrameContext &frameContext = context_.frameContexts.get(request); + IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(request); MaliC55Params params(buffers_.at(bufferId).planes()[0]); for (const auto &algo : algorithms()) @@ -346,7 +356,7 @@ void IPAMaliC55::fillParams(unsigned int request, void IPAMaliC55::processStats(unsigned int request, unsigned int bufferId, const ControlList &sensorControls) { - IPAFrameContext &frameContext = context_.frameContexts.get(request); + IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(request); const mali_c55_stats_buffer *stats = nullptr; stats = reinterpret_cast( diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp index f83da545be85..f2b387e09ac7 100644 --- a/src/ipa/rkisp1/algorithms/awb.cpp +++ b/src/ipa/rkisp1/algorithms/awb.cpp @@ -174,6 +174,8 @@ void Awb::queueRequest(IPAContext &context, awbAlgo_->handleControls(controls); frameContext.awb.autoEnabled = awb.autoEnabled; + frameContext.awb.gains = awb.automatic.gains; + frameContext.awb.temperatureK = awb.automatic.temperatureK; if (awb.autoEnabled) return; diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 88cf6afc219d..d0c753976dd4 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -67,8 +68,7 @@ public: void queueRequest(const uint32_t frame, const ControlList &controls) override; void computeParams(const uint32_t frame, const uint32_t bufferId) override; - void initializeFrameContext(const uint32_t frame, - IPAFrameContext &frameContext, + void initializeFrameContext(IPAFrameContext &frameContext, const ControlList &controls); void processStats(const uint32_t frame, const uint32_t bufferId, const ControlList &sensorControls) override; @@ -131,6 +131,10 @@ const ControlInfoMap::Map rkisp1Controls{ IPARkISP1::IPARkISP1() : context_(kMaxFrameContexts) { + context_.frameContexts.setInitCallback( + [this](IPAFrameContext &fc, const ControlList &c) { + this->initializeFrameContext(fc, c); + }); } std::string IPARkISP1::logPrefix() const @@ -217,8 +221,7 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision, void IPARkISP1::start(const ControlList &controls, StartResult *result) { - IPAFrameContext frameContext = {}; - initializeFrameContext(0, frameContext, controls); + IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(0, controls); result->controls = getSensorControls(frameContext); result->code = 0; } @@ -336,27 +339,23 @@ void IPARkISP1::unmapBuffers(const std::vector &ids) void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls) { - IPAFrameContext &frameContext = context_.frameContexts.alloc(frame); context_.debugMetadata.enableByControl(controls); - - initializeFrameContext(frame, frameContext, controls); + context_.frameContexts.getOrInitContext(frame, controls); } -void IPARkISP1::initializeFrameContext(const uint32_t frame, - IPAFrameContext &frameContext, - const ControlList &controls) +void IPARkISP1::initializeFrameContext(IPAFrameContext &fc, const ControlList &controls) { for (const auto &a : algorithms()) { Algorithm *algo = static_cast(a.get()); if (algo->disabled_) continue; - algo->queueRequest(context_, frame, frameContext, controls); + algo->queueRequest(context_, fc.frame(), fc, controls); } } void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId) { - IPAFrameContext &frameContext = context_.frameContexts.get(frame); + IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame); /* * \todo: This needs discussion. In raw mode, computeParams is @@ -385,7 +384,7 @@ void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId) void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId, const ControlList &sensorControls) { - IPAFrameContext &frameContext = context_.frameContexts.get(frame); + IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame); /* * In raw capture mode, the ISP is bypassed and no statistics buffer is diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index 7d25bdd26017..394107606713 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -47,6 +47,10 @@ public: IPASoftSimple() : context_(kMaxFrameContexts) { + context_.frameContexts.setInitCallback( + [this](IPAFrameContext &fc, const ControlList &c) { + this->initializeFrameContext(fc, c); + }); } ~IPASoftSimple(); @@ -73,6 +77,8 @@ protected: private: void updateExposure(double exposureMSV); + void initializeFrameContext(IPAFrameContext &frameContext, + const ControlList &controls); DebayerParams *params_; SwIspStats *stats_; @@ -277,17 +283,21 @@ void IPASoftSimple::stop() void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &controls) { - IPAFrameContext &frameContext = context_.frameContexts.alloc(frame); + context_.frameContexts.getOrInitContext(frame, controls); +} +void IPASoftSimple::initializeFrameContext(IPAFrameContext &frameContext, + const ControlList &controls) +{ for (const auto &algo : algorithms()) - algo->queueRequest(context_, frame, frameContext, controls); + algo->queueRequest(context_, frameContext.frame(), frameContext, controls); } void IPASoftSimple::computeParams(const uint32_t frame) { context_.activeState.combinedMatrix = Matrix::identity(); - IPAFrameContext &frameContext = context_.frameContexts.get(frame); + IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame); for (const auto &algo : algorithms()) algo->prepare(context_, frame, frameContext, params_); params_->combinedMatrix = context_.activeState.combinedMatrix; @@ -299,7 +309,7 @@ void IPASoftSimple::processStats(const uint32_t frame, [[maybe_unused]] const uint32_t bufferId, const ControlList &sensorControls) { - IPAFrameContext &frameContext = context_.frameContexts.get(frame); + IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame); frameContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get(); From patchwork Wed Mar 25 15:13:55 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26365 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 DD8B1BE086 for ; Wed, 25 Mar 2026 15:15:47 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 7977862CC9; Wed, 25 Mar 2026 16:15:47 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="TX4+K4z4"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 4773462C44 for ; Wed, 25 Mar 2026 16:15:46 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 4E3C41B98; Wed, 25 Mar 2026 16:14:28 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451668; bh=3OpmvkR+XHCPZ8nSjonhVBpIOe4SDmuym/y2EZWkYfU=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=TX4+K4z4CoxWSUeO1VCvhq2cIpBnOP/d+RBJiCDITDckef1ilNkCOoAcnQ18vgJtS Dj33LQvp8id40op2cYBtUoVl1U6JNtmnbggalY+7dbFg7/0+BDe4PDyUPfQTC0T60q NycLa5/szAk7QCC3cOq+HIUhQz4iAcnt+knErECM= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 23/32] libcamera: internal: Add SequenceSyncHelper class Date: Wed, 25 Mar 2026 16:13:55 +0100 Message-ID: <20260325151416.2114564-24-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" On a V4L2 buffer the assigned sequence is not known until the buffer is dequeued. But for per frame controls we have to prepare other data like sensor controls and ISP params in advance. So we try to anticipate the sequence number a given buffer will be. In a perfect world this works well as long as the initial sequence is assigned correctly. But it breaks as soon as things like running out of buffers or incomplete images happen. To make things even more complicated, in most cases more than one buffer is queued to the kernel at a time. So as soon as a sequence number doesn't match the expected one after dequeuing, most likely all the already queued buffers will be dequeued with the same error. It is not sufficient to simply add the correction after dequeuing because the error on all queued frames would accumulate and the whole system starts to oscillate. To work around that add a SequenceSyncHelper class that tracks the expected error and allows to easily query the necessary correction when queuing new buffers. Signed-off-by: Stefan Klug --- Todo: - Add class documentation Changes in v2: - Moved files to man src dir, to be able to reuse it in other pipelines - Added cancel() function. --- include/libcamera/internal/meson.build | 1 + .../libcamera/internal/sequence_sync_helper.h | 76 +++++++++++++++++++ src/libcamera/meson.build | 1 + src/libcamera/sequence_sync_helper.cpp | 21 +++++ 4 files changed, 99 insertions(+) create mode 100644 include/libcamera/internal/sequence_sync_helper.h create mode 100644 src/libcamera/sequence_sync_helper.cpp diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build index 4d2a09bd7041..80c425e13ccd 100644 --- a/include/libcamera/internal/meson.build +++ b/include/libcamera/internal/meson.build @@ -40,6 +40,7 @@ libcamera_internal_headers = files([ 'process.h', 'pub_key.h', 'request.h', + 'sequence_sync_helper.h', 'shared_mem_object.h', 'source_paths.h', 'sysfs.h', diff --git a/include/libcamera/internal/sequence_sync_helper.h b/include/libcamera/internal/sequence_sync_helper.h new file mode 100644 index 000000000000..407c6383dca6 --- /dev/null +++ b/include/libcamera/internal/sequence_sync_helper.h @@ -0,0 +1,76 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2025, Ideas on Board + * + * Sequence sync helper + */ + +#pragma once + +#include + +#include + +namespace libcamera { + +LOG_DECLARE_CATEGORY(SequenceSyncHelper) + +class SequenceSyncHelper +{ +public: + int gotFrame(size_t expectedSequence, size_t actualSequence) + { + ASSERT(!corrections_.empty()); + int diff = actualSequence - expectedSequence; + int corr = corrections_.front(); + corrections_.pop(); + expectedOffset_ -= corr; + int necessaryCorrection = diff - expectedOffset_; + correctionToApply_ += necessaryCorrection; + + LOG(SequenceSyncHelper, Debug) << "Sync frame " + << "expected: " << expectedSequence + << " actual: " << actualSequence + << " correction: " << corr + << " expectedOffset: " << expectedOffset_ + << " correctionToApply " << correctionToApply_; + + expectedOffset_ += necessaryCorrection; + return necessaryCorrection; + } + + void cancelFrame() + { + int corr = corrections_.front(); + corrections_.pop(); + expectedOffset_ -= corr; + } + + /* Value to be added to the source sequence */ + int correction() + { + return correctionToApply_; + } + + void pushCorrection(int correction) + { + corrections_.push(correction); + correctionToApply_ -= correction; + LOG(SequenceSyncHelper, Debug) + << "Push correction " << correction + << " correctionToApply " << correctionToApply_; + } + + void reset() + { + corrections_ = {}; + correctionToApply_ = 0; + expectedOffset_ = 0; + } + + std::queue corrections_; + int correctionToApply_ = 0; + int expectedOffset_ = 0; +}; + +} /* namespace libcamera */ diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index d15943586300..8b82555a5e81 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -49,6 +49,7 @@ libcamera_internal_sources = files([ 'pipeline_handler.cpp', 'process.cpp', 'pub_key.cpp', + 'sequence_sync_helper.cpp', 'shared_mem_object.cpp', 'source_paths.cpp', 'sysfs.cpp', diff --git a/src/libcamera/sequence_sync_helper.cpp b/src/libcamera/sequence_sync_helper.cpp new file mode 100644 index 000000000000..7f0b9c9111a7 --- /dev/null +++ b/src/libcamera/sequence_sync_helper.cpp @@ -0,0 +1,21 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2026, Ideas on Board. + * + * Helper to easily record debug metadata inside libcamera. + */ + +#include "libcamera/internal/sequence_sync_helper.h" + +#include + +namespace libcamera { + +LOG_DEFINE_CATEGORY(SequenceSyncHelper) + +/** + * \file debug_controls.h + * \brief Helper to synchronize buffer sequences + */ + +} /* namespace libcamera */ From patchwork Wed Mar 25 15:13:56 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26366 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 9D1DBC3306 for ; Wed, 25 Mar 2026 15:15:51 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 118BE62CC5; Wed, 25 Mar 2026 16:15:51 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="bpPzCe/a"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 15F3462CB1 for ; Wed, 25 Mar 2026 16:15:49 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id EB8301862; Wed, 25 Mar 2026 16:14:30 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451671; bh=UqvxXSxT4S9Qzmc+IYgd7Sf4uCo3mJ5q/3Ivvy/ssgo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=bpPzCe/ar7MAnKLVC6HcrdDm2jW8wIokJY7eSV8HWGzlow6/8oqWJD2ckYzdQFb/O YxZMC4MGVpyaww+mtgSXGHaKMQH/7EoDRHvjqH12JD9yvurC0OQmP6P93Cygif2qkD IaAHvU1GDhWU2WY5nXG+8C6tr1Ssy0iEVzq9uYj0= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 24/32] libcamera: internal: Add a BufferQueue class to handle buffer queues Date: Wed, 25 Mar 2026 16:13:56 +0100 Message-ID: <20260325151416.2114564-25-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Add a class that encapsulates a queue of v4l2 buffers and the typical use-cases. This simplifies manual queue management and helps in cases where a pre or postprocessing stage is needed. Signed-off-by: Stefan Klug --- Changes in v2: - Added this patch --- include/libcamera/internal/buffer_queue.h | 131 +++++++ include/libcamera/internal/meson.build | 1 + src/libcamera/buffer_queue.cpp | 449 ++++++++++++++++++++++ src/libcamera/meson.build | 1 + 4 files changed, 582 insertions(+) create mode 100644 include/libcamera/internal/buffer_queue.h create mode 100644 src/libcamera/buffer_queue.cpp diff --git a/include/libcamera/internal/buffer_queue.h b/include/libcamera/internal/buffer_queue.h new file mode 100644 index 000000000000..d5904baeed90 --- /dev/null +++ b/include/libcamera/internal/buffer_queue.h @@ -0,0 +1,131 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2025, Ideas on Board + * + * Sequence sync helper + */ + +#pragma once + +#include +#include + +#include +#include + +#include + +#include "sequence_sync_helper.h" + +namespace libcamera { + +LOG_DECLARE_CATEGORY(RkISP1Schedule) + +struct BufferQueueDelegateBase { + virtual ~BufferQueueDelegateBase() = default; + virtual int allocateBuffers(unsigned int count, + std::vector> *buffers) = 0; + virtual int importBuffers(unsigned int count) = 0; + virtual int releaseBuffers() = 0; + + virtual int queueBuffer(FrameBuffer *buffer) = 0; + + Signal bufferReady; +}; + +template +struct BufferQueueDelegate : public BufferQueueDelegateBase { + BufferQueueDelegate(T *video) : video_(video) + { + video_->bufferReady.connect(this, [this](FrameBuffer *buffer) { + this->bufferReady.emit(buffer); + }); + } + + int allocateBuffers(unsigned int count, + std::vector> *buffers) override + { + return video_->allocateBuffers(count, buffers); + } + + int importBuffers(unsigned int count) override + { + return video_->importBuffers(count); + } + + int queueBuffer(FrameBuffer *buffer) override + { + return video_->queueBuffer(buffer); + } + + int releaseBuffers() override + { + return video_->releaseBuffers(); + } + +private: + T *video_; +}; + +class BufferQueue +{ +public: + enum State { + Idle = 0, + Preparing, + Capturing, + Postprocessing + }; + + enum Flags { + PrepareStage = 1, + PostprocessStage = 2 + }; + + BufferQueue(std::unique_ptr &&delegate, int flags = 0, std::string name = {}); + + int allocateBuffers(unsigned int count); + int importBuffers(unsigned int count); + int releaseBuffers(); + + int sequenceCorrection(); + uint32_t nextSequence(); + + int prepareBuffer(uint32_t *sequence = nullptr); + int prepareBuffer(FrameBuffer *buffer, uint32_t *sequence = nullptr); + int preparedBuffer(); + + int queueBuffer(uint32_t *sequence = nullptr); + int queueBuffer(FrameBuffer *buffer, uint32_t *sequence = nullptr); + + void postprocessedBuffer(); + + bool empty(State state); + + FrameBuffer *front(State state); + + unsigned int expectedSequence(FrameBuffer *buffer) const; + const std::vector> &buffers() const; + + Signal bufferReady; + +protected: + void onBufferReady(FrameBuffer *buffer); + + int internalPrepareBuffer(FrameBuffer *buffer, uint32_t *sequence = nullptr); + int internalPreparedBuffer(); + void internalPostprocessedBuffer(); + + std::map> bufferState_; + std::map expectedSequence_; + std::vector> buffers_; + SequenceSyncHelper syncHelper_; + uint32_t nextSequence_; + std::string name_; + bool ownsBuffers_; + bool hasBuffers_; + int flags_; + std::unique_ptr delegate_; +}; + +} /* namespace libcamera */ diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build index 80c425e13ccd..8d25629daa24 100644 --- a/include/libcamera/internal/meson.build +++ b/include/libcamera/internal/meson.build @@ -4,6 +4,7 @@ subdir('tracepoints') libcamera_internal_headers = files([ 'bayer_format.h', + 'buffer_queue.h', 'byte_stream_buffer.h', 'camera.h', 'camera_controls.h', diff --git a/src/libcamera/buffer_queue.cpp b/src/libcamera/buffer_queue.cpp new file mode 100644 index 000000000000..46dbdd809d08 --- /dev/null +++ b/src/libcamera/buffer_queue.cpp @@ -0,0 +1,449 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2025, Ideas on Board + * + * BufferQueue implementation + */ + +#include "libcamera/internal/buffer_queue.h" + +#include + +namespace libcamera { + +LOG_DEFINE_CATEGORY(BufferQueue) + +/** + * \class BufferQueue + * \brief Helper class to handle buffer queues + * + * Handling buffer queues is a common task when dealing with V4L2 video device. + * Depending on the specific use case, additional functionalities are required: + * - Supporting internally allocated and imported buffers + * - Estimate the sequence number that a buffer will have after dequeuing + * - Correct for errors in that sequence scheme due to frames beeing dropped in + * the kernel. + * - Optionally adding a preparation stage for a buffer before it get's queued + * to the device + * - Optionally adding a postprocessing stage after dequeueing + * + * This class encapsulates these functionalities in one component. To support + * arbitrary V4L2VideoDevice like classes, the actual access to the buffer + * related functions is handled through the BufferQueueDelegateBase interface + * with BufferQueueDelegate providing a default implementation that works with + * the V4L2VideDevice class. + * + * On construction time, it must be specified if a preparation stage and/or a + * prostprocessing stage shall be added. + * + * Internally there are 4 queues, a buffer can be in: + * Idle, Preparing, Capturing, Postprocessing. + * + * After creation of the BufferQueue it is important to route all buffer related + * actions through the queue. That is: + * 1. Allocation/import of buffers + * 2. Queueing buffers + * 3. Releasing buffers + * 4. Handling the bufferReady signal. + * + * The callable functions depend on the flags passed to the constructor and are + * shown on the following state diagram. + * + * *------* + * | Idle | + * *------* + * | +----------------+ + * +-- prepareBuffer() ---->| Preparing | + * | +----------------+ + * | | + * | preparedBuffer() + * | | + * | +----------------+ + * \-- queueBuffer() ------>| Capturing | + * +----------------+ + * | + * +----------------+ + * | Postprocessing | --> bufferReady signal + * +----------------+ + * / | + * /-------------------------------/ | postprocessedBuffer() + * | /--------------------------------/ + * *------* + * | Idle | + * *------* + * + * Notes: + * - If buffers are not allocated by the queue but imported they never end up in + * the idle queue but are passed in by prepareBuffer()/queueBuffer() and leave + * the Queue after postprocessing. + * - If a preparing stage is used, queueBuffer can not be called. + * - If the postprocessing stage is disabled, it will still be used while + * emitting the bufferReady signal but the transition to idle happens + * automatically afterwards. + */ + +/** + * \brief Construct a BufferQueue + * \param[in] delegate The delegate + * \param[in] flags Optional flags + * \param[in] name Optional name + * + * Construct a buffer queue using the given delegate to forward the buffer + * handling to. The default queue has only Idle and queued states. This can be + * changed using the \a flags parameter, to either add a prepare stage, a + * postprocess stage or both. + */ +BufferQueue::BufferQueue(std::unique_ptr &&delegate, + int flags, std::string name) + : nextSequence_(0), name_(std::move(name)), ownsBuffers_(false), + hasBuffers_(false), flags_(flags), delegate_(std::move(delegate)) +{ + delegate_->bufferReady.connect(this, &BufferQueue::onBufferReady); +} + +/** + * \brief Allocate buffers + * \param[in] count The number of buffers to allocate + * + * This function allocates \a count buffers by calling allocateBuffers() on the + * delegate and forwarding the return value. A non negative return code is + * treated as success. The buffers are owned by the BufferQueue. + * + * \return The value returned by the BufferQueueDelegateBase::allocateBuffers() + */ +int BufferQueue::allocateBuffers(unsigned int count) +{ + ASSERT(!hasBuffers_); + buffers_.clear(); + int ret = delegate_->allocateBuffers(count, &buffers_); + if (ret < 0) + return ret; + + for (const auto &buffer : buffers_) + bufferState_[Idle].push_back(buffer.get()); + + hasBuffers_ = true; + ownsBuffers_ = true; + return ret; +} + +/** + * \brief Import buffers + * \param[in] count The number of buffers to import + * + * This function imports \a count buffers by calling importBuffers() on the + * delegate and forwarding the return value. A non negative return code is + * treated as success. + * + * \return The value returned by the BufferQueueDelegateBase::importBuffers() + */ +int BufferQueue::importBuffers(unsigned int count) +{ + int ret = delegate_->importBuffers(count); + if (ret < 0) + return ret; + + hasBuffers_ = true; + ownsBuffers_ = false; + return 0; +} + +int BufferQueue::sequenceCorrection() +{ + return syncHelper_.correction(); +} + +uint32_t BufferQueue::nextSequence() +{ + return nextSequence_ + syncHelper_.correction(); +} + +/** + * \brief Move the next buffer to prepare state + * \param[out] sequence The expected sequence of the buffer + * + * This function moves the front buffer from the idle queue to prepare queue. If + * \a sequence is provided it is set to the expected sequence number of that + * buffer. + * + * \note This function must only be called if the queue has a prepare state and + * owns the buffers. + * + * \return 0 on success, a negative error code otherwise + */ +int BufferQueue::prepareBuffer(uint32_t *sequence) +{ + ASSERT(hasBuffers_); + ASSERT(ownsBuffers_); + ASSERT(flags_ & PrepareStage); + ASSERT(!bufferState_[Idle].empty()); + + FrameBuffer *buffer = bufferState_[Idle].front(); + return prepareBuffer(buffer, sequence); +} + +/** + * \brief Move a buffer to prepare state + * \param[in] buffer The buffer + * \param[out] sequence The expected sequence of the buffer + * + * This function moves \a buffer to prepare queue. If + * \a sequence is provided it is set to the expected sequence number of that + * buffer. + * + * \note This function must only be called if the queue has a prepare state. If + * the queue owns the buffer, \a buffer must point to the front buffer of the + * idle queue. + * + * \return 0 on success, a negative error code otherwise + */ +int BufferQueue::prepareBuffer(FrameBuffer *buffer, uint32_t *sequence) +{ + ASSERT(flags_ & PrepareStage); + + return internalPrepareBuffer(buffer, sequence); +} + +/** + * \brief Exit prepare state + * + * This function pops the frontmost buffer from the prepare queue and queues it + * on the underlying device by calling queueBuffer() on the delegate. + * + * \note This function must only be called if the queue has a prepare state. + * + * \return 0 on success, a negative error code otherwise + */ +int BufferQueue::preparedBuffer() +{ + ASSERT(flags_ & PrepareStage); + + return internalPreparedBuffer(); +} + +/** + * \brief Queue the next buffer + * \param[out] sequence The expected sequence of the buffer + * + * This function queues the front buffer from the idle queue to the underlying + * device ba calling queueBuffer() on the delegate. If \a sequence is provided + * it is set to the expected sequence number of that buffer. + * + * \note This function must only be called if the queue does not have a prepare + * state and owns the buffers. + * + * \return 0 on success, a negative error code otherwise + */ +int BufferQueue::queueBuffer(uint32_t *sequence) +{ + ASSERT(hasBuffers_); + ASSERT(ownsBuffers_); + ASSERT(!bufferState_[Idle].empty()); + + FrameBuffer *buffer = bufferState_[Idle].front(); + return queueBuffer(buffer, sequence); +} + +/** + * \brief Queue a buffer + * \param[in] buffer The buffer + * \param[out] sequence The expected sequence of the buffer + * + * This function queues \a buffer to the underlying device ba calling + * queueBuffer() on the delegate. If \a sequence is provided it is set to the + * expected sequence number of that buffer. + * + * \note This function must only be called if the queue does not have a prepare + * state. If the queue owns the buffers, \a buffer must point to the front + * buffer of the idle queue. + * + * \return 0 on success, a negative error code otherwise + */ +int BufferQueue::queueBuffer(FrameBuffer *buffer, uint32_t *sequence) +{ + ASSERT(!(flags_ & PrepareStage)); + return internalPrepareBuffer(buffer, sequence); +} + +/** + * \brief Exit postprocessed state + * + * This function pops the frontmost buffer from the postprocess queue and puts it + * back to the idle queue in case the buffers are owned by the queue + * + * \note This function must only be called if the queue has a prepare state. + */ +void BufferQueue::postprocessedBuffer() +{ + ASSERT(hasBuffers_); + ASSERT(flags_ & PostprocessStage); + return internalPostprocessedBuffer(); +} + +/** + * \brief Release buffers + * + * This function releases the allocated or imported buffers by calling + * releaseBuffers() on the delegate. + * + * \return 0 on success, a negative error code otherwise + */ +int BufferQueue::releaseBuffers() +{ + ASSERT(bufferState_[BufferQueue::Idle].size() == buffers_.size()); + + bufferState_[BufferQueue::Idle] = {}; + buffers_.clear(); + hasBuffers_ = false; + + return delegate_->releaseBuffers(); +} + +/** + * \brief Check if queue is empty + * \param[in] state The state + * + * \return True if the queue for the given state is empty, false otherwise + */ +bool BufferQueue::empty(BufferQueue::State state) +{ + return bufferState_[state].empty(); +} + +/** + * \brief Get the front buffer of a queue + * \param[in] state The state + * + * \return The front buffer of the queue, or null otherwise + */ +FrameBuffer *BufferQueue::front(BufferQueue::State state) +{ + if (empty(state)) + return nullptr; + return bufferState_[state].front(); +} + +/** + * \brief Get the expected sequence for a buffer + * \param[in] buffer The buffer + * + * \return The front buffer of the queue, or null otherwise + */ +unsigned int BufferQueue::expectedSequence(FrameBuffer *buffer) const +{ + auto it = expectedSequence_.find(buffer); + ASSERT(it != expectedSequence_.end()); + return it->second; +} + +/** + * \brief Get the allocated buffers + * + * \return The buffers owned by the queue + */ +const std::vector> &BufferQueue::buffers() const +{ + return buffers_; +} + +void BufferQueue::onBufferReady(FrameBuffer *buffer) +{ + ASSERT(!empty(Capturing)); + + auto &meta = buffer->metadata(); + auto &queue = bufferState_[Capturing]; + + /* + * V4L2 does not guarantee that buffers are dequeued in order. We expect + * drivers to usually do so, and therefore warn, if a buffer is returned + * out of order. After streamoff V4L2VideoDevice returns the buffers in + * arbitrary order so there is no warning needed in that case. + */ + auto it = std::find(queue.begin(), queue.end(), buffer); + ASSERT(it != queue.end()); + + if (it != queue.begin() && + meta.status != FrameMetadata::FrameCancelled) + LOG(BufferQueue, Warning) << name_ << ": Dequeued buffer out of order " << buffer; + + queue.erase(it); + if (meta.status == FrameMetadata::FrameCancelled) { + syncHelper_.cancelFrame(); + if (ownsBuffers_) + bufferState_[Idle].push_back(buffer); + } else { + syncHelper_.gotFrame(expectedSequence_[buffer], meta.sequence); + bufferState_[Postprocessing].push_back(buffer); + } + + bufferReady.emit(buffer); + + if (!(flags_ & PostprocessStage) && + meta.status != FrameMetadata::FrameCancelled) + internalPostprocessedBuffer(); +} + +int BufferQueue::internalPrepareBuffer(FrameBuffer *buffer, uint32_t *sequence) +{ + ASSERT(hasBuffers_); + + if (ownsBuffers_) { + ASSERT(!bufferState_[Idle].empty()); + ASSERT(bufferState_[Idle].front() == buffer); + } + + LOG(BufferQueue, Debug) << name_ << ":Buffer prepare: " + << buffer; + int correction = syncHelper_.correction(); + nextSequence_ += correction; + expectedSequence_[buffer] = nextSequence_; + if (ownsBuffers_) + bufferState_[Idle].pop_front(); + bufferState_[Preparing].push_back(buffer); + syncHelper_.pushCorrection(correction); + + if (sequence) + *sequence = nextSequence_; + + nextSequence_++; + + if (!(flags_ & PrepareStage)) + return internalPreparedBuffer(); + + return 0; +} + +int BufferQueue::internalPreparedBuffer() +{ + ASSERT(!bufferState_[Preparing].empty()); + + auto &srcQueue = bufferState_[Preparing]; + FrameBuffer *buffer = srcQueue.front(); + + srcQueue.pop_front(); + int ret = delegate_->queueBuffer(buffer); + if (ret < 0) { + LOG(BufferQueue, Error) << "Failed to queue buffer: " + << strerror(-ret); + if (ownsBuffers_) + bufferState_[Idle].push_back(buffer); + return ret; + } + + LOG(BufferQueue, Debug) << name_ << " Queued buffer: " << buffer; + + bufferState_[Capturing].push_back(buffer); + return 0; +} + +void BufferQueue::internalPostprocessedBuffer() +{ + ASSERT(!empty(Postprocessing)); + + FrameBuffer *buffer = bufferState_[Postprocessing].front(); + bufferState_[Postprocessing].pop_front(); + if (ownsBuffers_) + bufferState_[Idle].push_back(buffer); +} + +} /* namespace libcamera */ diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index 8b82555a5e81..55bdb895f6b4 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -18,6 +18,7 @@ libcamera_public_sources = files([ libcamera_internal_sources = files([ 'bayer_format.cpp', + 'buffer_queue.cpp', 'byte_stream_buffer.cpp', 'camera_controls.cpp', 'camera_lens.cpp', From patchwork Wed Mar 25 15:13:57 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26367 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 E04DBC32F8 for ; Wed, 25 Mar 2026 15:15:53 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 95E8062CC9; Wed, 25 Mar 2026 16:15:53 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="sckeQkMX"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2063762CB1 for ; Wed, 25 Mar 2026 16:15:52 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id B211E1862; Wed, 25 Mar 2026 16:14:33 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451674; bh=MSXQQJi8LE2cnkAR6yhx1iug0DEiN/oipTl0Lozh39U=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=sckeQkMX1NIGXiuwvimpijbWkRB9YJRbBqq13TZqp/tM85vtiBdVJCaAQ1WV5QLA8 HDLOcPsGJHIcLrAhNm/0K76NtLLWYx3b4Rcr6NuarIorUinMnXa0d7Lb+3r7pOzi7L vuIaEjkuM1FaZvliJ+hJhZtl/3W5tRP+y5Zoc+uQ= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 25/32] pipeline: rkisp1: Decouple image, stats and param buffers Date: Wed, 25 Mar 2026 16:13:57 +0100 Message-ID: <20260325151416.2114564-26-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The current code creates FrameInfo objects that tie together params, stats and image buffers and expect that these always stay in these groupings. However there are cases where these sequences get out of sync (e.g. a scratch buffer is used for an image or a params buffer was too late and therefore gets applied one frame later). As these situations are timing related and cpu dependent it is impossible to guarantee the initial grouping of buffers. Split the buffers into separate queues for stats, images and params. Resynchronize on image buffers as soon as they are dequeued from V4L2 as these are the only buffers tied to the libcamera requests coming from the user application. Now handle the other buffer types according to their specific properties: Stats buffers only need to be tracked after dequing and can be tied to the corresponding request after the corresponding image buffer was dequeued. If params buffers get out of sync we can either inject the same set of parameters twice or skip one set of params. If image buffers get out of sync we need to update the expected sensor sequence, so that the next request is assigned the correct sensor sequence. Signed-off-by: Stefan Klug --- Changes in v2: - Mostly cosmetic changes Changes in v1: - Moved variables resets in start() further to the top Changes in v0.6: - Fixed multiple assertions on start/stop and a few corner cases - Fixed crash in dewarpRequestReady on stop Changes in v0.5 - Fixed possible use-after-free in RequestInfo --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 749 +++++++++++++++-------- 1 file changed, 482 insertions(+), 267 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index b70c551fc775..f3e0ee5d3028 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -6,6 +6,8 @@ */ #include +#include +#include #include #include #include @@ -45,6 +47,7 @@ #include "libcamera/internal/media_pipeline.h" #include "libcamera/internal/pipeline_handler.h" #include "libcamera/internal/request.h" +#include "libcamera/internal/sequence_sync_helper.h" #include "libcamera/internal/v4l2_subdevice.h" #include "libcamera/internal/v4l2_videodevice.h" #include "libcamera/internal/yaml_parser.h" @@ -54,48 +57,19 @@ namespace libcamera { LOG_DEFINE_CATEGORY(RkISP1) +LOG_DEFINE_CATEGORY(RkISP1Schedule) class PipelineHandlerRkISP1; class RkISP1CameraData; -struct RkISP1FrameInfo { - unsigned int frame; - Request *request; - FrameBuffer *paramBuffer; - FrameBuffer *statBuffer; - FrameBuffer *mainPathBuffer; - FrameBuffer *selfPathBuffer; - - bool paramDequeued; - bool metadataProcessed; -}; - -class RkISP1Frames -{ -public: - RkISP1Frames(PipelineHandler *pipe); - - RkISP1FrameInfo *create(const RkISP1CameraData *data, Request *request, - bool isRaw); - int destroy(unsigned int frame); - void clear(); - - RkISP1FrameInfo *find(unsigned int frame); - RkISP1FrameInfo *find(FrameBuffer *buffer); - RkISP1FrameInfo *find(Request *request); - -private: - PipelineHandlerRkISP1 *pipe_; - std::map frameInfo_; -}; class RkISP1CameraData : public Camera::Private { public: RkISP1CameraData(PipelineHandler *pipe, RkISP1MainPath *mainPath, RkISP1SelfPath *selfPath) - : Camera::Private(pipe), frame_(0), frameInfo_(pipe), + : Camera::Private(pipe), frame_(0), mainPath_(mainPath), selfPath_(selfPath), canUseDewarper_(false), usesDewarper_(false) { @@ -109,9 +83,12 @@ public: Stream selfPathStream_; std::unique_ptr sensor_; std::unique_ptr delayedCtrls_; + /* + * The sensor frame sequence of the last request queued to the pipeline + * handler. + */ unsigned int frame_; std::vector ipaBuffers_; - RkISP1Frames frameInfo_; RkISP1MainPath *mainPath_; RkISP1SelfPath *selfPath_; @@ -162,21 +139,54 @@ private: Transform combinedTransform_; }; +struct SensorFrameInfo { + Request *request = nullptr; + FrameBuffer *statsBuffer = nullptr; + ControlList metadata; + bool metadataProcessed = false; +}; + +struct RequestInfo { + Request *request = nullptr; + /* + * The estimated sensor sequence for this request. Only reliable when + * sequenceValid is true + */ + size_t sequence = 0; + bool sequenceValid = false; +}; + +struct ParamBufferInfo { + FrameBuffer *buffer = nullptr; + size_t expectedSequence = 0; +}; + +struct DewarpBufferInfo { + FrameBuffer *inputBuffer; + FrameBuffer *outputBuffer; +}; + namespace { /* - * Maximum number of requests that shall be queued into the pipeline to keep - * the regulation fast. - * \todo This needs revisiting as soon as buffers got decoupled from requests - * and/or a fast path for controls was implemented. + * This many buffers ensures that the pipeline runs smoothly, without frame + * drops. */ -static constexpr unsigned int kRkISP1MaxQueuedRequests = 4; +static constexpr unsigned int kRkISP1MinBufferCount = 6; /* - * This many internal buffers (or rather parameter and statistics buffer - * pairs) ensures that the pipeline runs smoothly, without frame drops. + * This many internal buffers (params and stats) are needed for smooth operation + * \todo In high framerate or high cpu load situations it might be necessary to + * increase this number. \todo: This also relates to max sensor delay and must + * always be >= maxSensor delay */ -static constexpr unsigned int kRkISP1MinBufferCount = 4; +static constexpr unsigned int kRkISP1InternalBufferCount = 4; + +/* + * This many internal image buffers between ISP and dewarper are needed for + * smooth operation. + */ +static constexpr unsigned int kRkISP1DewarpImageBufferCount = 4; } /* namespace */ @@ -209,18 +219,20 @@ private: friend RkISP1CameraData; friend RkISP1CameraConfiguration; - friend RkISP1Frames; int initLinks(Camera *camera, const RkISP1CameraConfiguration &config); int createCamera(MediaEntity *sensor); - void tryCompleteRequest(RkISP1FrameInfo *info); - void cancelDewarpRequest(RkISP1FrameInfo *info); + void tryCompleteRequests(); + void cancelDewarpRequest(Request *request); void imageBufferReady(FrameBuffer *buffer); void paramBufferReady(FrameBuffer *buffer); void statBufferReady(FrameBuffer *buffer); void dewarpBufferReady(FrameBuffer *buffer); void frameStart(uint32_t sequence); + void queueInternalBuffers(); + void computeParamBuffers(uint32_t maxSequence); + int allocateBuffers(Camera *camera); int freeBuffers(Camera *camera); @@ -243,140 +255,30 @@ private: std::vector> mainPathBuffers_; std::queue availableMainPathBuffers_; + bool running_ = false; + std::vector> paramBuffers_; std::vector> statBuffers_; std::queue availableParamBuffers_; std::queue availableStatBuffers_; + std::deque queuedRequests_; + + std::map sensorFrameInfos_; + + std::deque queuedDewarpBuffers_; + SequenceSyncHelper paramsSyncHelper_; + SequenceSyncHelper imageSyncHelper_; + + std::queue computingParamBuffers_; + std::queue queuedParamBuffers_; + + uint32_t nextParamsSequence_; + uint32_t nextStatsToProcess_; + Camera *activeCamera_; }; -RkISP1Frames::RkISP1Frames(PipelineHandler *pipe) - : pipe_(static_cast(pipe)) -{ -} - -RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *request, - bool isRaw) -{ - unsigned int frame = data->frame_; - - FrameBuffer *paramBuffer = nullptr; - FrameBuffer *statBuffer = nullptr; - FrameBuffer *mainPathBuffer = nullptr; - FrameBuffer *selfPathBuffer = nullptr; - - if (!isRaw) { - if (pipe_->availableParamBuffers_.empty()) { - LOG(RkISP1, Error) << "Parameters buffer underrun"; - return nullptr; - } - - if (pipe_->availableStatBuffers_.empty()) { - LOG(RkISP1, Error) << "Statistic buffer underrun"; - return nullptr; - } - - paramBuffer = pipe_->availableParamBuffers_.front(); - pipe_->availableParamBuffers_.pop(); - - statBuffer = pipe_->availableStatBuffers_.front(); - pipe_->availableStatBuffers_.pop(); - - if (data->usesDewarper_) { - mainPathBuffer = pipe_->availableMainPathBuffers_.front(); - pipe_->availableMainPathBuffers_.pop(); - } - } - - if (!mainPathBuffer) - mainPathBuffer = request->findBuffer(&data->mainPathStream_); - selfPathBuffer = request->findBuffer(&data->selfPathStream_); - - auto [it, inserted] = frameInfo_.try_emplace(frame); - ASSERT(inserted); - - auto &info = it->second; - - info.frame = frame; - info.request = request; - info.paramBuffer = paramBuffer; - info.mainPathBuffer = mainPathBuffer; - info.selfPathBuffer = selfPathBuffer; - info.statBuffer = statBuffer; - info.paramDequeued = false; - info.metadataProcessed = false; - - return &info; -} - -int RkISP1Frames::destroy(unsigned int frame) -{ - auto it = frameInfo_.find(frame); - if (it == frameInfo_.end()) - return -ENOENT; - - auto &info = it->second; - - pipe_->availableParamBuffers_.push(info.paramBuffer); - pipe_->availableStatBuffers_.push(info.statBuffer); - pipe_->availableMainPathBuffers_.push(info.mainPathBuffer); - - frameInfo_.erase(it); - - return 0; -} - -void RkISP1Frames::clear() -{ - for (const auto &[frame, info] : frameInfo_) { - pipe_->availableParamBuffers_.push(info.paramBuffer); - pipe_->availableStatBuffers_.push(info.statBuffer); - pipe_->availableMainPathBuffers_.push(info.mainPathBuffer); - } - - frameInfo_.clear(); -} - -RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame) -{ - auto itInfo = frameInfo_.find(frame); - - if (itInfo != frameInfo_.end()) - return &itInfo->second; - - LOG(RkISP1, Fatal) << "Can't locate info from frame"; - - return nullptr; -} - -RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer) -{ - for (auto &[frame, info] : frameInfo_) { - if (info.paramBuffer == buffer || - info.statBuffer == buffer || - info.mainPathBuffer == buffer || - info.selfPathBuffer == buffer) - return &info; - } - - LOG(RkISP1, Fatal) << "Can't locate info from buffer"; - - return nullptr; -} - -RkISP1FrameInfo *RkISP1Frames::find(Request *request) -{ - for (auto &[frame, info] : frameInfo_) { - if (info.request == request) - return &info; - } - - LOG(RkISP1, Fatal) << "Can't locate info from request"; - - return nullptr; -} - PipelineHandlerRkISP1 *RkISP1CameraData::pipe() { return static_cast(Camera::Private::pipe()); @@ -473,44 +375,95 @@ int RkISP1CameraData::loadTuningFile(const std::string &path) void RkISP1CameraData::paramsComputed(unsigned int frame, unsigned int bytesused) { PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe(); - RkISP1FrameInfo *info = frameInfo_.find(frame); - if (!info) - return; + ParamBufferInfo &pInfo = pipe->computingParamBuffers_.front(); + pipe->computingParamBuffers_.pop(); - info->paramBuffer->_d()->metadata().planes()[0].bytesused = bytesused; + ASSERT(pInfo.expectedSequence == frame); + FrameBuffer *buffer = pInfo.buffer; - int ret = pipe->param_->queueBuffer(info->paramBuffer); + LOG(RkISP1Schedule, Debug) << "Queue params for " << frame << " " << buffer; + + buffer->_d()->metadata().planes()[0].bytesused = bytesused; + int ret = pipe->param_->queueBuffer(buffer); if (ret < 0) { LOG(RkISP1, Error) << "Failed to queue parameter buffer: " << strerror(-ret); + pipe->availableParamBuffers_.push(buffer); return; } - pipe->stat_->queueBuffer(info->statBuffer); - - if (info->mainPathBuffer) - mainPath_->queueBuffer(info->mainPathBuffer); - - if (selfPath_ && info->selfPathBuffer) - selfPath_->queueBuffer(info->selfPathBuffer); + pipe->queuedParamBuffers_.push({ buffer, frame }); } void RkISP1CameraData::setSensorControls(unsigned int frame, const ControlList &sensorControls) { + /* We know delayed controls is prewarmed for frame 0 */ + if (frame == 0) + return; + + LOG(RkISP1Schedule, Debug) << "DelayedControls push " << frame; delayedCtrls_->push(frame, sensorControls); } void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &metadata) { - RkISP1FrameInfo *info = frameInfo_.find(frame); - if (!info) - return; + PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe(); - info->request->_d()->metadata().merge(metadata); - info->metadataProcessed = true; + LOG(RkISP1Schedule, Debug) << " metadataReady " << frame; - pipe()->tryCompleteRequest(info); + auto &info = pipe->sensorFrameInfos_[frame]; + + /* + * We don't necessarily know the request for that sequence number, + * as the dequeue of the image buffer might not have happened yet. + * So we check all known requests and store the metadata otherwise. + */ + for (auto &reqInfo : pipe->queuedRequests_) { + if (!reqInfo.sequenceValid) { + LOG(RkISP1Schedule, Debug) + << "Need to store metadata for later " << frame; + info.metadata = metadata; + break; + } + + if (frame > reqInfo.sequence) { + /* + * We will never get stats for that request. Log an + * error and return it. + */ + LOG(RkISP1, Warning) + << "Stats for frame " << reqInfo.sequence + << " got lost"; + auto &info2 = pipe->sensorFrameInfos_[reqInfo.sequence]; + info2.metadataProcessed = true; + ASSERT(info2.statsBuffer == nullptr); + continue; + } + + if (frame == reqInfo.sequence) { + reqInfo.request->_d()->metadata().merge(metadata); + break; + } + + /* We should never end up here */ + LOG(RkISP1, Error) << "Request for sequence " << frame + << " is already handled. Metadata was too late"; + + break; + } + + info.metadataProcessed = true; + /* + * info.statsBuffer can be null, if ipa->processStats() was called + * without a buffer to just fill the metadata. + */ + if (info.statsBuffer) + pipe->availableStatBuffers_.push(info.statsBuffer); + info.statsBuffer = nullptr; + + pipe->tryCompleteRequests(); + pipe->queueInternalBuffers(); } /* ----------------------------------------------------------------------------- @@ -777,7 +730,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() */ PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager) - : PipelineHandler(manager, kRkISP1MaxQueuedRequests), hasSelfPath_(true) + : PipelineHandler(manager), hasSelfPath_(true) { } @@ -1152,18 +1105,19 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) } }; if (!isRaw_) { - ret = param_->allocateBuffers(kRkISP1MinBufferCount, ¶mBuffers_); + ret = param_->allocateBuffers(kRkISP1InternalBufferCount, ¶mBuffers_); if (ret < 0) return ret; - ret = stat_->allocateBuffers(kRkISP1MinBufferCount, &statBuffers_); + ret = stat_->allocateBuffers(kRkISP1InternalBufferCount, &statBuffers_); if (ret < 0) return ret; } /* If the dewarper is being used, allocate internal buffers for ISP. */ if (data->usesDewarper_) { - ret = mainPath_.exportBuffers(kRkISP1MinBufferCount, &mainPathBuffers_); + ret = mainPath_.exportBuffers(kRkISP1DewarpImageBufferCount, + &mainPathBuffers_); if (ret < 0) return ret; @@ -1242,6 +1196,12 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL if (!!controls) ctrls = *controls; + paramsSyncHelper_.reset(); + imageSyncHelper_.reset(); + nextParamsSequence_ = 0; + nextStatsToProcess_ = 0; + data->frame_ = 0; + ipa::rkisp1::StartResult res; data->ipa_->start(ctrls, &res); if (res.code) { @@ -1253,8 +1213,6 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL data->sensor_->setControls(&res.controls); data->delayedCtrls_->reset(); - data->frame_ = 0; - if (!isRaw_) { ret = param_->streamOn(); if (ret) { @@ -1301,6 +1259,9 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL isp_->setFrameStartEnabled(true); activeCamera_ = camera; + running_ = true; + + queueInternalBuffers(); actions.release(); return 0; @@ -1310,6 +1271,9 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera) { RkISP1CameraData *data = cameraData(camera); int ret; + running_ = false; + + LOG(RkISP1Schedule, Debug) << "Stop device"; isp_->setFrameStartEnabled(false); @@ -1330,46 +1294,154 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera) LOG(RkISP1, Warning) << "Failed to stop parameters for " << camera->id(); + /* + * The param buffers are not returned in order, so the queue + * becomes useless. + */ + queuedParamBuffers_ = {}; + if (data->usesDewarper_) dewarper_->stop(); } - ASSERT(data->queuedRequests_.empty()); - data->frameInfo_.clear(); + tryCompleteRequests(); + + /* There can still be requests that are either waiting for metadata + or that contain buffers which were not yet queued at all. */ + while (!queuedRequests_.empty()) { + RequestInfo &reqInfo = queuedRequests_.front(); + cancelRequest(reqInfo.request); + queuedRequests_.pop_front(); + } + sensorFrameInfos_.clear(); + + ASSERT(queuedDewarpBuffers_.empty()); + ASSERT(queuedParamBuffers_.empty()); + ASSERT(computingParamBuffers_.empty()); freeBuffers(camera); activeCamera_ = nullptr; } -int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request) +void PipelineHandlerRkISP1::queueInternalBuffers() { - RkISP1CameraData *data = cameraData(camera); + if (!running_) + return; - RkISP1FrameInfo *info = data->frameInfo_.create(data, request, isRaw_); - if (!info) - return -ENOENT; + RkISP1CameraData *data = cameraData(activeCamera_); + + while (!availableStatBuffers_.empty()) { + FrameBuffer *buf = availableStatBuffers_.front(); + availableStatBuffers_.pop(); + data->pipe()->stat_->queueBuffer(buf); + } + + /* + * In case of the dewarper, there is a seperate buffer loop for the main + * path + */ + while (!availableMainPathBuffers_.empty()) { + FrameBuffer *buf = availableMainPathBuffers_.front(); + availableMainPathBuffers_.pop(); + + LOG(RkISP1Schedule, Debug) << "Queue mainPath " << buf; + data->mainPath_->queueBuffer(buf); + } +} + +void PipelineHandlerRkISP1::computeParamBuffers(uint32_t maxSequence) +{ + RkISP1CameraData *data = cameraData(activeCamera_); - data->ipa_->queueRequest(data->frame_, request->controls()); if (isRaw_) { - if (info->mainPathBuffer) - data->mainPath_->queueBuffer(info->mainPathBuffer); - - if (data->selfPath_ && info->selfPathBuffer) - data->selfPath_->queueBuffer(info->selfPathBuffer); - /* * Call computeParams with an empty param buffer to trigger the * setSensorControls signal. */ - data->ipa_->computeParams(data->frame_, 0); - } else { - data->ipa_->computeParams(data->frame_, - info->paramBuffer->cookie()); + data->ipa_->computeParams(maxSequence, 0); + return; } + while (nextParamsSequence_ <= maxSequence) { + if (availableParamBuffers_.empty()) { + LOG(RkISP1Schedule, Warning) + << "Ran out of parameter buffers"; + return; + } + + int correction = paramsSyncHelper_.correction(); + if (correction != 0) + LOG(RkISP1Schedule, Warning) + << "Correcting params sequence " + << correction; + + uint32_t paramsSequence; + if (correction >= 0) { + nextParamsSequence_ += correction; + paramsSyncHelper_.pushCorrection(correction); + paramsSequence = nextParamsSequence_++; + } else { + /* + * Inject the same sequence multiple times, to correct + * for the offset. + */ + paramsSyncHelper_.pushCorrection(-1); + paramsSequence = nextParamsSequence_; + } + + FrameBuffer *buf = availableParamBuffers_.front(); + availableParamBuffers_.pop(); + computingParamBuffers_.push({ buf, paramsSequence }); + LOG(RkISP1Schedule, Debug) << "Request params for " << paramsSequence; + data->ipa_->computeParams(paramsSequence, buf->cookie()); + } +} + +int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request) +{ + RkISP1CameraData *data = cameraData(camera); + + RequestInfo info; + info.request = request; + + int correction = imageSyncHelper_.correction(); + if (correction != 0) + LOG(RkISP1Schedule, Debug) + << "Correcting image sequence " + << data->frame_ << " to " << data->frame_ + correction; + data->frame_ += correction; + imageSyncHelper_.pushCorrection(correction); + info.sequence = data->frame_; data->frame_++; + LOG(RkISP1Schedule, Debug) << "Queue request. Request sequence: " + << request->sequence() + << " estimated sensor frame sequence: " << info.sequence + << " queue size: " << (queuedRequests_.size() + 1); + + data->ipa_->queueRequest(info.sequence, request->controls()); + + /* + * When the dewarper is used, the request buffers will be queued in + * imageBufferReady() + */ + if (!data->usesDewarper_) { + FrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_); + FrameBuffer *selfPathBuffer = request->findBuffer(&data->selfPathStream_); + if (mainPathBuffer) + data->mainPath_->queueBuffer(mainPathBuffer); + + if (data->selfPath_ && selfPathBuffer) + data->selfPath_->queueBuffer(selfPathBuffer); + } + + queuedRequests_.push_back(info); + + /* Kickstart computation of parameters. */ + if (info.sequence < kRkISP1InternalBufferCount) + computeParamBuffers(info.sequence); + return 0; } @@ -1523,8 +1595,11 @@ void PipelineHandlerRkISP1::frameStart(uint32_t sequence) return; RkISP1CameraData *data = cameraData(activeCamera_); + LOG(RkISP1Schedule, Debug) << "frameStart " << sequence; uint32_t sequenceToApply = sequence + data->delayedCtrls_->maxDelay(); data->delayedCtrls_->applyControls(sequenceToApply); + + computeParamBuffers(sequenceToApply + 1); } bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) @@ -1602,29 +1677,51 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) * Buffer Handling */ -void PipelineHandlerRkISP1::tryCompleteRequest(RkISP1FrameInfo *info) +void PipelineHandlerRkISP1::tryCompleteRequests() { - RkISP1CameraData *data = cameraData(activeCamera_); - Request *request = info->request; + std::optional lastDeletedSequence; - if (request->hasPendingBuffers()) + /* Complete finished requests */ + while (!queuedRequests_.empty()) { + RequestInfo info = queuedRequests_.front(); + + if (info.request->hasPendingBuffers()) + break; + + if (!info.sequenceValid) + break; + + if (!sensorFrameInfos_[info.sequence].metadataProcessed) + break; + + queuedRequests_.pop_front(); + + LOG(RkISP1Schedule, Debug) << "Complete request " << info.sequence; + completeRequest(info.request); + + sensorFrameInfos_[info.sequence].request = nullptr; + lastDeletedSequence = info.sequence; + } + + if (!lastDeletedSequence.has_value()) return; - if (!info->metadataProcessed) - return; + /* Drop all outdated sensor frame infos. */ + while (!sensorFrameInfos_.empty()) { + auto iter = sensorFrameInfos_.begin(); + if (iter->first > lastDeletedSequence.value()) + break; - if (!isRaw_ && !info->paramDequeued) - return; + ASSERT(iter->second.request == nullptr); + ASSERT(iter->second.statsBuffer == nullptr); - data->frameInfo_.destroy(info->frame); - - completeRequest(request); + sensorFrameInfos_.erase(iter); + } } -void PipelineHandlerRkISP1::cancelDewarpRequest(RkISP1FrameInfo *info) +void PipelineHandlerRkISP1::cancelDewarpRequest(Request *request) { RkISP1CameraData *data = cameraData(activeCamera_); - Request *request = info->request; /* * i.MX8MP is the only known platform with dewarper. It has * no self path. Hence, only main path buffer completion is @@ -1643,67 +1740,129 @@ void PipelineHandlerRkISP1::cancelDewarpRequest(RkISP1FrameInfo *info) } } - tryCompleteRequest(info); + tryCompleteRequests(); } void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) { ASSERT(activeCamera_); RkISP1CameraData *data = cameraData(activeCamera_); - - RkISP1FrameInfo *info = data->frameInfo_.find(buffer); - if (!info) - return; - const FrameMetadata &metadata = buffer->metadata(); - Request *request = info->request; + RequestInfo *reqInfo = nullptr; + + /* + * When the dewarper is used, the buffer is not yet tied to a request, + * so find the first request without a valid sequence. Otherwise find + * the request for that buffer. This is not necessarily the same, + * because after streamoff the buffers are returned in arbitrary order. + */ + for (auto &info : queuedRequests_) { + if (data->usesDewarper_) { + if (!info.sequenceValid) { + reqInfo = &info; + break; + } + } else { + if (info.request == buffer->request()) { + reqInfo = &info; + } + } + } + + if (!reqInfo) { + if (data->usesDewarper_) { + LOG(RkISP1Schedule, Info) + << "Image buffer ready, but no corresponding request"; + availableMainPathBuffers_.push(buffer); + return; + } + + LOG(RkISP1Schedule, Fatal) + << "Image buffer ready, but no corresponding request"; + } + + Request *request = reqInfo->request; + + LOG(RkISP1Schedule, Debug) << "Image buffer ready: " << buffer + << " Expected sequence: " << reqInfo->sequence + << " got: " << metadata.sequence; + + uint32_t sequence = metadata.sequence; + + /* + * If the frame was cancelled, the metadata sequnce is usually wrong and + * we assume that our guess was right. + */ + if (metadata.status == FrameMetadata::FrameCancelled) + sequence = reqInfo->sequence; + + /* We now know the buffer sequence that belongs to this request */ + int droppedFrames = imageSyncHelper_.gotFrame(reqInfo->sequence, sequence); + if (droppedFrames != 0) + LOG(RkISP1Schedule, Warning) + << "Frame " << reqInfo->sequence << ": Dropped " + << droppedFrames << " frames"; + + reqInfo->sequence = sequence; + reqInfo->sequenceValid = true; + + if (sensorFrameInfos_[sequence].metadataProcessed) { + LOG(RkISP1Schedule, Debug) + << "Apply stored metadata " << sequence; + request->_d()->metadata().merge(sensorFrameInfos_[sequence].metadata); + } if (metadata.status != FrameMetadata::FrameCancelled) { /* * Record the sensor's timestamp in the request metadata. * - * \todo The sensor timestamp should be better estimated by connecting - * to the V4L2Device::frameStart signal. + * \todo The sensor timestamp should be better estimated by + * connecting to the V4L2Device::frameStart signal. */ request->_d()->metadata().set(controls::SensorTimestamp, metadata.timestamp); + /* In raw mode call processStats() to fill the metadata */ if (isRaw_) { const ControlList &ctrls = - data->delayedCtrls_->get(metadata.sequence); - data->ipa_->processStats(info->frame, 0, ctrls); + data->delayedCtrls_->get(sequence); + data->ipa_->processStats(sequence, 0, ctrls); } } else { - if (isRaw_) - info->metadataProcessed = true; + /* No need to block waiting for metedata on that frame. */ + sensorFrameInfos_[sequence].metadataProcessed = true; } if (!data->usesDewarper_) { - completeBuffer(request, buffer); - tryCompleteRequest(info); + completeBuffer(reqInfo->request, buffer); + tryCompleteRequests(); return; } - /* Do not queue cancelled frames to dewarper. */ + /* Do not queue cancelled frames to the dewarper. */ if (metadata.status == FrameMetadata::FrameCancelled) { - cancelDewarpRequest(info); + cancelDewarpRequest(reqInfo->request); return; } dewarper_->setControls(&data->mainPathStream_, request->controls()); /* - * Queue input and output buffers to the dewarper. The output - * buffers for the dewarper are the buffers of the request, supplied - * by the application. + * Queue input and output buffers to the dewarper. The output buffers + * for the dewarper are the buffers of the request, supplied by the + * application. */ + DewarpBufferInfo dewarpInfo{ buffer, reqInfo->request->findBuffer(&data->mainPathStream_) }; + queuedDewarpBuffers_.push_back(dewarpInfo); + LOG(RkISP1Schedule, Debug) << "Queue dewarper " << dewarpInfo.inputBuffer + << " " << dewarpInfo.outputBuffer; int ret = dewarper_->queueBuffers(buffer, request->buffers()); if (ret < 0) { LOG(RkISP1, Error) << "Failed to queue buffers to dewarper: -" << strerror(-ret); - cancelDewarpRequest(info); + cancelDewarpRequest(reqInfo->request); return; } @@ -1713,29 +1872,59 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer) { - ASSERT(activeCamera_); - RkISP1CameraData *data = cameraData(activeCamera_); Request *request = buffer->request(); + const FrameMetadata &metadata = buffer->metadata(); - RkISP1FrameInfo *info = data->frameInfo_.find(buffer->request()); - if (!info) - return; + /* + * After stopping the dewarper, the buffers are returned out of order. + * Search the list for the corresponding info and handle it. In regular + * operation it will always be the first entry. + */ + for (DewarpBufferInfo &dwInfo : queuedDewarpBuffers_) { + if (dwInfo.outputBuffer != buffer) + continue; - completeBuffer(request, buffer); - tryCompleteRequest(info); + availableMainPathBuffers_.push(dwInfo.inputBuffer); + dwInfo.inputBuffer = nullptr; + dwInfo.outputBuffer = nullptr; + + if (metadata.status == FrameMetadata::FrameCancelled) + buffer->_d()->cancel(); + + completeBuffer(request, buffer); + } + + while (!queuedDewarpBuffers_.empty() && + queuedDewarpBuffers_.front().inputBuffer == nullptr) + queuedDewarpBuffers_.pop_front(); + + tryCompleteRequests(); + queueInternalBuffers(); } void PipelineHandlerRkISP1::paramBufferReady(FrameBuffer *buffer) { - ASSERT(activeCamera_); - RkISP1CameraData *data = cameraData(activeCamera_); + LOG(RkISP1Schedule, Debug) << "Param buffer ready " << buffer; - RkISP1FrameInfo *info = data->frameInfo_.find(buffer); - if (!info) + /* After stream off, the buffers are returned out of order, so + * we don't care about the rest. + */ + if (!running_) { + availableParamBuffers_.push(buffer); return; + } - info->paramDequeued = true; - tryCompleteRequest(info); + ParamBufferInfo pInfo = queuedParamBuffers_.front(); + queuedParamBuffers_.pop(); + + ASSERT(pInfo.buffer == buffer); + + size_t metaSequence = buffer->metadata().sequence; + LOG(RkISP1Schedule, Debug) << "Params buffer ready " + << " Expected: " << pInfo.expectedSequence + << " got: " << metaSequence; + paramsSyncHelper_.gotFrame(pInfo.expectedSequence, metaSequence); + availableParamBuffers_.push(buffer); } void PipelineHandlerRkISP1::statBufferReady(FrameBuffer *buffer) @@ -1743,21 +1932,47 @@ void PipelineHandlerRkISP1::statBufferReady(FrameBuffer *buffer) ASSERT(activeCamera_); RkISP1CameraData *data = cameraData(activeCamera_); - RkISP1FrameInfo *info = data->frameInfo_.find(buffer); - if (!info) - return; + size_t sequence = buffer->metadata().sequence; if (buffer->metadata().status == FrameMetadata::FrameCancelled) { - info->metadataProcessed = true; - tryCompleteRequest(info); + LOG(RkISP1Schedule, Warning) << "Stats cancelled " << sequence; + /* + * We can't assume that the sequence of the stat buffer is valid, + * so there is nothing left to do. + */ + availableStatBuffers_.push(buffer); return; } - if (data->frame_ <= buffer->metadata().sequence) - data->frame_ = buffer->metadata().sequence + 1; + LOG(RkISP1Schedule, Debug) << "Stats ready " << sequence; - data->ipa_->processStats(info->frame, info->statBuffer->cookie(), - data->delayedCtrls_->get(buffer->metadata().sequence)); + if (nextStatsToProcess_ != sequence) + LOG(RkISP1Schedule, Warning) << "Stats sequence out of sync." + << " Expected: " << nextStatsToProcess_ + << " got: " << sequence; + + if (nextStatsToProcess_ > sequence) { + LOG(RkISP1Schedule, Warning) << "Stats were too late. Ignored"; + availableStatBuffers_.push(buffer); + return; + } + + /* Send empty stats to ensure metadata gets created*/ + while (nextStatsToProcess_ < sequence) { + LOG(RkISP1Schedule, Warning) << "Send empty stats to fill metadata"; + data->ipa_->processStats(nextStatsToProcess_, 0, + data->delayedCtrls_->get(nextStatsToProcess_)); + + nextStatsToProcess_++; + } + + nextStatsToProcess_++; + + sensorFrameInfos_[sequence].statsBuffer = buffer; + + LOG(RkISP1Schedule, Debug) << "Process stats " << sequence; + data->ipa_->processStats(sequence, buffer->cookie(), + data->delayedCtrls_->get(sequence)); } REGISTER_PIPELINE_HANDLER(PipelineHandlerRkISP1, "rkisp1") From patchwork Wed Mar 25 15:13:58 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26368 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 94B28C3307 for ; Wed, 25 Mar 2026 15:15:56 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3090362CD2; Wed, 25 Mar 2026 16:15:56 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="vF0yjSK7"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B6C0662CD5 for ; Wed, 25 Mar 2026 16:15:54 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id C3DB31ADE; Wed, 25 Mar 2026 16:14:36 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451676; bh=fjxxuf012oHI0Q2/ChKYB5utpY/hbRP/Lqf0OVZX0Io=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=vF0yjSK7kaM/DrsXGY5bO7Y6pg1YByWEMkYiCLk+lpJNd6oJBlUMFwl6ETtUeUbMe RpUPMcl49zrUp2PDQf2NdBv2PBX0/L2hNRhMZXO2TJwuNXQ1MKuS+RiVRNoQ6uPQvh CIo9Lbh8yMeYmy1Z7rjw+0IZGW2xmyrrskbpTnjY= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 26/32] pipline: rkisp1: Reinstantiate maxQueuedRequestsDevice limit Date: Wed, 25 Mar 2026 16:13:58 +0100 Message-ID: <20260325151416.2114564-27-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" With the pipeline rework, the maxQueuedRequestsDevice should not be necessary anymore, as prepare() and therefore the calculation for the ISP regulation is called only as late as possible when a params buffer was dequeued. However with unlimited maxQueuedRequestsDevice all the incoming requests get immediately queued to the ipa with the sensor sequence number that was anticipated for that request at queueRequestDevice time. Now when the correction tries to mitigate dropped sequence numbers, it will call computeParams() with sensor frame numbers that were not anticipated for the requests queued to the IPA. There still might be a better solution to this, but reinstantiating the limit reduces the effect. Signed-off-by: Stefan Klug --- Changes in v2: - Added this patch --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index f3e0ee5d3028..46c157526ea1 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -730,7 +730,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate() */ PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager) - : PipelineHandler(manager), hasSelfPath_(true) + : PipelineHandler(manager, kRkISP1MinBufferCount), hasSelfPath_(true) { } From patchwork Wed Mar 25 15:13:59 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26369 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 0144FC32F9 for ; Wed, 25 Mar 2026 15:15:58 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A3D2362CD6; Wed, 25 Mar 2026 16:15:58 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="eEhLazop"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id BAAC562CC8 for ; Wed, 25 Mar 2026 16:15:57 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id B0D871ADE; Wed, 25 Mar 2026 16:14:39 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451679; bh=L6I3QomOQk/0PnCjuQoct8dbyFwyDZCKEHWfkXWWUJo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=eEhLazopPUNP1pzA24CGstZDeN777YH5oqQuYU3wRgiMOVRSvuZWV/AnEXuYduhMw Q+j1ri+ItUaZTVlMOgMBHr0OJVcnGEdTShk4CMul64kxKiTJhCr+O15o5B1LmWaoML RsaSFGB+JgretuaXIoyxdqxS54dNzejGGynHa6Ec= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Paul Elder Subject: [PATCH v2 27/32] pipeline: rkisp1: Correctly handle params buffer for frame 0 Date: Wed, 25 Mar 2026 16:13:59 +0100 Message-ID: <20260325151416.2114564-28-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The parameters for frame 0 are only active on frame 0 if the corresponding parameter buffer is queued before STREAMON is called on the isp. Therefore the normal mechanics of calling ipa->computeParams() do not work, as it gets called after the first request is queued and therefore the isp is already started. To fix that, handle the parameter buffer the same way the initial sensor controls are handled by passing it to the ipa->start() function and then queuing the params buffer before starting the isp. Signed-off-by: Stefan Klug Reviewed-by: Paul Elder --- Changes in v2: - Small cleanup - Fixed crash in raw case where a null buffer is passed on start - Collected tag --- include/libcamera/ipa/rkisp1.mojom | 5 ++- src/ipa/rkisp1/rkisp1.cpp | 39 +++++++++++++----------- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 19 ++++++++++-- 3 files changed, 42 insertions(+), 21 deletions(-) diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom index 4c29b53cd7f9..56c9fe8ab92a 100644 --- a/include/libcamera/ipa/rkisp1.mojom +++ b/include/libcamera/ipa/rkisp1.mojom @@ -17,6 +17,7 @@ struct IPAConfigInfo { struct StartResult { libcamera.ControlList controls; int32 code; + uint32 paramBufferBytesUsed; }; interface IPARkISP1Interface { @@ -25,7 +26,9 @@ interface IPARkISP1Interface { libcamera.IPACameraSensorInfo sensorInfo, libcamera.ControlInfoMap sensorControls) => (int32 ret, libcamera.ControlInfoMap ipaControls); - start(libcamera.ControlList controls) => (StartResult result); + start(libcamera.ControlList controls, + uint32 paramBufferId) + => (StartResult result); stop(); configure(IPAConfigInfo configInfo, diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index d0c753976dd4..8a657a170b0d 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -57,7 +57,8 @@ public: const IPACameraSensorInfo &sensorInfo, const ControlInfoMap &sensorControls, ControlInfoMap *ipaControls) override; - void start(const ControlList &controls, StartResult *result) override; + void start(const ControlList &controls, const uint32_t paramBufferId, + StartResult *result) override; void stop() override; int configure(const IPAConfigInfo &ipaConfig, @@ -77,6 +78,8 @@ protected: std::string logPrefix() const override; private: + uint32_t computeParamsInternal(IPAFrameContext &frameContext, const uint32_t bufferId); + void updateControls(const IPACameraSensorInfo &sensorInfo, const ControlInfoMap &sensorControls, ControlInfoMap *ipaControls); @@ -219,9 +222,11 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision, return 0; } -void IPARkISP1::start(const ControlList &controls, StartResult *result) +void IPARkISP1::start(const ControlList &controls, const uint32_t paramBufferId, + StartResult *result) { IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(0, controls); + result->paramBufferBytesUsed = computeParamsInternal(frameContext, paramBufferId); result->controls = getSensorControls(frameContext); result->code = 0; } @@ -353,32 +358,30 @@ void IPARkISP1::initializeFrameContext(IPAFrameContext &fc, const ControlList &c } } -void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId) +uint32_t IPARkISP1::computeParamsInternal(IPAFrameContext &frameContext, const uint32_t bufferId) { - IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame); - - /* - * \todo: This needs discussion. In raw mode, computeParams is - * called without a params buffer, to trigger the setSensorControls - * signal. Currently our algorithms don't support prepare calls with - * a nullptr. Do we need that or can we safely skip it? - */ - if (bufferId == 0) { - ControlList ctrls = getSensorControls(frameContext); - setSensorControls.emit(frame, ctrls); - return; - } + if (bufferId == 0) + return 0; RkISP1Params params(context_.configuration.paramFormat, mappedBuffers_.at(bufferId).planes()[0]); for (const auto &algo : algorithms()) - algo->prepare(context_, frame, frameContext, ¶ms); + algo->prepare(context_, frameContext.frame(), frameContext, ¶ms); + return params.bytesused(); +} + +void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId) +{ + IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame); + + uint32_t size = computeParamsInternal(frameContext, bufferId); ControlList ctrls = getSensorControls(frameContext); setSensorControls.emit(frame, ctrls); - paramsComputed.emit(frame, params.bytesused()); + if (bufferId != 0) + paramsComputed.emit(frame, size); } void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId, diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 46c157526ea1..c2af5ef8897f 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -105,8 +105,8 @@ public: bool canUseDewarper_; bool usesDewarper_; -private: void paramsComputed(unsigned int frame, unsigned int bytesused); +private: void setSensorControls(unsigned int frame, const ControlList &sensorControls); @@ -1202,13 +1202,28 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL nextStatsToProcess_ = 0; data->frame_ = 0; + uint32_t paramBufferId = 0; + FrameBuffer *paramBuffer = nullptr; + if (!isRaw_) { + paramBuffer = availableParamBuffers_.front(); + paramBufferId = paramBuffer->cookie(); + } + ipa::rkisp1::StartResult res; - data->ipa_->start(ctrls, &res); + data->ipa_->start(ctrls, paramBufferId, &res); if (res.code) { LOG(RkISP1, Error) << "Failed to start IPA " << camera->id(); return ret; } + + if (paramBuffer) { + availableParamBuffers_.pop(); + computingParamBuffers_.push({ paramBuffer, nextParamsSequence_++ }); + paramsSyncHelper_.pushCorrection(0); + data->paramsComputed(0, res.paramBufferBytesUsed); + } + actions += [&]() { data->ipa_->stop(); }; data->sensor_->setControls(&res.controls); data->delayedCtrls_->reset(); From patchwork Wed Mar 25 15:14:00 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26370 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 90265C3308 for ; Wed, 25 Mar 2026 15:16:01 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2D7FC62CDB; Wed, 25 Mar 2026 16:16:01 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="OP+ahLJX"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id CE69562CC8 for ; Wed, 25 Mar 2026 16:15:59 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id D30C81BCF; Wed, 25 Mar 2026 16:14:41 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451682; bh=+hCmkz/cH1E3R82fgcxsAE8s8Do8XB2aeHYxJIngR2E=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=OP+ahLJXfI++Ji/ElYzazLhCI/Fzz82p/6dCSkpt+K5XLCskqDONmTx2z6+ytHRhh mvBRBxAg3Svw4KIIPws0z4ONYyKxCt9Bx2hXtogO/YMCfzVebwFFYist6lSaLWJkxl 24k+KKH894oLCk5q/1Yw4VvwKtsK1LefCwR2BTvo= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 28/32] pipeline: rkisp1: Fix buffer metadata when using the dewarper Date: Wed, 25 Mar 2026 16:14:00 +0100 Message-ID: <20260325151416.2114564-29-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" When the dewarper is part of the pipeline, the output buffers shall still carry status (in case of FrameError), timestamp and sequence from the corresponding image buffer. Timestamp is automatically copied over by the m2m device. Manually transfer status and sequence. This change also fixes an issue where frames with error status were marked as successful after running through the dewarper. Signed-off-by: Stefan Klug --- Changes in v2: - Added this patch --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index c2af5ef8897f..b6592e87dcc4 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -163,6 +163,10 @@ struct ParamBufferInfo { struct DewarpBufferInfo { FrameBuffer *inputBuffer; + struct { + FrameMetadata::Status status; + unsigned int sequence; + } inputMeta; FrameBuffer *outputBuffer; }; @@ -1868,7 +1872,11 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) * for the dewarper are the buffers of the request, supplied by the * application. */ - DewarpBufferInfo dewarpInfo{ buffer, reqInfo->request->findBuffer(&data->mainPathStream_) }; + DewarpBufferInfo dewarpInfo{ + buffer, + { metadata.status, metadata.sequence }, + reqInfo->request->findBuffer(&data->mainPathStream_) + }; queuedDewarpBuffers_.push_back(dewarpInfo); LOG(RkISP1Schedule, Debug) << "Queue dewarper " << dewarpInfo.inputBuffer << " " << dewarpInfo.outputBuffer; @@ -1888,7 +1896,6 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer) { Request *request = buffer->request(); - const FrameMetadata &metadata = buffer->metadata(); /* * After stopping the dewarper, the buffers are returned out of order. @@ -1899,13 +1906,18 @@ void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer) if (dwInfo.outputBuffer != buffer) continue; + FrameMetadata &outputMeta = buffer->_d()->metadata(); + + if (outputMeta.status != FrameMetadata::FrameCancelled && + dwInfo.inputMeta.status == FrameMetadata::FrameError) + outputMeta.status = FrameMetadata::FrameError; + + outputMeta.sequence = dwInfo.inputMeta.sequence; + availableMainPathBuffers_.push(dwInfo.inputBuffer); dwInfo.inputBuffer = nullptr; dwInfo.outputBuffer = nullptr; - if (metadata.status == FrameMetadata::FrameCancelled) - buffer->_d()->cancel(); - completeBuffer(request, buffer); } From patchwork Wed Mar 25 15:14:01 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26371 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 43F86C330A for ; Wed, 25 Mar 2026 15:16:04 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id B804462CE3; Wed, 25 Mar 2026 16:16:03 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="g/SQqUz0"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3A84F62C4E for ; Wed, 25 Mar 2026 16:16:02 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 3F8D31BCF; Wed, 25 Mar 2026 16:14:44 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451684; bh=s0K8MHWvQEkthFW0dWwiLxTiBHilkSt4GYNaDePO0bo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=g/SQqUz00HQqzHXyLJWxcFyaRpmXoI2DD75q/rOpqSQQmc26cqHhegiMAhXLK731X THEeGdf8DdO74X2jiZj6Z9FjSm1WejjiIwXnCxeDvzG4+1sIMnTNXs1XO95J2Qjdee x198Z60x9dWwVLe7F6ZmfF5DmvTZIyCFpO3XVG+Y= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 29/32] pipeline: rkisp1: Pass bufferId to paramsComputed() Date: Wed, 25 Mar 2026 16:14:01 +0100 Message-ID: <20260325151416.2114564-30-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The frame number is not necessarily unique. This should not be the case, but even misbehaving kernel drivers should not be able to interfere with that on the libcamera side. Pass the bufferId, to have a guaranteed unique handle. Signed-off-by: Stefan Klug --- Changes in v2: - Added this patch --- include/libcamera/ipa/rkisp1.mojom | 2 +- src/ipa/rkisp1/rkisp1.cpp | 2 +- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom index 56c9fe8ab92a..e48bdf92c8f8 100644 --- a/include/libcamera/ipa/rkisp1.mojom +++ b/include/libcamera/ipa/rkisp1.mojom @@ -45,7 +45,7 @@ interface IPARkISP1Interface { }; interface IPARkISP1EventInterface { - paramsComputed(uint32 frame, uint32 bytesused); + paramsComputed(uint32 frame, uint32 bufferId, uint32 bytesused); setSensorControls(uint32 frame, libcamera.ControlList sensorControls); metadataReady(uint32 frame, libcamera.ControlList metadata); }; diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 8a657a170b0d..863cea13dfa3 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -381,7 +381,7 @@ void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId) setSensorControls.emit(frame, ctrls); if (bufferId != 0) - paramsComputed.emit(frame, size); + paramsComputed.emit(frame, bufferId, size); } void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId, diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index b6592e87dcc4..c46515110ffc 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -105,7 +105,7 @@ public: bool canUseDewarper_; bool usesDewarper_; - void paramsComputed(unsigned int frame, unsigned int bytesused); + void paramsComputed(unsigned int frame, unsigned int bufferId, unsigned int bytesused); private: void setSensorControls(unsigned int frame, const ControlList &sensorControls); @@ -376,14 +376,14 @@ int RkISP1CameraData::loadTuningFile(const std::string &path) return 0; } -void RkISP1CameraData::paramsComputed(unsigned int frame, unsigned int bytesused) +void RkISP1CameraData::paramsComputed(unsigned int frame, unsigned int bufferId, unsigned int bytesused) { PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe(); ParamBufferInfo &pInfo = pipe->computingParamBuffers_.front(); pipe->computingParamBuffers_.pop(); - ASSERT(pInfo.expectedSequence == frame); FrameBuffer *buffer = pInfo.buffer; + ASSERT(buffer->cookie() == bufferId); LOG(RkISP1Schedule, Debug) << "Queue params for " << frame << " " << buffer; @@ -1225,7 +1225,7 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL availableParamBuffers_.pop(); computingParamBuffers_.push({ paramBuffer, nextParamsSequence_++ }); paramsSyncHelper_.pushCorrection(0); - data->paramsComputed(0, res.paramBufferBytesUsed); + data->paramsComputed(0, paramBufferId, res.paramBufferBytesUsed); } actions += [&]() { data->ipa_->stop(); }; From patchwork Wed Mar 25 15:14:02 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26372 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 BED8EC32FA for ; Wed, 25 Mar 2026 15:16:06 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 68C0462CDF; Wed, 25 Mar 2026 16:16:06 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="HLH0ourw"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 96C0062CDA for ; Wed, 25 Mar 2026 16:16:05 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 9609D1AFB; Wed, 25 Mar 2026 16:14:47 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451687; bh=Iy4AqolszNI6X5zcfVsnGNJIhlZ4gPoEIOTgX2W+sfE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=HLH0ourwxUzJU+61e/jEYXX7CwuUiZ+9eZwf7v+PLsM/bb2Uk8ZmYu5VK6Kvrvd2r UkbOcKcOXwc7XTlQ2SIQnyPWLV45pwC5s3B0H+zJCEaKajwDvokugD9PMjutQBn4cY Ybkbq/jKCoeWK9pUUm45lUNhD0x7H6UQke8fOYwY= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 30/32] pipeline: rkisp1: rkisp1_path: Modify interface to be compatible with BufferQueue Date: Wed, 25 Mar 2026 16:14:02 +0100 Message-ID: <20260325151416.2114564-31-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" The BufferQueue delegate expects an interface equal to the one provided by the V4L2VideoDevice. Modify the RkISP1Path class to support that interface to prepare for the migration to BufferQueue. Signed-off-by: Stefan Klug --- Changes in v2: - Added this patch --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 44 ++++++++++++++----- src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 42 ++---------------- src/libcamera/pipeline/rkisp1/rkisp1_path.h | 23 ++++++++-- 3 files changed, 56 insertions(+), 53 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index c46515110ffc..998ee3c813bc 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -1099,14 +1099,15 @@ int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, S int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) { RkISP1CameraData *data = cameraData(camera); + utils::ScopeExitActions actions; unsigned int ipaBufferId = 1; int ret; - auto errorCleanup = utils::scope_exit{ [&]() { + actions += [&]() { paramBuffers_.clear(); statBuffers_.clear(); mainPathBuffers_.clear(); - } }; + }; if (!isRaw_) { ret = param_->allocateBuffers(kRkISP1InternalBufferCount, ¶mBuffers_); @@ -1127,6 +1128,22 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) for (std::unique_ptr &buffer : mainPathBuffers_) availableMainPathBuffers_.push(buffer.get()); + } else if (data->mainPath_->isEnabled()) { + ret = mainPath_.importBuffers(data->mainPathStream_.configuration().bufferCount); + + if (ret < 0) + return ret; + + actions += [&]() { mainPath_.releaseBuffers(); }; + } + + if (hasSelfPath_ && data->selfPath_->isEnabled()) { + ret = selfPath_.importBuffers(data->selfPathStream_.configuration().bufferCount); + + if (ret < 0) + return ret; + + actions += [&]() { selfPath_.releaseBuffers(); }; } auto pushBuffers = [&](const std::vector> &buffers, @@ -1147,7 +1164,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) data->ipa_->mapBuffers(data->ipaBuffers_); - errorCleanup.release(); + actions.release(); return 0; } @@ -1181,6 +1198,12 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera) if (stat_->releaseBuffers()) LOG(RkISP1, Error) << "Failed to release stat buffers"; + if (mainPath_.releaseBuffers()) + LOG(RkISP1, Error) << "Failed to release main path buffers"; + + if (hasSelfPath_ && selfPath_.releaseBuffers()) + LOG(RkISP1, Error) << "Failed to release self path buffers"; + return 0; } @@ -1263,16 +1286,17 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL } if (data->mainPath_->isEnabled()) { - ret = mainPath_.start(data->mainPathStream_.configuration().bufferCount); + ret = mainPath_.streamOn(); if (ret) return ret; - actions += [&]() { mainPath_.stop(); }; + actions += [&]() { mainPath_.streamOff(); }; } if (hasSelfPath_ && data->selfPath_->isEnabled()) { - ret = selfPath_.start(data->selfPathStream_.configuration().bufferCount); + ret = selfPath_.streamOn(); if (ret) return ret; + actions += [&]() { selfPath_.streamOff(); }; } isp_->setFrameStartEnabled(true); @@ -1299,8 +1323,8 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera) data->ipa_->stop(); if (hasSelfPath_) - selfPath_.stop(); - mainPath_.stop(); + selfPath_.streamOff(); + mainPath_.streamOff(); if (!isRaw_) { ret = stat_->streamOff(); @@ -1664,9 +1688,9 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) return false; isp_->frameStart.connect(this, &PipelineHandlerRkISP1::frameStart); - mainPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::imageBufferReady); + mainPath_.bufferReady.connect(this, &PipelineHandlerRkISP1::imageBufferReady); if (hasSelfPath_) - selfPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::imageBufferReady); + selfPath_.bufferReady.connect(this, &PipelineHandlerRkISP1::imageBufferReady); stat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statBufferReady); param_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramBufferReady); diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp index ef9cfbdc327a..80a01280e6f7 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp @@ -8,6 +8,7 @@ #include "rkisp1_path.h" #include +#include #include @@ -58,7 +59,7 @@ const std::map formatToMediaBus = { RkISP1Path::RkISP1Path(const char *name, const Span &formats, const Size &minResolution, const Size &maxResolution) - : name_(name), running_(false), formats_(formats), + : name_(name), formats_(formats), minResolution_(minResolution), maxResolution_(maxResolution), link_(nullptr) { @@ -77,6 +78,7 @@ bool RkISP1Path::init(std::shared_ptr media) if (video_->open() < 0) return false; + video_->bufferReady.connect(this, [this](FrameBuffer *buffer) { this->bufferReady.emit(buffer); }); populateFormats(); link_ = media->link("rkisp1_isp", 2, resizer, 0); @@ -480,44 +482,6 @@ int RkISP1Path::configure(const StreamConfiguration &config, return 0; } -int RkISP1Path::start(unsigned int bufferCount) -{ - int ret; - - if (running_) - return -EBUSY; - - ret = video_->importBuffers(bufferCount); - if (ret) - return ret; - - ret = video_->streamOn(); - if (ret) { - LOG(RkISP1, Error) - << "Failed to start " << name_ << " path"; - - video_->releaseBuffers(); - return ret; - } - - running_ = true; - - return 0; -} - -void RkISP1Path::stop() -{ - if (!running_) - return; - - if (video_->streamOff()) - LOG(RkISP1, Warning) << "Failed to stop " << name_ << " path"; - - video_->releaseBuffers(); - - running_ = false; -} - /* * \todo Remove the hardcoded resolutions and formats once kernels older than * v6.4 will stop receiving LTS support (scheduled for December 2027 for v6.1). diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h index 0c68e9eb99af..d2bce01c9336 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h @@ -58,19 +58,34 @@ public: return video_->exportBuffers(bufferCount, buffers); } - int start(unsigned int bufferCount); - void stop(); + int allocateBuffers(unsigned int bufferCount, + std::vector> *buffers) + { + return video_->allocateBuffers(bufferCount, buffers); + } + + int importBuffers(unsigned int count) + { + return video_->importBuffers(count); + } + + int releaseBuffers() + { + return video_->releaseBuffers(); + } + + int streamOn() { return video_->streamOn(); }; + int streamOff() { return video_->streamOff(); }; int queueBuffer(FrameBuffer *buffer) { return video_->queueBuffer(buffer); } - Signal &bufferReady() { return video_->bufferReady; } const Size &maxResolution() const { return maxResolution_; } + Signal bufferReady; private: void populateFormats(); Size filterSensorResolution(const CameraSensor *sensor); const char *name_; - bool running_; const Span formats_; std::set streamFormats_; From patchwork Wed Mar 25 15:14:03 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26373 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 A9D8AC330F for ; Wed, 25 Mar 2026 15:16:10 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 0A83D62CDA; Wed, 25 Mar 2026 16:16:10 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Nx44KS3C"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9349A62CDA for ; Wed, 25 Mar 2026 16:16:08 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 7491C1BCF; Wed, 25 Mar 2026 16:14:50 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451690; bh=SA7jM4U/YCt/5CDGIYRGgwJ47FMrJHgOfmAWoKdKPYk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Nx44KS3C9tKyCKc+gRJHdtdDudTqeEhxeKyxR9uMGycBaD3M3d7ezKMTK7YDU13xV s1taJ/84apRU0b8qCxH4nXnmZyK4DSlPOsOt5A5uqtX90ay9QR37l7KtyZw3N7DfZP D/cQ1FLKRWqzWtLI0vWv1eNG7GpSGZr4lo0t5q2Y= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 31/32] pipeline: rkisp1: Use BufferQueue for buffer handling Date: Wed, 25 Mar 2026 16:14:03 +0100 Message-ID: <20260325151416.2114564-32-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Migrate the pipeline handler to use the BufferQueue helper. This simplifies the code and makes it easier to understand. Signed-off-by: Stefan Klug --- Changes in v2: - Added this patch --- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 224 ++++++++--------------- 1 file changed, 79 insertions(+), 145 deletions(-) diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index 998ee3c813bc..e047f073d695 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -12,7 +12,6 @@ #include #include #include -#include #include #include @@ -35,6 +34,7 @@ #include #include +#include "libcamera/internal/buffer_queue.h" #include "libcamera/internal/camera.h" #include "libcamera/internal/camera_sensor.h" #include "libcamera/internal/camera_sensor_properties.h" @@ -89,6 +89,7 @@ public: */ unsigned int frame_; std::vector ipaBuffers_; + std::vector ipaFrameBuffers_; RkISP1MainPath *mainPath_; RkISP1SelfPath *selfPath_; @@ -156,11 +157,6 @@ struct RequestInfo { bool sequenceValid = false; }; -struct ParamBufferInfo { - FrameBuffer *buffer = nullptr; - size_t expectedSequence = 0; -}; - struct DewarpBufferInfo { FrameBuffer *inputBuffer; struct { @@ -247,6 +243,9 @@ private: std::unique_ptr param_; std::unique_ptr stat_; + std::unique_ptr paramQueue_; + std::unique_ptr statQueue_; + bool hasSelfPath_; bool isRaw_; @@ -256,28 +255,18 @@ private: std::unique_ptr dewarper_; /* Internal buffers used when dewarper is being used */ - std::vector> mainPathBuffers_; - std::queue availableMainPathBuffers_; + std::unique_ptr mainPathQueue_; bool running_ = false; - std::vector> paramBuffers_; - std::vector> statBuffers_; - std::queue availableParamBuffers_; - std::queue availableStatBuffers_; - std::deque queuedRequests_; std::map sensorFrameInfos_; std::deque queuedDewarpBuffers_; - SequenceSyncHelper paramsSyncHelper_; + SequenceSyncHelper imageSyncHelper_; - std::queue computingParamBuffers_; - std::queue queuedParamBuffers_; - - uint32_t nextParamsSequence_; uint32_t nextStatsToProcess_; Camera *activeCamera_; @@ -379,24 +368,19 @@ int RkISP1CameraData::loadTuningFile(const std::string &path) void RkISP1CameraData::paramsComputed(unsigned int frame, unsigned int bufferId, unsigned int bytesused) { PipelineHandlerRkISP1 *pipe = RkISP1CameraData::pipe(); - ParamBufferInfo &pInfo = pipe->computingParamBuffers_.front(); - pipe->computingParamBuffers_.pop(); - FrameBuffer *buffer = pInfo.buffer; + FrameBuffer *buffer = pipe->paramQueue_->front(BufferQueue::Preparing); + ASSERT(buffer->cookie() == bufferId); LOG(RkISP1Schedule, Debug) << "Queue params for " << frame << " " << buffer; buffer->_d()->metadata().planes()[0].bytesused = bytesused; - int ret = pipe->param_->queueBuffer(buffer); - if (ret < 0) { + + int ret = pipe->paramQueue_->preparedBuffer(); + if (ret < 0) LOG(RkISP1, Error) << "Failed to queue parameter buffer: " << strerror(-ret); - pipe->availableParamBuffers_.push(buffer); - return; - } - - pipe->queuedParamBuffers_.push({ buffer, frame }); } void RkISP1CameraData::setSensorControls(unsigned int frame, @@ -462,8 +446,11 @@ void RkISP1CameraData::metadataReady(unsigned int frame, const ControlList &meta * info.statsBuffer can be null, if ipa->processStats() was called * without a buffer to just fill the metadata. */ - if (info.statsBuffer) - pipe->availableStatBuffers_.push(info.statsBuffer); + if (info.statsBuffer) { + ASSERT(pipe->statQueue_->front(BufferQueue::Postprocessing) == info.statsBuffer); + pipe->statQueue_->postprocessedBuffer(); + } + info.statsBuffer = nullptr; pipe->tryCompleteRequests(); @@ -879,6 +866,15 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c) isRaw_ = info.colourEncoding == PixelFormatInfo::ColourEncodingRAW; data->usesDewarper_ = data->canUseDewarper_ && !isRaw_; + auto delegate = std::make_unique>(&mainPath_); + if (data->usesDewarper_) { + mainPathQueue_ = std::make_unique(std::move(delegate), BufferQueue::PostprocessStage, "MainPath"); + } else { + mainPathQueue_ = std::make_unique(std::move(delegate), 0, "MainPath"); + } + + mainPathQueue_->bufferReady.connect(this, &PipelineHandlerRkISP1::imageBufferReady); + Transform transform = config->combinedTransform(); bool transposeAfterIsp = false; if (data->usesDewarper_) { @@ -1088,9 +1084,9 @@ int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, S if (data->usesDewarper_) return dewarper_->exportBuffers(&data->mainPathStream_, count, buffers); else - return mainPath_.exportBuffers(count, buffers); + return data->mainPath_->exportBuffers(count, buffers); } else if (hasSelfPath_ && stream == &data->selfPathStream_) { - return selfPath_.exportBuffers(count, buffers); + return data->selfPath_->exportBuffers(count, buffers); } return -EINVAL; @@ -1102,39 +1098,35 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) utils::ScopeExitActions actions; unsigned int ipaBufferId = 1; int ret; + /* Start at index 1 */ + data->ipaFrameBuffers_.resize(1); actions += [&]() { - paramBuffers_.clear(); - statBuffers_.clear(); - mainPathBuffers_.clear(); + paramQueue_->releaseBuffers(); + statQueue_->releaseBuffers(); + mainPathQueue_->releaseBuffers(); }; if (!isRaw_) { - ret = param_->allocateBuffers(kRkISP1InternalBufferCount, ¶mBuffers_); + ret = paramQueue_->allocateBuffers(kRkISP1InternalBufferCount); if (ret < 0) return ret; - ret = stat_->allocateBuffers(kRkISP1InternalBufferCount, &statBuffers_); + ret = statQueue_->allocateBuffers(kRkISP1InternalBufferCount); if (ret < 0) return ret; } /* If the dewarper is being used, allocate internal buffers for ISP. */ if (data->usesDewarper_) { - ret = mainPath_.exportBuffers(kRkISP1DewarpImageBufferCount, - &mainPathBuffers_); + ret = mainPathQueue_->allocateBuffers(kRkISP1DewarpImageBufferCount); if (ret < 0) return ret; - - for (std::unique_ptr &buffer : mainPathBuffers_) - availableMainPathBuffers_.push(buffer.get()); } else if (data->mainPath_->isEnabled()) { - ret = mainPath_.importBuffers(data->mainPathStream_.configuration().bufferCount); + ret = mainPathQueue_->importBuffers(data->mainPathStream_.configuration().bufferCount); if (ret < 0) return ret; - - actions += [&]() { mainPath_.releaseBuffers(); }; } if (hasSelfPath_ && data->selfPath_->isEnabled()) { @@ -1146,8 +1138,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) actions += [&]() { selfPath_.releaseBuffers(); }; } - auto pushBuffers = [&](const std::vector> &buffers, - std::queue &queue) { + auto pushBuffers = [&](const std::vector> &buffers) { for (const std::unique_ptr &buffer : buffers) { Span planes = buffer->planes(); @@ -1155,12 +1146,12 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera) data->ipaBuffers_.emplace_back(buffer->cookie(), std::vector{ planes.begin(), planes.end() }); - queue.push(buffer.get()); + data->ipaFrameBuffers_.push_back(buffer.get()); } }; - pushBuffers(paramBuffers_, availableParamBuffers_); - pushBuffers(statBuffers_, availableStatBuffers_); + pushBuffers(paramQueue_->buffers()); + pushBuffers(statQueue_->buffers()); data->ipa_->mapBuffers(data->ipaBuffers_); @@ -1172,19 +1163,6 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera) { RkISP1CameraData *data = cameraData(camera); - while (!availableStatBuffers_.empty()) - availableStatBuffers_.pop(); - - while (!availableParamBuffers_.empty()) - availableParamBuffers_.pop(); - - while (!availableMainPathBuffers_.empty()) - availableMainPathBuffers_.pop(); - - paramBuffers_.clear(); - statBuffers_.clear(); - mainPathBuffers_.clear(); - std::vector ids; for (IPABuffer &ipabuf : data->ipaBuffers_) ids.push_back(ipabuf.id); @@ -1192,14 +1170,9 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera) data->ipa_->unmapBuffers(ids); data->ipaBuffers_.clear(); - if (param_->releaseBuffers()) - LOG(RkISP1, Error) << "Failed to release parameters buffers"; - - if (stat_->releaseBuffers()) - LOG(RkISP1, Error) << "Failed to release stat buffers"; - - if (mainPath_.releaseBuffers()) - LOG(RkISP1, Error) << "Failed to release main path buffers"; + paramQueue_->releaseBuffers(); + statQueue_->releaseBuffers(); + mainPathQueue_->releaseBuffers(); if (hasSelfPath_ && selfPath_.releaseBuffers()) LOG(RkISP1, Error) << "Failed to release self path buffers"; @@ -1223,16 +1196,14 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL if (!!controls) ctrls = *controls; - paramsSyncHelper_.reset(); imageSyncHelper_.reset(); - nextParamsSequence_ = 0; nextStatsToProcess_ = 0; data->frame_ = 0; uint32_t paramBufferId = 0; FrameBuffer *paramBuffer = nullptr; if (!isRaw_) { - paramBuffer = availableParamBuffers_.front(); + paramBuffer = paramQueue_->front(BufferQueue::Idle); paramBufferId = paramBuffer->cookie(); } @@ -1245,10 +1216,9 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL } if (paramBuffer) { - availableParamBuffers_.pop(); - computingParamBuffers_.push({ paramBuffer, nextParamsSequence_++ }); - paramsSyncHelper_.pushCorrection(0); - data->paramsComputed(0, paramBufferId, res.paramBufferBytesUsed); + uint32_t seq; + paramQueue_->prepareBuffer(&seq); + data->paramsComputed(seq, paramBufferId, res.paramBufferBytesUsed); } actions += [&]() { data->ipa_->stop(); }; @@ -1337,12 +1307,6 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera) LOG(RkISP1, Warning) << "Failed to stop parameters for " << camera->id(); - /* - * The param buffers are not returned in order, so the queue - * becomes useless. - */ - queuedParamBuffers_ = {}; - if (data->usesDewarper_) dewarper_->stop(); } @@ -1359,8 +1323,6 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera) sensorFrameInfos_.clear(); ASSERT(queuedDewarpBuffers_.empty()); - ASSERT(queuedParamBuffers_.empty()); - ASSERT(computingParamBuffers_.empty()); freeBuffers(camera); @@ -1374,29 +1336,30 @@ void PipelineHandlerRkISP1::queueInternalBuffers() RkISP1CameraData *data = cameraData(activeCamera_); - while (!availableStatBuffers_.empty()) { - FrameBuffer *buf = availableStatBuffers_.front(); - availableStatBuffers_.pop(); - data->pipe()->stat_->queueBuffer(buf); + while (!statQueue_->empty(BufferQueue::Idle)) { + int ret = statQueue_->queueBuffer(); + if (ret) + break; } /* * In case of the dewarper, there is a seperate buffer loop for the main * path */ - while (!availableMainPathBuffers_.empty()) { - FrameBuffer *buf = availableMainPathBuffers_.front(); - availableMainPathBuffers_.pop(); + if (!data->usesDewarper_) + return; - LOG(RkISP1Schedule, Debug) << "Queue mainPath " << buf; - data->mainPath_->queueBuffer(buf); + while (!mainPathQueue_->empty(BufferQueue::Idle)) { + LOG(RkISP1Schedule, Debug) << "Queue mainPath " << mainPathQueue_->front(BufferQueue::Idle); + int ret = mainPathQueue_->queueBuffer(); + if (ret) + break; } } void PipelineHandlerRkISP1::computeParamBuffers(uint32_t maxSequence) { RkISP1CameraData *data = cameraData(activeCamera_); - if (isRaw_) { /* * Call computeParams with an empty param buffer to trigger the @@ -1406,38 +1369,24 @@ void PipelineHandlerRkISP1::computeParamBuffers(uint32_t maxSequence) return; } - while (nextParamsSequence_ <= maxSequence) { - if (availableParamBuffers_.empty()) { + while (paramQueue_->nextSequence() <= maxSequence) { + if (paramQueue_->empty(BufferQueue::Idle)) { LOG(RkISP1Schedule, Warning) << "Ran out of parameter buffers"; return; } - int correction = paramsSyncHelper_.correction(); + int correction = paramQueue_->sequenceCorrection(); if (correction != 0) LOG(RkISP1Schedule, Warning) << "Correcting params sequence " << correction; uint32_t paramsSequence; - if (correction >= 0) { - nextParamsSequence_ += correction; - paramsSyncHelper_.pushCorrection(correction); - paramsSequence = nextParamsSequence_++; - } else { - /* - * Inject the same sequence multiple times, to correct - * for the offset. - */ - paramsSyncHelper_.pushCorrection(-1); - paramsSequence = nextParamsSequence_; - } - - FrameBuffer *buf = availableParamBuffers_.front(); - availableParamBuffers_.pop(); - computingParamBuffers_.push({ buf, paramsSequence }); + FrameBuffer *buffer = paramQueue_->front(BufferQueue::Idle); + paramQueue_->prepareBuffer(¶msSequence); LOG(RkISP1Schedule, Debug) << "Request params for " << paramsSequence; - data->ipa_->computeParams(paramsSequence, buf->cookie()); + data->ipa_->computeParams(paramsSequence, buffer->cookie()); } } @@ -1473,7 +1422,7 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request) FrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_); FrameBuffer *selfPathBuffer = request->findBuffer(&data->selfPathStream_); if (mainPathBuffer) - data->mainPath_->queueBuffer(mainPathBuffer); + mainPathQueue_->queueBuffer(mainPathBuffer); if (data->selfPath_ && selfPathBuffer) data->selfPath_->queueBuffer(selfPathBuffer); @@ -1676,10 +1625,18 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) if (stat_->open() < 0) return false; + statQueue_ = std::make_unique(std::make_unique>(stat_.get()), + BufferQueue::PostprocessStage, + "Stat"); + param_ = V4L2VideoDevice::fromEntityName(media_.get(), "rkisp1_params"); if (param_->open() < 0) return false; + paramQueue_ = std::make_unique(std::make_unique>(param_.get()), + BufferQueue::PrepareStage, + "Params"); + /* Locate and open the ISP main and self paths. */ if (!mainPath_.init(media_)) return false; @@ -1688,7 +1645,6 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator) return false; isp_->frameStart.connect(this, &PipelineHandlerRkISP1::frameStart); - mainPath_.bufferReady.connect(this, &PipelineHandlerRkISP1::imageBufferReady); if (hasSelfPath_) selfPath_.bufferReady.connect(this, &PipelineHandlerRkISP1::imageBufferReady); stat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statBufferReady); @@ -1816,7 +1772,7 @@ void PipelineHandlerRkISP1::imageBufferReady(FrameBuffer *buffer) if (data->usesDewarper_) { LOG(RkISP1Schedule, Info) << "Image buffer ready, but no corresponding request"; - availableMainPathBuffers_.push(buffer); + mainPathQueue_->postprocessedBuffer(); return; } @@ -1938,7 +1894,7 @@ void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer) outputMeta.sequence = dwInfo.inputMeta.sequence; - availableMainPathBuffers_.push(dwInfo.inputBuffer); + mainPathQueue_->postprocessedBuffer(); dwInfo.inputBuffer = nullptr; dwInfo.outputBuffer = nullptr; @@ -1957,25 +1913,10 @@ void PipelineHandlerRkISP1::paramBufferReady(FrameBuffer *buffer) { LOG(RkISP1Schedule, Debug) << "Param buffer ready " << buffer; - /* After stream off, the buffers are returned out of order, so - * we don't care about the rest. - */ - if (!running_) { - availableParamBuffers_.push(buffer); - return; - } - - ParamBufferInfo pInfo = queuedParamBuffers_.front(); - queuedParamBuffers_.pop(); - - ASSERT(pInfo.buffer == buffer); - size_t metaSequence = buffer->metadata().sequence; LOG(RkISP1Schedule, Debug) << "Params buffer ready " - << " Expected: " << pInfo.expectedSequence + << " Expected: " << paramQueue_->expectedSequence(buffer) << " got: " << metaSequence; - paramsSyncHelper_.gotFrame(pInfo.expectedSequence, metaSequence); - availableParamBuffers_.push(buffer); } void PipelineHandlerRkISP1::statBufferReady(FrameBuffer *buffer) @@ -1985,15 +1926,8 @@ void PipelineHandlerRkISP1::statBufferReady(FrameBuffer *buffer) size_t sequence = buffer->metadata().sequence; - if (buffer->metadata().status == FrameMetadata::FrameCancelled) { - LOG(RkISP1Schedule, Warning) << "Stats cancelled " << sequence; - /* - * We can't assume that the sequence of the stat buffer is valid, - * so there is nothing left to do. - */ - availableStatBuffers_.push(buffer); + if (buffer->metadata().status == FrameMetadata::FrameCancelled) return; - } LOG(RkISP1Schedule, Debug) << "Stats ready " << sequence; @@ -2004,7 +1938,7 @@ void PipelineHandlerRkISP1::statBufferReady(FrameBuffer *buffer) if (nextStatsToProcess_ > sequence) { LOG(RkISP1Schedule, Warning) << "Stats were too late. Ignored"; - availableStatBuffers_.push(buffer); + statQueue_->postprocessedBuffer(); return; } From patchwork Wed Mar 25 15:14:04 2026 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 26374 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 3D12DC3316 for ; Wed, 25 Mar 2026 15:16:13 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id ACA6B62CE8; Wed, 25 Mar 2026 16:16:12 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="fQuqcEG5"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 6229B62BB6 for ; Wed, 25 Mar 2026 16:16:11 +0100 (CET) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:b16a:5ed9:4ada:a95a]) by perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 37C8E1B98; Wed, 25 Mar 2026 16:14:53 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1774451693; bh=mo4fhPqKCeRqTMq1VS82qLaUbLgfc6H1VAwfvMjhmow=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=fQuqcEG5z+zzXPO0FMlSZlYqYVKHzCyYieo2EKfoTBMCSWlShHjNtsQqBq9k8pKss Tpgkc0w1ACz19tmgbL+JbjLuWh3cDd15DytdqSAHP9Fukw0EoA4eVxRshTxH6YHV7X kHUJER/HQ6fFJGQs+7XDGzIEfTeJYJwpB/U0T0f8= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v2 32/32] DNI: Move all queue/algo logic into FcLogic class Date: Wed, 25 Mar 2026 16:14:04 +0100 Message-ID: <20260325151416.2114564-33-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.51.0 In-Reply-To: <20260325151416.2114564-1-stefan.klug@ideasonboard.com> References: <20260325151416.2114564-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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" This is a prototype to toy around with the idea of moving all the typical IPA lgoc into an own FcLogic class (better names welcome). This could further reduce the boilerplate necessary to implement a typical IPA. Signed-off-by: Stefan Klug --- Changes in v2: - Added this patch --- src/ipa/libipa/algorithm.h | 5 + src/ipa/libipa/fc_logic.cpp | 20 ++++ src/ipa/libipa/fc_logic.h | 151 ++++++++++++++++++++++++++ src/ipa/libipa/fc_queue.h | 8 +- src/ipa/libipa/meson.build | 2 + src/ipa/rkisp1/algorithms/algorithm.h | 5 + src/ipa/rkisp1/ipa_context.h | 5 +- src/ipa/rkisp1/rkisp1.cpp | 61 ++++------- 8 files changed, 212 insertions(+), 45 deletions(-) create mode 100644 src/ipa/libipa/fc_logic.cpp create mode 100644 src/ipa/libipa/fc_logic.h diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h index 9a19dbd61b31..2d7372d2ff5e 100644 --- a/src/ipa/libipa/algorithm.h +++ b/src/ipa/libipa/algorithm.h @@ -38,6 +38,11 @@ public: return 0; } + virtual bool enabled() + { + return true; + } + virtual void queueRequest([[maybe_unused]] typename Module::Context &context, [[maybe_unused]] const uint32_t frame, [[maybe_unused]] typename Module::FrameContext &frameContext, diff --git a/src/ipa/libipa/fc_logic.cpp b/src/ipa/libipa/fc_logic.cpp new file mode 100644 index 000000000000..8b1917dfd40e --- /dev/null +++ b/src/ipa/libipa/fc_logic.cpp @@ -0,0 +1,20 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2022, Google Inc. + * + * IPA Frame context queue + */ + +#include "fc_logic.h" + +#include + +namespace libcamera { + +LOG_DEFINE_CATEGORY(FCLogic) + +namespace ipa { + +} /* namespace ipa */ + +} /* namespace libcamera */ diff --git a/src/ipa/libipa/fc_logic.h b/src/ipa/libipa/fc_logic.h new file mode 100644 index 000000000000..be51b09643b8 --- /dev/null +++ b/src/ipa/libipa/fc_logic.h @@ -0,0 +1,151 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2022, Google Inc. + * + * IPA Frame context queue + */ + +#pragma once + +#include +#include +#include + +#include +#include + +#include "algorithm.h" +#include "fc_queue.h" + +namespace libcamera { + +LOG_DECLARE_CATEGORY(FCLogic) + +namespace ipa { + +template +class FCLogic; + +template +class FCLogic +{ +public: + using Module = _Module; + using Algo = Algorithm; + + FCLogic(unsigned int size, Module *module, typename Module::Context &context) + : module_(module), contexts_(size), context_(context) + { + clear(); + } + + void clear() + { + for (typename Module::FrameContext &ctx : contexts_) { + ctx.frame_ = 0; + } + initialized_ = false; + lastPrepared_ = 0; + } + + typename Module::FrameContext &initFrameContext(unsigned int frame, const ControlList &controls = {}) + { + typename Module::FrameContext &fc = contexts_[frame % contexts_.size()]; + FrameContext &frameContext = fc; + + /* + * If the IPA algorithms try to access a frame context slot which + * has been already overwritten by a newer context, it means the + * frame context queue has overflowed and the desired context + * has been forever lost. The pipeline handler shall avoid + * queueing more requests to the IPA than the frame context + * queue size. + */ + if (frame < frameContext.frame_) + LOG(FCLogic, Fatal) << "Frame context for " << frame + << " has been overwritten by " + << frameContext.frame_; + + if (initialized_ && frame == frameContext.frame_) { + if (!controls.empty()) { + /* Too late to apply the controls. Store them for later. */ + LOG(FCLogic, Warning) + << "Request underrun. Controls for frame " + << frame << " are delayed "; + controlsToApply_.merge(controls, + ControlList::MergePolicy::OverwriteExisting); + } + + return fc; + } + + const ControlList *controls2 = &controls; + if (!controlsToApply_.empty()) { + LOG(FCLogic, Debug) << "Applied late controls on frame" << frame; + controlsToApply_.merge(controls, ControlList::MergePolicy::OverwriteExisting); + controls2 = &controlsToApply_; + } + + fc = {}; + frameContext.frame_ = frame; + for (auto const &a : module_->algorithms()) { + Algo *algo = static_cast(a.get()); + if (!algo->enabled()) + continue; + algo->queueRequest(context_, fc.frame(), fc, *controls2); + } + + initialized_ = true; + controlsToApply_.clear(); + + return fc; + } + + void prepareFrame(const uint32_t frame, typename Module::Params *params) + { + while (lastPrepared_ + 1 < frame) { + uint32_t f = lastPrepared_ + 1; + LOG(FCLogic, Warning) << "Collect skipped params for frame " << f; + prepareFrame(f, params); + } + + typename Module::FrameContext &frameContext = initFrameContext(frame); + for (auto const &algo : module_->algorithms()) + algo->prepare(context_, frameContext.frame(), frameContext, params); + + lastPrepared_ = std::max(lastPrepared_, frame); + } + + void processStats(const uint32_t frame, const typename Module::Stats *stats, ControlList &metadata) + { + /* + * If we apply stats on an uninitialized frame the activeState + * might end up very wrong (imagine exposureTime initialized to + * 0 but used together with the agc stats to upadet activeState. + */ + if (!isInitialize(frame)) + LOG(FCLogic, Error) << "Process stats called on an uninitialized FC " << frame; + + typename Module::FrameContext &frameContext = initFrameContext(frame); + for (auto const &algo : module_->algorithms()) + algo->process(context_, frameContext.frame(), frameContext, stats, metadata); + } + +private: + bool isInitialize(const uint32_t frame) + { + FrameContext &frameContext = contexts_[frame % contexts_.size()]; + return initialized_ && frame == frameContext.frame_; + } + + const Module *module_; + std::vector contexts_; + typename Module::Context &context_; + ControlList controlsToApply_; + uint32_t lastPrepared_; + bool initialized_; +}; + +} /* namespace ipa */ + +} /* namespace libcamera */ diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h index 633bf646d88d..34811cfb3514 100644 --- a/src/ipa/libipa/fc_queue.h +++ b/src/ipa/libipa/fc_queue.h @@ -22,11 +22,17 @@ namespace ipa { template class FCQueue; +template +class FCLogic; + struct FrameContext { uint32_t frame() const { return frame_; } private: - template friend class FCQueue; + template + friend class FCQueue; + template + friend class FCLogic; uint32_t frame_; }; diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build index 963c5ee73063..918279a89d27 100644 --- a/src/ipa/libipa/meson.build +++ b/src/ipa/libipa/meson.build @@ -9,6 +9,7 @@ libipa_headers = files([ 'camera_sensor_helper.h', 'colours.h', 'exposure_mode_helper.h', + 'fc_logic.h', 'fc_queue.h', 'fixedpoint.h', 'histogram.h', @@ -30,6 +31,7 @@ libipa_sources = files([ 'camera_sensor_helper.cpp', 'colours.cpp', 'exposure_mode_helper.cpp', + 'fc_logic.cpp', 'fc_queue.cpp', 'fixedpoint.cpp', 'histogram.cpp', diff --git a/src/ipa/rkisp1/algorithms/algorithm.h b/src/ipa/rkisp1/algorithms/algorithm.h index 715cfcd8298b..8b6644c0ee26 100644 --- a/src/ipa/rkisp1/algorithms/algorithm.h +++ b/src/ipa/rkisp1/algorithms/algorithm.h @@ -23,6 +23,11 @@ public: { } + virtual bool enabled() override + { + return !disabled_; + } + bool disabled_; bool supportsRaw_; }; diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index e61391bb1510..43ac9415a0d5 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/ipa_context.h @@ -236,8 +236,7 @@ struct IPAFrameContext : public FrameContext { }; struct IPAContext { - IPAContext(unsigned int frameContextSize) - : frameContexts(frameContextSize) + IPAContext() { } @@ -246,8 +245,6 @@ struct IPAContext { IPASessionConfiguration configuration; IPAActiveState activeState; - FCQueue frameContexts; - ControlInfoMap::Map ctrlMap; DebugMetadata debugMetadata; diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 863cea13dfa3..baebaa8ceb77 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -32,6 +32,7 @@ #include "libcamera/internal/yaml_parser.h" #include "algorithms/algorithm.h" +#include "libipa/fc_logic.h" #include "ipa_context.h" #include "params.h" @@ -78,7 +79,7 @@ protected: std::string logPrefix() const override; private: - uint32_t computeParamsInternal(IPAFrameContext &frameContext, const uint32_t bufferId); + uint32_t computeParamsInternal(const uint32_t frame, const uint32_t bufferId); void updateControls(const IPACameraSensorInfo &sensorInfo, const ControlInfoMap &sensorControls, @@ -92,6 +93,7 @@ private: /* Local parameter storage */ struct IPAContext context_; + FCLogic fcLogic_; }; namespace { @@ -132,12 +134,8 @@ const ControlInfoMap::Map rkisp1Controls{ } /* namespace */ IPARkISP1::IPARkISP1() - : context_(kMaxFrameContexts) + : Module(), context_(), fcLogic_(kMaxFrameContexts, this, context_) { - context_.frameContexts.setInitCallback( - [this](IPAFrameContext &fc, const ControlList &c) { - this->initializeFrameContext(fc, c); - }); } std::string IPARkISP1::logPrefix() const @@ -225,15 +223,15 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision, void IPARkISP1::start(const ControlList &controls, const uint32_t paramBufferId, StartResult *result) { - IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(0, controls); - result->paramBufferBytesUsed = computeParamsInternal(frameContext, paramBufferId); + IPAFrameContext &frameContext = fcLogic_.initFrameContext(0, controls); + result->paramBufferBytesUsed = computeParamsInternal(0, paramBufferId); result->controls = getSensorControls(frameContext); result->code = 0; } void IPARkISP1::stop() { - context_.frameContexts.clear(); + fcLogic_.clear(); } int IPARkISP1::configure(const IPAConfigInfo &ipaConfig, @@ -257,7 +255,7 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig, /* Clear the IPA context before the streaming session. */ context_.configuration = {}; context_.activeState = {}; - context_.frameContexts.clear(); + fcLogic_.clear(); context_.configuration.paramFormat = ipaConfig.paramFormat; @@ -345,20 +343,10 @@ void IPARkISP1::unmapBuffers(const std::vector &ids) void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls) { context_.debugMetadata.enableByControl(controls); - context_.frameContexts.getOrInitContext(frame, controls); + fcLogic_.initFrameContext(frame, controls); } -void IPARkISP1::initializeFrameContext(IPAFrameContext &fc, const ControlList &controls) -{ - for (const auto &a : algorithms()) { - Algorithm *algo = static_cast(a.get()); - if (algo->disabled_) - continue; - algo->queueRequest(context_, fc.frame(), fc, controls); - } -} - -uint32_t IPARkISP1::computeParamsInternal(IPAFrameContext &frameContext, const uint32_t bufferId) +uint32_t IPARkISP1::computeParamsInternal(const uint32_t frame, const uint32_t bufferId) { if (bufferId == 0) return 0; @@ -366,17 +354,16 @@ uint32_t IPARkISP1::computeParamsInternal(IPAFrameContext &frameContext, const u RkISP1Params params(context_.configuration.paramFormat, mappedBuffers_.at(bufferId).planes()[0]); - for (const auto &algo : algorithms()) - algo->prepare(context_, frameContext.frame(), frameContext, ¶ms); + fcLogic_.prepareFrame(frame, ¶ms); return params.bytesused(); } void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId) { - IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame); + uint32_t size = computeParamsInternal(frame, bufferId); - uint32_t size = computeParamsInternal(frameContext, bufferId); + IPAFrameContext &frameContext = fcLogic_.initFrameContext(frame); ControlList ctrls = getSensorControls(frameContext); setSensorControls.emit(frame, ctrls); @@ -387,7 +374,13 @@ void IPARkISP1::computeParams(const uint32_t frame, const uint32_t bufferId) void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId, const ControlList &sensorControls) { - IPAFrameContext &frameContext = context_.frameContexts.getOrInitContext(frame); + IPAFrameContext &frameContext = fcLogic_.initFrameContext(frame); + frameContext.sensor.exposure = + sensorControls.get(V4L2_CID_EXPOSURE).get(); + frameContext.sensor.gain = + context_.camHelper->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get()); + + ControlList metadata(controls::controls); /* * In raw capture mode, the ISP is bypassed and no statistics buffer is @@ -398,19 +391,7 @@ void IPARkISP1::processStats(const uint32_t frame, const uint32_t bufferId, stats = reinterpret_cast( mappedBuffers_.at(bufferId).planes()[0].data()); - frameContext.sensor.exposure = - sensorControls.get(V4L2_CID_EXPOSURE).get(); - frameContext.sensor.gain = - context_.camHelper->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get()); - - ControlList metadata(controls::controls); - - for (const auto &a : algorithms()) { - Algorithm *algo = static_cast(a.get()); - if (algo->disabled_) - continue; - algo->process(context_, frame, frameContext, stats, metadata); - } + fcLogic_.processStats(frame, stats, metadata); context_.debugMetadata.moveEntries(metadata); metadataReady.emit(frame, metadata);