| Message ID | 20251029172439.1513907-2-kieran.bingham@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi 2025. 10. 29. 18:24 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 its effective quantized value. > > Provide a new storage type 'Quantized' which can be defined based on a > set of type specific Traits to perform the conversions 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 | 134 +++++++++++++++++++++++++++++++++++ > src/ipa/libipa/quantized.h | 79 +++++++++++++++++++++ > 3 files changed, 215 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.h b/src/ipa/libipa/quantized.h > new file mode 100644 > index 000000000000..26cb8d619e15 > --- /dev/null > +++ b/src/ipa/libipa/quantized.h > @@ -0,0 +1,79 @@ > +/* 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> > + > +#include <libcamera/base/utils.h> > + > +namespace libcamera { > + > +namespace ipa { > + > +template<typename Traits> > +struct Quantized { > + using TraitsType = Traits; > + using quantized_type = typename Traits::quantized_type; Small thing, but I think the member type aliases should both be snake_case or PascalCase, but they shouldn't be mixed. > + static_assert(std::is_arithmetic_v<quantized_type>, > + "Quantized: quantized_type must be arithmetic"); > + > + /* Constructors */ > + Quantized() > + : Quantized(0.0f) {} > + Quantized(float x) { *this = x; } > + Quantized(quantized_type x) { *this = x; } > + > + Quantized &operator=(float x) > + { > + quantized_ = Traits::fromFloat(x); > + value_ = Traits::toFloat(quantized_); I might have missed the discussion, but I'm wondering if it should save `x` as is? > + return *this; > + } > + > + Quantized &operator=(quantized_type x) > + { > + value_ = Traits::toFloat(x); > + quantized_ = x; > + return *this; > + } > + > + float value() const noexcept { return value_; } > + quantized_type quantized() const noexcept { return quantized_; } > + > + std::string toString() const > + { > + std::ostringstream oss; > + > + oss << "Q:" << utils::hex(quantized_) > + << " V:" << value_; It's again a minor detail, but I'm wondering about a more compact representation without whitespace in it. Maybe something like value_ << '[' << utils::hex(quantized_) << ']' or '[' << value_ << '|' << utils::hex(quantized_) << ']' Maybe it's just me, but I like if the string representation is visibly a "single unit" even if there are many things in the same line. Regards, Barnabás Pőcze > + > + return oss.str(); > + } > + > + bool operator==(const Quantized &other) const noexcept > + { > + return quantized_ == other.quantized_; > + } > + > + bool operator!=(const Quantized &other) const noexcept > + { > + return !(*this == other); > + } > + > +private: > + quantized_type quantized_{}; > + float value_{}; > +}; > + > +} /* namespace ipa */ > + > +} /* namespace libcamera */
Quoting Barnabás Pőcze (2025-10-29 18:45:21) > Hi > > 2025. 10. 29. 18:24 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 its effective quantized value. > > > > Provide a new storage type 'Quantized' which can be defined based on a > > set of type specific Traits to perform the conversions 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 | 134 +++++++++++++++++++++++++++++++++++ > > src/ipa/libipa/quantized.h | 79 +++++++++++++++++++++ > > 3 files changed, 215 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.h b/src/ipa/libipa/quantized.h > > new file mode 100644 > > index 000000000000..26cb8d619e15 > > --- /dev/null > > +++ b/src/ipa/libipa/quantized.h > > @@ -0,0 +1,79 @@ > > +/* 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> > > + > > +#include <libcamera/base/utils.h> > > + > > +namespace libcamera { > > + > > +namespace ipa { > > + > > +template<typename Traits> > > +struct Quantized { > > + using TraitsType = Traits; TraitsType ends up being quite long when used in later patches too, I've wondered if it should be shorter - or perhaps just template<typename TraitsType> using Traits = TraitsType; so that Traits can be used here and by the later patches. > > + using quantized_type = typename Traits::quantized_type; > > Small thing, but I think the member type aliases should both be > snake_case or PascalCase, but they shouldn't be mixed. yes - agreed - sorry - I've gone through so many iterations and moving bits around from my test snippets and then back into patches, then not kept it consistent. I'll try to tidy that up. I'm pretty sure I had this as qType as well... not sure what works best yet. > > > > + static_assert(std::is_arithmetic_v<quantized_type>, > > + "Quantized: quantized_type must be arithmetic"); > > + > > + /* Constructors */ > > + Quantized() > > + : Quantized(0.0f) {} > > + Quantized(float x) { *this = x; } > > + Quantized(quantized_type x) { *this = x; } > > + > > + Quantized &operator=(float x) > > + { > > + quantized_ = Traits::fromFloat(x); > > + value_ = Traits::toFloat(quantized_); > > I might have missed the discussion, but I'm wondering if it should save `x` as is? There's no specific discusion, but in fact this is a fundamental part of the type. It stores the 'quantized' value of any given input - so the float value_ should be the representative value of quantized_ (once it has been quantized) so that includes all rounding and such. There might be use cases to /also/ store the original requested value - or perhaps make it easy to know what 'difference' there is between a requested value and what actually gets applied, but that can also be done by the caller/setter directly if required. > > > > + return *this; > > + } > > + > > + Quantized &operator=(quantized_type x) > > + { > > + value_ = Traits::toFloat(x); > > + quantized_ = x; > > + return *this; > > + } > > + > > + float value() const noexcept { return value_; } > > + quantized_type quantized() const noexcept { return quantized_; } > > + > > + std::string toString() const > > + { > > + std::ostringstream oss; > > + > > + oss << "Q:" << utils::hex(quantized_) > > + << " V:" << value_; > > It's again a minor detail, but I'm wondering about a more compact > representation without whitespace in it. Maybe something like > > value_ << '[' << utils::hex(quantized_) << ']' > > or > > '[' << value_ << '|' << utils::hex(quantized_) << ']' > > Maybe it's just me, but I like if the string representation is visibly > a "single unit" even if there are many things in the same line. > Absolutely - either of those is probably good too. I guess that's -1.0(0x80) vs [-1|0x80] or perhaps | makes that quite hard to read but this looks parsable: [-1:0x80] > > Regards, > Barnabás Pőcze > > > + > > + return oss.str(); > > + } > > + > > + bool operator==(const Quantized &other) const noexcept > > + { > > + return quantized_ == other.quantized_; > > + } > > + > > + bool operator!=(const Quantized &other) const noexcept > > + { > > + return !(*this == other); > > + } > > + > > +private: > > + quantized_type quantized_{}; > > + float value_{}; > > +}; > > + > > +} /* namespace ipa */ > > + > > +} /* namespace libcamera */ >
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..9772674159bd --- /dev/null +++ b/src/ipa/libipa/quantized.cpp @@ -0,0 +1,134 @@ +/* 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 libcamera::ipa::Quantized + * \brief Wrapper that stores a value in both quantized and floating-point form + * \tparam Traits The traits class defining the quantization behaviour + * + * The Quantized struct template provides a thin wrapper around a quantized + * representation of a floating-point value. It uses a traits type \a Traits + * to define the conversion policy between the floating-point domain and the + * quantized integer domain. + * + * Each Quantized instance maintains two synchronized members: + * - the quantized integer representation, and + * - the corresponding floating-point value. + * + * The traits type defines: + * - the integer storage type used for quantization, + * - the static conversion functions \c fromFloat() and \c toFloat(), and + * - optional metadata such as value ranges. + * + * Quantized provides convenient constructors and assignment operators from + * either representation, as well as comparison and string formatting utilities. + */ + +/** + * \typedef Quantized::TraitsType + * \brief The traits policy type defining the quantization behaviour + * + * Exposes the associated traits type used by this Quantized instance. + * This allows external code to refer to constants or metadata defined in + * the traits, such as \c TraitsType::min or \c TraitsType::max. + */ + +/** + * \typedef Quantized::quantized_type + * \brief The integer type used for the quantized representation + * + * This alias corresponds to \c TraitsType::quantized_type, as defined by + * the traits class. + */ + +/** + * \fn Quantized::Quantized(float x) + * \brief Construct a Quantized value from a floating-point number + * \param[in] x The floating-point value to be quantized + * + * Converts the floating-point input \a x to its quantized integer + * representation using the associated traits policy, and initializes + * both the quantized and floating-point members. + */ + +/** + * \fn Quantized::Quantized(quantized_type x) + * \brief Construct a Quantized value from an existing quantized integer + * \param[in] x The quantized integer value + * + * Converts the quantized integer \a x to its corresponding floating-point + * value using the traits policy, and initializes both internal members. + */ + +/** + * \fn Quantized::operator=(float x) + * \brief Assign a floating-point value to the Quantized object + * \param[in] x The floating-point value to assign + * \return A reference to the updated Quantized object + * + * Converts the floating-point value \a x to its quantized integer + * representation using the traits policy and updates both members. + */ + +/** + * \fn Quantized::operator=(quantized_type x) + * \brief Assign a quantized integer value to the Quantized object + * \param[in] x The quantized integer value to assign + * \return A reference to the updated Quantized object + * + * Converts the quantized integer \a x to its corresponding floating-point + * value using the traits policy and updates both members. + */ + +/** + * \fn Quantized::value() const noexcept + * \brief Retrieve the floating-point representation + * \return The floating-point value corresponding to the quantized value + */ + +/** + * \fn Quantized::quantized() const noexcept + * \brief Retrieve the quantized integer representation + * \return The quantized integer value + */ + +/** + * \fn Quantized::toString() const + * \brief Format the quantized and floating-point values as a string + * \return A string containing the hexadecimal quantized value and its + * floating-point equivalent. + */ + +/** + * \fn Quantized::operator==(const Quantized &other) const noexcept + * \brief Compare two Quantized objects for equality + * \param[in] other The other Quantized object to compare against + * \return True if both objects have the same quantized integer value + */ + +/** + * \fn Quantized::operator!=(const Quantized &other) const noexcept + * \brief Compare two Quantized objects for inequality + * \param[in] other The other Quantized object to compare against + * \return True if the quantized integer values differ + */ + +} /* namespace ipa */ + +} /* namespace libcamera */ diff --git a/src/ipa/libipa/quantized.h b/src/ipa/libipa/quantized.h new file mode 100644 index 000000000000..26cb8d619e15 --- /dev/null +++ b/src/ipa/libipa/quantized.h @@ -0,0 +1,79 @@ +/* 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> + +#include <libcamera/base/utils.h> + +namespace libcamera { + +namespace ipa { + +template<typename Traits> +struct Quantized { + using TraitsType = Traits; + using quantized_type = typename Traits::quantized_type; + static_assert(std::is_arithmetic_v<quantized_type>, + "Quantized: quantized_type must be arithmetic"); + + /* Constructors */ + Quantized() + : Quantized(0.0f) {} + Quantized(float x) { *this = x; } + Quantized(quantized_type x) { *this = x; } + + Quantized &operator=(float x) + { + quantized_ = Traits::fromFloat(x); + value_ = Traits::toFloat(quantized_); + return *this; + } + + Quantized &operator=(quantized_type x) + { + value_ = Traits::toFloat(x); + quantized_ = x; + return *this; + } + + float value() const noexcept { return value_; } + quantized_type quantized() const noexcept { return quantized_; } + + std::string toString() const + { + std::ostringstream oss; + + oss << "Q:" << utils::hex(quantized_) + << " V:" << value_; + + return oss.str(); + } + + bool operator==(const Quantized &other) const noexcept + { + return quantized_ == other.quantized_; + } + + bool operator!=(const Quantized &other) const noexcept + { + return !(*this == other); + } + +private: + quantized_type quantized_{}; + float value_{}; +}; + +} /* 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 its effective quantized value. Provide a new storage type 'Quantized' which can be defined based on a set of type specific Traits to perform the conversions 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 | 134 +++++++++++++++++++++++++++++++++++ src/ipa/libipa/quantized.h | 79 +++++++++++++++++++++ 3 files changed, 215 insertions(+) create mode 100644 src/ipa/libipa/quantized.cpp create mode 100644 src/ipa/libipa/quantized.h