[PATCH] test: ipa: rkisp1: utils: Fix floating and fixed point conversion test

[Paul Elder]

There was an issue where using map to store the test cases meant that
the test for ignoring unused bits was skipped because of clashing keys.
Fix this by changing the test to an array of tuples.

While at it, reorganize the tests so that it's possible to test only
reverse conversion, which was the case here to test that multiple fixed
point numbers (due to unused bits) would result in the same floating
point number.

While at it, also change the arbitrary floating comparison precision to
be more precise.

Also fix a missing documentation brief.

Fixes: 9d152e9c6 ipa: rkisp1: Add a helper to convert floating-point to
fixed-point
Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
---
 src/ipa/rkisp1/utils.cpp         |  1 +
 test/ipa/rkisp1/rkisp1-utils.cpp | 97 +++++++++++++++++++++++---------
 2 files changed, 70 insertions(+), 28 deletions(-)

diff --git a/src/ipa/rkisp1/utils.cpp b/src/ipa/rkisp1/utils.cpp
index 960ec64e9..8edcf5bf7 100644
--- a/src/ipa/rkisp1/utils.cpp
+++ b/src/ipa/rkisp1/utils.cpp
@@ -9,6 +9,7 @@
 
 /**
  * \file utils.h
+ * \brief Utility functions for rkisp1
  */
 
 namespace libcamera {
diff --git a/test/ipa/rkisp1/rkisp1-utils.cpp b/test/ipa/rkisp1/rkisp1-utils.cpp
index e48f8d362..4757ca069 100644
--- a/test/ipa/rkisp1/rkisp1-utils.cpp
+++ b/test/ipa/rkisp1/rkisp1-utils.cpp
@@ -7,8 +7,8 @@
 
 #include <cmath>
 #include <iostream>
-#include <map>
 #include <stdint.h>
+#include <tuple>
 
 #include "../src/ipa/rkisp1/utils.h"
 
@@ -21,53 +21,94 @@ using namespace ipa::rkisp1;
 class RkISP1UtilsTest : public Test
 {
 protected:
-	template<unsigned int IntPrec, unsigned FracPrec, typename T>
-	int testSingleFixedPoint(double input, T expected)
+	/* R for real, I for integer */
+	template<unsigned int IntPrec, unsigned FracPrec, typename I, typename R>
+	int testFixedToFloat(I input, R expected, R *output = nullptr)
 	{
-		T ret = utils::floatingToFixedPoint<IntPrec, FracPrec, T>(input);
-		if (ret != expected) {
-			cerr << "Expected " << input << " to convert to "
-			     << expected << ", got " << ret << std::endl;
+		R out = utils::fixedToFloatingPoint<IntPrec, FracPrec, R>(input);
+		if (output)
+			*output = out;
+		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;
 		}
 
-		/*
-		 * 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);
-		if (std::abs(f - input) > 0.005) {
-			cerr << "Reverse conversion expected " << ret
-			     << " to convert to " << input
-			     << ", got " << f << std::endl;
+		return TestPass;
+	}
+
+	/* R for real, I for integer */
+	template<unsigned int IntPrec, unsigned FracPrec, typename R, typename I>
+	int testFloatToFixed(R input, I expected, I *output = nullptr)
+	{
+		I out = utils::floatingToFixedPoint<IntPrec, FracPrec, I>(input);
+		if (output)
+			*output = out;
+		if (out != expected) {
+			cerr << "Expected " << input << " to convert to "
+			     << expected << ", got " << out << std::endl;
 			return TestFail;
 		}
 
 		return TestPass;
 	}
 
+	/* R for real, I for integer */
+	template<unsigned int IntPrec, unsigned FracPrec, typename R, typename I>
+	int testFullConversion(R input, I expected)
+	{
+		I outInt;
+		R outReal;
+		int status;
+
+		status = testFloatToFixed<IntPrec, FracPrec, R, I>(input, expected, &outInt);
+		if (status != TestPass)
+			return status;
+
+		status = testFixedToFloat<IntPrec, FracPrec, I, R>(outInt, input, &outReal);
+		if (status != TestPass)
+			return status;
+
+		return TestPass;
+	}
+
+
 	int testFixedPoint()
 	{
 		/*
 		 * The second 7.992 test is to test that unused bits don't
 		 * affect the result.
+		 *
+		 * Third parameter is for testing forward, fourth parameter is
+		 * for testing reverse.
 		 */
-		std::map<double, uint16_t> testCases = {
-			{ 7.992, 0x3ff },
-			{ 7.992, 0xbff },
-			{   0.2, 0x01a },
-			{  -0.2, 0x7e6 },
-			{  -0.8, 0x79a },
-			{  -0.4, 0x7cd },
-			{  -1.4, 0x74d },
-			{    -8, 0x400 },
-			{     0, 0 },
+		static const std::tuple<double, uint16_t, bool, bool> testCases[] = {
+			{ 7.992, 0x3ff,  true, true },
+			{ 7.992, 0xbff, false, true },
+			{   0.2, 0x01a,  true, true },
+			{  -0.2, 0x7e6,  true, true },
+			{  -0.8, 0x79a,  true, true },
+			{  -0.4, 0x7cd,  true, true },
+			{  -1.4, 0x74d,  true, true },
+			{    -8, 0x400,  true, true },
+			{     0,     0,  true, true },
 		};
 
 		int ret;
 		for (const auto &testCase : testCases) {
-			ret = testSingleFixedPoint<4, 7, uint16_t>(testCase.first,
-								   testCase.second);
+			double floating;
+			uint16_t fixed;
+			bool forward, backward;
+			std::tie(floating, fixed, forward, backward) = testCase;
+			if (forward && backward)
+				ret = testFullConversion<4, 7, double, uint16_t>(floating, fixed);
+			else if (forward)
+				ret = testFloatToFixed<4, 7, double, uint16_t>(floating, fixed);
+			else if (backward)
+				ret = testFixedToFloat<4, 7, uint16_t, double>(fixed, floating);
+
 			if (ret != TestPass)
 				return ret;
 		}
-- 
2.39.2