[{"id":33797,"web_url":"https://patchwork.libcamera.org/comment/33797/","msgid":"<174344224169.3394313.5397790304911977441@ping.linuxembedded.co.uk>","date":"2025-03-31T17:30:41","subject":"Re: [PATCH v2 10/17] ipa: rkisp1: ccm/lsc: Fix CCM/LSC based on\n\tmanual color temperature","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Stefan Klug (2025-03-19 16:11:15)\n> In RkISP1Awb::process(), the color temperature in the active state is\n> updated every time new statistics are available.  The CCM/LSC algorithms\n> use that value in prepare() to update the CCM/LSC. This is not correct\n> if the color temperature was specified manually and leads to visible\n> flicker even when AwbEnable is set to false.\n> \n> To fix that, track the auto and manual color temperature separately in\n> active state. In Awb::prepare() the current frame context is updated\n> with the corresponding value from active state. Change the algorithms to\n> fetch the color temperature from the frame context instead of the active\n> state in prepare().\n> \n> Fixes: 02308809548d (\"ipa: rkisp1: awb: Implement ColourTemperature control\")\n> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> \n> ---\n> \n> Changes in v2:\n> - None\n> ---\n>  src/ipa/rkisp1/algorithms/awb.cpp | 18 ++++++++++--------\n>  src/ipa/rkisp1/algorithms/ccm.cpp |  2 +-\n>  src/ipa/rkisp1/algorithms/lsc.cpp |  6 +++---\n>  src/ipa/rkisp1/ipa_context.h      |  2 +-\n>  4 files changed, 15 insertions(+), 13 deletions(-)\n> \n> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> index a9759e53f593..5e067e50cd52 100644\n> --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> @@ -128,7 +128,8 @@ int Awb::configure(IPAContext &context,\n>         context.activeState.awb.automatic.gains =\n>                 awbAlgo_->gainsFromColourTemperature(kDefaultColourTemperature);\n>         context.activeState.awb.autoEnabled = true;\n> -       context.activeState.awb.temperatureK = kDefaultColourTemperature;\n> +       context.activeState.awb.manual.temperatureK = kDefaultColourTemperature;\n> +       context.activeState.awb.automatic.temperatureK = kDefaultColourTemperature;\n>  \n>         /*\n>          * Define the measurement window for AWB as a centered rectangle\n> @@ -185,7 +186,7 @@ void Awb::queueRequest(IPAContext &context,\n>                 const auto &gains = awbAlgo_->gainsFromColourTemperature(*colourTemperature);\n>                 awb.manual.gains.r() = gains.r();\n>                 awb.manual.gains.b() = gains.b();\n> -               awb.temperatureK = *colourTemperature;\n> +               awb.manual.temperatureK = *colourTemperature;\n>                 update = true;\n>         }\n>  \n> @@ -194,7 +195,7 @@ void Awb::queueRequest(IPAContext &context,\n>                         << \"Set colour gains to \" << awb.manual.gains;\n>  \n>         frameContext.awb.gains = awb.manual.gains;\n> -       frameContext.awb.temperatureK = awb.temperatureK;\n> +       frameContext.awb.temperatureK = awb.manual.temperatureK;\n>  }\n>  \n>  /**\n> @@ -208,8 +209,9 @@ void Awb::prepare(IPAContext &context, const uint32_t frame,\n>          * most up-to-date automatic values we can read.\n>          */\n>         if (frameContext.awb.autoEnabled) {\n> -               frameContext.awb.gains = context.activeState.awb.automatic.gains;\n> -               frameContext.awb.temperatureK = context.activeState.awb.temperatureK;\n> +               auto &awb = context.activeState.awb;\n> +               frameContext.awb.gains = awb.automatic.gains;\n> +               frameContext.awb.temperatureK = awb.automatic.temperatureK;\n>         }\n>  \n>         auto gainConfig = params->block<BlockType::AwbGain>();\n> @@ -309,10 +311,10 @@ void Awb::process(IPAContext &context,\n>         RkISP1AwbStats awbStats{ rgbMeans };\n>         AwbResult awbResult = awbAlgo_->calculateAwb(awbStats, frameContext.lux.lux);\n>  \n> -       activeState.awb.temperatureK = awbResult.colourTemperature;\n> +       activeState.awb.automatic.temperatureK = awbResult.colourTemperature;\n>  \n>         /* Metadata shall contain the up to date measurement */\n> -       metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);\n> +       metadata.set(controls::ColourTemperature, activeState.awb.automatic.temperatureK);\n\nI think we discussed different options for this behaviour.\n\nDon't we expect to report the manually set colour temperature if that's\nwhat was set on that framecontext ? (So that the colour gains match the\ncolour temperature or such ?)\n\n\nThough I wish there was a way to report both so we can still what the\nautoregulation 'would think' even if it's set to manual...\n  \n>         /*\n>          * Clamp the gain values to the hardware, which expresses gains as Q2.8\n> @@ -333,7 +335,7 @@ void Awb::process(IPAContext &context,\n>                 << std::showpoint\n>                 << \"Means \" << rgbMeans << \", gains \"\n>                 << activeState.awb.automatic.gains << \", temp \"\n> -               << activeState.awb.temperatureK << \"K\";\n> +               << activeState.awb.automatic.temperatureK << \"K\";\n>  }\n>  \n>  RGB<double> Awb::calculateRgbMeans(const IPAFrameContext &frameContext, const rkisp1_cif_isp_awb_stat *awb) const\n> diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp\n> index eb8ca39e56a8..2e5e91006b55 100644\n> --- a/src/ipa/rkisp1/algorithms/ccm.cpp\n> +++ b/src/ipa/rkisp1/algorithms/ccm.cpp\n> @@ -88,7 +88,7 @@ void Ccm::setParameters(struct rkisp1_cif_isp_ctk_config &config,\n>  void Ccm::prepare(IPAContext &context, const uint32_t frame,\n>                   IPAFrameContext &frameContext, RkISP1Params *params)\n>  {\n> -       uint32_t ct = context.activeState.awb.temperatureK;\n> +       uint32_t ct = frameContext.awb.temperatureK;\n\n\nHrm. ... we need to make sure Awb algo always runs before Ccm in this\ninstance.\n\nI think we're \"in luck\" because of alphabetical sorting, but I think\nthis indicates we will need to indicate some sort of dependencies\nbetween algos...\n\n\n\n>  \n>         /*\n>          * \\todo The colour temperature will likely be noisy, add filtering to\n> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> index e47aa2f0727e..e7301bfec863 100644\n> --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> @@ -404,12 +404,12 @@ void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config,\n>  /**\n>   * \\copydoc libcamera::ipa::Algorithm::prepare\n>   */\n> -void LensShadingCorrection::prepare(IPAContext &context,\n> +void LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context,\n>                                     [[maybe_unused]] const uint32_t frame,\n> -                                   [[maybe_unused]] IPAFrameContext &frameContext,\n> +                                   IPAFrameContext &frameContext,\n>                                     RkISP1Params *params)\n>  {\n> -       uint32_t ct = context.activeState.awb.temperatureK;\n> +       uint32_t ct = frameContext.awb.temperatureK;\n\nIt is a good job Awb begins with 'a'...\n\n>         if (std::abs(static_cast<int>(ct) - static_cast<int>(lastAppliedCt_)) <\n>             kColourTemperatureChangeThreshhold)\n>                 return;\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index 6bc922a82971..769e9f114e23 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -91,12 +91,12 @@ struct IPAActiveState {\n>         struct {\n>                 struct AwbState {\n>                         RGB<double> gains;\n> +                       unsigned int temperatureK;\n>                 };\n>  \n>                 AwbState manual;\n>                 AwbState automatic;\n>  \n> -               unsigned int temperatureK;\n>                 bool autoEnabled;\n>         } awb;\n>  \n> -- \n> 2.43.0\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 4AAE7C323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 31 Mar 2025 17:30:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 72A5368981;\n\tMon, 31 Mar 2025 19:30:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0D26168967\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 31 Mar 2025 19:30:45 +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 437D7703;\n\tMon, 31 Mar 2025 19:28:53 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"tfoJZVtr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743442133;\n\tbh=Tn+oKqRAKdXbKAnFchJXqtt4dgV1QgCCyJ2GMM4ZIJM=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=tfoJZVtrYz0FAGKzLt22I84XNS7fZHo6btLvbL1OqPfSbDeK0JVLHLZtkw85I4CWc\n\tkUSOnQGs3J1Ns6yJa2UusVAuZ7dM4Z53ZNTTmL60nk2ZXzukp8DaBYBkuN4ia6hG5a\n\tUdoXeCTt1sOhizFWnufJeNIit1/Tl0anQMqxB06w=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250319161152.63625-11-stefan.klug@ideasonboard.com>","References":"<20250319161152.63625-1-stefan.klug@ideasonboard.com>\n\t<20250319161152.63625-11-stefan.klug@ideasonboard.com>","Subject":"Re: [PATCH v2 10/17] ipa: rkisp1: ccm/lsc: Fix CCM/LSC based on\n\tmanual color temperature","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 31 Mar 2025 18:30:41 +0100","Message-ID":"<174344224169.3394313.5397790304911977441@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":33863,"web_url":"https://patchwork.libcamera.org/comment/33863/","msgid":"<20250401201609.GA1739@pendragon.ideasonboard.com>","date":"2025-04-01T20:16:09","subject":"Re: [PATCH v2 10/17] ipa: rkisp1: ccm/lsc: Fix CCM/LSC based on\n\tmanual color temperature","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Mar 31, 2025 at 06:30:41PM +0100, Kieran Bingham wrote:\n> Quoting Stefan Klug (2025-03-19 16:11:15)\n> > In RkISP1Awb::process(), the color temperature in the active state is\n> > updated every time new statistics are available.  The CCM/LSC algorithms\n> > use that value in prepare() to update the CCM/LSC. This is not correct\n> > if the color temperature was specified manually and leads to visible\n> > flicker even when AwbEnable is set to false.\n> > \n> > To fix that, track the auto and manual color temperature separately in\n> > active state. In Awb::prepare() the current frame context is updated\n> > with the corresponding value from active state. Change the algorithms to\n> > fetch the color temperature from the frame context instead of the active\n> > state in prepare().\n> > \n> > Fixes: 02308809548d (\"ipa: rkisp1: awb: Implement ColourTemperature control\")\n> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> > \n> > ---\n> > \n> > Changes in v2:\n> > - None\n> > ---\n> >  src/ipa/rkisp1/algorithms/awb.cpp | 18 ++++++++++--------\n> >  src/ipa/rkisp1/algorithms/ccm.cpp |  2 +-\n> >  src/ipa/rkisp1/algorithms/lsc.cpp |  6 +++---\n> >  src/ipa/rkisp1/ipa_context.h      |  2 +-\n> >  4 files changed, 15 insertions(+), 13 deletions(-)\n> > \n> > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> > index a9759e53f593..5e067e50cd52 100644\n> > --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> > @@ -128,7 +128,8 @@ int Awb::configure(IPAContext &context,\n> >         context.activeState.awb.automatic.gains =\n> >                 awbAlgo_->gainsFromColourTemperature(kDefaultColourTemperature);\n> >         context.activeState.awb.autoEnabled = true;\n> > -       context.activeState.awb.temperatureK = kDefaultColourTemperature;\n> > +       context.activeState.awb.manual.temperatureK = kDefaultColourTemperature;\n> > +       context.activeState.awb.automatic.temperatureK = kDefaultColourTemperature;\n> >  \n> >         /*\n> >          * Define the measurement window for AWB as a centered rectangle\n> > @@ -185,7 +186,7 @@ void Awb::queueRequest(IPAContext &context,\n> >                 const auto &gains = awbAlgo_->gainsFromColourTemperature(*colourTemperature);\n> >                 awb.manual.gains.r() = gains.r();\n> >                 awb.manual.gains.b() = gains.b();\n> > -               awb.temperatureK = *colourTemperature;\n> > +               awb.manual.temperatureK = *colourTemperature;\n> >                 update = true;\n> >         }\n> >  \n> > @@ -194,7 +195,7 @@ void Awb::queueRequest(IPAContext &context,\n> >                         << \"Set colour gains to \" << awb.manual.gains;\n> >  \n> >         frameContext.awb.gains = awb.manual.gains;\n> > -       frameContext.awb.temperatureK = awb.temperatureK;\n> > +       frameContext.awb.temperatureK = awb.manual.temperatureK;\n> >  }\n> >  \n> >  /**\n> > @@ -208,8 +209,9 @@ void Awb::prepare(IPAContext &context, const uint32_t frame,\n> >          * most up-to-date automatic values we can read.\n> >          */\n> >         if (frameContext.awb.autoEnabled) {\n> > -               frameContext.awb.gains = context.activeState.awb.automatic.gains;\n> > -               frameContext.awb.temperatureK = context.activeState.awb.temperatureK;\n> > +               auto &awb = context.activeState.awb;\n> > +               frameContext.awb.gains = awb.automatic.gains;\n> > +               frameContext.awb.temperatureK = awb.automatic.temperatureK;\n> >         }\n> >  \n> >         auto gainConfig = params->block<BlockType::AwbGain>();\n> > @@ -309,10 +311,10 @@ void Awb::process(IPAContext &context,\n> >         RkISP1AwbStats awbStats{ rgbMeans };\n> >         AwbResult awbResult = awbAlgo_->calculateAwb(awbStats, frameContext.lux.lux);\n> >  \n> > -       activeState.awb.temperatureK = awbResult.colourTemperature;\n> > +       activeState.awb.automatic.temperatureK = awbResult.colourTemperature;\n> >  \n> >         /* Metadata shall contain the up to date measurement */\n> > -       metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);\n> > +       metadata.set(controls::ColourTemperature, activeState.awb.automatic.temperatureK);\n> \n> I think we discussed different options for this behaviour.\n> \n> Don't we expect to report the manually set colour temperature if that's\n> what was set on that framecontext ? (So that the colour gains match the\n> colour temperature or such ?)\n\nThat's the general rule for libcamera controls. The less we depart from\ngeneral control rules, the better (but there will always be some\nexceptions).\n\n> Though I wish there was a way to report both so we can still what the\n> autoregulation 'would think' even if it's set to manual...\n\nI'm increasingly leaning towards having two controls if we really need\nto expose the measured temperature while in manual mode. We'll have to\ndefine how those controls work in auto mode, and also decide if all\ncameras that support colour temperature need to support measuring in\nmanual mode.\n\n> >         /*\n> >          * Clamp the gain values to the hardware, which expresses gains as Q2.8\n> > @@ -333,7 +335,7 @@ void Awb::process(IPAContext &context,\n> >                 << std::showpoint\n> >                 << \"Means \" << rgbMeans << \", gains \"\n> >                 << activeState.awb.automatic.gains << \", temp \"\n> > -               << activeState.awb.temperatureK << \"K\";\n> > +               << activeState.awb.automatic.temperatureK << \"K\";\n> >  }\n> >  \n> >  RGB<double> Awb::calculateRgbMeans(const IPAFrameContext &frameContext, const rkisp1_cif_isp_awb_stat *awb) const\n> > diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp\n> > index eb8ca39e56a8..2e5e91006b55 100644\n> > --- a/src/ipa/rkisp1/algorithms/ccm.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/ccm.cpp\n> > @@ -88,7 +88,7 @@ void Ccm::setParameters(struct rkisp1_cif_isp_ctk_config &config,\n> >  void Ccm::prepare(IPAContext &context, const uint32_t frame,\n> >                   IPAFrameContext &frameContext, RkISP1Params *params)\n> >  {\n> > -       uint32_t ct = context.activeState.awb.temperatureK;\n> > +       uint32_t ct = frameContext.awb.temperatureK;\n> \n> \n> Hrm. ... we need to make sure Awb algo always runs before Ccm in this\n> instance.\n> \n> I think we're \"in luck\" because of alphabetical sorting, but I think\n> this indicates we will need to indicate some sort of dependencies\n> between algos...\n\nIt's not an alphabetical order matter, at least not alphabetical order\nof the C++ class names. Algorithms are run in the order they are\ninstantiated in the tuning file. I however agree we should be able to\nspecify dependencies, that would be safer.\n\n> >  \n> >         /*\n> >          * \\todo The colour temperature will likely be noisy, add filtering to\n> > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > index e47aa2f0727e..e7301bfec863 100644\n> > --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > @@ -404,12 +404,12 @@ void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config,\n> >  /**\n> >   * \\copydoc libcamera::ipa::Algorithm::prepare\n> >   */\n> > -void LensShadingCorrection::prepare(IPAContext &context,\n> > +void LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context,\n> >                                     [[maybe_unused]] const uint32_t frame,\n> > -                                   [[maybe_unused]] IPAFrameContext &frameContext,\n> > +                                   IPAFrameContext &frameContext,\n> >                                     RkISP1Params *params)\n> >  {\n> > -       uint32_t ct = context.activeState.awb.temperatureK;\n> > +       uint32_t ct = frameContext.awb.temperatureK;\n> \n> It is a good job Awb begins with 'a'...\n> \n> >         if (std::abs(static_cast<int>(ct) - static_cast<int>(lastAppliedCt_)) <\n> >             kColourTemperatureChangeThreshhold)\n> >                 return;\n> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > index 6bc922a82971..769e9f114e23 100644\n> > --- a/src/ipa/rkisp1/ipa_context.h\n> > +++ b/src/ipa/rkisp1/ipa_context.h\n> > @@ -91,12 +91,12 @@ struct IPAActiveState {\n> >         struct {\n> >                 struct AwbState {\n> >                         RGB<double> gains;\n> > +                       unsigned int temperatureK;\n> >                 };\n> >  \n> >                 AwbState manual;\n> >                 AwbState automatic;\n> >  \n> > -               unsigned int temperatureK;\n> >                 bool autoEnabled;\n> >         } awb;\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 AD9C6C3213\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  1 Apr 2025 20:16:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1859F6894F;\n\tTue,  1 Apr 2025 22:16:37 +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 460E068947\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  1 Apr 2025 22:16:35 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7D4AE6F9;\n\tTue,  1 Apr 2025 22:14:42 +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=\"FQn1PrPo\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743538482;\n\tbh=e/nhVDbqhwvmdhTK2zdBrGIbw0WWK1/zMzSYqRS8J/c=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FQn1PrPoBkRtLqoX935vRG1KXv0UgQZ3x0ogJ2gTt4QQWV4ZBKjEgYou1OOFIWDgs\n\tixy4ho2nNoZuZJgXJ6xEJOfGsZp72XNlStb8cTCGGLjv/yT9v8qgU+y1VXJH6VG5kZ\n\t2TYHG1X7rhGVFj9D0zATQzlvU16QwcizSoPmO8go=","Date":"Tue, 1 Apr 2025 23:16:09 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Stefan Klug <stefan.klug@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 10/17] ipa: rkisp1: ccm/lsc: Fix CCM/LSC based on\n\tmanual color temperature","Message-ID":"<20250401201609.GA1739@pendragon.ideasonboard.com>","References":"<20250319161152.63625-1-stefan.klug@ideasonboard.com>\n\t<20250319161152.63625-11-stefan.klug@ideasonboard.com>\n\t<174344224169.3394313.5397790304911977441@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<174344224169.3394313.5397790304911977441@ping.linuxembedded.co.uk>","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":33865,"web_url":"https://patchwork.libcamera.org/comment/33865/","msgid":"<eald3sdthpesh5ehgodworcfcgd5gygqaod3aws6iajs4t6quy@lqokwhoyfwck>","date":"2025-04-01T21:23:42","subject":"Re: [PATCH v2 10/17] ipa: rkisp1: ccm/lsc: Fix CCM/LSC based on\n\tmanual color temperature","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"On Tue, Apr 01, 2025 at 11:16:09PM +0300, Laurent Pinchart wrote:\n> On Mon, Mar 31, 2025 at 06:30:41PM +0100, Kieran Bingham wrote:\n> > Quoting Stefan Klug (2025-03-19 16:11:15)\n> > > In RkISP1Awb::process(), the color temperature in the active state is\n> > > updated every time new statistics are available.  The CCM/LSC algorithms\n> > > use that value in prepare() to update the CCM/LSC. This is not correct\n> > > if the color temperature was specified manually and leads to visible\n> > > flicker even when AwbEnable is set to false.\n> > > \n> > > To fix that, track the auto and manual color temperature separately in\n> > > active state. In Awb::prepare() the current frame context is updated\n> > > with the corresponding value from active state. Change the algorithms to\n> > > fetch the color temperature from the frame context instead of the active\n> > > state in prepare().\n> > > \n> > > Fixes: 02308809548d (\"ipa: rkisp1: awb: Implement ColourTemperature control\")\n> > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> > > \n> > > ---\n> > > \n> > > Changes in v2:\n> > > - None\n> > > ---\n> > >  src/ipa/rkisp1/algorithms/awb.cpp | 18 ++++++++++--------\n> > >  src/ipa/rkisp1/algorithms/ccm.cpp |  2 +-\n> > >  src/ipa/rkisp1/algorithms/lsc.cpp |  6 +++---\n> > >  src/ipa/rkisp1/ipa_context.h      |  2 +-\n> > >  4 files changed, 15 insertions(+), 13 deletions(-)\n> > > \n> > > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> > > index a9759e53f593..5e067e50cd52 100644\n> > > --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> > > +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> > > @@ -128,7 +128,8 @@ int Awb::configure(IPAContext &context,\n> > >         context.activeState.awb.automatic.gains =\n> > >                 awbAlgo_->gainsFromColourTemperature(kDefaultColourTemperature);\n> > >         context.activeState.awb.autoEnabled = true;\n> > > -       context.activeState.awb.temperatureK = kDefaultColourTemperature;\n> > > +       context.activeState.awb.manual.temperatureK = kDefaultColourTemperature;\n> > > +       context.activeState.awb.automatic.temperatureK = kDefaultColourTemperature;\n> > >  \n> > >         /*\n> > >          * Define the measurement window for AWB as a centered rectangle\n> > > @@ -185,7 +186,7 @@ void Awb::queueRequest(IPAContext &context,\n> > >                 const auto &gains = awbAlgo_->gainsFromColourTemperature(*colourTemperature);\n> > >                 awb.manual.gains.r() = gains.r();\n> > >                 awb.manual.gains.b() = gains.b();\n> > > -               awb.temperatureK = *colourTemperature;\n> > > +               awb.manual.temperatureK = *colourTemperature;\n> > >                 update = true;\n> > >         }\n> > >  \n> > > @@ -194,7 +195,7 @@ void Awb::queueRequest(IPAContext &context,\n> > >                         << \"Set colour gains to \" << awb.manual.gains;\n> > >  \n> > >         frameContext.awb.gains = awb.manual.gains;\n> > > -       frameContext.awb.temperatureK = awb.temperatureK;\n> > > +       frameContext.awb.temperatureK = awb.manual.temperatureK;\n> > >  }\n> > >  \n> > >  /**\n> > > @@ -208,8 +209,9 @@ void Awb::prepare(IPAContext &context, const uint32_t frame,\n> > >          * most up-to-date automatic values we can read.\n> > >          */\n> > >         if (frameContext.awb.autoEnabled) {\n> > > -               frameContext.awb.gains = context.activeState.awb.automatic.gains;\n> > > -               frameContext.awb.temperatureK = context.activeState.awb.temperatureK;\n> > > +               auto &awb = context.activeState.awb;\n> > > +               frameContext.awb.gains = awb.automatic.gains;\n> > > +               frameContext.awb.temperatureK = awb.automatic.temperatureK;\n> > >         }\n> > >  \n> > >         auto gainConfig = params->block<BlockType::AwbGain>();\n> > > @@ -309,10 +311,10 @@ void Awb::process(IPAContext &context,\n> > >         RkISP1AwbStats awbStats{ rgbMeans };\n> > >         AwbResult awbResult = awbAlgo_->calculateAwb(awbStats, frameContext.lux.lux);\n> > >  \n> > > -       activeState.awb.temperatureK = awbResult.colourTemperature;\n> > > +       activeState.awb.automatic.temperatureK = awbResult.colourTemperature;\n> > >  \n> > >         /* Metadata shall contain the up to date measurement */\n> > > -       metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);\n> > > +       metadata.set(controls::ColourTemperature, activeState.awb.automatic.temperatureK);\n> > \n> > I think we discussed different options for this behaviour.\n> > \n> > Don't we expect to report the manually set colour temperature if that's\n> > what was set on that framecontext ? (So that the colour gains match the\n> > colour temperature or such ?)\n> \n> That's the general rule for libcamera controls. The less we depart from\n> general control rules, the better (but there will always be some\n> exceptions).\n> \n> > Though I wish there was a way to report both so we can still what the\n> > autoregulation 'would think' even if it's set to manual...\n> \n> I'm increasingly leaning towards having two controls if we really need\n> to expose the measured temperature while in manual mode. We'll have to\n> define how those controls work in auto mode, and also decide if all\n> cameras that support colour temperature need to support measuring in\n> manual mode.\n> \n> > >         /*\n> > >          * Clamp the gain values to the hardware, which expresses gains as Q2.8\n> > > @@ -333,7 +335,7 @@ void Awb::process(IPAContext &context,\n> > >                 << std::showpoint\n> > >                 << \"Means \" << rgbMeans << \", gains \"\n> > >                 << activeState.awb.automatic.gains << \", temp \"\n> > > -               << activeState.awb.temperatureK << \"K\";\n> > > +               << activeState.awb.automatic.temperatureK << \"K\";\n> > >  }\n> > >  \n> > >  RGB<double> Awb::calculateRgbMeans(const IPAFrameContext &frameContext, const rkisp1_cif_isp_awb_stat *awb) const\n> > > diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp\n> > > index eb8ca39e56a8..2e5e91006b55 100644\n> > > --- a/src/ipa/rkisp1/algorithms/ccm.cpp\n> > > +++ b/src/ipa/rkisp1/algorithms/ccm.cpp\n> > > @@ -88,7 +88,7 @@ void Ccm::setParameters(struct rkisp1_cif_isp_ctk_config &config,\n> > >  void Ccm::prepare(IPAContext &context, const uint32_t frame,\n> > >                   IPAFrameContext &frameContext, RkISP1Params *params)\n> > >  {\n> > > -       uint32_t ct = context.activeState.awb.temperatureK;\n> > > +       uint32_t ct = frameContext.awb.temperatureK;\n> > \n> > \n> > Hrm. ... we need to make sure Awb algo always runs before Ccm in this\n> > instance.\n> > \n> > I think we're \"in luck\" because of alphabetical sorting, but I think\n> > this indicates we will need to indicate some sort of dependencies\n> > between algos...\n> \n> It's not an alphabetical order matter, at least not alphabetical order\n> of the C++ class names. Algorithms are run in the order they are\n> instantiated in the tuning file. I however agree we should be able to\n> specify dependencies, that would be safer.\n\nYes, declaring dependencies would be nice. At the moment the correct\norder is only manually ensured inside the tuning file generator. That\nwill break as soon as people start to manually modify the tuning files\nwithout knowing all the internals.\n\nWith WDR there is another algo where process() needs to run after AEGC\nhas processed the stats but prepare() needs to run before AEGC does\nprepare()... At the moment it works because our AEGC implementation is\nnot completely correct (it calculates the gain exposure split in process\nwhich imho needs to be moved to prepare() in the future). Limitless fun\nfor the future :-) \n\nCheers,\nStefan\n\n> \n> > >  \n> > >         /*\n> > >          * \\todo The colour temperature will likely be noisy, add filtering to\n> > > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > > index e47aa2f0727e..e7301bfec863 100644\n> > > --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> > > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > > @@ -404,12 +404,12 @@ void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config,\n> > >  /**\n> > >   * \\copydoc libcamera::ipa::Algorithm::prepare\n> > >   */\n> > > -void LensShadingCorrection::prepare(IPAContext &context,\n> > > +void LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context,\n> > >                                     [[maybe_unused]] const uint32_t frame,\n> > > -                                   [[maybe_unused]] IPAFrameContext &frameContext,\n> > > +                                   IPAFrameContext &frameContext,\n> > >                                     RkISP1Params *params)\n> > >  {\n> > > -       uint32_t ct = context.activeState.awb.temperatureK;\n> > > +       uint32_t ct = frameContext.awb.temperatureK;\n> > \n> > It is a good job Awb begins with 'a'...\n> > \n> > >         if (std::abs(static_cast<int>(ct) - static_cast<int>(lastAppliedCt_)) <\n> > >             kColourTemperatureChangeThreshhold)\n> > >                 return;\n> > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > > index 6bc922a82971..769e9f114e23 100644\n> > > --- a/src/ipa/rkisp1/ipa_context.h\n> > > +++ b/src/ipa/rkisp1/ipa_context.h\n> > > @@ -91,12 +91,12 @@ struct IPAActiveState {\n> > >         struct {\n> > >                 struct AwbState {\n> > >                         RGB<double> gains;\n> > > +                       unsigned int temperatureK;\n> > >                 };\n> > >  \n> > >                 AwbState manual;\n> > >                 AwbState automatic;\n> > >  \n> > > -               unsigned int temperatureK;\n> > >                 bool autoEnabled;\n> > >         } awb;\n> > >  \n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 56170C3213\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  1 Apr 2025 21:23:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A7A2468947;\n\tTue,  1 Apr 2025 23:23:47 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4958568947\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  1 Apr 2025 23:23:45 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:14c7:4fcc:495b:719f])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BB4CC741;\n\tTue,  1 Apr 2025 23:21:52 +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=\"L+3dWHIu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743542512;\n\tbh=SPg5/drPfqNUrKJ5TbDL+0p0d2wJfnAuAS2M37Sm5Xw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=L+3dWHIugTXgRpHKCjwF/yT4PkXVP5iJYOLsIZ9jHM8UZiPPCiqg6T1oaIWtdPa93\n\tnFIGIsNzVT+XAqYcVL4jBsMUhRDE0QZK98xomWc4H4VSx5XlwP+nVmZveD0O2aGh7E\n\tUZkEIwWhycDok90auwrBtJRrxZBQ06MN7Wscm+78=","Date":"Tue, 1 Apr 2025 23:23:42 +0200","From":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"Kieran Bingham <kieran.bingham@ideasonboard.com>, \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 10/17] ipa: rkisp1: ccm/lsc: Fix CCM/LSC based on\n\tmanual color temperature","Message-ID":"<eald3sdthpesh5ehgodworcfcgd5gygqaod3aws6iajs4t6quy@lqokwhoyfwck>","References":"<20250319161152.63625-1-stefan.klug@ideasonboard.com>\n\t<20250319161152.63625-11-stefan.klug@ideasonboard.com>\n\t<174344224169.3394313.5397790304911977441@ping.linuxembedded.co.uk>\n\t<20250401201609.GA1739@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250401201609.GA1739@pendragon.ideasonboard.com>","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":33868,"web_url":"https://patchwork.libcamera.org/comment/33868/","msgid":"<4mzedd2phqvp2tujqaqnxtj7flfnb35lvyemzaiwaceo27vujq@3wofmmwtmh47>","date":"2025-04-01T21:41:43","subject":"Re: [PATCH v2 10/17] ipa: rkisp1: ccm/lsc: Fix CCM/LSC based on\n\tmanual color temperature","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"On Mon, Mar 31, 2025 at 06:30:41PM +0100, Kieran Bingham wrote:\n> Quoting Stefan Klug (2025-03-19 16:11:15)\n> > In RkISP1Awb::process(), the color temperature in the active state is\n> > updated every time new statistics are available.  The CCM/LSC algorithms\n> > use that value in prepare() to update the CCM/LSC. This is not correct\n> > if the color temperature was specified manually and leads to visible\n> > flicker even when AwbEnable is set to false.\n> > \n> > To fix that, track the auto and manual color temperature separately in\n> > active state. In Awb::prepare() the current frame context is updated\n> > with the corresponding value from active state. Change the algorithms to\n> > fetch the color temperature from the frame context instead of the active\n> > state in prepare().\n> > \n> > Fixes: 02308809548d (\"ipa: rkisp1: awb: Implement ColourTemperature control\")\n> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>\n> > \n> > ---\n> > \n> > Changes in v2:\n> > - None\n> > ---\n> >  src/ipa/rkisp1/algorithms/awb.cpp | 18 ++++++++++--------\n> >  src/ipa/rkisp1/algorithms/ccm.cpp |  2 +-\n> >  src/ipa/rkisp1/algorithms/lsc.cpp |  6 +++---\n> >  src/ipa/rkisp1/ipa_context.h      |  2 +-\n> >  4 files changed, 15 insertions(+), 13 deletions(-)\n> > \n> > diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp\n> > index a9759e53f593..5e067e50cd52 100644\n> > --- a/src/ipa/rkisp1/algorithms/awb.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/awb.cpp\n> > @@ -128,7 +128,8 @@ int Awb::configure(IPAContext &context,\n> >         context.activeState.awb.automatic.gains =\n> >                 awbAlgo_->gainsFromColourTemperature(kDefaultColourTemperature);\n> >         context.activeState.awb.autoEnabled = true;\n> > -       context.activeState.awb.temperatureK = kDefaultColourTemperature;\n> > +       context.activeState.awb.manual.temperatureK = kDefaultColourTemperature;\n> > +       context.activeState.awb.automatic.temperatureK = kDefaultColourTemperature;\n> >  \n> >         /*\n> >          * Define the measurement window for AWB as a centered rectangle\n> > @@ -185,7 +186,7 @@ void Awb::queueRequest(IPAContext &context,\n> >                 const auto &gains = awbAlgo_->gainsFromColourTemperature(*colourTemperature);\n> >                 awb.manual.gains.r() = gains.r();\n> >                 awb.manual.gains.b() = gains.b();\n> > -               awb.temperatureK = *colourTemperature;\n> > +               awb.manual.temperatureK = *colourTemperature;\n> >                 update = true;\n> >         }\n> >  \n> > @@ -194,7 +195,7 @@ void Awb::queueRequest(IPAContext &context,\n> >                         << \"Set colour gains to \" << awb.manual.gains;\n> >  \n> >         frameContext.awb.gains = awb.manual.gains;\n> > -       frameContext.awb.temperatureK = awb.temperatureK;\n> > +       frameContext.awb.temperatureK = awb.manual.temperatureK;\n> >  }\n> >  \n> >  /**\n> > @@ -208,8 +209,9 @@ void Awb::prepare(IPAContext &context, const uint32_t frame,\n> >          * most up-to-date automatic values we can read.\n> >          */\n> >         if (frameContext.awb.autoEnabled) {\n> > -               frameContext.awb.gains = context.activeState.awb.automatic.gains;\n> > -               frameContext.awb.temperatureK = context.activeState.awb.temperatureK;\n> > +               auto &awb = context.activeState.awb;\n> > +               frameContext.awb.gains = awb.automatic.gains;\n> > +               frameContext.awb.temperatureK = awb.automatic.temperatureK;\n> >         }\n> >  \n> >         auto gainConfig = params->block<BlockType::AwbGain>();\n> > @@ -309,10 +311,10 @@ void Awb::process(IPAContext &context,\n> >         RkISP1AwbStats awbStats{ rgbMeans };\n> >         AwbResult awbResult = awbAlgo_->calculateAwb(awbStats, frameContext.lux.lux);\n> >  \n> > -       activeState.awb.temperatureK = awbResult.colourTemperature;\n> > +       activeState.awb.automatic.temperatureK = awbResult.colourTemperature;\n> >  \n> >         /* Metadata shall contain the up to date measurement */\n> > -       metadata.set(controls::ColourTemperature, activeState.awb.temperatureK);\n> > +       metadata.set(controls::ColourTemperature, activeState.awb.automatic.temperatureK);\n> \n> I think we discussed different options for this behaviour.\n> \n> Don't we expect to report the manually set colour temperature if that's\n> what was set on that framecontext ? (So that the colour gains match the\n> colour temperature or such ?)\n\nOuch good catch. Thank you. That line shouldn't be there at all. The\nmetadata gets set a few lines above. Turns out that it was a rebase\nerror that slipped in in b60bd37b1a49 (\"ipa: rkisp1: Move calculation of\nRGB means into own function\"). I'll fix in v3.\n\nBest regards,\nStefan\n\n> \n> \n> Though I wish there was a way to report both so we can still what the\n> autoregulation 'would think' even if it's set to manual...\n>   \n> >         /*\n> >          * Clamp the gain values to the hardware, which expresses gains as Q2.8\n> > @@ -333,7 +335,7 @@ void Awb::process(IPAContext &context,\n> >                 << std::showpoint\n> >                 << \"Means \" << rgbMeans << \", gains \"\n> >                 << activeState.awb.automatic.gains << \", temp \"\n> > -               << activeState.awb.temperatureK << \"K\";\n> > +               << activeState.awb.automatic.temperatureK << \"K\";\n> >  }\n> >  \n> >  RGB<double> Awb::calculateRgbMeans(const IPAFrameContext &frameContext, const rkisp1_cif_isp_awb_stat *awb) const\n> > diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp\n> > index eb8ca39e56a8..2e5e91006b55 100644\n> > --- a/src/ipa/rkisp1/algorithms/ccm.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/ccm.cpp\n> > @@ -88,7 +88,7 @@ void Ccm::setParameters(struct rkisp1_cif_isp_ctk_config &config,\n> >  void Ccm::prepare(IPAContext &context, const uint32_t frame,\n> >                   IPAFrameContext &frameContext, RkISP1Params *params)\n> >  {\n> > -       uint32_t ct = context.activeState.awb.temperatureK;\n> > +       uint32_t ct = frameContext.awb.temperatureK;\n> \n> \n> Hrm. ... we need to make sure Awb algo always runs before Ccm in this\n> instance.\n> \n> I think we're \"in luck\" because of alphabetical sorting, but I think\n> this indicates we will need to indicate some sort of dependencies\n> between algos...\n> \n> \n> \n> >  \n> >         /*\n> >          * \\todo The colour temperature will likely be noisy, add filtering to\n> > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > index e47aa2f0727e..e7301bfec863 100644\n> > --- a/src/ipa/rkisp1/algorithms/lsc.cpp\n> > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp\n> > @@ -404,12 +404,12 @@ void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config,\n> >  /**\n> >   * \\copydoc libcamera::ipa::Algorithm::prepare\n> >   */\n> > -void LensShadingCorrection::prepare(IPAContext &context,\n> > +void LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context,\n> >                                     [[maybe_unused]] const uint32_t frame,\n> > -                                   [[maybe_unused]] IPAFrameContext &frameContext,\n> > +                                   IPAFrameContext &frameContext,\n> >                                     RkISP1Params *params)\n> >  {\n> > -       uint32_t ct = context.activeState.awb.temperatureK;\n> > +       uint32_t ct = frameContext.awb.temperatureK;\n> \n> It is a good job Awb begins with 'a'...\n> \n> >         if (std::abs(static_cast<int>(ct) - static_cast<int>(lastAppliedCt_)) <\n> >             kColourTemperatureChangeThreshhold)\n> >                 return;\n> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > index 6bc922a82971..769e9f114e23 100644\n> > --- a/src/ipa/rkisp1/ipa_context.h\n> > +++ b/src/ipa/rkisp1/ipa_context.h\n> > @@ -91,12 +91,12 @@ struct IPAActiveState {\n> >         struct {\n> >                 struct AwbState {\n> >                         RGB<double> gains;\n> > +                       unsigned int temperatureK;\n> >                 };\n> >  \n> >                 AwbState manual;\n> >                 AwbState automatic;\n> >  \n> > -               unsigned int temperatureK;\n> >                 bool autoEnabled;\n> >         } awb;\n> >  \n> > -- \n> > 2.43.0\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 B1144C323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  1 Apr 2025 21:41:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D1C8E6894F;\n\tTue,  1 Apr 2025 23:41:48 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8E06168947\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  1 Apr 2025 23:41:47 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:14c7:4fcc:495b:719f])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DF119741;\n\tTue,  1 Apr 2025 23:39:54 +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=\"dcrz7csC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743543595;\n\tbh=efcuylOeaVX1BCz4qgejWDYFDdpk2sBnjNA7bDMvS7A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dcrz7csC2p4cswwrW5/63JFko4b1femci1gjjHoSwJPMeKKbEQDK/EbzbPABhn0KY\n\t5Ck/CLg8xIhX+GRLWLNnl3l2HsVkv4Asy4W+7K4YflbJJQuKP5URpMxumwEXHfEFVs\n\tbBhpu2SmybQyFOLqYGpCXlZyGYYU8e2weGkiVqgM=","Date":"Tue, 1 Apr 2025 23:41:43 +0200","From":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 10/17] ipa: rkisp1: ccm/lsc: Fix CCM/LSC based on\n\tmanual color temperature","Message-ID":"<4mzedd2phqvp2tujqaqnxtj7flfnb35lvyemzaiwaceo27vujq@3wofmmwtmh47>","References":"<20250319161152.63625-1-stefan.klug@ideasonboard.com>\n\t<20250319161152.63625-11-stefan.klug@ideasonboard.com>\n\t<174344224169.3394313.5397790304911977441@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<174344224169.3394313.5397790304911977441@ping.linuxembedded.co.uk>","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>"}}]