[v6,1/3] ipa: libipa: Add Matrix class

20234 diff mbox series

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@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>

---
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  | 124 ++++++++++++++++++++++++++
 src/ipa/libipa/matrix.h    | 173 +++++++++++++++++++++++++++++++++++++
 src/ipa/libipa/meson.build |   2 +
 3 files changed, 299 insertions(+)
 create mode 100644 src/ipa/libipa/matrix.cpp
 create mode 100644 src/ipa/libipa/matrix.h

Quoting Paul Elder (2024-06-07 09:09:04)
> 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@ideasonboard.com>
> Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
> 

Like the Vector class, my only query in here is that we're adding
internal support infrastructure without unit tests.

It seems like we're missing a unit test section for libipa - that
preceeds this patch (and Vector) so I think we can let these patches
continue without - but I think we really should add in unit tests to
common code classes if we expect them to be re-used!

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
> 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  | 124 ++++++++++++++++++++++++++
>  src/ipa/libipa/matrix.h    | 173 +++++++++++++++++++++++++++++++++++++
>  src/ipa/libipa/meson.build |   2 +
>  3 files changed, 299 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 000000000..350dbeb05
> --- /dev/null
> +++ b/src/ipa/libipa/matrix.cpp
> @@ -0,0 +1,124 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2019, Raspberry Pi Ltd
> + * Copyright (C) 2024, Paul Elder <paul.elder@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 R Number of rows in the matrix
> + * \tparam C Number of columns in the matrix
> + */
> +
> +/**
> + * \fn Matrix::Matrix()
> + * \brief Construct an identity matrix
> + */
> +
> +/**
> + * \fn Matrix::Matrix(const std::vector<T> &data)
> + * \brief Construct matrix from supplied data
> + * \param 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).
> + */
> +
> +/**
> + * \fn Matrix::readYaml
> + * \brief Populate the matrix with yaml data
> + * \param yaml Yaml data to populate the matrix with
> + *
> + * Any existing data in the matrix will be overwritten. The size of the data
> + * read from \a yaml must be equal to the product of the number of rows and
> + * columns of the matrix (RxC).
> + *
> + * The yaml data is expected to be a list with elements of type T.
> + *
> + * \return 0 on success, negative error code otherwise
> + */
> +
> +/**
> + * \fn Matrix::toString
> + * \brief Assemble and return a string describing the matrix
> + * \return A string describing the matrix
> + */
> +
> +/**
> + * \fn Span<const T, C> Matrix::operator[](size_t i) const
> + * \brief Index to a row in the matrix
> + * \param 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::Matrix<U, R, C> operator*(T d, const Matrix<U, R, C> &m)
> + * \brief Scalar product
> + * \tparam T Type of the numerical scalar value
> + * \tparam U Type of numerical values in the matrix
> + * \tparam R Number of rows in the matrix
> + * \tparam C Number of columns in the matrix
> + * \param d Scalar
> + * \param m Matrix
> + * \return Product of scalar \a d and matrix \a 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, R, C> operator+(const Matrix<T, R, C> &m1, const Matrix<T, R, C> &m2)
> + * \brief Matrix addition
> + * \tparam T Type of numerical values in the matrices
> + * \tparam R Number of rows in the matrices
> + * \tparam C 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 */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/libipa/matrix.h b/src/ipa/libipa/matrix.h
> new file mode 100644
> index 000000000..1f6447a3b
> --- /dev/null
> +++ b/src/ipa/libipa/matrix.h
> @@ -0,0 +1,173 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2019, Raspberry Pi Ltd
> + * Copyright (C) 2024, Paul Elder <paul.elder@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 R, unsigned int C,
> +        std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>
> +#else
> +template<typename T, unsigned int R, unsigned int C>
> +#endif /* __DOXYGEN__ */
> +class Matrix
> +{
> +public:
> +       Matrix()
> +               : data_(R * C, static_cast<T>(false))
> +       {
> +               for (size_t i = 0; i < std::min(R, C); i++)
> +                       (*this)[i][i] = static_cast<T>(true);
> +       }
> +
> +       Matrix(const std::vector<T> &data)
> +       {
> +               ASSERT(data.size() == R * C);
> +
> +               data_.clear();
> +               for (const T &x : data)
> +                       data_.push_back(x);
> +       }
> +
> +       ~Matrix() = default;
> +
> +       int readYaml(const libcamera::YamlObject &yaml)
> +       {
> +               if (yaml.size() != R * C) {
> +                       LOG(Matrix, Error)
> +                               << "Wrong number of values in matrix: expected "
> +                               << R * C << ", got " << yaml.size();
> +                       return -EINVAL;
> +               }
> +
> +               unsigned int i = 0;
> +               for (const auto &x : yaml.asList()) {
> +                       auto value = x.get<T>();
> +                       if (!value) {
> +                               LOG(Matrix, Error) << "Failed to read matrix value";
> +                               return -EINVAL;
> +                       }
> +
> +                       data_[i++] = *value;
> +               }
> +
> +               return 0;
> +       }
> +
> +       const std::string toString() const
> +       {
> +               std::stringstream out;
> +
> +               out << "Matrix { ";
> +               for (unsigned int i = 0; i < R; i++) {
> +                       out << "[ ";
> +                       for (unsigned int j = 0; j < C; j++) {
> +                               out << (*this)[i][j];
> +                               out << ((j + 1 < C) ? ", " : " ");
> +                       }
> +                       out << ((i + 1 < R) ? "], " : "]");
> +               }
> +               out << " }";
> +
> +               return out.str();
> +       }
> +
> +       Span<const T, C> operator[](size_t i) const
> +       {
> +               return Span<const T, C>{ &data_.data()[i * C], C };
> +       }
> +
> +       Span<T, C> operator[](size_t i)
> +       {
> +               return Span<T, C>{ &data_.data()[i * C], C };
> +       }
> +
> +private:
> +       std::vector<T> data_;
> +};
> +
> +#ifndef __DOXYGEN__
> +template<typename T, typename U, unsigned int R, unsigned int C,
> +        std::enable_if_t<std::is_arithmetic_v<T> && std::is_arithmetic_v<U>> * = nullptr>
> +#endif /* __DOXYGEN__ */
> +Matrix<U, R, C> operator*(T d, const Matrix<U, R, C> &m)
> +{
> +       Matrix<U, R, C> result;
> +
> +       for (unsigned int i = 0; i < R; i++)
> +               for (unsigned int j = 0; j < C; j++)
> +                       result[i][j] = d * m[i][j];
> +
> +       return result;
> +}
> +
> +#ifndef __DOXYGEN__
> +template<typename T,
> +        unsigned int R1, unsigned int C1,
> +        unsigned int R2, unsigned int C2,
> +        std::enable_if_t<std::is_arithmetic_v<T> && C1 == R2> * = nullptr>
> +#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 R, unsigned int C,
> +        std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>
> +#endif /* __DOXYGEN__ */
> +Matrix<T, R, C> operator+(const Matrix<T, R, C> &m1, const Matrix<T, R, C> &m2)
> +{
> +       Matrix<T, R, C> result;
> +
> +       for (unsigned int i = 0; i < R; i++)
> +               for (unsigned int j = 0; j < C; j++)
> +                       result[i][j] = m1[i][j] + m2[i][j];
> +
> +       return result;
> +}
> +
> +} /* namespace ipa */
> +
> +#ifndef __DOXYGEN__
> +template<typename T, unsigned int R, unsigned int C>
> +std::ostream &operator<<(std::ostream &out, const ipa::Matrix<T, R, C> &m)
> +{
> +       out << m.toString();
> +       return out;
> +}
> +#endif /* __DOXYGEN__ */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build
> index 8ec9c7847..2547a5b83 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',
>  ])
> @@ -18,6 +19,7 @@ libipa_sources = files([
>      'exposure_mode_helper.cpp',
>      'fc_queue.cpp',
>      'histogram.cpp',
> +    'matrix.cpp',
>      'module.cpp',
>      'pwl.cpp',
>  ])
> -- 
> 2.39.2
>