[PATCH v2 4/9] ipa: rkisp1: Remove MatrixInterpolator

Kieran Bingham kieran.bingham at ideasonboard.com
Fri Sep 13 11:15:10 CEST 2024


Quoting Stefan Klug (2024-09-13 08:57:22)
> The MatrixInterpolator is no longer used. Remove it.

That's an easy one.

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

> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> ---
>  src/ipa/libipa/matrix_interpolator.cpp | 103 ---------------------
>  src/ipa/libipa/matrix_interpolator.h   | 120 -------------------------
>  src/ipa/libipa/meson.build             |   2 -
>  3 files changed, 225 deletions(-)
>  delete mode 100644 src/ipa/libipa/matrix_interpolator.cpp
>  delete mode 100644 src/ipa/libipa/matrix_interpolator.h
> 
> diff --git a/src/ipa/libipa/matrix_interpolator.cpp b/src/ipa/libipa/matrix_interpolator.cpp
> deleted file mode 100644
> index d5188f8aa4e9..000000000000
> --- a/src/ipa/libipa/matrix_interpolator.cpp
> +++ /dev/null
> @@ -1,103 +0,0 @@
> -/* SPDX-License-Identifier: LGPL-2.1-or-later */
> -/*
> - * Copyright (C) 2024, Paul Elder <paul.elder at ideasonboard.com>
> - *
> - * Helper class for interpolating maps of matrices
> - */
> -#include "matrix_interpolator.h"
> -
> -#include <libcamera/base/log.h>
> -
> -/**
> - * \file matrix_interpolator.h
> - * \brief Helper class for interpolating maps of matrices
> - */
> -
> -namespace libcamera {
> -
> -LOG_DEFINE_CATEGORY(MatrixInterpolator)
> -
> -namespace ipa {
> -
> -/**
> - * \class MatrixInterpolator
> - * \brief Class for storing, retrieving, and interpolating matrices
> - * \tparam T Type of numerical values to be stored in the matrices
> - * \tparam R Number of rows in the matrices
> - * \tparam C Number of columns in the matrices
> - *
> - * The main use case is to pass a map from color temperatures to corresponding
> - * matrices (eg. color correction), and then requesting a matrix for a specific
> - * color temperature. This class will abstract away the interpolation portion.
> - */
> -
> -/**
> - * \fn MatrixInterpolator::MatrixInterpolator(const std::map<unsigned int, Matrix<T, R, C>> &matrices)
> - * \brief Construct a matrix interpolator from a map of matrices
> - * \param matrices Map from which to construct the matrix interpolator
> - */
> -
> -/**
> - * \fn MatrixInterpolator::reset()
> - * \brief Reset the matrix interpolator content to a single identity matrix
> - */
> -
> -/**
> - * \fn int MatrixInterpolator<T, R, C>::readYaml()
> - * \brief Initialize an MatrixInterpolator instance from yaml
> - * \tparam T Type of data stored in the matrices
> - * \tparam R Number of rows of the matrices
> - * \tparam C Number of columns of the matrices
> - * \param[in] yaml The yaml object that contains the map of unsigned integers to matrices
> - * \param[in] key_name The name of the key in the yaml object
> - * \param[in] matrix_name The name of the matrix in the yaml object
> - *
> - * The yaml object is expected to be a list of maps. Each map has two or more
> - * pairs: one of \a key_name to the key value (usually color temperature), and
> - * one or more of \a matrix_name to the matrix. This is a bit difficult to
> - * explain, so here is an example (in python, as it is easier to parse than
> - * yaml):
> - *       [
> - *               {
> - *                   'ct': 2860,
> - *                   'ccm': [ 2.12089, -0.52461, -0.59629,
> - *                           -0.85342,  2.80445, -0.95103,
> - *                           -0.26897, -1.14788,  2.41685 ],
> - *                   'offsets': [ 0, 0, 0 ]
> - *               },
> - *
> - *               {
> - *                   'ct': 2960,
> - *                   'ccm': [ 2.26962, -0.54174, -0.72789,
> - *                           -0.77008,  2.60271, -0.83262,
> - *                           -0.26036, -1.51254,  2.77289 ],
> - *                   'offsets': [ 0, 0, 0 ]
> - *               },
> - *
> - *               {
> - *                   'ct': 3603,
> - *                   'ccm': [ 2.18644, -0.66148, -0.52496,
> - *                           -0.77828,  2.69474, -0.91645,
> - *                           -0.25239, -0.83059,  2.08298 ],
> - *                   'offsets': [ 0, 0, 0 ]
> - *               },
> - *       ]
> - *
> - * In this case, \a key_name would be 'ct', and \a matrix_name can be either
> - * 'ccm' or 'offsets'. This way multiple matrix interpolators can be defined in
> - * one set of color temperature ranges in the tuning file, and they can be
> - * retrieved separately with the \a matrix_name parameter.
> - *
> - * \return Zero on success, negative error code otherwise
> - */
> -
> -/**
> - * \fn Matrix<T, R, C> MatrixInterpolator<T, R, C>::get(unsigned int key)
> - * \brief Retrieve a matrix from the list of matrices, interpolating if necessary
> - * \param[in] key The unsigned integer key of the matrix to retrieve
> - * \return The matrix corresponding to the color temperature
> - */
> -
> -} /* namespace ipa */
> -
> -} /* namespace libcamera */
> diff --git a/src/ipa/libipa/matrix_interpolator.h b/src/ipa/libipa/matrix_interpolator.h
> deleted file mode 100644
> index afbce5386b06..000000000000
> --- a/src/ipa/libipa/matrix_interpolator.h
> +++ /dev/null
> @@ -1,120 +0,0 @@
> -/* SPDX-License-Identifier: LGPL-2.1-or-later */
> -/*
> - * Copyright (C) 2024, Paul Elder <paul.elder at ideasonboard.com>
> - *
> - * Helper class for interpolating maps of matrices
> - */
> -
> -#pragma once
> -
> -#include <map>
> -#include <string>
> -
> -#include <libcamera/base/log.h>
> -
> -#include "libcamera/internal/yaml_parser.h"
> -
> -#include "matrix.h"
> -
> -namespace libcamera {
> -
> -LOG_DECLARE_CATEGORY(MatrixInterpolator)
> -
> -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 MatrixInterpolator
> -{
> -public:
> -       MatrixInterpolator()
> -       {
> -               reset();
> -       }
> -
> -       MatrixInterpolator(const std::map<unsigned int, Matrix<T, R, C>> &matrices)
> -       {
> -               for (const auto &pair : matrices)
> -                       matrices_[pair.first] = pair.second;
> -       }
> -
> -       ~MatrixInterpolator() {}
> -
> -       void reset()
> -       {
> -               matrices_.clear();
> -               matrices_[0] = Matrix<T, R, C>::identity();
> -       }
> -
> -       int readYaml(const libcamera::YamlObject &yaml,
> -                    const std::string &key_name,
> -                    const std::string &matrix_name)
> -       {
> -               matrices_.clear();
> -
> -               if (!yaml.isList()) {
> -                       LOG(MatrixInterpolator, Error) << "yaml object must be a list";
> -                       return -EINVAL;
> -               }
> -
> -               for (const auto &value : yaml.asList()) {
> -                       unsigned int ct = std::stoul(value[key_name].get<std::string>(""));
> -                       std::optional<Matrix<T, R, C>> matrix =
> -                               value[matrix_name].get<Matrix<T, R, C>>();
> -                       if (!matrix) {
> -                               LOG(MatrixInterpolator, Error) << "Failed to read matrix";
> -                               return -EINVAL;
> -                       }
> -
> -                       matrices_[ct] = *matrix;
> -
> -                       LOG(MatrixInterpolator, Debug)
> -                               << "Read matrix '" << matrix_name << "' for key '"
> -                               << key_name << "' " << ct << ": "
> -                               << matrices_[ct].toString();
> -               }
> -
> -               if (matrices_.size() < 1) {
> -                       LOG(MatrixInterpolator, Error) << "Need at least one matrix";
> -                       return -EINVAL;
> -               }
> -
> -               return 0;
> -       }
> -
> -       Matrix<T, R, C> get(unsigned int ct)
> -       {
> -               ASSERT(matrices_.size() > 0);
> -
> -               if (matrices_.size() == 1 ||
> -                   ct <= matrices_.begin()->first)
> -                       return matrices_.begin()->second;
> -
> -               if (ct >= matrices_.rbegin()->first)
> -                       return matrices_.rbegin()->second;
> -
> -               if (matrices_.find(ct) != matrices_.end())
> -                       return matrices_[ct];
> -
> -               /* The above four guarantee that this will succeed */
> -               auto iter = matrices_.upper_bound(ct);
> -               unsigned int ctUpper = iter->first;
> -               unsigned int ctLower = (--iter)->first;
> -
> -               double lambda = (ct - ctLower) / static_cast<double>(ctUpper - ctLower);
> -               Matrix<T, R, C> ret =
> -                       lambda * matrices_[ctUpper] + (1.0 - lambda) * matrices_[ctLower];
> -               return ret;
> -       }
> -
> -private:
> -       std::map<unsigned int, Matrix<T, R, C>> matrices_;
> -};
> -
> -} /* namespace ipa */
> -
> -} /* namespace libcamera */
> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build
> index 2c2712a7d252..3740178b2909 100644
> --- a/src/ipa/libipa/meson.build
> +++ b/src/ipa/libipa/meson.build
> @@ -9,7 +9,6 @@ libipa_headers = files([
>      'histogram.h',
>      'interpolator.h',
>      'matrix.h',
> -    'matrix_interpolator.h',
>      'module.h',
>      'pwl.h',
>      'vector.h',
> @@ -24,7 +23,6 @@ libipa_sources = files([
>      'histogram.cpp',
>      'interpolator.cpp',
>      'matrix.cpp',
> -    'matrix_interpolator.cpp',
>      'module.cpp',
>      'pwl.cpp',
>      'vector.cpp',
> -- 
> 2.43.0
>


More information about the libcamera-devel mailing list