[{"id":29220,"web_url":"https://patchwork.libcamera.org/comment/29220/","msgid":"","date":"2024-04-15T09:00:23","subject":"Re: [PATCH] libcamera: utils: Add a helper to convert floating-point\n\tto fixed-point","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Wed, Mar 27, 2024 at 05:57:00PM +0900, Paul Elder wrote:\n> Add a helper function to convert floating-point numbers to fixed-point\n> numbers.\n> \n> Signed-off-by: Paul Elder \n> ---\n> I originally needed this for rkisp1 crosstalk but then I realized it's\n> more efficient to just have the tuning tool output fixed-point and have\n> the IPA simply copy those values instead of calculating them on-the-fly.\n> Still, I made this helper so if people think it's useful...\n\nTurns out we do need it now! So that we can have human-readable (or\nmake-sensable) CCMs in the tuning file for rkisp1.\n\n\nPaul\n\n> ---\n> include/libcamera/base/utils.h | 27 +++++++++++++++++++++++++++\n> src/libcamera/base/utils.cpp | 10 ++++++++++\n> 2 files changed, 37 insertions(+)\n> \n> diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h\n> index 37d9af60..ab421cc7 100644\n> --- a/include/libcamera/base/utils.h\n> +++ b/include/libcamera/base/utils.h\n> @@ -369,6 +369,33 @@ decltype(auto) abs_diff(const T &a, const T &b)\n> \n> double strtod(const char *__restrict nptr, char **__restrict endptr);\n> \n> +#ifndef __DOXYGEN__\n> +template +\t std::enable_if_t &&\n> +\t\t\t std::is_floating_point_v> * = nullptr>\n> +#else\n> +template\n> +#endif\n> +constexpr R floatingToFixedPoint(T number)\n> +{\n> +\tstatic_assert(I + F <= sizeof(R) * 8);\n> +\n> +\tR integer = static_cast(number);\n> +\tT fractional = number - integer;\n> +\n> +\tR mask = (1 << I) - 1;\n> +\tR whole = (integer >= 0 ? integer : ~integer + 1) & mask;\n> +\n> +\tR frac = 0;\n> +\tfor (unsigned int i = 0; i < F; i++) {\n> +\t\tfractional *= 2;\n> +\t\tfrac <<= 1;\n> +\t\tfrac |= (static_cast(fractional) & 0x1);\n> +\t}\n> +\n> +\treturn (whole << F) | frac;\n> +}\n> +\n> } /* namespace utils */\n> \n> #ifndef __DOXYGEN__\n> diff --git a/src/libcamera/base/utils.cpp b/src/libcamera/base/utils.cpp\n> index 3b73b442..38bee317 100644\n> --- a/src/libcamera/base/utils.cpp\n> +++ b/src/libcamera/base/utils.cpp\n> @@ -521,6 +521,16 @@ double strtod(const char *__restrict nptr, char **__restrict endptr)\n> #endif\n> }\n> \n> +/**\n> + * \\fn R floatingToFixedPoint(T number)\n> + * \\brief Convert a floating point number to a fixed-point representation\n> + * \\tparam I Bit width of the integer part of the fixed-point\n> + * \\tparam F Bit width of the fractional part of the fixed-point\n> + * \\tparam R Return type of the fixed-point representation\n> + * \\tparam T Input type of the floating point representation\n> + * \\return The converted value\n> + */\n> +\n> } /* namespace utils */\n> \n> #ifndef __DOXYGEN__\n> -- \n> 2.39.2\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id DB8EABE08B\n\tfor ;\n\tMon, 15 Apr 2024 09:00:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C4BC763352;\n\tMon, 15 Apr 2024 11:00:34 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7A7C861B54\n\tfor ;\n\tMon, 15 Apr 2024 11:00:32 +0200 (CEST)","from pyrite.rasen.tech (h175-177-049-156.catv02.itscom.jp\n\t[175.177.49.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EEFB35B2\n\tfor ;\n\tMon, 15 Apr 2024 10:59:45 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ik2Sfros\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713171586;\n\tbh=Ti1yX5a6JgQvBVgWeszv66jJ1Aq1YLjnOit+pqTgtw0=;\n\th=Date:From:To:Subject:References:In-Reply-To:From;\n\tb=ik2SfrosW54BgdYD3vxy0vp2hG1qJagZsuNakT2j6kqy3e/q5pCUvAZTM4aFeElq7\n\tAzqh+99UTJRvwZ9VPPds/5GPb+SADinxtewWD5n3uIKRDl3XEjcy8v/grf/ua+oRHw\n\t3dd1BhsR5q/CcaJNH4ESKtU3kZ84pVrEWQuj/0nY=","Date":"Mon, 15 Apr 2024 18:00:23 +0900","From":"Paul Elder ","To":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: utils: Add a helper to convert floating-point\n\tto fixed-point","Message-ID":"","References":"<20240327085700.273232-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20240327085700.273232-1-paul.elder@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":29221,"web_url":"https://patchwork.libcamera.org/comment/29221/","msgid":"<20240415100338.nme2ulu2izqqetau@jasper>","date":"2024-04-15T10:03:38","subject":"Re: [PATCH] libcamera: utils: Add a helper to convert floating-point\n\tto fixed-point","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Paul,\n\nthank you for the patch.\n\nOn Mon, Apr 15, 2024 at 06:00:23PM +0900, Paul Elder wrote:\n> On Wed, Mar 27, 2024 at 05:57:00PM +0900, Paul Elder wrote:\n> > Add a helper function to convert floating-point numbers to fixed-point\n> > numbers.\n> > \n> > Signed-off-by: Paul Elder \n> > ---\n> > I originally needed this for rkisp1 crosstalk but then I realized it's\n> > more efficient to just have the tuning tool output fixed-point and have\n> > the IPA simply copy those values instead of calculating them on-the-fly.\n> > Still, I made this helper so if people think it's useful...\n> \n> Turns out we do need it now! So that we can have human-readable (or\n> make-sensable) CCMs in the tuning file for rkisp1.\n> \n> \n> Paul\n> \n> > ---\n> > include/libcamera/base/utils.h | 27 +++++++++++++++++++++++++++\n> > src/libcamera/base/utils.cpp | 10 ++++++++++\n> > 2 files changed, 37 insertions(+)\n> > \n> > diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h\n> > index 37d9af60..ab421cc7 100644\n> > --- a/include/libcamera/base/utils.h\n> > +++ b/include/libcamera/base/utils.h\n> > @@ -369,6 +369,33 @@ decltype(auto) abs_diff(const T &a, const T &b)\n> > \n> > double strtod(const char *__restrict nptr, char **__restrict endptr);\n> > \n> > +#ifndef __DOXYGEN__\n> > +template > +\t std::enable_if_t &&\n> > +\t\t\t std::is_floating_point_v> * = nullptr>\n> > +#else\n> > +template\n> > +#endif\n> > +constexpr R floatingToFixedPoint(T number)\n> > +{\n> > +\tstatic_assert(I + F <= sizeof(R) * 8);\n> > +\n> > +\tR integer = static_cast(number);\n> > +\tT fractional = number - integer;\n> > +\n> > +\tR mask = (1 << I) - 1;\n> > +\tR whole = (integer >= 0 ? integer : ~integer + 1) & mask;\n> > +\n> > +\tR frac = 0;\n> > +\tfor (unsigned int i = 0; i < F; i++) {\n> > +\t\tfractional *= 2;\n> > +\t\tfrac <<= 1;\n> > +\t\tfrac |= (static_cast(fractional) & 0x1);\n> > +\t}\n> > +\n> > +\treturn (whole << F) | frac;\n> > +}\n\nTo be honest I don't really get it. Why is that complexity needed? I\ndidn't try it out, but my naive appoach would be:\n\nreturn static_cast(std::round( number * (1 << F)))\n\nMaybe I'm missing something here.\n\nAs this ends up in the public includes, we should add a inverse function\nto convert fixed point to floating.\n\ne.g. \n\nreturn (static_castinput / static_cast(1 << F));\n\n\nI believe We should also add a simple testcase to ensure this doesn't\nbreak and produces the correct values.\n\nAre there commonly used fixed point formats? Maybe it would be helpful\nto add typedefs for these formats. Something like\n\nfloat2Fixed_11_5()\n\nas retyping these template params over and over might be error prone.\n(Maybe we could also leave that to the user of the function)\n\nBest regards,\nStefan\n\n> > +\n> > } /* namespace utils */\n> > \n> > #ifndef __DOXYGEN__\n> > diff --git a/src/libcamera/base/utils.cpp b/src/libcamera/base/utils.cpp\n> > index 3b73b442..38bee317 100644\n> > --- a/src/libcamera/base/utils.cpp\n> > +++ b/src/libcamera/base/utils.cpp\n> > @@ -521,6 +521,16 @@ double strtod(const char *__restrict nptr, char **__restrict endptr)\n> > #endif\n> > }\n> > \n> > +/**\n> > + * \\fn R floatingToFixedPoint(T number)\n> > + * \\brief Convert a floating point number to a fixed-point representation\n> > + * \\tparam I Bit width of the integer part of the fixed-point\n> > + * \\tparam F Bit width of the fractional part of the fixed-point\n> > + * \\tparam R Return type of the fixed-point representation\n> > + * \\tparam T Input type of the floating point representation\n> > + * \\return The converted value\n> > + */\n> > +\n> > } /* namespace utils */\n> > \n> > #ifndef __DOXYGEN__\n> > -- \n> > 2.39.2\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 22384C3213\n\tfor ;\n\tMon, 15 Apr 2024 10:03:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 098166333E;\n\tMon, 15 Apr 2024 12:03:42 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CFFA663339\n\tfor ;\n\tMon, 15 Apr 2024 12:03:40 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:7a0d:dd2e:881a:db83])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 13F88236;\n\tMon, 15 Apr 2024 12:02:55 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"LiSrgfmk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1713175375;\n\tbh=nqWbq3TU9F9C93sP5SqU2QTaPUy9wxvzOEPwuOoirvo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LiSrgfmkqlqU7JmleM/j+w4tdEtiSpSUai/CojgrRaDPJuceVTUrwjzyRsOt522C+\n\tNYPNq7Xo45Nful0tNcMTdyQd8R0npLBlK7xPFQffdJ2EtYmOT9Fd2pk227MWeK6cFV\n\ttXbBMbJ5e8XrXV4HCn4Y6Osn6d9NbS9/TpLXkEOQ=","Date":"Mon, 15 Apr 2024 12:03:38 +0200","From":"Stefan Klug ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH] libcamera: utils: Add a helper to convert floating-point\n\tto fixed-point","Message-ID":"<20240415100338.nme2ulu2izqqetau@jasper>","References":"<20240327085700.273232-1-paul.elder@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]