[v2] libcamera: matrix: Add read-only accessor to internal data
diff mbox series

Message ID 20250202232739.31548-1-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • [v2] libcamera: matrix: Add read-only accessor to internal data
Related show

Commit Message

Laurent Pinchart Feb. 2, 2025, 11:27 p.m. UTC
Add a data() function to the Matrix class to access the internal data.
This is useful for code that needs to use the matrix contents as a
linear array, as shown by the RkISP1::Ccm::process() function that needs
to copy the matrix data to a local variable. Simplify that function by
using the new accessor.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
Changes since v1:

- Expand function documentation
---
 include/libcamera/internal/matrix.h |  2 ++
 src/ipa/rkisp1/algorithms/ccm.cpp   |  7 +------
 src/libcamera/matrix.cpp            | 11 +++++++++++
 3 files changed, 14 insertions(+), 6 deletions(-)


base-commit: 9bc8b6a573a63b8c9f5588cdea6da5ae71abd138
--
Regards,

Laurent Pinchart

Comments

Milan Zamazal Feb. 3, 2025, 10:20 a.m. UTC | #1
Hi Laurent,

thank you for the update.

Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:

> Add a data() function to the Matrix class to access the internal data.
> This is useful for code that needs to use the matrix contents as a
> linear array, as shown by the RkISP1::Ccm::process() function that needs
> to copy the matrix data to a local variable. Simplify that function by
> using the new accessor.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Milan Zamazal <mzamazal@redhat.com>

> ---
> Changes since v1:
>
> - Expand function documentation
> ---
>  include/libcamera/internal/matrix.h |  2 ++
>  src/ipa/rkisp1/algorithms/ccm.cpp   |  7 +------
>  src/libcamera/matrix.cpp            | 11 +++++++++++
>  3 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h
> index 7a71028c473a..a055e6926c94 100644
> --- a/include/libcamera/internal/matrix.h
> +++ b/include/libcamera/internal/matrix.h
> @@ -66,6 +66,8 @@ public:
>  		return out.str();
>  	}
>
> +	Span<const T, Rows * Cols> data() const { return data_; }
> +
>  	Span<const T, Cols> operator[](size_t i) const
>  	{
>  		return Span<const T, Cols>{ &data_.data()[i * Cols], Cols };
> diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp
> index e2b5cf4d313e..eb8ca39e56a8 100644
> --- a/src/ipa/rkisp1/algorithms/ccm.cpp
> +++ b/src/ipa/rkisp1/algorithms/ccm.cpp
> @@ -120,12 +120,7 @@ void Ccm::process([[maybe_unused]] IPAContext &context,
>  		  [[maybe_unused]] const rkisp1_stat_buffer *stats,
>  		  ControlList &metadata)
>  {
> -	float m[9];
> -	for (unsigned int i = 0; i < 3; i++) {
> -		for (unsigned int j = 0; j < 3; j++)
> -			m[i * 3 + j] = frameContext.ccm.ccm[i][j];
> -	}
> -	metadata.set(controls::ColourCorrectionMatrix, m);
> +	metadata.set(controls::ColourCorrectionMatrix, frameContext.ccm.ccm.data());
>  }
>
>  REGISTER_IPA_ALGORITHM(Ccm, "Ccm")
> diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp
> index 4d95a19bfbb9..e7e027225666 100644
> --- a/src/libcamera/matrix.cpp
> +++ b/src/libcamera/matrix.cpp
> @@ -52,6 +52,17 @@ LOG_DEFINE_CATEGORY(Matrix)
>   * \return A string describing the matrix
>   */
>
> +/**
> + * \fn Matrix::data()
> + * \brief Access the matrix data as a linear array
> + *
> + * Access the contents of the matrix as a one-dimensional linear array of
> + * values in row-major order. The size of the array is equal to the product of
> + * the number of rows and columns of the matrix (Rows x Cols).
> + *
> + * \return A span referencing the matrix data as a linear array
> + */
> +
>  /**
>   * \fn Span<const T, Cols> Matrix::operator[](size_t i) const
>   * \brief Index to a row in the matrix
>
> base-commit: 9bc8b6a573a63b8c9f5588cdea6da5ae71abd138
> --
> Regards,
>
> Laurent Pinchart

Patch
diff mbox series

diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h
index 7a71028c473a..a055e6926c94 100644
--- a/include/libcamera/internal/matrix.h
+++ b/include/libcamera/internal/matrix.h
@@ -66,6 +66,8 @@  public:
 		return out.str();
 	}

+	Span<const T, Rows * Cols> data() const { return data_; }
+
 	Span<const T, Cols> operator[](size_t i) const
 	{
 		return Span<const T, Cols>{ &data_.data()[i * Cols], Cols };
diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp
index e2b5cf4d313e..eb8ca39e56a8 100644
--- a/src/ipa/rkisp1/algorithms/ccm.cpp
+++ b/src/ipa/rkisp1/algorithms/ccm.cpp
@@ -120,12 +120,7 @@  void Ccm::process([[maybe_unused]] IPAContext &context,
 		  [[maybe_unused]] const rkisp1_stat_buffer *stats,
 		  ControlList &metadata)
 {
-	float m[9];
-	for (unsigned int i = 0; i < 3; i++) {
-		for (unsigned int j = 0; j < 3; j++)
-			m[i * 3 + j] = frameContext.ccm.ccm[i][j];
-	}
-	metadata.set(controls::ColourCorrectionMatrix, m);
+	metadata.set(controls::ColourCorrectionMatrix, frameContext.ccm.ccm.data());
 }

 REGISTER_IPA_ALGORITHM(Ccm, "Ccm")
diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp
index 4d95a19bfbb9..e7e027225666 100644
--- a/src/libcamera/matrix.cpp
+++ b/src/libcamera/matrix.cpp
@@ -52,6 +52,17 @@  LOG_DEFINE_CATEGORY(Matrix)
  * \return A string describing the matrix
  */

+/**
+ * \fn Matrix::data()
+ * \brief Access the matrix data as a linear array
+ *
+ * Access the contents of the matrix as a one-dimensional linear array of
+ * values in row-major order. The size of the array is equal to the product of
+ * the number of rows and columns of the matrix (Rows x Cols).
+ *
+ * \return A span referencing the matrix data as a linear array
+ */
+
 /**
  * \fn Span<const T, Cols> Matrix::operator[](size_t i) const
  * \brief Index to a row in the matrix