[{"id":39151,"web_url":"https://patchwork.libcamera.org/comment/39151/","msgid":"","date":"2026-06-17T09:30:51","subject":"Re: [PATCH 05/10] ipa: libipa: Add GammaAlgorithm class","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Dan\n\nOn Tue, Jun 16, 2026 at 07:41:39AM +0100, Daniel Scally wrote:\n> Add a base GammaAlgorithm class that can be used by IPA specific\n> gamma algorithms to reduce the amount of work that they need to\n> implement.\n>\n> Signed-off-by: Daniel Scally \n> ---\n> src/ipa/libipa/gamma.cpp | 248 +++++++++++++++++++++++++++++++++++++++++++++\n> src/ipa/libipa/gamma.h | 93 +++++++++++++++++\n> src/ipa/libipa/meson.build | 2 +\n> 3 files changed, 343 insertions(+)\n>\n> diff --git a/src/ipa/libipa/gamma.cpp b/src/ipa/libipa/gamma.cpp\n> new file mode 100644\n> index 0000000000000000000000000000000000000000..f84f4cf7049f695f6a4ea21e0bcf851aef0f2c70\n> --- /dev/null\n> +++ b/src/ipa/libipa/gamma.cpp\n> @@ -0,0 +1,248 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2026 Ideas on Board Oy\n> + *\n> + * libIPA Gamma correction algorithm\n> + */\n> +\n> +#include \"gamma.h\"\n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/value_node.h\"\n> +\n> +/**\n> + * \\file gamma.h\n> + * \\brief libipa implementation of a gamma curve correction algorithm\n> + */\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa {\n> +\n> +LOG_DEFINE_CATEGORY(Gamma)\n> +\n> +namespace gamma {\n> +\n> +/**\n> + * \\struct ActiveState\n> + * \\brief Active gamma correction algorithm state\n> + *\n> + * \\var ActiveState::gamma\n> + * \\brief The gamma correction value applied as 1.0/gamma\n> + */\n> +\n> +/**\n> + * \\struct FrameContext\n> + * \\brief Per-frame gamma correction settings\n> + *\n> + * \\var FrameContext::gamma\n> + * \\brief The gamma correction value applied for this frame\n\nIf you want to specify 1.0/gamma in the documentation of\nActiveState::gamma please do the same here. Personally I wouldn't\nmention 1.0/gamma as it comes by the definition of what a compression gamma\ncurve is.\n\n> + *\n> + * \\var FrameContext::update\n> + * \\brief A flag instructing the algorithm to push an update to the hardware\n> + */\n> +\n> +} /* namespace gamma */\n> +\n> +/**\n> + * \\brief The default gamma correction value\n> + */\n> +const float kDefaultGamma = 2.2f;\n> +\n> +/**\n> + * \\class GammaAlgorithmBase\n> + * \\brief Base class for GammaAlgorithm to implement non-templated functions\n> + *\n> + * This base class for GammeaAlgorithm allows us to implement non templated\n> + * functions. IPA specific implementations shall derive from GammaAlgorithm and\n> + * not this class.\n> + */\n> +\n> +/**\n> + * \\fn GammaAlgorithmBase::GammaAlgorithmBase\n> + * \\brief Construct an instance of the class\n> + * \\param[in] nLutNodes Set the number of function knee-points expected by the\n> + * IPA algorithm\n> + */\n> +\n> +/**\n> + * \\brief Initialise the algorithm with the given tuning data\n> + * \\param[out] controls The ControlList into which this algorithm's supported\n> + * controls will be emplaced.\n> + * \\param[in] tuningData The tuning data to use with the algorithm\n> + * \\param[in] segments\tA vector of segment spacings to define a custom\n\nIs this weirdly spaced or does it only shows up this way in the diff ?\n\n> + * X coordinate system for the curve\n> + *\n> + * Parse \\a tuningData and \\a segments to initialize the gamma correction curve.\n> + * The tuning data may contain a default gamma value to use; otherwise the value\n> + * of \\a kDefaultGamma will be taken as the default. \\a segments may provide a\n\nBefore describing the segmentation methods, I think we should tell\nthat gamma is implemented as a pwl applied on a list of sampling\npoints. We should re-use (a slightly reworked version of) the\ndocumentation from the rkisp1 implementation (remember to remove it in\nthe next patches from rkisp1)\n\n * This algorithm implements gamma out curve compression with a\n * gamma value specified in \\a tuningData or with a default gamma\n * value of \\a kDefaultGamma.\n *\n * Gamma correction is internally implemented as a piecewise linear function\n * applied on a number of knots whose position is described by the optional\n * \\a segments argument.\n *\n * Useful links:\n * - https://www.cambridgeincolour.com/tutorials/gamma-correction.htm\n * - https://en.wikipedia.org/wiki/SRGB\n\n> + * view into an array of segment spacings, which can be used to vary the X\n\nGive the above introduction, this can be simplified as:\n\nThe optional \\a segments argument defines the segmentation of the gamma\nLUT sampling points by describing each segments relative length.\n\n> + * co-ordinate of the gamma correction curve. For example, if the piecewise\n> + * linear function of the correction curve is expected to have 16 knee-points, a\n\nI think the key is that there are 16 -equally spaced- knee-points\nhere. Give the above introduction I would simplify this as\n\nFor example, if the gamma correction has to be applied on 16 equally\nspaced sampling points, a \\a segments array like so:\n\n> + * \\a segments array like so:\n> + *\n> + * [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]\n> + *\n> + * would result in evenly spaced knee-points along the X-axis. Hardware may\n\nBreak the paragraph and insert an empty line after X-axis.\n\n> + * expect the knee-points to be spaced more densely towards the start of the\n> + * curve and more sparsely towards the end, in which case an alternative array\n> + * might bev\n\n\"might be:\"\n\n> + *\n> + * [1, 1, 1, 1, 2, 2, 2, 2, 4, 4, 4, 8, 8, 8, 8, 8]\n> + *\n> + * As the values in \\a segments represent the distance between two knee-points\n> + * relative to the total distance between the first and last point, the length\n> + * of \\a segments should be equal to the number of knee-points minus one. As a\n\nBreak the paragraph maybe ? Also\n\n If an IPA implementation doesn't provide \\a segments, the\n GammaAlgorithm class constructs and evenly-spaced default one.\n\n> + * convenience, a hardware-specific algorithm deriving from this class may omit\n> + * \\a segments, in which case an evenly-spaced default will be constructed.\n> + *\n> + * IPA modules are expected to call this function as part of their\n> + * implementation of Algorithm::init()\n> + *\n> + * @return 0 on success, a negative error code otherwise\n> + */\n> +int GammaAlgorithmBase::init(ControlInfoMap::Map &controls, const ValueNode &tuningData,\n> +\t\t\t Span segments)\n> +{\n> +\t/*\n> +\t * If the caller doesn't pass in a segment list we simply construct one\n> +\t * with equally spaced segments. We need one less segment than we have\n> +\t * LUT nodes.\n> +\t */\n> +\tunsigned int expectedNSegments = nLutNodes_ - 1;\n> +\n> +\tif (segments.empty())\n> +\t\tfor (unsigned int i = 0; i < expectedNSegments; i++)\n> +\t\t\tsegments_.push_back(1);\n> +\telse\n> +\t\tsegments_.assign(segments.begin(), segments.end());\n\nfor complex statements like this on I think we usually use {} (at\nleast in Linux)\n\n if () {\n for ()\n ...\n } else {\n ...\n }\n\n> +\n> +\tif (segments_.size() != expectedNSegments)\n> +\t\treturn -EINVAL;\n> +\n> +\tsegmentsSum_ = std::accumulate(segments_.begin(), segments_.end(), 0.0f);\n> +\n> +\tdefaultGamma_ = tuningData[\"gamma\"].get(kDefaultGamma);\n> +\tcontrols[&controls::Gamma] = ControlInfo(0.1f, 10.0f, defaultGamma_);\n\nshould the max gamma value be derived from the UQ template argument ?\n\n> +\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * \\brief Configure the gamma correction algorithm\n> + * \\param[out] state The gamma correction algorithm's active state\n> + *\n> + * Reset to the default gamma correction value.\n> + *\n> + * IPA modules are expected to call this function as part of their\n> + * implementation of Algorithm::configure()\n> + */\n> +void GammaAlgorithmBase::configure(gamma::ActiveState &state)\n> +{\n> +\tstate.gamma = defaultGamma_;\n> +}\n> +\n> +/**\n> + * \\brief Queue a request to the gamma correction algorithm\n> + * \\param[in] state The algorithm's active state\n> + * \\param[in] frame The current frame number\n> + * \\param[in] context The algorithm's frame context\n> + * \\param[in] controls The ControlList that was queued with the request\n> + *\n> + * Queue a new request to the gamma correction algorithm and handle any relevant\n> + * controls that were queued. The only control currently handled is:\n> + *\n> + * - controls::Gamma\n> + *\n> + * If a control with that ID is queued the value is stored in \\a state and\n> + * \\a context.\n> + *\n> + * IPA modules are expected to call this function as part of their\n> + * implementation of Algorithm::queueRequest()\n> + */\n> +void GammaAlgorithmBase::queueRequest(gamma::ActiveState &state,\n> +\t\t\t\t const uint32_t frame,\n> +\t\t\t\t gamma::FrameContext &context,\n> +\t\t\t\t const ControlList &controls)\n> +{\n> +\tif (frame == 0)\n> +\t\tcontext.update = true;\n> +\n> +\tconst auto &gamma = controls.get(controls::Gamma);\n> +\tif (gamma) {\n> +\t\tstate.gamma = *gamma;\n> +\t\tcontext.update = true;\n> +\t\tLOG(Gamma, Info) << \"Set gamma to \" << *gamma;\n> +\t}\n> +\n> +\tcontext.gamma = state.gamma;\n> +}\n> +\n> +/**\n> + * \\brief Populate metadata with the gamma correction values for a frame\n> + * \\param[in] context The frame context\n> + * \\param[out] metadata The ControlList of metadata for a frame\n> + *\n> + * Report the gamma value used to calculate the correction curve that was\n> + * applied to a frame.\n> + */\n> +void GammaAlgorithmBase::process(gamma::FrameContext &context, ControlList &metadata)\n> +{\n> +\tmetadata.set(controls::Gamma, context.gamma);\n> +}\n> +\n> +/**\n> + * \\var GammaAlgorithmBase::nLutNodes_\n> + * \\brief The number of knee-points in the gamma correction curve\n> + */\n> +\n> +/**\n> + * \\var GammaAlgorithmBase::defaultGamma_\n> + * \\brief The default gamma parameter used at stream start\n\ns/used at stream start// ?\n\n> + */\n> +\n> +/**\n> + * \\var GammaAlgorithmBase::segments_\n> + * \\brief The vector of segment sizes describing the space between knee-points\n> + */\n> +\n> +/**\n> + * \\var GammaAlgorithmBase::segmentsSum_\n> + * \\brief The sum of \\a GammaAlgorithmBase::segments_\n> + */\n> +\n> +/**\n> + * \\class GammaAlgorithm\n> + * \\brief The libipa gamma correction algorithm\n> + * \\tparam nLutNodes The number of knee-points in the algorithm's function\n\n * \\tparam nLutNodes The number of knee-points in the gamma correction\n * pwl approximation\n\nif you want to use \"knee points\" ?\nOr\n\n * \\tparam nLutNodes The number of gamma LUT sampling points\n\n?\n\n> + * \\tparam UQ The fixedpoint representation of the function's values\n\nof the gamma correction values\n\n> + *\n> + * Gamma correction adjusts for the differences in the way light is perceived\n> + * by a camera and the human eye by applying a function to the input values.\n> + * The GammaAlgorithm class facilitates this by building a piecewise linear\n> + * function from a gamma parameter and supplying it in the hardware-specific\n> + * formats defined by the IPA algorithms.\n\nAh, that's the intro I was suggesting (in the wrong place most\nprobably). Could you see if any of what I suggested still applies\nthere to better clarify the usage of 'segments' ?\n\n> + *\n> + * IPA modules are expected to store an instance of GammaAlgorithm as a class\n> + * member, templated with the format and number of knee-points in the PWL\n> + * expected by their hardware and then call its functions in their overload of\n> + * the Algorithm class's function.\n> + *\n> + * When an application queues a new value for the gamma parameter with a\n> + * Request, the GammaAlgorithm will recalculate and populate the new LUT to be\n> + * sent to the ISP.\n> + */\n> +\n> +/**\n> + * \\fn GammaAlgorithm::prepare()\n> + * \\tparam T The type of data expected by the hardware's look-up table\n> + * \\param[in] context The frame context\n> + * \\param[out] lut The Span into which to place the calculated look-up table\n> + */\n> +\n> +} /* namespace ipa */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/libipa/gamma.h b/src/ipa/libipa/gamma.h\n> new file mode 100644\n> index 0000000000000000000000000000000000000000..2b449d9ec41bc96f576e664afca33acb8267a8e6\n> --- /dev/null\n> +++ b/src/ipa/libipa/gamma.h\n> @@ -0,0 +1,93 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2026 Ideas on Board Oy\n> + *\n> + * libIPA Gamma correction algorithm\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/value_node.h\"\n> +\n> +#include \"fixedpoint.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa {\n> +\n> +LOG_DECLARE_CATEGORY(Gamma)\n> +\n> +namespace gamma {\n> +\n> +struct ActiveState {\n> +\tdouble gamma;\n> +};\n> +\n> +struct FrameContext {\n> +\tdouble gamma;\n> +\tbool update;\n> +};\n> +\n> +} /* namespace gamma */\n> +\n> +class GammaAlgorithmBase\n> +{\n> +public:\n> +\tGammaAlgorithmBase(unsigned int nLutNodes)\n> +\t\t: nLutNodes_(nLutNodes)\n> +\t{\n> +\t}\n> +\n> +\tint init(ControlInfoMap::Map &controls, const ValueNode &tuningData,\n> +\t\t Span segments = {});\n> +\n> +\tvoid configure(gamma::ActiveState &state);\n> +\tvoid queueRequest(gamma::ActiveState &state, const uint32_t frame,\n> +\t\t\t gamma::FrameContext &context, const ControlList &controls);\n> +\tvoid process(gamma::FrameContext &context, ControlList &metadata);\n> +\n> +protected:\n> +\tunsigned int nLutNodes_;\n> +\tfloat defaultGamma_;\n> +\tstd::vector segments_;\n> +\tunsigned int segmentsSum_;\n> +};\n> +\n> +template\n> +class GammaAlgorithm : public GammaAlgorithmBase\n> +{\n> +public:\n> +\tGammaAlgorithm()\n> +\t\t: GammaAlgorithmBase(nLutNodes)\n> +\t{\n> +\t}\n> +\n> +\ttemplate\n> +\tvoid prepare(gamma::FrameContext &context, Span lut)\n> +\t{\n> +\t\tfloat x = 0;\n> +\n> +\t\tfor (unsigned int i = 0; i < nLutNodes_; i++) {\n> +\t\t\tfloat gamma = std::pow(x / segmentsSum_,\n> +\t\t\t\t\t 1.0 / context.gamma);\n> +\t\t\tlut[i] = UQ(gamma).quantized();\n> +\n> +\t\t\tLOG(Gamma, Debug) << \"LUT[\" << i << \"]=\" << gamma << \"(\" << lut[i] << \")\";\n\nI bet you can easily break this to multiple lines.\n\nWith documentation adjusted:\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> +\n> +\t\t\tif (i < segments_.size())\n> +\t\t\t\tx += segments_[i];\n> +\t\t}\n> +\t}\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 da6ea0c5e13000f78b2196c7334610c350f1ad13..565da9be9059f2167d107061d643e58202e655ef 100644\n> --- a/src/ipa/libipa/meson.build\n> +++ b/src/ipa/libipa/meson.build\n> @@ -12,6 +12,7 @@ libipa_headers = files([\n> 'exposure_mode_helper.h',\n> 'fc_queue.h',\n> 'fixedpoint.h',\n> + 'gamma.h',\n> 'histogram.h',\n> 'interpolator.h',\n> 'lsc.h',\n> @@ -37,6 +38,7 @@ libipa_sources = files([\n> 'exposure_mode_helper.cpp',\n> 'fc_queue.cpp',\n> 'fixedpoint.cpp',\n> + 'gamma.cpp',\n> 'histogram.cpp',\n> 'interpolator.cpp',\n> 'lsc.cpp',\n>\n> --\n> 2.43.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 8A1B0BF415\n\tfor ;\n\tWed, 17 Jun 2026 09:30:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B0F2D626F3;\n\tWed, 17 Jun 2026 11:30:56 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1542E60579\n\tfor ;\n\tWed, 17 Jun 2026 11:30:55 +0200 (CEST)","from ideasonboard.com (mob-109-113-4-199.net.vodafone.it\n\t[109.113.4.199])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 900A32F8;\n\tWed, 17 Jun 2026 11:30:20 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"W21msgnJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1781688620;\n\tbh=xRwRi+1baEJez/IXmgYdH0YWHh88e1H7uatZeyNE7gU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=W21msgnJPPzshLurYtzkLVnJtnBDh2Jc5KO/mOTBT1lzNOLCYFb4yLRygp0LofB20\n\t4QcBm4DcZ55akC8Ckb0UCZJZ1DS7q1swaYsGkOjGcytk6MqqLjtE6MSvnPC2+Ggnca\n\tP6H6wl8dPcPDlRAEsq3cY+qzYtB+KZ7lGxsL00i0=","Date":"Wed, 17 Jun 2026 11:30:51 +0200","From":"Jacopo Mondi ","To":"Daniel Scally ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 05/10] ipa: libipa: Add GammaAlgorithm class","Message-ID":"","References":"<20260616-ipu3-libipa-rework-v1-0-d4448b54f1d8@ideasonboard.com>\n\t<20260616-ipu3-libipa-rework-v1-5-d4448b54f1d8@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20260616-ipu3-libipa-rework-v1-5-d4448b54f1d8@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":39188,"web_url":"https://patchwork.libcamera.org/comment/39188/","msgid":"<46302008-7a5a-4ed8-8046-5fcc695d0d72@ideasonboard.com>","date":"2026-06-19T08:41:49","subject":"Re: [PATCH 05/10] ipa: libipa: Add GammaAlgorithm class","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Jacopo - thanks for all the reviews\n\nOn 17/06/2026 10:30, Jacopo Mondi wrote:\n> Hi Dan\n> \n> On Tue, Jun 16, 2026 at 07:41:39AM +0100, Daniel Scally wrote:\n>> Add a base GammaAlgorithm class that can be used by IPA specific\n>> gamma algorithms to reduce the amount of work that they need to\n>> implement.\n>>\n>> Signed-off-by: Daniel Scally \n>> ---\n>> src/ipa/libipa/gamma.cpp | 248 +++++++++++++++++++++++++++++++++++++++++++++\n>> src/ipa/libipa/gamma.h | 93 +++++++++++++++++\n>> src/ipa/libipa/meson.build | 2 +\n>> 3 files changed, 343 insertions(+)\n>>\n>> diff --git a/src/ipa/libipa/gamma.cpp b/src/ipa/libipa/gamma.cpp\n>> new file mode 100644\n>> index 0000000000000000000000000000000000000000..f84f4cf7049f695f6a4ea21e0bcf851aef0f2c70\n>> --- /dev/null\n>> +++ b/src/ipa/libipa/gamma.cpp\n>> @@ -0,0 +1,248 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2026 Ideas on Board Oy\n>> + *\n>> + * libIPA Gamma correction algorithm\n>> + */\n>> +\n>> +#include \"gamma.h\"\n>> +\n>> +#include \n>> +\n>> +#include \n>> +\n>> +#include \"libcamera/internal/value_node.h\"\n>> +\n>> +/**\n>> + * \\file gamma.h\n>> + * \\brief libipa implementation of a gamma curve correction algorithm\n>> + */\n>> +\n>> +namespace libcamera {\n>> +\n>> +namespace ipa {\n>> +\n>> +LOG_DEFINE_CATEGORY(Gamma)\n>> +\n>> +namespace gamma {\n>> +\n>> +/**\n>> + * \\struct ActiveState\n>> + * \\brief Active gamma correction algorithm state\n>> + *\n>> + * \\var ActiveState::gamma\n>> + * \\brief The gamma correction value applied as 1.0/gamma\n>> + */\n>> +\n>> +/**\n>> + * \\struct FrameContext\n>> + * \\brief Per-frame gamma correction settings\n>> + *\n>> + * \\var FrameContext::gamma\n>> + * \\brief The gamma correction value applied for this frame\n> \n> If you want to specify 1.0/gamma in the documentation of\n> ActiveState::gamma please do the same here. Personally I wouldn't\n> mention 1.0/gamma as it comes by the definition of what a compression gamma\n> curve is.\n\nOkedokey, I'll drop it from ActiveState::gamma\n> \n>> + *\n>> + * \\var FrameContext::update\n>> + * \\brief A flag instructing the algorithm to push an update to the hardware\n>> + */\n>> +\n>> +} /* namespace gamma */\n>> +\n>> +/**\n>> + * \\brief The default gamma correction value\n>> + */\n>> +const float kDefaultGamma = 2.2f;\n>> +\n>> +/**\n>> + * \\class GammaAlgorithmBase\n>> + * \\brief Base class for GammaAlgorithm to implement non-templated functions\n>> + *\n>> + * This base class for GammeaAlgorithm allows us to implement non templated\n>> + * functions. IPA specific implementations shall derive from GammaAlgorithm and\n>> + * not this class.\n>> + */\n>> +\n>> +/**\n>> + * \\fn GammaAlgorithmBase::GammaAlgorithmBase\n>> + * \\brief Construct an instance of the class\n>> + * \\param[in] nLutNodes Set the number of function knee-points expected by the\n>> + * IPA algorithm\n>> + */\n>> +\n>> +/**\n>> + * \\brief Initialise the algorithm with the given tuning data\n>> + * \\param[out] controls The ControlList into which this algorithm's supported\n>> + * controls will be emplaced.\n>> + * \\param[in] tuningData The tuning data to use with the algorithm\n>> + * \\param[in] segments\tA vector of segment spacings to define a custom\n> \n> Is this weirdly spaced or does it only shows up this way in the diff ?\n\nIt's a tab! Good catch.\n> \n>> + * X coordinate system for the curve\n>> + *\n>> + * Parse \\a tuningData and \\a segments to initialize the gamma correction curve.\n>> + * The tuning data may contain a default gamma value to use; otherwise the value\n>> + * of \\a kDefaultGamma will be taken as the default. \\a segments may provide a\n> \n> Before describing the segmentation methods, I think we should tell\n> that gamma is implemented as a pwl applied on a list of sampling\n> points. We should re-use (a slightly reworked version of) the\n> documentation from the rkisp1 implementation (remember to remove it in\n> the next patches from rkisp1)\n\nYou're right that this could do with expanding even though the comment on \nGammaAlgorithm::GammaAlgorithm() has the intro, I'll work on it and take in the suggestions below \nwhere I can.\n\n> \n> * This algorithm implements gamma out curve compression with a\n> * gamma value specified in \\a tuningData or with a default gamma\n> * value of \\a kDefaultGamma.\n> *\n> * Gamma correction is internally implemented as a piecewise linear function\n> * applied on a number of knots whose position is described by the optional\n> * \\a segments argument.\n> *\n> * Useful links:\n> * - https://www.cambridgeincolour.com/tutorials/gamma-correction.htm\n> * - https://en.wikipedia.org/wiki/SRGB\n> \n>> + * view into an array of segment spacings, which can be used to vary the X\n> \n> Give the above introduction, this can be simplified as:\n> \n> The optional \\a segments argument defines the segmentation of the gamma\n> LUT sampling points by describing each segments relative length.\n> \n>> + * co-ordinate of the gamma correction curve. For example, if the piecewise\n>> + * linear function of the correction curve is expected to have 16 knee-points, a\n> \n> I think the key is that there are 16 -equally spaced- knee-points\n> here. Give the above introduction I would simplify this as\n> \n> For example, if the gamma correction has to be applied on 16 equally\n> spaced sampling points, a \\a segments array like so:\n> \n>> + * \\a segments array like so:\n>> + *\n>> + * [1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1]\n>> + *\n>> + * would result in evenly spaced knee-points along the X-axis. Hardware may\n> \n> Break the paragraph and insert an empty line after X-axis.\n> \n>> + * expect the knee-points to be spaced more densely towards the start of the\n>> + * curve and more sparsely towards the end, in which case an alternative array\n>> + * might bev\n> \n> \"might be:\"\n> \n>> + *\n>> + * [1, 1, 1, 1, 2, 2, 2, 2, 4, 4, 4, 8, 8, 8, 8, 8]\n>> + *\n>> + * As the values in \\a segments represent the distance between two knee-points\n>> + * relative to the total distance between the first and last point, the length\n>> + * of \\a segments should be equal to the number of knee-points minus one. As a\n> \n> Break the paragraph maybe ? Also\n> \n> If an IPA implementation doesn't provide \\a segments, the\n> GammaAlgorithm class constructs and evenly-spaced default one.\n> \n>> + * convenience, a hardware-specific algorithm deriving from this class may omit\n>> + * \\a segments, in which case an evenly-spaced default will be constructed.\n>> + *\n>> + * IPA modules are expected to call this function as part of their\n>> + * implementation of Algorithm::init()\n>> + *\n>> + * @return 0 on success, a negative error code otherwise\n>> + */\n>> +int GammaAlgorithmBase::init(ControlInfoMap::Map &controls, const ValueNode &tuningData,\n>> +\t\t\t Span segments)\n>> +{\n>> +\t/*\n>> +\t * If the caller doesn't pass in a segment list we simply construct one\n>> +\t * with equally spaced segments. We need one less segment than we have\n>> +\t * LUT nodes.\n>> +\t */\n>> +\tunsigned int expectedNSegments = nLutNodes_ - 1;\n>> +\n>> +\tif (segments.empty())\n>> +\t\tfor (unsigned int i = 0; i < expectedNSegments; i++)\n>> +\t\t\tsegments_.push_back(1);\n>> +\telse\n>> +\t\tsegments_.assign(segments.begin(), segments.end());\n> \n> for complex statements like this on I think we usually use {} (at\n> least in Linux)\n\nAck!\n> \n> if () {\n> for ()\n> ...\n> } else {\n> ...\n> }\n> \n>> +\n>> +\tif (segments_.size() != expectedNSegments)\n>> +\t\treturn -EINVAL;\n>> +\n>> +\tsegmentsSum_ = std::accumulate(segments_.begin(), segments_.end(), 0.0f);\n>> +\n>> +\tdefaultGamma_ = tuningData[\"gamma\"].get(kDefaultGamma);\n>> +\tcontrols[&controls::Gamma] = ControlInfo(0.1f, 10.0f, defaultGamma_);\n> \n> should the max gamma value be derived from the UQ template argument ?\n\nNo I don't think so. Because we use UQ<0, N> the UQ::TraitsType::max value wouldn't be appropriate \nbecause it will always be 1.0...the gamma parameter just controls the steepness of the curve from \n0.0 to 1.0 which comes closer and closer to being a right angle as you increase the parameter, but I \nthink there's no theoretical \"maximum\", so whatever number we choose here is somewhat arbitrary.\n\nIn theory we could make use of the fact that the UQ<0, N> can only represent values within a certain \nprecision, and make an assessment about the maximum gain that could be practically applied based on \nthe ability of that format to represent the different points with different quantized values...but \nexperimenting with that, the rkisp1's UQ<0, 10> format is sufficient to accommodate gamma parameters \nup to ~96.0 which I think makes no sense.\n\nPerhaps we should just select and document a min and max, even if they're somewhat arbitrary? \nPlotting some curves to play around, even 10.0 looks a bit \"why would anyone choose this setting\" \nand I lean towards a lower maximum making more sense. A gamma of 1.0 is a straight line, which is \nperhaps a more reasonable minimum too, unless we want to allow an inverted curve?\n\nThanks\nDan\n\n> \n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +/**\n>> + * \\brief Configure the gamma correction algorithm\n>> + * \\param[out] state The gamma correction algorithm's active state\n>> + *\n>> + * Reset to the default gamma correction value.\n>> + *\n>> + * IPA modules are expected to call this function as part of their\n>> + * implementation of Algorithm::configure()\n>> + */\n>> +void GammaAlgorithmBase::configure(gamma::ActiveState &state)\n>> +{\n>> +\tstate.gamma = defaultGamma_;\n>> +}\n>> +\n>> +/**\n>> + * \\brief Queue a request to the gamma correction algorithm\n>> + * \\param[in] state The algorithm's active state\n>> + * \\param[in] frame The current frame number\n>> + * \\param[in] context The algorithm's frame context\n>> + * \\param[in] controls The ControlList that was queued with the request\n>> + *\n>> + * Queue a new request to the gamma correction algorithm and handle any relevant\n>> + * controls that were queued. The only control currently handled is:\n>> + *\n>> + * - controls::Gamma\n>> + *\n>> + * If a control with that ID is queued the value is stored in \\a state and\n>> + * \\a context.\n>> + *\n>> + * IPA modules are expected to call this function as part of their\n>> + * implementation of Algorithm::queueRequest()\n>> + */\n>> +void GammaAlgorithmBase::queueRequest(gamma::ActiveState &state,\n>> +\t\t\t\t const uint32_t frame,\n>> +\t\t\t\t gamma::FrameContext &context,\n>> +\t\t\t\t const ControlList &controls)\n>> +{\n>> +\tif (frame == 0)\n>> +\t\tcontext.update = true;\n>> +\n>> +\tconst auto &gamma = controls.get(controls::Gamma);\n>> +\tif (gamma) {\n>> +\t\tstate.gamma = *gamma;\n>> +\t\tcontext.update = true;\n>> +\t\tLOG(Gamma, Info) << \"Set gamma to \" << *gamma;\n>> +\t}\n>> +\n>> +\tcontext.gamma = state.gamma;\n>> +}\n>> +\n>> +/**\n>> + * \\brief Populate metadata with the gamma correction values for a frame\n>> + * \\param[in] context The frame context\n>> + * \\param[out] metadata The ControlList of metadata for a frame\n>> + *\n>> + * Report the gamma value used to calculate the correction curve that was\n>> + * applied to a frame.\n>> + */\n>> +void GammaAlgorithmBase::process(gamma::FrameContext &context, ControlList &metadata)\n>> +{\n>> +\tmetadata.set(controls::Gamma, context.gamma);\n>> +}\n>> +\n>> +/**\n>> + * \\var GammaAlgorithmBase::nLutNodes_\n>> + * \\brief The number of knee-points in the gamma correction curve\n>> + */\n>> +\n>> +/**\n>> + * \\var GammaAlgorithmBase::defaultGamma_\n>> + * \\brief The default gamma parameter used at stream start\n> \n> s/used at stream start// ?\n> \n>> + */\n>> +\n>> +/**\n>> + * \\var GammaAlgorithmBase::segments_\n>> + * \\brief The vector of segment sizes describing the space between knee-points\n>> + */\n>> +\n>> +/**\n>> + * \\var GammaAlgorithmBase::segmentsSum_\n>> + * \\brief The sum of \\a GammaAlgorithmBase::segments_\n>> + */\n>> +\n>> +/**\n>> + * \\class GammaAlgorithm\n>> + * \\brief The libipa gamma correction algorithm\n>> + * \\tparam nLutNodes The number of knee-points in the algorithm's function\n> \n> * \\tparam nLutNodes The number of knee-points in the gamma correction\n> * pwl approximation\n> \n> if you want to use \"knee points\" ?\n> Or\n> \n> * \\tparam nLutNodes The number of gamma LUT sampling points\n> \n> ?\n> \n>> + * \\tparam UQ The fixedpoint representation of the function's values\n> \n> of the gamma correction values\n> \n>> + *\n>> + * Gamma correction adjusts for the differences in the way light is perceived\n>> + * by a camera and the human eye by applying a function to the input values.\n>> + * The GammaAlgorithm class facilitates this by building a piecewise linear\n>> + * function from a gamma parameter and supplying it in the hardware-specific\n>> + * formats defined by the IPA algorithms.\n> \n> Ah, that's the intro I was suggesting (in the wrong place most\n> probably). Could you see if any of what I suggested still applies\n> there to better clarify the usage of 'segments' ?\n> \n>> + *\n>> + * IPA modules are expected to store an instance of GammaAlgorithm as a class\n>> + * member, templated with the format and number of knee-points in the PWL\n>> + * expected by their hardware and then call its functions in their overload of\n>> + * the Algorithm class's function.\n>> + *\n>> + * When an application queues a new value for the gamma parameter with a\n>> + * Request, the GammaAlgorithm will recalculate and populate the new LUT to be\n>> + * sent to the ISP.\n>> + */\n>> +\n>> +/**\n>> + * \\fn GammaAlgorithm::prepare()\n>> + * \\tparam T The type of data expected by the hardware's look-up table\n>> + * \\param[in] context The frame context\n>> + * \\param[out] lut The Span into which to place the calculated look-up table\n>> + */\n>> +\n>> +} /* namespace ipa */\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/ipa/libipa/gamma.h b/src/ipa/libipa/gamma.h\n>> new file mode 100644\n>> index 0000000000000000000000000000000000000000..2b449d9ec41bc96f576e664afca33acb8267a8e6\n>> --- /dev/null\n>> +++ b/src/ipa/libipa/gamma.h\n>> @@ -0,0 +1,93 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2026 Ideas on Board Oy\n>> + *\n>> + * libIPA Gamma correction algorithm\n>> + */\n>> +\n>> +#pragma once\n>> +\n>> +#include \n>> +#include \n>> +\n>> +#include \n>> +#include \n>> +\n>> +#include \n>> +\n>> +#include \"libcamera/internal/value_node.h\"\n>> +\n>> +#include \"fixedpoint.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +namespace ipa {\n>> +\n>> +LOG_DECLARE_CATEGORY(Gamma)\n>> +\n>> +namespace gamma {\n>> +\n>> +struct ActiveState {\n>> +\tdouble gamma;\n>> +};\n>> +\n>> +struct FrameContext {\n>> +\tdouble gamma;\n>> +\tbool update;\n>> +};\n>> +\n>> +} /* namespace gamma */\n>> +\n>> +class GammaAlgorithmBase\n>> +{\n>> +public:\n>> +\tGammaAlgorithmBase(unsigned int nLutNodes)\n>> +\t\t: nLutNodes_(nLutNodes)\n>> +\t{\n>> +\t}\n>> +\n>> +\tint init(ControlInfoMap::Map &controls, const ValueNode &tuningData,\n>> +\t\t Span segments = {});\n>> +\n>> +\tvoid configure(gamma::ActiveState &state);\n>> +\tvoid queueRequest(gamma::ActiveState &state, const uint32_t frame,\n>> +\t\t\t gamma::FrameContext &context, const ControlList &controls);\n>> +\tvoid process(gamma::FrameContext &context, ControlList &metadata);\n>> +\n>> +protected:\n>> +\tunsigned int nLutNodes_;\n>> +\tfloat defaultGamma_;\n>> +\tstd::vector segments_;\n>> +\tunsigned int segmentsSum_;\n>> +};\n>> +\n>> +template\n>> +class GammaAlgorithm : public GammaAlgorithmBase\n>> +{\n>> +public:\n>> +\tGammaAlgorithm()\n>> +\t\t: GammaAlgorithmBase(nLutNodes)\n>> +\t{\n>> +\t}\n>> +\n>> +\ttemplate\n>> +\tvoid prepare(gamma::FrameContext &context, Span lut)\n>> +\t{\n>> +\t\tfloat x = 0;\n>> +\n>> +\t\tfor (unsigned int i = 0; i < nLutNodes_; i++) {\n>> +\t\t\tfloat gamma = std::pow(x / segmentsSum_,\n>> +\t\t\t\t\t 1.0 / context.gamma);\n>> +\t\t\tlut[i] = UQ(gamma).quantized();\n>> +\n>> +\t\t\tLOG(Gamma, Debug) << \"LUT[\" << i << \"]=\" << gamma << \"(\" << lut[i] << \")\";\n> \n> I bet you can easily break this to multiple lines.\n> \n> With documentation adjusted:\n> Reviewed-by: Jacopo Mondi \n> \n> Thanks\n> j\n> \n>> +\n>> +\t\t\tif (i < segments_.size())\n>> +\t\t\t\tx += segments_[i];\n>> +\t\t}\n>> +\t}\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 da6ea0c5e13000f78b2196c7334610c350f1ad13..565da9be9059f2167d107061d643e58202e655ef 100644\n>> --- a/src/ipa/libipa/meson.build\n>> +++ b/src/ipa/libipa/meson.build\n>> @@ -12,6 +12,7 @@ libipa_headers = files([\n>> 'exposure_mode_helper.h',\n>> 'fc_queue.h',\n>> 'fixedpoint.h',\n>> + 'gamma.h',\n>> 'histogram.h',\n>> 'interpolator.h',\n>> 'lsc.h',\n>> @@ -37,6 +38,7 @@ libipa_sources = files([\n>> 'exposure_mode_helper.cpp',\n>> 'fc_queue.cpp',\n>> 'fixedpoint.cpp',\n>> + 'gamma.cpp',\n>> 'histogram.cpp',\n>> 'interpolator.cpp',\n>> 'lsc.cpp',\n>>\n>> --\n>> 2.43.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 51EFEC3261\n\tfor ;\n\tFri, 19 Jun 2026 08:41:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8F14A656C4;\n\tFri, 19 Jun 2026 10:41:54 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2861B61F3F\n\tfor ;\n\tFri, 19 Jun 2026 10:41:53 +0200 (CEST)","from [192.168.0.43]\n\t(chfd-03-b2-v4wan-176392-cust229.vm15.cable.virginm.net\n\t[82.19.20.230])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4BC351494;\n\tFri, 19 Jun 2026 10:41:17 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"fknQlpzN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1781858477;\n\tbh=NNOnLVWQk0Z19aOEjk//s6CT9m18AylzK85ATB9/CVo=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=fknQlpzNKhYhlL3E45dTK4OLcY1OV72PxNStfoHvMYwBPhJTKZtny7//5+RM4fQ7c\n\tiH/M9YbmGdFt+jm5OtuW3DVmctDUC0KN98fIOC8xUzVzYVO58CUxqAqyrFmEg/R0dl\n\tbxALzZklk2ChqG5kimZPBoJpuJJd4XxAOKGyu1Ys=","Message-ID":"<46302008-7a5a-4ed8-8046-5fcc695d0d72@ideasonboard.com>","Date":"Fri, 19 Jun 2026 09:41:49 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 05/10] ipa: libipa: Add GammaAlgorithm class","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20260616-ipu3-libipa-rework-v1-0-d4448b54f1d8@ideasonboard.com>\n\t<20260616-ipu3-libipa-rework-v1-5-d4448b54f1d8@ideasonboard.com>\n\t","Content-Language":"en-US","From":"Dan Scally ","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","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\" "}}]