[{"id":32557,"web_url":"https://patchwork.libcamera.org/comment/32557/","msgid":"<173344457883.1906024.11909089515986110724@ping.linuxembedded.co.uk>","date":"2024-12-06T00:22:58","subject":"Re: [PATCH v5 3/4] libcamera: software_isp: Add support for contrast\n\tcontrol","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Milan Zamazal (2024-11-28 12:52:24)\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 support for contrast control, which can improve\n> e.g. a flat looking image. Based on the provided contrast value, it\n> applies a simple S-curve modification to the image.\n> \n> The contrast algorithm just handles the provided values, while the\n> S-curve is applied in the gamma algorithm on the computed gamma curve\n> whenever the contrast value changes. Since the algorithm is applied\n> only on the lookup table already present, its overhead is negligible.\n> \n> The contrast value range is 0..2 and corresponds to the whole range from\n> a completely flat contrast to an infinite contrast, 1.0 being the normal\n> value. This makes the user visible range intuitive and easy to use in\n> GUI sliders, while complying with Contrast control definition. There is\n> no unified range in the hardware pipelines, for example rkisp1 uses\n> 0..1.993 range while rpi uses 0..10 range.\n> \n> This is a preparation patch without actually providing the control\n> itself, which is done in the following patch.\n> \n> Signed-off-by: Milan Zamazal \n\nWell, I've just tested this - and it does indeed affect the image, and\nrespond correctly to a slider control in camshark ... so that's progress\nfor the soft-isp from my perspective, and I know more will be built on\ntop of this:\n\nTested-by: Kieran Bingham \nReviewed-by: Kieran Bingham \n\n> ---\n> src/ipa/simple/algorithms/lut.cpp | 40 +++++++++++++++++++++++++++----\n> src/ipa/simple/algorithms/lut.h | 5 ++++\n> src/ipa/simple/ipa_context.h | 7 ++++++\n> 3 files changed, 47 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/ipa/simple/algorithms/lut.cpp b/src/ipa/simple/algorithms/lut.cpp\n> index 9744e773..dd76e117 100644\n> --- a/src/ipa/simple/algorithms/lut.cpp\n> +++ b/src/ipa/simple/algorithms/lut.cpp\n> @@ -9,14 +9,19 @@\n> \n> #include \n> #include \n> +#include \n> #include \n> \n> #include \n> \n> #include \"simple/ipa_context.h\"\n> \n> +#include \"control_ids.h\"\n> +\n> namespace libcamera {\n> \n> +LOG_DEFINE_CATEGORY(IPASoftLut)\n> +\n> namespace ipa::soft::algorithms {\n> \n> int Lut::configure(IPAContext &context,\n> @@ -24,24 +29,48 @@ int Lut::configure(IPAContext &context,\n> {\n> /* Gamma value is fixed */\n> context.configuration.gamma = 0.5;\n> + context.activeState.knobs.contrast = std::optional();\n> updateGammaTable(context);\n> \n> return 0;\n> }\n> \n> +void Lut::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(IPASoftLut, Debug) << \"Setting contrast to \" << contrast.value();\n> + }\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 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> + /* Convert 0..2 to 0..infinity; avoid actual inifinity at tan(pi/2) */\n> + double contrastExp = tan(std::clamp(contrast * M_PI_4, 0.0, M_PI_2 - 0.00001));\n> + /* Apply simple S-curve */\n> + if (normalized < 0.5)\n> + normalized = 0.5 * std::pow(normalized / 0.5, contrastExp);\n> + else\n> + normalized = 1.0 - 0.5 * std::pow((1.0 - normalized) / 0.5, contrastExp);\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 +84,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/lut.h b/src/ipa/simple/algorithms/lut.h\n> index b635987d..ef2df147 100644\n> --- a/src/ipa/simple/algorithms/lut.h\n> +++ b/src/ipa/simple/algorithms/lut.h\n> @@ -20,6 +20,11 @@ public:\n> ~Lut() = default;\n> \n> int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> + void queueRequest(typename Module::Context &context,\n> + const uint32_t frame,\n> + typename Module::FrameContext &frameContext,\n> + const ControlList &controls)\n> + override;\n> void prepare(IPAContext &context,\n> const uint32_t frame,\n> IPAFrameContext &frameContext,\n> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> index fd7343e9..0c2f7021 100644\n> --- a/src/ipa/simple/ipa_context.h\n> +++ b/src/ipa/simple/ipa_context.h\n> @@ -11,6 +11,8 @@\n> #include \n> #include \n> \n> +#include \n> +\n> #include \n> \n> namespace libcamera {\n> @@ -48,7 +50,12 @@ struct IPAActiveState {\n> struct {\n> std::array gammaTable;\n> uint8_t blackLevel;\n> + double contrast;\n> } gamma;\n> + struct {\n> + /* 0..2 range, 1.0 = normal */\n> + std::optional contrast;\n> + } knobs;\n> };\n> \n> struct IPAFrameContext : public FrameContext {\n> -- \n> 2.44.2\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 E17C3BF415\n\tfor ;\n\tFri, 6 Dec 2024 00:23:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EC5456611C;\n\tFri, 6 Dec 2024 01:23:04 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 16C64660C8\n\tfor ;\n\tFri, 6 Dec 2024 01:23:02 +0100 (CET)","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 CB5DB16A;\n\tFri, 6 Dec 2024 01:22:32 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"EtuVwWuu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1733444552;\n\tbh=0GvGL1LTN3u50iF/dDpPTQkkU8EKHAhGhZcSyXGHvpQ=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=EtuVwWuu4EerTrJj8Aa4o+mpxLb7GQjc0224S+4EcYgVEzmGJQ+R/uG3sVYMEOgkG\n\toNMWNgSWq2PPaf9j0BrxhKbWlXUQuZfjll/mHrMndj+oySlAlFH3w1BiOVzw70RVvI\n\tnvTJqxxtTvPmrWAZcKrb0H0u9FEKB1XSaxjWDxyU=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241128125226.683249-4-mzamazal@redhat.com>","References":"<20241128125226.683249-1-mzamazal@redhat.com>\n\t<20241128125226.683249-4-mzamazal@redhat.com>","Subject":"Re: [PATCH v5 3/4] libcamera: software_isp: Add support for contrast\n\tcontrol","From":"Kieran Bingham ","Cc":"Milan Zamazal ,\n\tLaurent Pinchart ,\n\tStefan Klug ","To":"Milan Zamazal , libcamera-devel@lists.libcamera.org","Date":"Fri, 06 Dec 2024 00:22:58 +0000","Message-ID":"<173344457883.1906024.11909089515986110724@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":32558,"web_url":"https://patchwork.libcamera.org/comment/32558/","msgid":"<173344476774.1906024.614840800384475020@ping.linuxembedded.co.uk>","date":"2024-12-06T00:26:07","subject":"Re: [PATCH v5 3/4] libcamera: software_isp: Add support for contrast\n\tcontrol","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Kieran Bingham (2024-12-06 00:22:58)\n> Quoting Milan Zamazal (2024-11-28 12:52:24)\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 support for contrast control, which can improve\n> > e.g. a flat looking image. Based on the provided contrast value, it\n> > applies a simple S-curve modification to the image.\n> > \n> > The contrast algorithm just handles the provided values, while the\n> > S-curve is applied in the gamma algorithm on the computed gamma curve\n> > whenever the contrast value changes. Since the algorithm is applied\n> > only on the lookup table already present, its overhead is negligible.\n> > \n> > The contrast value range is 0..2 and corresponds to the whole range from\n> > a completely flat contrast to an infinite contrast, 1.0 being the normal\n> > value. This makes the user visible range intuitive and easy to use in\n> > GUI sliders, while complying with Contrast control definition. There is\n> > no unified range in the hardware pipelines, for example rkisp1 uses\n> > 0..1.993 range while rpi uses 0..10 range.\n> > \n> > This is a preparation patch without actually providing the control\n> > itself, which is done in the following patch.\n> > \n> > Signed-off-by: Milan Zamazal \n> \n> Well, I've just tested this - and it does indeed affect the image, and\n> respond correctly to a slider control in camshark ... so that's progress\n> for the soft-isp from my perspective, and I know more will be built on\n> top of this:\n> \n> Tested-by: Kieran Bingham \n> Reviewed-by: Kieran Bingham \n> \n> > ---\n> > src/ipa/simple/algorithms/lut.cpp | 40 +++++++++++++++++++++++++++----\n> > src/ipa/simple/algorithms/lut.h | 5 ++++\n> > src/ipa/simple/ipa_context.h | 7 ++++++\n> > 3 files changed, 47 insertions(+), 5 deletions(-)\n> > \n> > diff --git a/src/ipa/simple/algorithms/lut.cpp b/src/ipa/simple/algorithms/lut.cpp\n> > index 9744e773..dd76e117 100644\n> > --- a/src/ipa/simple/algorithms/lut.cpp\n> > +++ b/src/ipa/simple/algorithms/lut.cpp\n> > @@ -9,14 +9,19 @@\n> > \n> > #include \n> > #include \n> > +#include \n> > #include \n> > \n> > #include \n> > \n> > #include \"simple/ipa_context.h\"\n> > \n> > +#include \"control_ids.h\"\n> > +\n> > namespace libcamera {\n> > \n> > +LOG_DEFINE_CATEGORY(IPASoftLut)\n> > +\n> > namespace ipa::soft::algorithms {\n> > \n> > int Lut::configure(IPAContext &context,\n> > @@ -24,24 +29,48 @@ int Lut::configure(IPAContext &context,\n> > {\n> > /* Gamma value is fixed */\n> > context.configuration.gamma = 0.5;\n> > + context.activeState.knobs.contrast = std::optional();\n> > updateGammaTable(context);\n> > \n> > return 0;\n> > }\n> > \n> > +void Lut::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(IPASoftLut, Debug) << \"Setting contrast to \" << contrast.value();\n> > + }\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 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> > + /* Convert 0..2 to 0..infinity; avoid actual inifinity at tan(pi/2) */\n> > + double contrastExp = tan(std::clamp(contrast * M_PI_4, 0.0, M_PI_2 - 0.00001));\n> > + /* Apply simple S-curve */\n> > + if (normalized < 0.5)\n> > + normalized = 0.5 * std::pow(normalized / 0.5, contrastExp);\n> > + else\n> > + normalized = 1.0 - 0.5 * std::pow((1.0 - normalized) / 0.5, contrastExp);\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\nIt would be nice if both the black level and the contrast were reported\nin the metadata of the completed frame.\n\nI guess that can be a patch on top as more work is coming on top of\nthis, but any control that affects the image should be reporting what\nvalue was applied when that image completes in the request metadata.\n\n--\nKieran\n\n\n> > }\n> > \n> > void Lut::prepare(IPAContext &context,\n> > @@ -55,7 +84,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/lut.h b/src/ipa/simple/algorithms/lut.h\n> > index b635987d..ef2df147 100644\n> > --- a/src/ipa/simple/algorithms/lut.h\n> > +++ b/src/ipa/simple/algorithms/lut.h\n> > @@ -20,6 +20,11 @@ public:\n> > ~Lut() = default;\n> > \n> > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> > + void queueRequest(typename Module::Context &context,\n> > + const uint32_t frame,\n> > + typename Module::FrameContext &frameContext,\n> > + const ControlList &controls)\n> > + override;\n> > void prepare(IPAContext &context,\n> > const uint32_t frame,\n> > IPAFrameContext &frameContext,\n> > diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> > index fd7343e9..0c2f7021 100644\n> > --- a/src/ipa/simple/ipa_context.h\n> > +++ b/src/ipa/simple/ipa_context.h\n> > @@ -11,6 +11,8 @@\n> > #include \n> > #include \n> > \n> > +#include \n> > +\n> > #include \n> > \n> > namespace libcamera {\n> > @@ -48,7 +50,12 @@ struct IPAActiveState {\n> > struct {\n> > std::array gammaTable;\n> > uint8_t blackLevel;\n> > + double contrast;\n> > } gamma;\n> > + struct {\n> > + /* 0..2 range, 1.0 = normal */\n> > + std::optional contrast;\n> > + } knobs;\n> > };\n> > \n> > struct IPAFrameContext : public FrameContext {\n> > -- \n> > 2.44.2\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 33048BF415\n\tfor ;\n\tFri, 6 Dec 2024 00:26:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0711A6611C;\n\tFri, 6 Dec 2024 01:26:12 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 97509660C8\n\tfor ;\n\tFri, 6 Dec 2024 01:26:10 +0100 (CET)","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 5F6FC16A;\n\tFri, 6 Dec 2024 01:25:41 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"lFUTyygN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1733444741;\n\tbh=UiyjuYDMc9PfKpqeBDgky9ANR1xUdNDm4jnYxLqdCj4=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=lFUTyygNgGr72V1FdjFIBmlxnM+wMGV3rEtcKokUnNxotEOC9Xx9rSOh+clM7cmCm\n\tSITMa4z7yT4M2tpBR3qvxmAkr9cRMUvP0lmtjigicc4Mia7VAtbHUS0wMok9JsTwNR\n\tQkPn0HkbpO1n72+MH94eWWnbzgwrfWVqRcHmC9O0=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<173344457883.1906024.11909089515986110724@ping.linuxembedded.co.uk>","References":"<20241128125226.683249-1-mzamazal@redhat.com>\n\t<20241128125226.683249-4-mzamazal@redhat.com>\n\t<173344457883.1906024.11909089515986110724@ping.linuxembedded.co.uk>","Subject":"Re: [PATCH v5 3/4] libcamera: software_isp: Add support for contrast\n\tcontrol","From":"Kieran Bingham ","Cc":"Milan Zamazal ,\n\tLaurent Pinchart ,\n\tStefan Klug ","To":"Milan Zamazal , libcamera-devel@lists.libcamera.org","Date":"Fri, 06 Dec 2024 00:26:07 +0000","Message-ID":"<173344476774.1906024.614840800384475020@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":32566,"web_url":"https://patchwork.libcamera.org/comment/32566/","msgid":"<87h67hxctu.fsf@redhat.com>","date":"2024-12-06T11:36:45","subject":"Re: [PATCH v5 3/4] libcamera: software_isp: Add support for\n\tcontrast control","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 and testing.\n\nKieran Bingham writes:\n\n> Quoting Kieran Bingham (2024-12-06 00:22:58)\n>> Quoting Milan Zamazal (2024-11-28 12:52:24)\n>> > Software ISP is currently fully automatic and doesn't allow image\n>\n>> > modifications by explicitly set control values. The user has no means\n>> > to make the image looking better.\n>> > \n>> > This patch introduces support for contrast control, which can improve\n>> > e.g. a flat looking image. Based on the provided contrast value, it\n>> > applies a simple S-curve modification to the image.\n>> > \n>> > The contrast algorithm just handles the provided values, while the\n>> > S-curve is applied in the gamma algorithm on the computed gamma curve\n>> > whenever the contrast value changes. Since the algorithm is applied\n>> > only on the lookup table already present, its overhead is negligible.\n>> > \n>> > The contrast value range is 0..2 and corresponds to the whole range from\n>> > a completely flat contrast to an infinite contrast, 1.0 being the normal\n>> > value. This makes the user visible range intuitive and easy to use in\n>> > GUI sliders, while complying with Contrast control definition. There is\n>> > no unified range in the hardware pipelines, for example rkisp1 uses\n>> > 0..1.993 range while rpi uses 0..10 range.\n>> > \n>> > This is a preparation patch without actually providing the control\n>> > itself, which is done in the following patch.\n>> > \n>> > Signed-off-by: Milan Zamazal \n>> \n>> Well, I've just tested this - and it does indeed affect the image, and\n>> respond correctly to a slider control in camshark ... so that's progress\n>> for the soft-isp from my perspective, and I know more will be built on\n>> top of this:\n>> \n>> Tested-by: Kieran Bingham \n>> Reviewed-by: Kieran Bingham \n>> \n>> > ---\n>> > src/ipa/simple/algorithms/lut.cpp | 40 +++++++++++++++++++++++++++----\n>> > src/ipa/simple/algorithms/lut.h | 5 ++++\n>> > src/ipa/simple/ipa_context.h | 7 ++++++\n>> > 3 files changed, 47 insertions(+), 5 deletions(-)\n>> > \n>> > diff --git a/src/ipa/simple/algorithms/lut.cpp b/src/ipa/simple/algorithms/lut.cpp\n>> > index 9744e773..dd76e117 100644\n>> > --- a/src/ipa/simple/algorithms/lut.cpp\n>> > +++ b/src/ipa/simple/algorithms/lut.cpp\n>> > @@ -9,14 +9,19 @@\n>> > \n>> > #include \n>> > #include \n>> > +#include \n>> > #include \n>> > \n>> > #include \n>> > \n>> > #include \"simple/ipa_context.h\"\n>> > \n>> > +#include \"control_ids.h\"\n>> > +\n>> > namespace libcamera {\n>> > \n>> > +LOG_DEFINE_CATEGORY(IPASoftLut)\n>> > +\n>> > namespace ipa::soft::algorithms {\n>> > \n>> > int Lut::configure(IPAContext &context,\n>> > @@ -24,24 +29,48 @@ int Lut::configure(IPAContext &context,\n>> > {\n>> > /* Gamma value is fixed */\n>> > context.configuration.gamma = 0.5;\n>> > + context.activeState.knobs.contrast = std::optional();\n>> > updateGammaTable(context);\n>> > \n>> > return 0;\n>> > }\n>> > \n>> > +void Lut::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(IPASoftLut, Debug) << \"Setting contrast to \" << contrast.value();\n>> > + }\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 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>> > + /* Convert 0..2 to 0..infinity; avoid actual inifinity at tan(pi/2) */\n>> > + double contrastExp = tan(std::clamp(contrast * M_PI_4, 0.0, M_PI_2 - 0.00001));\n>> > + /* Apply simple S-curve */\n>> > + if (normalized < 0.5)\n>> > + normalized = 0.5 * std::pow(normalized / 0.5, contrastExp);\n>> > + else\n>> > + normalized = 1.0 - 0.5 * std::pow((1.0 - normalized) / 0.5, contrastExp);\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> It would be nice if both the black level and the contrast were reported\n> in the metadata of the completed frame.\n\nYes.\n\n> I guess that can be a patch on top as more work is coming on top of\n> this, but any control that affects the image should be reporting what\n> value was applied when that image completes in the request metadata.\n\nThe primary problem is that we don't report metadata at all from\nsoftware ISP. We just can produce it and other possible metadata items\nare also missing I think. It'd be probably best to fix it all at once,\non top of this series (but separately).\n\nThere are your patches from June in this direction, would you like to\nresume work on them or should I take them over?\n\n> --\n> Kieran\n>\n>\n>> > }\n>> > \n>> > void Lut::prepare(IPAContext &context,\n>> > @@ -55,7 +84,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/lut.h b/src/ipa/simple/algorithms/lut.h\n>> > index b635987d..ef2df147 100644\n>> > --- a/src/ipa/simple/algorithms/lut.h\n>> > +++ b/src/ipa/simple/algorithms/lut.h\n>> > @@ -20,6 +20,11 @@ public:\n>> > ~Lut() = default;\n>> > \n>> > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n>> > + void queueRequest(typename Module::Context &context,\n>> > + const uint32_t frame,\n>> > + typename Module::FrameContext &frameContext,\n>> > + const ControlList &controls)\n>> > + override;\n>> > void prepare(IPAContext &context,\n>> > const uint32_t frame,\n>> > IPAFrameContext &frameContext,\n>> > diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n>> > index fd7343e9..0c2f7021 100644\n>> > --- a/src/ipa/simple/ipa_context.h\n>> > +++ b/src/ipa/simple/ipa_context.h\n>> > @@ -11,6 +11,8 @@\n>> > #include \n>> > #include \n>> > \n>> > +#include \n>> > +\n>> > #include \n>> > \n>> > namespace libcamera {\n>> > @@ -48,7 +50,12 @@ struct IPAActiveState {\n>> > struct {\n>> > std::array gammaTable;\n>> > uint8_t blackLevel;\n>> > + double contrast;\n>> > } gamma;\n>> > + struct {\n>> > + /* 0..2 range, 1.0 = normal */\n>> > + std::optional contrast;\n>> > + } knobs;\n>> > };\n>> > \n>> > struct IPAFrameContext : public FrameContext {\n>> > -- \n>> > 2.44.2\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 0DED0BF415\n\tfor ;\n\tFri, 6 Dec 2024 11:36:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 18F976612A;\n\tFri, 6 Dec 2024 12:36:53 +0100 (CET)","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 0315B618B3\n\tfor ;\n\tFri, 6 Dec 2024 12:36:51 +0100 (CET)","from mail-wm1-f70.google.com (mail-wm1-f70.google.com\n\t[209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-290-T4w7ol15NOC4SCc4ebtQiA-1; Fri, 06 Dec 2024 06:36:49 -0500","by mail-wm1-f70.google.com with SMTP id\n\t5b1f17b1804b1-434a90febb8so11927555e9.1\n\tfor ;\n\tFri, 06 Dec 2024 03:36:49 -0800 (PST)","from nuthatch (ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-434d5280fc4sm91199425e9.24.2024.12.06.03.36.45\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 06 Dec 2024 03:36:46 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"e72B9JQW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1733485010;\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=sRzz8vimoJGCu1+Y+v6WfxEFjqkL6u55fN1/c+xmVkE=;\n\tb=e72B9JQWOpLBHwxo1ia7jvPwPkm9p0jaZts0Rr7B4iYygiMT31VPljTaBJprVrjCqbutfk\n\tyzYK3dioMcZyVX3c1S/Ka6RnOFDxbeNMFm14cnxzXinOpcgs0CcId317TD5ZdE4B09vWkN\n\tlvbup5Mkloa+H19DDfFg4WzlooEvjUE=","X-MC-Unique":"T4w7ol15NOC4SCc4ebtQiA-1","X-Mimecast-MFC-AGG-ID":"T4w7ol15NOC4SCc4ebtQiA","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1733485008; x=1734089808;\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=sRzz8vimoJGCu1+Y+v6WfxEFjqkL6u55fN1/c+xmVkE=;\n\tb=oG7YK+wgK9kakadN0lMM2z//r/tJccSfLzGks5wl1qu/LQ2WenkHqAu0oYoiWmJxHs\n\t0eCAK2dqwVoGHuHb+84SUKpAjsyCcIi2fp9vzw4XYmUpfjyv8OkURitkRtrUBC3QFRRz\n\tfGlPg3+5MtJx9qh+m4p4h6T5XIejylvO9YJ8qaZQrCbch1wPmnk+fHcTUWswTSFPRq14\n\tjDmH0EMRYCLa9+j/wWD1dqYaTkcuksI9qgoy2DT9xiFJJmetx3NGFtxTK4pfx6v8Jned\n\tth49gjhBcMI9edLjR0hv+a2WcScN1+hB379R7oCQwLtWwmtGcqiY1Y96ljTiP9ski1rG\n\t2VDA==","X-Gm-Message-State":"AOJu0Yz04vwzHfRpmmmHy2RnwVAtLS6ZuJBtV4+Amr+kQI+r4vDIDl6+\n\tyqsxyc0SPo9pEIVCjQZbltWJhyfALAau+Ju70Ju6frkeZHOqiUPMs92eAdvsfXCUb9pjh5F2GfU\n\t4TDnJmvru/2gUrAkwfna1cb9DJiR3bO0GQj147YPFRpzqJYwzc5l1T4aV07G+3rvubWzRnrDCgX\n\tMvldo=","X-Gm-Gg":"ASbGnctLVn+rndU7+mNyUs+QnwcFaJ+nD8uxK6gjad7TqvxZVicqG0V5Eq7AWglzcLC\n\tMhCh9YEPfBTRr8hvHh2tNicGjEOaaHJWxcZKAO2C66r/iCbiJKIvbu5xcogj0rreX/gi7AV5dgk\n\te9Q0Ucop/KFDPSkRtDdrih9LaBm+gJEowVrjXHV6GrYgE7HYaEParzF3b+RPiM2iXXG1qPCObhK\n\t9BzoKZMKQklh9dPPtSJQ/SC1rmjFGPFUOKPpdntfa7FAszb+NtRtCCO0T1eYGJe52Sv7js=","X-Received":["by 2002:a05:600c:1f96:b0:431:55af:a230 with SMTP id\n\t5b1f17b1804b1-434ddee050emr19436735e9.33.1733485007997; \n\tFri, 06 Dec 2024 03:36:47 -0800 (PST)","by 2002:a05:600c:1f96:b0:431:55af:a230 with SMTP id\n\t5b1f17b1804b1-434ddee050emr19436555e9.33.1733485007601; \n\tFri, 06 Dec 2024 03:36:47 -0800 (PST)"],"X-Google-Smtp-Source":"AGHT+IF8SB5JduUYfnVq0NPpxQrD/RsDETayXBL5SVzlu3GKvzzCV/pvb1NoBEW3/khto6Hkx0gsFQ==","From":"Milan Zamazal ","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org, Laurent Pinchart\n\t, Stefan Klug\n\t","Subject":"Re: [PATCH v5 3/4] libcamera: software_isp: Add support for\n\tcontrast control","In-Reply-To":"<173344476774.1906024.614840800384475020@ping.linuxembedded.co.uk>\n\t(Kieran Bingham's message of \"Fri, 06 Dec 2024 00:26:07 +0000\")","References":"<20241128125226.683249-1-mzamazal@redhat.com>\n\t<20241128125226.683249-4-mzamazal@redhat.com>\n\t<173344457883.1906024.11909089515986110724@ping.linuxembedded.co.uk>\n\t<173344476774.1906024.614840800384475020@ping.linuxembedded.co.uk>","Date":"Fri, 06 Dec 2024 12:36:45 +0100","Message-ID":"<87h67hxctu.fsf@redhat.com>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"8TGwM-Sd1GLb8QxU9yPf8dcK_esPLmcFZufXHwLw9ww_1733485008","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\" "}},{"id":32571,"web_url":"https://patchwork.libcamera.org/comment/32571/","msgid":"<173348831787.4084589.13714679652094169735@ping.linuxembedded.co.uk>","date":"2024-12-06T12:31:57","subject":"Re: [PATCH v5 3/4] libcamera: software_isp: Add support for contrast\n\tcontrol","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Milan,\n\nQuoting Milan Zamazal (2024-12-06 11:36:45)\n> Hi Kieran,\n> \n> thank you for review and testing.\n> \n> Kieran Bingham writes:\n> \n> > Quoting Kieran Bingham (2024-12-06 00:22:58)\n> >> Quoting Milan Zamazal (2024-11-28 12:52:24)\n> >> > Software ISP is currently fully automatic and doesn't allow image\n> >\n> >> > modifications by explicitly set control values. The user has no means\n> >> > to make the image looking better.\n> >> > \n> >> > This patch introduces support for contrast control, which can improve\n> >> > e.g. a flat looking image. Based on the provided contrast value, it\n> >> > applies a simple S-curve modification to the image.\n> >> > \n> >> > The contrast algorithm just handles the provided values, while the\n> >> > S-curve is applied in the gamma algorithm on the computed gamma curve\n> >> > whenever the contrast value changes. Since the algorithm is applied\n> >> > only on the lookup table already present, its overhead is negligible.\n> >> > \n> >> > The contrast value range is 0..2 and corresponds to the whole range from\n> >> > a completely flat contrast to an infinite contrast, 1.0 being the normal\n> >> > value. This makes the user visible range intuitive and easy to use in\n> >> > GUI sliders, while complying with Contrast control definition. There is\n> >> > no unified range in the hardware pipelines, for example rkisp1 uses\n> >> > 0..1.993 range while rpi uses 0..10 range.\n> >> > \n> >> > This is a preparation patch without actually providing the control\n> >> > itself, which is done in the following patch.\n> >> > \n> >> > Signed-off-by: Milan Zamazal \n> >> \n> >> Well, I've just tested this - and it does indeed affect the image, and\n> >> respond correctly to a slider control in camshark ... so that's progress\n> >> for the soft-isp from my perspective, and I know more will be built on\n> >> top of this:\n> >> \n> >> Tested-by: Kieran Bingham \n> >> Reviewed-by: Kieran Bingham \n> >> \n> >> > ---\n> >> > src/ipa/simple/algorithms/lut.cpp | 40 +++++++++++++++++++++++++++----\n> >> > src/ipa/simple/algorithms/lut.h | 5 ++++\n> >> > src/ipa/simple/ipa_context.h | 7 ++++++\n> >> > 3 files changed, 47 insertions(+), 5 deletions(-)\n> >> > \n> >> > diff --git a/src/ipa/simple/algorithms/lut.cpp b/src/ipa/simple/algorithms/lut.cpp\n> >> > index 9744e773..dd76e117 100644\n> >> > --- a/src/ipa/simple/algorithms/lut.cpp\n> >> > +++ b/src/ipa/simple/algorithms/lut.cpp\n> >> > @@ -9,14 +9,19 @@\n> >> > \n> >> > #include \n> >> > #include \n> >> > +#include \n> >> > #include \n> >> > \n> >> > #include \n> >> > \n> >> > #include \"simple/ipa_context.h\"\n> >> > \n> >> > +#include \"control_ids.h\"\n> >> > +\n> >> > namespace libcamera {\n> >> > \n> >> > +LOG_DEFINE_CATEGORY(IPASoftLut)\n> >> > +\n> >> > namespace ipa::soft::algorithms {\n> >> > \n> >> > int Lut::configure(IPAContext &context,\n> >> > @@ -24,24 +29,48 @@ int Lut::configure(IPAContext &context,\n> >> > {\n> >> > /* Gamma value is fixed */\n> >> > context.configuration.gamma = 0.5;\n> >> > + context.activeState.knobs.contrast = std::optional();\n> >> > updateGammaTable(context);\n> >> > \n> >> > return 0;\n> >> > }\n> >> > \n> >> > +void Lut::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(IPASoftLut, Debug) << \"Setting contrast to \" << contrast.value();\n> >> > + }\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 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> >> > + /* Convert 0..2 to 0..infinity; avoid actual inifinity at tan(pi/2) */\n> >> > + double contrastExp = tan(std::clamp(contrast * M_PI_4, 0.0, M_PI_2 - 0.00001));\n> >> > + /* Apply simple S-curve */\n> >> > + if (normalized < 0.5)\n> >> > + normalized = 0.5 * std::pow(normalized / 0.5, contrastExp);\n> >> > + else\n> >> > + normalized = 1.0 - 0.5 * std::pow((1.0 - normalized) / 0.5, contrastExp);\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> > It would be nice if both the black level and the contrast were reported\n> > in the metadata of the completed frame.\n> \n> Yes.\n> \n> > I guess that can be a patch on top as more work is coming on top of\n> > this, but any control that affects the image should be reporting what\n> > value was applied when that image completes in the request metadata.\n> \n> The primary problem is that we don't report metadata at all from\n> software ISP. We just can produce it and other possible metadata items\n> are also missing I think. It'd be probably best to fix it all at once,\n> on top of this series (but separately).\n> \n> There are your patches from June in this direction, would you like to\n> resume work on them or should I take them over?\n\nAha yes - there's \n https://patchwork.libcamera.org/project/libcamera/list/?series=4405\n\nIf you're able to take that over I'd be quite happy to see it progress.\nLooking at the first patch, I think Laurent asked for it to be updated\nto report the values as applied based on the FrameContext - which you\nnow have in place, but wasn't available back then.\n\nI don't have a lot of time to look at this so I think you would get\nthere before me!\n\nEqually though - I don't think that blocks this patch here..\n\n> \n> > --\n> > Kieran\n> >\n> >\n> >> > }\n> >> > \n> >> > void Lut::prepare(IPAContext &context,\n> >> > @@ -55,7 +84,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/lut.h b/src/ipa/simple/algorithms/lut.h\n> >> > index b635987d..ef2df147 100644\n> >> > --- a/src/ipa/simple/algorithms/lut.h\n> >> > +++ b/src/ipa/simple/algorithms/lut.h\n> >> > @@ -20,6 +20,11 @@ public:\n> >> > ~Lut() = default;\n> >> > \n> >> > int configure(IPAContext &context, const IPAConfigInfo &configInfo) override;\n> >> > + void queueRequest(typename Module::Context &context,\n> >> > + const uint32_t frame,\n> >> > + typename Module::FrameContext &frameContext,\n> >> > + const ControlList &controls)\n> >> > + override;\n> >> > void prepare(IPAContext &context,\n> >> > const uint32_t frame,\n> >> > IPAFrameContext &frameContext,\n> >> > diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h\n> >> > index fd7343e9..0c2f7021 100644\n> >> > --- a/src/ipa/simple/ipa_context.h\n> >> > +++ b/src/ipa/simple/ipa_context.h\n> >> > @@ -11,6 +11,8 @@\n> >> > #include \n> >> > #include \n> >> > \n> >> > +#include \n> >> > +\n> >> > #include \n> >> > \n> >> > namespace libcamera {\n> >> > @@ -48,7 +50,12 @@ struct IPAActiveState {\n> >> > struct {\n> >> > std::array gammaTable;\n> >> > uint8_t blackLevel;\n> >> > + double contrast;\n> >> > } gamma;\n> >> > + struct {\n> >> > + /* 0..2 range, 1.0 = normal */\n> >> > + std::optional contrast;\n> >> > + } knobs;\n> >> > };\n> >> > \n> >> > struct IPAFrameContext : public FrameContext {\n> >> > -- \n> >> > 2.44.2\n> >> >\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0134FBF415\n\tfor ;\n\tFri, 6 Dec 2024 12:32:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D992166144;\n\tFri, 6 Dec 2024 13:32:01 +0100 (CET)","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 A1503618B3\n\tfor ;\n\tFri, 6 Dec 2024 13:32:00 +0100 (CET)","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 18A4574C;\n\tFri, 6 Dec 2024 13:31:31 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"v1sZvzKQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1733488291;\n\tbh=dHMjzcvMXtDWcw5GEwxYuoHitTsyTibOUS/lPgU+xYc=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=v1sZvzKQjrnszFLzLotSQMHWyhRJrIK6+EU8C5xbF8uETafYNIL0VvkjUitewPGxA\n\txPP/qIX/K7qOTiu/wDPyW94eWnzDZ+r/RGEjtmdxUIsm55sv9Ptiv/cfhUF8NA2jZJ\n\t3N7HK4t23WnROBxbF9EEt93uve+IrYieiTqA/d6s=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<87h67hxctu.fsf@redhat.com>","References":"<20241128125226.683249-1-mzamazal@redhat.com>\n\t<20241128125226.683249-4-mzamazal@redhat.com>\n\t<173344457883.1906024.11909089515986110724@ping.linuxembedded.co.uk>\n\t<173344476774.1906024.614840800384475020@ping.linuxembedded.co.uk>\n\t<87h67hxctu.fsf@redhat.com>","Subject":"Re: [PATCH v5 3/4] libcamera: software_isp: Add support for contrast\n\tcontrol","From":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org,\n\tLaurent Pinchart ,\n\tStefan Klug ","To":"Milan Zamazal ","Date":"Fri, 06 Dec 2024 12:31:57 +0000","Message-ID":"<173348831787.4084589.13714679652094169735@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\" "}}]