[{"id":33199,"web_url":"https://patchwork.libcamera.org/comment/33199/","msgid":"<Z5dQznjMXKFc9h_I@pyrite.rasen.tech>","date":"2025-01-27T09:24:30","subject":"Re: [PATCH v2 03/17] libipa: Add AWB algorithm base class","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Thu, Jan 23, 2025 at 12:40:53PM +0100, Stefan Klug wrote:\n> Add a class to provide a generic interface for auto white balance\n> algorithms. Concrete AWB algorithms are expected to subclass the\n> AwbAlgorithm class to implement their functionality.\n> \n> IPAs are expected to subclass the AwbStats class and implement the\n> necessary functions to give the algorithm access to the hardware\n> specific statistics data.\n> \n> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>\n\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n> \n> ---\n> \n> Changes in v2:\n> - Fixed language errors in the documentation and commit message\n> ---\n>  src/ipa/libipa/awb.cpp     | 137 +++++++++++++++++++++++++++++++++++++\n>  src/ipa/libipa/awb.h       |  51 ++++++++++++++\n>  src/ipa/libipa/meson.build |   2 +\n>  3 files changed, 190 insertions(+)\n>  create mode 100644 src/ipa/libipa/awb.cpp\n>  create mode 100644 src/ipa/libipa/awb.h\n> \n> diff --git a/src/ipa/libipa/awb.cpp b/src/ipa/libipa/awb.cpp\n> new file mode 100644\n> index 000000000000..4db97f6a10fc\n> --- /dev/null\n> +++ b/src/ipa/libipa/awb.cpp\n> @@ -0,0 +1,137 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024 Ideas on Board Oy\n> + *\n> + * Generic AWB algorithms\n> + */\n> +\n> +#include \"awb.h\"\n> +\n> +#include <libcamera/base/log.h>\n> +\n> +/**\n> + * \\file awb.h\n> + * \\brief Base classes for AWB algorithms\n> + */\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(Awb)\n> +\n> +namespace ipa {\n> +\n> +/**\n> + * \\class AwbResult\n> + * \\brief The result of an awb calculation\n> + *\n> + * This class holds the result of an auto white balance calculation.\n> + */\n> +\n> +/**\n> + * \\var AwbResult::gains\n> + * \\brief The calculated white balance gains\n> + */\n> +\n> +/**\n> + * \\var AwbResult::colourTemperature\n> + * \\brief The calculated colour temperature in Kelvin\n> + */\n> +\n> +/**\n> + * \\class AwbStats\n> + * \\brief An abstraction class wrapping hardware-specific AWB statistics\n> + *\n> + * Pipeline handlers using an AWB algorithm based on the AwbAlgorithm class need\n> + * to implement this class to give the algorithm access to the hardware-specific\n> + * statistics data.\n> + */\n> +\n> +/**\n> + * \\fn AwbStats::computeColourError\n> + * \\brief Compute an error value for when the given gains would be applied\n> + * \\param[in] gains The gains to apply\n> + *\n> + * Compute an error value (non-greyness) assuming the given \\a gains would be\n> + * applied. To keep the actual implementations computationally inexpensive,\n> + * the squared colour error shall be returned.\n> + *\n> + * If the awb statistics provide multiple zones, the sum over all zones needs to\n> + * calculated.\n> + *\n> + * \\return The computed error value\n> + */\n> +\n> +/**\n> + * \\fn AwbStats::getRGBMeans\n> + * \\brief Get RGB means of the statistics\n> + *\n> + * Fetch the RGB means from the statistics. The values of each channel are\n> + * dimensionless and only the ratios are used for further calculations. This is\n> + * used by the simple gray world model to calculate the gains to apply.\n> + *\n> + * \\return The RGB means\n> + */\n> +\n> +/**\n> + * \\class AwbAlgorithm\n> + * \\brief A base class for auto white balance algorithms\n> + *\n> + * This class is a base class for auto white balance algorithms. It defines an\n> + * interface for the algorithms to implement, and is used by the IPAs to\n> + * interact with the concrete implementation.\n> + */\n> +\n> +/**\n> + * \\fn AwbAlgorithm::init\n> + * \\brief Initialize the algorithm with the given tuning data\n> + * \\param[in] tuningData The tuning data to use for the algorithm\n> + *\n> + * \\return 0 on success, a negative error code otherwise\n> + */\n> +\n> +/**\n> + * \\fn AwbAlgorithm::calculateAwb\n> + * \\brief Calculate awb data from the given statistics\n> + * \\param[in] stats The statistics to use for the calculation\n> + * \\param[in] lux The lux value of the scene\n> + *\n> + * Calculate an AwbResult object from the given statistics and lux value. A \\a\n> + * lux value of 0 means it is unknown or invalid and the algorithm shall ignore\n> + * it.\n> + *\n> + * \\return The awb result\n> + */\n> +\n> +/**\n> + * \\fn AwbAlgorithm::gainsFromColourTemperature\n> + * \\brief Compute white balance gains from a colour temperature\n> + * \\param[in] colourTemperature The colour temperature in Kelvin\n> + *\n> + * Compute the white balance gains from a \\a colourTemperature. This function\n> + * does not take any statistics into account. It is used to compute the colour\n> + * gains when the user manually specifies a colour temperature.\n> + *\n> + * \\return The colour gains\n> + */\n> +\n> +/**\n> + * \\fn AwbAlgorithm::controls\n> + * \\brief Get the controls info map for this algorithm\n> + *\n> + * \\return The controls info map\n> + */\n> +\n> +/**\n> + * \\fn AwbAlgorithm::handleControls\n> + * \\param[in] controls The controls to handle\n> + * \\brief Handle the controls supplied in a request\n> + */\n> +\n> +/**\n> + * \\var AwbAlgorithm::controls_\n> + * \\brief Controls info map for the controls provided by the algorithm\n> + */\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/libipa/awb.h b/src/ipa/libipa/awb.h\n> new file mode 100644\n> index 000000000000..2dd471606ec4\n> --- /dev/null\n> +++ b/src/ipa/libipa/awb.h\n> @@ -0,0 +1,51 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024 Ideas on Board Oy\n> + *\n> + * Generic AWB algorithms\n> + */\n> +\n> +#pragma once\n> +\n> +#include <libcamera/controls.h>\n> +#include \"libcamera/internal/yaml_parser.h\"\n> +\n> +#include \"vector.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa {\n> +\n> +struct AwbResult {\n> +\tRGB<double> gains;\n> +\tdouble colourTemperature;\n> +};\n> +\n> +struct AwbStats {\n> +\tvirtual double computeColourError(const RGB<double> &gains) const = 0;\n> +\tvirtual RGB<double> getRGBMeans() const = 0;\n> +};\n> +\n> +class AwbAlgorithm\n> +{\n> +public:\n> +\tvirtual ~AwbAlgorithm() = default;\n> +\n> +\tvirtual int init(const YamlObject &tuningData) = 0;\n> +\tvirtual AwbResult calculateAwb(const AwbStats &stats, int lux) = 0;\n> +\tvirtual RGB<double> gainsFromColourTemperature(double colourTemperature) = 0;\n> +\n> +\tconst ControlInfoMap::Map &controls() const\n> +\t{\n> +\t\treturn controls_;\n> +\t}\n> +\n> +\tvirtual void handleControls([[maybe_unused]] const ControlList &controls) {}\n> +\n> +protected:\n> +\tControlInfoMap::Map controls_;\n> +};\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n> index f2b2f4be50db..03e879c5834f 100644\n> --- a/src/ipa/libipa/meson.build\n> +++ b/src/ipa/libipa/meson.build\n> @@ -3,6 +3,7 @@\n>  libipa_headers = files([\n>      'agc_mean_luminance.h',\n>      'algorithm.h',\n> +    'awb.h',\n>      'camera_sensor_helper.h',\n>      'colours.h',\n>      'exposure_mode_helper.h',\n> @@ -20,6 +21,7 @@ libipa_headers = files([\n>  libipa_sources = files([\n>      'agc_mean_luminance.cpp',\n>      'algorithm.cpp',\n> +    'awb.cpp',\n>      'camera_sensor_helper.cpp',\n>      'colours.cpp',\n>      'exposure_mode_helper.cpp',\n> -- \n> 2.43.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 F1705BDCC1\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 27 Jan 2025 09:24:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 00F9F6855D;\n\tMon, 27 Jan 2025 10:24:40 +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 C27116851B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Jan 2025 10:24:37 +0100 (CET)","from pyrite.rasen.tech (unknown [206.0.71.13])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2BB09352;\n\tMon, 27 Jan 2025 10:23:29 +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=\"Xsdpm6I5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1737969811;\n\tbh=T9PrVxZvy/KXRcJGR4+O8vBdKxUQoLnLDq25a1au4ew=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Xsdpm6I5KG3GWo5EkeNwpEnYIwpMfQQYLCwAFuIMeooDalqKzp20tcDxHO8Ce20lL\n\tNa6xDAWWaSWR7jxYBdmVQ7z6Lazqt4DsoFs2hGx1+m6Y9M2Fraia0nKySPDDlA5ASh\n\tF9J6TYzaY0J5whjHl6SRrh/A/yESRA4EmmnTNsTM=","Date":"Mon, 27 Jan 2025 04:24:30 -0500","From":"Paul Elder <paul.elder@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tDaniel Scally <dan.scally@ideasonboard.com>","Subject":"Re: [PATCH v2 03/17] libipa: Add AWB algorithm base class","Message-ID":"<Z5dQznjMXKFc9h_I@pyrite.rasen.tech>","References":"<20250123114204.79321-1-stefan.klug@ideasonboard.com>\n\t<20250123114204.79321-4-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20250123114204.79321-4-stefan.klug@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33416,"web_url":"https://patchwork.libcamera.org/comment/33416/","msgid":"<20250223221428.GA16159@pendragon.ideasonboard.com>","date":"2025-02-23T22:14:28","subject":"Re: [PATCH v2 03/17] libipa: Add AWB algorithm base class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Jan 23, 2025 at 12:40:53PM +0100, Stefan Klug wrote:\n> Add a class to provide a generic interface for auto white balance\n> algorithms. Concrete AWB algorithms are expected to subclass the\n> AwbAlgorithm class to implement their functionality.\n> \n> IPAs are expected to subclass the AwbStats class and implement the\n> necessary functions to give the algorithm access to the hardware\n> specific statistics data.\n> \n> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>\n> \n> ---\n> \n> Changes in v2:\n> - Fixed language errors in the documentation and commit message\n> ---\n>  src/ipa/libipa/awb.cpp     | 137 +++++++++++++++++++++++++++++++++++++\n>  src/ipa/libipa/awb.h       |  51 ++++++++++++++\n>  src/ipa/libipa/meson.build |   2 +\n>  3 files changed, 190 insertions(+)\n>  create mode 100644 src/ipa/libipa/awb.cpp\n>  create mode 100644 src/ipa/libipa/awb.h\n> \n> diff --git a/src/ipa/libipa/awb.cpp b/src/ipa/libipa/awb.cpp\n> new file mode 100644\n> index 000000000000..4db97f6a10fc\n> --- /dev/null\n> +++ b/src/ipa/libipa/awb.cpp\n> @@ -0,0 +1,137 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024 Ideas on Board Oy\n> + *\n> + * Generic AWB algorithms\n> + */\n> +\n> +#include \"awb.h\"\n> +\n> +#include <libcamera/base/log.h>\n> +\n> +/**\n> + * \\file awb.h\n> + * \\brief Base classes for AWB algorithms\n> + */\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(Awb)\n> +\n> +namespace ipa {\n> +\n> +/**\n> + * \\class AwbResult\n> + * \\brief The result of an awb calculation\n> + *\n> + * This class holds the result of an auto white balance calculation.\n> + */\n> +\n> +/**\n> + * \\var AwbResult::gains\n> + * \\brief The calculated white balance gains\n> + */\n> +\n> +/**\n> + * \\var AwbResult::colourTemperature\n> + * \\brief The calculated colour temperature in Kelvin\n> + */\n> +\n> +/**\n> + * \\class AwbStats\n> + * \\brief An abstraction class wrapping hardware-specific AWB statistics\n> + *\n> + * Pipeline handlers using an AWB algorithm based on the AwbAlgorithm class need\n> + * to implement this class to give the algorithm access to the hardware-specific\n> + * statistics data.\n> + */\n> +\n> +/**\n> + * \\fn AwbStats::computeColourError\n> + * \\brief Compute an error value for when the given gains would be applied\n> + * \\param[in] gains The gains to apply\n> + *\n> + * Compute an error value (non-greyness) assuming the given \\a gains would be\n> + * applied. To keep the actual implementations computationally inexpensive,\n> + * the squared colour error shall be returned.\n> + *\n> + * If the awb statistics provide multiple zones, the sum over all zones needs to\n> + * calculated.\n> + *\n> + * \\return The computed error value\n> + */\n> +\n> +/**\n> + * \\fn AwbStats::getRGBMeans\n> + * \\brief Get RGB means of the statistics\n> + *\n> + * Fetch the RGB means from the statistics. The values of each channel are\n> + * dimensionless and only the ratios are used for further calculations. This is\n\nThat's not entirely true. The grey world algorithm clamps the red and\nblue means to a minimum value of 1.0 to avoid divisions by zero. If this\nfunction were to return r/g and b/g ratios instead of r and g means in\npixel value units, we would have a problem.\n\nI'm not sure how to best fix it, if the documentation should simply be\ncorrected, or if the greyworld code should be adjusted. Could you please\nsend a patch ?\n\n> + * used by the simple gray world model to calculate the gains to apply.\n> + *\n> + * \\return The RGB means\n> + */\n> +\n> +/**\n> + * \\class AwbAlgorithm\n> + * \\brief A base class for auto white balance algorithms\n> + *\n> + * This class is a base class for auto white balance algorithms. It defines an\n> + * interface for the algorithms to implement, and is used by the IPAs to\n> + * interact with the concrete implementation.\n> + */\n> +\n> +/**\n> + * \\fn AwbAlgorithm::init\n> + * \\brief Initialize the algorithm with the given tuning data\n> + * \\param[in] tuningData The tuning data to use for the algorithm\n> + *\n> + * \\return 0 on success, a negative error code otherwise\n> + */\n> +\n> +/**\n> + * \\fn AwbAlgorithm::calculateAwb\n> + * \\brief Calculate awb data from the given statistics\n> + * \\param[in] stats The statistics to use for the calculation\n> + * \\param[in] lux The lux value of the scene\n> + *\n> + * Calculate an AwbResult object from the given statistics and lux value. A \\a\n> + * lux value of 0 means it is unknown or invalid and the algorithm shall ignore\n> + * it.\n> + *\n> + * \\return The awb result\n> + */\n> +\n> +/**\n> + * \\fn AwbAlgorithm::gainsFromColourTemperature\n> + * \\brief Compute white balance gains from a colour temperature\n> + * \\param[in] colourTemperature The colour temperature in Kelvin\n> + *\n> + * Compute the white balance gains from a \\a colourTemperature. This function\n> + * does not take any statistics into account. It is used to compute the colour\n> + * gains when the user manually specifies a colour temperature.\n> + *\n> + * \\return The colour gains\n> + */\n> +\n> +/**\n> + * \\fn AwbAlgorithm::controls\n> + * \\brief Get the controls info map for this algorithm\n> + *\n> + * \\return The controls info map\n> + */\n> +\n> +/**\n> + * \\fn AwbAlgorithm::handleControls\n> + * \\param[in] controls The controls to handle\n> + * \\brief Handle the controls supplied in a request\n> + */\n> +\n> +/**\n> + * \\var AwbAlgorithm::controls_\n> + * \\brief Controls info map for the controls provided by the algorithm\n> + */\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/libipa/awb.h b/src/ipa/libipa/awb.h\n> new file mode 100644\n> index 000000000000..2dd471606ec4\n> --- /dev/null\n> +++ b/src/ipa/libipa/awb.h\n> @@ -0,0 +1,51 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024 Ideas on Board Oy\n> + *\n> + * Generic AWB algorithms\n> + */\n> +\n> +#pragma once\n> +\n> +#include <libcamera/controls.h>\n> +#include \"libcamera/internal/yaml_parser.h\"\n> +\n> +#include \"vector.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa {\n> +\n> +struct AwbResult {\n> +\tRGB<double> gains;\n> +\tdouble colourTemperature;\n> +};\n> +\n> +struct AwbStats {\n> +\tvirtual double computeColourError(const RGB<double> &gains) const = 0;\n> +\tvirtual RGB<double> getRGBMeans() const = 0;\n> +};\n> +\n> +class AwbAlgorithm\n> +{\n> +public:\n> +\tvirtual ~AwbAlgorithm() = default;\n> +\n> +\tvirtual int init(const YamlObject &tuningData) = 0;\n> +\tvirtual AwbResult calculateAwb(const AwbStats &stats, int lux) = 0;\n> +\tvirtual RGB<double> gainsFromColourTemperature(double colourTemperature) = 0;\n> +\n> +\tconst ControlInfoMap::Map &controls() const\n> +\t{\n> +\t\treturn controls_;\n> +\t}\n> +\n> +\tvirtual void handleControls([[maybe_unused]] const ControlList &controls) {}\n> +\n> +protected:\n> +\tControlInfoMap::Map controls_;\n> +};\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n> index f2b2f4be50db..03e879c5834f 100644\n> --- a/src/ipa/libipa/meson.build\n> +++ b/src/ipa/libipa/meson.build\n> @@ -3,6 +3,7 @@\n>  libipa_headers = files([\n>      'agc_mean_luminance.h',\n>      'algorithm.h',\n> +    'awb.h',\n>      'camera_sensor_helper.h',\n>      'colours.h',\n>      'exposure_mode_helper.h',\n> @@ -20,6 +21,7 @@ libipa_headers = files([\n>  libipa_sources = files([\n>      'agc_mean_luminance.cpp',\n>      'algorithm.cpp',\n> +    'awb.cpp',\n>      'camera_sensor_helper.cpp',\n>      'colours.cpp',\n>      'exposure_mode_helper.cpp',","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 711BCC324E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 23 Feb 2025 22:14:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8739D686B6;\n\tSun, 23 Feb 2025 23:14:47 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E6AA6686A7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 23 Feb 2025 23:14:45 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A7D09496;\n\tSun, 23 Feb 2025 23:13:19 +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=\"wQXVPMy+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1740348799;\n\tbh=pBO/vfDz7sptKzmFuY748ZCuWptuED9n6WQWQZkbpYE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=wQXVPMy+/xrcZUwLZppgpID3pBdA43h8jrCY8wmWEbDMoOfgA5+ozArbbzvdOwL8N\n\tat/lhS5fx7mYwZ7nAPPimD4h+6dHy/7QxHHiiavCWnpoVOmbsEZeA7hZAHqwj4MU5a\n\t9k++UOgEcGkOAfmlFrBpIttrEwBEQ7282HRk8u/8=","Date":"Mon, 24 Feb 2025 00:14:28 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tDaniel Scally <dan.scally@ideasonboard.com>","Subject":"Re: [PATCH v2 03/17] libipa: Add AWB algorithm base class","Message-ID":"<20250223221428.GA16159@pendragon.ideasonboard.com>","References":"<20250123114204.79321-1-stefan.klug@ideasonboard.com>\n\t<20250123114204.79321-4-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250123114204.79321-4-stefan.klug@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33453,"web_url":"https://patchwork.libcamera.org/comment/33453/","msgid":"<85msebmac0.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-02-24T19:06:23","subject":"Re: [PATCH v2 03/17] libipa: Add AWB algorithm base class","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Stefan,\n\nStefan Klug <stefan.klug@ideasonboard.com> writes:\n\n> Add a class to provide a generic interface for auto white balance\n> algorithms. Concrete AWB algorithms are expected to subclass the\n> AwbAlgorithm class to implement their functionality.\n>\n> IPAs are expected to subclass the AwbStats class and implement the\n> necessary functions to give the algorithm access to the hardware\n> specific statistics data.\n>\n> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>\n>\n> ---\n>\n> Changes in v2:\n> - Fixed language errors in the documentation and commit message\n> ---\n>  src/ipa/libipa/awb.cpp     | 137 +++++++++++++++++++++++++++++++++++++\n>  src/ipa/libipa/awb.h       |  51 ++++++++++++++\n>  src/ipa/libipa/meson.build |   2 +\n>  3 files changed, 190 insertions(+)\n>  create mode 100644 src/ipa/libipa/awb.cpp\n>  create mode 100644 src/ipa/libipa/awb.h\n>\n> diff --git a/src/ipa/libipa/awb.cpp b/src/ipa/libipa/awb.cpp\n> new file mode 100644\n> index 000000000000..4db97f6a10fc\n> --- /dev/null\n> +++ b/src/ipa/libipa/awb.cpp\n> @@ -0,0 +1,137 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024 Ideas on Board Oy\n> + *\n> + * Generic AWB algorithms\n> + */\n> +\n> +#include \"awb.h\"\n> +\n> +#include <libcamera/base/log.h>\n> +\n> +/**\n> + * \\file awb.h\n> + * \\brief Base classes for AWB algorithms\n> + */\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(Awb)\n> +\n> +namespace ipa {\n> +\n> +/**\n> + * \\class AwbResult\n> + * \\brief The result of an awb calculation\n> + *\n> + * This class holds the result of an auto white balance calculation.\n> + */\n> +\n> +/**\n> + * \\var AwbResult::gains\n> + * \\brief The calculated white balance gains\n> + */\n> +\n> +/**\n> + * \\var AwbResult::colourTemperature\n> + * \\brief The calculated colour temperature in Kelvin\n> + */\n> +\n> +/**\n> + * \\class AwbStats\n> + * \\brief An abstraction class wrapping hardware-specific AWB statistics\n> + *\n> + * Pipeline handlers using an AWB algorithm based on the AwbAlgorithm class need\n> + * to implement this class to give the algorithm access to the hardware-specific\n> + * statistics data.\n> + */\n> +\n> +/**\n> + * \\fn AwbStats::computeColourError\n> + * \\brief Compute an error value for when the given gains would be applied\n> + * \\param[in] gains The gains to apply\n> + *\n> + * Compute an error value (non-greyness) assuming the given \\a gains would be\n> + * applied. To keep the actual implementations computationally inexpensive,\n> + * the squared colour error shall be returned.\n> + *\n> + * If the awb statistics provide multiple zones, the sum over all zones needs to\n> + * calculated.\n> + *\n> + * \\return The computed error value\n> + */\n> +\n> +/**\n> + * \\fn AwbStats::getRGBMeans\n> + * \\brief Get RGB means of the statistics\n> + *\n> + * Fetch the RGB means from the statistics. The values of each channel are\n> + * dimensionless and only the ratios are used for further calculations. This is\n> + * used by the simple gray world model to calculate the gains to apply.\n> + *\n> + * \\return The RGB means\n> + */\n> +\n> +/**\n> + * \\class AwbAlgorithm\n> + * \\brief A base class for auto white balance algorithms\n> + *\n> + * This class is a base class for auto white balance algorithms. It defines an\n> + * interface for the algorithms to implement, and is used by the IPAs to\n> + * interact with the concrete implementation.\n> + */\n> +\n> +/**\n> + * \\fn AwbAlgorithm::init\n> + * \\brief Initialize the algorithm with the given tuning data\n> + * \\param[in] tuningData The tuning data to use for the algorithm\n> + *\n> + * \\return 0 on success, a negative error code otherwise\n> + */\n> +\n> +/**\n> + * \\fn AwbAlgorithm::calculateAwb\n> + * \\brief Calculate awb data from the given statistics\n> + * \\param[in] stats The statistics to use for the calculation\n> + * \\param[in] lux The lux value of the scene\n> + *\n> + * Calculate an AwbResult object from the given statistics and lux value. A \\a\n> + * lux value of 0 means it is unknown or invalid and the algorithm shall ignore\n> + * it.\n> + *\n> + * \\return The awb result\n> + */\n> +\n> +/**\n> + * \\fn AwbAlgorithm::gainsFromColourTemperature\n> + * \\brief Compute white balance gains from a colour temperature\n> + * \\param[in] colourTemperature The colour temperature in Kelvin\n> + *\n> + * Compute the white balance gains from a \\a colourTemperature. This function\n> + * does not take any statistics into account. It is used to compute the colour\n> + * gains when the user manually specifies a colour temperature.\n> + *\n> + * \\return The colour gains\n> + */\n> +\n> +/**\n> + * \\fn AwbAlgorithm::controls\n> + * \\brief Get the controls info map for this algorithm\n> + *\n> + * \\return The controls info map\n> + */\n> +\n> +/**\n> + * \\fn AwbAlgorithm::handleControls\n> + * \\param[in] controls The controls to handle\n> + * \\brief Handle the controls supplied in a request\n> + */\n> +\n> +/**\n> + * \\var AwbAlgorithm::controls_\n> + * \\brief Controls info map for the controls provided by the algorithm\n> + */\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/libipa/awb.h b/src/ipa/libipa/awb.h\n> new file mode 100644\n> index 000000000000..2dd471606ec4\n> --- /dev/null\n> +++ b/src/ipa/libipa/awb.h\n> @@ -0,0 +1,51 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024 Ideas on Board Oy\n> + *\n> + * Generic AWB algorithms\n> + */\n> +\n> +#pragma once\n> +\n> +#include <libcamera/controls.h>\n> +#include \"libcamera/internal/yaml_parser.h\"\n> +\n> +#include \"vector.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa {\n> +\n> +struct AwbResult {\n> +\tRGB<double> gains;\n> +\tdouble colourTemperature;\n> +};\n> +\n> +struct AwbStats {\n> +\tvirtual double computeColourError(const RGB<double> &gains) const = 0;\n> +\tvirtual RGB<double> getRGBMeans() const = 0;\n> +};\n\nThis causes a build failure with gcc 11.5.0 for me:\n\n  In file included from ../src/ipa/libipa/awb_grey.h:13,\n                   from ../src/ipa/libipa/awb_grey.cpp:8:\n  ../src/ipa/libipa/awb.h:27:8: error: 'struct libcamera::ipa::AwbStats' has virtual functions and accessible non-virtual destructor [-Werror=non-virtual-dtor]\n     27 | struct AwbStats {\n        |        ^~~~~~~~\n  cc1plus: all warnings being treated as errors\n\ngcc 14 doesn't suffer from this problem.  Adding\n\n  virtual ~AwbStats() = default;\n\nfixes the build with gcc 11.5.0.\n\n> +class AwbAlgorithm\n> +{\n> +public:\n> +\tvirtual ~AwbAlgorithm() = default;\n> +\n> +\tvirtual int init(const YamlObject &tuningData) = 0;\n> +\tvirtual AwbResult calculateAwb(const AwbStats &stats, int lux) = 0;\n> +\tvirtual RGB<double> gainsFromColourTemperature(double colourTemperature) = 0;\n> +\n> +\tconst ControlInfoMap::Map &controls() const\n> +\t{\n> +\t\treturn controls_;\n> +\t}\n> +\n> +\tvirtual void handleControls([[maybe_unused]] const ControlList &controls) {}\n> +\n> +protected:\n> +\tControlInfoMap::Map controls_;\n> +};\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n> index f2b2f4be50db..03e879c5834f 100644\n> --- a/src/ipa/libipa/meson.build\n> +++ b/src/ipa/libipa/meson.build\n> @@ -3,6 +3,7 @@\n>  libipa_headers = files([\n>      'agc_mean_luminance.h',\n>      'algorithm.h',\n> +    'awb.h',\n>      'camera_sensor_helper.h',\n>      'colours.h',\n>      'exposure_mode_helper.h',\n> @@ -20,6 +21,7 @@ libipa_headers = files([\n>  libipa_sources = files([\n>      'agc_mean_luminance.cpp',\n>      'algorithm.cpp',\n> +    'awb.cpp',\n>      'camera_sensor_helper.cpp',\n>      'colours.cpp',\n>      'exposure_mode_helper.cpp',","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 A20B1C324E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Feb 2025 19:06:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A1170686FA;\n\tMon, 24 Feb 2025 20:06:30 +0100 (CET)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0081D686F4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Feb 2025 20:06:28 +0100 (CET)","from mail-wm1-f72.google.com (mail-wm1-f72.google.com\n\t[209.85.128.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-156-O9E0SxAqND6QvL0Wuy-BEQ-1; Mon, 24 Feb 2025 14:06:26 -0500","by mail-wm1-f72.google.com with SMTP id\n\t5b1f17b1804b1-43aafafe6b7so3994095e9.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Feb 2025 11:06:26 -0800 (PST)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-38f258b434fsm32903215f8f.16.2025.02.24.11.06.23\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 24 Feb 2025 11:06:23 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"jCigOONR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1740423987;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=bpou8GAb5XRQvnDMlEufcQSdKy4TS9QNWJxV0gcNmYM=;\n\tb=jCigOONRcb31W+DdIHqpPCbkp/AJLJsm5eYGyrXG12UMP4jSzotDNnaSKEnzk4sHFxrjqb\n\to1NVYCwGEIVQJGz3gfziHg7VgXWvHhk74YiGIpCCqNGK8TV3+lEYEoyRlbushxKw/rqNlP\n\txHtzvy3mzTL00VzYvwytab/2Qm758do=","X-MC-Unique":"O9E0SxAqND6QvL0Wuy-BEQ-1","X-Mimecast-MFC-AGG-ID":"O9E0SxAqND6QvL0Wuy-BEQ_1740423985","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1740423985; x=1741028785;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=bpou8GAb5XRQvnDMlEufcQSdKy4TS9QNWJxV0gcNmYM=;\n\tb=TRKDc+GkV1LdQ0arOllAJxPnKCTI/WcAbmdT3sTLuA7xrz6raz+KQTzZbmkoy3NQ40\n\toMnAEEI53AdJaeKgQRMds+mBIWjLPN4pxlZoXtReMLZUEA4Skt/jxWyi/kQ85Hasn7gr\n\ti9DeKUfP3MKxAx/VHkNBOLxB2eGjXWaMiaPgv5g6SQWEJgeiWmJZxLZRIH/maAjvuU1P\n\taN6eLpwWxucxBPTt2i3eL/evZKie0hoEgcjIA2OiqumqNgwAnLjNYQ4tDvqqp6wTOow/\n\tw6SJnvnBdYVbINloTvZ97ZZUwZijdgND1zG19Ugb5gjSVrhzgNcP7NonzVTYLtcLLz6O\n\tp9tg==","X-Gm-Message-State":"AOJu0Ywb5AoTxme9qSiIxb6fp7RYnLtxB/kPgRP+cnKzdOvsi+muhPBG\n\t6SMM/NQSBQ8A9RoLy22ZmTWIFd5YJFic+b97XF1UMLp0H0AgkLEcteWL6oAUaS9k/mH/ofYUlx/\n\tJx4SJoKxojutsgRdBepZMA6qtPH6Mb7C9sL0Bi+qluJnqZNKuKdPXxIp8uMooga/L7KQ5vqk=","X-Gm-Gg":"ASbGncsIFEi9j/97wXfyGCW1KYPtB+DqZsmuO+HSyn6QnuK9gNacVDAzBfB/9PvGBiV\n\tPYXbdqjnkQC4ShH+SN3Yzm1PUPiBC7EfA+9bzojLTVt1gSY+JdGdoef3XZKKrTI8/tZPe5jPcTA\n\t8eA59pV0mcOGRmbOeiaahJ4bfzxQFjHHvnRaPkqz0yiroGPfcOouMLVeSG3JmkOsL6f18N8mpOp\n\trrWxW66NBQbf+bmVipuArMZI3ve65qXAdgZd0TDQcmsYVh+lIY7Fn3+08hx3p2J/NOHl8+58Qyl\n\t+uKkvJ3bwd0qrxq4qUnkZ4ES++Jd88G3GHPg3+fUYvZUjMq080ykksrJw7yrtsLZLP5B","X-Received":["by 2002:a05:600c:4ed1:b0:439:8ada:e3b0 with SMTP id\n\t5b1f17b1804b1-439ae21636emr116882645e9.19.1740423984834; \n\tMon, 24 Feb 2025 11:06:24 -0800 (PST)","by 2002:a05:600c:4ed1:b0:439:8ada:e3b0 with SMTP id\n\t5b1f17b1804b1-439ae21636emr116882365e9.19.1740423984352; \n\tMon, 24 Feb 2025 11:06:24 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IHK4n1X69cQ56OVKdenMyVfZp9PF1XdNIrMj6C2LR+re98y+bDjc18K6WBI/VL2OSpuLIjZ5A==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,  Daniel Scally\n\t<dan.scally@ideasonboard.com>","Subject":"Re: [PATCH v2 03/17] libipa: Add AWB algorithm base class","In-Reply-To":"<20250123114204.79321-4-stefan.klug@ideasonboard.com> (Stefan\n\tKlug's message of \"Thu, 23 Jan 2025 12:40:53 +0100\")","References":"<20250123114204.79321-1-stefan.klug@ideasonboard.com>\n\t<20250123114204.79321-4-stefan.klug@ideasonboard.com>","Date":"Mon, 24 Feb 2025 20:06:23 +0100","Message-ID":"<85msebmac0.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"qvhQ70r9qiubPVFkx5XV0WhK2KgxydENVx-IZzcjm5w_1740423985","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":33471,"web_url":"https://patchwork.libcamera.org/comment/33471/","msgid":"<20250224215231.GP6778@pendragon.ideasonboard.com>","date":"2025-02-24T21:52:31","subject":"Re: [PATCH v2 03/17] libipa: Add AWB algorithm base class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Milan,\n\nOn Mon, Feb 24, 2025 at 08:06:23PM +0100, Milan Zamazal wrote:\n> Stefan Klug writes:\n> \n> > Add a class to provide a generic interface for auto white balance\n> > algorithms. Concrete AWB algorithms are expected to subclass the\n> > AwbAlgorithm class to implement their functionality.\n> >\n> > IPAs are expected to subclass the AwbStats class and implement the\n> > necessary functions to give the algorithm access to the hardware\n> > specific statistics data.\n> >\n> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> > Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>\n> >\n> > ---\n> >\n> > Changes in v2:\n> > - Fixed language errors in the documentation and commit message\n> > ---\n> >  src/ipa/libipa/awb.cpp     | 137 +++++++++++++++++++++++++++++++++++++\n> >  src/ipa/libipa/awb.h       |  51 ++++++++++++++\n> >  src/ipa/libipa/meson.build |   2 +\n> >  3 files changed, 190 insertions(+)\n> >  create mode 100644 src/ipa/libipa/awb.cpp\n> >  create mode 100644 src/ipa/libipa/awb.h\n> >\n> > diff --git a/src/ipa/libipa/awb.cpp b/src/ipa/libipa/awb.cpp\n> > new file mode 100644\n> > index 000000000000..4db97f6a10fc\n> > --- /dev/null\n> > +++ b/src/ipa/libipa/awb.cpp\n> > @@ -0,0 +1,137 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2024 Ideas on Board Oy\n> > + *\n> > + * Generic AWB algorithms\n> > + */\n> > +\n> > +#include \"awb.h\"\n> > +\n> > +#include <libcamera/base/log.h>\n> > +\n> > +/**\n> > + * \\file awb.h\n> > + * \\brief Base classes for AWB algorithms\n> > + */\n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DEFINE_CATEGORY(Awb)\n> > +\n> > +namespace ipa {\n> > +\n> > +/**\n> > + * \\class AwbResult\n> > + * \\brief The result of an awb calculation\n> > + *\n> > + * This class holds the result of an auto white balance calculation.\n> > + */\n> > +\n> > +/**\n> > + * \\var AwbResult::gains\n> > + * \\brief The calculated white balance gains\n> > + */\n> > +\n> > +/**\n> > + * \\var AwbResult::colourTemperature\n> > + * \\brief The calculated colour temperature in Kelvin\n> > + */\n> > +\n> > +/**\n> > + * \\class AwbStats\n> > + * \\brief An abstraction class wrapping hardware-specific AWB statistics\n> > + *\n> > + * Pipeline handlers using an AWB algorithm based on the AwbAlgorithm class need\n> > + * to implement this class to give the algorithm access to the hardware-specific\n> > + * statistics data.\n> > + */\n> > +\n> > +/**\n> > + * \\fn AwbStats::computeColourError\n> > + * \\brief Compute an error value for when the given gains would be applied\n> > + * \\param[in] gains The gains to apply\n> > + *\n> > + * Compute an error value (non-greyness) assuming the given \\a gains would be\n> > + * applied. To keep the actual implementations computationally inexpensive,\n> > + * the squared colour error shall be returned.\n> > + *\n> > + * If the awb statistics provide multiple zones, the sum over all zones needs to\n> > + * calculated.\n> > + *\n> > + * \\return The computed error value\n> > + */\n> > +\n> > +/**\n> > + * \\fn AwbStats::getRGBMeans\n> > + * \\brief Get RGB means of the statistics\n> > + *\n> > + * Fetch the RGB means from the statistics. The values of each channel are\n> > + * dimensionless and only the ratios are used for further calculations. This is\n> > + * used by the simple gray world model to calculate the gains to apply.\n> > + *\n> > + * \\return The RGB means\n> > + */\n> > +\n> > +/**\n> > + * \\class AwbAlgorithm\n> > + * \\brief A base class for auto white balance algorithms\n> > + *\n> > + * This class is a base class for auto white balance algorithms. It defines an\n> > + * interface for the algorithms to implement, and is used by the IPAs to\n> > + * interact with the concrete implementation.\n> > + */\n> > +\n> > +/**\n> > + * \\fn AwbAlgorithm::init\n> > + * \\brief Initialize the algorithm with the given tuning data\n> > + * \\param[in] tuningData The tuning data to use for the algorithm\n> > + *\n> > + * \\return 0 on success, a negative error code otherwise\n> > + */\n> > +\n> > +/**\n> > + * \\fn AwbAlgorithm::calculateAwb\n> > + * \\brief Calculate awb data from the given statistics\n> > + * \\param[in] stats The statistics to use for the calculation\n> > + * \\param[in] lux The lux value of the scene\n> > + *\n> > + * Calculate an AwbResult object from the given statistics and lux value. A \\a\n> > + * lux value of 0 means it is unknown or invalid and the algorithm shall ignore\n> > + * it.\n> > + *\n> > + * \\return The awb result\n> > + */\n> > +\n> > +/**\n> > + * \\fn AwbAlgorithm::gainsFromColourTemperature\n> > + * \\brief Compute white balance gains from a colour temperature\n> > + * \\param[in] colourTemperature The colour temperature in Kelvin\n> > + *\n> > + * Compute the white balance gains from a \\a colourTemperature. This function\n> > + * does not take any statistics into account. It is used to compute the colour\n> > + * gains when the user manually specifies a colour temperature.\n> > + *\n> > + * \\return The colour gains\n> > + */\n> > +\n> > +/**\n> > + * \\fn AwbAlgorithm::controls\n> > + * \\brief Get the controls info map for this algorithm\n> > + *\n> > + * \\return The controls info map\n> > + */\n> > +\n> > +/**\n> > + * \\fn AwbAlgorithm::handleControls\n> > + * \\param[in] controls The controls to handle\n> > + * \\brief Handle the controls supplied in a request\n> > + */\n> > +\n> > +/**\n> > + * \\var AwbAlgorithm::controls_\n> > + * \\brief Controls info map for the controls provided by the algorithm\n> > + */\n> > +\n> > +} /* namespace ipa */\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/ipa/libipa/awb.h b/src/ipa/libipa/awb.h\n> > new file mode 100644\n> > index 000000000000..2dd471606ec4\n> > --- /dev/null\n> > +++ b/src/ipa/libipa/awb.h\n> > @@ -0,0 +1,51 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2024 Ideas on Board Oy\n> > + *\n> > + * Generic AWB algorithms\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include <libcamera/controls.h>\n> > +#include \"libcamera/internal/yaml_parser.h\"\n> > +\n> > +#include \"vector.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +namespace ipa {\n> > +\n> > +struct AwbResult {\n> > +\tRGB<double> gains;\n> > +\tdouble colourTemperature;\n> > +};\n> > +\n> > +struct AwbStats {\n> > +\tvirtual double computeColourError(const RGB<double> &gains) const = 0;\n> > +\tvirtual RGB<double> getRGBMeans() const = 0;\n> > +};\n> \n> This causes a build failure with gcc 11.5.0 for me:\n> \n>   In file included from ../src/ipa/libipa/awb_grey.h:13,\n>                    from ../src/ipa/libipa/awb_grey.cpp:8:\n>   ../src/ipa/libipa/awb.h:27:8: error: 'struct libcamera::ipa::AwbStats' has virtual functions and accessible non-virtual destructor [-Werror=non-virtual-dtor]\n>      27 | struct AwbStats {\n>         |        ^~~~~~~~\n>   cc1plus: all warnings being treated as errors\n\nI wondered why my gcc 11.5.0 doesn't warn about this. Adding\n-Wnon-virtual-dtor triggered the warning. I'll send a patch to enable\nit.\n\n> gcc 14 doesn't suffer from this problem.  Adding\n> \n>   virtual ~AwbStats() = default;\n> \n> fixes the build with gcc 11.5.0.\n\nI think declaring a protected non-virtual descriptor would be better, as\nwe never need to destroy instances from a base class pointer. I'll send\na patch as well.\n\n> > +class AwbAlgorithm\n> > +{\n> > +public:\n> > +\tvirtual ~AwbAlgorithm() = default;\n> > +\n> > +\tvirtual int init(const YamlObject &tuningData) = 0;\n> > +\tvirtual AwbResult calculateAwb(const AwbStats &stats, int lux) = 0;\n> > +\tvirtual RGB<double> gainsFromColourTemperature(double colourTemperature) = 0;\n> > +\n> > +\tconst ControlInfoMap::Map &controls() const\n> > +\t{\n> > +\t\treturn controls_;\n> > +\t}\n> > +\n> > +\tvirtual void handleControls([[maybe_unused]] const ControlList &controls) {}\n> > +\n> > +protected:\n> > +\tControlInfoMap::Map controls_;\n> > +};\n> > +\n> > +} /* namespace ipa */\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build\n> > index f2b2f4be50db..03e879c5834f 100644\n> > --- a/src/ipa/libipa/meson.build\n> > +++ b/src/ipa/libipa/meson.build\n> > @@ -3,6 +3,7 @@\n> >  libipa_headers = files([\n> >      'agc_mean_luminance.h',\n> >      'algorithm.h',\n> > +    'awb.h',\n> >      'camera_sensor_helper.h',\n> >      'colours.h',\n> >      'exposure_mode_helper.h',\n> > @@ -20,6 +21,7 @@ libipa_headers = files([\n> >  libipa_sources = files([\n> >      'agc_mean_luminance.cpp',\n> >      'algorithm.cpp',\n> > +    'awb.cpp',\n> >      'camera_sensor_helper.cpp',\n> >      'colours.cpp',\n> >      'exposure_mode_helper.cpp',","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 9DBC8C324E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Feb 2025 21:52:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CA4266870E;\n\tMon, 24 Feb 2025 22:52:52 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1086861856\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Feb 2025 22:52:51 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 27058220;\n\tMon, 24 Feb 2025 22:51:24 +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=\"P37idQig\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1740433884;\n\tbh=pbI1Dsw3jNsjMotx0mYhIB9csRO9oUMwEkTKmAN9eXs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=P37idQigBXaOLIHdp9s5aIcG+MJo4KbF7T/B3WKxr2uw7ECWg5hB+VdHuW84SAJLJ\n\tEHRmKQtQ7XH1AwAYxVn++VsYV+jpyKQ4OmrYvfZbLHOJx074b20zGSiTIWNtr7d4GS\n\tSLiCDatPzoF6nVHRL6bw4C6LyqC65c25vt941N0A=","Date":"Mon, 24 Feb 2025 23:52:31 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Milan Zamazal <mzamazal@redhat.com>","Cc":"Stefan Klug <stefan.klug@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org,\n\tDaniel Scally <dan.scally@ideasonboard.com>","Subject":"Re: [PATCH v2 03/17] libipa: Add AWB algorithm base class","Message-ID":"<20250224215231.GP6778@pendragon.ideasonboard.com>","References":"<20250123114204.79321-1-stefan.klug@ideasonboard.com>\n\t<20250123114204.79321-4-stefan.klug@ideasonboard.com>\n\t<85msebmac0.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<85msebmac0.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]