libcamera: utils: Add a helper to convert floating-point to fixed-point
diff mbox series

Message ID 20240327085700.273232-1-paul.elder@ideasonboard.com
State Superseded
Headers show
Series
  • libcamera: utils: Add a helper to convert floating-point to fixed-point
Related show

Commit Message

Paul Elder March 27, 2024, 8:57 a.m. UTC 
Add a helper function to convert floating-point numbers to fixed-point
numbers.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
I originally needed this for rkisp1 crosstalk but then I realized it's
more efficient to just have the tuning tool output fixed-point and have
the IPA simply copy those values instead of calculating them on-the-fly.
Still, I made this helper so if people think it's useful...
---
 include/libcamera/base/utils.h | 27 +++++++++++++++++++++++++++
 src/libcamera/base/utils.cpp   | 10 ++++++++++
 2 files changed, 37 insertions(+)

Comments

Paul Elder April 15, 2024, 9 a.m. UTC | #1 
On Wed, Mar 27, 2024 at 05:57:00PM +0900, Paul Elder wrote:
> Add a helper function to convert floating-point numbers to fixed-point
> numbers.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
> I originally needed this for rkisp1 crosstalk but then I realized it's
> more efficient to just have the tuning tool output fixed-point and have
> the IPA simply copy those values instead of calculating them on-the-fly.
> Still, I made this helper so if people think it's useful...

Turns out we do need it now! So that we can have human-readable (or
make-sensable) CCMs in the tuning file for rkisp1.


Paul

> ---
>  include/libcamera/base/utils.h | 27 +++++++++++++++++++++++++++
>  src/libcamera/base/utils.cpp   | 10 ++++++++++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h
> index 37d9af60..ab421cc7 100644
> --- a/include/libcamera/base/utils.h
> +++ b/include/libcamera/base/utils.h
> @@ -369,6 +369,33 @@ decltype(auto) abs_diff(const T &a, const T &b)
>  
>  double strtod(const char *__restrict nptr, char **__restrict endptr);
>  
> +#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(I + F <= sizeof(R) * 8);
> +
> +	R integer = static_cast<R>(number);
> +	T fractional = number - integer;
> +
> +	R mask = (1 << I) - 1;
> +	R whole = (integer >= 0 ? integer : ~integer + 1) & mask;
> +
> +	R frac = 0;
> +	for (unsigned int i = 0; i < F; i++) {
> +		fractional *= 2;
> +		frac <<= 1;
> +		frac |= (static_cast<R>(fractional) & 0x1);
> +	}
> +
> +	return (whole << F) | frac;
> +}
> +
>  } /* namespace utils */
>  
>  #ifndef __DOXYGEN__
> diff --git a/src/libcamera/base/utils.cpp b/src/libcamera/base/utils.cpp
> index 3b73b442..38bee317 100644
> --- a/src/libcamera/base/utils.cpp
> +++ b/src/libcamera/base/utils.cpp
> @@ -521,6 +521,16 @@ double strtod(const char *__restrict nptr, char **__restrict endptr)
>  #endif
>  }
>  
> +/**
> + * \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
> + * \return The converted value
> + */
> +
>  } /* namespace utils */
>  
>  #ifndef __DOXYGEN__
> -- 
> 2.39.2
>
Stefan Klug April 15, 2024, 10:03 a.m. UTC | #2 
Hi Paul,

thank you for the patch.

