[{"id":31141,"web_url":"https://patchwork.libcamera.org/comment/31141/","msgid":"<172591519405.2319503.12692639132963479534@ping.linuxembedded.co.uk>","date":"2024-09-09T20:53:14","subject":"Re: [PATCH v6 14/18] libcamera: software_isp: Move color handling to\n\tan algorithm module","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Milan Zamazal (2024-09-06 13:09:23)\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\nAha, I was just commenting on that, but now I see this. I agree, lets\nkeep such a change separate - this is just a refactor patch.\n\n\n> \n> Signed-off-by: Milan Zamazal \n> ---\n> src/ipa/simple/algorithms/awb.cpp | 70 ++++++++++++++++++++++++\n> src/ipa/simple/algorithms/awb.h | 32 +++++++++++\n> src/ipa/simple/algorithms/lut.cpp | 78 +++++++++++++++++++++++++++\n> src/ipa/simple/algorithms/lut.h | 35 ++++++++++++\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..f89f1d3d\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 \n> +#include \n> +\n> +#include \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::init(IPAContext &context,\n> + [[maybe_unused]] const YamlObject &tuningData)\n> +{\n> + auto &gains = context.activeState.gains;\n> + gains.red = gains.green = gains.blue = 256;\n> +\n> + return 0;\n> +}\n> +\n> +void Awb::process(IPAContext &context,\n> + [[maybe_unused]] const uint32_t frame,\n> + [[maybe_unused]] IPAFrameContext &frameContext,\n> + const SwIspStats *stats,\n> + [[maybe_unused]] ControlList &metadata)\n> +{\n> + const SwIspStats::Histogram &histogram = stats->yHistogram;\n> + const uint8_t blackLevel = context.activeState.blc.level;\n> +\n> + /*\n> + * Black level must be subtracted to get the correct AWB ratios, they\n> + * would be off if they were computed from the whole brightness range\n> + * rather than from the sensor range.\n> + */\n> + const uint64_t nPixels = std::accumulate(\n> + histogram.begin(), histogram.end(), 0);\n> + const uint64_t offset = blackLevel * nPixels;\n> + const uint64_t sumR = stats->sumR_ - offset / 4;\n> + const uint64_t sumG = stats->sumG_ - offset / 2;\n> + const uint64_t sumB = stats->sumB_ - offset / 4;\n> +\n> + /*\n> + * Calculate red and blue gains for AWB.\n> + * Clamp max gain at 4.0, this also avoids 0 division.\n> + * Gain: 128 = 0.5, 256 = 1.0, 512 = 2.0, etc.\n> + */\n> + auto &gains = context.activeState.gains;\n> + gains.red = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR;\n> + gains.blue = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB;\n> + /* Green gain is fixed to 256 */\n\nWhy are we encoding fixed point maths here instead of floats?\n\nEdit: Never mind ;-) It was like this before and will be tackled\nseparately.\n\n\n> +\n> + LOG(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..235249b6\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> + Awb() = default;\n> + ~Awb() = default;\n> +\n> + int init(IPAContext &context, const YamlObject &tuningData) override;\n> + void process(IPAContext &context,\n> + const uint32_t frame,\n> + IPAFrameContext &frameContext,\n> + const SwIspStats *stats,\n> + 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..6781f34e\n> --- /dev/null\n> +++ b/src/ipa/simple/algorithms/lut.cpp\n> @@ -0,0 +1,78 @@\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 \n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"simple/ipa_context.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::soft::algorithms {\n> +\n> +int Lut::init(IPAContext &context, [[maybe_unused]] const YamlObject &tuningData)\n> +{\n> + /* Gamma value is fixed */\n> + context.configuration.gamma = 0.5;\n\nI think we specify the gamma control as '1/gamma' ... so I'm curious if\nwe should initialise as gamma = 1 / 2; (But I get that's a bit\nsuperfluos right now, and not for this patch which is just refactoring\nthe structures anyway).\n\n\n\n> + updateGammaTable(context);\n> +\n> + return 0;\n> +}\n> +\n> +void Lut::updateGammaTable(IPAContext &context)\n> +{\n> + auto &gammaTable = context.activeState.gamma.gammaTable;\n> + auto blackLevel = context.activeState.blc.level;\n> + const unsigned int blackIndex = blackLevel * gammaTable.size() / 256;\n> + std::fill(gammaTable.begin(), gammaTable.begin() + blackIndex, 0);\n> + const float divisor = gammaTable.size() - blackIndex - 1.0;\n> + for (unsigned int i = blackIndex; i < gammaTable.size(); i++)\n> + gammaTable[i] = UINT8_MAX * std::pow((i - blackIndex) / divisor,\n> + context.configuration.gamma);\n> + context.activeState.gamma.blackLevel = blackLevel;\n> +}\n> +\n> +void Lut::prepare(IPAContext &context,\n> + [[maybe_unused]] const uint32_t frame,\n> + [[maybe_unused]] IPAFrameContext &frameContext,\n> + [[maybe_unused]] DebayerParams *params)\n> +{\n> + /*\n> + * Update the gamma table if needed. This means if black level changes\n> + * and since the black level gets updated only if a lower value is\n> + * observed, it's not permanently prone to minor fluctuations or\n> + * rounding errors.\n> + */\n> + if (context.activeState.gamma.blackLevel != context.activeState.blc.level)\n> + updateGammaTable(context);\n> + auto &gains = context.activeState.gains;\n> + auto &gammaTable = context.activeState.gamma.gammaTable;\n> + const unsigned int gammaTableSize = gammaTable.size();\n> + for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {\n> + const unsigned int div = static_cast(DebayerParams::kRGBLookupSize) *\n> + 256 / gammaTableSize;\n> + /* Apply gamma after gain! */\n> + unsigned int idx;\n> + idx = std::min({ i * gains.red / div, gammaTableSize - 1 });\n> + params->red[i] = gammaTable[idx];\n> + idx = std::min({ i * gains.green / div, gammaTableSize - 1 });\n> + params->green[i] = gammaTable[idx];\n> + idx = std::min({ i * gains.blue / div, gammaTableSize - 1 });\n> + params->blue[i] = gammaTable[idx];\n> + }\n> +}\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..2c034e9f\n> --- /dev/null\n> +++ b/src/ipa/simple/algorithms/lut.h\n> @@ -0,0 +1,35 @@\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> + Lut() = default;\n> + ~Lut() = default;\n> +\n> + int init(IPAContext &context, const YamlObject &tuningData)\n> + override;\n> + void prepare(IPAContext &context,\n> + const uint32_t frame,\n> + IPAFrameContext &frameContext,\n> + DebayerParams *params) override;\n> +\n> +private:\n> + void 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 \n> #include \n> \n> #include \n> @@ -16,12 +17,25 @@ namespace libcamera {\n> namespace ipa::soft {\n> \n> struct IPASessionConfiguration {\n> + float gamma;\n> };\n> \n> struct IPAActiveState {\n> struct {\n> uint8_t level;\n> } blc;\n> +\n> + struct {\n> + unsigned int red;\n> + unsigned int green;\n> + unsigned int blue;\n> + } gains;\n> +\n> + static constexpr unsigned int kGammaLookupSize = 1024;\n> + struct {\n> + std::array gammaTable;\n> + uint8_t blackLevel;\n\nWhat is the distinction between blc.level and gamma.blackLevel?\n\nI think as this is just restructuring to modules - I'd overlook this for\nfuture cleanups if the redundancy could/should be removed anyway so:\n\n\nReviewed-by: Kieran Bingham \n\n\n> + } 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 \n> -#include \n> #include \n> #include \n> \n> @@ -93,9 +91,6 @@ private:\n> std::unique_ptr camHelper_;\n> ControlInfoMap sensorInfoMap_;\n> \n> - static constexpr unsigned int kGammaLookupSize = 1024;\n> - std::array gammaTable_;\n> - int lastBlackLevel_ = -1;\n> /* Local parameter storage */\n> struct IPAContext context_;\n> \n> @@ -283,6 +278,7 @@ void IPASoftSimple::fillParamsBuffer(const uint32_t frame)\n> IPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> for (auto const &algo : algorithms())\n> algo->prepare(context_, frame, frameContext, params_);\n> + setIspParams.emit();\n> }\n> \n> void IPASoftSimple::processStats(const uint32_t frame,\n> @@ -300,62 +296,6 @@ void IPASoftSimple::processStats(const uint32_t frame,\n> for (auto const &algo : algorithms())\n> algo->process(context_, frame, frameContext, stats_, metadata);\n> \n> - SwIspStats::Histogram histogram = stats_->yHistogram;\n> - const uint8_t blackLevel = context_.activeState.blc.level;\n> -\n> - /*\n> - * Black level must be subtracted to get the correct AWB ratios, they\n> - * would be off if they were computed from the whole brightness range\n> - * rather than from the sensor range.\n> - */\n> - const uint64_t nPixels = std::accumulate(\n> - histogram.begin(), histogram.end(), 0);\n> - const uint64_t offset = blackLevel * nPixels;\n> - const uint64_t sumR = stats_->sumR_ - offset / 4;\n> - const uint64_t sumG = stats_->sumG_ - offset / 2;\n> - const uint64_t sumB = stats_->sumB_ - offset / 4;\n> -\n> - /*\n> - * Calculate red and blue gains for AWB.\n> - * Clamp max gain at 4.0, this also avoids 0 division.\n> - * Gain: 128 = 0.5, 256 = 1.0, 512 = 2.0, etc.\n> - */\n> - const unsigned int gainR = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR;\n> - const unsigned int gainB = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB;\n> - /* Green gain and gamma values are fixed */\n> - constexpr unsigned int gainG = 256;\n> -\n> - /* Update the gamma table if needed */\n> - if (blackLevel != lastBlackLevel_) {\n> - constexpr float gamma = 0.5;\n> - const unsigned int blackIndex = blackLevel * kGammaLookupSize / 256;\n> - std::fill(gammaTable_.begin(), gammaTable_.begin() + blackIndex, 0);\n> - const float divisor = kGammaLookupSize - blackIndex - 1.0;\n> - for (unsigned int i = blackIndex; i < kGammaLookupSize; i++)\n> - gammaTable_[i] = UINT8_MAX *\n> - std::pow((i - blackIndex) / divisor, gamma);\n> -\n> - lastBlackLevel_ = blackLevel;\n> - }\n> -\n> - for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {\n> - constexpr unsigned int div =\n> - DebayerParams::kRGBLookupSize * 256 / kGammaLookupSize;\n> - unsigned int idx;\n> -\n> - /* Apply gamma after gain! */\n> - idx = std::min({ i * gainR / div, (kGammaLookupSize - 1) });\n> - params_->red[i] = gammaTable_[idx];\n> -\n> - idx = std::min({ i * gainG / div, (kGammaLookupSize - 1) });\n> - params_->green[i] = gammaTable_[idx];\n> -\n> - idx = std::min({ i * gainB / div, (kGammaLookupSize - 1) });\n> - params_->blue[i] = gammaTable_[idx];\n> - }\n> -\n> - setIspParams.emit();\n> -\n> /* \\todo Switch to the libipa/algorithm.h API someday. */\n> \n> /*\n> @@ -372,6 +312,7 @@ void IPASoftSimple::processStats(const uint32_t frame,\n> * Calculate Mean Sample Value (MSV) according to formula from:\n> * https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf\n> */\n> + const uint8_t blackLevel = context_.activeState.blc.level;\n> const unsigned int blackLevelHistIdx =\n> blackLevel / (256 / SwIspStats::kYHistogramSize);\n> const unsigned int histogramSize =\n> @@ -421,7 +362,6 @@ void IPASoftSimple::processStats(const uint32_t frame,\n> \n> LOG(IPASoft, Debug) << \"exposureMSV \" << exposureMSV\n> << \" exp \" << exposure_ << \" again \" << again_\n> - << \" gain R/B \" << gainR << \"/\" << gainB\n> << \" black level \" << static_cast(blackLevel);\n> }\n> \n> -- \n> 2.44.1\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 3C40AC324C\n\tfor ;\n\tMon, 9 Sep 2024 20:53:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C5F46634F7;\n\tMon, 9 Sep 2024 22:53:18 +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 E5703634EB\n\tfor ;\n\tMon, 9 Sep 2024 22:53:16 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3FC8D827;\n\tMon, 9 Sep 2024 22:52:00 +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=\"Hal8Z0CT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1725915120;\n\tbh=h/iGwCNBjJhAtL15x4Om/W+SShCjUGGzkYyy0gf4otg=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=Hal8Z0CT5tSsg1AXT1yu7ThphskOttm4mKUsX0cQSveJPlxn76Wm4TBwdeigng/YG\n\t4Mv2WcaIhRtSwd7cUZc7XUXRqMi3QQRSVAYcUuR67RadGBAf1GlNqkAk34BbYF4Yvr\n\tX7rSkaD3z9bxVTPAhr1ComeJ2nKbNIGdHPwn+EcY=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240906120927.4071508-15-mzamazal@redhat.com>","References":"<20240906120927.4071508-1-mzamazal@redhat.com>\n\t<20240906120927.4071508-15-mzamazal@redhat.com>","Subject":"Re: [PATCH v6 14/18] libcamera: software_isp: Move color handling to\n\tan algorithm module","From":"Kieran Bingham ","Cc":"Milan Zamazal ,\n\tUmang Jain ,\n\tLaurent Pinchart ,\n\tDaniel Scally ","To":"Milan Zamazal , libcamera-devel@lists.libcamera.org","Date":"Mon, 09 Sep 2024 21:53:14 +0100","Message-ID":"<172591519405.2319503.12692639132963479534@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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":31277,"web_url":"https://patchwork.libcamera.org/comment/31277/","msgid":"<87y13nle1j.fsf@redhat.com>","date":"2024-09-19T18:00:08","subject":"Re: [PATCH v6 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 Kieran,\n\nthank you for review.\n\nKieran Bingham writes:\n\n> Quoting Milan Zamazal (2024-09-06 13:09:23)\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>\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> Aha, I was just commenting on that, but now I see this. I agree, lets\n> keep such a change separate - this is just a refactor patch.\n>\n>\n>> \n>> Signed-off-by: Milan Zamazal \n>> ---\n>> src/ipa/simple/algorithms/awb.cpp | 70 ++++++++++++++++++++++++\n>> src/ipa/simple/algorithms/awb.h | 32 +++++++++++\n>> src/ipa/simple/algorithms/lut.cpp | 78 +++++++++++++++++++++++++++\n>> src/ipa/simple/algorithms/lut.h | 35 ++++++++++++\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..f89f1d3d\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 \n>> +#include \n>> +\n>> +#include \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::init(IPAContext &context,\n>> + [[maybe_unused]] const YamlObject &tuningData)\n>> +{\n>> + auto &gains = context.activeState.gains;\n>> + gains.red = gains.green = gains.blue = 256;\n>> +\n>> + return 0;\n>> +}\n>> +\n>> +void Awb::process(IPAContext &context,\n>> + [[maybe_unused]] const uint32_t frame,\n>> + [[maybe_unused]] IPAFrameContext &frameContext,\n>> + const SwIspStats *stats,\n>> + [[maybe_unused]] ControlList &metadata)\n>> +{\n>> + const SwIspStats::Histogram &histogram = stats->yHistogram;\n>> + const uint8_t blackLevel = context.activeState.blc.level;\n>> +\n>> + /*\n>> + * Black level must be subtracted to get the correct AWB ratios, they\n>> + * would be off if they were computed from the whole brightness range\n>> + * rather than from the sensor range.\n>> + */\n>> + const uint64_t nPixels = std::accumulate(\n>> + histogram.begin(), histogram.end(), 0);\n>> + const uint64_t offset = blackLevel * nPixels;\n>> + const uint64_t sumR = stats->sumR_ - offset / 4;\n>> + const uint64_t sumG = stats->sumG_ - offset / 2;\n>> + const uint64_t sumB = stats->sumB_ - offset / 4;\n>> +\n>> + /*\n>> + * Calculate red and blue gains for AWB.\n>> + * Clamp max gain at 4.0, this also avoids 0 division.\n>> + * Gain: 128 = 0.5, 256 = 1.0, 512 = 2.0, etc.\n>> + */\n>> + auto &gains = context.activeState.gains;\n>> + gains.red = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR;\n>> + gains.blue = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB;\n>> + /* Green gain is fixed to 256 */\n>\n> Why are we encoding fixed point maths here instead of floats?\n>\n> Edit: Never mind ;-) It was like this before and will be tackled\n> separately.\n>\n>\n>> +\n>> + LOG(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..235249b6\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>> + Awb() = default;\n>> + ~Awb() = default;\n>> +\n>> + int init(IPAContext &context, const YamlObject &tuningData) override;\n>> + void process(IPAContext &context,\n>> + const uint32_t frame,\n>> + IPAFrameContext &frameContext,\n>> + const SwIspStats *stats,\n>> + 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..6781f34e\n>> --- /dev/null\n>> +++ b/src/ipa/simple/algorithms/lut.cpp\n>> @@ -0,0 +1,78 @@\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 \n>> +#include \n>> +#include \n>> +\n>> +#include \n>> +\n>> +#include \"simple/ipa_context.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +namespace ipa::soft::algorithms {\n>> +\n>> +int Lut::init(IPAContext &context, [[maybe_unused]] const YamlObject &tuningData)\n>> +{\n>> + /* Gamma value is fixed */\n>> + context.configuration.gamma = 0.5;\n>\n> I think we specify the gamma control as '1/gamma' ... so I'm curious if\n> we should initialise as gamma = 1 / 2; (But I get that's a bit\n> superfluos right now, and not for this patch which is just refactoring\n> the structures anyway).\n\nYes, changing from 0.5 to 1/2 in a refactoring patch would add noise.\nWhether to stick with 0.5 or change it 1/2 later, I don't care. Maybe\nif we have a reason to use a non-fixed gamma one day, it'll get changed\nanyway.\n\n>> + updateGammaTable(context);\n>> +\n>> + return 0;\n>> +}\n>> +\n>> +void Lut::updateGammaTable(IPAContext &context)\n>> +{\n>> + auto &gammaTable = context.activeState.gamma.gammaTable;\n>> + auto blackLevel = context.activeState.blc.level;\n>> + const unsigned int blackIndex = blackLevel * gammaTable.size() / 256;\n>> + std::fill(gammaTable.begin(), gammaTable.begin() + blackIndex, 0);\n>> + const float divisor = gammaTable.size() - blackIndex - 1.0;\n>> + for (unsigned int i = blackIndex; i < gammaTable.size(); i++)\n>> + gammaTable[i] = UINT8_MAX * std::pow((i - blackIndex) / divisor,\n>> + context.configuration.gamma);\n>> + context.activeState.gamma.blackLevel = blackLevel;\n>> +}\n>> +\n>> +void Lut::prepare(IPAContext &context,\n>> + [[maybe_unused]] const uint32_t frame,\n>> + [[maybe_unused]] IPAFrameContext &frameContext,\n>> + [[maybe_unused]] DebayerParams *params)\n>> +{\n>> + /*\n>> + * Update the gamma table if needed. This means if black level changes\n>> + * and since the black level gets updated only if a lower value is\n>> + * observed, it's not permanently prone to minor fluctuations or\n>> + * rounding errors.\n>> + */\n>> + if (context.activeState.gamma.blackLevel != context.activeState.blc.level)\n>> + updateGammaTable(context);\n>> + auto &gains = context.activeState.gains;\n>> + auto &gammaTable = context.activeState.gamma.gammaTable;\n>> + const unsigned int gammaTableSize = gammaTable.size();\n>> + for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {\n>> + const unsigned int div = static_cast(DebayerParams::kRGBLookupSize) *\n>> + 256 / gammaTableSize;\n>> + /* Apply gamma after gain! */\n>> + unsigned int idx;\n>> + idx = std::min({ i * gains.red / div, gammaTableSize - 1 });\n>> + params->red[i] = gammaTable[idx];\n>> + idx = std::min({ i * gains.green / div, gammaTableSize - 1 });\n>> + params->green[i] = gammaTable[idx];\n>> + idx = std::min({ i * gains.blue / div, gammaTableSize - 1 });\n>> + params->blue[i] = gammaTable[idx];\n>> + }\n>> +}\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..2c034e9f\n>> --- /dev/null\n>> +++ b/src/ipa/simple/algorithms/lut.h\n>> @@ -0,0 +1,35 @@\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>> + Lut() = default;\n>> + ~Lut() = default;\n>> +\n>> + int init(IPAContext &context, const YamlObject &tuningData)\n>> + override;\n>> + void prepare(IPAContext &context,\n>> + const uint32_t frame,\n>> + IPAFrameContext &frameContext,\n>> + DebayerParams *params) override;\n>> +\n>> +private:\n>> + void 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 \n>> #include \n>> \n>> #include \n>> @@ -16,12 +17,25 @@ namespace libcamera {\n>> namespace ipa::soft {\n>> \n>> struct IPASessionConfiguration {\n>> + float gamma;\n>> };\n>> \n>> struct IPAActiveState {\n>> struct {\n>> uint8_t level;\n>> } blc;\n>> +\n>> + struct {\n>> + unsigned int red;\n>> + unsigned int green;\n>> + unsigned int blue;\n>> + } gains;\n>> +\n>> + static constexpr unsigned int kGammaLookupSize = 1024;\n>> + struct {\n>> + std::array gammaTable;\n>> + uint8_t blackLevel;\n>\n> What is the distinction between blc.level and gamma.blackLevel?\n\nblc.level is the black level. gamma.blackLevel tracks the last black\nlevel used when the gamma lookup table was computed;\nif blc.level != gamma.blackLevel then it means the gamma table must be\nrecomputed. Not particularly pretty but the other options that came to\nmy mind were even uglier.\n\n> I think as this is just restructuring to modules - I'd overlook this for\n> future cleanups if the redundancy could/should be removed anyway so:\n>\n>\n> Reviewed-by: Kieran Bingham \n>\n>\n>> + } 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 \n>> -#include \n>> #include \n>> #include \n>> \n>> @@ -93,9 +91,6 @@ private:\n>> std::unique_ptr camHelper_;\n>> ControlInfoMap sensorInfoMap_;\n>> \n>> - static constexpr unsigned int kGammaLookupSize = 1024;\n>> - std::array gammaTable_;\n>> - int lastBlackLevel_ = -1;\n>> /* Local parameter storage */\n>> struct IPAContext context_;\n>> \n>> @@ -283,6 +278,7 @@ void IPASoftSimple::fillParamsBuffer(const uint32_t frame)\n>> IPAFrameContext &frameContext = context_.frameContexts.get(frame);\n>> for (auto const &algo : algorithms())\n>> algo->prepare(context_, frame, frameContext, params_);\n>> + setIspParams.emit();\n>> }\n>> \n>> void IPASoftSimple::processStats(const uint32_t frame,\n>> @@ -300,62 +296,6 @@ void IPASoftSimple::processStats(const uint32_t frame,\n>> for (auto const &algo : algorithms())\n>> algo->process(context_, frame, frameContext, stats_, metadata);\n>> \n>> - SwIspStats::Histogram histogram = stats_->yHistogram;\n>> - const uint8_t blackLevel = context_.activeState.blc.level;\n>> -\n>> - /*\n>> - * Black level must be subtracted to get the correct AWB ratios, they\n>> - * would be off if they were computed from the whole brightness range\n>> - * rather than from the sensor range.\n>> - */\n>> - const uint64_t nPixels = std::accumulate(\n>> - histogram.begin(), histogram.end(), 0);\n>> - const uint64_t offset = blackLevel * nPixels;\n>> - const uint64_t sumR = stats_->sumR_ - offset / 4;\n>> - const uint64_t sumG = stats_->sumG_ - offset / 2;\n>> - const uint64_t sumB = stats_->sumB_ - offset / 4;\n>> -\n>> - /*\n>> - * Calculate red and blue gains for AWB.\n>> - * Clamp max gain at 4.0, this also avoids 0 division.\n>> - * Gain: 128 = 0.5, 256 = 1.0, 512 = 2.0, etc.\n>> - */\n>> - const unsigned int gainR = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR;\n>> - const unsigned int gainB = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB;\n>> - /* Green gain and gamma values are fixed */\n>> - constexpr unsigned int gainG = 256;\n>> -\n>> - /* Update the gamma table if needed */\n>> - if (blackLevel != lastBlackLevel_) {\n>> - constexpr float gamma = 0.5;\n>> - const unsigned int blackIndex = blackLevel * kGammaLookupSize / 256;\n>> - std::fill(gammaTable_.begin(), gammaTable_.begin() + blackIndex, 0);\n>> - const float divisor = kGammaLookupSize - blackIndex - 1.0;\n>> - for (unsigned int i = blackIndex; i < kGammaLookupSize; i++)\n>> - gammaTable_[i] = UINT8_MAX *\n>> - std::pow((i - blackIndex) / divisor, gamma);\n>> -\n>> - lastBlackLevel_ = blackLevel;\n>> - }\n>> -\n>> - for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {\n>> - constexpr unsigned int div =\n>> - DebayerParams::kRGBLookupSize * 256 / kGammaLookupSize;\n>> - unsigned int idx;\n>> -\n>> - /* Apply gamma after gain! */\n>> - idx = std::min({ i * gainR / div, (kGammaLookupSize - 1) });\n>> - params_->red[i] = gammaTable_[idx];\n>> -\n>> - idx = std::min({ i * gainG / div, (kGammaLookupSize - 1) });\n>> - params_->green[i] = gammaTable_[idx];\n>> -\n>> - idx = std::min({ i * gainB / div, (kGammaLookupSize - 1) });\n>> - params_->blue[i] = gammaTable_[idx];\n>> - }\n>> -\n>> - setIspParams.emit();\n>> -\n>> /* \\todo Switch to the libipa/algorithm.h API someday. */\n>> \n>> /*\n>> @@ -372,6 +312,7 @@ void IPASoftSimple::processStats(const uint32_t frame,\n>> * Calculate Mean Sample Value (MSV) according to formula from:\n>> * https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf\n>> */\n>> + const uint8_t blackLevel = context_.activeState.blc.level;\n>> const unsigned int blackLevelHistIdx =\n>> blackLevel / (256 / SwIspStats::kYHistogramSize);\n>> const unsigned int histogramSize =\n>> @@ -421,7 +362,6 @@ void IPASoftSimple::processStats(const uint32_t frame,\n>> \n>> LOG(IPASoft, Debug) << \"exposureMSV \" << exposureMSV\n>> << \" exp \" << exposure_ << \" again \" << again_\n>> - << \" gain R/B \" << gainR << \"/\" << gainB\n>> << \" black level \" << static_cast(blackLevel);\n>> }\n>> \n>> -- \n>> 2.44.1\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 5C6F3C324C\n\tfor ;\n\tThu, 19 Sep 2024 18:00:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 44ED0634FC;\n\tThu, 19 Sep 2024 20:00:21 +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 346F7618E7\n\tfor ;\n\tThu, 19 Sep 2024 20:00:17 +0200 (CEST)","from mail-ej1-f71.google.com (mail-ej1-f71.google.com\n\t[209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-377-MBxO4VxdMCahncw5qVwmsA-1; Thu, 19 Sep 2024 14:00:13 -0400","by mail-ej1-f71.google.com with SMTP id\n\ta640c23a62f3a-a8d15eff783so71334066b.3\n\tfor ;\n\tThu, 19 Sep 2024 11:00: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\ta640c23a62f3a-a90610f3aaesm749632066b.53.2024.09.19.11.00.08\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 19 Sep 2024 11:00: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=\"I+N/O7TA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1726768816;\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=E4g1g52W6IjPPDJLsQOcqnr4jBNEqrig2v7y2tohdOg=;\n\tb=I+N/O7TAA+Cqd4uymoc1PyMXgd2M/8MOmfo//7rUEI+TpxQfRLA0tTNBqo7cUedLKiqATK\n\tTuo07rrDJvKk1CVZ8G8gzmaEYtLSsWoRggGAN1K/GmtESGzptQ37U1MQynCktZQPg025Ks\n\tKdFZfBT5D6eA5P5MOwZyVcw4Y5KYYRc=","X-MC-Unique":"MBxO4VxdMCahncw5qVwmsA-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1726768811; x=1727373611;\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=E4g1g52W6IjPPDJLsQOcqnr4jBNEqrig2v7y2tohdOg=;\n\tb=J/UN7oyAsLJwJhrU/aErjJDUFniA0GgRxJ6X2Kav/V790K+UzJV+C2r6hYKrJ4udOa\n\tO5mPyCgSha1NUmKhIObbNCniNN+NtVcAGs3l7s8chMRqzIW6LpnfM2HNNrfYvCYAetq5\n\tb8gj7rIWy7CFn+s+oYfNMWheJFHfbXXGe9F5vQtrDIxCwEMgtfVs32j32vwlKShqmuq9\n\tdKn9ocrtYv6eTanS3kuo0x6NcFeI/bT839dX7d+Ko4RbuDXTj+5vOOZUT+Eo6Aen9p8s\n\tkxCg8nXIdfhSPi1uJrDC5zAnOhVa/jbY8NU3bpWKFWtJfMDWmyqLPn6btwE8f13NxIWk\n\tSLkQ==","X-Gm-Message-State":"AOJu0YwRdPdWX3x4wvBM1XqYNdjS96ZeUvNdfxu0Q2WXhYk6xF8Co/JU\n\txVct9uRQMDNL5490eTUBMf7/qK6zG1YJnGWFhaMmIrhQr3/l7vLj/YSce6vIIB4Bg904Zqku2fu\n\t3d93wkMTDxMImMqJBrcgxhB2RcvOHSCrGM8YH8QJE/WeSe9X2mq4XNMg/Wq4yFZNd29LCPX8=","X-Received":["by 2002:a17:907:d3db:b0:a90:41a9:7c3e with SMTP id\n\ta640c23a62f3a-a90d51c3df2mr2681366b.65.1726768811008; \n\tThu, 19 Sep 2024 11:00:11 -0700 (PDT)","by 2002:a17:907:d3db:b0:a90:41a9:7c3e with SMTP id\n\ta640c23a62f3a-a90d51c3df2mr2678066b.65.1726768810386; \n\tThu, 19 Sep 2024 11:00:10 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IHqGvSPW2+Br7x/4joZGxlmDmafL7oxV4r0Hq9JDSBe4w/1x4vhHa7aWt3col3nlAhFUGoogw==","From":"Milan Zamazal ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org, Umang Jain\n\t, Laurent Pinchart\n\t, Daniel Scally\n\t","Subject":"Re: [PATCH v6 14/18] libcamera: software_isp: Move color handling\n\tto an algorithm module","In-Reply-To":"<172591519405.2319503.12692639132963479534@ping.linuxembedded.co.uk>\n\t(Kieran Bingham's message of \"Mon, 09 Sep 2024 21:53:14 +0100\")","References":"<20240906120927.4071508-1-mzamazal@redhat.com>\n\t<20240906120927.4071508-15-mzamazal@redhat.com>\n\t<172591519405.2319503.12692639132963479534@ping.linuxembedded.co.uk>","Date":"Thu, 19 Sep 2024 20:00:08 +0200","Message-ID":"<87y13nle1j.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\" "}}]