| Message ID | 20251026233048.175689-2-kieran.bingham@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi 2025. 10. 27. 0:30 keltezéssel, Kieran Bingham írta: > Frequently when handling data in IPA components we must convert and > store user interface values which may be floating point values, and > perform a specific operation or conversion to quantize this to a > hardware value. > > This value may be to a fixed point type, or more custom code mappings, > but in either case it is important to contain both the required hardware > value, with it's effective quantized value. it's -> its ? > > Provide a new storage type 'Quantized' which can be read publicly but > must only be written through a controller class known as a Quantizer. > > Quantizers can be customised and specified by the owner of the data > object and must implement conversion between floats and the underlying > hardware type. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/ipa/libipa/meson.build | 2 + > src/ipa/libipa/quantized.cpp | 167 +++++++++++++++++++++++++++++++++++ > src/ipa/libipa/quantized.h | 90 +++++++++++++++++++ > 3 files changed, 259 insertions(+) > create mode 100644 src/ipa/libipa/quantized.cpp > create mode 100644 src/ipa/libipa/quantized.h > > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build > index 660be94054fa..804289778f72 100644 > --- a/src/ipa/libipa/meson.build > +++ b/src/ipa/libipa/meson.build > @@ -17,6 +17,7 @@ libipa_headers = files([ > 'lux.h', > 'module.h', > 'pwl.h', > + 'quantized.h', > ]) > > libipa_sources = files([ > @@ -36,6 +37,7 @@ libipa_sources = files([ > 'lux.cpp', > 'module.cpp', > 'pwl.cpp', > + 'quantized.cpp', > ]) > > libipa_includes = include_directories('..') > diff --git a/src/ipa/libipa/quantized.cpp b/src/ipa/libipa/quantized.cpp > new file mode 100644 > index 000000000000..0078c6f740a9 > --- /dev/null > +++ b/src/ipa/libipa/quantized.cpp > @@ -0,0 +1,167 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2025, Ideas On Board. > + * > + * Helper class to manage conversions between floating point types and quantized > + * storage and representation of those values. > + */ > + > +#include "quantized.h" > + > +/** > + * \file quantized.h > + * \brief Quantized storage and Quantizer representations > + */ > + > +namespace libcamera { > + > +namespace ipa { > + > +/** > + * \struct Quantized > + * \brief Quantized stores both an internal and quantized representation of a value > + * \tparam T The quantized type (e.g., uint16_t, int16_t). > + * > + * This struct provides read-only access to both representations of the value. > + * It does not perform any conversions or quantization logic itself. This must > + * be handled externally by a quantizer that knows the appropriate behaviour. > + * > + * This type can be used to store values that need to be represented in both > + * fixed-point or integer (for hardware interfaces) and floating-point (for user > + * facing interfaces) > + * > + * It is intended to be used in shared context structures where both > + * hardware-level and software-level representations of a value are required. > + */ > + > +/** > + * \fn float Quantized::value() const noexcept > + * \brief Get the floating-point representation of the quantized value > + * > + * This function returns the floating-point form that was either directly assigned > + * or computed from the quantized value. > + * > + * \return The floating-point value > + */ > + > +/** > + * \fn T Quantized::quantized() const noexcept > + * \brief Get the raw quantized representation of the value > + * > + * This function returns the internal quantized value (e.g. \c uint8_t or \c int16_t), > + * which is typically used for hardware-level programming or serialization. > + * > + * \return The quantized value of type \c T > + */ > + > +/** > + * \fn std::string Quantized::toString() const > + * \brief Generate a string representation of the quantized and float values > + * > + * Produces a human-readable string including both the quantized and floating-point > + * values, typically in the format: "Q:0xXX F:float". > + * > + * \return A formatted string representing the internal state > + */ > + > +/** > + * \fn bool Quantized::operator==(const Quantized<T> &other) const noexcept > + * \brief Compare two quantized values for equality > + * > + * Two \c Quantized<T> objects are equal if their quantized values are equal. > + * > + * \param other The \c Quantized<T> to compare with > + * \return \c true if equal, \c false otherwise > + */ > + > +/** > + * \fn bool Quantized::operator!=(const Quantized<T> &other) const noexcept > + * \brief Compare two quantized values for inequality > + * > + * Two \c Quantized<T> objects are not equal if their quantized values differ. > + * > + * \param other The \c Quantized<T> to compare with > + * \return \c true if not equal, \c false otherwise > + */ > + > +/** > + * \var Quantized::quantized_ > + * \brief The raw quantized value > + * > + * This member stores the fixed-point or integer representation of the value, typically > + * used for interfacing with hardware or protocols that require compact formats. > + * > + * It must only be updated or accessed through a Quantizer in conjunction with > + * corresponding updates to \c Quantized::value_. > + */ > + > +/** > + * \var Quantized::value_ > + * \brief The floating-point representation of the quantized value > + * > + * This member holds the floating-point equivalent of the quantized value, usually > + * for use in calculations or presentation to higher-level software components. > + * > + * It must only be updated or accessed through a Quantizer in conjunction with > + * corresponding updates to \c Quantized::quantized_. > + */ > + > +/** > + * \class Quantizer > + * \brief Interface for converting between floating-point and quantized types > + * \tparam T The quantized type (e.g., uint16_t, int16_t). > + * > + * This abstract class defines the interface for quantizers that handle > + * conversions between floating-point values and their quantized representations. > + * Specific quantization handlers should implement this interface and provide > + * the toFloat and fromFloat methods specifically for their types. > + */ > + > +/** > + * \typedef Quantizer::qType > + * \brief The underlying quantized type used by the quantizer > + * > + * This alias exposes the quantized type (e.g. \c uint8_t or \c int16_t) used internally > + * by the \c Quantizer, which is helpful in template and debug contexts. > + */ > + > +/** > + * \fn Quantizer::fromFloat(T val) > + * \brief Convert a floating-point value to its quantized representation > + * > + * \param val The floating-point value > + * \return The quantized equivalent > + */ > + > +/** > + * \fn Quantizer::toFloat(T val) > + * \brief Convert a quantized value to its floating-point representation > + * > + * \param val The quantized value > + * \return The floating-point equivalent > + */ > + > +/** > + * \fn Quantizer::set(float val) > + * \brief Set the value of a Quantized<T> using a floating-point input > + * > + * This function updates both the quantized and floating-point > + * representations stored in the Quantized<T>, storing the quantized > + * version of the float input. > + * > + * \param val The floating-point value to set > + */ > + > +/** > + * \fn Quantizer::set(T val) > + * \brief Set the value of a Quantized<T> using a quantized input > + * > + * This function updates both the quantized and floating-point > + * representations stored in the Quantized<T>. > + * > + * \param val The quantized value to set > + */ > + > +} /* namespace ipa */ > + > +} /* namespace libcamera */ > diff --git a/src/ipa/libipa/quantized.h b/src/ipa/libipa/quantized.h > new file mode 100644 > index 000000000000..1c1963cf0848 > --- /dev/null > +++ b/src/ipa/libipa/quantized.h > @@ -0,0 +1,90 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2025, Ideas On Board. > + * > + * Helper class to manage conversions between floating point types and quantized > + * storage and representation of those values. > + */ > + > +#pragma once > + > +#include <iomanip> > +#include <sstream> > +#include <stdint.h> > +#include <type_traits> > + > +namespace libcamera { > + > +namespace ipa { > + > +template<typename Q> > +class Quantizer; > + > +template<typename T> > +struct Quantized { > + static_assert(std::is_arithmetic_v<T>, "Quantized: T must be an arithmetic type"); > + > + float value() const noexcept { return value_; } > + T quantized() const noexcept { return quantized_; } > + > + std::string toString() const I think I would implement `operator<<`. > + { > + using UT = std::make_unsigned_t<T>; > + std::ostringstream oss; > + > + oss << "Q:0x" << std::hex << std::uppercase << std::setw(sizeof(T) * 2) > + << std::setfill('0'); > + > + if constexpr (std::is_unsigned_v<T>) { > + oss << static_cast<unsigned>(quantized_); > + } else { > + oss << static_cast<unsigned>(static_cast<UT>(quantized_)); I think you can drop the `if` and keep only this second branch, it should work the same as far as I can see. Also, since you cast to `unsigned`, there should probably be a static assert ensuring that the value fits. I'm guessing it's done to handle `{u,}int8_t`? In that case one could do oss << +static_cast<UT>(quantized_); to ensure that they are promoted to `int` and not printed as characters, and then there is no need to cast to `unsigned`. > + } > + > + oss << " F:" << value_; > + > + return oss.str(); > + } > + > + bool operator==(const Quantized<T> &other) const noexcept You can drop the `<T>` part as it is unnecessary inside the definition. > + { > + return quantized_ == other.quantized_ && value_ == other.value_; I worry a bit about floating point comparisons with `==`. And if `value_` is the same, shouldn't that imply that `quantized_` is as well? > + } > + > + bool operator!=(const Quantized<T> &other) const noexcept > + { > + return !(*this == other); > + } > + > +protected: > + T quantized_{}; > + float value_{}; > +}; > + > +template<typename T> > +class Quantizer : public Quantized<T> > +{ > +public: > + using qType = T; > + > + virtual ~Quantizer() = default; > + > + virtual T fromFloat(float val) const = 0; > + virtual float toFloat(T val) const = 0; I'm looking at the use cases, and I'm not sure that these need to be `virtual` in the first place. In fact, I think I'd completely eliminate the `Quantizer` class: template<typename Traits> struct Quantized { using quantized_type = typename Traits::quantized_type; Quantized() : Quantized(0.0f) { } Quantized(float x) { *this = x; } Quantized(quantized_type x) { *this = x; } Quantized& operator=(float x) { quantized_ = Traits::fromFloat(x); value_ = x; return *this; } Quantized& operator=(quantized_type x) { value_ = Traits::toFloat(x); quantized_ = x; return *this; } /* most others stay the same */ private: quantized_type quantized_; float value_; }; /* ... */ struct RkISP1HueQTraits { using quantized_type = int8_t; static quantized_type fromFloat(float v) { ... } static float toFloat(quantized_type v) { ... } }; /* ... */ Quantized<RkISP1HueQTraits> hue = ...; This has advantages in my opinion: * the default initialization will always be correct * currently it is assumed that 0.0f maps to "0" of the quantized type * type checking: you will not be able to assign/compare/etc. quantized quantities with different trait types * virtual functions are eliminated * if the traits type implements the methods inline, then those can be directly inlined, which is most likely worth it if it's just a simple division or similar It is entirely possible that I am missing a use case that wouldn't work with the above, so I'm wondering if you have looked at an approach similar to the above? Regards, Barnabás Pőcze > + > + void set(float val) > + { > + this->quantized_ = fromFloat(val); > + this->value_ = toFloat(this->quantized_); > + } > + > + void set(T val) > + { > + this->quantized_ = val; > + this->value_ = toFloat(val); > + } > +}; > + > +} /* namespace ipa */ > + > +} /* namespace libcamera */ > -- > 2.51.0 >
On Mon, Oct 27, 2025 at 10:37:21AM +0100, Barnabás Pőcze wrote: > Hi > > 2025. 10. 27. 0:30 keltezéssel, Kieran Bingham írta: > > Frequently when handling data in IPA components we must convert and > > store user interface values which may be floating point values, and > > perform a specific operation or conversion to quantize this to a > > hardware value. > > > > This value may be to a fixed point type, or more custom code mappings, > > but in either case it is important to contain both the required hardware > > value, with it's effective quantized value. > > it's -> its ? > > > > > > Provide a new storage type 'Quantized' which can be read publicly but > > must only be written through a controller class known as a Quantizer. > > > > Quantizers can be customised and specified by the owner of the data > > object and must implement conversion between floats and the underlying > > hardware type. > > > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > --- > > src/ipa/libipa/meson.build | 2 + > > src/ipa/libipa/quantized.cpp | 167 +++++++++++++++++++++++++++++++++++ > > src/ipa/libipa/quantized.h | 90 +++++++++++++++++++ > > 3 files changed, 259 insertions(+) > > create mode 100644 src/ipa/libipa/quantized.cpp > > create mode 100644 src/ipa/libipa/quantized.h > > > > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build > > index 660be94054fa..804289778f72 100644 > > --- a/src/ipa/libipa/meson.build > > +++ b/src/ipa/libipa/meson.build > > @@ -17,6 +17,7 @@ libipa_headers = files([ > > 'lux.h', > > 'module.h', > > 'pwl.h', > > + 'quantized.h', > > ]) > > > > libipa_sources = files([ > > @@ -36,6 +37,7 @@ libipa_sources = files([ > > 'lux.cpp', > > 'module.cpp', > > 'pwl.cpp', > > + 'quantized.cpp', > > ]) > > > > libipa_includes = include_directories('..') > > diff --git a/src/ipa/libipa/quantized.cpp b/src/ipa/libipa/quantized.cpp > > new file mode 100644 > > index 000000000000..0078c6f740a9 > > --- /dev/null > > +++ b/src/ipa/libipa/quantized.cpp > > @@ -0,0 +1,167 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2025, Ideas On Board. > > + * > > + * Helper class to manage conversions between floating point types and quantized > > + * storage and representation of those values. > > + */ > > + > > +#include "quantized.h" > > + > > +/** > > + * \file quantized.h > > + * \brief Quantized storage and Quantizer representations > > + */ > > + > > +namespace libcamera { > > + > > +namespace ipa { > > + > > +/** > > + * \struct Quantized > > + * \brief Quantized stores both an internal and quantized representation of a value > > + * \tparam T The quantized type (e.g., uint16_t, int16_t). > > + * > > + * This struct provides read-only access to both representations of the value. > > + * It does not perform any conversions or quantization logic itself. This must > > + * be handled externally by a quantizer that knows the appropriate behaviour. > > + * > > + * This type can be used to store values that need to be represented in both > > + * fixed-point or integer (for hardware interfaces) and floating-point (for user > > + * facing interfaces) > > + * > > + * It is intended to be used in shared context structures where both > > + * hardware-level and software-level representations of a value are required. > > + */ > > + > > +/** > > + * \fn float Quantized::value() const noexcept > > + * \brief Get the floating-point representation of the quantized value > > + * > > + * This function returns the floating-point form that was either directly assigned > > + * or computed from the quantized value. > > + * > > + * \return The floating-point value > > + */ > > + > > +/** > > + * \fn T Quantized::quantized() const noexcept > > + * \brief Get the raw quantized representation of the value > > + * > > + * This function returns the internal quantized value (e.g. \c uint8_t or \c int16_t), > > + * which is typically used for hardware-level programming or serialization. > > + * > > + * \return The quantized value of type \c T > > + */ > > + > > +/** > > + * \fn std::string Quantized::toString() const > > + * \brief Generate a string representation of the quantized and float values > > + * > > + * Produces a human-readable string including both the quantized and floating-point > > + * values, typically in the format: "Q:0xXX F:float". > > + * > > + * \return A formatted string representing the internal state > > + */ > > + > > +/** > > + * \fn bool Quantized::operator==(const Quantized<T> &other) const noexcept > > + * \brief Compare two quantized values for equality > > + * > > + * Two \c Quantized<T> objects are equal if their quantized values are equal. > > + * > > + * \param other The \c Quantized<T> to compare with > > + * \return \c true if equal, \c false otherwise > > + */ > > + > > +/** > > + * \fn bool Quantized::operator!=(const Quantized<T> &other) const noexcept > > + * \brief Compare two quantized values for inequality > > + * > > + * Two \c Quantized<T> objects are not equal if their quantized values differ. > > + * > > + * \param other The \c Quantized<T> to compare with > > + * \return \c true if not equal, \c false otherwise > > + */ > > + > > +/** > > + * \var Quantized::quantized_ > > + * \brief The raw quantized value > > + * > > + * This member stores the fixed-point or integer representation of the value, typically > > + * used for interfacing with hardware or protocols that require compact formats. > > + * > > + * It must only be updated or accessed through a Quantizer in conjunction with > > + * corresponding updates to \c Quantized::value_. > > + */ > > + > > +/** > > + * \var Quantized::value_ > > + * \brief The floating-point representation of the quantized value > > + * > > + * This member holds the floating-point equivalent of the quantized value, usually > > + * for use in calculations or presentation to higher-level software components. > > + * > > + * It must only be updated or accessed through a Quantizer in conjunction with > > + * corresponding updates to \c Quantized::quantized_. > > + */ > > + > > +/** > > + * \class Quantizer > > + * \brief Interface for converting between floating-point and quantized types > > + * \tparam T The quantized type (e.g., uint16_t, int16_t). > > + * > > + * This abstract class defines the interface for quantizers that handle > > + * conversions between floating-point values and their quantized representations. > > + * Specific quantization handlers should implement this interface and provide > > + * the toFloat and fromFloat methods specifically for their types. > > + */ > > + > > +/** > > + * \typedef Quantizer::qType > > + * \brief The underlying quantized type used by the quantizer > > + * > > + * This alias exposes the quantized type (e.g. \c uint8_t or \c int16_t) used internally > > + * by the \c Quantizer, which is helpful in template and debug contexts. > > + */ > > + > > +/** > > + * \fn Quantizer::fromFloat(T val) > > + * \brief Convert a floating-point value to its quantized representation > > + * > > + * \param val The floating-point value > > + * \return The quantized equivalent > > + */ > > + > > +/** > > + * \fn Quantizer::toFloat(T val) > > + * \brief Convert a quantized value to its floating-point representation > > + * > > + * \param val The quantized value > > + * \return The floating-point equivalent > > + */ > > + > > +/** > > + * \fn Quantizer::set(float val) > > + * \brief Set the value of a Quantized<T> using a floating-point input > > + * > > + * This function updates both the quantized and floating-point > > + * representations stored in the Quantized<T>, storing the quantized > > + * version of the float input. > > + * > > + * \param val The floating-point value to set > > + */ > > + > > +/** > > + * \fn Quantizer::set(T val) > > + * \brief Set the value of a Quantized<T> using a quantized input > > + * > > + * This function updates both the quantized and floating-point > > + * representations stored in the Quantized<T>. > > + * > > + * \param val The quantized value to set > > + */ > > + > > +} /* namespace ipa */ > > + > > +} /* namespace libcamera */ > > diff --git a/src/ipa/libipa/quantized.h b/src/ipa/libipa/quantized.h > > new file mode 100644 > > index 000000000000..1c1963cf0848 > > --- /dev/null > > +++ b/src/ipa/libipa/quantized.h > > @@ -0,0 +1,90 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2025, Ideas On Board. > > + * > > + * Helper class to manage conversions between floating point types and quantized > > + * storage and representation of those values. > > + */ > > + > > +#pragma once > > + > > +#include <iomanip> > > +#include <sstream> > > +#include <stdint.h> > > +#include <type_traits> > > + > > +namespace libcamera { > > + > > +namespace ipa { > > + > > +template<typename Q> > > +class Quantizer; > > + > > +template<typename T> > > +struct Quantized { > > + static_assert(std::is_arithmetic_v<T>, "Quantized: T must be an arithmetic type"); > > + > > + float value() const noexcept { return value_; } > > + T quantized() const noexcept { return quantized_; } > > + > > + std::string toString() const > > I think I would implement `operator<<`. > > > > + { > > + using UT = std::make_unsigned_t<T>; > > + std::ostringstream oss; > > + > > + oss << "Q:0x" << std::hex << std::uppercase << std::setw(sizeof(T) * 2) > > + << std::setfill('0'); > > + > > + if constexpr (std::is_unsigned_v<T>) { > > + oss << static_cast<unsigned>(quantized_); > > + } else { > > + oss << static_cast<unsigned>(static_cast<UT>(quantized_)); > > I think you can drop the `if` and keep only this second branch, > it should work the same as far as I can see. > > Also, since you cast to `unsigned`, there should probably be a > static assert ensuring that the value fits. I'm guessing it's > done to handle `{u,}int8_t`? In that case one could do > > oss << +static_cast<UT>(quantized_); > > to ensure that they are promoted to `int` and not printed as characters, > and then there is no need to cast to `unsigned`. > > > > + } > > + > > + oss << " F:" << value_; > > + > > + return oss.str(); > > + } > > + > > + bool operator==(const Quantized<T> &other) const noexcept > > You can drop the `<T>` part as it is unnecessary inside the definition. > > > > + { > > + return quantized_ == other.quantized_ && value_ == other.value_; > > I worry a bit about floating point comparisons with `==`. And if `value_` is the > same, shouldn't that imply that `quantized_` is as well? > > > > + } > > + > > + bool operator!=(const Quantized<T> &other) const noexcept > > + { > > + return !(*this == other); > > + } > > + > > +protected: > > + T quantized_{}; > > + float value_{}; > > +}; > > + > > +template<typename T> > > +class Quantizer : public Quantized<T> > > +{ > > +public: > > + using qType = T; > > + > > + virtual ~Quantizer() = default; > > + > > + virtual T fromFloat(float val) const = 0; > > + virtual float toFloat(T val) const = 0; > > I'm looking at the use cases, and I'm not sure that these need to be `virtual` in > the first place. In fact, I think I'd completely eliminate the `Quantizer` class: > > template<typename Traits> > struct Quantized { > using quantized_type = typename Traits::quantized_type; > > Quantized() : Quantized(0.0f) { } > Quantized(float x) { *this = x; } > Quantized(quantized_type x) { *this = x; } > Quantized& operator=(float x) { quantized_ = Traits::fromFloat(x); value_ = x; return *this; } > Quantized& operator=(quantized_type x) { value_ = Traits::toFloat(x); quantized_ = x; return *this; } > > /* most others stay the same */ > > private: > quantized_type quantized_; > float value_; > }; > > /* ... */ > > struct RkISP1HueQTraits { > using quantized_type = int8_t; > static quantized_type fromFloat(float v) { ... } > static float toFloat(quantized_type v) { ... } > }; > > /* ... */ > > Quantized<RkISP1HueQTraits> hue = ...; > > > This has advantages in my opinion: > > * the default initialization will always be correct > * currently it is assumed that 0.0f maps to "0" of the quantized type > * type checking: you will not be able to assign/compare/etc. quantized quantities > with different trait types > * virtual functions are eliminated > * if the traits type implements the methods inline, then those can be > directly inlined, which is most likely worth it if it's just a simple division or similar Object slicing is also eliminated. > It is entirely possible that I am missing a use case that wouldn't work with > the above, so I'm wondering if you have looked at an approach similar to the above? > > Regards, > Barnabás Pőcze > > > > + > > + void set(float val) > > + { > > + this->quantized_ = fromFloat(val); > > + this->value_ = toFloat(this->quantized_); > > + } > > + > > + void set(T val) > > + { > > + this->quantized_ = val; > > + this->value_ = toFloat(val); > > + } > > +}; > > + > > +} /* namespace ipa */ > > + > > +} /* namespace libcamera */
Quoting Laurent Pinchart (2025-10-27 09:40:41) > On Mon, Oct 27, 2025 at 10:37:21AM +0100, Barnabás Pőcze wrote: > > Hi > > > > 2025. 10. 27. 0:30 keltezéssel, Kieran Bingham írta: > > > Frequently when handling data in IPA components we must convert and > > > store user interface values which may be floating point values, and > > > perform a specific operation or conversion to quantize this to a > > > hardware value. > > > > > > This value may be to a fixed point type, or more custom code mappings, > > > but in either case it is important to contain both the required hardware > > > value, with it's effective quantized value. > > > > it's -> its ? Ack, > > > > > > > > > > Provide a new storage type 'Quantized' which can be read publicly but > > > must only be written through a controller class known as a Quantizer. > > > > > > Quantizers can be customised and specified by the owner of the data > > > object and must implement conversion between floats and the underlying > > > hardware type. > > > > > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > --- > > > src/ipa/libipa/meson.build | 2 + > > > src/ipa/libipa/quantized.cpp | 167 +++++++++++++++++++++++++++++++++++ > > > src/ipa/libipa/quantized.h | 90 +++++++++++++++++++ > > > 3 files changed, 259 insertions(+) > > > create mode 100644 src/ipa/libipa/quantized.cpp > > > create mode 100644 src/ipa/libipa/quantized.h > > > > > > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build > > > index 660be94054fa..804289778f72 100644 > > > --- a/src/ipa/libipa/meson.build > > > +++ b/src/ipa/libipa/meson.build > > > @@ -17,6 +17,7 @@ libipa_headers = files([ > > > 'lux.h', > > > 'module.h', > > > 'pwl.h', > > > + 'quantized.h', > > > ]) > > > > > > libipa_sources = files([ > > > @@ -36,6 +37,7 @@ libipa_sources = files([ > > > 'lux.cpp', > > > 'module.cpp', > > > 'pwl.cpp', > > > + 'quantized.cpp', > > > ]) > > > > > > libipa_includes = include_directories('..') > > > diff --git a/src/ipa/libipa/quantized.cpp b/src/ipa/libipa/quantized.cpp > > > new file mode 100644 > > > index 000000000000..0078c6f740a9 > > > --- /dev/null > > > +++ b/src/ipa/libipa/quantized.cpp > > > @@ -0,0 +1,167 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2025, Ideas On Board. > > > + * > > > + * Helper class to manage conversions between floating point types and quantized > > > + * storage and representation of those values. > > > + */ > > > + > > > +#include "quantized.h" > > > + > > > +/** > > > + * \file quantized.h > > > + * \brief Quantized storage and Quantizer representations > > > + */ > > > + > > > +namespace libcamera { > > > + > > > +namespace ipa { > > > + > > > +/** > > > + * \struct Quantized > > > + * \brief Quantized stores both an internal and quantized representation of a value > > > + * \tparam T The quantized type (e.g., uint16_t, int16_t). > > > + * > > > + * This struct provides read-only access to both representations of the value. > > > + * It does not perform any conversions or quantization logic itself. This must > > > + * be handled externally by a quantizer that knows the appropriate behaviour. > > > + * > > > + * This type can be used to store values that need to be represented in both > > > + * fixed-point or integer (for hardware interfaces) and floating-point (for user > > > + * facing interfaces) > > > + * > > > + * It is intended to be used in shared context structures where both > > > + * hardware-level and software-level representations of a value are required. > > > + */ > > > + > > > +/** > > > + * \fn float Quantized::value() const noexcept > > > + * \brief Get the floating-point representation of the quantized value > > > + * > > > + * This function returns the floating-point form that was either directly assigned > > > + * or computed from the quantized value. > > > + * > > > + * \return The floating-point value > > > + */ > > > + > > > +/** > > > + * \fn T Quantized::quantized() const noexcept > > > + * \brief Get the raw quantized representation of the value > > > + * > > > + * This function returns the internal quantized value (e.g. \c uint8_t or \c int16_t), > > > + * which is typically used for hardware-level programming or serialization. > > > + * > > > + * \return The quantized value of type \c T > > > + */ > > > + > > > +/** > > > + * \fn std::string Quantized::toString() const > > > + * \brief Generate a string representation of the quantized and float values > > > + * > > > + * Produces a human-readable string including both the quantized and floating-point > > > + * values, typically in the format: "Q:0xXX F:float". > > > + * > > > + * \return A formatted string representing the internal state > > > + */ > > > + > > > +/** > > > + * \fn bool Quantized::operator==(const Quantized<T> &other) const noexcept > > > + * \brief Compare two quantized values for equality > > > + * > > > + * Two \c Quantized<T> objects are equal if their quantized values are equal. > > > + * > > > + * \param other The \c Quantized<T> to compare with > > > + * \return \c true if equal, \c false otherwise > > > + */ > > > + > > > +/** > > > + * \fn bool Quantized::operator!=(const Quantized<T> &other) const noexcept > > > + * \brief Compare two quantized values for inequality > > > + * > > > + * Two \c Quantized<T> objects are not equal if their quantized values differ. > > > + * > > > + * \param other The \c Quantized<T> to compare with > > > + * \return \c true if not equal, \c false otherwise > > > + */ > > > + > > > +/** > > > + * \var Quantized::quantized_ > > > + * \brief The raw quantized value > > > + * > > > + * This member stores the fixed-point or integer representation of the value, typically > > > + * used for interfacing with hardware or protocols that require compact formats. > > > + * > > > + * It must only be updated or accessed through a Quantizer in conjunction with > > > + * corresponding updates to \c Quantized::value_. > > > + */ > > > + > > > +/** > > > + * \var Quantized::value_ > > > + * \brief The floating-point representation of the quantized value > > > + * > > > + * This member holds the floating-point equivalent of the quantized value, usually > > > + * for use in calculations or presentation to higher-level software components. > > > + * > > > + * It must only be updated or accessed through a Quantizer in conjunction with > > > + * corresponding updates to \c Quantized::quantized_. > > > + */ > > > + > > > +/** > > > + * \class Quantizer > > > + * \brief Interface for converting between floating-point and quantized types > > > + * \tparam T The quantized type (e.g., uint16_t, int16_t). > > > + * > > > + * This abstract class defines the interface for quantizers that handle > > > + * conversions between floating-point values and their quantized representations. > > > + * Specific quantization handlers should implement this interface and provide > > > + * the toFloat and fromFloat methods specifically for their types. > > > + */ > > > + > > > +/** > > > + * \typedef Quantizer::qType > > > + * \brief The underlying quantized type used by the quantizer > > > + * > > > + * This alias exposes the quantized type (e.g. \c uint8_t or \c int16_t) used internally > > > + * by the \c Quantizer, which is helpful in template and debug contexts. > > > + */ > > > + > > > +/** > > > + * \fn Quantizer::fromFloat(T val) > > > + * \brief Convert a floating-point value to its quantized representation > > > + * > > > + * \param val The floating-point value > > > + * \return The quantized equivalent > > > + */ > > > + > > > +/** > > > + * \fn Quantizer::toFloat(T val) > > > + * \brief Convert a quantized value to its floating-point representation > > > + * > > > + * \param val The quantized value > > > + * \return The floating-point equivalent > > > + */ > > > + > > > +/** > > > + * \fn Quantizer::set(float val) > > > + * \brief Set the value of a Quantized<T> using a floating-point input > > > + * > > > + * This function updates both the quantized and floating-point > > > + * representations stored in the Quantized<T>, storing the quantized > > > + * version of the float input. > > > + * > > > + * \param val The floating-point value to set > > > + */ > > > + > > > +/** > > > + * \fn Quantizer::set(T val) > > > + * \brief Set the value of a Quantized<T> using a quantized input > > > + * > > > + * This function updates both the quantized and floating-point > > > + * representations stored in the Quantized<T>. > > > + * > > > + * \param val The quantized value to set > > > + */ > > > + > > > +} /* namespace ipa */ > > > + > > > +} /* namespace libcamera */ > > > diff --git a/src/ipa/libipa/quantized.h b/src/ipa/libipa/quantized.h > > > new file mode 100644 > > > index 000000000000..1c1963cf0848 > > > --- /dev/null > > > +++ b/src/ipa/libipa/quantized.h > > > @@ -0,0 +1,90 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2025, Ideas On Board. > > > + * > > > + * Helper class to manage conversions between floating point types and quantized > > > + * storage and representation of those values. > > > + */ > > > + > > > +#pragma once > > > + > > > +#include <iomanip> > > > +#include <sstream> > > > +#include <stdint.h> > > > +#include <type_traits> > > > + > > > +namespace libcamera { > > > + > > > +namespace ipa { > > > + > > > +template<typename Q> > > > +class Quantizer; > > > + > > > +template<typename T> > > > +struct Quantized { > > > + static_assert(std::is_arithmetic_v<T>, "Quantized: T must be an arithmetic type"); > > > + > > > + float value() const noexcept { return value_; } > > > + T quantized() const noexcept { return quantized_; } > > > + > > > + std::string toString() const > > > > I think I would implement `operator<<`. Yes, I hadn't tackled that yet - but indeed it makes sense and should be easy. > > > > > > > + { > > > + using UT = std::make_unsigned_t<T>; > > > + std::ostringstream oss; > > > + > > > + oss << "Q:0x" << std::hex << std::uppercase << std::setw(sizeof(T) * 2) > > > + << std::setfill('0'); > > > + > > > + if constexpr (std::is_unsigned_v<T>) { > > > + oss << static_cast<unsigned>(quantized_); > > > + } else { > > > + oss << static_cast<unsigned>(static_cast<UT>(quantized_)); > > > > I think you can drop the `if` and keep only this second branch, > > it should work the same as far as I can see. > > Hahah yes - I now can't work out why I branched. I remember it was displaying incorrectly on signed types ... but indeed there's no need to add the branch! > > Also, since you cast to `unsigned`, there should probably be a > > static assert ensuring that the value fits. I'm guessing it's > > done to handle `{u,}int8_t`? In that case one could do > > > > oss << +static_cast<UT>(quantized_); > > > > to ensure that they are promoted to `int` and not printed as characters, > > and then there is no need to cast to `unsigned`. Yes, at one point I was getting incorrect chars over the weekend so that's where all this came from. I'll retest with your proposal thanks. > > > > > > > + } > > > + > > > + oss << " F:" << value_; > > > + > > > + return oss.str(); > > > + } > > > + > > > + bool operator==(const Quantized<T> &other) const noexcept > > > > You can drop the `<T>` part as it is unnecessary inside the definition. > > > > > > > + { > > > + return quantized_ == other.quantized_ && value_ == other.value_; > > > > I worry a bit about floating point comparisons with `==`. And if `value_` is the > > same, shouldn't that imply that `quantized_` is as well? In the unit tests I've added on top of this series for FixedPointQuantizer (I presume that makes it easy to guess where this series goes after) I have to check floats are close enough but not exact, so I think you're right. > > > + } > > > + > > > + bool operator!=(const Quantized<T> &other) const noexcept > > > + { > > > + return !(*this == other); > > > + } > > > + > > > +protected: > > > + T quantized_{}; > > > + float value_{}; > > > +}; > > > + > > > +template<typename T> > > > +class Quantizer : public Quantized<T> > > > +{ > > > +public: > > > + using qType = T; > > > + > > > + virtual ~Quantizer() = default; > > > + > > > + virtual T fromFloat(float val) const = 0; > > > + virtual float toFloat(T val) const = 0; > > > > I'm looking at the use cases, and I'm not sure that these need to be `virtual` in > > the first place. In fact, I think I'd completely eliminate the `Quantizer` class: > > > > template<typename Traits> > > struct Quantized { Hrm ... my only concern here is that now I have to make sure the Traits are known in the ipa_context.h 'globally', where in my version the Quantized is just storage. But - that might not be a problem ... > > using quantized_type = typename Traits::quantized_type; > > > > Quantized() : Quantized(0.0f) { } > > Quantized(float x) { *this = x; } > > Quantized(quantized_type x) { *this = x; } > > Quantized& operator=(float x) { quantized_ = Traits::fromFloat(x); value_ = x; return *this; } > > Quantized& operator=(quantized_type x) { value_ = Traits::toFloat(x); quantized_ = x; return *this; } > > > > /* most others stay the same */ > > > > private: > > quantized_type quantized_; > > float value_; > > }; > > > > /* ... */ > > > > struct RkISP1HueQTraits { > > using quantized_type = int8_t; > > static quantized_type fromFloat(float v) { ... } > > static float toFloat(quantized_type v) { ... } > > }; > > > > /* ... */ > > > > Quantized<RkISP1HueQTraits> hue = ...; > > > > > > This has advantages in my opinion: > > > > * the default initialization will always be correct > > * currently it is assumed that 0.0f maps to "0" of the quantized type > > * type checking: you will not be able to assign/compare/etc. quantized quantities > > with different trait types > > * virtual functions are eliminated > > * if the traits type implements the methods inline, then those can be > > directly inlined, which is most likely worth it if it's just a simple division or similar > > Object slicing is also eliminated. I'll have to work through the above and try it out but I think that looks promising. I really wanted to keep constexpr initialisation - and the virtuals here in Quantizer were killing that off. > > It is entirely possible that I am missing a use case that wouldn't work with > > the above, so I'm wondering if you have looked at an approach similar to the above? My main use case goals are: - Quantized can not be modified without a type that knows how - Quantized can be *read* for either of the two raw types. - Quantizer() is just a base interface and should never be usable directly - Derived Quantizers like FixedPointQuantizer or ContrastQuantiser/HueQuantizer are the implementation per use case to set the values. I could also imagine gain codes being set through this sort of interface, and if I can work through the signed bit extension issues I'm having on FixedPointQuantizer - I think things like ContrastQuantizer end up being a specialisation of that. As a nod to the future for my planned extended use cases I see: using Q1_7 = FixedPointQuantizer<1, 7, int8_t>; using UQ1_7 = FixedPointQuantizer<1, 7, uint8_t>; using Q12_4 = FixedPointQuantizer<12, 4, int16_t>; using UQ12_4 = FixedPointQuantizer<12, 4, uint16_t>; using ContrastQ = Q1_7; ... Now all the ContrastSaturationQuantizer implementation detail falls into a single line. const auto &contrast = controls.get(controls::Contrast); if (contrast) { ContrastQ value = *contrast; ... } And going further: template<typename BaseQuantizer, int Scale> class ScaledFixedPointQuantizer : public Quantizer<typename BaseQuantizer::qType>; /* Hue is between -90 (-128) and +89.xx (+127) */ using HueQ = ScaledFixedPointQuantizer<Q1_7, 90>; HueQ hue = *controls.get(controls::Hue); ... hue is now the quantized fixed point representation of the incoming control So that's what I'm trying to conjure up - and have work in progress for. I think if the generic Traits for FixedPointQuantizer fall into place, it may not be such a concern having the Quantized type know those traits in the common ipa_context.h anyway... -- Kieran > > > Regards, > > Barnabás Pőcze > > > > > > > + > > > + void set(float val) > > > + { > > > + this->quantized_ = fromFloat(val); > > > + this->value_ = toFloat(this->quantized_); > > > + } > > > + > > > + void set(T val) > > > + { > > > + this->quantized_ = val; > > > + this->value_ = toFloat(val); > > > + } > > > +}; > > > + > > > +} /* namespace ipa */ > > > + > > > +} /* namespace libcamera */ > > -- > Regards, > > Laurent Pinchart
diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build index 660be94054fa..804289778f72 100644 --- a/src/ipa/libipa/meson.build +++ b/src/ipa/libipa/meson.build @@ -17,6 +17,7 @@ libipa_headers = files([ 'lux.h', 'module.h', 'pwl.h', + 'quantized.h', ]) libipa_sources = files([ @@ -36,6 +37,7 @@ libipa_sources = files([ 'lux.cpp', 'module.cpp', 'pwl.cpp', + 'quantized.cpp', ]) libipa_includes = include_directories('..') diff --git a/src/ipa/libipa/quantized.cpp b/src/ipa/libipa/quantized.cpp new file mode 100644 index 000000000000..0078c6f740a9 --- /dev/null +++ b/src/ipa/libipa/quantized.cpp @@ -0,0 +1,167 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2025, Ideas On Board. + * + * Helper class to manage conversions between floating point types and quantized + * storage and representation of those values. + */ + +#include "quantized.h" + +/** + * \file quantized.h + * \brief Quantized storage and Quantizer representations + */ + +namespace libcamera { + +namespace ipa { + +/** + * \struct Quantized + * \brief Quantized stores both an internal and quantized representation of a value + * \tparam T The quantized type (e.g., uint16_t, int16_t). + * + * This struct provides read-only access to both representations of the value. + * It does not perform any conversions or quantization logic itself. This must + * be handled externally by a quantizer that knows the appropriate behaviour. + * + * This type can be used to store values that need to be represented in both + * fixed-point or integer (for hardware interfaces) and floating-point (for user + * facing interfaces) + * + * It is intended to be used in shared context structures where both + * hardware-level and software-level representations of a value are required. + */ + +/** + * \fn float Quantized::value() const noexcept + * \brief Get the floating-point representation of the quantized value + * + * This function returns the floating-point form that was either directly assigned + * or computed from the quantized value. + * + * \return The floating-point value + */ + +/** + * \fn T Quantized::quantized() const noexcept + * \brief Get the raw quantized representation of the value + * + * This function returns the internal quantized value (e.g. \c uint8_t or \c int16_t), + * which is typically used for hardware-level programming or serialization. + * + * \return The quantized value of type \c T + */ + +/** + * \fn std::string Quantized::toString() const + * \brief Generate a string representation of the quantized and float values + * + * Produces a human-readable string including both the quantized and floating-point + * values, typically in the format: "Q:0xXX F:float". + * + * \return A formatted string representing the internal state + */ + +/** + * \fn bool Quantized::operator==(const Quantized<T> &other) const noexcept + * \brief Compare two quantized values for equality + * + * Two \c Quantized<T> objects are equal if their quantized values are equal. + * + * \param other The \c Quantized<T> to compare with + * \return \c true if equal, \c false otherwise + */ + +/** + * \fn bool Quantized::operator!=(const Quantized<T> &other) const noexcept + * \brief Compare two quantized values for inequality + * + * Two \c Quantized<T> objects are not equal if their quantized values differ. + * + * \param other The \c Quantized<T> to compare with + * \return \c true if not equal, \c false otherwise + */ + +/** + * \var Quantized::quantized_ + * \brief The raw quantized value + * + * This member stores the fixed-point or integer representation of the value, typically + * used for interfacing with hardware or protocols that require compact formats. + * + * It must only be updated or accessed through a Quantizer in conjunction with + * corresponding updates to \c Quantized::value_. + */ + +/** + * \var Quantized::value_ + * \brief The floating-point representation of the quantized value + * + * This member holds the floating-point equivalent of the quantized value, usually + * for use in calculations or presentation to higher-level software components. + * + * It must only be updated or accessed through a Quantizer in conjunction with + * corresponding updates to \c Quantized::quantized_. + */ + +/** + * \class Quantizer + * \brief Interface for converting between floating-point and quantized types + * \tparam T The quantized type (e.g., uint16_t, int16_t). + * + * This abstract class defines the interface for quantizers that handle + * conversions between floating-point values and their quantized representations. + * Specific quantization handlers should implement this interface and provide + * the toFloat and fromFloat methods specifically for their types. + */ + +/** + * \typedef Quantizer::qType + * \brief The underlying quantized type used by the quantizer + * + * This alias exposes the quantized type (e.g. \c uint8_t or \c int16_t) used internally + * by the \c Quantizer, which is helpful in template and debug contexts. + */ + +/** + * \fn Quantizer::fromFloat(T val) + * \brief Convert a floating-point value to its quantized representation + * + * \param val The floating-point value + * \return The quantized equivalent + */ + +/** + * \fn Quantizer::toFloat(T val) + * \brief Convert a quantized value to its floating-point representation + * + * \param val The quantized value + * \return The floating-point equivalent + */ + +/** + * \fn Quantizer::set(float val) + * \brief Set the value of a Quantized<T> using a floating-point input + * + * This function updates both the quantized and floating-point + * representations stored in the Quantized<T>, storing the quantized + * version of the float input. + * + * \param val The floating-point value to set + */ + +/** + * \fn Quantizer::set(T val) + * \brief Set the value of a Quantized<T> using a quantized input + * + * This function updates both the quantized and floating-point + * representations stored in the Quantized<T>. + * + * \param val The quantized value to set + */ + +} /* namespace ipa */ + +} /* namespace libcamera */ diff --git a/src/ipa/libipa/quantized.h b/src/ipa/libipa/quantized.h new file mode 100644 index 000000000000..1c1963cf0848 --- /dev/null +++ b/src/ipa/libipa/quantized.h @@ -0,0 +1,90 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2025, Ideas On Board. + * + * Helper class to manage conversions between floating point types and quantized + * storage and representation of those values. + */ + +#pragma once + +#include <iomanip> +#include <sstream> +#include <stdint.h> +#include <type_traits> + +namespace libcamera { + +namespace ipa { + +template<typename Q> +class Quantizer; + +template<typename T> +struct Quantized { + static_assert(std::is_arithmetic_v<T>, "Quantized: T must be an arithmetic type"); + + float value() const noexcept { return value_; } + T quantized() const noexcept { return quantized_; } + + std::string toString() const + { + using UT = std::make_unsigned_t<T>; + std::ostringstream oss; + + oss << "Q:0x" << std::hex << std::uppercase << std::setw(sizeof(T) * 2) + << std::setfill('0'); + + if constexpr (std::is_unsigned_v<T>) { + oss << static_cast<unsigned>(quantized_); + } else { + oss << static_cast<unsigned>(static_cast<UT>(quantized_)); + } + + oss << " F:" << value_; + + return oss.str(); + } + + bool operator==(const Quantized<T> &other) const noexcept + { + return quantized_ == other.quantized_ && value_ == other.value_; + } + + bool operator!=(const Quantized<T> &other) const noexcept + { + return !(*this == other); + } + +protected: + T quantized_{}; + float value_{}; +}; + +template<typename T> +class Quantizer : public Quantized<T> +{ +public: + using qType = T; + + virtual ~Quantizer() = default; + + virtual T fromFloat(float val) const = 0; + virtual float toFloat(T val) const = 0; + + void set(float val) + { + this->quantized_ = fromFloat(val); + this->value_ = toFloat(this->quantized_); + } + + void set(T val) + { + this->quantized_ = val; + this->value_ = toFloat(val); + } +}; + +} /* namespace ipa */ + +} /* namespace libcamera */
Frequently when handling data in IPA components we must convert and store user interface values which may be floating point values, and perform a specific operation or conversion to quantize this to a hardware value. This value may be to a fixed point type, or more custom code mappings, but in either case it is important to contain both the required hardware value, with it's effective quantized value. Provide a new storage type 'Quantized' which can be read publicly but must only be written through a controller class known as a Quantizer. Quantizers can be customised and specified by the owner of the data object and must implement conversion between floats and the underlying hardware type. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- src/ipa/libipa/meson.build | 2 + src/ipa/libipa/quantized.cpp | 167 +++++++++++++++++++++++++++++++++++ src/ipa/libipa/quantized.h | 90 +++++++++++++++++++ 3 files changed, 259 insertions(+) create mode 100644 src/ipa/libipa/quantized.cpp create mode 100644 src/ipa/libipa/quantized.h