From patchwork Thu Apr 3 15:49:06 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23117 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 4AE4FC327D for ; Thu, 3 Apr 2025 15:49:41 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 01136689A1; Thu, 3 Apr 2025 17:49:40 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="XnT7DVGe"; 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 0310C68994 for ; Thu, 3 Apr 2025 17:49:38 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:6d9d:9854:3fc1:4bb2]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 4C71D8FA; Thu, 3 Apr 2025 17:47:44 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1743695264; bh=wqVFTWlEEvkd8wBvHVZ2TIAwLhLy8s/Y2fDw4vYnFcA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=XnT7DVGe4fTQ/rhS9ZTBYLk8Oyqa4V8YnDqI6u7iXX1pnB4qIkLz87/JQzrTp+nOJ RUtpJzFRXQinxPSnspnFcOXLmnPVQpUpsX40WxsprMYKTRXE7bw61souPQAF8g3+99 nRZoqJqgKJsOwCiiwQmP0Qv89NziZd+0SJClMMPQ= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Laurent Pinchart Subject: [PATCH v3 01/16] libcamera: matrix: Replace SFINAE with static_asserts Date: Thu, 3 Apr 2025 17:49:06 +0200 Message-ID: <20250403154925.382973-2-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250403154925.382973-1-stefan.klug@ideasonboard.com> References: <20250403154925.382973-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" SFINAE is difficult to read and not needed in these cases. Replace it with static_asserts. The idea came from [1] where it is stated: "The use of enable_if seems misguided to me. SFINAE is useful for the situation where we consider multiple candidates for something (overloads or class template specializations) and try to choose the correct one, without causing compilation to fail." [1]: https://stackoverflow.com/questions/62109526/c-friend-template-that-use-sfinae Signed-off-by: Stefan Klug Reviewed-by: Laurent Pinchart --- Changes in v2: - Added this patch Changes in v3: - Left SFINAE in place for the operators as static asserts could cause issues there --- include/libcamera/internal/matrix.h | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h index a055e6926c94..b9c3d41ef855 100644 --- a/include/libcamera/internal/matrix.h +++ b/include/libcamera/internal/matrix.h @@ -19,14 +19,11 @@ namespace libcamera { LOG_DECLARE_CATEGORY(Matrix) -#ifndef __DOXYGEN__ -template> * = nullptr> -#else template -#endif /* __DOXYGEN__ */ class Matrix { + static_assert(std::is_arithmetic_v, "Matrix type must be arithmetic"); + public: Matrix() { @@ -123,16 +120,10 @@ Matrix operator*(const Matrix &m, T d) return d * m; } -#ifndef __DOXYGEN__ -template * = nullptr> -#else -template -#endif /* __DOXYGEN__ */ -Matrix operator*(const Matrix &m1, const Matrix &m2) +template +constexpr Matrix operator*(const Matrix &m1, const Matrix &m2) { + static_assert(C1 == R2, "Matrix dimensions must match for multiplication"); Matrix result; for (unsigned int i = 0; i < R1; i++) { From patchwork Thu Apr 3 15:49:07 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23118 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 E0CB0C327D for ; Thu, 3 Apr 2025 15:49:42 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 97C9B689A8; Thu, 3 Apr 2025 17:49:42 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="UjaJQHE2"; 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 EB826689A4 for ; Thu, 3 Apr 2025 17:49:40 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:6d9d:9854:3fc1:4bb2]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 458AB8FA; Thu, 3 Apr 2025 17:47:47 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1743695267; bh=/lShqxVKPC83P8c7ZF16LUmZo5jxREkUBIJoqFx7/k0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=UjaJQHE2eskJNvLGNEU0NJZCJdv+SG4T6z9XdcV6/hEIkE+/MHGnPfAJaB9hM696u NF+sQmGnD7ZTlb4zt3tJZO5v1qvqzCu8jLwE+lyfEGOmnYFzj9slydyMEi+1H8KQN3 Cq9gOEtQhGsKSZGEO63tJf+zPgY38KneMV2N7RfA= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Laurent Pinchart Subject: [PATCH v3 02/16] libcamera: matrix: Make most functions constexpr Date: Thu, 3 Apr 2025 17:49:07 +0200 Message-ID: <20250403154925.382973-3-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250403154925.382973-1-stefan.klug@ideasonboard.com> References: <20250403154925.382973-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" By zero-initializing the data_ member we can make most functions constexpr which will come in handy in upcoming patches. Note that this is due to C++17. In C++20 we will be able to leave data_ uninitialized for constexpr. The Matrix(std::array) version of the constructor can not be constexpr because std::copy only became constexpr in C++20. Signed-off-by: Stefan Klug Reviewed-by: Laurent Pinchart --- Changes in v2: - Added this patch Changes in v3: - Added comment on data_ initializer --- include/libcamera/internal/matrix.h | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h index b9c3d41ef855..512c1162c3bc 100644 --- a/include/libcamera/internal/matrix.h +++ b/include/libcamera/internal/matrix.h @@ -25,9 +25,8 @@ class Matrix static_assert(std::is_arithmetic_v, "Matrix type must be arithmetic"); public: - Matrix() + constexpr Matrix() { - data_.fill(static_cast(0)); } Matrix(const std::array &data) @@ -35,7 +34,7 @@ public: std::copy(data.begin(), data.end(), data_.begin()); } - static Matrix identity() + static constexpr Matrix identity() { Matrix ret; for (size_t i = 0; i < std::min(Rows, Cols); i++) @@ -63,14 +62,14 @@ public: return out.str(); } - Span data() const { return data_; } + constexpr Span data() const { return data_; } - Span operator[](size_t i) const + constexpr Span operator[](size_t i) const { return Span{ &data_.data()[i * Cols], Cols }; } - Span operator[](size_t i) + constexpr Span operator[](size_t i) { return Span{ &data_.data()[i * Cols], Cols }; } @@ -88,7 +87,12 @@ public: } private: - std::array data_; + /* + * \todo The initializer is only necessary for the constructor to be + * constexpr in C++17. Remove the initializer as soon as we are on + * C++20. + */ + std::array data_ = {}; }; #ifndef __DOXYGEN__ @@ -141,7 +145,7 @@ constexpr Matrix operator*(const Matrix &m1, const Matrix< } template -Matrix operator+(const Matrix &m1, const Matrix &m2) +constexpr Matrix operator+(const Matrix &m1, const Matrix &m2) { Matrix result; From patchwork Thu Apr 3 15:49:08 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23119 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 98046C327D for ; Thu, 3 Apr 2025 15:49:45 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 50537689AA; Thu, 3 Apr 2025 17:49:45 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="DX2kuY85"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id CDD41689A4 for ; Thu, 3 Apr 2025 17:49:43 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:6d9d:9854:3fc1:4bb2]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2845B11E9; Thu, 3 Apr 2025 17:47:50 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1743695270; bh=BVgpPik1tU5qZTphpyFI3gMMCarUSWs3223pG75Rli8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=DX2kuY85j/htvaGR9SfWLXyabHS5phnGtLRtnRyD/SgpYppZyannq7zJddbjm2shl yKHXy3IuTwAk/hKjoQ9TEHWzQuQ3u2r6XKu+L/0PTGYsB57OzH9HCMRuDNJ+VGeLBy +aVev0V9m9+FOgPEhJU6aebS6l/CXgCFiE/Yn884= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Kieran Bingham , Laurent Pinchart Subject: [PATCH v3 03/16] libcamera: matrix: Add a Span based constructor Date: Thu, 3 Apr 2025 17:49:08 +0200 Message-ID: <20250403154925.382973-4-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250403154925.382973-1-stefan.klug@ideasonboard.com> References: <20250403154925.382973-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 one wants to create a Matrix from existing data, currently the only way is via std::array. Add a Span based constructor to allow creation from vectors and alike. Signed-off-by: Stefan Klug Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- Changes in v2: - Added this patch Changes in v3: - Pass the Span by value --- include/libcamera/internal/matrix.h | 5 +++++ src/libcamera/matrix.cpp | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h index 512c1162c3bc..6d40567af0a0 100644 --- a/include/libcamera/internal/matrix.h +++ b/include/libcamera/internal/matrix.h @@ -34,6 +34,11 @@ public: std::copy(data.begin(), data.end(), data_.begin()); } + Matrix(const Span data) + { + std::copy(data.begin(), data.end(), data_.begin()); + } + static constexpr Matrix identity() { Matrix ret; diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp index e7e027225666..49e2aa3b4f2c 100644 --- a/src/libcamera/matrix.cpp +++ b/src/libcamera/matrix.cpp @@ -41,6 +41,16 @@ LOG_DEFINE_CATEGORY(Matrix) * number of rows and columns of the matrix (Rows x Cols). */ +/** + * \fn Matrix::Matrix(const Span data) + * \brief Construct a matrix from supplied data + * \param[in] data Data from which to construct a matrix + * + * \a data is a one-dimensional Span and will be turned into a matrix in + * row-major order. The size of \a data must be equal to the product of the + * number of rows and columns of the matrix (Rows x Cols). + */ + /** * \fn Matrix::identity() * \brief Construct an identity matrix From patchwork Thu Apr 3 15:49:09 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23120 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 1FCCFC327D for ; Thu, 3 Apr 2025 15:49:49 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D1299689B0; Thu, 3 Apr 2025 17:49:48 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="OGQD3KOF"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2EFD4689AB for ; Thu, 3 Apr 2025 17:49:47 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:6d9d:9854:3fc1:4bb2]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8614D11E9; Thu, 3 Apr 2025 17:47:53 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1743695273; bh=fS1S7ccPPk7/gi+Who3oQC3Xq9n+HV5xUYPEfyPmlJg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=OGQD3KOFsKFE339b8vAPsN1RJwLOVFSAopNBrYAYKXdEka0LZZ86TJux/E6scXccG qFAJvXEgfw+vOmEhFm57vuBHdeq1cfXQosawcWL79bRu4o4nwg3vD2Tg4lTLYCv2xY dzROQkZKG/tWFCK4YPiLsQXNoAziN52LxxzSruP8= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Laurent Pinchart Subject: [PATCH v3 04/16] libcamera: vector: Add a Span based constructor Date: Thu, 3 Apr 2025 17:49:09 +0200 Message-ID: <20250403154925.382973-5-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250403154925.382973-1-stefan.klug@ideasonboard.com> References: <20250403154925.382973-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 one wants to create a Vector from existing data, currently the only way is via std::array. Add a Span based constructor to allow creation from std::vectors and alike. While at it, replace the manual loop with std::copy. Signed-off-by: Stefan Klug Reviewed-by: Laurent Pinchart --- Changes in v2: - Added this patch Changes in v3: - Improved commit message - Added constexpr to the contructor - Pass Span by value - Removed initializer of data_ member which was only needed for the default constructor to be actually constexpr. But Vector is not used as constexpr anywhere, so we can leave that as is (This will fix itself with C++20). --- include/libcamera/internal/vector.h | 8 ++++++-- src/libcamera/vector.cpp | 8 ++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/include/libcamera/internal/vector.h b/include/libcamera/internal/vector.h index a67a09474204..4e9ef1ee6853 100644 --- a/include/libcamera/internal/vector.h +++ b/include/libcamera/internal/vector.h @@ -42,8 +42,12 @@ public: constexpr Vector(const std::array &data) { - for (unsigned int i = 0; i < Rows; i++) - data_[i] = data[i]; + std::copy(data.begin(), data.end(), data_.begin()); + } + + constexpr Vector(const Span data) + { + std::copy(data.begin(), data.end(), data_.begin()); } const T &operator[](size_t i) const diff --git a/src/libcamera/vector.cpp b/src/libcamera/vector.cpp index 85ca2208245a..5567d5b8defb 100644 --- a/src/libcamera/vector.cpp +++ b/src/libcamera/vector.cpp @@ -44,6 +44,14 @@ LOG_DEFINE_CATEGORY(Vector) * The size of \a data must be equal to the dimension size Rows of the vector. */ +/** + * \fn Vector::Vector(const Span data) + * \brief Construct vector from supplied data + * \param data Data from which to construct a vector + * + * The size of \a data must be equal to the dimension size Rows of the vector. + */ + /** * \fn T Vector::operator[](size_t i) const * \brief Index to an element in the vector From patchwork Thu Apr 3 15:49:10 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23121 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 04AC2C327D for ; Thu, 3 Apr 2025 15:49:52 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id A271B689D2; Thu, 3 Apr 2025 17:49:51 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="mDLO6sUM"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1345E689AD for ; Thu, 3 Apr 2025 17:49:50 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:6d9d:9854:3fc1:4bb2]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 62BBD11E9; Thu, 3 Apr 2025 17:47:56 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1743695276; bh=4F4zExUEB+OlZERtsESklQcZSlANX3zcMQjF6KU8TpY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=mDLO6sUM5sWaNFQ53UE2WWy+r7V8wyyKvZjnY7+/bh98BGX/BaApieWjtc4JgQzpB VDm4u1NkJ1oVSPyCyPVJ2S7cdS6k2C41tHmDEIYpNwlUJ2hsGHgUsrIofUUc9loyJ9 sdfVOoGzvrDviQA6xF3uhl9jCym2zQBQKRJO7qHY= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Laurent Pinchart Subject: [PATCH v3 05/16] libcamera: matrix: Add inverse() function Date: Thu, 3 Apr 2025 17:49:10 +0200 Message-ID: <20250403154925.382973-6-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250403154925.382973-1-stefan.klug@ideasonboard.com> References: <20250403154925.382973-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 calculations in upcoming algorithm patches, the inverse of a matrix is required. Add an implementation of the inverse() function for square matrices. Signed-off-by: Stefan Klug Signed-off-by: Laurent Pinchart --- Changes in v2: - Replaced the implementation by a generic one provided by Laurent that supports arbitrary square matrices instead of 2x2 and 3x3 only. - Moved the implementation into the cpp file. Changes inv3: - Added stack allocated scratchBuffers to matrixInvert so that no heap allocations are necessary. --- include/libcamera/internal/matrix.h | 23 ++++ src/libcamera/matrix.cpp | 166 ++++++++++++++++++++++++++++ 2 files changed, 189 insertions(+) diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h index 6d40567af0a0..a07a47701336 100644 --- a/include/libcamera/internal/matrix.h +++ b/include/libcamera/internal/matrix.h @@ -19,6 +19,12 @@ namespace libcamera { LOG_DECLARE_CATEGORY(Matrix) +#ifndef __DOXYGEN__ +template +bool matrixInvert(Span dataIn, Span dataOut, unsigned int dim, + Span scratchBuffer, Span swapBuffer); +#endif /* __DOXYGEN__ */ + template class Matrix { @@ -91,6 +97,23 @@ public: return *this; } + Matrix inverse(bool *ok = nullptr) const + { + static_assert(Rows == Cols, "Matrix must be square"); + + Matrix inverse; + std::array scratchBuffer; + std::array swapBuffer; + bool res = matrixInvert(Span(data_), + Span(inverse.data_), + Rows, + Span(scratchBuffer), + Span(swapBuffer)); + if (ok) + *ok = res; + return inverse; + } + private: /* * \todo The initializer is only necessary for the constructor to be diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp index 49e2aa3b4f2c..68fc1b7bd5ac 100644 --- a/src/libcamera/matrix.cpp +++ b/src/libcamera/matrix.cpp @@ -7,6 +7,12 @@ #include "libcamera/internal/matrix.h" +#include +#include +#include +#include +#include + #include /** @@ -87,6 +93,20 @@ LOG_DEFINE_CATEGORY(Matrix) * \return Row \a i from the matrix, as a Span */ +/** + * \fn Matrix::inverse(bool *ok) const + * \param[out] ok Indicate if the matrix was successfully inverted + * \brief Compute the inverse of the matrix + * + * This function computes the inverse of the matrix. It is only implemented for + * matrices of float and double types. If \a ok is provided it will be set to a + * boolean value to indicate of the inversion was successful. This can be used + * to check if the matrix is singular, in which case the function will return + * an identity matrix. + * + * \return The inverse of the matrix + */ + /** * \fn Matrix::operator[](size_t i) * \copydoc Matrix::operator[](size_t i) const @@ -142,6 +162,152 @@ LOG_DEFINE_CATEGORY(Matrix) */ #ifndef __DOXYGEN__ +template +bool matrixInvert(Span dataIn, Span dataOut, unsigned int dim, + Span scratchBuffer, Span swapBuffer) +{ + /* + * Convenience class to access matrix data, providing a row-major (i,j) + * element accessor through the call operator, and the ability to swap + * rows without modifying the backing storage. + */ + class MatrixAccessor + { + public: + MatrixAccessor(Span data, Span swapBuffer, unsigned int rows, unsigned int cols) + : data_(data), swap_(swapBuffer), rows_(rows), cols_(cols) + { + ASSERT(swap_.size() == rows); + std::iota(swap_.begin(), swap_.end(), T{ 0 }); + } + + T &operator()(unsigned int row, unsigned int col) + { + assert(row < rows_ && col < cols_); + return data_[index(row, col)]; + } + + void swap(unsigned int a, unsigned int b) + { + assert(a < rows_ && a < cols_); + std::swap(swap_[a], swap_[b]); + } + + private: + unsigned int index(unsigned int row, unsigned int col) const + { + return swap_[row] * cols_ + col; + } + + Span data_; + Span swap_; + unsigned int rows_; + unsigned int cols_; + }; + + /* + * Matrix inversion using Gaussian elimination. + * + * Start by augmenting the original matrix with an identiy matrix of + * the same size. + */ + ASSERT(scratchBuffer.size() == dim * dim * 2); + MatrixAccessor matrix(scratchBuffer, swapBuffer, dim, dim * 2); + + for (unsigned int i = 0; i < dim; ++i) { + for (unsigned int j = 0; j < dim; ++j) { + matrix(i, j) = dataIn[i * dim + j]; + matrix(i, j + dim) = T{ 0 }; + } + matrix(i, i + dim) = T{ 1 }; + } + + /* Start by triangularizing the input . */ + for (unsigned int pivot = 0; pivot < dim; ++pivot) { + /* + * Locate the next pivot. To improve numerical stability, use + * the row with the largest value in the pivot's column. + */ + unsigned int row; + T maxValue{ 0 }; + + for (unsigned int i = pivot; i < dim; ++i) { + T value = std::abs(matrix(i, pivot)); + if (maxValue < value) { + maxValue = value; + row = i; + } + } + + /* + * If no pivot is found in the column, the matrix is not + * invertible. Return an identity matrix. + */ + if (maxValue == 0) { + std::fill(dataOut.begin(), dataOut.end(), T{ 0 }); + for (unsigned int i = 0; i < dim; ++i) + dataOut[i * dim + i] = T{ 1 }; + return false; + } + + /* Swap rows to bring the pivot in the right location. */ + matrix.swap(pivot, row); + + /* Process all rows below the pivot to zero the pivot column. */ + const T pivotValue = matrix(pivot, pivot); + + for (unsigned int i = pivot + 1; i < dim; ++i) { + const T factor = matrix(i, pivot) / pivotValue; + + /* + * We know the element in the pivot column will be 0, + * hardcode it instead of computing it. + */ + matrix(i, pivot) = T{ 0 }; + + for (unsigned int j = pivot + 1; j < dim * 2; ++j) + matrix(i, j) -= matrix(pivot, j) * factor; + } + } + + /* + * Then diagonalize the input, walking the diagonal backwards. There's + * no need to update the input matrix, as all the values we would write + * in the top-right triangle aren't used in further calculations (and + * would all by definition be zero). + */ + for (unsigned int pivot = dim - 1; pivot > 0; --pivot) { + const T pivotValue = matrix(pivot, pivot); + + for (unsigned int i = 0; i < pivot; ++i) { + const T factor = matrix(i, pivot) / pivotValue; + + for (unsigned int j = dim; j < dim * 2; ++j) + matrix(i, j) -= matrix(pivot, j) * factor; + } + } + + /* + * Finally, normalize the diagonal and store the result in the output + * data. + */ + for (unsigned int i = 0; i < dim; ++i) { + const T factor = matrix(i, i); + + for (unsigned int j = 0; j < dim; ++j) + dataOut[i * dim + j] = matrix(i, j + dim) / factor; + } + + return true; +} + +template bool matrixInvert(Span dataIn, Span dataOut, + unsigned int dim, Span scratchBuffer, + Span swapBuffer); +template bool matrixInvert(Span data, Span dataOut, + unsigned int dim, Span scratchBuffer, + Span swapBuffer); + /* * The YAML data shall be a list of numerical values. Its size shall be equal * to the product of the number of rows and columns of the matrix (Rows x From patchwork Thu Apr 3 15:49:11 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23122 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 E4612C327D for ; Thu, 3 Apr 2025 15:49:53 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id AAAF1689F4; Thu, 3 Apr 2025 17:49:53 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Aw4VqXci"; 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 0C389689B0 for ; Thu, 3 Apr 2025 17:49:53 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:6d9d:9854:3fc1:4bb2]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 5537511E9; Thu, 3 Apr 2025 17:47:59 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1743695279; bh=Z1BF7QKEtfoUmzmAWPN4o6NI5gfCG08cz/aVy8qvqnA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Aw4VqXcio7g/sABCZdc8whfDrrJwQhn4Qeo++ZVWXRQLcrZtTbOUlnfZTog9v3V9/ Kdz7VcJat7Galey94Tjqq0tSYZrPP8kPHk+2rnVzAxkC71GRSEIOg8G+ajCU9MAIpg NJjZme4UDWG5qpHXR9jykMvCZEuJXsUuiF0hK470= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Laurent Pinchart Subject: [PATCH v3 06/16] test: Add minimal test for Matrix Date: Thu, 3 Apr 2025 17:49:11 +0200 Message-ID: <20250403154925.382973-7-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250403154925.382973-1-stefan.klug@ideasonboard.com> References: <20250403154925.382973-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 few tests for the Matrix class. This is not full fledged but at least a starter. Signed-off-by: Stefan Klug Reviewed-by: Laurent Pinchart --- Changes in v2: - Added this patch Changes in v3: - Fixed test class name --- test/matrix.cpp | 53 ++++++++++++++++++++++++++++++++++++++++++++++++ test/meson.build | 1 + 2 files changed, 54 insertions(+) create mode 100644 test/matrix.cpp diff --git a/test/matrix.cpp b/test/matrix.cpp new file mode 100644 index 000000000000..3a1c8b4c88a2 --- /dev/null +++ b/test/matrix.cpp @@ -0,0 +1,53 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2024, Ideas on Board Oy + * + * Vector tests + */ + +#include "libcamera/internal/matrix.h" + +#include +#include + +#include "test.h" + +using namespace libcamera; + +#define ASSERT_EQ(a, b) \ + if ((a) != (b)) { \ + std::cout << #a " != " #b << " (line " << __LINE__ << ")" \ + << std::endl; \ + return TestFail; \ + } + +class MatrixTest : public Test +{ +protected: + int run() + { + Matrix m1; + + ASSERT_EQ(m1[0][0], 0.0); + ASSERT_EQ(m1[0][1], 0.0); + + constexpr Matrix m2 = Matrix().identity(); + ASSERT_EQ(m2[0][0], 1.0); + ASSERT_EQ(m2[0][1], 0.0); + ASSERT_EQ(m2[1][0], 0.0); + ASSERT_EQ(m2[1][1], 1.0); + + Matrix m3{ { 2.0, 0.0, 0.0, 2.0 } }; + Matrix m4 = m3.inverse(); + + Matrix m5 = m3 * m4; + ASSERT_EQ(m5[0][0], 1.0); + ASSERT_EQ(m5[0][1], 0.0); + ASSERT_EQ(m5[1][0], 0.0); + ASSERT_EQ(m5[1][1], 1.0); + + return TestPass; + } +}; + +TEST_REGISTER(MatrixTest) diff --git a/test/meson.build b/test/meson.build index 4095664994fd..52f04364e4fc 100644 --- a/test/meson.build +++ b/test/meson.build @@ -60,6 +60,7 @@ internal_tests = [ {'name': 'file', 'sources': ['file.cpp']}, {'name': 'flags', 'sources': ['flags.cpp']}, {'name': 'hotplug-cameras', 'sources': ['hotplug-cameras.cpp']}, + {'name': 'matrix', 'sources': ['matrix.cpp']}, {'name': 'message', 'sources': ['message.cpp']}, {'name': 'object', 'sources': ['object.cpp']}, {'name': 'object-delete', 'sources': ['object-delete.cpp']}, From patchwork Thu Apr 3 15:49:12 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23123 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 D7302C327D for ; Thu, 3 Apr 2025 15:49:57 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 92682689F4; Thu, 3 Apr 2025 17:49:57 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="B5veR5re"; 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 C4E3D689B0 for ; Thu, 3 Apr 2025 17:49:55 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:6d9d:9854:3fc1:4bb2]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 230168FA; Thu, 3 Apr 2025 17:48:02 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1743695282; bh=mtbnJFQv/Y2YhJwnkiiTJr0KzlkWp0aEySUOnnfgGe4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=B5veR5reK9otmbwpaB9BZp6Rb1kR5lyDcMsCJjRi0SWjibBe5A8dz/TMMQ6mqnDNp gpv8T5iZW6oGdroGXbWUE13Rmfs+YtL4goaSmx9sfCWmBaapLA21bPlvVRQCSPKSBk 07WP0FIkvSEkXi3+IX8j6ccQh/JWc6gzG/UH0rAM= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Laurent Pinchart , Kieran Bingham Subject: [PATCH v3 07/16] libcamera: matrix: Extend multiplication operator to heterogenous types Date: Thu, 3 Apr 2025 17:49:12 +0200 Message-ID: <20250403154925.382973-8-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250403154925.382973-1-stefan.klug@ideasonboard.com> References: <20250403154925.382973-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: Laurent Pinchart It is useful to multiply matrices of heterogneous types, for instance float and double. Extend the multiplication operator to support this, avoiding the need to convert one of the matrices. The type of the returned matrix is selected automatically to avoid loosing precision. Signed-off-by: Laurent Pinchart Signed-off-by: Stefan Klug Acked-by: Kieran Bingham --- Changes in v2: - Added this patch Changes in v3: - Added my SoB tag - Collected tags --- include/libcamera/internal/matrix.h | 10 ++++++---- src/libcamera/matrix.cpp | 5 +++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h index a07a47701336..47513b9950e4 100644 --- a/include/libcamera/internal/matrix.h +++ b/include/libcamera/internal/matrix.h @@ -8,6 +8,7 @@ #include #include +#include #include #include @@ -152,15 +153,16 @@ Matrix operator*(const Matrix &m, T d) return d * m; } -template -constexpr Matrix operator*(const Matrix &m1, const Matrix &m2) +template +constexpr Matrix, R1, C2> operator*(const Matrix &m1, + const Matrix &m2) { static_assert(C1 == R2, "Matrix dimensions must match for multiplication"); - Matrix result; + Matrix, R1, C2> result; for (unsigned int i = 0; i < R1; i++) { for (unsigned int j = 0; j < C2; j++) { - T sum = 0; + std::common_type_t sum = 0; for (unsigned int k = 0; k < C1; k++) sum += m1[i][k] * m2[k][j]; diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp index 68fc1b7bd5ac..ed22263b58f8 100644 --- a/src/libcamera/matrix.cpp +++ b/src/libcamera/matrix.cpp @@ -138,11 +138,12 @@ LOG_DEFINE_CATEGORY(Matrix) */ /** - * \fn Matrix operator*(const Matrix &m1, const Matrix &m2) + * \fn operator*(const Matrix &m1, const Matrix &m2) * \brief Matrix multiplication - * \tparam T Type of numerical values in the matrices + * \tparam T1 Type of numerical values in the first matrix * \tparam R1 Number of rows in the first matrix * \tparam C1 Number of columns in the first matrix + * \tparam T2 Type of numerical values in the secont matrix * \tparam R2 Number of rows in the second matrix * \tparam C2 Number of columns in the second matrix * \param m1 Multiplicand matrix From patchwork Thu Apr 3 15:49:13 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23124 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 B222FC327D for ; Thu, 3 Apr 2025 15:50:00 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 5F23A68A8E; Thu, 3 Apr 2025 17:50:00 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="SslfHmvS"; 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 21F5468A4C for ; Thu, 3 Apr 2025 17:49:59 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:6d9d:9854:3fc1:4bb2]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 6DE7D11E9; Thu, 3 Apr 2025 17:48:05 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1743695285; bh=/2ARLNaOLEkA/VETV+UoeUvmFPf2wikIcAnv73CnN5Y=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=SslfHmvSOfuLwaopzUN1qOHfVRu7o34XAJbLI+5RpKdazbJ60Vja+L6iJLdDRn24J FI7YxvxOypWOwL4tfvaSwhQJSDuz7Qk+3Z7kDLRU+IhzU9pfKKWSmNuI6GZRkGqxv4 RCdGe0d5wnGBGTyXiVYyWVxXCxy9EAMeYzZ5cieY= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Laurent Pinchart , Kieran Bingham Subject: [PATCH v3 08/16] libcamera: vector: Extend matrix multiplication operator to heterogenous types Date: Thu, 3 Apr 2025 17:49:13 +0200 Message-ID: <20250403154925.382973-9-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250403154925.382973-1-stefan.klug@ideasonboard.com> References: <20250403154925.382973-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: Laurent Pinchart It is useful to multiply matrices and vectors of heterogeneous types, for instance float and double. Extend the multiplication operator to support this, avoiding the need to convert one of the operations. The type of the returned vector is selected automatically to avoid loosing precision. Signed-off-by: Laurent Pinchart Signed-off-by: Stefan Klug Acked-by: Kieran Bingham --- Changes in v2: - Added this patch Changes in v3: - Collected tag --- include/libcamera/internal/vector.h | 9 +++++---- src/libcamera/vector.cpp | 5 +++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/include/libcamera/internal/vector.h b/include/libcamera/internal/vector.h index 4e9ef1ee6853..16b6aef0b38f 100644 --- a/include/libcamera/internal/vector.h +++ b/include/libcamera/internal/vector.h @@ -13,6 +13,7 @@ #include #include #include +#include #include #include @@ -295,13 +296,13 @@ private: template using RGB = Vector; -template -Vector operator*(const Matrix &m, const Vector &v) +template +Vector, Rows> operator*(const Matrix &m, const Vector &v) { - Vector result; + Vector, Rows> result; for (unsigned int i = 0; i < Rows; i++) { - T sum = 0; + std::common_type_t sum = 0; for (unsigned int j = 0; j < Cols; j++) sum += m[i][j] * v[j]; result[i] = sum; diff --git a/src/libcamera/vector.cpp b/src/libcamera/vector.cpp index 5567d5b8defb..4dad1b9001c5 100644 --- a/src/libcamera/vector.cpp +++ b/src/libcamera/vector.cpp @@ -308,9 +308,10 @@ LOG_DEFINE_CATEGORY(Vector) */ /** - * \fn Vector operator*(const Matrix &m, const Vector &v) + * \fn operator*(const Matrix &m, const Vector &v) * \brief Multiply a matrix by a vector - * \tparam T Numerical type of the contents of the matrix and vector + * \tparam T Numerical type of the contents of the matrix + * \tparam U Numerical type of the contents of the vector * \tparam Rows The number of rows in the matrix * \tparam Cols The number of columns in the matrix (= rows in the vector) * \param m The matrix From patchwork Thu Apr 3 15:49:14 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23125 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 72149C327D for ; Thu, 3 Apr 2025 15:50:04 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2EA1268A9F; Thu, 3 Apr 2025 17:50:04 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="SAD2XuI5"; 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 669B568A54 for ; Thu, 3 Apr 2025 17:50:02 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:6d9d:9854:3fc1:4bb2]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id A635E8FA; Thu, 3 Apr 2025 17:48:08 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1743695288; bh=Saqcg+2LVaEKh27fsCjH5nbdr8FI9+BjiGwzGC1Veag=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=SAD2XuI5x7eBoJCIz9z7GDp9/Po60q4LAvuhv12hCbURdANHEYsI/5ji1/EAaq11j nWgQpyoIEwBDvc3U6EtmbkvTlMOeVUTG1+Ui50lTdJPRoaUDZ29rZFKR3DKSWbas/e QPrlR6Hff3+6oHVM1frs4pd5CfSKrfc4F2mFOapE= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Kieran Bingham , Laurent Pinchart Subject: [PATCH v3 09/16] ipa: rkisp1: Refactor automatic/manual structure in IPAActiveState Date: Thu, 3 Apr 2025 17:49:14 +0200 Message-ID: <20250403154925.382973-10-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250403154925.382973-1-stefan.klug@ideasonboard.com> References: <20250403154925.382973-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" Swap gains and automatic/manual in the IPAActiveState structure. This is in preparation to adding another member, which is easier in the new structure. The patch contains no functional changes. Signed-off-by: Stefan Klug Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- Changes in v2: - Use one named struct instead of two anonymous ones Changes in v3: - Collected tags - Updated documentation --- src/ipa/rkisp1/algorithms/awb.cpp | 24 ++++++++++++------------ src/ipa/rkisp1/ipa_context.cpp | 13 ++++++++----- src/ipa/rkisp1/ipa_context.h | 10 ++++++---- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp index eafe93081bb1..a9759e53f593 100644 --- a/src/ipa/rkisp1/algorithms/awb.cpp +++ b/src/ipa/rkisp1/algorithms/awb.cpp @@ -124,8 +124,8 @@ int Awb::init(IPAContext &context, const YamlObject &tuningData) int Awb::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) { - context.activeState.awb.gains.manual = RGB{ 1.0 }; - context.activeState.awb.gains.automatic = + context.activeState.awb.manual.gains = RGB{ 1.0 }; + context.activeState.awb.automatic.gains = awbAlgo_->gainsFromColourTemperature(kDefaultColourTemperature); context.activeState.awb.autoEnabled = true; context.activeState.awb.temperatureK = kDefaultColourTemperature; @@ -173,8 +173,8 @@ void Awb::queueRequest(IPAContext &context, const auto &colourTemperature = controls.get(controls::ColourTemperature); bool update = false; if (colourGains) { - awb.gains.manual.r() = (*colourGains)[0]; - awb.gains.manual.b() = (*colourGains)[1]; + awb.manual.gains.r() = (*colourGains)[0]; + awb.manual.gains.b() = (*colourGains)[1]; /* * \todo Colour temperature reported in metadata is now * incorrect, as we can't deduce the temperature from the gains. @@ -183,17 +183,17 @@ void Awb::queueRequest(IPAContext &context, update = true; } else if (colourTemperature) { const auto &gains = awbAlgo_->gainsFromColourTemperature(*colourTemperature); - awb.gains.manual.r() = gains.r(); - awb.gains.manual.b() = gains.b(); + awb.manual.gains.r() = gains.r(); + awb.manual.gains.b() = gains.b(); awb.temperatureK = *colourTemperature; update = true; } if (update) LOG(RkISP1Awb, Debug) - << "Set colour gains to " << awb.gains.manual; + << "Set colour gains to " << awb.manual.gains; - frameContext.awb.gains = awb.gains.manual; + frameContext.awb.gains = awb.manual.gains; frameContext.awb.temperatureK = awb.temperatureK; } @@ -208,7 +208,7 @@ void Awb::prepare(IPAContext &context, const uint32_t frame, * most up-to-date automatic values we can read. */ if (frameContext.awb.autoEnabled) { - frameContext.awb.gains = context.activeState.awb.gains.automatic; + frameContext.awb.gains = context.activeState.awb.automatic.gains; frameContext.awb.temperatureK = context.activeState.awb.temperatureK; } @@ -325,14 +325,14 @@ void Awb::process(IPAContext &context, /* Filter the values to avoid oscillations. */ double speed = 0.2; awbResult.gains = awbResult.gains * speed + - activeState.awb.gains.automatic * (1 - speed); + activeState.awb.automatic.gains * (1 - speed); - activeState.awb.gains.automatic = awbResult.gains; + activeState.awb.automatic.gains = awbResult.gains; LOG(RkISP1Awb, Debug) << std::showpoint << "Means " << rgbMeans << ", gains " - << activeState.awb.gains.automatic << ", temp " + << activeState.awb.automatic.gains << ", temp " << activeState.awb.temperatureK << "K"; } diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp index 99611bd5b390..39b97d143e95 100644 --- a/src/ipa/rkisp1/ipa_context.cpp +++ b/src/ipa/rkisp1/ipa_context.cpp @@ -191,14 +191,17 @@ namespace libcamera::ipa::rkisp1 { * \var IPAActiveState::awb * \brief State for the Automatic White Balance algorithm * - * \struct IPAActiveState::awb.gains + * \var IPAActiveState::awb::AwbState + * \brief Struct for the AWB regulation state + * + * \struct IPAActiveState::awb::AwbState.gains * \brief White balance gains * - * \var IPAActiveState::awb.gains.manual - * \brief Manual white balance gains (set through requests) + * \var IPAActiveState::awb.manual + * \brief Manual regulation state (set through requests) * - * \var IPAActiveState::awb.gains.automatic - * \brief Automatic white balance gains (computed by the algorithm) + * \var IPAActiveState::awb.automatic + * \brief Automatic regulation state (computed by the algorithm) * * \var IPAActiveState::awb.temperatureK * \brief Estimated color temperature diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index 474f7036f003..6bc922a82971 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/ipa_context.h @@ -89,10 +89,12 @@ struct IPAActiveState { } agc; struct { - struct { - RGB manual; - RGB automatic; - } gains; + struct AwbState { + RGB gains; + }; + + AwbState manual; + AwbState automatic; unsigned int temperatureK; bool autoEnabled; From patchwork Thu Apr 3 15:49:15 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23126 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 33A7FC327D for ; Thu, 3 Apr 2025 15:50:07 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E8A8668AA4; Thu, 3 Apr 2025 17:50:06 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="mPaj8Wmy"; 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 5245768A99 for ; Thu, 3 Apr 2025 17:50:05 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:6d9d:9854:3fc1:4bb2]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id A895B11E9; Thu, 3 Apr 2025 17:48:11 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1743695291; bh=m6PCB6BkuTeM5Q0Jw/DmKkQLqGea+CVDblqFGnecTOo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=mPaj8WmyQF7iZy/7Fr7Gyca8I6kaY6rj8L+F66p5h8UhO6imvqgB3TeqO/5Zl/57c PDrPKCX7ru/856IRJ0xz2/vkAVExu0EjJI2WfvCQK8R/YtKbrwrU7gRHjyR76Wos4b JUs2JlheLPSA5gO8RUGY6ZPS139kkKnmcAi0W9UA= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 10/16] ipa: rkisp1: algorithms: awb: Fix wrong colour temperature reporting Date: Thu, 3 Apr 2025 17:49:15 +0200 Message-ID: <20250403154925.382973-11-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250403154925.382973-1-stefan.klug@ideasonboard.com> References: <20250403154925.382973-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 commit b60bd37b1a49 ("ipa: rkisp1: Move calculation of RGB means into own function") the output of the current measured colour temperature as metadata was incorrectly added. Remove it. Fixes: b60bd37b1a49 ("ipa: rkisp1: Move calculation of RGB means into own function") Signed-off-by: Stefan Klug --- Changes in v3: - Added this patch --- src/ipa/rkisp1/algorithms/awb.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp index a9759e53f593..79c4c658406d 100644 --- a/src/ipa/rkisp1/algorithms/awb.cpp +++ b/src/ipa/rkisp1/algorithms/awb.cpp @@ -311,9 +311,6 @@ void Awb::process(IPAContext &context, activeState.awb.temperatureK = awbResult.colourTemperature; - /* Metadata shall contain the up to date measurement */ - metadata.set(controls::ColourTemperature, activeState.awb.temperatureK); - /* * Clamp the gain values to the hardware, which expresses gains as Q2.8 * unsigned integer values. Set the minimum just above zero to avoid From patchwork Thu Apr 3 15:49:16 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23127 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 2A178C327D for ; Thu, 3 Apr 2025 15:50:09 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D5A6F68AA8; Thu, 3 Apr 2025 17:50:08 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="mne3UvtY"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 062D368A9D for ; Thu, 3 Apr 2025 17:50:08 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:6d9d:9854:3fc1:4bb2]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 58DF511E9; Thu, 3 Apr 2025 17:48:14 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1743695294; bh=afdwa1VAMc7wsQSQhj/okcIeAZ3l+Zxq+xMthG3B45U=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=mne3UvtY5mNqIuPFaIJSUxuJFlx/Vxg6JHJGSwwObVQxj6WtyOry0+jN/7Rafxmo4 l5G6/uVWEXQZhR6WsLeycAmgGB+CiF54z+yx2t9SeHrC7rvoZT76GhlvEB8lIYQ/Qk GweBMWzFl9n8hr2p210tG1HmuSttDpREsIEArVrc= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug Subject: [PATCH v3 11/16] ipa: rkisp1: ccm/lsc: Fix CCM/LSC based on manual color temperature Date: Thu, 3 Apr 2025 17:49:16 +0200 Message-ID: <20250403154925.382973-12-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250403154925.382973-1-stefan.klug@ideasonboard.com> References: <20250403154925.382973-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 RkISP1Awb::process(), the color temperature in the active state is updated every time new statistics are available. The CCM/LSC algorithms use that value in prepare() to update the CCM/LSC. This is not correct if the color temperature was specified manually and leads to visible flicker even when AwbEnable is set to false. To fix that, track the auto and manual color temperature separately in active state. In Awb::prepare() the current frame context is updated with the corresponding value from active state. Change the algorithms to fetch the color temperature from the frame context instead of the active state in prepare(). Fixes: 02308809548d ("ipa: rkisp1: awb: Implement ColourTemperature control") Signed-off-by: Stefan Klug --- Changes in v2: - None Changes in v3: - Move incorrect colour temperature metadata to separate fixup patch - Updated documentation --- src/ipa/rkisp1/algorithms/awb.cpp | 16 +++++++++------- src/ipa/rkisp1/algorithms/ccm.cpp | 2 +- src/ipa/rkisp1/algorithms/lsc.cpp | 6 +++--- src/ipa/rkisp1/ipa_context.cpp | 6 +++--- src/ipa/rkisp1/ipa_context.h | 2 +- 5 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp index 79c4c658406d..0795b8e5b1e1 100644 --- a/src/ipa/rkisp1/algorithms/awb.cpp +++ b/src/ipa/rkisp1/algorithms/awb.cpp @@ -128,7 +128,8 @@ int Awb::configure(IPAContext &context, context.activeState.awb.automatic.gains = awbAlgo_->gainsFromColourTemperature(kDefaultColourTemperature); context.activeState.awb.autoEnabled = true; - context.activeState.awb.temperatureK = kDefaultColourTemperature; + context.activeState.awb.manual.temperatureK = kDefaultColourTemperature; + context.activeState.awb.automatic.temperatureK = kDefaultColourTemperature; /* * Define the measurement window for AWB as a centered rectangle @@ -185,7 +186,7 @@ void Awb::queueRequest(IPAContext &context, const auto &gains = awbAlgo_->gainsFromColourTemperature(*colourTemperature); awb.manual.gains.r() = gains.r(); awb.manual.gains.b() = gains.b(); - awb.temperatureK = *colourTemperature; + awb.manual.temperatureK = *colourTemperature; update = true; } @@ -194,7 +195,7 @@ void Awb::queueRequest(IPAContext &context, << "Set colour gains to " << awb.manual.gains; frameContext.awb.gains = awb.manual.gains; - frameContext.awb.temperatureK = awb.temperatureK; + frameContext.awb.temperatureK = awb.manual.temperatureK; } /** @@ -208,8 +209,9 @@ void Awb::prepare(IPAContext &context, const uint32_t frame, * most up-to-date automatic values we can read. */ if (frameContext.awb.autoEnabled) { - frameContext.awb.gains = context.activeState.awb.automatic.gains; - frameContext.awb.temperatureK = context.activeState.awb.temperatureK; + auto &awb = context.activeState.awb; + frameContext.awb.gains = awb.automatic.gains; + frameContext.awb.temperatureK = awb.automatic.temperatureK; } auto gainConfig = params->block(); @@ -309,7 +311,7 @@ void Awb::process(IPAContext &context, RkISP1AwbStats awbStats{ rgbMeans }; AwbResult awbResult = awbAlgo_->calculateAwb(awbStats, frameContext.lux.lux); - activeState.awb.temperatureK = awbResult.colourTemperature; + activeState.awb.automatic.temperatureK = awbResult.colourTemperature; /* * Clamp the gain values to the hardware, which expresses gains as Q2.8 @@ -330,7 +332,7 @@ void Awb::process(IPAContext &context, << std::showpoint << "Means " << rgbMeans << ", gains " << activeState.awb.automatic.gains << ", temp " - << activeState.awb.temperatureK << "K"; + << activeState.awb.automatic.temperatureK << "K"; } RGB Awb::calculateRgbMeans(const IPAFrameContext &frameContext, const rkisp1_cif_isp_awb_stat *awb) const diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp index eb8ca39e56a8..2e5e91006b55 100644 --- a/src/ipa/rkisp1/algorithms/ccm.cpp +++ b/src/ipa/rkisp1/algorithms/ccm.cpp @@ -88,7 +88,7 @@ void Ccm::setParameters(struct rkisp1_cif_isp_ctk_config &config, void Ccm::prepare(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, RkISP1Params *params) { - uint32_t ct = context.activeState.awb.temperatureK; + uint32_t ct = frameContext.awb.temperatureK; /* * \todo The colour temperature will likely be noisy, add filtering to diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp index e47aa2f0727e..e7301bfec863 100644 --- a/src/ipa/rkisp1/algorithms/lsc.cpp +++ b/src/ipa/rkisp1/algorithms/lsc.cpp @@ -404,12 +404,12 @@ void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config, /** * \copydoc libcamera::ipa::Algorithm::prepare */ -void LensShadingCorrection::prepare(IPAContext &context, +void LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context, [[maybe_unused]] const uint32_t frame, - [[maybe_unused]] IPAFrameContext &frameContext, + IPAFrameContext &frameContext, RkISP1Params *params) { - uint32_t ct = context.activeState.awb.temperatureK; + uint32_t ct = frameContext.awb.temperatureK; if (std::abs(static_cast(ct) - static_cast(lastAppliedCt_)) < kColourTemperatureChangeThreshhold) return; diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp index 39b97d143e95..7bc42e6de415 100644 --- a/src/ipa/rkisp1/ipa_context.cpp +++ b/src/ipa/rkisp1/ipa_context.cpp @@ -197,15 +197,15 @@ namespace libcamera::ipa::rkisp1 { * \struct IPAActiveState::awb::AwbState.gains * \brief White balance gains * + * \var IPAActiveState::awb::AwbState.temperatureK + * \brief Estimated color temperature + * * \var IPAActiveState::awb.manual * \brief Manual regulation state (set through requests) * * \var IPAActiveState::awb.automatic * \brief Automatic regulation state (computed by the algorithm) * - * \var IPAActiveState::awb.temperatureK - * \brief Estimated color temperature - * * \var IPAActiveState::awb.autoEnabled * \brief Whether the Auto White Balance algorithm is enabled */ diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index 6bc922a82971..769e9f114e23 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/ipa_context.h @@ -91,12 +91,12 @@ struct IPAActiveState { struct { struct AwbState { RGB gains; + unsigned int temperatureK; }; AwbState manual; AwbState automatic; - unsigned int temperatureK; bool autoEnabled; } awb; From patchwork Thu Apr 3 15:49:17 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23128 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 B1A38C327D for ; Thu, 3 Apr 2025 15:50:12 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 7E791689A6; Thu, 3 Apr 2025 17:50:12 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="vhmIra8J"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8492468AA4 for ; Thu, 3 Apr 2025 17:50:10 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:6d9d:9854:3fc1:4bb2]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D7787187E; Thu, 3 Apr 2025 17:48:16 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1743695297; bh=XVJ6k1NgH9yzrpCcHd6VFsu7oZJjgiXNQoL9pqOdxVM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=vhmIra8JDLKGoaBKOU5dxsEMiPHHL8ZzKy2ohH51wq3BzZeFXs5ol70Hy/L9H0VpT RUEI4Blah6+35Dg4bnGyV5ENVDXRRHH9ITmVgxnG/jE/+3qbFmzdsA6oJH6K8MT1Ad 4bf1ELoU6UecaW2qCHL+lScNWn/bpWRGkpm5i8Z0= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Kieran Bingham Subject: [PATCH v3 12/16] ipa: rkisp1: Implement manual ColourCorrectionMatrix control Date: Thu, 3 Apr 2025 17:49:17 +0200 Message-ID: <20250403154925.382973-13-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250403154925.382973-1-stefan.klug@ideasonboard.com> References: <20250403154925.382973-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 manual ColourCorrectionMatrix control. This was already discussed while implementing manual colour temperature but was never implemented. The control allows to manually specify the CCM when AwbEnable is false. Signed-off-by: Stefan Klug Reviewed-by: Kieran Bingham --- Changes in v2: - None Changes in v3: - Removed unnecessary log statement - Improved debug logging for manual ccm - Added documentation for IPAActiveState::ccm - Added documentation for IPAFrameContext::ccm --- src/ipa/rkisp1/algorithms/ccm.cpp | 62 ++++++++++++++++++++++++++++--- src/ipa/rkisp1/algorithms/ccm.h | 6 +++ src/ipa/rkisp1/ipa_context.cpp | 19 ++++++++++ src/ipa/rkisp1/ipa_context.h | 3 +- 4 files changed, 84 insertions(+), 6 deletions(-) diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp index 2e5e91006b55..3a96a5427bc6 100644 --- a/src/ipa/rkisp1/algorithms/ccm.cpp +++ b/src/ipa/rkisp1/algorithms/ccm.cpp @@ -36,17 +36,25 @@ namespace ipa::rkisp1::algorithms { LOG_DEFINE_CATEGORY(RkISP1Ccm) +constexpr Matrix kIdentity3x3 = Matrix::identity(); + /** * \copydoc libcamera::ipa::Algorithm::init */ int Ccm::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData) { + auto &cmap = context.ctrlMap; + cmap[&controls::ColourCorrectionMatrix] = ControlInfo( + ControlValue(-8.0f), + ControlValue(7.993f), + ControlValue(kIdentity3x3.data())); + int ret = ccm_.readYaml(tuningData["ccms"], "ct", "ccm"); if (ret < 0) { LOG(RkISP1Ccm, Warning) << "Failed to parse 'ccm' " << "parameter from tuning file; falling back to unit matrix"; - ccm_.setData({ { 0, Matrix::identity() } }); + ccm_.setData({ { 0, kIdentity3x3 } }); } ret = offsets_.readYaml(tuningData["ccms"], "ct", "offsets"); @@ -61,13 +69,51 @@ int Ccm::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData return 0; } +/** + * \copydoc libcamera::ipa::Algorithm::configure + */ +int Ccm::configure(IPAContext &context, + [[maybe_unused]] const IPACameraSensorInfo &configInfo) +{ + auto &as = context.activeState; + as.ccm.manual = kIdentity3x3; + as.ccm.automatic = ccm_.getInterpolated(as.awb.automatic.temperatureK); + return 0; +} + +void Ccm::queueRequest(IPAContext &context, + [[maybe_unused]] const uint32_t frame, + IPAFrameContext &frameContext, + const ControlList &controls) +{ + /* Nothing to do here, the ccm will be calculated in prepare() */ + if (frameContext.awb.autoEnabled) + return; + + auto &ccm = context.activeState.ccm; + + const auto &colourTemperature = controls.get(controls::ColourTemperature); + const auto &ccmMatrix = controls.get(controls::ColourCorrectionMatrix); + if (ccmMatrix) { + ccm.manual = Matrix(*ccmMatrix); + LOG(RkISP1Ccm, Debug) + << "Setting manual CCM from CCM control to " << ccm.manual; + } else if (colourTemperature) { + ccm.manual = ccm_.getInterpolated(*colourTemperature); + LOG(RkISP1Ccm, Debug) + << "Setting manual CCM from CT control to " << ccm.manual; + } + + frameContext.ccm.ccm = ccm.manual; +} + void Ccm::setParameters(struct rkisp1_cif_isp_ctk_config &config, const Matrix &matrix, const Matrix &offsets) { /* * 4 bit integer and 7 bit fractional, ranging from -8 (0x400) to - * +7.992 (0x3ff) + * +7.9921875 (0x3ff) */ for (unsigned int i = 0; i < 3; i++) { for (unsigned int j = 0; j < 3; j++) @@ -88,14 +134,20 @@ void Ccm::setParameters(struct rkisp1_cif_isp_ctk_config &config, void Ccm::prepare(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, RkISP1Params *params) { - uint32_t ct = frameContext.awb.temperatureK; + if (!frameContext.awb.autoEnabled) { + auto config = params->block(); + config.setEnabled(true); + setParameters(*config, frameContext.ccm.ccm, Matrix()); + return; + } + uint32_t ct = frameContext.awb.temperatureK; /* * \todo The colour temperature will likely be noisy, add filtering to * avoid updating the CCM matrix all the time. */ if (frame > 0 && ct == ct_) { - frameContext.ccm.ccm = context.activeState.ccm.ccm; + frameContext.ccm.ccm = context.activeState.ccm.automatic; return; } @@ -103,7 +155,7 @@ void Ccm::prepare(IPAContext &context, const uint32_t frame, Matrix ccm = ccm_.getInterpolated(ct); Matrix offsets = offsets_.getInterpolated(ct); - context.activeState.ccm.ccm = ccm; + context.activeState.ccm.automatic = ccm; frameContext.ccm.ccm = ccm; auto config = params->block(); diff --git a/src/ipa/rkisp1/algorithms/ccm.h b/src/ipa/rkisp1/algorithms/ccm.h index a5d9a9a45e5d..c301e6e531c8 100644 --- a/src/ipa/rkisp1/algorithms/ccm.h +++ b/src/ipa/rkisp1/algorithms/ccm.h @@ -26,6 +26,12 @@ public: ~Ccm() = default; int init(IPAContext &context, const YamlObject &tuningData) override; + int configure(IPAContext &context, + const IPACameraSensorInfo &configInfo) override; + void queueRequest(IPAContext &context, + const uint32_t frame, + IPAFrameContext &frameContext, + const ControlList &controls) override; void prepare(IPAContext &context, const uint32_t frame, IPAFrameContext &frameContext, RkISP1Params *params) override; diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp index 7bc42e6de415..cec73c9610da 100644 --- a/src/ipa/rkisp1/ipa_context.cpp +++ b/src/ipa/rkisp1/ipa_context.cpp @@ -210,6 +210,17 @@ namespace libcamera::ipa::rkisp1 { * \brief Whether the Auto White Balance algorithm is enabled */ +/** + * \var IPAActiveState::ccm + * \brief State for the Colour Correction Matrix algorithm + * + * \var IPAActiveState::ccm.manual + * \brief Manual CCM (set through requests) + * + * \var IPAActiveState::awb.automatic + * \brief Automatic CCM (computed by the algorithm) + */ + /** * \var IPAActiveState::cproc * \brief State for the Color Processing algorithm @@ -355,6 +366,14 @@ namespace libcamera::ipa::rkisp1 { * \brief Whether the Auto White Balance algorithm is enabled */ +/** + * \var IPAFrameContext::ccm + * \brief Colour Correction Matrix parameters for this frame + * + * \struct IPAFrameContext::ccm.ccm + * \brief Colour Correction Matrix + */ + /** * \var IPAFrameContext::cproc * \brief Color Processing parameters for this frame diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h index 769e9f114e23..f0d504215d34 100644 --- a/src/ipa/rkisp1/ipa_context.h +++ b/src/ipa/rkisp1/ipa_context.h @@ -101,7 +101,8 @@ struct IPAActiveState { } awb; struct { - Matrix ccm; + Matrix manual; + Matrix automatic; } ccm; struct { From patchwork Thu Apr 3 15:49:18 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23129 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 48C0DC327D for ; Thu, 3 Apr 2025 15:50:15 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 07E0D68AB0; Thu, 3 Apr 2025 17:50:15 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="O5xNAHxn"; 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 CDC3268AAA for ; Thu, 3 Apr 2025 17:50:13 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:6d9d:9854:3fc1:4bb2]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id F04CC1624; Thu, 3 Apr 2025 17:48:19 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1743695300; bh=0EBIs75tGHiJIMtf4sqRcoFq8xrksXXMdoCdOFfqdYk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=O5xNAHxnxIhvJNNLOPmiLWp0FTlsA/fTAl/FvUbcpFL0TcR8W+u6T4oLgrzvNdN78 5fEJnPNZwuZMcym8oispdND8kFYFiwLJuIN3LjAqPEvWu+endpUXCFAffpAqOxZf2j E9Omv0aeTNLnyq2prRmpvpD26nlQs0P6LiIo9Pmc= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Kieran Bingham Subject: [PATCH v3 13/16] libipa: awb: Make result of gainsFromColourTemp optional Date: Thu, 3 Apr 2025 17:49:18 +0200 Message-ID: <20250403154925.382973-14-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250403154925.382973-1-stefan.klug@ideasonboard.com> References: <20250403154925.382973-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 grey world AWB case, if no colour gains are contained in the tuning file, the colour gains get reset to 1 when the colour temperature is set manually. This is unexpected and undesirable. Allow the gainsFromColourTemp() function to return a std::nullopt to handle that case. While at it, remove an unnecessary import from rkisp1/algorithms/awb.h. Signed-off-by: Stefan Klug Reviewed-by: Kieran Bingham --- Changes in v2: - Added this patch Changes in v3: - Added #include to libipa/awb.h - Removed stray include from rkisp1/awb.h - Reordered colour temperature assignment for better clarity --- src/ipa/libipa/awb.cpp | 2 +- src/ipa/libipa/awb.h | 3 ++- src/ipa/libipa/awb_bayes.cpp | 4 ++-- src/ipa/libipa/awb_bayes.h | 2 +- src/ipa/libipa/awb_grey.cpp | 6 +++--- src/ipa/libipa/awb_grey.h | 2 +- src/ipa/rkisp1/algorithms/awb.cpp | 18 ++++++++++++------ src/ipa/rkisp1/algorithms/awb.h | 2 -- 8 files changed, 22 insertions(+), 17 deletions(-) diff --git a/src/ipa/libipa/awb.cpp b/src/ipa/libipa/awb.cpp index 925fac232709..214bac8b56a6 100644 --- a/src/ipa/libipa/awb.cpp +++ b/src/ipa/libipa/awb.cpp @@ -114,7 +114,7 @@ namespace ipa { * does not take any statistics into account. It is used to compute the colour * gains when the user manually specifies a colour temperature. * - * \return The colour gains + * \return The colour gains or std::nullopt if the conversion is not possible */ /** diff --git a/src/ipa/libipa/awb.h b/src/ipa/libipa/awb.h index 4bab7a451ce5..f4a86038635f 100644 --- a/src/ipa/libipa/awb.h +++ b/src/ipa/libipa/awb.h @@ -8,6 +8,7 @@ #pragma once #include +#include #include #include @@ -39,7 +40,7 @@ public: virtual int init(const YamlObject &tuningData) = 0; virtual AwbResult calculateAwb(const AwbStats &stats, unsigned int lux) = 0; - virtual RGB gainsFromColourTemperature(double colourTemperature) = 0; + virtual std::optional> gainsFromColourTemperature(double colourTemperature) = 0; const ControlInfoMap::Map &controls() const { diff --git a/src/ipa/libipa/awb_bayes.cpp b/src/ipa/libipa/awb_bayes.cpp index d1d0eaf0e68f..d2bcbd83d7f8 100644 --- a/src/ipa/libipa/awb_bayes.cpp +++ b/src/ipa/libipa/awb_bayes.cpp @@ -270,7 +270,7 @@ void AwbBayes::handleControls(const ControlList &controls) } } -RGB AwbBayes::gainsFromColourTemperature(double colourTemperature) +std::optional> AwbBayes::gainsFromColourTemperature(double colourTemperature) { /* * \todo In the RaspberryPi code, the ct curve was interpolated in @@ -278,7 +278,7 @@ RGB AwbBayes::gainsFromColourTemperature(double colourTemperature) * intuitive, as the gains are in linear space. But I can't prove it. */ const auto &gains = colourGainCurve_.getInterpolated(colourTemperature); - return { { gains[0], 1.0, gains[1] } }; + return RGB{ { gains[0], 1.0, gains[1] } }; } AwbResult AwbBayes::calculateAwb(const AwbStats &stats, unsigned int lux) diff --git a/src/ipa/libipa/awb_bayes.h b/src/ipa/libipa/awb_bayes.h index bb933038d6d2..47ef3cce4d58 100644 --- a/src/ipa/libipa/awb_bayes.h +++ b/src/ipa/libipa/awb_bayes.h @@ -27,7 +27,7 @@ public: int init(const YamlObject &tuningData) override; AwbResult calculateAwb(const AwbStats &stats, unsigned int lux) override; - RGB gainsFromColourTemperature(double temperatureK) override; + std::optional> gainsFromColourTemperature(double temperatureK) override; void handleControls(const ControlList &controls) override; private: diff --git a/src/ipa/libipa/awb_grey.cpp b/src/ipa/libipa/awb_grey.cpp index d3d2132b8869..d252edb2b6c6 100644 --- a/src/ipa/libipa/awb_grey.cpp +++ b/src/ipa/libipa/awb_grey.cpp @@ -98,15 +98,15 @@ AwbResult AwbGrey::calculateAwb(const AwbStats &stats, [[maybe_unused]] unsigned * \return The colour gains if a colour temperature curve is available, * [1, 1, 1] otherwise. */ -RGB AwbGrey::gainsFromColourTemperature(double colourTemperature) +std::optional> AwbGrey::gainsFromColourTemperature(double colourTemperature) { if (!colourGainCurve_) { LOG(Awb, Error) << "No gains defined"; - return RGB({ 1.0, 1.0, 1.0 }); + return std::nullopt; } auto gains = colourGainCurve_->getInterpolated(colourTemperature); - return { { gains[0], 1.0, gains[1] } }; + return RGB{ { gains[0], 1.0, gains[1] } }; } } /* namespace ipa */ diff --git a/src/ipa/libipa/awb_grey.h b/src/ipa/libipa/awb_grey.h index 7ec7bfa5da9a..f82a368d11cc 100644 --- a/src/ipa/libipa/awb_grey.h +++ b/src/ipa/libipa/awb_grey.h @@ -25,7 +25,7 @@ public: int init(const YamlObject &tuningData) override; AwbResult calculateAwb(const AwbStats &stats, unsigned int lux) override; - RGB gainsFromColourTemperature(double colourTemperature) override; + std::optional> gainsFromColourTemperature(double colourTemperature) override; private: std::optional>> colourGainCurve_; diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp index 0795b8e5b1e1..f5415a0bb99d 100644 --- a/src/ipa/rkisp1/algorithms/awb.cpp +++ b/src/ipa/rkisp1/algorithms/awb.cpp @@ -125,8 +125,12 @@ int Awb::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) { context.activeState.awb.manual.gains = RGB{ 1.0 }; - context.activeState.awb.automatic.gains = - awbAlgo_->gainsFromColourTemperature(kDefaultColourTemperature); + auto gains = awbAlgo_->gainsFromColourTemperature(kDefaultColourTemperature); + if (gains) + context.activeState.awb.automatic.gains = *gains; + else + context.activeState.awb.automatic.gains = RGB{ 1.0 }; + context.activeState.awb.autoEnabled = true; context.activeState.awb.manual.temperatureK = kDefaultColourTemperature; context.activeState.awb.automatic.temperatureK = kDefaultColourTemperature; @@ -183,11 +187,13 @@ void Awb::queueRequest(IPAContext &context, */ update = true; } else if (colourTemperature) { - const auto &gains = awbAlgo_->gainsFromColourTemperature(*colourTemperature); - awb.manual.gains.r() = gains.r(); - awb.manual.gains.b() = gains.b(); awb.manual.temperatureK = *colourTemperature; - update = true; + const auto &gains = awbAlgo_->gainsFromColourTemperature(*colourTemperature); + if (gains) { + awb.manual.gains.r() = gains->r(); + awb.manual.gains.b() = gains->b(); + update = true; + } } if (update) diff --git a/src/ipa/rkisp1/algorithms/awb.h b/src/ipa/rkisp1/algorithms/awb.h index 7e6c3862e699..02651cc732bf 100644 --- a/src/ipa/rkisp1/algorithms/awb.h +++ b/src/ipa/rkisp1/algorithms/awb.h @@ -7,8 +7,6 @@ #pragma once -#include - #include "libcamera/internal/vector.h" #include "libipa/awb.h" From patchwork Thu Apr 3 15:49:19 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23130 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 03D0FC327D for ; Thu, 3 Apr 2025 15:50:20 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BC4A968AAA; Thu, 3 Apr 2025 17:50:19 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="U1OiH8SA"; 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 8086D68AAA for ; Thu, 3 Apr 2025 17:50:17 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:6d9d:9854:3fc1:4bb2]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id B669216B0; Thu, 3 Apr 2025 17:48:23 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1743695303; bh=ZzN7OkSIfM672x0jenTKoBWqnOwUQxCwrET3J40FR88=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=U1OiH8SA1WTxVjVOPAUh8rZKBWMfta/UuyVD/PpK7tte+8D1wrrFL8Y9TiWhz0Rcm X+C2MQBcpA4FRP+9sTmY3G5zb9mxVzUD/GvIq2u9giQGi2KL8q78yNmFfNq7n/P8A4 WFjYk5ws4hKUZoQoQ8rP2g8VBW0TfQrV6R8R+KQo= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Kieran Bingham Subject: [PATCH v3 14/16] ipa: rkisp1: Damp color temperature regulation Date: Thu, 3 Apr 2025 17:49:19 +0200 Message-ID: <20250403154925.382973-15-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250403154925.382973-1-stefan.klug@ideasonboard.com> References: <20250403154925.382973-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" Damp the regulation of the color temperature with the same factor as the gains. Not damping the color temperature leads to visible flicker, as the CCM changes too much. Signed-off-by: Stefan Klug Reviewed-by: Kieran Bingham --- Changes in v2: - Collected tags Changes in v3: - Dropped incorrect call to estimateCCT() --- src/ipa/rkisp1/algorithms/awb.cpp | 5 +++-- src/ipa/rkisp1/algorithms/ccm.cpp | 4 ---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp index f5415a0bb99d..47b29725af51 100644 --- a/src/ipa/rkisp1/algorithms/awb.cpp +++ b/src/ipa/rkisp1/algorithms/awb.cpp @@ -317,8 +317,6 @@ void Awb::process(IPAContext &context, RkISP1AwbStats awbStats{ rgbMeans }; AwbResult awbResult = awbAlgo_->calculateAwb(awbStats, frameContext.lux.lux); - activeState.awb.automatic.temperatureK = awbResult.colourTemperature; - /* * Clamp the gain values to the hardware, which expresses gains as Q2.8 * unsigned integer values. Set the minimum just above zero to avoid @@ -329,9 +327,12 @@ void Awb::process(IPAContext &context, /* Filter the values to avoid oscillations. */ double speed = 0.2; + double ct = awbResult.colourTemperature; + ct = ct * speed + activeState.awb.automatic.temperatureK * (1 - speed); awbResult.gains = awbResult.gains * speed + activeState.awb.automatic.gains * (1 - speed); + activeState.awb.automatic.temperatureK = static_cast(ct); activeState.awb.automatic.gains = awbResult.gains; LOG(RkISP1Awb, Debug) diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp index 3a96a5427bc6..de2b6fe775aa 100644 --- a/src/ipa/rkisp1/algorithms/ccm.cpp +++ b/src/ipa/rkisp1/algorithms/ccm.cpp @@ -142,10 +142,6 @@ void Ccm::prepare(IPAContext &context, const uint32_t frame, } uint32_t ct = frameContext.awb.temperatureK; - /* - * \todo The colour temperature will likely be noisy, add filtering to - * avoid updating the CCM matrix all the time. - */ if (frame > 0 && ct == ct_) { frameContext.ccm.ccm = context.activeState.ccm.automatic; return; From patchwork Thu Apr 3 15:49:20 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23131 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 7C950C327D for ; Thu, 3 Apr 2025 15:50:21 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3A2B968AB7; Thu, 3 Apr 2025 17:50:21 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="VecF87Wg"; 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 3F0D2689A2 for ; Thu, 3 Apr 2025 17:50:20 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:6d9d:9854:3fc1:4bb2]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8BEA419DF; Thu, 3 Apr 2025 17:48:26 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1743695306; bh=Sbt6LwHK1YT9TZAYtQnDyRTKlwqAeTCuFXpew9fnwkY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=VecF87WgMzr/Jm0R2j43rOW0MGfIQxVR7ZnYrZjeDMY0S5nZdKwoP0lmvhMzkGrEU 7+VRsK7UjMOFqO7WjyMdHG3se8Et3mN3B/gVngzgtrsM7t4ZWVgVkMRXt7kL6DeE7p EgkK44+yjPBj0IGoq8By9QBXJWkxZtESTS8DD5yM= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Kieran Bingham , Laurent Pinchart Subject: [PATCH v3 15/16] ipa: rkisp1: awb: Take the CCM into account for the AWB gains calculation Date: Thu, 3 Apr 2025 17:49:20 +0200 Message-ID: <20250403154925.382973-16-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250403154925.382973-1-stefan.klug@ideasonboard.com> References: <20250403154925.382973-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 AWB measurements are taken after the CCM. This can be seen by enabling debug logging on AWB, disabling AWB (stats will still be processed) and manually chaning the CCM. This means that the estimated colour temperature and the corresponding CCM also lead to changed rgbMeans which in turn leads to oscillations. Fix that by applying the inverse transform on the rgbMeans. Signed-off-by: Stefan Klug Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- Changes in v2: - Improved commit message - Added comment in the code Changes in v3: - Fixed typo in comment --- src/ipa/rkisp1/algorithms/awb.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp index 47b29725af51..03449e87e1a0 100644 --- a/src/ipa/rkisp1/algorithms/awb.cpp +++ b/src/ipa/rkisp1/algorithms/awb.cpp @@ -397,6 +397,12 @@ RGB Awb::calculateRgbMeans(const IPAFrameContext &frameContext, const rk rgbMeans = rgbMeans.max(0.0); } + /* + * The ISP computes the AWB means after applying the CCM. Apply the + * inverse as we want to get the raw means before the colour gains. + */ + rgbMeans = frameContext.ccm.ccm.inverse() * rgbMeans; + /* * The ISP computes the AWB means after applying the colour gains, * divide by the gains that were used to get the raw means from the From patchwork Thu Apr 3 15:49:21 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Klug X-Patchwork-Id: 23132 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 092F0C327D for ; Thu, 3 Apr 2025 15:50:25 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id B8D5368AB7; Thu, 3 Apr 2025 17:50:24 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="X7vsoyFA"; 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 CAAF268AB5 for ; Thu, 3 Apr 2025 17:50:22 +0200 (CEST) Received: from ideasonboard.com (unknown [IPv6:2a00:6020:448c:6c00:6d9d:9854:3fc1:4bb2]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 22A7F1624; Thu, 3 Apr 2025 17:48:29 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1743695309; bh=kQ2fDfNpxKxYjPice0bjRindnmwA0nJ8zzwa8VIAV4w=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=X7vsoyFANVJHc1BXsOT+ZA/txtmTuM9U2U9rynLwP9mlOMZxv1YOOLclhxhU3P3Jj 0N4PQHrV9y4oW47gETH7Mbl9zk6eL6Ry0EW6ANYFPfL0HqwHbijL4OHUg8n7ngwjne Vm/WN2xa1gh7Lnpvtujpuh2Z3xD/2Tf7eRTnJZEY= From: Stefan Klug To: libcamera-devel@lists.libcamera.org Cc: Stefan Klug , Kieran Bingham Subject: [PATCH v3 16/16] ipa: rkisp1: awb: Avoid division by zero Date: Thu, 3 Apr 2025 17:49:21 +0200 Message-ID: <20250403154925.382973-17-stefan.klug@ideasonboard.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250403154925.382973-1-stefan.klug@ideasonboard.com> References: <20250403154925.382973-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" As the gains can also be specified manually, the regulation can run into numeric instabilities by dividing by near zero. Mitigate that by applying a small minium value. Signed-off-by: Stefan Klug Reviewed-by: Kieran Bingham --- Changes in v2: - Collected tag --- src/ipa/rkisp1/algorithms/awb.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp index 03449e87e1a0..84aa1d29419d 100644 --- a/src/ipa/rkisp1/algorithms/awb.cpp +++ b/src/ipa/rkisp1/algorithms/awb.cpp @@ -406,9 +406,9 @@ RGB Awb::calculateRgbMeans(const IPAFrameContext &frameContext, const rk /* * The ISP computes the AWB means after applying the colour gains, * divide by the gains that were used to get the raw means from the - * sensor. + * sensor. Apply a minimum value to avoid divisions by near-zero. */ - rgbMeans /= frameContext.awb.gains; + rgbMeans /= frameContext.awb.gains.max(0.01); return rgbMeans; }