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

Kieran Bingham kieran.bingham at ideasonboard.com
Mon Jun 10 12:00:17 CEST 2024


Quoting Paul Elder (2024-06-07 11:03:23)
> 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 moving the offending test out of the loop.
> 
> 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

I think it's better to keep trailier lines on a single line even if it
extends beyond 72/80 chars.

I think the title is normally in brackets and quotes too?

Fixes: 9d152e9c6 ("ipa: rkisp1: Add a helper to convert floating-point to fixed-point")

But just trivial nits. That doesn't seem to be enforced by checkstyle
presently.


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


> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Stefan Klug <stefan.klug at ideasonboard.com>
> 
> ---
> Changes in v2:
> - remove the added complexity that would've made it clearner to add a
>   larger variety of tests because the variety wasn't there
> ---
>  test/ipa/rkisp1/rkisp1-utils.cpp | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/test/ipa/rkisp1/rkisp1-utils.cpp b/test/ipa/rkisp1/rkisp1-utils.cpp
> index e48f8d362..b1863894c 100644
> --- a/test/ipa/rkisp1/rkisp1-utils.cpp
> +++ b/test/ipa/rkisp1/rkisp1-utils.cpp
> @@ -21,7 +21,23 @@ using namespace ipa::rkisp1;
>  class RkISP1UtilsTest : public Test
>  {
>  protected:
> -       template<unsigned int IntPrec, unsigned FracPrec, typename T>
> +       /* 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 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<unsigned int IntPrec, unsigned int FracPrec, typename T>
>         int testSingleFixedPoint(double input, T expected)
>         {
>                 T ret = utils::floatingToFixedPoint<IntPrec, FracPrec, T>(input);
> @@ -54,7 +70,6 @@ protected:
>                  */
>                 std::map<double, uint16_t> testCases = {
>                         { 7.992, 0x3ff },
> -                       { 7.992, 0xbff },
>                         {   0.2, 0x01a },
>                         {  -0.2, 0x7e6 },
>                         {  -0.8, 0x79a },
> @@ -72,6 +87,11 @@ protected:
>                                 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;
>         }
>  
> -- 
> 2.39.2
>


More information about the libcamera-devel mailing list