[PATCH 3/4] libcamera: matrix: Use Span instead of vector for construction

Laurent Pinchart laurent.pinchart at ideasonboard.com
Mon Nov 18 17:12:47 CET 2024


Hi Stefan,

Thank you for the patch.

On Mon, Nov 18, 2024 at 04:05:06PM +0100, Stefan Klug wrote:
> Use a Span for construction to be more flexible. Additionally add an
> assert for the correct size of the span.
> 
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> ---
>  include/libcamera/internal/matrix.h |  3 ++-
>  src/libcamera/matrix.cpp            | 10 ++++++++++
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libcamera/internal/matrix.h b/include/libcamera/internal/matrix.h
> index b82d33f98658..28ea68a55af9 100644
> --- a/include/libcamera/internal/matrix.h
> +++ b/include/libcamera/internal/matrix.h
> @@ -39,8 +39,9 @@ public:
>  		std::copy(data.begin(), data.end(), data_.begin());
>  	}
>  
> -	Matrix(const std::vector<T> &data)
> +	Matrix(const Span<T> &data)

A span is certainly better than a vector.

>  	{
> +		ASSERT(data.size() == Rows * Cols);

It would be nice to drop this constructor in favour of an std::array
constructor, as indicated in my review of 2/4. I haven't checked if that
would be possible.

>  		std::copy(data.begin(), data.end(), data_.begin());
>  	}
>  
> diff --git a/src/libcamera/matrix.cpp b/src/libcamera/matrix.cpp
> index c3ed54895b22..bb9fd3003996 100644
> --- a/src/libcamera/matrix.cpp
> +++ b/src/libcamera/matrix.cpp
> @@ -31,6 +31,16 @@ LOG_DEFINE_CATEGORY(Matrix)
>   * \brief Construct a zero matrix
>   */
>  
> +/**
> + * \fn Matrix::Matrix(std::initializer_list<T> data)
> + * \brief Construct a matrix from supplied data
> + * \param[in] data Data from which to construct a matrix
> + *
> + * \a data is a one-dimensional vector 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).
> + */

This is unrelated to the commit message.

> +
>  /**
>   * \fn Matrix::Matrix(const Span<T> &data)

And this was added in 2/4. I think something went wrong at some point
during a split & rebase.

>   * \brief Construct a matrix from supplied data

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list