[PATCH] libcamera: matrix: Add read-only accessor to internal data

[Milan Zamazal]

Hi Laurent,

thank you for the patch.

Laurent Pinchart <laurent.pinchart at 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 shows 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 at ideasonboard.com>
> ---
>  include/libcamera/internal/matrix.h | 2 ++
>  src/ipa/rkisp1/algorithms/ccm.cpp   | 7 +------
>  src/libcamera/matrix.cpp            | 6 ++++++
>  3 files changed, 9 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..d9d34867b0a3 100644
> --- a/src/libcamera/matrix.cpp
> +++ b/src/libcamera/matrix.cpp
> @@ -52,6 +52,12 @@ LOG_DEFINE_CATEGORY(Matrix)
>   * \return A string describing the matrix
>   */
>  
> +/**
> + * \fn Matrix::data()
> + * \brief Access the internal data as a linear array
> + * \return A span referencing the internal data as a linear array

"Internal data" can be anything and if it is supposed to be used in
places like metadata or for any "real" purpose, it should be better
specified.  Something like the documentation of `data' argument in the
constructor?

> + */
> +
>  /**
>   * \fn Span<const T, Cols> Matrix::operator[](size_t i) const
>   * \brief Index to a row in the matrix
>
> base-commit: 8aef7b4dfb12f2f9bf0513625231b85a5b8f5440