On Mon, Apr 15, 2024 at 06:00:23PM +0900, Paul Elder wrote:
> On Wed, Mar 27, 2024 at 05:57:00PM +0900, Paul Elder wrote:
> > Add a helper function to convert floating-point numbers to fixed-point
> > numbers.
> > 
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > ---
> > I originally needed this for rkisp1 crosstalk but then I realized it's
> > more efficient to just have the tuning tool output fixed-point and have
> > the IPA simply copy those values instead of calculating them on-the-fly.
> > Still, I made this helper so if people think it's useful...
> 
> Turns out we do need it now! So that we can have human-readable (or
> make-sensable) CCMs in the tuning file for rkisp1.
> 
> 
> Paul
> 
> > ---
> >  include/libcamera/base/utils.h | 27 +++++++++++++++++++++++++++
> >  src/libcamera/base/utils.cpp   | 10 ++++++++++
> >  2 files changed, 37 insertions(+)
> > 
> > diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h
> > index 37d9af60..ab421cc7 100644
> > --- a/include/libcamera/base/utils.h
> > +++ b/include/libcamera/base/utils.h
> > @@ -369,6 +369,33 @@ decltype(auto) abs_diff(const T &a, const T &b)
> >  
> >  double strtod(const char *__restrict nptr, char **__restrict endptr);
> >  
> > +#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(I + F <= sizeof(R) * 8);
> > +
> > +	R integer = static_cast<R>(number);
> > +	T fractional = number - integer;
> > +
> > +	R mask = (1 << I) - 1;
> > +	R whole = (integer >= 0 ? integer : ~integer + 1) & mask;
> > +
> > +	R frac = 0;
> > +	for (unsigned int i = 0; i < F; i++) {
> > +		fractional *= 2;
> > +		frac <<= 1;
> > +		frac |= (static_cast<R>(fractional) & 0x1);
> > +	}
> > +
> > +	return (whole << F) | frac;
> > +}

To be honest I don't really get it. Why is that complexity needed? I
didn't try it out, but my naive appoach would be:

return static_cast<R>(std::round( number * (1 << F)))

Maybe I'm missing something here.

As this ends up in the public includes, we should add a inverse function
to convert fixed point to floating.

e.g. 

return (static_cast<double>input / static_cast<double>(1 << F));


I believe We should also add a simple testcase to ensure this doesn't
break and produces the correct values.

Are there commonly used fixed point formats? Maybe it would be helpful
to add typedefs for these formats. Something like

float2Fixed_11_5()

as retyping these template params over and over might be error prone.
(Maybe we could also leave that to the user of the function)

Best regards,
Stefan

> > +
> >  } /* namespace utils */
> >  
> >  #ifndef __DOXYGEN__
> > diff --git a/src/libcamera/base/utils.cpp b/src/libcamera/base/utils.cpp
> > index 3b73b442..38bee317 100644
> > --- a/src/libcamera/base/utils.cpp
> > +++ b/src/libcamera/base/utils.cpp
> > @@ -521,6 +521,16 @@ double strtod(const char *__restrict nptr, char **__restrict endptr)
> >  #endif
> >  }
> >  
> > +/**
> > + * \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
> > + * \return The converted value
> > + */
> > +
> >  } /* namespace utils */
> >  
> >  #ifndef __DOXYGEN__
> > -- 
> > 2.39.2
> >

Patch
diff mbox series 

diff --git a/include/libcamera/base/utils.h b/include/libcamera/base/utils.h
index 37d9af60..ab421cc7 100644
--- a/include/libcamera/base/utils.h
+++ b/include/libcamera/base/utils.h
@@ -369,6 +369,33 @@  decltype(auto) abs_diff(const T &a, const T &b)
 
 double strtod(const char *__restrict nptr, char **__restrict endptr);
 
+#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(I + F <= sizeof(R) * 8);
+
+	R integer = static_cast<R>(number);
+	T fractional = number - integer;
+
+	R mask = (1 << I) - 1;
+	R whole = (integer >= 0 ? integer : ~integer + 1) & mask;
+
+	R frac = 0;
+	for (unsigned int i = 0; i < F; i++) {
+		fractional *= 2;
+		frac <<= 1;
+		frac |= (static_cast<R>(fractional) & 0x1);
+	}
+
+	return (whole << F) | frac;
+}
+
 } /* namespace utils */
 
 #ifndef __DOXYGEN__
diff --git a/src/libcamera/base/utils.cpp b/src/libcamera/base/utils.cpp
index 3b73b442..38bee317 100644
--- a/src/libcamera/base/utils.cpp
+++ b/src/libcamera/base/utils.cpp
@@ -521,6 +521,16 @@  double strtod(const char *__restrict nptr, char **__restrict endptr)
 #endif
 }
 
+/**
+ * \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
+ * \return The converted value
+ */
+
 } /* namespace utils */
 
 #ifndef __DOXYGEN__