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

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

Hi Paul,

thanks for the patch.

On Fri, May 17, 2024 at 05:01:27PM +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@ideasonboard.com>
> Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
> 
> ---
> 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  |  17 ++++
>  src/ipa/libipa/matrix.h    | 161 +++++++++++++++++++++++++++++++++++++
>  src/ipa/libipa/meson.build |   2 +
>  3 files changed, 180 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..c222c3f03
> --- /dev/null
> +++ b/src/ipa/libipa/matrix.cpp
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2019, Raspberry Pi Ltd
> + * Copyright (C) 2024, Paul Elder <paul.elder@ideasonboard.com>
> + *
> + * matrix.cpp - Matrix and related operations
> + */
> +
> +#include "matrix.h"
> +
> +#include <libcamera/base/log.h>
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(Matrix)
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/libipa/matrix.h b/src/ipa/libipa/matrix.h
> new file mode 100644
> index 000000000..1bb794870
> --- /dev/null
> +++ b/src/ipa/libipa/matrix.h
> @@ -0,0 +1,161 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2019, Raspberry Pi Ltd
> + * Copyright (C) 2024, Paul Elder <paul.elder@ideasonboard.com>
> + *
> + * matrix.cpp - 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 {
> +
> +template<typename T, unsigned int R, unsigned int C,
> +	 std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>
> +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

As operator<< is implemented now, the code could go there, and this
function can be dropped, as there is no reason to keep it.

> +	{
> +		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 };
> +	}

I see, this was the last proposal from Barnabás. Actually, I like the
(i,j) version better, as the span now points to data of the matrix, but
its lifetime is not tied to the matrix... but that shouldn't be
a blocker.

Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com> 

Cheers,
Stefan


> +
> +	Span<T, C> operator[](size_t i)
> +	{
> +		return Span<T, C>{ &data_.data()[i * C], C };
> +	}
> +
> +private:
> +	std::vector<T> data_;
> +};
> +
> +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>
> +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;
> +}
> +
> +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>
> +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;
> +}
> +
> +template<typename T, unsigned int R, unsigned int C,
> +	 std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>
> +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 */
> +
> +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;
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build
> index 1b3faf8d5..1e34355fe 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
>

Hi Paul - thanks for the patch

On 17/05/2024 09:01, 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@ideasonboard.com>
> Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
>
> ---
> 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  |  17 ++++
>   src/ipa/libipa/matrix.h    | 161 +++++++++++++++++++++++++++++++++++++
>   src/ipa/libipa/meson.build |   2 +
>   3 files changed, 180 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..c222c3f03
> --- /dev/null
> +++ b/src/ipa/libipa/matrix.cpp
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2019, Raspberry Pi Ltd
> + * Copyright (C) 2024, Paul Elder <paul.elder@ideasonboard.com>
> + *
> + * matrix.cpp - Matrix and related operations
> + */
> +
> +#include "matrix.h"
> +
> +#include <libcamera/base/log.h>
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(Matrix)
> +
> +} /* namespace libcamera */


Do we not need Doxygen comments in here?

> diff --git a/src/ipa/libipa/matrix.h b/src/ipa/libipa/matrix.h
> new file mode 100644
> index 000000000..1bb794870
> --- /dev/null
> +++ b/src/ipa/libipa/matrix.h
> @@ -0,0 +1,161 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (C) 2019, Raspberry Pi Ltd
> + * Copyright (C) 2024, Paul Elder <paul.elder@ideasonboard.com>
> + *
> + * matrix.cpp - 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 {
> +
> +template<typename T, unsigned int R, unsigned int C,
> +	 std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>
> +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_;
> +};
> +
> +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>
> +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;
> +}
> +
> +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>
> +Matrix<T, R1, C2> operator*(const Matrix<T, R1, C1> &m1, const Matrix<T, R2, C2> &m2)
Don't we want to restrict this operation to matrices with matching dimensions? I would expect a 
single set of R and C for both of the matrices.
> +{
> +	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;
> +}
> +
> +template<typename T, unsigned int R, unsigned int C,
> +	 std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>
> +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 */
> +
> +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;
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build
> index 1b3faf8d5..1e34355fe 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'
>   ])

On Mon, May 20, 2024 at 03:08:37PM +0100, Dan Scally wrote:
> Hi Paul - thanks for the patch
> 
> On 17/05/2024 09:01, 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@ideasonboard.com>
> > Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > 
> > ---
> > 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  |  17 ++++
> >   src/ipa/libipa/matrix.h    | 161 +++++++++++++++++++++++++++++++++++++
> >   src/ipa/libipa/meson.build |   2 +
> >   3 files changed, 180 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..c222c3f03
> > --- /dev/null
> > +++ b/src/ipa/libipa/matrix.cpp
> > @@ -0,0 +1,17 @@
> > +/* SPDX-License-Identifier: BSD-2-Clause */
> > +/*
> > + * Copyright (C) 2019, Raspberry Pi Ltd
> > + * Copyright (C) 2024, Paul Elder <paul.elder@ideasonboard.com>
> > + *
> > + * matrix.cpp - Matrix and related operations
> > + */
> > +
> > +#include "matrix.h"
> > +
> > +#include <libcamera/base/log.h>
> > +
> > +namespace libcamera {
> > +
> > +LOG_DEFINE_CATEGORY(Matrix)
> > +
> > +} /* namespace libcamera */
> 
> 
> Do we not need Doxygen comments in here?

Technically it's not required since it's not public API but it's
probably better to have them yeah...

> 
> > diff --git a/src/ipa/libipa/matrix.h b/src/ipa/libipa/matrix.h
> > new file mode 100644
> > index 000000000..1bb794870
> > --- /dev/null
> > +++ b/src/ipa/libipa/matrix.h
> > @@ -0,0 +1,161 @@
> > +/* SPDX-License-Identifier: BSD-2-Clause */
> > +/*
> > + * Copyright (C) 2019, Raspberry Pi Ltd
> > + * Copyright (C) 2024, Paul Elder <paul.elder@ideasonboard.com>
> > + *
> > + * matrix.cpp - 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 {
> > +
> > +template<typename T, unsigned int R, unsigned int C,
> > +	 std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>
> > +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_;
> > +};
> > +
> > +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>
> > +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;
> > +}
> > +
> > +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>
> > +Matrix<T, R1, C2> operator*(const Matrix<T, R1, C1> &m1, const Matrix<T, R2, C2> &m2)
> Don't we want to restrict this operation to matrices with matching
> dimensions? I would expect a single set of R and C for both of the matrices.

On one hand yeah, but on the other hand I thought this was more
thorough. SFINAE ought to protect us against non-matching dimensions.


Paul

> > +{
> > +	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;
> > +}
> > +
> > +template<typename T, unsigned int R, unsigned int C,
> > +	 std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>
> > +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 */
> > +
> > +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;
> > +}
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build
> > index 1b3faf8d5..1e34355fe 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'
> >   ])