[{"id":31675,"web_url":"https://patchwork.libcamera.org/comment/31675/","msgid":"<172851212524.532453.15536968784495669501@ping.linuxembedded.co.uk>","date":"2024-10-09T22:15:25","subject":"Re: [PATCH v2 1/2] libcamera: software_isp: Add contrast algorithm","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Milan Zamazal (2024-10-09 20:33:16)\n> Software ISP is currently fully automatic and doesn't allow image\n> modifications by explicitly set control values.  The user has no means\n> to make the image looking better.\n> \n> This patch introduces contrast control algorithm, which can improve\n> e.g. a flat looking image.  Based on the provided contrast value with a\n> range 0..infinity and 1.0 being the normal value, it applies a simple\n> S-curve modification to the image.  The contrast algorithm just handles\n> the provided values, while the S-curve is applied in the gamma algorithm\n> on the computed gamma curve whenever the contrast value changes.  Since\n> the algorithm is applied only on the lookup table already present, its\n> overhead is negligible.\n> \n> This is a preparation patch without actually activating the contrast\n> algorithm, which will be done in the following patch.\n> \n> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> ---\n>  src/ipa/simple/algorithms/contrast.cpp | 45 ++++++++++++++++++++++++++\n>  src/ipa/simple/algorithms/contrast.h   | 37 +++++++++++++++++++++\n>  src/ipa/simple/algorithms/lut.cpp      | 20 +++++++++---\n>  src/ipa/simple/algorithms/meson.build  |  1 +\n>  src/ipa/simple/ipa_context.h           |  7 ++++\n>  5 files changed, 105 insertions(+), 5 deletions(-)\n>  create mode 100644 src/ipa/simple/algorithms/contrast.cpp\n>  create mode 100644 src/ipa/simple/algorithms/contrast.h\n> \n> diff --git a/src/ipa/simple/algorithms/contrast.cpp b/src/ipa/simple/algorithms/contrast.cpp\n> new file mode 100644\n> index 000000000..1a2c14cf9\n> --- /dev/null\n> +++ b/src/ipa/simple/algorithms/contrast.cpp\n> @@ -0,0 +1,45 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Red Hat Inc.\n> + *\n> + * Contrast adjustment\n> + */\n> +\n> +#include \"contrast.h\"\n> +\n> +#include <optional>\n> +\n> +#include <libcamera/base/log.h>\n> +\n> +#include \"control_ids.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(IPASoftContrast)\n> +\n> +namespace ipa::soft::algorithms {\n> +\n> +int Contrast::configure(typename Module::Context &context,\n> +                       [[maybe_unused]] const typename Module::Config &configInfo)\n> +{\n> +       context.activeState.knobs.contrast = std::optional<double>();\n> +       return 0;\n> +}\n> +\n> +void Contrast::queueRequest(typename Module::Context &context,\n> +                           [[maybe_unused]] const uint32_t frame,\n> +                           [[maybe_unused]] typename Module::FrameContext &frameContext,\n> +                           const ControlList &controls)\n> +{\n> +       const auto &contrast = controls.get(controls::Contrast);\n> +       if (contrast.has_value()) {\n> +               context.activeState.knobs.contrast = contrast;\n> +               LOG(IPASoftContrast, Debug) << \"Setting contrast to \" << contrast.value();\n> +       }\n\nIs this only working by luck that 'Contrast' is alphabetically before\n'LUT' ?\n\nOr in fact, maybe it doesn't matter so much if it's being set here in\nqueueRequest() but it's a separate call in process() anyway to actually\nconsume it.\n\nBut if this impacts the Lut algorithm, it makes me think this is really\na control for that algorithm? Or maybe it doesn't matter... But I don't\nthink we would have 'one algorithm per control'. It's reasonable to have\na colorprocessing algorithm that would manage the LUT +\nContrast+Saturation or such for example...\n\n\n\n> +}\n> +\n> +REGISTER_IPA_ALGORITHM(Contrast, \"Contrast\")\n> +\n> +} /* namespace ipa::soft::algorithms */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/simple/algorithms/contrast.h b/src/ipa/simple/algorithms/contrast.h\n> new file mode 100644\n> index 000000000..0b3933099\n> --- /dev/null\n> +++ b/src/ipa/simple/algorithms/contrast.h\n> @@ -0,0 +1,37 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Red Hat Inc.\n> + *\n> + * Contrast adjustment\n> + */\n> +\n> +#pragma once\n> +\n> +#include <libcamera/controls.h>\n> +\n> +#include \"algorithm.h\"\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::soft::algorithms {\n> +\n> +class Contrast : public Algorithm\n> +{\n> +public:\n> +       Contrast() = default;\n> +       ~Contrast() = default;\n> +\n> +       int configure(typename Module::Context &context,\n> +                     const typename Module::Config &configInfo)\n> +               override;\n> +\n> +       void queueRequest(typename Module::Context &context,\n> +                         const uint32_t frame,\n> +                         typename Module::FrameContext &frameContext,\n> +                         const ControlList &controls)\n> +               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> index 9744e773a..8ebc824ce 100644\n> --- a/src/ipa/simple/algorithms/lut.cpp\n> +++ b/src/ipa/simple/algorithms/lut.cpp\n> @@ -32,16 +32,25 @@ int Lut::configure(IPAContext &context,\n>  void Lut::updateGammaTable(IPAContext &context)\n>  {\n>         auto &gammaTable = context.activeState.gamma.gammaTable;\n> -       auto blackLevel = context.activeState.blc.level;\n> +       const auto blackLevel = context.activeState.blc.level;\n>         const unsigned int blackIndex = blackLevel * gammaTable.size() / 256;\n> +       const auto contrast = context.activeState.knobs.contrast.value_or(1.0);\n>  \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> +       for (unsigned int i = blackIndex; i < gammaTable.size(); i++) {\n> +               double normalized = (i - blackIndex) / divisor;\n> +               /* Apply simple S-curve */\n> +               if (normalized < 0.5)\n> +                       normalized = 0.5 * std::pow(normalized / 0.5, contrast);\n> +               else\n> +                       normalized = 1.0 - 0.5 * std::pow((1.0 - normalized) / 0.5, contrast);\n> +               gammaTable[i] = UINT8_MAX *\n> +                               std::pow(normalized, context.configuration.gamma);\n> +       }\n>  \n>         context.activeState.gamma.blackLevel = blackLevel;\n> +       context.activeState.gamma.contrast = contrast;\n>  }\n>  \n>  void Lut::prepare(IPAContext &context,\n> @@ -55,7 +64,8 @@ void Lut::prepare(IPAContext &context,\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> +       if (context.activeState.gamma.blackLevel != context.activeState.blc.level ||\n> +           context.activeState.gamma.contrast != context.activeState.knobs.contrast)\n>                 updateGammaTable(context);\n>  \n>         auto &gains = context.activeState.gains;\n> diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build\n> index 37a2eb534..d75a7b2a1 100644\n> --- a/src/ipa/simple/algorithms/meson.build\n> +++ b/src/ipa/simple/algorithms/meson.build\n> @@ -4,5 +4,6 @@ soft_simple_ipa_algorithms = files([\n>      'awb.cpp',\n>      'agc.cpp',\n>      'blc.cpp',\n> +    'contrast.cpp',\n>      'lut.cpp',\n>  ])\n> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> index 3519f20f6..5118d6abf 100644\n> --- a/src/ipa/simple/ipa_context.h\n> +++ b/src/ipa/simple/ipa_context.h\n> @@ -8,8 +8,11 @@\n>  #pragma once\n>  \n>  #include <array>\n> +#include <optional>\n>  #include <stdint.h>\n>  \n> +#include <libcamera/controls.h>\n> +\n>  #include <libipa/fc_queue.h>\n>  \n>  namespace libcamera {\n> @@ -44,7 +47,11 @@ struct IPAActiveState {\n>         struct {\n>                 std::array<double, kGammaLookupSize> gammaTable;\n>                 uint8_t blackLevel;\n> +               double contrast;\n>         } gamma;\n> +       struct {\n> +               std::optional<double> contrast; // 0..inf, 1 = neutral\n\nWe avoid C++ commments throughout the project.\n\nIs the definition of the value for contrast documented by the Control in\nlibcamera ? Or is this specific to the implementation detail here ?\n\n\n\n> +       } knobs;\n>  };\n>  \n>  struct IPAFrameContext : public FrameContext {\n> -- \n> 2.44.1\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 A8FF1C32DE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  9 Oct 2024 22:15:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4915D65369;\n\tThu, 10 Oct 2024 00:15:30 +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 2DC4A618C5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 10 Oct 2024 00:15:28 +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 55A69226;\n\tThu, 10 Oct 2024 00:13:50 +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=\"M2fqfRhn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1728512030;\n\tbh=dorioRthJgZUo1jjJbBMoDxGfYoy2dt+sPuRitWxmBs=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=M2fqfRhnF0OvLyaa4f7b8LwoDejjAGrPaDnd83BgXBeEw/3S29hor6AiZkCsxHhJo\n\tFmatCQ28g15NLJbanKiqXngcK2JkACIPIySBKEDnWe10G7vyagKOLoGs9LXmoEs105\n\t840pySrc7s4RX7iEJRHdsTZ/rqLKKHDP5vdW3kWE=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241009193318.2394762-2-mzamazal@redhat.com>","References":"<20241009193318.2394762-1-mzamazal@redhat.com>\n\t<20241009193318.2394762-2-mzamazal@redhat.com>","Subject":"Re: [PATCH v2 1/2] libcamera: software_isp: Add contrast algorithm","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Milan Zamazal <mzamazal@redhat.com>","To":"Milan Zamazal <mzamazal@redhat.com>, libcamera-devel@lists.libcamera.org","Date":"Wed, 09 Oct 2024 23:15:25 +0100","Message-ID":"<172851212524.532453.15536968784495669501@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":"<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":31695,"web_url":"https://patchwork.libcamera.org/comment/31695/","msgid":"<87ed4oqqpk.fsf@redhat.com>","date":"2024-10-10T08:58:47","subject":"Re: [PATCH v2 1/2] libcamera: software_isp: Add contrast algorithm","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 <kieran.bingham@ideasonboard.com> writes:\n\n> Quoting Milan Zamazal (2024-10-09 20:33:16)\n>> Software ISP is currently fully automatic and doesn't allow image\n>> modifications by explicitly set control values.  The user has no means\n>\n>> to make the image looking better.\n>> \n>> This patch introduces contrast control algorithm, which can improve\n>> e.g. a flat looking image.  Based on the provided contrast value with a\n>> range 0..infinity and 1.0 being the normal value, it applies a simple\n>> S-curve modification to the image.  The contrast algorithm just handles\n>> the provided values, while the S-curve is applied in the gamma algorithm\n>> on the computed gamma curve whenever the contrast value changes.  Since\n>> the algorithm is applied only on the lookup table already present, its\n>> overhead is negligible.\n>> \n>> This is a preparation patch without actually activating the contrast\n>> algorithm, which will be done in the following patch.\n>> \n>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>> ---\n>>  src/ipa/simple/algorithms/contrast.cpp | 45 ++++++++++++++++++++++++++\n>>  src/ipa/simple/algorithms/contrast.h   | 37 +++++++++++++++++++++\n>>  src/ipa/simple/algorithms/lut.cpp      | 20 +++++++++---\n>>  src/ipa/simple/algorithms/meson.build  |  1 +\n>>  src/ipa/simple/ipa_context.h           |  7 ++++\n>>  5 files changed, 105 insertions(+), 5 deletions(-)\n>>  create mode 100644 src/ipa/simple/algorithms/contrast.cpp\n>>  create mode 100644 src/ipa/simple/algorithms/contrast.h\n>> \n>> diff --git a/src/ipa/simple/algorithms/contrast.cpp b/src/ipa/simple/algorithms/contrast.cpp\n>> new file mode 100644\n>> index 000000000..1a2c14cf9\n>> --- /dev/null\n>> +++ b/src/ipa/simple/algorithms/contrast.cpp\n>> @@ -0,0 +1,45 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2024, Red Hat Inc.\n>> + *\n>> + * Contrast adjustment\n>> + */\n>> +\n>> +#include \"contrast.h\"\n>> +\n>> +#include <optional>\n>> +\n>> +#include <libcamera/base/log.h>\n>> +\n>> +#include \"control_ids.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +LOG_DEFINE_CATEGORY(IPASoftContrast)\n>> +\n>> +namespace ipa::soft::algorithms {\n>> +\n>> +int Contrast::configure(typename Module::Context &context,\n>> +                       [[maybe_unused]] const typename Module::Config &configInfo)\n>> +{\n>> +       context.activeState.knobs.contrast = std::optional<double>();\n>> +       return 0;\n>> +}\n>> +\n>> +void Contrast::queueRequest(typename Module::Context &context,\n>> +                           [[maybe_unused]] const uint32_t frame,\n>> +                           [[maybe_unused]] typename Module::FrameContext &frameContext,\n>> +                           const ControlList &controls)\n>> +{\n>> +       const auto &contrast = controls.get(controls::Contrast);\n>> +       if (contrast.has_value()) {\n>> +               context.activeState.knobs.contrast = contrast;\n>> +               LOG(IPASoftContrast, Debug) << \"Setting contrast to \" << contrast.value();\n>> +       }\n>\n> Is this only working by luck that 'Contrast' is alphabetically before\n> 'LUT' ?\n\nContrast is put before Lut in the algorithm list in uncalibrated.yaml in\nthe following patch (the algorithm is just defined and not actually used\nin this patch).\n\n> Or in fact, maybe it doesn't matter so much if it's being set here in\n> queueRequest() but it's a separate call in process() anyway to actually\n> consume it.\n>\n> But if this impacts the Lut algorithm, it makes me think this is really\n> a control for that algorithm? Or maybe it doesn't matter... But I don't\n> think we would have 'one algorithm per control'. It's reasonable to have\n> a colorprocessing algorithm that would manage the LUT +\n> Contrast+Saturation or such for example...\n\nOK, I can put it into Lut.\n\n>> +}\n>> +\n>> +REGISTER_IPA_ALGORITHM(Contrast, \"Contrast\")\n>> +\n>> +} /* namespace ipa::soft::algorithms */\n>> +\n>> +} /* namespace libcamera */\n>> diff --git a/src/ipa/simple/algorithms/contrast.h b/src/ipa/simple/algorithms/contrast.h\n>> new file mode 100644\n>> index 000000000..0b3933099\n>> --- /dev/null\n>> +++ b/src/ipa/simple/algorithms/contrast.h\n>> @@ -0,0 +1,37 @@\n>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> +/*\n>> + * Copyright (C) 2024, Red Hat Inc.\n>> + *\n>> + * Contrast adjustment\n>> + */\n>> +\n>> +#pragma once\n>> +\n>> +#include <libcamera/controls.h>\n>> +\n>> +#include \"algorithm.h\"\n>> +\n>> +namespace libcamera {\n>> +\n>> +namespace ipa::soft::algorithms {\n>> +\n>> +class Contrast : public Algorithm\n>> +{\n>> +public:\n>> +       Contrast() = default;\n>> +       ~Contrast() = default;\n>> +\n>> +       int configure(typename Module::Context &context,\n>> +                     const typename Module::Config &configInfo)\n>> +               override;\n>> +\n>> +       void queueRequest(typename Module::Context &context,\n>> +                         const uint32_t frame,\n>> +                         typename Module::FrameContext &frameContext,\n>> +                         const ControlList &controls)\n>> +               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>> index 9744e773a..8ebc824ce 100644\n>> --- a/src/ipa/simple/algorithms/lut.cpp\n>> +++ b/src/ipa/simple/algorithms/lut.cpp\n>> @@ -32,16 +32,25 @@ int Lut::configure(IPAContext &context,\n>>  void Lut::updateGammaTable(IPAContext &context)\n>>  {\n>>         auto &gammaTable = context.activeState.gamma.gammaTable;\n>> -       auto blackLevel = context.activeState.blc.level;\n>> +       const auto blackLevel = context.activeState.blc.level;\n>>         const unsigned int blackIndex = blackLevel * gammaTable.size() / 256;\n>> +       const auto contrast = context.activeState.knobs.contrast.value_or(1.0);\n>>  \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>> +       for (unsigned int i = blackIndex; i < gammaTable.size(); i++) {\n>> +               double normalized = (i - blackIndex) / divisor;\n>> +               /* Apply simple S-curve */\n>> +               if (normalized < 0.5)\n>> +                       normalized = 0.5 * std::pow(normalized / 0.5, contrast);\n>> +               else\n>> +                       normalized = 1.0 - 0.5 * std::pow((1.0 - normalized) / 0.5, contrast);\n>> +               gammaTable[i] = UINT8_MAX *\n>> +                               std::pow(normalized, context.configuration.gamma);\n>> +       }\n>>  \n>>         context.activeState.gamma.blackLevel = blackLevel;\n>> +       context.activeState.gamma.contrast = contrast;\n>>  }\n>>  \n>>  void Lut::prepare(IPAContext &context,\n>> @@ -55,7 +64,8 @@ void Lut::prepare(IPAContext &context,\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>> +       if (context.activeState.gamma.blackLevel != context.activeState.blc.level ||\n>> +           context.activeState.gamma.contrast != context.activeState.knobs.contrast)\n>>                 updateGammaTable(context);\n>>  \n>>         auto &gains = context.activeState.gains;\n>> diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build\n>> index 37a2eb534..d75a7b2a1 100644\n>> --- a/src/ipa/simple/algorithms/meson.build\n>> +++ b/src/ipa/simple/algorithms/meson.build\n>> @@ -4,5 +4,6 @@ soft_simple_ipa_algorithms = files([\n>>      'awb.cpp',\n>>      'agc.cpp',\n>>      'blc.cpp',\n>> +    'contrast.cpp',\n>>      'lut.cpp',\n>>  ])\n>> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n>> index 3519f20f6..5118d6abf 100644\n>> --- a/src/ipa/simple/ipa_context.h\n>> +++ b/src/ipa/simple/ipa_context.h\n>> @@ -8,8 +8,11 @@\n>>  #pragma once\n>>  \n>>  #include <array>\n>> +#include <optional>\n>>  #include <stdint.h>\n>>  \n>> +#include <libcamera/controls.h>\n>> +\n>>  #include <libipa/fc_queue.h>\n>>  \n>>  namespace libcamera {\n>> @@ -44,7 +47,11 @@ struct IPAActiveState {\n>>         struct {\n>>                 std::array<double, kGammaLookupSize> gammaTable;\n>>                 uint8_t blackLevel;\n>> +               double contrast;\n>>         } gamma;\n>> +       struct {\n>> +               std::optional<double> contrast; // 0..inf, 1 = neutral\n>\n> We avoid C++ commments throughout the project.\n\nOK.\n\n> Is the definition of the value for contrast documented by the Control in\n> libcamera ? Or is this specific to the implementation detail here ?\n\nPartially, control_ids_core.yaml says:\n\n  \"Normal contrast is given by the value 1.0; larger values produce\n  images with more contrast.\"\n\nThe bounds are not defined there.\n\n>> +       } knobs;\n>>  };\n>>  \n>>  struct IPAFrameContext : public FrameContext {\n>> -- \n>> 2.44.1\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 01709C32E0\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 10 Oct 2024 08:58:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 881946536C;\n\tThu, 10 Oct 2024 10:58:55 +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 C439163536\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 10 Oct 2024 10:58:53 +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-637-5cBPj1YZNRO83DMLXUXvHw-1; Thu, 10 Oct 2024 04:58:51 -0400","by mail-wm1-f71.google.com with SMTP id\n\t5b1f17b1804b1-42cb479fab2so9185785e9.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 10 Oct 2024 01:58:51 -0700 (PDT)","from nuthatch (nat-pool-brq-t.redhat.com. [213.175.37.10])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-431182d793fsm9570185e9.9.2024.10.10.01.58.48\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tThu, 10 Oct 2024 01:58:48 -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=\"Q9VH3K2Z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1728550732;\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=LrNcaAPGnS+RAQG2OdQM/o/oEX5AIUwwXwR4+TrkvIo=;\n\tb=Q9VH3K2ZChuvEA39O9cjquMeilqDPx10RzFTfMfIKzPHmA5XV/mWNWjizCkK6hD75SIZAx\n\tc7LgcqSHiP9BsmK6k+XbozCvmBBge5UZ0Ss2QIjBKidlimd3yTnI2RmE1Y8COH5fGY3o8h\n\tVNvM00iM535nEUZm5vRXxWapwjsoNGc=","X-MC-Unique":"5cBPj1YZNRO83DMLXUXvHw-1","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1728550730; x=1729155530;\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=LrNcaAPGnS+RAQG2OdQM/o/oEX5AIUwwXwR4+TrkvIo=;\n\tb=st7zuGemcpJYdIrLqqHqyQJYg5ktYkHVxO1kw6bOGoC7Xl3e5zKV/edczG+kgJ8IWi\n\tLJsrP+DSsj1LUtqQy3j1H1czF7rlDAwrlvCVRnERLzQjsjIz8nOExrZtZNDx8O2luhc+\n\tHQkoRpP8RO56uUSZvxRV6dQwE+YZUPFkWQ4GSkk5js2YXV5Q7CLfkWDqW8UxRV6Bmsum\n\tnpbaR2tR5dHgNXfAkA8XAC6neuirZqgFmkEOkyr5b5pGh1utUco0TAncR5I1GWgYDSBs\n\thWiMH1ZkjdqjMI1NaMz0r/GLHJ3GpH5PYaMGnCvoqQiiVlss7jcDqP8LvgrER8tYc/Ge\n\thhZQ==","X-Gm-Message-State":"AOJu0Yz1yAa9DwNkeiUVx8sns+LuXTanAZA9MrLWlVce3dClOXHk3UBd\n\tov5+Ikvnd0Ncd54hXcvA8yHo87WKLF0+52UvOaFDqJ1r/sUJMTbELego9AFQLbnj8Kw9BCxAgzC\n\tZrBWCXWxSEjCVLN24b8sq63A65tXHXXv1J/yeMn+xuGy2uggvL1T/OzAnsWGOHZg0rz8oz+j6Iz\n\tTksZxnm8joUK4BLRDkL8K35N02ZqGZgEEYgccz6ZKBLeUQPjwaA+GWadU=","X-Received":["by 2002:a05:600c:4394:b0:431:15c5:6439 with SMTP id\n\t5b1f17b1804b1-43115c566b4mr18616945e9.13.1728550729909; \n\tThu, 10 Oct 2024 01:58:49 -0700 (PDT)","by 2002:a05:600c:4394:b0:431:15c5:6439 with SMTP id\n\t5b1f17b1804b1-43115c566b4mr18616785e9.13.1728550729424; \n\tThu, 10 Oct 2024 01:58:49 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IE4gItswq2lBa0+3P3sEG4P4A7M7OcTJ12cI6aujLB7rX1B+p4Ia+EP027Lx0n9ZGO8kCeXBg==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 1/2] libcamera: software_isp: Add contrast algorithm","In-Reply-To":"<172851212524.532453.15536968784495669501@ping.linuxembedded.co.uk>\n\t(Kieran Bingham's message of \"Wed, 09 Oct 2024 23:15:25 +0100\")","References":"<20241009193318.2394762-1-mzamazal@redhat.com>\n\t<20241009193318.2394762-2-mzamazal@redhat.com>\n\t<172851212524.532453.15536968784495669501@ping.linuxembedded.co.uk>","Date":"Thu, 10 Oct 2024 10:58:47 +0200","Message-ID":"<87ed4oqqpk.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>"}}]