| Message ID | 20260213-kbingham-quantizers-v7-15-1626b9aaabf1@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Quoting Kieran Bingham (2026-02-14 01:57:54) > With all users of the floatingToFixedPoint and fixedToFloatingPoint > calls converted to use the FixedPoint Quantized types, remove the calls > and documentation and move the implementation inline in the > FixedPointTraits implementation. > > Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > v5: > - Reflow text when moved > > v6: > - Use UT{1} << F instead of 1 << F > --- > src/ipa/libipa/fixedpoint.cpp | 22 ------------- > src/ipa/libipa/fixedpoint.h | 75 +++++++++++++------------------------------ > 2 files changed, 22 insertions(+), 75 deletions(-) > > diff --git a/src/ipa/libipa/fixedpoint.cpp b/src/ipa/libipa/fixedpoint.cpp > index caa9ce0fc1ec..7eee3614df6a 100644 > --- a/src/ipa/libipa/fixedpoint.cpp > +++ b/src/ipa/libipa/fixedpoint.cpp > @@ -15,28 +15,6 @@ 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 > - */ > - > /** > * \struct libcamera::ipa::FixedPointQTraits > * \brief Traits type implementing fixed-point quantisation conversions > diff --git a/src/ipa/libipa/fixedpoint.h b/src/ipa/libipa/fixedpoint.h > index b6b611df7fc3..a0da31209271 100644 > --- a/src/ipa/libipa/fixedpoint.h > +++ b/src/ipa/libipa/fixedpoint.h > @@ -16,55 +16,6 @@ namespace libcamera { > > namespace ipa { > > -#ifndef __DOXYGEN__ > -template<unsigned int I, unsigned int F, typename R, typename T, > - std::enable_if_t<std::is_integral_v<R> && > - std::is_floating_point_v<T>> * = nullptr> > -#else > -template<unsigned int I, unsigned int F, typename R, typename T> > -#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<R>(static_cast<int>(std::round(number * (1 << F)))) & mask; > - > - return frac; > -} > - > -#ifndef __DOXYGEN__ > -template<unsigned int I, unsigned int F, typename R, typename T, > - std::enable_if_t<std::is_floating_point_v<R> && > - std::is_integral_v<T>> * = nullptr> > -#else > -template<unsigned int I, unsigned int F, typename R, typename T> > -#endif > -constexpr R fixedToFloatingPoint(T number) > -{ > - static_assert(sizeof(int) >= sizeof(T)); > - static_assert(I + F <= sizeof(T) * 8); > - > - if constexpr (std::is_unsigned_v<T>) > - return static_cast<R>(number) / static_cast<R>(T{ 1 } << F); > - > - /* > - * 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<int>(static_cast<unsigned>(number) << remaining_bits) >> remaining_bits; > - return static_cast<R>(t) / static_cast<R>(1 << F); > -} > - > template<unsigned int I, unsigned int F, typename T> > struct FixedPointQTraits { > private: > @@ -97,11 +48,23 @@ public: > > static constexpr float toFloat(QuantizedType q) > { > - return fixedToFloatingPoint<I, F, float, T>(q); > + if constexpr (std::is_unsigned_v<T>) > + return static_cast<float>(q) / static_cast<float>(UT{ 1 } << F); > + > + /* > + * 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. > + */ > + unsigned int remaining_bits = sizeof(UT) * 8 - (I + F); > + T t = static_cast<T>(static_cast<UT>(q) << remaining_bits) >> remaining_bits; > + return static_cast<float>(t) / static_cast<float>(UT{ 1 } << F); > } > > - static constexpr float min = fixedToFloatingPoint<I, F, float>(static_cast<T>(qMin)); > - static constexpr float max = fixedToFloatingPoint<I, F, float>(static_cast<T>(qMax)); > + static constexpr float min = toFloat(qMin); > + static constexpr float max = toFloat(qMax); > > static_assert(min < max, "FixedPointQTraits: Minimum must be less than maximum"); > > @@ -109,7 +72,13 @@ public: > static QuantizedType fromFloat(float v) > { > v = std::clamp(v, min, max); > - return floatingToFixedPoint<I, F, T, float>(v); > + > + /* > + * 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/ > + */ > + return static_cast<UT>(static_cast<T>(std::round(v * (1 << F)))) & bitMask; > } > }; > > > -- > 2.52.0 >
diff --git a/src/ipa/libipa/fixedpoint.cpp b/src/ipa/libipa/fixedpoint.cpp index caa9ce0fc1ec..7eee3614df6a 100644 --- a/src/ipa/libipa/fixedpoint.cpp +++ b/src/ipa/libipa/fixedpoint.cpp @@ -15,28 +15,6 @@ 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 - */ - /** * \struct libcamera::ipa::FixedPointQTraits * \brief Traits type implementing fixed-point quantisation conversions diff --git a/src/ipa/libipa/fixedpoint.h b/src/ipa/libipa/fixedpoint.h index b6b611df7fc3..a0da31209271 100644 --- a/src/ipa/libipa/fixedpoint.h +++ b/src/ipa/libipa/fixedpoint.h @@ -16,55 +16,6 @@ namespace libcamera { namespace ipa { -#ifndef __DOXYGEN__ -template<unsigned int I, unsigned int F, typename R, typename T, - std::enable_if_t<std::is_integral_v<R> && - std::is_floating_point_v<T>> * = nullptr> -#else -template<unsigned int I, unsigned int F, typename R, typename T> -#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<R>(static_cast<int>(std::round(number * (1 << F)))) & mask; - - return frac; -} - -#ifndef __DOXYGEN__ -template<unsigned int I, unsigned int F, typename R, typename T, - std::enable_if_t<std::is_floating_point_v<R> && - std::is_integral_v<T>> * = nullptr> -#else -template<unsigned int I, unsigned int F, typename R, typename T> -#endif -constexpr R fixedToFloatingPoint(T number) -{ - static_assert(sizeof(int) >= sizeof(T)); - static_assert(I + F <= sizeof(T) * 8); - - if constexpr (std::is_unsigned_v<T>) - return static_cast<R>(number) / static_cast<R>(T{ 1 } << F); - - /* - * 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<int>(static_cast<unsigned>(number) << remaining_bits) >> remaining_bits; - return static_cast<R>(t) / static_cast<R>(1 << F); -} - template<unsigned int I, unsigned int F, typename T> struct FixedPointQTraits { private: @@ -97,11 +48,23 @@ public: static constexpr float toFloat(QuantizedType q) { - return fixedToFloatingPoint<I, F, float, T>(q); + if constexpr (std::is_unsigned_v<T>) + return static_cast<float>(q) / static_cast<float>(UT{ 1 } << F); + + /* + * 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. + */ + unsigned int remaining_bits = sizeof(UT) * 8 - (I + F); + T t = static_cast<T>(static_cast<UT>(q) << remaining_bits) >> remaining_bits; + return static_cast<float>(t) / static_cast<float>(UT{ 1 } << F); } - static constexpr float min = fixedToFloatingPoint<I, F, float>(static_cast<T>(qMin)); - static constexpr float max = fixedToFloatingPoint<I, F, float>(static_cast<T>(qMax)); + static constexpr float min = toFloat(qMin); + static constexpr float max = toFloat(qMax); static_assert(min < max, "FixedPointQTraits: Minimum must be less than maximum"); @@ -109,7 +72,13 @@ public: static QuantizedType fromFloat(float v) { v = std::clamp(v, min, max); - return floatingToFixedPoint<I, F, T, float>(v); + + /* + * 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/ + */ + return static_cast<UT>(static_cast<T>(std::round(v * (1 << F)))) & bitMask; } };