From 3dd5725a84f5ac71c384431ffc4929962aeaf6b2 Mon Sep 17 00:00:00 2001 From: Daniel Scally Date: Fri, 15 Nov 2024 12:25:30 +0000 Subject: libipa: Centralise Fixed / Floating point convertors 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 Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart --- src/ipa/libipa/fixedpoint.cpp | 42 +++++++++++++++ src/ipa/libipa/fixedpoint.h | 65 +++++++++++++++++++++++ src/ipa/libipa/meson.build | 2 + src/ipa/rkisp1/algorithms/ccm.cpp | 4 +- src/ipa/rkisp1/meson.build | 1 - src/ipa/rkisp1/utils.cpp | 42 --------------- src/ipa/rkisp1/utils.h | 65 ----------------------- test/ipa/libipa/fixedpoint.cpp | 108 ++++++++++++++++++++++++++++++++++++++ test/ipa/libipa/meson.build | 1 + test/ipa/meson.build | 1 - test/ipa/rkisp1/meson.build | 15 ------ test/ipa/rkisp1/rkisp1-utils.cpp | 108 -------------------------------------- 12 files changed, 220 insertions(+), 234 deletions(-) create mode 100644 src/ipa/libipa/fixedpoint.cpp create mode 100644 src/ipa/libipa/fixedpoint.h delete mode 100644 src/ipa/rkisp1/utils.cpp delete mode 100644 src/ipa/rkisp1/utils.h create mode 100644 test/ipa/libipa/fixedpoint.cpp delete mode 100644 test/ipa/rkisp1/meson.build delete mode 100644 test/ipa/rkisp1/rkisp1-utils.cpp diff --git a/src/ipa/libipa/fixedpoint.cpp b/src/ipa/libipa/fixedpoint.cpp new file mode 100644 index 00000000..6b698fc5 --- /dev/null +++ b/src/ipa/libipa/fixedpoint.cpp @@ -0,0 +1,42 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2024, Paul Elder + * + * Fixed / floating point conversions + */ + +#include "fixedpoint.h" + +/** + * \file fixedpoint.h + */ + +namespace libcamera { + +namespace ipa { + +/** + * \fn R floatingToFixedPoint(T number) + * \brief Convert a floating point number to a fixed-point representation + * \tparam I Bit width of the integer part of the fixed-point + * \tparam F Bit width of the fractional part of the fixed-point + * \tparam R Return type of the fixed-point representation + * \tparam T Input type of the floating point representation + * \param number The floating point number to convert to fixed point + * \return The converted value + */ + +/** + * \fn R fixedToFloatingPoint(T number) + * \brief Convert a fixed-point number to a floating point representation + * \tparam I Bit width of the integer part of the fixed-point + * \tparam F Bit width of the fractional part of the fixed-point + * \tparam R Return type of the floating point representation + * \tparam T Input type of the fixed-point representation + * \param number The fixed point number to convert to floating point + * \return The converted value + */ + +} /* namespace ipa */ + +} /* namespace libcamera */ diff --git a/src/ipa/libipa/fixedpoint.h b/src/ipa/libipa/fixedpoint.h new file mode 100644 index 00000000..709cf50f --- /dev/null +++ b/src/ipa/libipa/fixedpoint.h @@ -0,0 +1,65 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2024, Paul Elder + * + * Fixed / floating point conversions + */ + +#pragma once + +#include +#include + +namespace libcamera { + +namespace ipa { + +#ifndef __DOXYGEN__ +template && + std::is_floating_point_v> * = nullptr> +#else +template +#endif +constexpr R floatingToFixedPoint(T number) +{ + static_assert(sizeof(int) >= sizeof(R)); + static_assert(I + F <= sizeof(R) * 8); + + /* + * The intermediate cast to int is needed on arm platforms to properly + * cast negative values. See + * https://embeddeduse.com/2013/08/25/casting-a-negative-float-to-an-unsigned-int/ + */ + R mask = (1 << (F + I)) - 1; + R frac = static_cast(static_cast(std::round(number * (1 << F)))) & mask; + + return frac; +} + +#ifndef __DOXYGEN__ +template && + std::is_integral_v> * = nullptr> +#else +template +#endif +constexpr R fixedToFloatingPoint(T number) +{ + static_assert(sizeof(int) >= sizeof(T)); + static_assert(I + F <= sizeof(T) * 8); + + /* + * Recreate the upper bits in case of a negative number by shifting the sign + * bit from the fixed point to the first bit of the unsigned and then right shifting + * by the same amount which keeps the sign bit in place. + * This can be optimized by the compiler quite well. + */ + int remaining_bits = sizeof(int) * 8 - (I + F); + int t = static_cast(static_cast(number) << remaining_bits) >> remaining_bits; + return static_cast(t) / static_cast(1 << F); +} + +} /* namespace ipa */ + +} /* namespace libcamera */ diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build index 93ae25da..a7f16ff6 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', @@ -22,6 +23,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/src/ipa/rkisp1/utils.cpp b/src/ipa/rkisp1/utils.cpp deleted file mode 100644 index 960ec64e..00000000 --- a/src/ipa/rkisp1/utils.cpp +++ /dev/null @@ -1,42 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -/* - * Copyright (C) 2024, Paul Elder - * - * Miscellaneous utility functions specific to rkisp1 - */ - -#include "utils.h" - -/** - * \file utils.h - */ - -namespace libcamera { - -namespace ipa::rkisp1::utils { - -/** - * \fn R floatingToFixedPoint(T number) - * \brief Convert a floating point number to a fixed-point representation - * \tparam I Bit width of the integer part of the fixed-point - * \tparam F Bit width of the fractional part of the fixed-point - * \tparam R Return type of the fixed-point representation - * \tparam T Input type of the floating point representation - * \param number The floating point number to convert to fixed point - * \return The converted value - */ - -/** - * \fn R fixedToFloatingPoint(T number) - * \brief Convert a fixed-point number to a floating point representation - * \tparam I Bit width of the integer part of the fixed-point - * \tparam F Bit width of the fractional part of the fixed-point - * \tparam R Return type of the floating point representation - * \tparam T Input type of the fixed-point representation - * \param number The fixed point number to convert to floating point - * \return The converted value - */ - -} /* namespace ipa::rkisp1::utils */ - -} /* namespace libcamera */ diff --git a/src/ipa/rkisp1/utils.h b/src/ipa/rkisp1/utils.h deleted file mode 100644 index 5f38b50b..00000000 --- a/src/ipa/rkisp1/utils.h +++ /dev/null @@ -1,65 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -/* - * Copyright (C) 2024, Paul Elder - * - * Miscellaneous utility functions specific to rkisp1 - */ - -#pragma once - -#include -#include - -namespace libcamera { - -namespace ipa::rkisp1::utils { - -#ifndef __DOXYGEN__ -template && - std::is_floating_point_v> * = nullptr> -#else -template -#endif -constexpr R floatingToFixedPoint(T number) -{ - static_assert(sizeof(int) >= sizeof(R)); - static_assert(I + F <= sizeof(R) * 8); - - /* - * The intermediate cast to int is needed on arm platforms to properly - * cast negative values. See - * https://embeddeduse.com/2013/08/25/casting-a-negative-float-to-an-unsigned-int/ - */ - R mask = (1 << (F + I)) - 1; - R frac = static_cast(static_cast(std::round(number * (1 << F)))) & mask; - - return frac; -} - -#ifndef __DOXYGEN__ -template && - std::is_integral_v> * = nullptr> -#else -template -#endif -constexpr R fixedToFloatingPoint(T number) -{ - static_assert(sizeof(int) >= sizeof(T)); - static_assert(I + F <= sizeof(T) * 8); - - /* - * Recreate the upper bits in case of a negative number by shifting the sign - * bit from the fixed point to the first bit of the unsigned and then right shifting - * by the same amount which keeps the sign bit in place. - * This can be optimized by the compiler quite well. - */ - int remaining_bits = sizeof(int) * 8 - (I + F); - int t = static_cast(static_cast(number) << remaining_bits) >> remaining_bits; - return static_cast(t) / static_cast(1 << F); -} - -} /* namespace ipa::rkisp1::utils */ - -} /* namespace libcamera */ diff --git a/test/ipa/libipa/fixedpoint.cpp b/test/ipa/libipa/fixedpoint.cpp new file mode 100644 index 00000000..99eb662d --- /dev/null +++ b/test/ipa/libipa/fixedpoint.cpp @@ -0,0 +1,108 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2024, Paul Elder + * + * Fixed / Floating point utility tests + */ + +#include +#include +#include +#include + +#include "../src/ipa/libipa/fixedpoint.h" + +#include "test.h" + +using namespace std; +using namespace libcamera; +using namespace ipa; + +class FixedPointUtilsTest : public Test +{ +protected: + /* R for real, I for integer */ + template + int testFixedToFloat(I input, R expected) + { + R out = fixedToFloatingPoint(input); + R prec = 1.0 / (1 << FracPrec); + if (std::abs(out - expected) > prec) { + cerr << "Reverse conversion expected " << input + << " to convert to " << expected + << ", got " << out << std::endl; + return TestFail; + } + + return TestPass; + } + + template + int testSingleFixedPoint(double input, T expected) + { + T ret = floatingToFixedPoint(input); + if (ret != expected) { + cerr << "Expected " << input << " to convert to " + << expected << ", got " << ret << std::endl; + return TestFail; + } + + /* + * The precision check is fairly arbitrary but is based on what + * the rkisp1 is capable of in the crosstalk module. + */ + double f = fixedToFloatingPoint(ret); + if (std::abs(f - input) > 0.005) { + cerr << "Reverse conversion expected " << ret + << " to convert to " << input + << ", got " << f << std::endl; + return TestFail; + } + + return TestPass; + } + + int testFixedPoint() + { + /* + * The second 7.992 test is to test that unused bits don't + * affect the result. + */ + std::map testCases = { + { 7.992, 0x3ff }, + { 0.2, 0x01a }, + { -0.2, 0x7e6 }, + { -0.8, 0x79a }, + { -0.4, 0x7cd }, + { -1.4, 0x74d }, + { -8, 0x400 }, + { 0, 0 }, + }; + + int ret; + for (const auto &testCase : testCases) { + ret = testSingleFixedPoint<4, 7, uint16_t>(testCase.first, + testCase.second); + if (ret != TestPass) + return ret; + } + + /* Special case with a superfluous one in the unused bits */ + ret = testFixedToFloat<4, 7, uint16_t, double>(0xbff, 7.992); + if (ret != TestPass) + return ret; + + return TestPass; + } + + int run() + { + /* fixed point conversion test */ + if (testFixedPoint() != TestPass) + return TestFail; + + return TestPass; + } +}; + +TEST_REGISTER(FixedPointUtilsTest) diff --git a/test/ipa/libipa/meson.build b/test/ipa/libipa/meson.build index f9b3c46d..0f4155d2 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']}, {'name': 'vector', 'sources': ['vector.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 diff --git a/test/ipa/rkisp1/rkisp1-utils.cpp b/test/ipa/rkisp1/rkisp1-utils.cpp deleted file mode 100644 index b1863894..00000000 --- a/test/ipa/rkisp1/rkisp1-utils.cpp +++ /dev/null @@ -1,108 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ -/* - * Copyright (C) 2024, Paul Elder - * - * Miscellaneous utility tests - */ - -#include -#include -#include -#include - -#include "../src/ipa/rkisp1/utils.h" - -#include "test.h" - -using namespace std; -using namespace libcamera; -using namespace ipa::rkisp1; - -class RkISP1UtilsTest : public Test -{ -protected: - /* R for real, I for integer */ - template - int testFixedToFloat(I input, R expected) - { - R out = utils::fixedToFloatingPoint(input); - R prec = 1.0 / (1 << FracPrec); - if (std::abs(out - expected) > prec) { - cerr << "Reverse conversion expected " << input - << " to convert to " << expected - << ", got " << out << std::endl; - return TestFail; - } - - return TestPass; - } - - template - int testSingleFixedPoint(double input, T expected) - { - T ret = utils::floatingToFixedPoint(input); - if (ret != expected) { - cerr << "Expected " << input << " to convert to " - << expected << ", got " << ret << std::endl; - return TestFail; - } - - /* - * The precision check is fairly arbitrary but is based on what - * the rkisp1 is capable of in the crosstalk module. - */ - double f = utils::fixedToFloatingPoint(ret); - if (std::abs(f - input) > 0.005) { - cerr << "Reverse conversion expected " << ret - << " to convert to " << input - << ", got " << f << std::endl; - return TestFail; - } - - return TestPass; - } - - int testFixedPoint() - { - /* - * The second 7.992 test is to test that unused bits don't - * affect the result. - */ - std::map testCases = { - { 7.992, 0x3ff }, - { 0.2, 0x01a }, - { -0.2, 0x7e6 }, - { -0.8, 0x79a }, - { -0.4, 0x7cd }, - { -1.4, 0x74d }, - { -8, 0x400 }, - { 0, 0 }, - }; - - int ret; - for (const auto &testCase : testCases) { - ret = testSingleFixedPoint<4, 7, uint16_t>(testCase.first, - testCase.second); - if (ret != TestPass) - return ret; - } - - /* Special case with a superfluous one in the unused bits */ - ret = testFixedToFloat<4, 7, uint16_t, double>(0xbff, 7.992); - if (ret != TestPass) - return ret; - - return TestPass; - } - - int run() - { - /* fixed point conversion test */ - if (testFixedPoint() != TestPass) - return TestFail; - - return TestPass; - } -}; - -TEST_REGISTER(RkISP1UtilsTest) -- cgit v1.2.1