[{"id":30784,"web_url":"https://patchwork.libcamera.org/comment/30784/","msgid":"","date":"2024-08-13T10:54:03","subject":"Re: [PATCH v3 19/23] libcamera: software_isp: Move color handling to\n\tan algorithm module","submitter":{"id":156,"url":"https://patchwork.libcamera.org/api/people/156/","name":"Dan Scally","email":"dan.scally@ideasonboard.com"},"content":"Hi Milan\n\nOn 17/07/2024 09:54, Milan Zamazal wrote:\n> After black level handling has been moved to an algorithm module, white\n> balance and the construction of color tables can be moved to an\n> algorithm module too. This time, the moved code is split between stats\n> processing and parameter construction methods.\n>\n> This is the only part of the software ISP algorithms that sets the\n> parameters so emitting setIspParams can be moved to prepare() method.\n>\n> A more natural representation of the gains (and black level) would be\n> floating point numbers. This is not done here in order to minimize\n> changes in code movements. It will be addressed in a followup patch.\n>\n> Signed-off-by: Milan Zamazal \n> ---\n> src/ipa/simple/algorithms/colors.cpp | 121 ++++++++++++++++++++++++++\n> src/ipa/simple/algorithms/colors.h | 46 ++++++++++\n> src/ipa/simple/algorithms/meson.build | 1 +\n> src/ipa/simple/data/uncalibrated.yaml | 1 +\n> src/ipa/simple/ipa_context.cpp | 30 +++++++\n> src/ipa/simple/ipa_context.h | 12 +++\n> src/ipa/simple/soft_simple.cpp | 66 +-------------\n> 7 files changed, 215 insertions(+), 62 deletions(-)\n> create mode 100644 src/ipa/simple/algorithms/colors.cpp\n> create mode 100644 src/ipa/simple/algorithms/colors.h\n>\n> diff --git a/src/ipa/simple/algorithms/colors.cpp b/src/ipa/simple/algorithms/colors.cpp\n> new file mode 100644\n> index 00000000..60fdca15\n> --- /dev/null\n> +++ b/src/ipa/simple/algorithms/colors.cpp\n> @@ -0,0 +1,121 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Red Hat Inc.\n> + *\n> + * Color gains (AWB + gamma)\n> + */\n> +\n> +#include \"colors.h\"\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"simple/ipa_context.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(IPASoftColors)\n> +\n> +namespace ipa::soft::algorithms {\n> +\n> +static constexpr unsigned int kGammaLookupSize = 1024;\n> +\n> +Colors::Colors()\n> +{\n> +}\n> +\n> +int Colors::init(IPAContext &context,\n> +\t\t [[maybe_unused]] const YamlObject &tuningData)\n> +{\n> +\t/* Gamma value is fixed */\n> +\tcontext.configuration.gamma = 0.5;\n> +\tupdateGammaTable(context);\n> +\n> +\tauto &gains = context.activeState.gains;\n> +\tgains.red = gains.green = gains.blue = 256;\n> +\n> +\treturn 0;\n> +}\n\n\nI think my preference would be to split this into \"Gamma\" and \"Awb\" algorithms...it adds boilerplate \nof course, but having the separation makes them much easier to understand in my opinion.\n\n> +\n> +void Colors::updateGammaTable(IPAContext &context)\n> +{\n> +\tauto &gammaTable = context.activeState.gamma.gammaTable;\n> +\tauto &blackLevel = context.activeState.gamma.blackLevel;\n> +\tblackLevel = context.activeState.black.level;\ndouble assignment\n> +\tconst unsigned int blackIndex =\n> +\t\tblackLevel * IPAActiveState::kGammaLookupSize / 256;\n> +\tstd::fill(gammaTable.begin(), gammaTable.begin() + blackIndex, 0);\n> +\tconst float divisor = kGammaLookupSize - blackIndex - 1.0;\n> +\tfor (unsigned int i = blackIndex; i < kGammaLookupSize; i++)\n> +\t\tgammaTable[i] = UINT8_MAX * powf((i - blackIndex) / divisor,\n> +\t\t\t\t\t\t context.configuration.gamma);\n> +}\n> +\n> +void Colors::prepare(IPAContext &context,\n> +\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n> +\t\t DebayerParams *params)\n> +{\n> +\t/* Update the gamma table if needed */\n> +\tif (context.activeState.gamma.blackLevel != context.activeState.black.level)\nIs it worth adding some tolerance with abs_diff()? In my experience values are almost never exactly \nthe same frame-to-frame.\n> +\t\tupdateGammaTable(context);\n> +\n> +\tauto &gains = context.activeState.gains;\n> +\tauto &gammaTable = context.activeState.gamma.gammaTable;\n> +\tfor (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {\n> +\t\tconstexpr unsigned int div =\n> +\t\t\tstatic_cast(DebayerParams::kRGBLookupSize) * 256 / kGammaLookupSize;\n> +\t\t/* Apply gamma after gain! */\n> +\t\tunsigned int idx;\n> +\t\tidx = std::min({ i * gains.red / div, kGammaLookupSize - 1 });\n> +\t\tparams->red[i] = gammaTable[idx];\n> +\t\tidx = std::min({ i * gains.green / div, kGammaLookupSize - 1 });\n> +\t\tparams->green[i] = gammaTable[idx];\n> +\t\tidx = std::min({ i * gains.blue / div, kGammaLookupSize - 1 });\n> +\t\tparams->blue[i] = gammaTable[idx];\n> +\t}\n> +}\n> +\n> +void Colors::process(IPAContext &context,\n> +\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n> +\t\t const SwIspStats *stats,\n> +\t\t [[maybe_unused]] ControlList &metadata)\n> +{\n> +\tconst SwIspStats::Histogram &histogram = stats->yHistogram;\n> +\tconst uint8_t blackLevel = context.activeState.black.level;\n> +\n> +\t/*\n> +\t * Black level must be subtracted to get the correct AWB ratios, they\n> +\t * would be off if they were computed from the whole brightness range\n> +\t * rather than from the sensor range.\n> +\t */\n> +\tconst uint64_t nPixels = std::accumulate(\n> +\t\thistogram.begin(), histogram.end(), 0);\n> +\tconst uint64_t offset = blackLevel * nPixels;\n> +\tconst uint64_t sumR = stats->sumR_ - offset / 4;\n> +\tconst uint64_t sumG = stats->sumG_ - offset / 2;\n> +\tconst uint64_t sumB = stats->sumB_ - offset / 4;\n> +\n> +\t/*\n> +\t * Calculate red and blue gains for AWB.\n> +\t * Clamp max gain at 4.0, this also avoids 0 division.\n> +\t * Gain: 128 = 0.5, 256 = 1.0, 512 = 2.0, etc.\n> +\t */\n> +\tauto &gains = context.activeState.gains;\n> +\tgains.red = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR;\n> +\tgains.blue = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB;\n> +\t/* Green gain is fixed to 256 */\n> +\n> +\tLOG(IPASoftColors, Debug) << \"gain R/B \" << gains.red << \"/\" << gains.blue;\n> +}\n> +\n> +REGISTER_IPA_ALGORITHM(Colors, \"Colors\")\n> +\n> +} /* namespace ipa::soft::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/simple/algorithms/colors.h b/src/ipa/simple/algorithms/colors.h\n> new file mode 100644\n> index 00000000..2c2f7752\n> --- /dev/null\n> +++ b/src/ipa/simple/algorithms/colors.h\n> @@ -0,0 +1,46 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Red Hat Inc.\n> + *\n> + * Color gains (AWB + gamma)\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +\n> +#include \"libcamera/internal/software_isp/debayer_params.h\"\n> +#include \"libcamera/internal/software_isp/swisp_stats.h\"\n> +\n> +#include \"algorithm.h\"\n> +#include \"ipa_context.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::soft::algorithms {\n> +\n> +class Colors : public Algorithm\n> +{\n> +public:\n> +\tColors();\n> +\t~Colors() = default;\n> +\n> +\tint init(IPAContext &context, const YamlObject &tuningData)\n> +\t\toverride;\n> +\tvoid prepare(IPAContext &context,\n> +\t\t const uint32_t frame,\n> +\t\t IPAFrameContext &frameContext,\n> +\t\t DebayerParams *params) override;\n> +\tvoid process(IPAContext &context,\n> +\t\t const uint32_t frame,\n> +\t\t IPAFrameContext &frameContext,\n> +\t\t const SwIspStats *stats,\n> +\t\t ControlList &metadata) override;\n> +\n> +private:\n> +\tvoid updateGammaTable(IPAContext &context);\n> +};\n> +\n> +} /* namespace ipa::soft::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build\n> index 1f63c220..324c7b4e 100644\n> --- a/src/ipa/simple/algorithms/meson.build\n> +++ b/src/ipa/simple/algorithms/meson.build\n> @@ -2,4 +2,5 @@\n> \n> soft_simple_ipa_algorithms = files([\n> 'blc.cpp',\n> + 'colors.cpp',\n> ])\n> diff --git a/src/ipa/simple/data/uncalibrated.yaml b/src/ipa/simple/data/uncalibrated.yaml\n> index e0d03d96..7e5149e5 100644\n> --- a/src/ipa/simple/data/uncalibrated.yaml\n> +++ b/src/ipa/simple/data/uncalibrated.yaml\n> @@ -4,4 +4,5 @@\n> version: 1\n> algorithms:\n> - BlackLevel:\n> + - Colors:\n> ...\n> diff --git a/src/ipa/simple/ipa_context.cpp b/src/ipa/simple/ipa_context.cpp\n> index 268cff32..5fa492cb 100644\n> --- a/src/ipa/simple/ipa_context.cpp\n> +++ b/src/ipa/simple/ipa_context.cpp\n> @@ -50,6 +50,11 @@ namespace libcamera::ipa::soft {\n> * \\brief The current state of IPA algorithms\n> */\n> \n> +/**\n> + * \\var IPASessionConfiguration::gamma\n> + * \\brief Gamma value to be used in the raw image processing\n> + */\n> +\n> /**\n> * \\var IPAActiveState::black\n> * \\brief Context for the Black Level algorithm\n> @@ -58,4 +63,29 @@ namespace libcamera::ipa::soft {\n> * \\brief Current determined black level\n> */\n> \n> +/**\n> + * \\var IPAActiveState::gains\n> + * \\brief Context for gains in the Colors algorithm\n> + *\n> + * \\var IPAActiveState::gains.red\n> + * \\brief Gain of red color\n> + *\n> + * \\var IPAActiveState::gains.green\n> + * \\brief Gain of green color\n> + *\n> + * \\var IPAActiveState::gains.blue\n> + * \\brief Gain of blue color\n> + */\n> +\n> +/**\n> + * \\var IPAActiveState::gamma\n> + * \\brief Context for gamma in the Colors algorithm\n> + *\n> + * \\var IPAActiveState::gamma.gammaTable\n> + * \\brief Computed gamma table\n> + *\n> + * \\var IPAActiveState::gamma.blackLevel\n> + * \\brief Black level used for the gamma table computation\n> + */\n> +\n> } /* namespace libcamera::ipa::soft */\n> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> index ed07dbb8..979ddb8f 100644\n> --- a/src/ipa/simple/ipa_context.h\n> +++ b/src/ipa/simple/ipa_context.h\n> @@ -8,6 +8,7 @@\n> \n> #pragma once\n> \n> +#include \n> #include \n> \n> #include \n> @@ -17,12 +18,23 @@ namespace libcamera {\n> namespace ipa::soft {\n> \n> struct IPASessionConfiguration {\n> +\tfloat gamma;\n> };\n> \n> struct IPAActiveState {\n> \tstruct {\n> \t\tuint8_t level;\n> \t} black;\n> +\tstruct {\n> +\t\tunsigned int red;\n> +\t\tunsigned int green;\n> +\t\tunsigned int blue;\n> +\t} gains;\n> +\tstatic constexpr unsigned int kGammaLookupSize = 1024;\n> +\tstruct {\n> +\t\tstd::array gammaTable;\n> +\t\tuint8_t blackLevel;\n> +\t} gamma;\n> };\n> \n> struct IPAFrameContext : public FrameContext {\n> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n> index 2fb3a473..2658e359 100644\n> --- a/src/ipa/simple/soft_simple.cpp\n> +++ b/src/ipa/simple/soft_simple.cpp\n> @@ -5,8 +5,6 @@\n> * Simple Software Image Processing Algorithm module\n> */\n> \n> -#include \n> -#include \n> #include \n> #include \n> \n> @@ -92,9 +90,6 @@ private:\n> \tstd::unique_ptr camHelper_;\n> \tControlInfoMap sensorInfoMap_;\n> \n> -\tstatic constexpr unsigned int kGammaLookupSize = 1024;\n> -\tstd::array gammaTable_;\n> -\tint lastBlackLevel_ = -1;\n> \t/* Local parameter storage */\n> \tstruct IPAContext context_;\n> \n> @@ -282,6 +277,7 @@ void IPASoftSimple::prepare(const uint32_t frame)\n> \tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> \tfor (auto const &algo : algorithms())\n> \t\talgo->prepare(context_, frame, frameContext, params_);\n> +\tsetIspParams.emit();\n> }\n> \n> void IPASoftSimple::processStats(\n> @@ -300,62 +296,6 @@ void IPASoftSimple::processStats(\n> \tfor (auto const &algo : algorithms())\n> \t\talgo->process(context_, frame, frameContext, stats_, metadata);\n> \n> -\tSwIspStats::Histogram histogram = stats_->yHistogram;\n> -\tconst uint8_t blackLevel = context_.activeState.black.level;\n> -\n> -\t/*\n> -\t * Black level must be subtracted to get the correct AWB ratios, they\n> -\t * would be off if they were computed from the whole brightness range\n> -\t * rather than from the sensor range.\n> -\t */\n> -\tconst uint64_t nPixels = std::accumulate(\n> -\t\thistogram.begin(), histogram.end(), 0);\n> -\tconst uint64_t offset = blackLevel * nPixels;\n> -\tconst uint64_t sumR = stats_->sumR_ - offset / 4;\n> -\tconst uint64_t sumG = stats_->sumG_ - offset / 2;\n> -\tconst uint64_t sumB = stats_->sumB_ - offset / 4;\n> -\n> -\t/*\n> -\t * Calculate red and blue gains for AWB.\n> -\t * Clamp max gain at 4.0, this also avoids 0 division.\n> -\t * Gain: 128 = 0.5, 256 = 1.0, 512 = 2.0, etc.\n> -\t */\n> -\tconst unsigned int gainR = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR;\n> -\tconst unsigned int gainB = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB;\n> -\t/* Green gain and gamma values are fixed */\n> -\tconstexpr unsigned int gainG = 256;\n> -\n> -\t/* Update the gamma table if needed */\n> -\tif (blackLevel != lastBlackLevel_) {\n> -\t\tconstexpr float gamma = 0.5;\n> -\t\tconst unsigned int blackIndex = blackLevel * kGammaLookupSize / 256;\n> -\t\tstd::fill(gammaTable_.begin(), gammaTable_.begin() + blackIndex, 0);\n> -\t\tconst float divisor = kGammaLookupSize - blackIndex - 1.0;\n> -\t\tfor (unsigned int i = blackIndex; i < kGammaLookupSize; i++)\n> -\t\t\tgammaTable_[i] = UINT8_MAX *\n> -\t\t\t\t\t std::pow((i - blackIndex) / divisor, gamma);\n> -\n> -\t\tlastBlackLevel_ = blackLevel;\n> -\t}\n> -\n> -\tfor (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {\n> -\t\tconstexpr unsigned int div =\n> -\t\t\tDebayerParams::kRGBLookupSize * 256 / kGammaLookupSize;\n> -\t\tunsigned int idx;\n> -\n> -\t\t/* Apply gamma after gain! */\n> -\t\tidx = std::min({ i * gainR / div, (kGammaLookupSize - 1) });\n> -\t\tparams_->red[i] = gammaTable_[idx];\n> -\n> -\t\tidx = std::min({ i * gainG / div, (kGammaLookupSize - 1) });\n> -\t\tparams_->green[i] = gammaTable_[idx];\n> -\n> -\t\tidx = std::min({ i * gainB / div, (kGammaLookupSize - 1) });\n> -\t\tparams_->blue[i] = gammaTable_[idx];\n> -\t}\n> -\n> -\tsetIspParams.emit();\n> -\n> \t/* \\todo Switch to the libipa/algorithm.h API someday. */\n> \n> \t/*\n> @@ -372,6 +312,7 @@ void IPASoftSimple::processStats(\n> \t * Calculate Mean Sample Value (MSV) according to formula from:\n> \t * https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf\n> \t */\n> +\tconst uint8_t blackLevel = context_.activeState.black.level;\n> \tconst unsigned int blackLevelHistIdx =\n> \t\tblackLevel / (256 / SwIspStats::kYHistogramSize);\n> \tconst unsigned int histogramSize =\n> @@ -421,7 +362,8 @@ void IPASoftSimple::processStats(\n> \n> \tLOG(IPASoft, Debug) << \"exposureMSV \" << exposureMSV\n> \t\t\t << \" exp \" << exposure_ << \" again \" << again_\n> -\t\t\t << \" gain R/B \" << gainR << \"/\" << gainB\n> +\t\t\t << \" gain R/B \" << context_.activeState.gains.red\n> +\t\t\t << \"/\" << context_.activeState.gains.blue\n> \t\t\t << \" black level \" << static_cast(blackLevel);\n> }\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 46F96C323E\n\tfor ;\n\tTue, 13 Aug 2024 10:54:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6E405633B5;\n\tTue, 13 Aug 2024 12:54:08 +0200 (CEST)","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 960D063382\n\tfor ;\n\tTue, 13 Aug 2024 12:54:06 +0200 (CEST)","from [192.168.0.43]\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8740F2EC\n\tfor ;\n\tTue, 13 Aug 2024 12:53:09 +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=\"hKzIhHWP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1723546389;\n\tbh=qtvtsdTR/i55I+Uij31lm+8ZMC3eZImSoWvZVwdpmWU=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=hKzIhHWPD9sIwEAXZSV1DqrSUxpwFyX3JksDqNW2EfElvZ55eZMIQRe9NiymwgHHU\n\t8zxZ6hT/s81G8CSGqwhWb/bN1PUyZw6DqbQ0xjNQ1xUv1oukgudz+uxFFWLnunfkOo\n\tI7skufDlmVkHN1X46WuOVrFpLdw7ahVzw8GRJXGg=","Message-ID":"","Date":"Tue, 13 Aug 2024 11:54:03 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v3 19/23] libcamera: software_isp: Move color handling to\n\tan algorithm module","To":"libcamera-devel@lists.libcamera.org","References":"<20240717085444.289997-1-mzamazal@redhat.com>\n\t<20240717085444.289997-20-mzamazal@redhat.com>","Content-Language":"en-US","From":"Dan Scally ","Autocrypt":"addr=dan.scally@ideasonboard.com; keydata=\n\txsFNBGLydlEBEADa5O2s0AbUguprfvXOQun/0a8y2Vk6BqkQALgeD6KnXSWwaoCULp18etYW\n\tB31bfgrdphXQ5kUQibB0ADK8DERB4wrzrUb5CMxLBFE7mQty+v5NsP0OFNK9XTaAOcmD+Ove\n\teIjYvqurAaro91jrRVrS1gBRxIFqyPgNvwwL+alMZhn3/2jU2uvBmuRrgnc/e9cHKiuT3Dtq\n\tMHGPKL2m+plk+7tjMoQFfexoQ1JKugHAjxAhJfrkXh6uS6rc01bYCyo7ybzg53m1HLFJdNGX\n\tsUKR+dQpBs3SY4s66tc1sREJqdYyTsSZf80HjIeJjU/hRunRo4NjRIJwhvnK1GyjOvvuCKVU\n\tRWpY8dNjNu5OeAfdrlvFJOxIE9M8JuYCQTMULqd1NuzbpFMjc9524U3Cngs589T7qUMPb1H1\n\tNTA81LmtJ6Y+IV5/kiTUANflpzBwhu18Ok7kGyCq2a2jsOcVmk8gZNs04gyjuj8JziYwwLbf\n\tvzABwpFVcS8aR+nHIZV1HtOzyw8CsL8OySc3K9y+Y0NRpziMRvutrppzgyMb9V+N31mK9Mxl\n\t1YkgaTl4ciNWpdfUe0yxH03OCuHi3922qhPLF4XX5LN+NaVw5Xz2o3eeWklXdouxwV7QlN33\n\tu4+u2FWzKxDqO6WLQGjxPE0mVB4Gh5Pa1Vb0ct9Ctg0qElvtGQARAQABzShEYW4gU2NhbGx5\n\tIDxkYW4uc2NhbGx5QGlkZWFzb25ib2FyZC5jb20+wsGNBBMBCAA3FiEEsdtt8OWP7+8SNfQe\n\tkiQuh/L+GMQFAmLydlIFCQWjmoACGwMECwkIBwUVCAkKCwUWAgMBAAAKCRCSJC6H8v4YxDI2\n\tEAC2Gz0iyaXJkPInyshrREEWbo0CA6v5KKf3I/HlMPqkZ48bmGoYm4mEQGFWZJAT3K4ir8bg\n\tcEfs9V54gpbrZvdwS4abXbUK4WjKwEs8HK3XJv1WXUN2bsz5oEJWZUImh9gD3naiLLI9QMMm\n\tw/aZkT+NbN5/2KvChRWhdcha7+2Te4foOY66nIM+pw2FZM6zIkInLLUik2zXOhaZtqdeJZQi\n\tHSPU9xu7TRYN4cvdZAnSpG7gQqmLm5/uGZN1/sB3kHTustQtSXKMaIcD/DMNI3JN/t+RJVS7\n\tc0Jh/ThzTmhHyhxx3DRnDIy7kwMI4CFvmhkVC2uNs9kWsj1DuX5kt8513mvfw2OcX9UnNKmZ\n\tnhNCuF6DxVrL8wjOPuIpiEj3V+K7DFF1Cxw1/yrLs8dYdYh8T8vCY2CHBMsqpESROnTazboh\n\tAiQ2xMN1cyXtX11Qwqm5U3sykpLbx2BcmUUUEAKNsM//Zn81QXKG8vOx0ZdMfnzsCaCzt8f6\n\t9dcDBBI3tJ0BI9ByiocqUoL6759LM8qm18x3FYlxvuOs4wSGPfRVaA4yh0pgI+ModVC2Pu3y\n\tejE/IxeatGqJHh6Y+iJzskdi27uFkRixl7YJZvPJAbEn7kzSi98u/5ReEA8Qhc8KO/B7wprj\n\txjNMZNYd0Eth8+WkixHYj752NT5qshKJXcyUU87BTQRi8nZSARAAx0BJayh1Fhwbf4zoY56x\n\txHEpT6DwdTAYAetd3yiKClLVJadYxOpuqyWa1bdfQWPb+h4MeXbWw/53PBgn7gI2EA7ebIRC\n\tPJJhAIkeym7hHZoxqDQTGDJjxFEL11qF+U3rhWiL2Zt0Pl+zFq0eWYYVNiXjsIS4FI2+4m16\n\ttPbDWZFJnSZ828VGtRDQdhXfx3zyVX21lVx1bX4/OZvIET7sVUufkE4hrbqrrufre7wsjD1t\n\t8MQKSapVrr1RltpzPpScdoxknOSBRwOvpp57pJJe5A0L7+WxJ+vQoQXj0j+5tmIWOAV1qBQp\n\thyoyUk9JpPfntk2EKnZHWaApFp5TcL6c5LhUvV7F6XwOjGPuGlZQCWXee9dr7zym8iR3irWT\n\t+49bIh5PMlqSLXJDYbuyFQHFxoiNdVvvf7etvGfqFYVMPVjipqfEQ38ST2nkzx+KBICz7uwj\n\tJwLBdTXzGFKHQNckGMl7F5QdO/35An/QcxBnHVMXqaSd12tkJmoRVWduwuuoFfkTY5mUV3uX\n\txGj3iVCK4V+ezOYA7c2YolfRCNMTza6vcK/P4tDjjsyBBZrCCzhBvd4VVsnnlZhVaIxoky4K\n\taL+AP+zcQrUZmXmgZjXOLryGnsaeoVrIFyrU6ly90s1y3KLoPsDaTBMtnOdwxPmo1xisH8oL\n\ta/VRgpFBfojLPxMAEQEAAcLBfAQYAQgAJhYhBLHbbfDlj+/vEjX0HpIkLofy/hjEBQJi8nZT\n\tBQkFo5qAAhsMAAoJEJIkLofy/hjEXPcQAMIPNqiWiz/HKu9W4QIf1OMUpKn3YkVIj3p3gvfM\n\tRes4fGX94Ji599uLNrPoxKyaytC4R6BTxVriTJjWK8mbo9jZIRM4vkwkZZ2bu98EweSucxbp\n\tvjESsvMXGgxniqV/RQ/3T7LABYRoIUutARYq58p5HwSP0frF0fdFHYdTa2g7MYZl1ur2JzOC\n\tFHRpGadlNzKDE3fEdoMobxHB3Lm6FDml5GyBAA8+dQYVI0oDwJ3gpZPZ0J5Vx9RbqXe8RDuR\n\tdu90hvCJkq7/tzSQ0GeD3BwXb9/R/A4dVXhaDd91Q1qQXidI+2jwhx8iqiYxbT+DoAUkQRQy\n\txBtoCM1CxH7u45URUgD//fxYr3D4B1SlonA6vdaEdHZOGwECnDpTxecENMbz/Bx7qfrmd901\n\tD+N9SjIwrbVhhSyUXYnSUb8F+9g2RDY42Sk7GcYxIeON4VzKqWM7hpkXZ47pkK0YodO+dRKM\n\tyMcoUWrTK0Uz6UzUGKoJVbxmSW/EJLEGoI5p3NWxWtScEVv8mO49gqQdrRIOheZycDmHnItt\n\t9Qjv00uFhEwv2YfiyGk6iGF2W40s2pH2t6oeuGgmiZ7g6d0MEK8Ql/4zPItvr1c1rpwpXUC1\n\tu1kQWgtnNjFHX3KiYdqjcZeRBiry1X0zY+4Y24wUU0KsEewJwjhmCKAsju1RpdlPg2kC","In-Reply-To":"<20240717085444.289997-20-mzamazal@redhat.com>","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\" "}},{"id":30790,"web_url":"https://patchwork.libcamera.org/comment/30790/","msgid":"<87y150wm1y.fsf@redhat.com>","date":"2024-08-13T14:10:33","subject":"Re: [PATCH v3 19/23] libcamera: software_isp: Move color handling\n\tto an algorithm module","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Dan,\n\nthank you for review.\n\nDan Scally writes:\n\n> Hi Milan\n>\n> On 17/07/2024 09:54, Milan Zamazal wrote:\n>> After black level handling has been moved to an algorithm module, white\n>> balance and the construction of color tables can be moved to an\n>> algorithm module too. This time, the moved code is split between stats\n>> processing and parameter construction methods.\n>>\n>> This is the only part of the software ISP algorithms that sets the\n>> parameters so emitting setIspParams can be moved to prepare() method.\n>>\n>> A more natural representation of the gains (and black level) would be\n>> floating point numbers. This is not done here in order to minimize\n>> changes in code movements. It will be addressed in a followup patch.\n>>\n>> Signed-off-by: Milan Zamazal \n>> ---\n>> src/ipa/simple/algorithms/colors.cpp | 121 ++++++++++++++++++++++++++\n>> src/ipa/simple/algorithms/colors.h | 46 ++++++++++\n>> src/ipa/simple/algorithms/meson.build | 1 +\n>> src/ipa/simple/data/uncalibrated.yaml | 1 +\n>> src/ipa/simple/ipa_context.cpp | 30 +++++++\n>> src/ipa/simple/ipa_context.h | 12 +++\n>> src/ipa/simple/soft_simple.cpp | 66 +-------------\n>> 7 files changed, 215 insertions(+), 62 deletions(-)\n>> create mode 100644 src/ipa/simple/algorithms/colors.cpp\n>> create mode 100644 src/ipa/simple/algorithms/colors.h\n>>\n>> diff --git a/src/ipa/simple/algorithms/colors.cpp b/src/ipa/simple/algorithms/colors.cpp\n>> new file mode 100644\n>> index 00000000..60fdca15\n>> --- /dev/null\n>> +++ b/src/ipa/simple/algorithms/colors.cpp\n>> @@ -0,0 +1,121 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2024, Red Hat Inc.\n>> + *\n>> + * Color gains (AWB + gamma)\n>> + */\n>> +\n>> +#include \"colors.h\"\n>> +\n>> +#include \n>> +#include \n>> +#include \n>> +#include \n>> +\n>> +#include \n>> +\n>> +#include \"simple/ipa_context.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +LOG_DEFINE_CATEGORY(IPASoftColors)\n>> +\n>> +namespace ipa::soft::algorithms {\n>> +\n>> +static constexpr unsigned int kGammaLookupSize = 1024;\n>> +\n>> +Colors::Colors()\n>> +{\n>> +}\n>> +\n>> +int Colors::init(IPAContext &context,\n>> +\t\t [[maybe_unused]] const YamlObject &tuningData)\n>> +{\n>> +\t/* Gamma value is fixed */\n>> +\tcontext.configuration.gamma = 0.5;\n>> +\tupdateGammaTable(context);\n>> +\n>> +\tauto &gains = context.activeState.gains;\n>> +\tgains.red = gains.green = gains.blue = 256;\n>> +\n>> +\treturn 0;\n>> +}\n>\n>\n> I think my preference would be to split this into \"Gamma\" and \"Awb\" algorithms...it adds boilerplate of course, but having the separation\n> makes them much easier to understand in my opinion.\n\nMakes sense, will do.\n\n>> +\n>> +void Colors::updateGammaTable(IPAContext &context)\n>> +{\n>> +\tauto &gammaTable = context.activeState.gamma.gammaTable;\n>> +\tauto &blackLevel = context.activeState.gamma.blackLevel;\n>> +\tblackLevel = context.activeState.black.level;\n> double assignment\n\nNot really because the first variable is a reference. But it is\nconfusing, I'll change it.\n\n>> +\tconst unsigned int blackIndex =\n>> +\t\tblackLevel * IPAActiveState::kGammaLookupSize / 256;\n>> +\tstd::fill(gammaTable.begin(), gammaTable.begin() + blackIndex, 0);\n>> +\tconst float divisor = kGammaLookupSize - blackIndex - 1.0;\n>> +\tfor (unsigned int i = blackIndex; i < kGammaLookupSize; i++)\n>> +\t\tgammaTable[i] = UINT8_MAX * powf((i - blackIndex) / divisor,\n>> +\t\t\t\t\t\t context.configuration.gamma);\n>> +}\n>> +\n>> +void Colors::prepare(IPAContext &context,\n>> +\t\t [[maybe_unused]] const uint32_t frame,\n>> +\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n>> +\t\t DebayerParams *params)\n>> +{\n>> +\t/* Update the gamma table if needed */\n>> +\tif (context.activeState.gamma.blackLevel != context.activeState.black.level)\n> Is it worth adding some tolerance with abs_diff()? In my experience\n> values are almost never exactly the same frame-to-frame.\n\nThe current black level computation updates the value only if it is\nsmaller than the previous one so it should stabilize sooner or later.\nMaybe worth to add to the comment here.\n\n>> +\t\tupdateGammaTable(context);\n>> +\n>> +\tauto &gains = context.activeState.gains;\n>> +\tauto &gammaTable = context.activeState.gamma.gammaTable;\n>> +\tfor (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {\n>> +\t\tconstexpr unsigned int div =\n>> +\t\t\tstatic_cast(DebayerParams::kRGBLookupSize) * 256 / kGammaLookupSize;\n>> +\t\t/* Apply gamma after gain! */\n>> +\t\tunsigned int idx;\n>> +\t\tidx = std::min({ i * gains.red / div, kGammaLookupSize - 1 });\n>> +\t\tparams->red[i] = gammaTable[idx];\n>> +\t\tidx = std::min({ i * gains.green / div, kGammaLookupSize - 1 });\n>> +\t\tparams->green[i] = gammaTable[idx];\n>> +\t\tidx = std::min({ i * gains.blue / div, kGammaLookupSize - 1 });\n>> +\t\tparams->blue[i] = gammaTable[idx];\n>> +\t}\n>> +}\n>> +\n>> +void Colors::process(IPAContext &context,\n>> +\t\t [[maybe_unused]] const uint32_t frame,\n>> +\t\t [[maybe_unused]] IPAFrameContext &frameContext,\n>> +\t\t const SwIspStats *stats,\n>> +\t\t [[maybe_unused]] ControlList &metadata)\n>> +{\n>> +\tconst SwIspStats::Histogram &histogram = stats->yHistogram;\n>> +\tconst uint8_t blackLevel = context.activeState.black.level;\n>> +\n>> +\t/*\n>> +\t * Black level must be subtracted to get the correct AWB ratios, they\n>> +\t * would be off if they were computed from the whole brightness range\n>> +\t * rather than from the sensor range.\n>> +\t */\n>> +\tconst uint64_t nPixels = std::accumulate(\n>> +\t\thistogram.begin(), histogram.end(), 0);\n>> +\tconst uint64_t offset = blackLevel * nPixels;\n>> +\tconst uint64_t sumR = stats->sumR_ - offset / 4;\n>> +\tconst uint64_t sumG = stats->sumG_ - offset / 2;\n>> +\tconst uint64_t sumB = stats->sumB_ - offset / 4;\n>> +\n>> +\t/*\n>> +\t * Calculate red and blue gains for AWB.\n>> +\t * Clamp max gain at 4.0, this also avoids 0 division.\n>> +\t * Gain: 128 = 0.5, 256 = 1.0, 512 = 2.0, etc.\n>> +\t */\n>> +\tauto &gains = context.activeState.gains;\n>> +\tgains.red = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR;\n>> +\tgains.blue = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB;\n>> +\t/* Green gain is fixed to 256 */\n>> +\n>> +\tLOG(IPASoftColors, Debug) << \"gain R/B \" << gains.red << \"/\" << gains.blue;\n>> +}\n>> +\n>> +REGISTER_IPA_ALGORITHM(Colors, \"Colors\")\n>> +\n>> +} /* namespace ipa::soft::algorithms */\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/ipa/simple/algorithms/colors.h b/src/ipa/simple/algorithms/colors.h\n>> new file mode 100644\n>> index 00000000..2c2f7752\n>> --- /dev/null\n>> +++ b/src/ipa/simple/algorithms/colors.h\n>> @@ -0,0 +1,46 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2024, Red Hat Inc.\n>> + *\n>> + * Color gains (AWB + gamma)\n>> + */\n>> +\n>> +#pragma once\n>> +\n>> +#include \n>> +\n>> +#include \"libcamera/internal/software_isp/debayer_params.h\"\n>> +#include \"libcamera/internal/software_isp/swisp_stats.h\"\n>> +\n>> +#include \"algorithm.h\"\n>> +#include \"ipa_context.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +namespace ipa::soft::algorithms {\n>> +\n>> +class Colors : public Algorithm\n>> +{\n>> +public:\n>> +\tColors();\n>> +\t~Colors() = default;\n>> +\n>> +\tint init(IPAContext &context, const YamlObject &tuningData)\n>> +\t\toverride;\n>> +\tvoid prepare(IPAContext &context,\n>> +\t\t const uint32_t frame,\n>> +\t\t IPAFrameContext &frameContext,\n>> +\t\t DebayerParams *params) override;\n>> +\tvoid process(IPAContext &context,\n>> +\t\t const uint32_t frame,\n>> +\t\t IPAFrameContext &frameContext,\n>> +\t\t const SwIspStats *stats,\n>> +\t\t ControlList &metadata) override;\n>> +\n>> +private:\n>> +\tvoid updateGammaTable(IPAContext &context);\n>> +};\n>> +\n>> +} /* namespace ipa::soft::algorithms */\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build\n>> index 1f63c220..324c7b4e 100644\n>> --- a/src/ipa/simple/algorithms/meson.build\n>> +++ b/src/ipa/simple/algorithms/meson.build\n>> @@ -2,4 +2,5 @@\n>> soft_simple_ipa_algorithms = files([\n>> 'blc.cpp',\n>> + 'colors.cpp',\n>> ])\n>> diff --git a/src/ipa/simple/data/uncalibrated.yaml b/src/ipa/simple/data/uncalibrated.yaml\n>> index e0d03d96..7e5149e5 100644\n>> --- a/src/ipa/simple/data/uncalibrated.yaml\n>> +++ b/src/ipa/simple/data/uncalibrated.yaml\n>> @@ -4,4 +4,5 @@\n>> version: 1\n>> algorithms:\n>> - BlackLevel:\n>> + - Colors:\n>> ...\n>> diff --git a/src/ipa/simple/ipa_context.cpp b/src/ipa/simple/ipa_context.cpp\n>> index 268cff32..5fa492cb 100644\n>> --- a/src/ipa/simple/ipa_context.cpp\n>> +++ b/src/ipa/simple/ipa_context.cpp\n>> @@ -50,6 +50,11 @@ namespace libcamera::ipa::soft {\n>> * \\brief The current state of IPA algorithms\n>> */\n>> +/**\n>> + * \\var IPASessionConfiguration::gamma\n>> + * \\brief Gamma value to be used in the raw image processing\n>> + */\n>> +\n>> /**\n>> * \\var IPAActiveState::black\n>> * \\brief Context for the Black Level algorithm\n>> @@ -58,4 +63,29 @@ namespace libcamera::ipa::soft {\n>> * \\brief Current determined black level\n>> */\n>> +/**\n>> + * \\var IPAActiveState::gains\n>> + * \\brief Context for gains in the Colors algorithm\n>> + *\n>> + * \\var IPAActiveState::gains.red\n>> + * \\brief Gain of red color\n>> + *\n>> + * \\var IPAActiveState::gains.green\n>> + * \\brief Gain of green color\n>> + *\n>> + * \\var IPAActiveState::gains.blue\n>> + * \\brief Gain of blue color\n>> + */\n>> +\n>> +/**\n>> + * \\var IPAActiveState::gamma\n>> + * \\brief Context for gamma in the Colors algorithm\n>> + *\n>> + * \\var IPAActiveState::gamma.gammaTable\n>> + * \\brief Computed gamma table\n>> + *\n>> + * \\var IPAActiveState::gamma.blackLevel\n>> + * \\brief Black level used for the gamma table computation\n>> + */\n>> +\n>> } /* namespace libcamera::ipa::soft */\n>> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n>> index ed07dbb8..979ddb8f 100644\n>> --- a/src/ipa/simple/ipa_context.h\n>> +++ b/src/ipa/simple/ipa_context.h\n>> @@ -8,6 +8,7 @@\n>> #pragma once\n>> +#include \n>> #include \n>> #include \n>> @@ -17,12 +18,23 @@ namespace libcamera {\n>> namespace ipa::soft {\n>> struct IPASessionConfiguration {\n>> +\tfloat gamma;\n>> };\n>> struct IPAActiveState {\n>> \tstruct {\n>> \t\tuint8_t level;\n>> \t} black;\n>> +\tstruct {\n>> +\t\tunsigned int red;\n>> +\t\tunsigned int green;\n>> +\t\tunsigned int blue;\n>> +\t} gains;\n>> +\tstatic constexpr unsigned int kGammaLookupSize = 1024;\n>> +\tstruct {\n>> +\t\tstd::array gammaTable;\n>> +\t\tuint8_t blackLevel;\n>> +\t} gamma;\n>> };\n>> struct IPAFrameContext : public FrameContext {\n>> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp\n>> index 2fb3a473..2658e359 100644\n>> --- a/src/ipa/simple/soft_simple.cpp\n>> +++ b/src/ipa/simple/soft_simple.cpp\n>> @@ -5,8 +5,6 @@\n>> * Simple Software Image Processing Algorithm module\n>> */\n>> -#include \n>> -#include \n>> #include \n>> #include \n>> @@ -92,9 +90,6 @@ private:\n>> \tstd::unique_ptr camHelper_;\n>> \tControlInfoMap sensorInfoMap_;\n>> -\tstatic constexpr unsigned int kGammaLookupSize = 1024;\n>> -\tstd::array gammaTable_;\n>> -\tint lastBlackLevel_ = -1;\n>> \t/* Local parameter storage */\n>> \tstruct IPAContext context_;\n>> @@ -282,6 +277,7 @@ void IPASoftSimple::prepare(const uint32_t frame)\n>> \tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n>> \tfor (auto const &algo : algorithms())\n>> \t\talgo->prepare(context_, frame, frameContext, params_);\n>> +\tsetIspParams.emit();\n>> }\n>> void IPASoftSimple::processStats(\n>> @@ -300,62 +296,6 @@ void IPASoftSimple::processStats(\n>> \tfor (auto const &algo : algorithms())\n>> \t\talgo->process(context_, frame, frameContext, stats_, metadata);\n>> -\tSwIspStats::Histogram histogram = stats_->yHistogram;\n>> -\tconst uint8_t blackLevel = context_.activeState.black.level;\n>> -\n>> -\t/*\n>> -\t * Black level must be subtracted to get the correct AWB ratios, they\n>> -\t * would be off if they were computed from the whole brightness range\n>> -\t * rather than from the sensor range.\n>> -\t */\n>> -\tconst uint64_t nPixels = std::accumulate(\n>> -\t\thistogram.begin(), histogram.end(), 0);\n>> -\tconst uint64_t offset = blackLevel * nPixels;\n>> -\tconst uint64_t sumR = stats_->sumR_ - offset / 4;\n>> -\tconst uint64_t sumG = stats_->sumG_ - offset / 2;\n>> -\tconst uint64_t sumB = stats_->sumB_ - offset / 4;\n>> -\n>> -\t/*\n>> -\t * Calculate red and blue gains for AWB.\n>> -\t * Clamp max gain at 4.0, this also avoids 0 division.\n>> -\t * Gain: 128 = 0.5, 256 = 1.0, 512 = 2.0, etc.\n>> -\t */\n>> -\tconst unsigned int gainR = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR;\n>> -\tconst unsigned int gainB = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB;\n>> -\t/* Green gain and gamma values are fixed */\n>> -\tconstexpr unsigned int gainG = 256;\n>> -\n>> -\t/* Update the gamma table if needed */\n>> -\tif (blackLevel != lastBlackLevel_) {\n>> -\t\tconstexpr float gamma = 0.5;\n>> -\t\tconst unsigned int blackIndex = blackLevel * kGammaLookupSize / 256;\n>> -\t\tstd::fill(gammaTable_.begin(), gammaTable_.begin() + blackIndex, 0);\n>> -\t\tconst float divisor = kGammaLookupSize - blackIndex - 1.0;\n>> -\t\tfor (unsigned int i = blackIndex; i < kGammaLookupSize; i++)\n>> -\t\t\tgammaTable_[i] = UINT8_MAX *\n>> -\t\t\t\t\t std::pow((i - blackIndex) / divisor, gamma);\n>> -\n>> -\t\tlastBlackLevel_ = blackLevel;\n>> -\t}\n>> -\n>> -\tfor (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {\n>> -\t\tconstexpr unsigned int div =\n>> -\t\t\tDebayerParams::kRGBLookupSize * 256 / kGammaLookupSize;\n>> -\t\tunsigned int idx;\n>> -\n>> -\t\t/* Apply gamma after gain! */\n>> -\t\tidx = std::min({ i * gainR / div, (kGammaLookupSize - 1) });\n>> -\t\tparams_->red[i] = gammaTable_[idx];\n>> -\n>> -\t\tidx = std::min({ i * gainG / div, (kGammaLookupSize - 1) });\n>> -\t\tparams_->green[i] = gammaTable_[idx];\n>> -\n>> -\t\tidx = std::min({ i * gainB / div, (kGammaLookupSize - 1) });\n>> -\t\tparams_->blue[i] = gammaTable_[idx];\n>> -\t}\n>> -\n>> -\tsetIspParams.emit();\n>> -\n>> \t/* \\todo Switch to the libipa/algorithm.h API someday. */\n>> \t/*\n>> @@ -372,6 +312,7 @@ void IPASoftSimple::processStats(\n>> \t * Calculate Mean Sample Value (MSV) according to formula from:\n>> \t * https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf\n>> \t */\n>> +\tconst uint8_t blackLevel = context_.activeState.black.level;\n>> \tconst unsigned int blackLevelHistIdx =\n>> \t\tblackLevel / (256 / SwIspStats::kYHistogramSize);\n>> \tconst unsigned int histogramSize =\n>> @@ -421,7 +362,8 @@ void IPASoftSimple::processStats(\n>> \tLOG(IPASoft, Debug) << \"exposureMSV \" << exposureMSV\n>> \t\t\t << \" exp \" << exposure_ << \" again \" << again_\n>> -\t\t\t << \" gain R/B \" << gainR << \"/\" << gainB\n>> +\t\t\t << \" gain R/B \" << context_.activeState.gains.red\n>> +\t\t\t << \"/\" << context_.activeState.gains.blue\n>> \t\t\t << \" black level \" << static_cast(blackLevel);\n>> }\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 C6A1FC323E\n\tfor ;\n\tTue, 13 Aug 2024 14:10:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 71651633B5;\n\tTue, 13 Aug 2024 16:10:41 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 456A963382\n\tfor ;\n\tTue, 13 Aug 2024 16:10:40 +0200 (CEST)","from mail-wr1-f71.google.com (mail-wr1-f71.google.com\n\t[209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-283-106XbwJBNuCXep8HQp6o0g-1; Tue, 13 Aug 2024 10:10:37 -0400","by mail-wr1-f71.google.com with SMTP id\n\tffacd0b85a97d-3687eca5980so2715911f8f.0\n\tfor ;\n\tTue, 13 Aug 2024 07:10:36 -0700 (PDT)","from nuthatch (nat-pool-brq-t.redhat.com. [213.175.37.10])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-36e4cfee71esm10334859f8f.50.2024.08.13.07.10.34\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 13 Aug 2024 07:10:34 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"htbaPLLq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1723558239;\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=74NDlKkyJ8H//A/u+xI7Ac+Xv/NAnEoqwPlqIF6xNdM=;\n\tb=htbaPLLq/l3suKOTCXX/0sY7bcyn4chPfPFT/QvJfrvWr27S0lhmlb6PGmwnXxIvywmuMU\n\tkNDo/WRZ4Ssi+CD7MOJiKsuX/GcBExGlNW9DEPJThDaimHIa/WmfKDo0sWePGfcOvIqoSI\n\tiagtXoLyxcG5LyVT7kRGj1b4A5M4NP0=","X-MC-Unique":"106XbwJBNuCXep8HQp6o0g-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1723558236; x=1724163036;\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=74NDlKkyJ8H//A/u+xI7Ac+Xv/NAnEoqwPlqIF6xNdM=;\n\tb=mslMx+nTVMfp7LtiErUkCL2Wj4hhubvG+77gLxDimGq4U4grRvuQiB8/cNrl5kPfGm\n\tyUcaRCJTr6fQbJG0PYtUU6iGq0WgbWnBlrr7nobKyRpFpJd/8CPK+75bgefwyR/ZodsZ\n\tdnPRG3BFXgYWB7ppv+o85zcmDGnIy3tOp2lIyXuLuArkdQ+uUhxaiHo8qyzr1H18vRnu\n\t4X7cG7Zk8iUhCl1luHm7HcG6cgJrJ6KmIGFuw7Chg1ReJKUy+8B0UsPWhsqONJXp0xAf\n\tVVCOlCZmsz6y5lTHtiFmHZSNKBPV/PTJEnrJsBRiHJSQJ8S8xZgb1l0Bgsgf8jgy8LMv\n\tHbGQ==","X-Gm-Message-State":"AOJu0YxolIjXXC3CLO45zhkMDLqx/74MDHSUiVlaEDiUbKsEXNtdcFsm\n\tTylOj+E8be2rwNyjtV8JnYF/ZhHAyJHLmeIFSd5uXxMFkQzIun1rCjdtm93kp5yRRCS4Ung8UZm\n\tr7Y7erxYfg076mr4J/qxoJf8J/GJhun2NE2e+bauuBjLHm+kEQliP6cKipWQJOuKcjTpyUecYGq\n\tpLcdyeqTjh7GCyOmw6yJM2vc2oRWAcfFH0YGcktADYqTTa3agHh4sA1Ek=","X-Received":["by 2002:a5d:5009:0:b0:367:98e6:362b with SMTP id\n\tffacd0b85a97d-3716cd2e83amr2594630f8f.42.1723558235549; \n\tTue, 13 Aug 2024 07:10:35 -0700 (PDT)","by 2002:a5d:5009:0:b0:367:98e6:362b with SMTP id\n\tffacd0b85a97d-3716cd2e83amr2594580f8f.42.1723558234705; \n\tTue, 13 Aug 2024 07:10:34 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IEumfxgHmwnhnpLvvIXDF0lFTkUnxomTtph2KlO0BDfYmD5orqqH9howDJKOP3akFtfmUrweQ==","From":"Milan Zamazal ","To":"Dan Scally ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v3 19/23] libcamera: software_isp: Move color handling\n\tto an algorithm module","In-Reply-To":" (Dan\n\tScally's message of \"Tue, 13 Aug 2024 11:54:03 +0100\")","References":"<20240717085444.289997-1-mzamazal@redhat.com>\n\t<20240717085444.289997-20-mzamazal@redhat.com>\n\t","Date":"Tue, 13 Aug 2024 16:10:33 +0200","Message-ID":"<87y150wm1y.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","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":"","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\" "}}]