[PATCH v4 01/11] libipa: Centralise Fixed / Floating point convertors

Kieran Bingham kieran.bingham at ideasonboard.com
Fri Nov 15 13:47:08 CET 2024


Quoting Daniel Scally (2024-11-15 12:25:30)
> The rkisp1 IPA has some utility functions to convert between fixed
> and floating point numbers. Move those to libipa so they're available
> for use in other IPA modules too.
> 
> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>

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

> ---
> Changes in v4:
> 
>         - New patch
> 
>  .../{rkisp1/utils.cpp => libipa/fixedpoint.cpp}  | 10 +++++-----
>  src/ipa/{rkisp1/utils.h => libipa/fixedpoint.h}  |  6 +++---
>  src/ipa/libipa/meson.build                       |  2 ++
>  src/ipa/rkisp1/algorithms/ccm.cpp                |  4 ++--
>  src/ipa/rkisp1/meson.build                       |  1 -
>  .../rkisp1-utils.cpp => libipa/fixedpoint.cpp}   | 16 ++++++++--------
>  test/ipa/libipa/meson.build                      |  1 +
>  test/ipa/meson.build                             |  1 -
>  test/ipa/rkisp1/meson.build                      | 15 ---------------
>  9 files changed, 21 insertions(+), 35 deletions(-)
>  rename src/ipa/{rkisp1/utils.cpp => libipa/fixedpoint.cpp} (87%)
>  rename src/ipa/{rkisp1/utils.h => libipa/fixedpoint.h} (93%)
>  rename test/ipa/{rkisp1/rkisp1-utils.cpp => libipa/fixedpoint.cpp} (85%)
>  delete mode 100644 test/ipa/rkisp1/meson.build
> 
> diff --git a/src/ipa/rkisp1/utils.cpp b/src/ipa/libipa/fixedpoint.cpp
> similarity index 87%
> rename from src/ipa/rkisp1/utils.cpp
> rename to src/ipa/libipa/fixedpoint.cpp
> index 960ec64e..6b698fc5 100644
> --- a/src/ipa/rkisp1/utils.cpp
> +++ b/src/ipa/libipa/fixedpoint.cpp
> @@ -2,18 +2,18 @@
>  /*
>   * Copyright (C) 2024, Paul Elder <paul.elder at ideasonboard.com>
>   *
> - * Miscellaneous utility functions specific to rkisp1
> + * Fixed / floating point conversions
>   */
>  
> -#include "utils.h"
> +#include "fixedpoint.h"
>  
>  /**
> - * \file utils.h
> + * \file fixedpoint.h
>   */
>  
>  namespace libcamera {
>  
> -namespace ipa::rkisp1::utils {
> +namespace ipa {
>  
>  /**
>   * \fn R floatingToFixedPoint(T number)
> @@ -37,6 +37,6 @@ namespace ipa::rkisp1::utils {
>   * \return The converted value
>   */
>  
> -} /* namespace ipa::rkisp1::utils */
> +} /* namespace ipa */
>  
>  } /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/utils.h b/src/ipa/libipa/fixedpoint.h
> similarity index 93%
> rename from src/ipa/rkisp1/utils.h
> rename to src/ipa/libipa/fixedpoint.h
> index 5f38b50b..709cf50f 100644
> --- a/src/ipa/rkisp1/utils.h
> +++ b/src/ipa/libipa/fixedpoint.h
> @@ -2,7 +2,7 @@
>  /*
>   * Copyright (C) 2024, Paul Elder <paul.elder at ideasonboard.com>
>   *
> - * Miscellaneous utility functions specific to rkisp1
> + * Fixed / floating point conversions
>   */
>  
>  #pragma once
> @@ -12,7 +12,7 @@
>  
>  namespace libcamera {
>  
> -namespace ipa::rkisp1::utils {
> +namespace ipa {
>  
>  #ifndef __DOXYGEN__
>  template<unsigned int I, unsigned int F, typename R, typename T,
> @@ -60,6 +60,6 @@ constexpr R fixedToFloatingPoint(T number)
>         return static_cast<R>(t) / static_cast<R>(1 << F);
>  }
>  
> -} /* namespace ipa::rkisp1::utils */
> +} /* namespace ipa */
>  
>  } /* namespace libcamera */
> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build
> index 788d037a..a1d662c9 100644
> --- a/src/ipa/libipa/meson.build
> +++ b/src/ipa/libipa/meson.build
> @@ -7,6 +7,7 @@ libipa_headers = files([
>      'colours.h',
>      'exposure_mode_helper.h',
>      'fc_queue.h',
> +    'fixedpoint.h',
>      'histogram.h',
>      'interpolator.h',
>      'lsc_polynomial.h',
> @@ -23,6 +24,7 @@ libipa_sources = files([
>      'colours.cpp',
>      'exposure_mode_helper.cpp',
>      'fc_queue.cpp',
> +    'fixedpoint.cpp',
>      'histogram.cpp',
>      'interpolator.cpp',
>      'lsc_polynomial.cpp',
> diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp
> index 6b7d2e2c..e2b5cf4d 100644
> --- a/src/ipa/rkisp1/algorithms/ccm.cpp
> +++ b/src/ipa/rkisp1/algorithms/ccm.cpp
> @@ -18,7 +18,7 @@
>  
>  #include "libcamera/internal/yaml_parser.h"
>  
> -#include "../utils.h"
> +#include "libipa/fixedpoint.h"
>  #include "libipa/interpolator.h"
>  
>  /**
> @@ -72,7 +72,7 @@ void Ccm::setParameters(struct rkisp1_cif_isp_ctk_config &config,
>         for (unsigned int i = 0; i < 3; i++) {
>                 for (unsigned int j = 0; j < 3; j++)
>                         config.coeff[i][j] =
> -                               utils::floatingToFixedPoint<4, 7, uint16_t, double>(matrix[i][j]);
> +                               floatingToFixedPoint<4, 7, uint16_t, double>(matrix[i][j]);
>         }
>  
>         for (unsigned int i = 0; i < 3; i++)
> diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build
> index 34844f14..26a9fa40 100644
> --- a/src/ipa/rkisp1/meson.build
> +++ b/src/ipa/rkisp1/meson.build
> @@ -9,7 +9,6 @@ rkisp1_ipa_sources = files([
>      'ipa_context.cpp',
>      'params.cpp',
>      'rkisp1.cpp',
> -    'utils.cpp',
>  ])
>  
>  rkisp1_ipa_sources += rkisp1_ipa_algorithms
> diff --git a/test/ipa/rkisp1/rkisp1-utils.cpp b/test/ipa/libipa/fixedpoint.cpp
> similarity index 85%
> rename from test/ipa/rkisp1/rkisp1-utils.cpp
> rename to test/ipa/libipa/fixedpoint.cpp
> index b1863894..99eb662d 100644
> --- a/test/ipa/rkisp1/rkisp1-utils.cpp
> +++ b/test/ipa/libipa/fixedpoint.cpp
> @@ -2,7 +2,7 @@
>  /*
>   * Copyright (C) 2024, Paul Elder <paul.elder at ideasonboard.com>
>   *
> - * Miscellaneous utility tests
> + * Fixed / Floating point utility tests
>   */
>  
>  #include <cmath>
> @@ -10,22 +10,22 @@
>  #include <map>
>  #include <stdint.h>
>  
> -#include "../src/ipa/rkisp1/utils.h"
> +#include "../src/ipa/libipa/fixedpoint.h"
>  
>  #include "test.h"
>  
>  using namespace std;
>  using namespace libcamera;
> -using namespace ipa::rkisp1;
> +using namespace ipa;
>  
> -class RkISP1UtilsTest : public Test
> +class FixedPointUtilsTest : public Test
>  {
>  protected:
>         /* R for real, I for integer */
>         template<unsigned int IntPrec, unsigned int FracPrec, typename I, typename R>
>         int testFixedToFloat(I input, R expected)
>         {
> -               R out = utils::fixedToFloatingPoint<IntPrec, FracPrec, R>(input);
> +               R out = fixedToFloatingPoint<IntPrec, FracPrec, R>(input);
>                 R prec = 1.0 / (1 << FracPrec);
>                 if (std::abs(out - expected) > prec) {
>                         cerr << "Reverse conversion expected " << input
> @@ -40,7 +40,7 @@ protected:
>         template<unsigned int IntPrec, unsigned int FracPrec, typename T>
>         int testSingleFixedPoint(double input, T expected)
>         {
> -               T ret = utils::floatingToFixedPoint<IntPrec, FracPrec, T>(input);
> +               T ret = floatingToFixedPoint<IntPrec, FracPrec, T>(input);
>                 if (ret != expected) {
>                         cerr << "Expected " << input << " to convert to "
>                              << expected << ", got " << ret << std::endl;
> @@ -51,7 +51,7 @@ protected:
>                  * The precision check is fairly arbitrary but is based on what
>                  * the rkisp1 is capable of in the crosstalk module.
>                  */
> -               double f = utils::fixedToFloatingPoint<IntPrec, FracPrec, double>(ret);
> +               double f = fixedToFloatingPoint<IntPrec, FracPrec, double>(ret);
>                 if (std::abs(f - input) > 0.005) {
>                         cerr << "Reverse conversion expected " << ret
>                              << " to convert to " << input
> @@ -105,4 +105,4 @@ protected:
>         }
>  };
>  
> -TEST_REGISTER(RkISP1UtilsTest)
> +TEST_REGISTER(FixedPointUtilsTest)
> diff --git a/test/ipa/libipa/meson.build b/test/ipa/libipa/meson.build
> index 4d2427db..281e6b1f 100644
> --- a/test/ipa/libipa/meson.build
> +++ b/test/ipa/libipa/meson.build
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
>  libipa_test = [
> +    {'name': 'fixedpoint', 'sources': ['fixedpoint.cpp']},
>      {'name': 'interpolator', 'sources': ['interpolator.cpp']},
>  ]
>  
> diff --git a/test/ipa/meson.build b/test/ipa/meson.build
> index 63820de5..ceed15ba 100644
> --- a/test/ipa/meson.build
> +++ b/test/ipa/meson.build
> @@ -1,7 +1,6 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
>  subdir('libipa')
> -subdir('rkisp1')
>  
>  ipa_test = [
>      {'name': 'ipa_module_test', 'sources': ['ipa_module_test.cpp']},
> diff --git a/test/ipa/rkisp1/meson.build b/test/ipa/rkisp1/meson.build
> deleted file mode 100644
> index 894523da..00000000
> --- a/test/ipa/rkisp1/meson.build
> +++ /dev/null
> @@ -1,15 +0,0 @@
> -# SPDX-License-Identifier: CC0-1.0
> -
> -rkisp1_ipa_test = [
> -    {'name': 'rkisp1-utils', 'sources': ['rkisp1-utils.cpp']},
> -]
> -
> -foreach test : rkisp1_ipa_test
> -    exe = executable(test['name'], test['sources'],
> -                     dependencies : [libcamera_private, libipa_dep],
> -                     link_with : [test_libraries],
> -                     include_directories : [test_includes_internal,
> -                                            '../../../src/ipa/rkisp1/'])
> -
> -    test(test['name'], exe, suite : 'ipa')
> -endforeach
> -- 
> 2.30.2
>


More information about the libcamera-devel mailing list