[PATCH v9 1/3] ipa: libipa: Add Matrix class
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Jun 16 19:10:25 CEST 2024
Hi Paul,
Thank you for the patch.
On Fri, Jun 14, 2024 at 08:37:55PM +0900, Paul Elder wrote:
> Add a class to represent a Matrix object and operations for adding
> matrices, multipling a matrix by a scalar, and multiplying two matrices.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Stefan Klug <stefan.klug at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> ---
> No change in v9
>
> Changes in v8:
> - remove readYaml and replace with yaml get
> - which makes this patch depend on "ipa: libipa: Vector and Pwl
> improvements"
> - make default constructor make a zero matrix, and add an identity()
> static function
> - add operator*=
> - add operator* for reverse order of scalar-matrix product
> - s/\<R\>/Rows/g, s/\<C\>/Cols/g
>
> Changes in v7:
> - fix copyright and license
>
> Changes in v6:
> - fix doxygen
>
> Changes in v5:
> - add documentation
>
> Changes in v4:
> - remove stray semicolons
> - add operator<<
> - clean up/optimize constructor
> - replace get() and set() with operator[] (and a second [] can be used
> as operator[] returns a Span)
>
> Changes in v3:
> - fix template parameters of operator* to allow different types for the
> scalar multiplier and the matrix's number type
> - clear data in constructors
> - fix assert in constructor
>
> Changes v2:
> - make rows and columns into template arguments
> - initialize to identity matrix on construction
> - add getter and setter
> - change from struct to class
> - fix matrix multiplication
> - clean up unused includes
> - avoid dereferencing an absent std::optional
> ---
> src/ipa/libipa/matrix.cpp | 149 +++++++++++++++++++++++++++
> src/ipa/libipa/matrix.h | 204 +++++++++++++++++++++++++++++++++++++
> src/ipa/libipa/meson.build | 2 +
> 3 files changed, 355 insertions(+)
> create mode 100644 src/ipa/libipa/matrix.cpp
> create mode 100644 src/ipa/libipa/matrix.h
>
> diff --git a/src/ipa/libipa/matrix.cpp b/src/ipa/libipa/matrix.cpp
> new file mode 100644
> index 000000000000..4f49ac4d8b6c
> --- /dev/null
> +++ b/src/ipa/libipa/matrix.cpp
> @@ -0,0 +1,149 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Paul Elder <paul.elder at ideasonboard.com>
> + *
> + * Matrix and related operations
> + */
> +
> +#include "matrix.h"
> +
> +#include <libcamera/base/log.h>
> +
> +/**
> + * \file matrix.h
> + * \brief Matrix class
> + */
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(Matrix)
> +
> +namespace ipa {
> +
> +/**
> + * \class Matrix
> + * \brief Matrix class
> + * \tparam T Type of numerical values to be stored in the matrix
> + * \tparam Rows Number of rows in the matrix
> + * \tparam Cols Number of columns in the matrix
> + */
> +
> +/**
> + * \fn Matrix::Matrix()
> + * \brief Construct a zero matrix
> + */
> +
> +/**
> + * \fn Matrix::Matrix(const std::vector<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 (RxC).
s/RxC/Rows x Cols/
> + */
> +
> +/**
> + * \fn Matrix::identity()
> + * \brief Construct an identity matrix
> + */
> +
> +/**
> + * \fn Matrix::toString
s/$/()/
> + * \brief Assemble and return a string describing the matrix
> + * \return A string describing the matrix
> + */
> +
> +/**
> + * \fn Span<const T, Cols> Matrix::operator[](size_t i) const
> + * \brief Index to a row in the matrix
> + * \param[in] i Index of row to retrieve
> + *
> + * This operator[] returns a Span, which can then be indexed into again with
> + * another operator[], allowing a convenient m[i][j] to access elements of the
> + * matrix. Note that the lifetime of the Span returned by this first-level
> + * operator[] is bound to that of the Matrix itself, so it is not recommended
> + * to save the Span that is the result of this operator[].
> + *
> + * \return Row \a i from the matrix, as a Span
> + */
> +
> +/**
> + * \fn Matrix::operator[](size_t i)
> + * \copydoc Matrix::operator[](size_t i) const
> + */
> +
> +/**
> + * \fn Matrix<T, Rows, Cols> &Matrix::operator*=(U d)
> + * \brief Scalar product
* \brief Multiply the matrix by a scalar in-place
> + * \tparam U Type of the numerical scalar value
> + * \param d Scalar
> + * \return Product of this matrix and scalar \a d
> + */
> +
> +/**
> + * \fn Matrix::Matrix<U, Rows, Cols> operator*(T d, const Matrix<U, Rows, Cols> &m)
> + * \brief Scalar product
* \brief Multiply the matrix by a scalar
> + * \tparam T Type of the numerical scalar value
> + * \tparam U Type of numerical values in the matrix
> + * \tparam Rows Number of rows in the matrix
> + * \tparam Cols Number of columns in the matrix
> + * \param d Scalar
> + * \param m Matrix
> + * \return Product of scalar \a d and matrix \a m
> + */
> +
> +/**
> + * \fn Matrix::Matrix<U, Rows, Cols> operator*(const Matrix<U, Rows, Cols> &m, T d)
> + * \copydoc operator*(T d, const Matrix<U, Rows, Cols> &m)
> + */
> +
> +/**
> + * \fn Matrix<T, R1, C2> operator*(const Matrix<T, R1, C1> &m1, const Matrix<T, R2, C2> &m2)
> + * \brief Matrix multiplication
> + * \tparam T Type of numerical values in the matrices
> + * \tparam R1 Number of rows in the first matrix
> + * \tparam C1 Number of columns in the first matrix
> + * \tparam R2 Number of rows in the second matrix
> + * \tparam C2 Number of columns in the second matrix
> + * \param m1 Multiplicand matrix
> + * \param m2 Multiplier matrix
> + * \return Matrix product of matrices \a m1 and \a m2
> + */
> +
> +/**
> + * \fn Matrix<T, Rows, Cols> operator+(const Matrix<T, Rows, Cols> &m1, const Matrix<T, Rows, Cols> &m2)
> + * \brief Matrix addition
> + * \tparam T Type of numerical values in the matrices
> + * \tparam Rows Number of rows in the matrices
> + * \tparam Cols Number of columns in the matrices
> + * \param m1 Summand matrix
> + * \param m2 Summand matrix
> + * \return Matrix sum of matrices \a m1 and \a m2
> + */
> +
> +} /* namespace ipa */
> +
> +#ifndef __DOXYGEN__
> +/*
> + * 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 (RxC). The
Rows x Cols here too.
> + * values shall be stored in row-major order.
> + */
> +bool matrixValidateYaml(const YamlObject &obj, unsigned int size)
> +{
> + if (!obj.isList())
> + return false;
> +
> + if (obj.size() != size) {
> + LOG(Matrix, Error)
> + << "Wrong number of values in matrix: expected "
> + << size << ", got " << obj.size();
> + return false;
> + }
> +
> + return true;
> +}
> +#endif /* __DOXYGEN__ */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/libipa/matrix.h b/src/ipa/libipa/matrix.h
> new file mode 100644
> index 000000000000..3a6b9b303acc
> --- /dev/null
> +++ b/src/ipa/libipa/matrix.h
> @@ -0,0 +1,204 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Paul Elder <paul.elder at ideasonboard.com>
> + *
> + * Matrix and related operations
> + */
> +#pragma once
> +
> +#include <algorithm>
> +#include <cmath>
> +#include <sstream>
> +#include <vector>
> +
> +#include <libcamera/base/log.h>
> +#include <libcamera/base/span.h>
> +
> +#include "libcamera/internal/yaml_parser.h"
> +
> +namespace libcamera {
> +
> +LOG_DECLARE_CATEGORY(Matrix)
> +
> +namespace ipa {
> +
> +#ifndef __DOXYGEN__
> +template<typename T, unsigned int Rows, unsigned int Cols,
> + std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>
> +#else
> +template<typename T, unsigned int Rows, unsigned int Cols>
> +#endif /* __DOXYGEN__ */
> +class Matrix
> +{
> +public:
> + Matrix()
> + {
> + data_.fill(static_cast<T>(0));
> + }
> +
> + Matrix(const std::vector<T> &data)
> + {
> + std::copy(data.begin(), data.end(), data_.begin());
> + }
> +
> + static Matrix identity()
> + {
> + Matrix ret;
> + for (size_t i = 0; i < std::min(Rows, Cols); i++)
> + ret[i][i] = static_cast<T>(1);
> + return ret;
> + }
> +
> + ~Matrix() = default;
> +
> + const std::string toString() const
> + {
> + std::stringstream out;
> +
> + out << "Matrix { ";
> + for (unsigned int i = 0; i < Rows; i++) {
> + out << "[ ";
> + for (unsigned int j = 0; j < Cols; j++) {
> + out << (*this)[i][j];
> + out << ((j + 1 < Cols) ? ", " : " ");
> + }
> + out << ((i + 1 < Rows) ? "], " : "]");
> + }
> + out << " }";
> +
> + return out.str();
> + }
> +
> + Span<const T, Cols> operator[](size_t i) const
> + {
> + return Span<const T, Cols>{ &data_.data()[i * Cols], Cols };
> + }
> +
> + Span<T, Cols> operator[](size_t i)
> + {
> + return Span<T, Cols>{ &data_.data()[i * Cols], Cols };
> + }
> +
> +#ifndef __DOXYGEN__
> + template<typename U, std::enable_if_t<std::is_arithmetic_v<U>>>
> +#else
> + template<typename U>
> +#endif /* __DOXYGEN__ */
> + Matrix<T, Rows, Cols> &operator*=(U d)
> + {
> + for (unsigned int i = 0; i < Rows * Cols; i++)
> + data_[i] *= d;
> + return *this;
> + }
> +
> +private:
> + std::array<T, Rows * Cols> data_;
> +};
> +
> +#ifndef __DOXYGEN__
> +template<typename T, typename U, unsigned int Rows, unsigned int Cols,
> + std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>
> +#else
> +template<typename T, typename U, unsigned int Rows, unsigned int Cols>
> +#endif /* __DOXYGEN__ */
> +Matrix<U, Rows, Cols> operator*(T d, const Matrix<U, Rows, Cols> &m)
> +{
> + Matrix<U, Rows, Cols> result;
> +
> + for (unsigned int i = 0; i < Rows; i++)
> + for (unsigned int j = 0; j < Cols; j++)
> + result[i][j] = d * m[i][j];
Curly braces for the outer loop.
> +
> + return result;
> +}
> +
> +#ifndef __DOXYGEN__
> +template<typename T, typename U, unsigned int Rows, unsigned int Cols,
> + std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>
> +#else
> +template<typename T, typename U, unsigned int Rows, unsigned int Cols>
> +#endif /* __DOXYGEN__ */
> +Matrix<U, Rows, Cols> operator*(const Matrix<U, Rows, Cols> &m, T d)
> +{
> + return d * m;
> +}
> +
> +#ifndef __DOXYGEN__
> +template<typename T,
> + unsigned int R1, unsigned int C1,
> + unsigned int R2, unsigned int C2,
> + std::enable_if_t<C1 == R2> * = nullptr>
> +#else
> +template<typename T, unsigned int R1, unsigned int C1, unsigned int R2, unsigned in C2>
> +#endif /* __DOXYGEN__ */
> +Matrix<T, R1, C2> operator*(const Matrix<T, R1, C1> &m1, const Matrix<T, R2, C2> &m2)
> +{
> + Matrix<T, R1, C2> result;
> +
> + for (unsigned int i = 0; i < R1; i++) {
> + for (unsigned int j = 0; j < C2; j++) {
> + T sum = 0;
> +
> + for (unsigned int k = 0; k < C1; k++)
> + sum += m1[i][k] * m2[k][j];
> +
> + result[i][j] = sum;
> + }
> + }
> +
> + return result;
> +}
> +
> +#ifndef __DOXYGEN__
> +template<typename T, unsigned int Rows, unsigned int Cols>
> +#endif /* __DOXYGEN__ */
No need to hide the template from Doxygen.
> +Matrix<T, Rows, Cols> operator+(const Matrix<T, Rows, Cols> &m1, const Matrix<T, Rows, Cols> &m2)
> +{
> + Matrix<T, Rows, Cols> result;
> +
> + for (unsigned int i = 0; i < Rows; i++)
> + for (unsigned int j = 0; j < Cols; j++)
> + result[i][j] = m1[i][j] + m2[i][j];
Curly braces for the outer loop.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> +
> + return result;
> +}
> +
> +#ifndef __DOXYGEN__
> +bool matrixValidateYaml(const YamlObject &obj, unsigned int size);
> +#endif /* __DOXYGEN__ */
> +
> +} /* namespace ipa */
> +
> +#ifndef __DOXYGEN__
> +template<typename T, unsigned int Rows, unsigned int Cols>
> +std::ostream &operator<<(std::ostream &out, const ipa::Matrix<T, Rows, Cols> &m)
> +{
> + out << m.toString();
> + return out;
> +}
> +
> +template<typename T, unsigned int Rows, unsigned int Cols>
> +struct YamlObject::Getter<ipa::Matrix<T, Rows, Cols>> {
> + std::optional<ipa::Matrix<T, Rows, Cols>> get(const YamlObject &obj) const
> + {
> + if (!ipa::matrixValidateYaml(obj, Rows * Cols))
> + return std::nullopt;
> +
> + ipa::Matrix<T, Rows, Cols> matrix;
> + T *data = &matrix[0][0];
> +
> + unsigned int i = 0;
> + for (const YamlObject &entry : obj.asList()) {
> + const auto value = entry.get<T>();
> + if (!value)
> + return std::nullopt;
> +
> + data[i++] = *value;
> + }
> +
> + return matrix;
> + }
> +};
> +#endif /* __DOXYGEN__ */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build
> index 496084233de7..1952dccf6236 100644
> --- a/src/ipa/libipa/meson.build
> +++ b/src/ipa/libipa/meson.build
> @@ -7,6 +7,7 @@ libipa_headers = files([
> 'exposure_mode_helper.h',
> 'fc_queue.h',
> 'histogram.h',
> + 'matrix.h',
> 'module.h',
> 'pwl.h',
> 'vector.h',
> @@ -19,6 +20,7 @@ libipa_sources = files([
> 'exposure_mode_helper.cpp',
> 'fc_queue.cpp',
> 'histogram.cpp',
> + 'matrix.cpp',
> 'module.cpp',
> 'pwl.cpp',
> 'vector.cpp',
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list