[{"id":36477,"web_url":"https://patchwork.libcamera.org/comment/36477/","msgid":"<5a95febf-9505-41be-a702-c264ce562cee@ideasonboard.com>","date":"2025-10-27T09:37:21","subject":"Re: [PATCH 1/6] libipa: Provide a Quantized type and Quantizer\n\tsupport","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 10. 27. 0:30 keltezéssel, Kieran Bingham írta:\n> Frequently when handling data in IPA components we must convert and\n> store user interface values which may be floating point values, and\n> perform a specific operation or conversion to quantize this to a\n> hardware value.\n> \n> This value may be to a fixed point type, or more custom code mappings,\n> but in either case it is important to contain both the required hardware\n> value, with it's effective quantized value.\n\nit's -> its ?\n\n\n> \n> Provide a new storage type 'Quantized' which can be read publicly but\n> must only be written through a controller class known as a Quantizer.\n> \n> Quantizers can be customised and specified by the owner of the data\n> object and must implement conversion between floats and the underlying\n> hardware type.\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> src/ipa/libipa/meson.build | 2 +\n> src/ipa/libipa/quantized.cpp | 167 +++++++++++++++++++++++++++++++++++\n> src/ipa/libipa/quantized.h | 90 +++++++++++++++++++\n> 3 files changed, 259 insertions(+)\n> create mode 100644 src/ipa/libipa/quantized.cpp\n> create mode 100644 src/ipa/libipa/quantized.h\n> \n> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n> index 660be94054fa..804289778f72 100644\n> --- a/src/ipa/libipa/meson.build\n> +++ b/src/ipa/libipa/meson.build\n> @@ -17,6 +17,7 @@ libipa_headers = files([\n> 'lux.h',\n> 'module.h',\n> 'pwl.h',\n> + 'quantized.h',\n> ])\n> \n> libipa_sources = files([\n> @@ -36,6 +37,7 @@ libipa_sources = files([\n> 'lux.cpp',\n> 'module.cpp',\n> 'pwl.cpp',\n> + 'quantized.cpp',\n> ])\n> \n> libipa_includes = include_directories('..')\n> diff --git a/src/ipa/libipa/quantized.cpp b/src/ipa/libipa/quantized.cpp\n> new file mode 100644\n> index 000000000000..0078c6f740a9\n> --- /dev/null\n> +++ b/src/ipa/libipa/quantized.cpp\n> @@ -0,0 +1,167 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board.\n> + *\n> + * Helper class to manage conversions between floating point types and quantized\n> + * storage and representation of those values.\n> + */\n> +\n> +#include \"quantized.h\"\n> +\n> +/**\n> + * \\file quantized.h\n> + * \\brief Quantized storage and Quantizer representations\n> + */\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa {\n> +\n> +/**\n> + * \\struct Quantized\n> + * \\brief Quantized stores both an internal and quantized representation of a value\n> + * \\tparam T The quantized type (e.g., uint16_t, int16_t).\n> + *\n> + * This struct provides read-only access to both representations of the value.\n> + * It does not perform any conversions or quantization logic itself. This must\n> + * be handled externally by a quantizer that knows the appropriate behaviour.\n> + *\n> + * This type can be used to store values that need to be represented in both\n> + * fixed-point or integer (for hardware interfaces) and floating-point (for user\n> + * facing interfaces)\n> + *\n> + * It is intended to be used in shared context structures where both\n> + * hardware-level and software-level representations of a value are required.\n> + */\n> +\n> +/**\n> + * \\fn float Quantized::value() const noexcept\n> + * \\brief Get the floating-point representation of the quantized value\n> + *\n> + * This function returns the floating-point form that was either directly assigned\n> + * or computed from the quantized value.\n> + *\n> + * \\return The floating-point value\n> + */\n> +\n> +/**\n> + * \\fn T Quantized::quantized() const noexcept\n> + * \\brief Get the raw quantized representation of the value\n> + *\n> + * This function returns the internal quantized value (e.g. \\c uint8_t or \\c int16_t),\n> + * which is typically used for hardware-level programming or serialization.\n> + *\n> + * \\return The quantized value of type \\c T\n> + */\n> +\n> +/**\n> + * \\fn std::string Quantized::toString() const\n> + * \\brief Generate a string representation of the quantized and float values\n> + *\n> + * Produces a human-readable string including both the quantized and floating-point\n> + * values, typically in the format: \"Q:0xXX F:float\".\n> + *\n> + * \\return A formatted string representing the internal state\n> + */\n> +\n> +/**\n> + * \\fn bool Quantized::operator==(const Quantized &other) const noexcept\n> + * \\brief Compare two quantized values for equality\n> + *\n> + * Two \\c Quantized objects are equal if their quantized values are equal.\n> + *\n> + * \\param other The \\c Quantized to compare with\n> + * \\return \\c true if equal, \\c false otherwise\n> + */\n> +\n> +/**\n> + * \\fn bool Quantized::operator!=(const Quantized &other) const noexcept\n> + * \\brief Compare two quantized values for inequality\n> + *\n> + * Two \\c Quantized objects are not equal if their quantized values differ.\n> + *\n> + * \\param other The \\c Quantized to compare with\n> + * \\return \\c true if not equal, \\c false otherwise\n> + */\n> +\n> +/**\n> + * \\var Quantized::quantized_\n> + * \\brief The raw quantized value\n> + *\n> + * This member stores the fixed-point or integer representation of the value, typically\n> + * used for interfacing with hardware or protocols that require compact formats.\n> + *\n> + * It must only be updated or accessed through a Quantizer in conjunction with\n> + * corresponding updates to \\c Quantized::value_.\n> + */\n> +\n> +/**\n> + * \\var Quantized::value_\n> + * \\brief The floating-point representation of the quantized value\n> + *\n> + * This member holds the floating-point equivalent of the quantized value, usually\n> + * for use in calculations or presentation to higher-level software components.\n> + *\n> + * It must only be updated or accessed through a Quantizer in conjunction with\n> + * corresponding updates to \\c Quantized::quantized_.\n> + */\n> +\n> +/**\n> + * \\class Quantizer\n> + * \\brief Interface for converting between floating-point and quantized types\n> + * \\tparam T The quantized type (e.g., uint16_t, int16_t).\n> + *\n> + * This abstract class defines the interface for quantizers that handle\n> + * conversions between floating-point values and their quantized representations.\n> + * Specific quantization handlers should implement this interface and provide\n> + * the toFloat and fromFloat methods specifically for their types.\n> + */\n> +\n> +/**\n> + * \\typedef Quantizer::qType\n> + * \\brief The underlying quantized type used by the quantizer\n> + *\n> + * This alias exposes the quantized type (e.g. \\c uint8_t or \\c int16_t) used internally\n> + * by the \\c Quantizer, which is helpful in template and debug contexts.\n> + */\n> +\n> +/**\n> + * \\fn Quantizer::fromFloat(T val)\n> + * \\brief Convert a floating-point value to its quantized representation\n> + *\n> + * \\param val The floating-point value\n> + * \\return The quantized equivalent\n> + */\n> +\n> +/**\n> + * \\fn Quantizer::toFloat(T val)\n> + * \\brief Convert a quantized value to its floating-point representation\n> + *\n> + * \\param val The quantized value\n> + * \\return The floating-point equivalent\n> + */\n> +\n> +/**\n> + * \\fn Quantizer::set(float val)\n> + * \\brief Set the value of a Quantized using a floating-point input\n> + *\n> + * This function updates both the quantized and floating-point\n> + * representations stored in the Quantized, storing the quantized\n> + * version of the float input.\n> + *\n> + * \\param val The floating-point value to set\n> + */\n> +\n> +/**\n> + * \\fn Quantizer::set(T val)\n> + * \\brief Set the value of a Quantized using a quantized input\n> + *\n> + * This function updates both the quantized and floating-point\n> + * representations stored in the Quantized.\n> + *\n> + * \\param val The quantized value to set\n> + */\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/libipa/quantized.h b/src/ipa/libipa/quantized.h\n> new file mode 100644\n> index 000000000000..1c1963cf0848\n> --- /dev/null\n> +++ b/src/ipa/libipa/quantized.h\n> @@ -0,0 +1,90 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2025, Ideas On Board.\n> + *\n> + * Helper class to manage conversions between floating point types and quantized\n> + * storage and representation of those values.\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +namespace libcamera {\n> +\n> +namespace ipa {\n> +\n> +template\n> +class Quantizer;\n> +\n> +template\n> +struct Quantized {\n> +\tstatic_assert(std::is_arithmetic_v, \"Quantized: T must be an arithmetic type\");\n> +\n> +\tfloat value() const noexcept { return value_; }\n> +\tT quantized() const noexcept { return quantized_; }\n> +\n> +\tstd::string toString() const\n\nI think I would implement `operator<<`.\n\n\n> +\t{\n> +\t\tusing UT = std::make_unsigned_t;\n> +\t\tstd::ostringstream oss;\n> +\n> +\t\toss << \"Q:0x\" << std::hex << std::uppercase << std::setw(sizeof(T) * 2)\n> +\t\t << std::setfill('0');\n> +\n> +\t\tif constexpr (std::is_unsigned_v) {\n> +\t\t\toss << static_cast(quantized_);\n> +\t\t} else {\n> +\t\t\toss << static_cast(static_cast(quantized_));\n\nI think you can drop the `if` and keep only this second branch,\nit should work the same as far as I can see.\n\nAlso, since you cast to `unsigned`, there should probably be a\nstatic assert ensuring that the value fits. I'm guessing it's\ndone to handle `{u,}int8_t`? In that case one could do\n\n oss << +static_cast(quantized_);\n\nto ensure that they are promoted to `int` and not printed as characters,\nand then there is no need to cast to `unsigned`.\n\n\n> +\t\t}\n> +\n> +\t\toss << \" F:\" << value_;\n> +\n> +\t\treturn oss.str();\n> +\t}\n> +\n> +\tbool operator==(const Quantized &other) const noexcept\n\nYou can drop the `` part as it is unnecessary inside the definition.\n\n\n> +\t{\n> +\t\treturn quantized_ == other.quantized_ && value_ == other.value_;\n\nI worry a bit about floating point comparisons with `==`. And if `value_` is the\nsame, shouldn't that imply that `quantized_` is as well?\n\n\n> +\t}\n> +\n> +\tbool operator!=(const Quantized &other) const noexcept\n> +\t{\n> +\t\treturn !(*this == other);\n> +\t}\n> +\n> +protected:\n> +\tT quantized_{};\n> +\tfloat value_{};\n> +};\n> +\n> +template\n> +class Quantizer : public Quantized\n> +{\n> +public:\n> +\tusing qType = T;\n> +\n> +\tvirtual ~Quantizer() = default;\n> +\n> +\tvirtual T fromFloat(float val) const = 0;\n> +\tvirtual float toFloat(T val) const = 0;\n\nI'm looking at the use cases, and I'm not sure that these need to be `virtual` in\nthe first place. In fact, I think I'd completely eliminate the `Quantizer` class:\n\n template\n struct Quantized {\n using quantized_type = typename Traits::quantized_type;\n\n Quantized() : Quantized(0.0f) { }\n Quantized(float x) { *this = x; }\n Quantized(quantized_type x) { *this = x; }\n Quantized& operator=(float x) { quantized_ = Traits::fromFloat(x); value_ = x; return *this; }\n Quantized& operator=(quantized_type x) { value_ = Traits::toFloat(x); quantized_ = x; return *this; }\n\n /* most others stay the same */\n\n private:\n quantized_type quantized_;\n float value_;\n };\n\n /* ... */\n\n struct RkISP1HueQTraits {\n using quantized_type = int8_t;\n static quantized_type fromFloat(float v) { ... }\n static float toFloat(quantized_type v) { ... }\n };\n\n /* ... */\n\n Quantized hue = ...;\n\n\nThis has advantages in my opinion:\n\n * the default initialization will always be correct\n * currently it is assumed that 0.0f maps to \"0\" of the quantized type\n * type checking: you will not be able to assign/compare/etc. quantized quantities\n with different trait types\n * virtual functions are eliminated\n * if the traits type implements the methods inline, then those can be\n directly inlined, which is most likely worth it if it's just a simple division or similar\n\nIt is entirely possible that I am missing a use case that wouldn't work with\nthe above, so I'm wondering if you have looked at an approach similar to the above?\n\nRegards,\nBarnabás Pőcze\n\n\n> +\n> +\tvoid set(float val)\n> +\t{\n> +\t\tthis->quantized_ = fromFloat(val);\n> +\t\tthis->value_ = toFloat(this->quantized_);\n> +\t}\n> +\n> +\tvoid set(T val)\n> +\t{\n> +\t\tthis->quantized_ = val;\n> +\t\tthis->value_ = toFloat(val);\n> +\t}\n> +};\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\n> --\n> 2.51.0\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 CC026C3259\n\tfor ;\n\tMon, 27 Oct 2025 09:37:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A963560722;\n\tMon, 27 Oct 2025 10:37:27 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0A55460453\n\tfor ;\n\tMon, 27 Oct 2025 10:37:26 +0100 (CET)","from [192.168.33.25] (185.182.215.162.nat.pool.zt.hu\n\t[185.182.215.162])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 32F561661;\n\tMon, 27 Oct 2025 10:35:38 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"OGaGSDkB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761557738;\n\tbh=1FTilTxsv4UQ6o3ulMJgX8GBXoSXRa/2RgRePwMDQEI=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=OGaGSDkB7Q4+I/RLe4LwXRmQgUQ/XNvaZ2RUjrv8ezfYhkqGMsJuieIrA/ca17OH+\n\tK5EEZ/6R1d6ZEY93YQh9JF73wquGNFYIU1oYHmTe/U5j8555leQvtmZHNPmls0eN+1\n\thlUh2nlfag90UUfbiDS3ElhtL8vbMlPC033cBAkU=","Message-ID":"<5a95febf-9505-41be-a702-c264ce562cee@ideasonboard.com>","Date":"Mon, 27 Oct 2025 10:37:21 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 1/6] libipa: Provide a Quantized type and Quantizer\n\tsupport","To":"Kieran Bingham ,\n\tlibcamera devel ","References":"<20251026233048.175689-1-kieran.bingham@ideasonboard.com>\n\t\n\t<20251026233048.175689-2-kieran.bingham@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251026233048.175689-2-kieran.bingham@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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":36478,"web_url":"https://patchwork.libcamera.org/comment/36478/","msgid":"<20251027094041.GC1544@pendragon.ideasonboard.com>","date":"2025-10-27T09:40:41","subject":"Re: [PATCH 1/6] libipa: Provide a Quantized type and Quantizer\n\tsupport","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Oct 27, 2025 at 10:37:21AM +0100, Barnabás Pőcze wrote:\n> Hi\n> \n> 2025. 10. 27. 0:30 keltezéssel, Kieran Bingham írta:\n> > Frequently when handling data in IPA components we must convert and\n> > store user interface values which may be floating point values, and\n> > perform a specific operation or conversion to quantize this to a\n> > hardware value.\n> > \n> > This value may be to a fixed point type, or more custom code mappings,\n> > but in either case it is important to contain both the required hardware\n> > value, with it's effective quantized value.\n> \n> it's -> its ?\n> \n> \n> > \n> > Provide a new storage type 'Quantized' which can be read publicly but\n> > must only be written through a controller class known as a Quantizer.\n> > \n> > Quantizers can be customised and specified by the owner of the data\n> > object and must implement conversion between floats and the underlying\n> > hardware type.\n> > \n> > Signed-off-by: Kieran Bingham \n> > ---\n> > src/ipa/libipa/meson.build | 2 +\n> > src/ipa/libipa/quantized.cpp | 167 +++++++++++++++++++++++++++++++++++\n> > src/ipa/libipa/quantized.h | 90 +++++++++++++++++++\n> > 3 files changed, 259 insertions(+)\n> > create mode 100644 src/ipa/libipa/quantized.cpp\n> > create mode 100644 src/ipa/libipa/quantized.h\n> > \n> > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n> > index 660be94054fa..804289778f72 100644\n> > --- a/src/ipa/libipa/meson.build\n> > +++ b/src/ipa/libipa/meson.build\n> > @@ -17,6 +17,7 @@ libipa_headers = files([\n> > 'lux.h',\n> > 'module.h',\n> > 'pwl.h',\n> > + 'quantized.h',\n> > ])\n> > \n> > libipa_sources = files([\n> > @@ -36,6 +37,7 @@ libipa_sources = files([\n> > 'lux.cpp',\n> > 'module.cpp',\n> > 'pwl.cpp',\n> > + 'quantized.cpp',\n> > ])\n> > \n> > libipa_includes = include_directories('..')\n> > diff --git a/src/ipa/libipa/quantized.cpp b/src/ipa/libipa/quantized.cpp\n> > new file mode 100644\n> > index 000000000000..0078c6f740a9\n> > --- /dev/null\n> > +++ b/src/ipa/libipa/quantized.cpp\n> > @@ -0,0 +1,167 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2025, Ideas On Board.\n> > + *\n> > + * Helper class to manage conversions between floating point types and quantized\n> > + * storage and representation of those values.\n> > + */\n> > +\n> > +#include \"quantized.h\"\n> > +\n> > +/**\n> > + * \\file quantized.h\n> > + * \\brief Quantized storage and Quantizer representations\n> > + */\n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace ipa {\n> > +\n> > +/**\n> > + * \\struct Quantized\n> > + * \\brief Quantized stores both an internal and quantized representation of a value\n> > + * \\tparam T The quantized type (e.g., uint16_t, int16_t).\n> > + *\n> > + * This struct provides read-only access to both representations of the value.\n> > + * It does not perform any conversions or quantization logic itself. This must\n> > + * be handled externally by a quantizer that knows the appropriate behaviour.\n> > + *\n> > + * This type can be used to store values that need to be represented in both\n> > + * fixed-point or integer (for hardware interfaces) and floating-point (for user\n> > + * facing interfaces)\n> > + *\n> > + * It is intended to be used in shared context structures where both\n> > + * hardware-level and software-level representations of a value are required.\n> > + */\n> > +\n> > +/**\n> > + * \\fn float Quantized::value() const noexcept\n> > + * \\brief Get the floating-point representation of the quantized value\n> > + *\n> > + * This function returns the floating-point form that was either directly assigned\n> > + * or computed from the quantized value.\n> > + *\n> > + * \\return The floating-point value\n> > + */\n> > +\n> > +/**\n> > + * \\fn T Quantized::quantized() const noexcept\n> > + * \\brief Get the raw quantized representation of the value\n> > + *\n> > + * This function returns the internal quantized value (e.g. \\c uint8_t or \\c int16_t),\n> > + * which is typically used for hardware-level programming or serialization.\n> > + *\n> > + * \\return The quantized value of type \\c T\n> > + */\n> > +\n> > +/**\n> > + * \\fn std::string Quantized::toString() const\n> > + * \\brief Generate a string representation of the quantized and float values\n> > + *\n> > + * Produces a human-readable string including both the quantized and floating-point\n> > + * values, typically in the format: \"Q:0xXX F:float\".\n> > + *\n> > + * \\return A formatted string representing the internal state\n> > + */\n> > +\n> > +/**\n> > + * \\fn bool Quantized::operator==(const Quantized &other) const noexcept\n> > + * \\brief Compare two quantized values for equality\n> > + *\n> > + * Two \\c Quantized objects are equal if their quantized values are equal.\n> > + *\n> > + * \\param other The \\c Quantized to compare with\n> > + * \\return \\c true if equal, \\c false otherwise\n> > + */\n> > +\n> > +/**\n> > + * \\fn bool Quantized::operator!=(const Quantized &other) const noexcept\n> > + * \\brief Compare two quantized values for inequality\n> > + *\n> > + * Two \\c Quantized objects are not equal if their quantized values differ.\n> > + *\n> > + * \\param other The \\c Quantized to compare with\n> > + * \\return \\c true if not equal, \\c false otherwise\n> > + */\n> > +\n> > +/**\n> > + * \\var Quantized::quantized_\n> > + * \\brief The raw quantized value\n> > + *\n> > + * This member stores the fixed-point or integer representation of the value, typically\n> > + * used for interfacing with hardware or protocols that require compact formats.\n> > + *\n> > + * It must only be updated or accessed through a Quantizer in conjunction with\n> > + * corresponding updates to \\c Quantized::value_.\n> > + */\n> > +\n> > +/**\n> > + * \\var Quantized::value_\n> > + * \\brief The floating-point representation of the quantized value\n> > + *\n> > + * This member holds the floating-point equivalent of the quantized value, usually\n> > + * for use in calculations or presentation to higher-level software components.\n> > + *\n> > + * It must only be updated or accessed through a Quantizer in conjunction with\n> > + * corresponding updates to \\c Quantized::quantized_.\n> > + */\n> > +\n> > +/**\n> > + * \\class Quantizer\n> > + * \\brief Interface for converting between floating-point and quantized types\n> > + * \\tparam T The quantized type (e.g., uint16_t, int16_t).\n> > + *\n> > + * This abstract class defines the interface for quantizers that handle\n> > + * conversions between floating-point values and their quantized representations.\n> > + * Specific quantization handlers should implement this interface and provide\n> > + * the toFloat and fromFloat methods specifically for their types.\n> > + */\n> > +\n> > +/**\n> > + * \\typedef Quantizer::qType\n> > + * \\brief The underlying quantized type used by the quantizer\n> > + *\n> > + * This alias exposes the quantized type (e.g. \\c uint8_t or \\c int16_t) used internally\n> > + * by the \\c Quantizer, which is helpful in template and debug contexts.\n> > + */\n> > +\n> > +/**\n> > + * \\fn Quantizer::fromFloat(T val)\n> > + * \\brief Convert a floating-point value to its quantized representation\n> > + *\n> > + * \\param val The floating-point value\n> > + * \\return The quantized equivalent\n> > + */\n> > +\n> > +/**\n> > + * \\fn Quantizer::toFloat(T val)\n> > + * \\brief Convert a quantized value to its floating-point representation\n> > + *\n> > + * \\param val The quantized value\n> > + * \\return The floating-point equivalent\n> > + */\n> > +\n> > +/**\n> > + * \\fn Quantizer::set(float val)\n> > + * \\brief Set the value of a Quantized using a floating-point input\n> > + *\n> > + * This function updates both the quantized and floating-point\n> > + * representations stored in the Quantized, storing the quantized\n> > + * version of the float input.\n> > + *\n> > + * \\param val The floating-point value to set\n> > + */\n> > +\n> > +/**\n> > + * \\fn Quantizer::set(T val)\n> > + * \\brief Set the value of a Quantized using a quantized input\n> > + *\n> > + * This function updates both the quantized and floating-point\n> > + * representations stored in the Quantized.\n> > + *\n> > + * \\param val The quantized value to set\n> > + */\n> > +\n> > +} /* namespace ipa */\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/ipa/libipa/quantized.h b/src/ipa/libipa/quantized.h\n> > new file mode 100644\n> > index 000000000000..1c1963cf0848\n> > --- /dev/null\n> > +++ b/src/ipa/libipa/quantized.h\n> > @@ -0,0 +1,90 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2025, Ideas On Board.\n> > + *\n> > + * Helper class to manage conversions between floating point types and quantized\n> > + * storage and representation of those values.\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace ipa {\n> > +\n> > +template\n> > +class Quantizer;\n> > +\n> > +template\n> > +struct Quantized {\n> > +\tstatic_assert(std::is_arithmetic_v, \"Quantized: T must be an arithmetic type\");\n> > +\n> > +\tfloat value() const noexcept { return value_; }\n> > +\tT quantized() const noexcept { return quantized_; }\n> > +\n> > +\tstd::string toString() const\n> \n> I think I would implement `operator<<`.\n> \n> \n> > +\t{\n> > +\t\tusing UT = std::make_unsigned_t;\n> > +\t\tstd::ostringstream oss;\n> > +\n> > +\t\toss << \"Q:0x\" << std::hex << std::uppercase << std::setw(sizeof(T) * 2)\n> > +\t\t << std::setfill('0');\n> > +\n> > +\t\tif constexpr (std::is_unsigned_v) {\n> > +\t\t\toss << static_cast(quantized_);\n> > +\t\t} else {\n> > +\t\t\toss << static_cast(static_cast(quantized_));\n> \n> I think you can drop the `if` and keep only this second branch,\n> it should work the same as far as I can see.\n> \n> Also, since you cast to `unsigned`, there should probably be a\n> static assert ensuring that the value fits. I'm guessing it's\n> done to handle `{u,}int8_t`? In that case one could do\n> \n> oss << +static_cast(quantized_);\n> \n> to ensure that they are promoted to `int` and not printed as characters,\n> and then there is no need to cast to `unsigned`.\n> \n> \n> > +\t\t}\n> > +\n> > +\t\toss << \" F:\" << value_;\n> > +\n> > +\t\treturn oss.str();\n> > +\t}\n> > +\n> > +\tbool operator==(const Quantized &other) const noexcept\n> \n> You can drop the `` part as it is unnecessary inside the definition.\n> \n> \n> > +\t{\n> > +\t\treturn quantized_ == other.quantized_ && value_ == other.value_;\n> \n> I worry a bit about floating point comparisons with `==`. And if `value_` is the\n> same, shouldn't that imply that `quantized_` is as well?\n> \n> \n> > +\t}\n> > +\n> > +\tbool operator!=(const Quantized &other) const noexcept\n> > +\t{\n> > +\t\treturn !(*this == other);\n> > +\t}\n> > +\n> > +protected:\n> > +\tT quantized_{};\n> > +\tfloat value_{};\n> > +};\n> > +\n> > +template\n> > +class Quantizer : public Quantized\n> > +{\n> > +public:\n> > +\tusing qType = T;\n> > +\n> > +\tvirtual ~Quantizer() = default;\n> > +\n> > +\tvirtual T fromFloat(float val) const = 0;\n> > +\tvirtual float toFloat(T val) const = 0;\n> \n> I'm looking at the use cases, and I'm not sure that these need to be `virtual` in\n> the first place. In fact, I think I'd completely eliminate the `Quantizer` class:\n> \n> template\n> struct Quantized {\n> using quantized_type = typename Traits::quantized_type;\n> \n> Quantized() : Quantized(0.0f) { }\n> Quantized(float x) { *this = x; }\n> Quantized(quantized_type x) { *this = x; }\n> Quantized& operator=(float x) { quantized_ = Traits::fromFloat(x); value_ = x; return *this; }\n> Quantized& operator=(quantized_type x) { value_ = Traits::toFloat(x); quantized_ = x; return *this; }\n> \n> /* most others stay the same */\n> \n> private:\n> quantized_type quantized_;\n> float value_;\n> };\n> \n> /* ... */\n> \n> struct RkISP1HueQTraits {\n> using quantized_type = int8_t;\n> static quantized_type fromFloat(float v) { ... }\n> static float toFloat(quantized_type v) { ... }\n> };\n> \n> /* ... */\n> \n> Quantized hue = ...;\n> \n> \n> This has advantages in my opinion:\n> \n> * the default initialization will always be correct\n> * currently it is assumed that 0.0f maps to \"0\" of the quantized type\n> * type checking: you will not be able to assign/compare/etc. quantized quantities\n> with different trait types\n> * virtual functions are eliminated\n> * if the traits type implements the methods inline, then those can be\n> directly inlined, which is most likely worth it if it's just a simple division or similar\n\nObject slicing is also eliminated.\n\n> It is entirely possible that I am missing a use case that wouldn't work with\n> the above, so I'm wondering if you have looked at an approach similar to the above?\n> \n> Regards,\n> Barnabás Pőcze\n> \n> \n> > +\n> > +\tvoid set(float val)\n> > +\t{\n> > +\t\tthis->quantized_ = fromFloat(val);\n> > +\t\tthis->value_ = toFloat(this->quantized_);\n> > +\t}\n> > +\n> > +\tvoid set(T val)\n> > +\t{\n> > +\t\tthis->quantized_ = val;\n> > +\t\tthis->value_ = toFloat(val);\n> > +\t}\n> > +};\n> > +\n> > +} /* namespace ipa */\n> > +\n> > +} /* namespace libcamera */","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 55069BE080\n\tfor ;\n\tMon, 27 Oct 2025 09:40:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0CD726074D;\n\tMon, 27 Oct 2025 10:40:57 +0100 (CET)","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 44CF860453\n\tfor ;\n\tMon, 27 Oct 2025 10:40:55 +0100 (CET)","from pendragon.ideasonboard.com (unknown [193.209.96.36])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 0C89F1661; \n\tMon, 27 Oct 2025 10:39:06 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"nqRShlXH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761557947;\n\tbh=WPv7f7nAU83sXZgIqvwmlsJiYmX07ui3ghauDx++B5k=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nqRShlXHcLuuquG8/Td4AIJcSwBdKwS6ItpDL94a9+d1pMyKu8jnLShl+Xi44Nu/A\n\tE1uVg9fl0M9l+3KoQ6Za4wivgtDTpl2BvK6WvumsJRPS/rQxe85iCz3aeptcoDKCAE\n\tdbkjw/c24RPtdHK3h4B/n3nFQ2lsZ9apJELs3+5k=","Date":"Mon, 27 Oct 2025 11:40:41 +0200","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Kieran Bingham ,\n\tlibcamera devel ","Subject":"Re: [PATCH 1/6] libipa: Provide a Quantized type and Quantizer\n\tsupport","Message-ID":"<20251027094041.GC1544@pendragon.ideasonboard.com>","References":"<20251026233048.175689-1-kieran.bingham@ideasonboard.com>\n\t\n\t<20251026233048.175689-2-kieran.bingham@ideasonboard.com>\n\t<5a95febf-9505-41be-a702-c264ce562cee@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<5a95febf-9505-41be-a702-c264ce562cee@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":36482,"web_url":"https://patchwork.libcamera.org/comment/36482/","msgid":"<176156712298.2256500.4535037273042187843@ping.linuxembedded.co.uk>","date":"2025-10-27T12:12:02","subject":"Re: [PATCH 1/6] libipa: Provide a Quantized type and Quantizer\n\tsupport","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2025-10-27 09:40:41)\n> On Mon, Oct 27, 2025 at 10:37:21AM +0100, Barnabás Pőcze wrote:\n> > Hi\n> > \n> > 2025. 10. 27. 0:30 keltezéssel, Kieran Bingham írta:\n> > > Frequently when handling data in IPA components we must convert and\n> > > store user interface values which may be floating point values, and\n> > > perform a specific operation or conversion to quantize this to a\n> > > hardware value.\n> > > \n> > > This value may be to a fixed point type, or more custom code mappings,\n> > > but in either case it is important to contain both the required hardware\n> > > value, with it's effective quantized value.\n> > \n> > it's -> its ?\n\nAck,\n\n\n> > \n> > \n> > > \n> > > Provide a new storage type 'Quantized' which can be read publicly but\n> > > must only be written through a controller class known as a Quantizer.\n> > > \n> > > Quantizers can be customised and specified by the owner of the data\n> > > object and must implement conversion between floats and the underlying\n> > > hardware type.\n> > > \n> > > Signed-off-by: Kieran Bingham \n> > > ---\n> > > src/ipa/libipa/meson.build | 2 +\n> > > src/ipa/libipa/quantized.cpp | 167 +++++++++++++++++++++++++++++++++++\n> > > src/ipa/libipa/quantized.h | 90 +++++++++++++++++++\n> > > 3 files changed, 259 insertions(+)\n> > > create mode 100644 src/ipa/libipa/quantized.cpp\n> > > create mode 100644 src/ipa/libipa/quantized.h\n> > > \n> > > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n> > > index 660be94054fa..804289778f72 100644\n> > > --- a/src/ipa/libipa/meson.build\n> > > +++ b/src/ipa/libipa/meson.build\n> > > @@ -17,6 +17,7 @@ libipa_headers = files([\n> > > 'lux.h',\n> > > 'module.h',\n> > > 'pwl.h',\n> > > + 'quantized.h',\n> > > ])\n> > > \n> > > libipa_sources = files([\n> > > @@ -36,6 +37,7 @@ libipa_sources = files([\n> > > 'lux.cpp',\n> > > 'module.cpp',\n> > > 'pwl.cpp',\n> > > + 'quantized.cpp',\n> > > ])\n> > > \n> > > libipa_includes = include_directories('..')\n> > > diff --git a/src/ipa/libipa/quantized.cpp b/src/ipa/libipa/quantized.cpp\n> > > new file mode 100644\n> > > index 000000000000..0078c6f740a9\n> > > --- /dev/null\n> > > +++ b/src/ipa/libipa/quantized.cpp\n> > > @@ -0,0 +1,167 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2025, Ideas On Board.\n> > > + *\n> > > + * Helper class to manage conversions between floating point types and quantized\n> > > + * storage and representation of those values.\n> > > + */\n> > > +\n> > > +#include \"quantized.h\"\n> > > +\n> > > +/**\n> > > + * \\file quantized.h\n> > > + * \\brief Quantized storage and Quantizer representations\n> > > + */\n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +namespace ipa {\n> > > +\n> > > +/**\n> > > + * \\struct Quantized\n> > > + * \\brief Quantized stores both an internal and quantized representation of a value\n> > > + * \\tparam T The quantized type (e.g., uint16_t, int16_t).\n> > > + *\n> > > + * This struct provides read-only access to both representations of the value.\n> > > + * It does not perform any conversions or quantization logic itself. This must\n> > > + * be handled externally by a quantizer that knows the appropriate behaviour.\n> > > + *\n> > > + * This type can be used to store values that need to be represented in both\n> > > + * fixed-point or integer (for hardware interfaces) and floating-point (for user\n> > > + * facing interfaces)\n> > > + *\n> > > + * It is intended to be used in shared context structures where both\n> > > + * hardware-level and software-level representations of a value are required.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn float Quantized::value() const noexcept\n> > > + * \\brief Get the floating-point representation of the quantized value\n> > > + *\n> > > + * This function returns the floating-point form that was either directly assigned\n> > > + * or computed from the quantized value.\n> > > + *\n> > > + * \\return The floating-point value\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn T Quantized::quantized() const noexcept\n> > > + * \\brief Get the raw quantized representation of the value\n> > > + *\n> > > + * This function returns the internal quantized value (e.g. \\c uint8_t or \\c int16_t),\n> > > + * which is typically used for hardware-level programming or serialization.\n> > > + *\n> > > + * \\return The quantized value of type \\c T\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn std::string Quantized::toString() const\n> > > + * \\brief Generate a string representation of the quantized and float values\n> > > + *\n> > > + * Produces a human-readable string including both the quantized and floating-point\n> > > + * values, typically in the format: \"Q:0xXX F:float\".\n> > > + *\n> > > + * \\return A formatted string representing the internal state\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn bool Quantized::operator==(const Quantized &other) const noexcept\n> > > + * \\brief Compare two quantized values for equality\n> > > + *\n> > > + * Two \\c Quantized objects are equal if their quantized values are equal.\n> > > + *\n> > > + * \\param other The \\c Quantized to compare with\n> > > + * \\return \\c true if equal, \\c false otherwise\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn bool Quantized::operator!=(const Quantized &other) const noexcept\n> > > + * \\brief Compare two quantized values for inequality\n> > > + *\n> > > + * Two \\c Quantized objects are not equal if their quantized values differ.\n> > > + *\n> > > + * \\param other The \\c Quantized to compare with\n> > > + * \\return \\c true if not equal, \\c false otherwise\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\var Quantized::quantized_\n> > > + * \\brief The raw quantized value\n> > > + *\n> > > + * This member stores the fixed-point or integer representation of the value, typically\n> > > + * used for interfacing with hardware or protocols that require compact formats.\n> > > + *\n> > > + * It must only be updated or accessed through a Quantizer in conjunction with\n> > > + * corresponding updates to \\c Quantized::value_.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\var Quantized::value_\n> > > + * \\brief The floating-point representation of the quantized value\n> > > + *\n> > > + * This member holds the floating-point equivalent of the quantized value, usually\n> > > + * for use in calculations or presentation to higher-level software components.\n> > > + *\n> > > + * It must only be updated or accessed through a Quantizer in conjunction with\n> > > + * corresponding updates to \\c Quantized::quantized_.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\class Quantizer\n> > > + * \\brief Interface for converting between floating-point and quantized types\n> > > + * \\tparam T The quantized type (e.g., uint16_t, int16_t).\n> > > + *\n> > > + * This abstract class defines the interface for quantizers that handle\n> > > + * conversions between floating-point values and their quantized representations.\n> > > + * Specific quantization handlers should implement this interface and provide\n> > > + * the toFloat and fromFloat methods specifically for their types.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\typedef Quantizer::qType\n> > > + * \\brief The underlying quantized type used by the quantizer\n> > > + *\n> > > + * This alias exposes the quantized type (e.g. \\c uint8_t or \\c int16_t) used internally\n> > > + * by the \\c Quantizer, which is helpful in template and debug contexts.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn Quantizer::fromFloat(T val)\n> > > + * \\brief Convert a floating-point value to its quantized representation\n> > > + *\n> > > + * \\param val The floating-point value\n> > > + * \\return The quantized equivalent\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn Quantizer::toFloat(T val)\n> > > + * \\brief Convert a quantized value to its floating-point representation\n> > > + *\n> > > + * \\param val The quantized value\n> > > + * \\return The floating-point equivalent\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn Quantizer::set(float val)\n> > > + * \\brief Set the value of a Quantized using a floating-point input\n> > > + *\n> > > + * This function updates both the quantized and floating-point\n> > > + * representations stored in the Quantized, storing the quantized\n> > > + * version of the float input.\n> > > + *\n> > > + * \\param val The floating-point value to set\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\fn Quantizer::set(T val)\n> > > + * \\brief Set the value of a Quantized using a quantized input\n> > > + *\n> > > + * This function updates both the quantized and floating-point\n> > > + * representations stored in the Quantized.\n> > > + *\n> > > + * \\param val The quantized value to set\n> > > + */\n> > > +\n> > > +} /* namespace ipa */\n> > > +\n> > > +} /* namespace libcamera */\n> > > diff --git a/src/ipa/libipa/quantized.h b/src/ipa/libipa/quantized.h\n> > > new file mode 100644\n> > > index 000000000000..1c1963cf0848\n> > > --- /dev/null\n> > > +++ b/src/ipa/libipa/quantized.h\n> > > @@ -0,0 +1,90 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2025, Ideas On Board.\n> > > + *\n> > > + * Helper class to manage conversions between floating point types and quantized\n> > > + * storage and representation of those values.\n> > > + */\n> > > +\n> > > +#pragma once\n> > > +\n> > > +#include \n> > > +#include \n> > > +#include \n> > > +#include \n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +namespace ipa {\n> > > +\n> > > +template\n> > > +class Quantizer;\n> > > +\n> > > +template\n> > > +struct Quantized {\n> > > + static_assert(std::is_arithmetic_v, \"Quantized: T must be an arithmetic type\");\n> > > +\n> > > + float value() const noexcept { return value_; }\n> > > + T quantized() const noexcept { return quantized_; }\n> > > +\n> > > + std::string toString() const\n> > \n> > I think I would implement `operator<<`.\n\n\nYes, I hadn't tackled that yet - but indeed it makes sense and should be\neasy.\n\n\n> > \n> > \n> > > + {\n> > > + using UT = std::make_unsigned_t;\n> > > + std::ostringstream oss;\n> > > +\n> > > + oss << \"Q:0x\" << std::hex << std::uppercase << std::setw(sizeof(T) * 2)\n> > > + << std::setfill('0');\n> > > +\n> > > + if constexpr (std::is_unsigned_v) {\n> > > + oss << static_cast(quantized_);\n> > > + } else {\n> > > + oss << static_cast(static_cast(quantized_));\n> > \n> > I think you can drop the `if` and keep only this second branch,\n> > it should work the same as far as I can see.\n> > \n\nHahah yes - I now can't work out why I branched. I remember it was\ndisplaying incorrectly on signed types ... but indeed there's no need to\nadd the branch!\n\n> > Also, since you cast to `unsigned`, there should probably be a\n> > static assert ensuring that the value fits. I'm guessing it's\n> > done to handle `{u,}int8_t`? In that case one could do\n> > \n> > oss << +static_cast(quantized_);\n> > \n> > to ensure that they are promoted to `int` and not printed as characters,\n> > and then there is no need to cast to `unsigned`.\n\nYes, at one point I was getting incorrect chars over the weekend so\nthat's where all this came from. I'll retest with your proposal thanks.\n\n\n\n> > \n> > \n> > > + }\n> > > +\n> > > + oss << \" F:\" << value_;\n> > > +\n> > > + return oss.str();\n> > > + }\n> > > +\n> > > + bool operator==(const Quantized &other) const noexcept\n> > \n> > You can drop the `` part as it is unnecessary inside the definition.\n> > \n> > \n> > > + {\n> > > + return quantized_ == other.quantized_ && value_ == other.value_;\n> > \n> > I worry a bit about floating point comparisons with `==`. And if `value_` is the\n> > same, shouldn't that imply that `quantized_` is as well?\n\nIn the unit tests I've added on top of this series for\nFixedPointQuantizer (I presume that makes it easy to guess where this\nseries goes after) I have to check floats are close enough but not\nexact, so I think you're right.\n\n> > > + }\n> > > +\n> > > + bool operator!=(const Quantized &other) const noexcept\n> > > + {\n> > > + return !(*this == other);\n> > > + }\n> > > +\n> > > +protected:\n> > > + T quantized_{};\n> > > + float value_{};\n> > > +};\n> > > +\n> > > +template\n> > > +class Quantizer : public Quantized\n> > > +{\n> > > +public:\n> > > + using qType = T;\n> > > +\n> > > + virtual ~Quantizer() = default;\n> > > +\n> > > + virtual T fromFloat(float val) const = 0;\n> > > + virtual float toFloat(T val) const = 0;\n> > \n> > I'm looking at the use cases, and I'm not sure that these need to be `virtual` in\n> > the first place. In fact, I think I'd completely eliminate the `Quantizer` class:\n> > \n> > template\n> > struct Quantized {\n\nHrm ... my only concern here is that now I have to make sure the Traits\nare known in the ipa_context.h 'globally', where in my version the\nQuantized is just storage.\n\nBut - that might not be a problem ...\n\n\n> > using quantized_type = typename Traits::quantized_type;\n> > \n> > Quantized() : Quantized(0.0f) { }\n> > Quantized(float x) { *this = x; }\n> > Quantized(quantized_type x) { *this = x; }\n> > Quantized& operator=(float x) { quantized_ = Traits::fromFloat(x); value_ = x; return *this; }\n> > Quantized& operator=(quantized_type x) { value_ = Traits::toFloat(x); quantized_ = x; return *this; }\n> > \n> > /* most others stay the same */\n> > \n> > private:\n> > quantized_type quantized_;\n> > float value_;\n> > };\n> > \n> > /* ... */\n> > \n> > struct RkISP1HueQTraits {\n> > using quantized_type = int8_t;\n> > static quantized_type fromFloat(float v) { ... }\n> > static float toFloat(quantized_type v) { ... }\n> > };\n> > \n> > /* ... */\n> > \n> > Quantized hue = ...;\n> > \n> > \n> > This has advantages in my opinion:\n> > \n> > * the default initialization will always be correct\n> > * currently it is assumed that 0.0f maps to \"0\" of the quantized type\n> > * type checking: you will not be able to assign/compare/etc. quantized quantities\n> > with different trait types\n> > * virtual functions are eliminated\n> > * if the traits type implements the methods inline, then those can be\n> > directly inlined, which is most likely worth it if it's just a simple division or similar\n> \n> Object slicing is also eliminated.\n\nI'll have to work through the above and try it out but I think that\nlooks promising.\n\nI really wanted to keep constexpr initialisation - and the virtuals here\nin Quantizer were killing that off.\n\n> > It is entirely possible that I am missing a use case that wouldn't work with\n> > the above, so I'm wondering if you have looked at an approach similar to the above?\n\nMy main use case goals are:\n - Quantized can not be modified without a type that knows how\n - Quantized can be *read* for either of the two raw types.\n\n - Quantizer() is just a base interface and should never be usable\n directly\n\n - Derived Quantizers like FixedPointQuantizer or\n ContrastQuantiser/HueQuantizer are the implementation per use case\n to set the values.\n\nI could also imagine gain codes being set through this sort of\ninterface, and if I can work through the signed bit extension issues\nI'm having on FixedPointQuantizer - I think things like\nContrastQuantizer end up being a specialisation of that.\n\nAs a nod to the future for my planned extended use cases I see:\n\n\n using Q1_7 = FixedPointQuantizer<1, 7, int8_t>;\n using UQ1_7 = FixedPointQuantizer<1, 7, uint8_t>;\n using Q12_4 = FixedPointQuantizer<12, 4, int16_t>;\n using UQ12_4 = FixedPointQuantizer<12, 4, uint16_t>;\n\n using ContrastQ = Q1_7;\n ... Now all the ContrastSaturationQuantizer implementation detail\n falls into a single line.\n\n \tconst auto &contrast = controls.get(controls::Contrast);\n \tif (contrast) {\n\t\tContrastQ value = *contrast;\n\t\t...\n\t}\n\n\nAnd going further:\n\ntemplate\nclass ScaledFixedPointQuantizer : public Quantizer;\n\n /* Hue is between -90 (-128) and +89.xx (+127) */\n using HueQ = ScaledFixedPointQuantizer;\n\n HueQ hue = *controls.get(controls::Hue);\n ... hue is now the quantized fixed point representation of the\n incoming control\n\n\n\nSo that's what I'm trying to conjure up - and have work in progress for.\n\nI think if the generic Traits for FixedPointQuantizer fall into place,\nit may not be such a concern having the Quantized type know those traits\nin the common ipa_context.h anyway...\n\n\n\n--\n\nKieran\n\n\n\n\n\n\n> \n> > Regards,\n> > Barnabás Pőcze\n> > \n> > \n> > > +\n> > > + void set(float val)\n> > > + {\n> > > + this->quantized_ = fromFloat(val);\n> > > + this->value_ = toFloat(this->quantized_);\n> > > + }\n> > > +\n> > > + void set(T val)\n> > > + {\n> > > + this->quantized_ = val;\n> > > + this->value_ = toFloat(val);\n> > > + }\n> > > +};\n> > > +\n> > > +} /* namespace ipa */\n> > > +\n> > > +} /* namespace libcamera */\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 E8205C3259\n\tfor ;\n\tMon, 27 Oct 2025 12:12:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 126F26074D;\n\tMon, 27 Oct 2025 13:12:07 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C89066069A\n\tfor ;\n\tMon, 27 Oct 2025 13:12:05 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EA0111661;\n\tMon, 27 Oct 2025 13:10:17 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"XzSYu7zu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1761567018;\n\tbh=/8W5542ajB0IvktsFZ5wliF0+F5VUWw/pd8qF2+9tHY=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=XzSYu7zuD9uMu9HkFRr8/AKqOz9tknB0XVKJyrpOqt/2SCmim5SKTtUvdKueMbn4C\n\tM996KXwj01yaQukfqbW55wFyHm+3ox79izJoZnpS2MZWwHuJJvoVIX8sFgJrLdKWz+\n\tdwmOByELIQ9w0yzws8wGfyoW3T0LYkKELH8yCeJs=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20251027094041.GC1544@pendragon.ideasonboard.com>","References":"<20251026233048.175689-1-kieran.bingham@ideasonboard.com>\n\t\n\t<20251026233048.175689-2-kieran.bingham@ideasonboard.com>\n\t<5a95febf-9505-41be-a702-c264ce562cee@ideasonboard.com>\n\t<20251027094041.GC1544@pendragon.ideasonboard.com>","Subject":"Re: [PATCH 1/6] libipa: Provide a Quantized type and Quantizer\n\tsupport","From":"Kieran Bingham ","Cc":"libcamera devel ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tLaurent Pinchart ","Date":"Mon, 27 Oct 2025 12:12:02 +0000","Message-ID":"<176156712298.2256500.4535037273042187843@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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\" "}}]