[{"id":31428,"web_url":"https://patchwork.libcamera.org/comment/31428/","msgid":"<91520df7-627d-43a2-95bb-97927c9c2cd4@ideasonboard.com>","date":"2024-09-27T06:29:18","subject":"Re: [PATCH v7 14/18] libcamera: software_isp: Move color handling to\n\tan algorithm module","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hello Milan,\n\nThank you for the patch.\n\nOn 21/09/24 12:01 am, 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 algorithm\n> modules too.\n>\n> This time, the moved code is split between stats processing and\n> parameter construction methods.  It is also split to two algorithm\n> modules:\n>\n> - White balance computation.\n>\n> - Gamma table computation and color lookup tables construction.  While\n>    this applies the color gains computed by the white balance algorithm,\n>    it is not directly related to white balance.  And we may want to\n>    modify the color lookup tables in future according to other parameters\n>    than just gamma and white balance gains.\n>\n> Gamma table computation and color lookup tables construction could be\n> split to separate algorithms too.  But there is no big need for that now\n> so they are kept together for simplicity.\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 <mzamazal@redhat.com>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n>   src/ipa/simple/algorithms/awb.cpp     | 70 ++++++++++++++++++++++++\n>   src/ipa/simple/algorithms/awb.h       | 32 +++++++++++\n>   src/ipa/simple/algorithms/lut.cpp     | 79 +++++++++++++++++++++++++++\n>   src/ipa/simple/algorithms/lut.h       | 34 ++++++++++++\n>   src/ipa/simple/algorithms/meson.build |  2 +\n>   src/ipa/simple/data/uncalibrated.yaml |  2 +\n>   src/ipa/simple/ipa_context.cpp        | 30 ++++++++++\n>   src/ipa/simple/ipa_context.h          | 14 +++++\n>   src/ipa/simple/soft_simple.cpp        | 64 +---------------------\n>   9 files changed, 265 insertions(+), 62 deletions(-)\n>   create mode 100644 src/ipa/simple/algorithms/awb.cpp\n>   create mode 100644 src/ipa/simple/algorithms/awb.h\n>   create mode 100644 src/ipa/simple/algorithms/lut.cpp\n>   create mode 100644 src/ipa/simple/algorithms/lut.h\n>\n> diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp\n> new file mode 100644\n> index 00000000..3d76cc2f\n> --- /dev/null\n> +++ b/src/ipa/simple/algorithms/awb.cpp\n> @@ -0,0 +1,70 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Red Hat Inc.\n> + *\n> + * Auto white balance\n> + */\n> +\n> +#include \"awb.h\"\n> +\n> +#include <numeric>\n> +#include <stdint.h>\n> +\n> +#include <libcamera/base/log.h>\n> +\n> +#include \"simple/ipa_context.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(IPASoftAwb)\n> +\n> +namespace ipa::soft::algorithms {\n> +\n> +int Awb::configure(IPAContext &context,\n> +\t\t   [[maybe_unused]] const IPAConfigInfo &configInfo)\n> +{\n> +\tauto &gains = context.activeState.gains;\n> +\tgains.red = gains.green = gains.blue = 256;\n> +\n> +\treturn 0;\n> +}\n> +\n> +void Awb::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.blc.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(IPASoftAwb, Debug) << \"gain R/B \" << gains.red << \"/\" << gains.blue;\n> +}\n> +\n> +REGISTER_IPA_ALGORITHM(Awb, \"Awb\")\n> +\n> +} /* namespace ipa::soft::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/simple/algorithms/awb.h b/src/ipa/simple/algorithms/awb.h\n> new file mode 100644\n> index 00000000..db1496cd\n> --- /dev/null\n> +++ b/src/ipa/simple/algorithms/awb.h\n> @@ -0,0 +1,32 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Red Hat Inc.\n> + *\n> + * Auto white balance\n> + */\n> +\n> +#pragma once\n> +\n> +#include \"algorithm.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::soft::algorithms {\n> +\n> +class Awb : public Algorithm\n> +{\n> +public:\n> +\tAwb() = default;\n> +\t~Awb() = default;\n> +\n> +\tint configure(IPAContext &context, const IPAConfigInfo &configInfo) 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> +\n> +} /* namespace ipa::soft::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/simple/algorithms/lut.cpp b/src/ipa/simple/algorithms/lut.cpp\n> new file mode 100644\n> index 00000000..44e13864\n> --- /dev/null\n> +++ b/src/ipa/simple/algorithms/lut.cpp\n> @@ -0,0 +1,79 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Red Hat Inc.\n> + *\n> + * Color lookup tables construction\n> + */\n> +\n> +#include \"lut.h\"\n> +\n> +#include <algorithm>\n> +#include <cmath>\n> +#include <stdint.h>\n> +\n> +#include <libcamera/base/log.h>\n> +\n> +#include \"simple/ipa_context.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::soft::algorithms {\n> +\n> +int Lut::configure(IPAContext &context,\n> +\t\t   [[maybe_unused]] const IPAConfigInfo &configInfo)\n> +{\n> +\t/* Gamma value is fixed */\n> +\tcontext.configuration.gamma = 0.5;\n> +\tupdateGammaTable(context);\n> +\n> +\treturn 0;\n> +}\n> +\n> +void Lut::updateGammaTable(IPAContext &context)\n> +{\n> +\tauto &gammaTable = context.activeState.gamma.gammaTable;\n> +\tauto blackLevel = context.activeState.blc.level;\n> +\tconst unsigned int blackIndex = blackLevel * gammaTable.size() / 256;\n> +\tstd::fill(gammaTable.begin(), gammaTable.begin() + blackIndex, 0);\n> +\tconst float divisor = gammaTable.size() - blackIndex - 1.0;\n> +\tfor (unsigned int i = blackIndex; i < gammaTable.size(); i++)\n> +\t\tgammaTable[i] = UINT8_MAX * std::pow((i - blackIndex) / divisor,\n> +\t\t\t\t\t\t     context.configuration.gamma);\n> +\tcontext.activeState.gamma.blackLevel = blackLevel;\n> +}\n\nPlease introduce newline(s), for appropriate segmentation. It will help \nwith readability of the code.\n\n> +\n> +void Lut::prepare(IPAContext &context,\n> +\t\t  [[maybe_unused]] const uint32_t frame,\n> +\t\t  [[maybe_unused]] IPAFrameContext &frameContext,\n> +\t\t  [[maybe_unused]] DebayerParams *params)\n> +{\n> +\t/*\n> +\t * Update the gamma table if needed. This means if black level changes\n> +\t * and since the black level gets updated only if a lower value is\n> +\t * observed, it's not permanently prone to minor fluctuations or\n> +\t * rounding errors.\n> +\t */\n> +\tif (context.activeState.gamma.blackLevel != context.activeState.blc.level)\n> +\t\tupdateGammaTable(context);\n> +\tauto &gains = context.activeState.gains;\n> +\tauto &gammaTable = context.activeState.gamma.gammaTable;\n> +\tconst unsigned int gammaTableSize = gammaTable.size();\n> +\tfor (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {\n> +\t\tconst unsigned int div = static_cast<double>(DebayerParams::kRGBLookupSize) *\n> +\t\t\t\t\t 256 / gammaTableSize;\n> +\t\t/* Apply gamma after gain! */\n> +\t\tunsigned int idx;\n> +\t\tidx = std::min({ i * gains.red / div, gammaTableSize - 1 });\n> +\t\tparams->red[i] = gammaTable[idx];\n> +\t\tidx = std::min({ i * gains.green / div, gammaTableSize - 1 });\n> +\t\tparams->green[i] = gammaTable[idx];\n> +\t\tidx = std::min({ i * gains.blue / div, gammaTableSize - 1 });\n> +\t\tparams->blue[i] = gammaTable[idx];\n> +\t}\n> +}\n\nDitto.\n\nOtherwise, looks good to me on cursory glance,\n\nAcked-by: Umang Jain <umang.jain@ideasonboard.com>\n> +\n> +REGISTER_IPA_ALGORITHM(Lut, \"Lut\")\n> +\n> +} /* namespace ipa::soft::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/simple/algorithms/lut.h b/src/ipa/simple/algorithms/lut.h\n> new file mode 100644\n> index 00000000..b635987d\n> --- /dev/null\n> +++ b/src/ipa/simple/algorithms/lut.h\n> @@ -0,0 +1,34 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Red Hat Inc.\n> + *\n> + * Color lookup tables construction\n> + */\n> +\n> +#pragma once\n> +\n> +#include \"algorithm.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::soft::algorithms {\n> +\n> +class Lut : public Algorithm\n> +{\n> +public:\n> +\tLut() = default;\n> +\t~Lut() = default;\n> +\n> +\tint configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> +\tvoid prepare(IPAContext &context,\n> +\t\t     const uint32_t frame,\n> +\t\t     IPAFrameContext &frameContext,\n> +\t\t     DebayerParams *params) 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..f575611e 100644\n> --- a/src/ipa/simple/algorithms/meson.build\n> +++ b/src/ipa/simple/algorithms/meson.build\n> @@ -1,5 +1,7 @@\n>   # SPDX-License-Identifier: CC0-1.0\n>   \n>   soft_simple_ipa_algorithms = files([\n> +    'awb.cpp',\n>       'blc.cpp',\n> +    'lut.cpp',\n>   ])\n> diff --git a/src/ipa/simple/data/uncalibrated.yaml b/src/ipa/simple/data/uncalibrated.yaml\n> index e0d03d96..893a0b92 100644\n> --- a/src/ipa/simple/data/uncalibrated.yaml\n> +++ b/src/ipa/simple/data/uncalibrated.yaml\n> @@ -4,4 +4,6 @@\n>   version: 1\n>   algorithms:\n>     - BlackLevel:\n> +  - Awb:\n> +  - Lut:\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 ac2a59d7..737bbbc3 100644\n> --- a/src/ipa/simple/ipa_context.h\n> +++ b/src/ipa/simple/ipa_context.h\n> @@ -7,6 +7,7 @@\n>   \n>   #pragma once\n>   \n> +#include <array>\n>   #include <stdint.h>\n>   \n>   #include <libipa/fc_queue.h>\n> @@ -16,12 +17,25 @@ 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} blc;\n> +\n> +\tstruct {\n> +\t\tunsigned int red;\n> +\t\tunsigned int green;\n> +\t\tunsigned int blue;\n> +\t} gains;\n> +\n> +\tstatic constexpr unsigned int kGammaLookupSize = 1024;\n> +\tstruct {\n> +\t\tstd::array<double, kGammaLookupSize> 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 3332e2b1..f8d923c5 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 <cmath>\n> -#include <numeric>\n>   #include <stdint.h>\n>   #include <sys/mman.h>\n>   \n> @@ -93,9 +91,6 @@ private:\n>   \tstd::unique_ptr<CameraSensorHelper> camHelper_;\n>   \tControlInfoMap sensorInfoMap_;\n>   \n> -\tstatic constexpr unsigned int kGammaLookupSize = 1024;\n> -\tstd::array<uint8_t, kGammaLookupSize> gammaTable_;\n> -\tint lastBlackLevel_ = -1;\n>   \t/* Local parameter storage */\n>   \tstruct IPAContext context_;\n>   \n> @@ -283,6 +278,7 @@ void IPASoftSimple::fillParamsBuffer(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(const uint32_t frame,\n> @@ -300,62 +296,6 @@ void IPASoftSimple::processStats(const uint32_t frame,\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.blc.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(const uint32_t frame,\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.blc.level;\n>   \tconst unsigned int blackLevelHistIdx =\n>   \t\tblackLevel / (256 / SwIspStats::kYHistogramSize);\n>   \tconst unsigned int histogramSize =\n> @@ -421,7 +362,6 @@ void IPASoftSimple::processStats(const uint32_t frame,\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    << \" black level \" << static_cast<unsigned int>(blackLevel);\n>   }\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 6B335C0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 27 Sep 2024 06:29:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2CBF16350F;\n\tFri, 27 Sep 2024 08:29:25 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 42A9760553\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 27 Sep 2024 08:29:23 +0200 (CEST)","from [IPV6:2405:201:2015:f873:55d7:c02e:b2eb:ee3f] (unknown\n\t[IPv6:2405:201:2015:f873:55d7:c02e:b2eb:ee3f])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8A493163;\n\tFri, 27 Sep 2024 08:27:53 +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=\"twFJtwVZ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727418474;\n\tbh=hzrZMwboYUTKbKzg8AWyVJQ2S0uIrNLE/K4wv40gHPw=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=twFJtwVZmz3/1vD3Xol3KnLGwGWtqWwXsiW9LkaBf0V5iAmEYmHqvCUFAhjbS6FdG\n\tUbFxKqNMeVZboyPJpe09yqp3xoQlLiPULVEZ1bw7yJwyNTlZ09KazaercYhNyXmtjr\n\tQ4HeQfjdh00ryNq4SPLrZ8ecK/fUsUgaom6bQo7A=","Message-ID":"<91520df7-627d-43a2-95bb-97927c9c2cd4@ideasonboard.com>","Date":"Fri, 27 Sep 2024 11:59:18 +0530","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v7 14/18] libcamera: software_isp: Move color handling to\n\tan algorithm module","To":"Milan Zamazal <mzamazal@redhat.com>, libcamera-devel@lists.libcamera.org","Cc":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tDaniel Scally <dan.scally@ideasonboard.com>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>","References":"<20240920183143.1674117-1-mzamazal@redhat.com>\n\t<20240920183143.1674117-15-mzamazal@redhat.com>","Content-Language":"en-US","From":"Umang Jain <umang.jain@ideasonboard.com>","In-Reply-To":"<20240920183143.1674117-15-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":"<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":31440,"web_url":"https://patchwork.libcamera.org/comment/31440/","msgid":"<87bk09p70a.fsf@redhat.com>","date":"2024-09-27T13:21:09","subject":"Re: [PATCH v7 14/18] 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 Umang,\n\nthank you for reviews.\n\nUmang Jain <umang.jain@ideasonboard.com> writes:\n\n> Hello Milan,\n>\n> Thank you for the patch.\n>\n> On 21/09/24 12:01 am, 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 algorithm\n>> modules too.\n>>\n>> This time, the moved code is split between stats processing and\n>> parameter construction methods.  It is also split to two algorithm\n>> modules:\n>>\n>> - White balance computation.\n>>\n>> - Gamma table computation and color lookup tables construction.  While\n>>    this applies the color gains computed by the white balance algorithm,\n>>    it is not directly related to white balance.  And we may want to\n>>    modify the color lookup tables in future according to other parameters\n>>    than just gamma and white balance gains.\n>>\n>> Gamma table computation and color lookup tables construction could be\n>> split to separate algorithms too.  But there is no big need for that now\n>> so they are kept together for simplicity.\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 <mzamazal@redhat.com>\n>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>> ---\n>>   src/ipa/simple/algorithms/awb.cpp     | 70 ++++++++++++++++++++++++\n>>   src/ipa/simple/algorithms/awb.h       | 32 +++++++++++\n>>   src/ipa/simple/algorithms/lut.cpp     | 79 +++++++++++++++++++++++++++\n>>   src/ipa/simple/algorithms/lut.h       | 34 ++++++++++++\n>>   src/ipa/simple/algorithms/meson.build |  2 +\n>>   src/ipa/simple/data/uncalibrated.yaml |  2 +\n>>   src/ipa/simple/ipa_context.cpp        | 30 ++++++++++\n>>   src/ipa/simple/ipa_context.h          | 14 +++++\n>>   src/ipa/simple/soft_simple.cpp        | 64 +---------------------\n>>   9 files changed, 265 insertions(+), 62 deletions(-)\n>>   create mode 100644 src/ipa/simple/algorithms/awb.cpp\n>>   create mode 100644 src/ipa/simple/algorithms/awb.h\n>>   create mode 100644 src/ipa/simple/algorithms/lut.cpp\n>>   create mode 100644 src/ipa/simple/algorithms/lut.h\n>>\n>> diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp\n>> new file mode 100644\n>> index 00000000..3d76cc2f\n>> --- /dev/null\n>> +++ b/src/ipa/simple/algorithms/awb.cpp\n>> @@ -0,0 +1,70 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2024, Red Hat Inc.\n>> + *\n>> + * Auto white balance\n>> + */\n>> +\n>> +#include \"awb.h\"\n>> +\n>> +#include <numeric>\n>> +#include <stdint.h>\n>> +\n>> +#include <libcamera/base/log.h>\n>> +\n>> +#include \"simple/ipa_context.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +LOG_DEFINE_CATEGORY(IPASoftAwb)\n>> +\n>> +namespace ipa::soft::algorithms {\n>> +\n>> +int Awb::configure(IPAContext &context,\n>> +\t\t   [[maybe_unused]] const IPAConfigInfo &configInfo)\n>> +{\n>> +\tauto &gains = context.activeState.gains;\n>> +\tgains.red = gains.green = gains.blue = 256;\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +void Awb::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.blc.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(IPASoftAwb, Debug) << \"gain R/B \" << gains.red << \"/\" << gains.blue;\n>> +}\n>> +\n>> +REGISTER_IPA_ALGORITHM(Awb, \"Awb\")\n>> +\n>> +} /* namespace ipa::soft::algorithms */\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/ipa/simple/algorithms/awb.h b/src/ipa/simple/algorithms/awb.h\n>> new file mode 100644\n>> index 00000000..db1496cd\n>> --- /dev/null\n>> +++ b/src/ipa/simple/algorithms/awb.h\n>> @@ -0,0 +1,32 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2024, Red Hat Inc.\n>> + *\n>> + * Auto white balance\n>> + */\n>> +\n>> +#pragma once\n>> +\n>> +#include \"algorithm.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +namespace ipa::soft::algorithms {\n>> +\n>> +class Awb : public Algorithm\n>> +{\n>> +public:\n>> +\tAwb() = default;\n>> +\t~Awb() = default;\n>> +\n>> +\tint configure(IPAContext &context, const IPAConfigInfo &configInfo) 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>> +\n>> +} /* namespace ipa::soft::algorithms */\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/ipa/simple/algorithms/lut.cpp b/src/ipa/simple/algorithms/lut.cpp\n>> new file mode 100644\n>> index 00000000..44e13864\n>> --- /dev/null\n>> +++ b/src/ipa/simple/algorithms/lut.cpp\n>> @@ -0,0 +1,79 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2024, Red Hat Inc.\n>> + *\n>> + * Color lookup tables construction\n>> + */\n>> +\n>> +#include \"lut.h\"\n>> +\n>> +#include <algorithm>\n>> +#include <cmath>\n>> +#include <stdint.h>\n>> +\n>> +#include <libcamera/base/log.h>\n>> +\n>> +#include \"simple/ipa_context.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +namespace ipa::soft::algorithms {\n>> +\n>> +int Lut::configure(IPAContext &context,\n>> +\t\t   [[maybe_unused]] const IPAConfigInfo &configInfo)\n>> +{\n>> +\t/* Gamma value is fixed */\n>> +\tcontext.configuration.gamma = 0.5;\n>> +\tupdateGammaTable(context);\n>> +\n>> +\treturn 0;\n>> +}\n>> +\n>> +void Lut::updateGammaTable(IPAContext &context)\n>> +{\n>> +\tauto &gammaTable = context.activeState.gamma.gammaTable;\n>> +\tauto blackLevel = context.activeState.blc.level;\n>> +\tconst unsigned int blackIndex = blackLevel * gammaTable.size() / 256;\n>> +\tstd::fill(gammaTable.begin(), gammaTable.begin() + blackIndex, 0);\n>> +\tconst float divisor = gammaTable.size() - blackIndex - 1.0;\n>> +\tfor (unsigned int i = blackIndex; i < gammaTable.size(); i++)\n>> +\t\tgammaTable[i] = UINT8_MAX * std::pow((i - blackIndex) / divisor,\n>> +\t\t\t\t\t\t     context.configuration.gamma);\n>> +\tcontext.activeState.gamma.blackLevel = blackLevel;\n>> +}\n>\n> Please introduce newline(s), for appropriate segmentation. It will help with readability of the code.\n\nI often have trouble to know where to put empty lines but I'll add a\ncouple of them here and below, HTH.\n\n>> +\n>> +void Lut::prepare(IPAContext &context,\n>> +\t\t  [[maybe_unused]] const uint32_t frame,\n>> +\t\t  [[maybe_unused]] IPAFrameContext &frameContext,\n>> +\t\t  [[maybe_unused]] DebayerParams *params)\n>> +{\n>> +\t/*\n>> +\t * Update the gamma table if needed. This means if black level changes\n>> +\t * and since the black level gets updated only if a lower value is\n>> +\t * observed, it's not permanently prone to minor fluctuations or\n>> +\t * rounding errors.\n>> +\t */\n>> +\tif (context.activeState.gamma.blackLevel != context.activeState.blc.level)\n>> +\t\tupdateGammaTable(context);\n>> +\tauto &gains = context.activeState.gains;\n>> +\tauto &gammaTable = context.activeState.gamma.gammaTable;\n>> +\tconst unsigned int gammaTableSize = gammaTable.size();\n>> +\tfor (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {\n>> +\t\tconst unsigned int div = static_cast<double>(DebayerParams::kRGBLookupSize) *\n>> +\t\t\t\t\t 256 / gammaTableSize;\n>> +\t\t/* Apply gamma after gain! */\n>> +\t\tunsigned int idx;\n>> +\t\tidx = std::min({ i * gains.red / div, gammaTableSize - 1 });\n>> +\t\tparams->red[i] = gammaTable[idx];\n>> +\t\tidx = std::min({ i * gains.green / div, gammaTableSize - 1 });\n>> +\t\tparams->green[i] = gammaTable[idx];\n>> +\t\tidx = std::min({ i * gains.blue / div, gammaTableSize - 1 });\n>> +\t\tparams->blue[i] = gammaTable[idx];\n>> +\t}\n>> +}\n>\n> Ditto.\n>\n> Otherwise, looks good to me on cursory glance,\n>\n> Acked-by: Umang Jain <umang.jain@ideasonboard.com>\n>> +\n>> +REGISTER_IPA_ALGORITHM(Lut, \"Lut\")\n>> +\n>> +} /* namespace ipa::soft::algorithms */\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/ipa/simple/algorithms/lut.h b/src/ipa/simple/algorithms/lut.h\n>> new file mode 100644\n>> index 00000000..b635987d\n>> --- /dev/null\n>> +++ b/src/ipa/simple/algorithms/lut.h\n>> @@ -0,0 +1,34 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2024, Red Hat Inc.\n>> + *\n>> + * Color lookup tables construction\n>> + */\n>> +\n>> +#pragma once\n>> +\n>> +#include \"algorithm.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +namespace ipa::soft::algorithms {\n>> +\n>> +class Lut : public Algorithm\n>> +{\n>> +public:\n>> +\tLut() = default;\n>> +\t~Lut() = default;\n>> +\n>> +\tint configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n>> +\tvoid prepare(IPAContext &context,\n>> +\t\t     const uint32_t frame,\n>> +\t\t     IPAFrameContext &frameContext,\n>> +\t\t     DebayerParams *params) 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..f575611e 100644\n>> --- a/src/ipa/simple/algorithms/meson.build\n>> +++ b/src/ipa/simple/algorithms/meson.build\n>> @@ -1,5 +1,7 @@\n>>   # SPDX-License-Identifier: CC0-1.0\n>>     soft_simple_ipa_algorithms = files([\n>> +    'awb.cpp',\n>>       'blc.cpp',\n>> +    'lut.cpp',\n>>   ])\n>> diff --git a/src/ipa/simple/data/uncalibrated.yaml b/src/ipa/simple/data/uncalibrated.yaml\n>> index e0d03d96..893a0b92 100644\n>> --- a/src/ipa/simple/data/uncalibrated.yaml\n>> +++ b/src/ipa/simple/data/uncalibrated.yaml\n>> @@ -4,4 +4,6 @@\n>>   version: 1\n>>   algorithms:\n>>     - BlackLevel:\n>> +  - Awb:\n>> +  - Lut:\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 ac2a59d7..737bbbc3 100644\n>> --- a/src/ipa/simple/ipa_context.h\n>> +++ b/src/ipa/simple/ipa_context.h\n>> @@ -7,6 +7,7 @@\n>>     #pragma once\n>>   +#include <array>\n>>   #include <stdint.h>\n>>     #include <libipa/fc_queue.h>\n>> @@ -16,12 +17,25 @@ 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} blc;\n>> +\n>> +\tstruct {\n>> +\t\tunsigned int red;\n>> +\t\tunsigned int green;\n>> +\t\tunsigned int blue;\n>> +\t} gains;\n>> +\n>> +\tstatic constexpr unsigned int kGammaLookupSize = 1024;\n>> +\tstruct {\n>> +\t\tstd::array<double, kGammaLookupSize> 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 3332e2b1..f8d923c5 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 <cmath>\n>> -#include <numeric>\n>>   #include <stdint.h>\n>>   #include <sys/mman.h>\n>>   @@ -93,9 +91,6 @@ private:\n>>   \tstd::unique_ptr<CameraSensorHelper> camHelper_;\n>>   \tControlInfoMap sensorInfoMap_;\n>>   -\tstatic constexpr unsigned int kGammaLookupSize = 1024;\n>> -\tstd::array<uint8_t, kGammaLookupSize> gammaTable_;\n>> -\tint lastBlackLevel_ = -1;\n>>   \t/* Local parameter storage */\n>>   \tstruct IPAContext context_;\n>>   @@ -283,6 +278,7 @@ void IPASoftSimple::fillParamsBuffer(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(const uint32_t frame,\n>> @@ -300,62 +296,6 @@ void IPASoftSimple::processStats(const uint32_t frame,\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.blc.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(const uint32_t frame,\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.blc.level;\n>>   \tconst unsigned int blackLevelHistIdx =\n>>   \t\tblackLevel / (256 / SwIspStats::kYHistogramSize);\n>>   \tconst unsigned int histogramSize =\n>> @@ -421,7 +362,6 @@ void IPASoftSimple::processStats(const uint32_t frame,\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    << \" black level \" << static_cast<unsigned int>(blackLevel);\n>>   }\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 5831CC0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 27 Sep 2024 13:21:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 09CBE6350F;\n\tFri, 27 Sep 2024 15:21:18 +0200 (CEST)","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 47D0A634F2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 27 Sep 2024 15:21:15 +0200 (CEST)","from mail-wm1-f71.google.com (mail-wm1-f71.google.com\n\t[209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-548-9uPNbKXZPsabErqgAbQsOw-1; Fri, 27 Sep 2024 09:21:12 -0400","by mail-wm1-f71.google.com with SMTP id\n\t5b1f17b1804b1-42f310f0ed2so20714605e9.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 27 Sep 2024 06:21:12 -0700 (PDT)","from nuthatch (ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-42e96a55336sm74049445e9.47.2024.09.27.06.21.09\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 27 Sep 2024 06:21:09 -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=\"hNghuEWv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1727443274;\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=eHUwgLz0hgPb49A8N7dcWT/v3w/xdbXs6IOUJyEZyq4=;\n\tb=hNghuEWvMtKxKnoolKhHK+pxHZeQ7Odc5PyTnZ1kxuuX9AhIwsqP4rqZgBX9WmHhj7LFU1\n\tUT6I9YIfcPQYEf/wJD+0kcgPI6We38eeKwb0LJ2eH3OBe+kZpfKnoZj6wjKNjDwEzUV3p9\n\tE5Z92+L58a6rs/4vgKv1Zcqrz1Y1Y1M=","X-MC-Unique":"9uPNbKXZPsabErqgAbQsOw-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1727443271; x=1728048071;\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=eHUwgLz0hgPb49A8N7dcWT/v3w/xdbXs6IOUJyEZyq4=;\n\tb=uocb6EHlWnIqW2Q9Le6538PG9AubssDGubiYssqBvpELf43Dx3wFMBgbijTSrGoEsB\n\tMXc0tjVfqcgOgdwkMB0USVgDBHHyxoAXcI03Efoxm6ea7msl+o3NPNQxxFMKAQynWd5t\n\tTGn+8MyClOqzyDJt6QrP2TnR6Kre+dQ1WgX5jd6PP0WALcd2LAfjHDK5ooNwOVaTsw4k\n\tBiOn825GIY2ZJqxsRZqwxbY1Zl31bzXg0ZiGKSYLQRpLZWtc6zt7QjmZrzYV4bA4Dl9T\n\t7W29//VswCqATZp7mcOAkYVBp+wG8hLwAXWvinNdVV8YtdtfqLKlK6MQ7zp5+HIF/pqS\n\taovg==","X-Gm-Message-State":"AOJu0YzCO6hgO24QOenFqU3DkK3zeVoJ/yB12k749VwNp5Ri8AlA8jI0\n\tWgtF7i71Dx2oMwuRIL4WazK5tf6+aUc2FQldFTQLFmPEx4svzzUGw79hY2t77tkeBxAKNmzUTfw\n\td9vK9RewnwCXn1XHK7YRe9WF0iJ8l3p/TOjFpwxQ177nIZnAra0vB0wzTdIHkaY+u4lROcpU=","X-Received":["by 2002:a05:600c:470a:b0:42c:aeaa:6b0d with SMTP id\n\t5b1f17b1804b1-42f58433a02mr27105245e9.9.1727443271158; \n\tFri, 27 Sep 2024 06:21:11 -0700 (PDT)","by 2002:a05:600c:470a:b0:42c:aeaa:6b0d with SMTP id\n\t5b1f17b1804b1-42f58433a02mr27104945e9.9.1727443270574; \n\tFri, 27 Sep 2024 06:21:10 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IGSXy6/E2+AL9+dszv1CVloJrk1LnCLfujOJIu7GmFNmcllXxX+o1MGobpLfLaaRPvnqodkVw==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Umang Jain <umang.jain@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,  Laurent Pinchart\n\t<laurent.pinchart@ideasonboard.com>,  Daniel Scally\n\t<dan.scally@ideasonboard.com>,  Kieran Bingham\n\t<kieran.bingham@ideasonboard.com>","Subject":"Re: [PATCH v7 14/18] libcamera: software_isp: Move color handling\n\tto an algorithm module","In-Reply-To":"<91520df7-627d-43a2-95bb-97927c9c2cd4@ideasonboard.com> (Umang\n\tJain's message of \"Fri, 27 Sep 2024 11:59:18 +0530\")","References":"<20240920183143.1674117-1-mzamazal@redhat.com>\n\t<20240920183143.1674117-15-mzamazal@redhat.com>\n\t<91520df7-627d-43a2-95bb-97927c9c2cd4@ideasonboard.com>","Date":"Fri, 27 Sep 2024 15:21:09 +0200","Message-ID":"<87bk09p70a.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":"<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>"}}]