[{"id":35331,"web_url":"https://patchwork.libcamera.org/comment/35331/","msgid":"","date":"2025-08-11T09:05:51","subject":"Re: [PATCH v2] ipa: rpi: ccm: Implement \"manual\" CCM mode","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nOn Mon, 4 Aug 2025 at 15:48, David Plowman\n wrote:\n>\n> The CCM algorithm will now let an explicit colour matrix be set when\n> AWB is in manual mode.\n>\n> We must handle any controls that can cause the AWB to be enabled or\n> disabled first, so that we know the AWB's state correctly when we come\n> to set the CCM.\n>\n> Signed-off-by: David Plowman \n\nLooks reasonable to me.\n\nReviewed-by: Naushir Patuck \n\n> ---\n> src/ipa/rpi/common/ipa_base.cpp | 180 +++++++++++++++++++------\n> src/ipa/rpi/common/ipa_base.h | 2 +\n> src/ipa/rpi/controller/ccm_algorithm.h | 6 +\n> src/ipa/rpi/controller/rpi/ccm.cpp | 24 +++-\n> src/ipa/rpi/controller/rpi/ccm.h | 5 +-\n> 5 files changed, 170 insertions(+), 47 deletions(-)\n>\n> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> index ce2343e9..6448e6ab 100644\n> --- a/src/ipa/rpi/common/ipa_base.cpp\n> +++ b/src/ipa/rpi/common/ipa_base.cpp\n> @@ -94,6 +94,7 @@ const ControlInfoMap::Map ipaColourControls{\n> { &controls::AwbEnable, ControlInfo(false, true) },\n> { &controls::AwbMode, ControlInfo(controls::AwbModeValues) },\n> { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },\n> + { &controls::ColourCorrectionMatrix, ControlInfo(0.0f, 8.0f) },\n> { &controls::ColourTemperature, ControlInfo(100, 100000) },\n> { &controls::Saturation, ControlInfo(0.0f, 32.0f, 1.0f) },\n> };\n> @@ -126,7 +127,7 @@ namespace ipa::RPi {\n> IpaBase::IpaBase()\n> : controller_(), frameLengths_(FrameLengthsQueueSize, 0s), statsMetadataOutput_(false),\n> stitchSwapBuffers_(false), frameCount_(0), mistrustCount_(0), lastRunTimestamp_(0),\n> - firstStart_(true), flickerState_({ 0, 0s })\n> + firstStart_(true), flickerState_({ 0, 0s }), awbEnabled_(true)\n> {\n> }\n>\n> @@ -821,6 +822,102 @@ void IpaBase::applyControls(const ControlList &controls)\n> }\n> }\n>\n> + /*\n> + * We must also handle any AWB on/off changes first, so that the CCM algorithm\n> + * knows its state correctly.\n> + */\n> + const auto awbEnable = controls.get(controls::AwbEnable);\n> + if (awbEnable)\n> + do {\n> + /* Silently ignore this control for a mono sensor. */\n> + if (monoSensor_)\n> + break;\n> +\n> + RPiController::AwbAlgorithm *awb = dynamic_cast(\n> + controller_.getAlgorithm(\"awb\"));\n> + if (!awb) {\n> + LOG(IPARPI, Warning)\n> + << \"Could not set AWB_ENABLE - no AWB algorithm\";\n> + break;\n> + }\n> +\n> + awbEnabled_ = *awbEnable;\n> +\n> + if (!awbEnabled_)\n> + awb->disableAuto();\n> + else {\n> + awb->enableAuto();\n> +\n> + /* The CCM algorithm must go back to auto as well. */\n> + RPiController::CcmAlgorithm *ccm = dynamic_cast(\n> + controller_.getAlgorithm(\"ccm\"));\n> + if (ccm)\n> + ccm->enableAuto();\n> + }\n> +\n> + libcameraMetadata_.set(controls::AwbEnable, awbEnabled_);\n> + } while (false);\n> +\n> + const auto colourGains = controls.get(controls::ColourGains);\n> + if (colourGains)\n> + do {\n> + /* Silently ignore this control for a mono sensor. */\n> + if (monoSensor_)\n> + break;\n> +\n> + auto gains = *colourGains;\n> + RPiController::AwbAlgorithm *awb = dynamic_cast(\n> + controller_.getAlgorithm(\"awb\"));\n> + if (!awb) {\n> + LOG(IPARPI, Warning)\n> + << \"Could not set COLOUR_GAINS - no AWB algorithm\";\n> + break;\n> + }\n> +\n> + awb->setManualGains(gains[0], gains[1]);\n> + if (gains[0] != 0.0f && gains[1] != 0.0f) {\n> + /* A gain of 0.0f will switch back to auto mode. */\n> + libcameraMetadata_.set(controls::ColourGains,\n> + { gains[0], gains[1] });\n> +\n> + awbEnabled_ = false; /* doing this puts AWB into manual mode */\n> + } else {\n> + awbEnabled_ = true; /* doing this puts AWB into auto mode */\n> +\n> + /* The CCM algorithm must go back to auto as well. */\n> + RPiController::CcmAlgorithm *ccm = dynamic_cast(\n> + controller_.getAlgorithm(\"ccm\"));\n> + if (ccm)\n> + ccm->enableAuto();\n> + }\n> +\n> + /* This metadata will get reported back automatically. */\n> + break;\n> + } while (false);\n> +\n> + const auto colourTemperature = controls.get(controls::ColourTemperature);\n> + if (colourTemperature)\n> + do {\n> + /* Silently ignore this control for a mono sensor. */\n> + if (monoSensor_)\n> + break;\n> +\n> + auto temperatureK = *colourTemperature;\n> + RPiController::AwbAlgorithm *awb = dynamic_cast(\n> + controller_.getAlgorithm(\"awb\"));\n> + if (!awb) {\n> + LOG(IPARPI, Warning)\n> + << \"Could not set COLOUR_TEMPERATURE - no AWB algorithm\";\n> + break;\n> + }\n> +\n> + awb->setColourTemperature(temperatureK);\n> + awbEnabled_ = false; /* doing this puts AWB into manual mode */\n> +\n> + /* This metadata will get reported back automatically. */\n> + break;\n> + } while (false);\n> +\n> /* Iterate over controls */\n> for (auto const &ctrl : controls) {\n> LOG(IPARPI, Debug) << \"Request ctrl: \"\n> @@ -1037,25 +1134,7 @@ void IpaBase::applyControls(const ControlList &controls)\n> }\n>\n> case controls::AWB_ENABLE: {\n> - /* Silently ignore this control for a mono sensor. */\n> - if (monoSensor_)\n> - break;\n> -\n> - RPiController::AwbAlgorithm *awb = dynamic_cast(\n> - controller_.getAlgorithm(\"awb\"));\n> - if (!awb) {\n> - LOG(IPARPI, Warning)\n> - << \"Could not set AWB_ENABLE - no AWB algorithm\";\n> - break;\n> - }\n> -\n> - if (ctrl.second.get() == false)\n> - awb->disableAuto();\n> - else\n> - awb->enableAuto();\n> -\n> - libcameraMetadata_.set(controls::AwbEnable,\n> - ctrl.second.get());\n> + /* We handled this one above. */\n> break;\n> }\n>\n> @@ -1080,47 +1159,60 @@ void IpaBase::applyControls(const ControlList &controls)\n> LOG(IPARPI, Error) << \"AWB mode \" << idx\n> << \" not recognised\";\n> }\n> +\n> break;\n> }\n>\n> case controls::COLOUR_GAINS: {\n> - /* Silently ignore this control for a mono sensor. */\n> - if (monoSensor_)\n> - break;\n> -\n> - auto gains = ctrl.second.get>();\n> - RPiController::AwbAlgorithm *awb = dynamic_cast(\n> - controller_.getAlgorithm(\"awb\"));\n> - if (!awb) {\n> - LOG(IPARPI, Warning)\n> - << \"Could not set COLOUR_GAINS - no AWB algorithm\";\n> - break;\n> - }\n> -\n> - awb->setManualGains(gains[0], gains[1]);\n> - if (gains[0] != 0.0f && gains[1] != 0.0f)\n> - /* A gain of 0.0f will switch back to auto mode. */\n> - libcameraMetadata_.set(controls::ColourGains,\n> - { gains[0], gains[1] });\n> + /* We handled this one above. */\n> break;\n> }\n>\n> case controls::COLOUR_TEMPERATURE: {\n> - /* Silently ignore this control for a mono sensor. */\n> + /* We handled this one above. */\n> + break;\n> + }\n> +\n> + case controls::COLOUR_CORRECTION_MATRIX: {\n> if (monoSensor_)\n> break;\n>\n> - auto temperatureK = ctrl.second.get();\n> + auto floats = ctrl.second.get>();\n> + RPiController::CcmAlgorithm *ccm = dynamic_cast(\n> + controller_.getAlgorithm(\"ccm\"));\n> + if (!ccm) {\n> + LOG(IPARPI, Warning)\n> + << \"Could not set COLOUR_CORRECTION_MATRIX - no CCM algorithm\";\n> + break;\n> + }\n> +\n> RPiController::AwbAlgorithm *awb = dynamic_cast(\n> controller_.getAlgorithm(\"awb\"));\n> - if (!awb) {\n> + if (awb && awbEnabled_) {\n> LOG(IPARPI, Warning)\n> - << \"Could not set COLOUR_TEMPERATURE - no AWB algorithm\";\n> + << \"Could not set COLOUR_CORRECTION_MATRIX - AWB is active\";\n> break;\n> }\n>\n> - awb->setColourTemperature(temperatureK);\n> - /* This metadata will get reported back automatically. */\n> + /* We are guaranteed this control contains 9 values. Nevertheless: */\n> + assert(floats.size() == 9);\n> +\n> + Matrix matrix;\n> + for (std::size_t i = 0; i < 3; ++i)\n> + for (std::size_t j = 0; j < 3; ++j)\n> + matrix[i][j] = static_cast(floats[i * 3 + j]);\n> +\n> + ccm->setCcm(matrix);\n> +\n> + /*\n> + * But if AWB is running, go back to auto mode. The CCM gets remembered,\n> + * which avoids the race between setting the CCM and disabling AWB in\n> + * the same set of controls.\n> + */\n> + if (awbEnabled_)\n> + ccm->enableAuto();\n> +\n> + /* This metadata will be reported back automatically. */\n> break;\n> }\n>\n> diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h\n> index e2f6e330..5348f2ea 100644\n> --- a/src/ipa/rpi/common/ipa_base.h\n> +++ b/src/ipa/rpi/common/ipa_base.h\n> @@ -140,6 +140,8 @@ private:\n> int32_t mode;\n> utils::Duration manualPeriod;\n> } flickerState_;\n> +\n> + bool awbEnabled_;\n> };\n>\n> } /* namespace ipa::RPi */\n> diff --git a/src/ipa/rpi/controller/ccm_algorithm.h b/src/ipa/rpi/controller/ccm_algorithm.h\n> index 6678ba75..785c408a 100644\n> --- a/src/ipa/rpi/controller/ccm_algorithm.h\n> +++ b/src/ipa/rpi/controller/ccm_algorithm.h\n> @@ -6,16 +6,22 @@\n> */\n> #pragma once\n>\n> +#include \"libcamera/internal/matrix.h\"\n> +\n> #include \"algorithm.h\"\n>\n> namespace RPiController {\n>\n> +using Matrix3x3 = libcamera::Matrix;\n> +\n> class CcmAlgorithm : public Algorithm\n> {\n> public:\n> CcmAlgorithm(Controller *controller) : Algorithm(controller) {}\n> /* A CCM algorithm must provide the following: */\n> + virtual void enableAuto() = 0;\n> virtual void setSaturation(double saturation) = 0;\n> + virtual void setCcm(Matrix3x3 const &matrix) = 0;\n> };\n>\n> } /* namespace RPiController */\n> diff --git a/src/ipa/rpi/controller/rpi/ccm.cpp b/src/ipa/rpi/controller/rpi/ccm.cpp\n> index 8607f152..c42c0141 100644\n> --- a/src/ipa/rpi/controller/rpi/ccm.cpp\n> +++ b/src/ipa/rpi/controller/rpi/ccm.cpp\n> @@ -32,7 +32,10 @@ LOG_DEFINE_CATEGORY(RPiCcm)\n> using Matrix3x3 = Matrix;\n>\n> Ccm::Ccm(Controller *controller)\n> - : CcmAlgorithm(controller), saturation_(1.0) {}\n> + : CcmAlgorithm(controller), enableAuto_(true), saturation_(1.0),\n> + manualCcm_({ 1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0 })\n> +{\n> +}\n>\n> char const *Ccm::name() const\n> {\n> @@ -78,11 +81,22 @@ int Ccm::read(const libcamera::YamlObject ¶ms)\n> return 0;\n> }\n>\n> +void Ccm::enableAuto()\n> +{\n> + enableAuto_ = true;\n> +}\n> +\n> void Ccm::setSaturation(double saturation)\n> {\n> saturation_ = saturation;\n> }\n>\n> +void Ccm::setCcm(Matrix3x3 const &matrix)\n> +{\n> + enableAuto_ = false;\n> + manualCcm_ = matrix;\n> +}\n> +\n> void Ccm::initialise()\n> {\n> }\n> @@ -151,7 +165,13 @@ void Ccm::prepare(Metadata *imageMetadata)\n> LOG(RPiCcm, Warning) << \"no colour temperature found\";\n> if (!luxOk)\n> LOG(RPiCcm, Warning) << \"no lux value found\";\n> - Matrix3x3 ccm = calculateCcm(config_.ccms, awb.temperatureK);\n> +\n> + Matrix3x3 ccm;\n> + if (enableAuto_)\n> + ccm = calculateCcm(config_.ccms, awb.temperatureK);\n> + else\n> + ccm = manualCcm_;\n> +\n> double saturation = saturation_;\n> struct CcmStatus ccmStatus;\n> ccmStatus.saturation = saturation;\n> diff --git a/src/ipa/rpi/controller/rpi/ccm.h b/src/ipa/rpi/controller/rpi/ccm.h\n> index c05dbb17..70f28ed3 100644\n> --- a/src/ipa/rpi/controller/rpi/ccm.h\n> +++ b/src/ipa/rpi/controller/rpi/ccm.h\n> @@ -8,7 +8,6 @@\n>\n> #include \n>\n> -#include \"libcamera/internal/matrix.h\"\n> #include \n>\n> #include \"../ccm_algorithm.h\"\n> @@ -33,13 +32,17 @@ public:\n> Ccm(Controller *controller = NULL);\n> char const *name() const override;\n> int read(const libcamera::YamlObject ¶ms) override;\n> + void enableAuto() override;\n> void setSaturation(double saturation) override;\n> + void setCcm(Matrix3x3 const &matrix) override;\n> void initialise() override;\n> void prepare(Metadata *imageMetadata) override;\n>\n> private:\n> CcmConfig config_;\n> + bool enableAuto_;\n> double saturation_;\n> + Matrix3x3 manualCcm_;\n> };\n>\n> } /* namespace RPiController */\n> --\n> 2.39.5\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 E7D47BEFBE\n\tfor ;\n\tMon, 11 Aug 2025 09:06:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B65B46921E;\n\tMon, 11 Aug 2025 11:06:26 +0200 (CEST)","from mail-vs1-xe2a.google.com (mail-vs1-xe2a.google.com\n\t[IPv6:2607:f8b0:4864:20::e2a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EB4856145E\n\tfor ;\n\tMon, 11 Aug 2025 11:06:24 +0200 (CEST)","by mail-vs1-xe2a.google.com with SMTP id\n\tada2fe7eead31-4fe986a94c3so79360137.3\n\tfor ;\n\tMon, 11 Aug 2025 02:06:24 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"EscX7npf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1754903184; x=1755507984;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=r4hPv8pFNxNmwIRD44zLtdapwVIOrchHDIallVOycR8=;\n\tb=EscX7npfQepr8RrCTae4vz5iC6VvMBBtNfBclzwLDBJ6OGV9Xg7lVE5Dzdh4bHULkB\n\tJvH9hCbnh5lwDD1wB0RmBwGl9gnAAJpe8qmS1v91EjGYzZ7fpud4FOYNWMXPXoURQsrT\n\tFTq5yiRPQhWUZf1ugbj376gTK1BR7L2WLFtztMCv8hX3xuCnkGVnpe+mmeGQYaYKl7Kj\n\t08pmWlAb0gXuBNv784dU2GvR3xuqxV/uC+Uw8Om0jmIqAnjIstbHeWFFpVulFbAWcBK3\n\tMPkwO8fEhs6yT0rQ7UxJraqJCavB/6GDdUjYpKPX+WkqZE51lmI6KbzBAA4Jaxtw31Hz\n\t/GWA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1754903184; x=1755507984;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=r4hPv8pFNxNmwIRD44zLtdapwVIOrchHDIallVOycR8=;\n\tb=s4/65h7EOGI3UPqNRGIgTwgV/MYr4GhcmhVBDhxBtmuzwUStJLUbovKcdSdDjvhIz3\n\tm9fs1DSrAnxZS3FxemplyqAWkZ3rvSMOfeY2erR/JxOQ2ccbGxMjnvjff0pR+1go/Yb/\n\tbO7csh1dg6rB69r/XbPtY8SHeAGMIGPl9RkFRU1vbIltcob3kqwJ8ZpShu/cdpoRMsa1\n\tnGyGJGgl7SShYqpXzyXDv+3AwYK4Pv1J2QsXHUM27o7cCtK4YBUV46hQqQiU0BwSa30R\n\tvESWWJWvnGg0flsizoejAzhVOI4N9sdr8ceJ5FrKQieS15bUgD7bJlH1Ajxt8qVURGbs\n\tqTVQ==","X-Gm-Message-State":"AOJu0YwpdIARGo9Zh8k5XbxY4to4ecsGRvuBDoY7dKGuCDE5+9eHmD7e\n\tm1BSxfR3P73NXcN2TeN/MP3UvJd3oz24fswvXFb7MpZeECU1V0tLiuULtallRlRl5xx7KFiEexh\n\tQh4c6c0HvVGmaYjarj/fAMllboEqQpIm5c7P2xobqDrn9dyNkae2DSYA=","X-Gm-Gg":"ASbGnct4QvZPy5yhmCGIJEhyk9aTddEFQiXIqooAuaoiEXCbPBm8o/N7YfYMvcuOY71\n\tf6bSgBXS8NlkMqZekJBrHPQEzmZquWNp55QxRiWKnUlSzD34lnVvpJXTGM++7Z0HJGASUisyZyy\n\tTUPNI8rsEiHPVoUA5gQ0WK7Hmk2LXMc5RdwYPZ8DCWPWJYa63bO+drxKhwfDxBX+Bf0PXu9VBn+\n\tBDvd1gRerNq9sgIzDRrPKgSpnsK54Fl/Jrxdw==","X-Google-Smtp-Source":"AGHT+IF7oMs6K50Z5BI6Fw3XLhnkcJOJHRcKzyza/eVxD/cESJF8IAj53ZRp2ZbRu1XEfFiJAIQitDRuYv2UjO7nD+g=","X-Received":"by 2002:a05:6102:38d4:b0:4f3:22ba:ac97 with SMTP id\n\tada2fe7eead31-50774eea69cmr627742137.5.1754903183416; Mon, 11 Aug 2025\n\t02:06:23 -0700 (PDT)","MIME-Version":"1.0","References":"<20250804144804.16965-1-david.plowman@raspberrypi.com>","In-Reply-To":"<20250804144804.16965-1-david.plowman@raspberrypi.com>","From":"Naushir Patuck ","Date":"Mon, 11 Aug 2025 10:05:51 +0100","X-Gm-Features":"Ac12FXxlzV0dxA6t_oCUw1W788qaF36aKbNmdaMAIdgMOSLipQ_4n-bnkwqIOYA","Message-ID":"","Subject":"Re: [PATCH v2] ipa: rpi: ccm: Implement \"manual\" CCM mode","To":"David Plowman ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","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":35332,"web_url":"https://patchwork.libcamera.org/comment/35332/","msgid":"<175490506503.560048.9977274206332933623@ping.linuxembedded.co.uk>","date":"2025-08-11T09:37:45","subject":"Re: [PATCH v2] ipa: rpi: ccm: Implement \"manual\" CCM mode","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck (2025-08-11 10:05:51)\n> Hi David,\n> \n> On Mon, 4 Aug 2025 at 15:48, David Plowman\n> wrote:\n> >\n> > The CCM algorithm will now let an explicit colour matrix be set when\n> > AWB is in manual mode.\n> >\n> > We must handle any controls that can cause the AWB to be enabled or\n> > disabled first, so that we know the AWB's state correctly when we come\n> > to set the CCM.\n> >\n> > Signed-off-by: David Plowman \n> \n> Looks reasonable to me.\n> \n> Reviewed-by: Naushir Patuck \n\nHappy to merge whenever you like here.\n\nI'm curious - v1 said something about the current documentation not\ngiving the desired behavour.\n\nShould we change the documentation or update the expected behaviour? or\nare you ok with this one ?\n\n(I expect we can still merge this one and discuss separately)\n--\nKieran\n\n\n> \n> > ---\n> > src/ipa/rpi/common/ipa_base.cpp | 180 +++++++++++++++++++------\n> > src/ipa/rpi/common/ipa_base.h | 2 +\n> > src/ipa/rpi/controller/ccm_algorithm.h | 6 +\n> > src/ipa/rpi/controller/rpi/ccm.cpp | 24 +++-\n> > src/ipa/rpi/controller/rpi/ccm.h | 5 +-\n> > 5 files changed, 170 insertions(+), 47 deletions(-)\n> >\n> > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> > index ce2343e9..6448e6ab 100644\n> > --- a/src/ipa/rpi/common/ipa_base.cpp\n> > +++ b/src/ipa/rpi/common/ipa_base.cpp\n> > @@ -94,6 +94,7 @@ const ControlInfoMap::Map ipaColourControls{\n> > { &controls::AwbEnable, ControlInfo(false, true) },\n> > { &controls::AwbMode, ControlInfo(controls::AwbModeValues) },\n> > { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },\n> > + { &controls::ColourCorrectionMatrix, ControlInfo(0.0f, 8.0f) },\n> > { &controls::ColourTemperature, ControlInfo(100, 100000) },\n> > { &controls::Saturation, ControlInfo(0.0f, 32.0f, 1.0f) },\n> > };\n> > @@ -126,7 +127,7 @@ namespace ipa::RPi {\n> > IpaBase::IpaBase()\n> > : controller_(), frameLengths_(FrameLengthsQueueSize, 0s), statsMetadataOutput_(false),\n> > stitchSwapBuffers_(false), frameCount_(0), mistrustCount_(0), lastRunTimestamp_(0),\n> > - firstStart_(true), flickerState_({ 0, 0s })\n> > + firstStart_(true), flickerState_({ 0, 0s }), awbEnabled_(true)\n> > {\n> > }\n> >\n> > @@ -821,6 +822,102 @@ void IpaBase::applyControls(const ControlList &controls)\n> > }\n> > }\n> >\n> > + /*\n> > + * We must also handle any AWB on/off changes first, so that the CCM algorithm\n> > + * knows its state correctly.\n> > + */\n> > + const auto awbEnable = controls.get(controls::AwbEnable);\n> > + if (awbEnable)\n> > + do {\n> > + /* Silently ignore this control for a mono sensor. */\n> > + if (monoSensor_)\n> > + break;\n> > +\n> > + RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > + controller_.getAlgorithm(\"awb\"));\n> > + if (!awb) {\n> > + LOG(IPARPI, Warning)\n> > + << \"Could not set AWB_ENABLE - no AWB algorithm\";\n> > + break;\n> > + }\n> > +\n> > + awbEnabled_ = *awbEnable;\n> > +\n> > + if (!awbEnabled_)\n> > + awb->disableAuto();\n> > + else {\n> > + awb->enableAuto();\n> > +\n> > + /* The CCM algorithm must go back to auto as well. */\n> > + RPiController::CcmAlgorithm *ccm = dynamic_cast(\n> > + controller_.getAlgorithm(\"ccm\"));\n> > + if (ccm)\n> > + ccm->enableAuto();\n> > + }\n> > +\n> > + libcameraMetadata_.set(controls::AwbEnable, awbEnabled_);\n> > + } while (false);\n> > +\n> > + const auto colourGains = controls.get(controls::ColourGains);\n> > + if (colourGains)\n> > + do {\n> > + /* Silently ignore this control for a mono sensor. */\n> > + if (monoSensor_)\n> > + break;\n> > +\n> > + auto gains = *colourGains;\n> > + RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > + controller_.getAlgorithm(\"awb\"));\n> > + if (!awb) {\n> > + LOG(IPARPI, Warning)\n> > + << \"Could not set COLOUR_GAINS - no AWB algorithm\";\n> > + break;\n> > + }\n> > +\n> > + awb->setManualGains(gains[0], gains[1]);\n> > + if (gains[0] != 0.0f && gains[1] != 0.0f) {\n> > + /* A gain of 0.0f will switch back to auto mode. */\n> > + libcameraMetadata_.set(controls::ColourGains,\n> > + { gains[0], gains[1] });\n> > +\n> > + awbEnabled_ = false; /* doing this puts AWB into manual mode */\n> > + } else {\n> > + awbEnabled_ = true; /* doing this puts AWB into auto mode */\n> > +\n> > + /* The CCM algorithm must go back to auto as well. */\n> > + RPiController::CcmAlgorithm *ccm = dynamic_cast(\n> > + controller_.getAlgorithm(\"ccm\"));\n> > + if (ccm)\n> > + ccm->enableAuto();\n> > + }\n> > +\n> > + /* This metadata will get reported back automatically. */\n> > + break;\n> > + } while (false);\n> > +\n> > + const auto colourTemperature = controls.get(controls::ColourTemperature);\n> > + if (colourTemperature)\n> > + do {\n> > + /* Silently ignore this control for a mono sensor. */\n> > + if (monoSensor_)\n> > + break;\n> > +\n> > + auto temperatureK = *colourTemperature;\n> > + RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > + controller_.getAlgorithm(\"awb\"));\n> > + if (!awb) {\n> > + LOG(IPARPI, Warning)\n> > + << \"Could not set COLOUR_TEMPERATURE - no AWB algorithm\";\n> > + break;\n> > + }\n> > +\n> > + awb->setColourTemperature(temperatureK);\n> > + awbEnabled_ = false; /* doing this puts AWB into manual mode */\n> > +\n> > + /* This metadata will get reported back automatically. */\n> > + break;\n> > + } while (false);\n> > +\n> > /* Iterate over controls */\n> > for (auto const &ctrl : controls) {\n> > LOG(IPARPI, Debug) << \"Request ctrl: \"\n> > @@ -1037,25 +1134,7 @@ void IpaBase::applyControls(const ControlList &controls)\n> > }\n> >\n> > case controls::AWB_ENABLE: {\n> > - /* Silently ignore this control for a mono sensor. */\n> > - if (monoSensor_)\n> > - break;\n> > -\n> > - RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > - controller_.getAlgorithm(\"awb\"));\n> > - if (!awb) {\n> > - LOG(IPARPI, Warning)\n> > - << \"Could not set AWB_ENABLE - no AWB algorithm\";\n> > - break;\n> > - }\n> > -\n> > - if (ctrl.second.get() == false)\n> > - awb->disableAuto();\n> > - else\n> > - awb->enableAuto();\n> > -\n> > - libcameraMetadata_.set(controls::AwbEnable,\n> > - ctrl.second.get());\n> > + /* We handled this one above. */\n> > break;\n> > }\n> >\n> > @@ -1080,47 +1159,60 @@ void IpaBase::applyControls(const ControlList &controls)\n> > LOG(IPARPI, Error) << \"AWB mode \" << idx\n> > << \" not recognised\";\n> > }\n> > +\n> > break;\n> > }\n> >\n> > case controls::COLOUR_GAINS: {\n> > - /* Silently ignore this control for a mono sensor. */\n> > - if (monoSensor_)\n> > - break;\n> > -\n> > - auto gains = ctrl.second.get>();\n> > - RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > - controller_.getAlgorithm(\"awb\"));\n> > - if (!awb) {\n> > - LOG(IPARPI, Warning)\n> > - << \"Could not set COLOUR_GAINS - no AWB algorithm\";\n> > - break;\n> > - }\n> > -\n> > - awb->setManualGains(gains[0], gains[1]);\n> > - if (gains[0] != 0.0f && gains[1] != 0.0f)\n> > - /* A gain of 0.0f will switch back to auto mode. */\n> > - libcameraMetadata_.set(controls::ColourGains,\n> > - { gains[0], gains[1] });\n> > + /* We handled this one above. */\n> > break;\n> > }\n> >\n> > case controls::COLOUR_TEMPERATURE: {\n> > - /* Silently ignore this control for a mono sensor. */\n> > + /* We handled this one above. */\n> > + break;\n> > + }\n> > +\n> > + case controls::COLOUR_CORRECTION_MATRIX: {\n> > if (monoSensor_)\n> > break;\n> >\n> > - auto temperatureK = ctrl.second.get();\n> > + auto floats = ctrl.second.get>();\n> > + RPiController::CcmAlgorithm *ccm = dynamic_cast(\n> > + controller_.getAlgorithm(\"ccm\"));\n> > + if (!ccm) {\n> > + LOG(IPARPI, Warning)\n> > + << \"Could not set COLOUR_CORRECTION_MATRIX - no CCM algorithm\";\n> > + break;\n> > + }\n> > +\n> > RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > controller_.getAlgorithm(\"awb\"));\n> > - if (!awb) {\n> > + if (awb && awbEnabled_) {\n> > LOG(IPARPI, Warning)\n> > - << \"Could not set COLOUR_TEMPERATURE - no AWB algorithm\";\n> > + << \"Could not set COLOUR_CORRECTION_MATRIX - AWB is active\";\n> > break;\n> > }\n> >\n> > - awb->setColourTemperature(temperatureK);\n> > - /* This metadata will get reported back automatically. */\n> > + /* We are guaranteed this control contains 9 values. Nevertheless: */\n> > + assert(floats.size() == 9);\n> > +\n> > + Matrix matrix;\n> > + for (std::size_t i = 0; i < 3; ++i)\n> > + for (std::size_t j = 0; j < 3; ++j)\n> > + matrix[i][j] = static_cast(floats[i * 3 + j]);\n> > +\n> > + ccm->setCcm(matrix);\n> > +\n> > + /*\n> > + * But if AWB is running, go back to auto mode. The CCM gets remembered,\n> > + * which avoids the race between setting the CCM and disabling AWB in\n> > + * the same set of controls.\n> > + */\n> > + if (awbEnabled_)\n> > + ccm->enableAuto();\n> > +\n> > + /* This metadata will be reported back automatically. */\n> > break;\n> > }\n> >\n> > diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h\n> > index e2f6e330..5348f2ea 100644\n> > --- a/src/ipa/rpi/common/ipa_base.h\n> > +++ b/src/ipa/rpi/common/ipa_base.h\n> > @@ -140,6 +140,8 @@ private:\n> > int32_t mode;\n> > utils::Duration manualPeriod;\n> > } flickerState_;\n> > +\n> > + bool awbEnabled_;\n> > };\n> >\n> > } /* namespace ipa::RPi */\n> > diff --git a/src/ipa/rpi/controller/ccm_algorithm.h b/src/ipa/rpi/controller/ccm_algorithm.h\n> > index 6678ba75..785c408a 100644\n> > --- a/src/ipa/rpi/controller/ccm_algorithm.h\n> > +++ b/src/ipa/rpi/controller/ccm_algorithm.h\n> > @@ -6,16 +6,22 @@\n> > */\n> > #pragma once\n> >\n> > +#include \"libcamera/internal/matrix.h\"\n> > +\n> > #include \"algorithm.h\"\n> >\n> > namespace RPiController {\n> >\n> > +using Matrix3x3 = libcamera::Matrix;\n> > +\n> > class CcmAlgorithm : public Algorithm\n> > {\n> > public:\n> > CcmAlgorithm(Controller *controller) : Algorithm(controller) {}\n> > /* A CCM algorithm must provide the following: */\n> > + virtual void enableAuto() = 0;\n> > virtual void setSaturation(double saturation) = 0;\n> > + virtual void setCcm(Matrix3x3 const &matrix) = 0;\n> > };\n> >\n> > } /* namespace RPiController */\n> > diff --git a/src/ipa/rpi/controller/rpi/ccm.cpp b/src/ipa/rpi/controller/rpi/ccm.cpp\n> > index 8607f152..c42c0141 100644\n> > --- a/src/ipa/rpi/controller/rpi/ccm.cpp\n> > +++ b/src/ipa/rpi/controller/rpi/ccm.cpp\n> > @@ -32,7 +32,10 @@ LOG_DEFINE_CATEGORY(RPiCcm)\n> > using Matrix3x3 = Matrix;\n> >\n> > Ccm::Ccm(Controller *controller)\n> > - : CcmAlgorithm(controller), saturation_(1.0) {}\n> > + : CcmAlgorithm(controller), enableAuto_(true), saturation_(1.0),\n> > + manualCcm_({ 1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0 })\n> > +{\n> > +}\n> >\n> > char const *Ccm::name() const\n> > {\n> > @@ -78,11 +81,22 @@ int Ccm::read(const libcamera::YamlObject ¶ms)\n> > return 0;\n> > }\n> >\n> > +void Ccm::enableAuto()\n> > +{\n> > + enableAuto_ = true;\n> > +}\n> > +\n> > void Ccm::setSaturation(double saturation)\n> > {\n> > saturation_ = saturation;\n> > }\n> >\n> > +void Ccm::setCcm(Matrix3x3 const &matrix)\n> > +{\n> > + enableAuto_ = false;\n> > + manualCcm_ = matrix;\n> > +}\n> > +\n> > void Ccm::initialise()\n> > {\n> > }\n> > @@ -151,7 +165,13 @@ void Ccm::prepare(Metadata *imageMetadata)\n> > LOG(RPiCcm, Warning) << \"no colour temperature found\";\n> > if (!luxOk)\n> > LOG(RPiCcm, Warning) << \"no lux value found\";\n> > - Matrix3x3 ccm = calculateCcm(config_.ccms, awb.temperatureK);\n> > +\n> > + Matrix3x3 ccm;\n> > + if (enableAuto_)\n> > + ccm = calculateCcm(config_.ccms, awb.temperatureK);\n> > + else\n> > + ccm = manualCcm_;\n> > +\n> > double saturation = saturation_;\n> > struct CcmStatus ccmStatus;\n> > ccmStatus.saturation = saturation;\n> > diff --git a/src/ipa/rpi/controller/rpi/ccm.h b/src/ipa/rpi/controller/rpi/ccm.h\n> > index c05dbb17..70f28ed3 100644\n> > --- a/src/ipa/rpi/controller/rpi/ccm.h\n> > +++ b/src/ipa/rpi/controller/rpi/ccm.h\n> > @@ -8,7 +8,6 @@\n> >\n> > #include \n> >\n> > -#include \"libcamera/internal/matrix.h\"\n> > #include \n> >\n> > #include \"../ccm_algorithm.h\"\n> > @@ -33,13 +32,17 @@ public:\n> > Ccm(Controller *controller = NULL);\n> > char const *name() const override;\n> > int read(const libcamera::YamlObject ¶ms) override;\n> > + void enableAuto() override;\n> > void setSaturation(double saturation) override;\n> > + void setCcm(Matrix3x3 const &matrix) override;\n> > void initialise() override;\n> > void prepare(Metadata *imageMetadata) override;\n> >\n> > private:\n> > CcmConfig config_;\n> > + bool enableAuto_;\n> > double saturation_;\n> > + Matrix3x3 manualCcm_;\n> > };\n> >\n> > } /* namespace RPiController */\n> > --\n> > 2.39.5\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 23B8FBEFBE\n\tfor ;\n\tMon, 11 Aug 2025 09:37:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3E65761464;\n\tMon, 11 Aug 2025 11:37:49 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B10B06145E\n\tfor ;\n\tMon, 11 Aug 2025 11:37:47 +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 9B33B4A4;\n\tMon, 11 Aug 2025 11:36:55 +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=\"EmnOL8QV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1754905015;\n\tbh=yXSAJpnbFJS1vNzbiwAfzLAQIsk0EPpui3bDe/CFO5Q=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=EmnOL8QVQ3a8bQZFbaNmydMtBfXqE8YQKslBYwuo2e93YFa4HrzM4uHs2vZNFERl6\n\tF5t+XhUDikzY1FVrSdKZ9tSJbAGKSzI/Rj6erClC73J7D+JviTW9N9xoBAA+HHiQ22\n\tPpH0npuLqMLam+71XlZMtcZfIqiomhWuz6qQla+Y=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20250804144804.16965-1-david.plowman@raspberrypi.com>\n\t","Subject":"Re: [PATCH v2] ipa: rpi: ccm: Implement \"manual\" CCM mode","From":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","To":"David Plowman ,\n\tNaushir Patuck ","Date":"Mon, 11 Aug 2025 10:37:45 +0100","Message-ID":"<175490506503.560048.9977274206332933623@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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":35333,"web_url":"https://patchwork.libcamera.org/comment/35333/","msgid":"","date":"2025-08-11T10:55:20","subject":"Re: [PATCH v2] ipa: rpi: ccm: Implement \"manual\" CCM mode","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Kieran\n\nWell, I don't think it's worth worrying about too much because I\nbelieve I've implemented what the documentation describes.\n\nThe point was merely that the documentation doesn't describe a\nbehaviour that's super convenient for us. In our world, the CCM and\nAWB algorithm are separate things and don't actually know about each\nother, whereas the documented behaviour grabs the \"AWB enable\" and\nuses that to put the CCM into automatic or manual mode too. TBH, I\ncan't see why it would be disallowed to set a manual CCM even when AWB\nis running, but there you go.\n\nv1 of the patch made a bit of a pig's ear of the state that I'd added\nto couple these two algorithms together. But whatever, I think v2 is\ncorrect and I don't really feel motivated to change anything!!\n\nThanks\nDavid\n\nOn Mon, 11 Aug 2025 at 10:37, Kieran Bingham\n wrote:\n>\n> Quoting Naushir Patuck (2025-08-11 10:05:51)\n> > Hi David,\n> >\n> > On Mon, 4 Aug 2025 at 15:48, David Plowman\n> > wrote:\n> > >\n> > > The CCM algorithm will now let an explicit colour matrix be set when\n> > > AWB is in manual mode.\n> > >\n> > > We must handle any controls that can cause the AWB to be enabled or\n> > > disabled first, so that we know the AWB's state correctly when we come\n> > > to set the CCM.\n> > >\n> > > Signed-off-by: David Plowman \n> >\n> > Looks reasonable to me.\n> >\n> > Reviewed-by: Naushir Patuck \n>\n> Happy to merge whenever you like here.\n>\n> I'm curious - v1 said something about the current documentation not\n> giving the desired behavour.\n>\n> Should we change the documentation or update the expected behaviour? or\n> are you ok with this one ?\n>\n> (I expect we can still merge this one and discuss separately)\n> --\n> Kieran\n>\n>\n> >\n> > > ---\n> > > src/ipa/rpi/common/ipa_base.cpp | 180 +++++++++++++++++++------\n> > > src/ipa/rpi/common/ipa_base.h | 2 +\n> > > src/ipa/rpi/controller/ccm_algorithm.h | 6 +\n> > > src/ipa/rpi/controller/rpi/ccm.cpp | 24 +++-\n> > > src/ipa/rpi/controller/rpi/ccm.h | 5 +-\n> > > 5 files changed, 170 insertions(+), 47 deletions(-)\n> > >\n> > > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> > > index ce2343e9..6448e6ab 100644\n> > > --- a/src/ipa/rpi/common/ipa_base.cpp\n> > > +++ b/src/ipa/rpi/common/ipa_base.cpp\n> > > @@ -94,6 +94,7 @@ const ControlInfoMap::Map ipaColourControls{\n> > > { &controls::AwbEnable, ControlInfo(false, true) },\n> > > { &controls::AwbMode, ControlInfo(controls::AwbModeValues) },\n> > > { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },\n> > > + { &controls::ColourCorrectionMatrix, ControlInfo(0.0f, 8.0f) },\n> > > { &controls::ColourTemperature, ControlInfo(100, 100000) },\n> > > { &controls::Saturation, ControlInfo(0.0f, 32.0f, 1.0f) },\n> > > };\n> > > @@ -126,7 +127,7 @@ namespace ipa::RPi {\n> > > IpaBase::IpaBase()\n> > > : controller_(), frameLengths_(FrameLengthsQueueSize, 0s), statsMetadataOutput_(false),\n> > > stitchSwapBuffers_(false), frameCount_(0), mistrustCount_(0), lastRunTimestamp_(0),\n> > > - firstStart_(true), flickerState_({ 0, 0s })\n> > > + firstStart_(true), flickerState_({ 0, 0s }), awbEnabled_(true)\n> > > {\n> > > }\n> > >\n> > > @@ -821,6 +822,102 @@ void IpaBase::applyControls(const ControlList &controls)\n> > > }\n> > > }\n> > >\n> > > + /*\n> > > + * We must also handle any AWB on/off changes first, so that the CCM algorithm\n> > > + * knows its state correctly.\n> > > + */\n> > > + const auto awbEnable = controls.get(controls::AwbEnable);\n> > > + if (awbEnable)\n> > > + do {\n> > > + /* Silently ignore this control for a mono sensor. */\n> > > + if (monoSensor_)\n> > > + break;\n> > > +\n> > > + RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > > + controller_.getAlgorithm(\"awb\"));\n> > > + if (!awb) {\n> > > + LOG(IPARPI, Warning)\n> > > + << \"Could not set AWB_ENABLE - no AWB algorithm\";\n> > > + break;\n> > > + }\n> > > +\n> > > + awbEnabled_ = *awbEnable;\n> > > +\n> > > + if (!awbEnabled_)\n> > > + awb->disableAuto();\n> > > + else {\n> > > + awb->enableAuto();\n> > > +\n> > > + /* The CCM algorithm must go back to auto as well. */\n> > > + RPiController::CcmAlgorithm *ccm = dynamic_cast(\n> > > + controller_.getAlgorithm(\"ccm\"));\n> > > + if (ccm)\n> > > + ccm->enableAuto();\n> > > + }\n> > > +\n> > > + libcameraMetadata_.set(controls::AwbEnable, awbEnabled_);\n> > > + } while (false);\n> > > +\n> > > + const auto colourGains = controls.get(controls::ColourGains);\n> > > + if (colourGains)\n> > > + do {\n> > > + /* Silently ignore this control for a mono sensor. */\n> > > + if (monoSensor_)\n> > > + break;\n> > > +\n> > > + auto gains = *colourGains;\n> > > + RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > > + controller_.getAlgorithm(\"awb\"));\n> > > + if (!awb) {\n> > > + LOG(IPARPI, Warning)\n> > > + << \"Could not set COLOUR_GAINS - no AWB algorithm\";\n> > > + break;\n> > > + }\n> > > +\n> > > + awb->setManualGains(gains[0], gains[1]);\n> > > + if (gains[0] != 0.0f && gains[1] != 0.0f) {\n> > > + /* A gain of 0.0f will switch back to auto mode. */\n> > > + libcameraMetadata_.set(controls::ColourGains,\n> > > + { gains[0], gains[1] });\n> > > +\n> > > + awbEnabled_ = false; /* doing this puts AWB into manual mode */\n> > > + } else {\n> > > + awbEnabled_ = true; /* doing this puts AWB into auto mode */\n> > > +\n> > > + /* The CCM algorithm must go back to auto as well. */\n> > > + RPiController::CcmAlgorithm *ccm = dynamic_cast(\n> > > + controller_.getAlgorithm(\"ccm\"));\n> > > + if (ccm)\n> > > + ccm->enableAuto();\n> > > + }\n> > > +\n> > > + /* This metadata will get reported back automatically. */\n> > > + break;\n> > > + } while (false);\n> > > +\n> > > + const auto colourTemperature = controls.get(controls::ColourTemperature);\n> > > + if (colourTemperature)\n> > > + do {\n> > > + /* Silently ignore this control for a mono sensor. */\n> > > + if (monoSensor_)\n> > > + break;\n> > > +\n> > > + auto temperatureK = *colourTemperature;\n> > > + RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > > + controller_.getAlgorithm(\"awb\"));\n> > > + if (!awb) {\n> > > + LOG(IPARPI, Warning)\n> > > + << \"Could not set COLOUR_TEMPERATURE - no AWB algorithm\";\n> > > + break;\n> > > + }\n> > > +\n> > > + awb->setColourTemperature(temperatureK);\n> > > + awbEnabled_ = false; /* doing this puts AWB into manual mode */\n> > > +\n> > > + /* This metadata will get reported back automatically. */\n> > > + break;\n> > > + } while (false);\n> > > +\n> > > /* Iterate over controls */\n> > > for (auto const &ctrl : controls) {\n> > > LOG(IPARPI, Debug) << \"Request ctrl: \"\n> > > @@ -1037,25 +1134,7 @@ void IpaBase::applyControls(const ControlList &controls)\n> > > }\n> > >\n> > > case controls::AWB_ENABLE: {\n> > > - /* Silently ignore this control for a mono sensor. */\n> > > - if (monoSensor_)\n> > > - break;\n> > > -\n> > > - RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > > - controller_.getAlgorithm(\"awb\"));\n> > > - if (!awb) {\n> > > - LOG(IPARPI, Warning)\n> > > - << \"Could not set AWB_ENABLE - no AWB algorithm\";\n> > > - break;\n> > > - }\n> > > -\n> > > - if (ctrl.second.get() == false)\n> > > - awb->disableAuto();\n> > > - else\n> > > - awb->enableAuto();\n> > > -\n> > > - libcameraMetadata_.set(controls::AwbEnable,\n> > > - ctrl.second.get());\n> > > + /* We handled this one above. */\n> > > break;\n> > > }\n> > >\n> > > @@ -1080,47 +1159,60 @@ void IpaBase::applyControls(const ControlList &controls)\n> > > LOG(IPARPI, Error) << \"AWB mode \" << idx\n> > > << \" not recognised\";\n> > > }\n> > > +\n> > > break;\n> > > }\n> > >\n> > > case controls::COLOUR_GAINS: {\n> > > - /* Silently ignore this control for a mono sensor. */\n> > > - if (monoSensor_)\n> > > - break;\n> > > -\n> > > - auto gains = ctrl.second.get>();\n> > > - RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > > - controller_.getAlgorithm(\"awb\"));\n> > > - if (!awb) {\n> > > - LOG(IPARPI, Warning)\n> > > - << \"Could not set COLOUR_GAINS - no AWB algorithm\";\n> > > - break;\n> > > - }\n> > > -\n> > > - awb->setManualGains(gains[0], gains[1]);\n> > > - if (gains[0] != 0.0f && gains[1] != 0.0f)\n> > > - /* A gain of 0.0f will switch back to auto mode. */\n> > > - libcameraMetadata_.set(controls::ColourGains,\n> > > - { gains[0], gains[1] });\n> > > + /* We handled this one above. */\n> > > break;\n> > > }\n> > >\n> > > case controls::COLOUR_TEMPERATURE: {\n> > > - /* Silently ignore this control for a mono sensor. */\n> > > + /* We handled this one above. */\n> > > + break;\n> > > + }\n> > > +\n> > > + case controls::COLOUR_CORRECTION_MATRIX: {\n> > > if (monoSensor_)\n> > > break;\n> > >\n> > > - auto temperatureK = ctrl.second.get();\n> > > + auto floats = ctrl.second.get>();\n> > > + RPiController::CcmAlgorithm *ccm = dynamic_cast(\n> > > + controller_.getAlgorithm(\"ccm\"));\n> > > + if (!ccm) {\n> > > + LOG(IPARPI, Warning)\n> > > + << \"Could not set COLOUR_CORRECTION_MATRIX - no CCM algorithm\";\n> > > + break;\n> > > + }\n> > > +\n> > > RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > > controller_.getAlgorithm(\"awb\"));\n> > > - if (!awb) {\n> > > + if (awb && awbEnabled_) {\n> > > LOG(IPARPI, Warning)\n> > > - << \"Could not set COLOUR_TEMPERATURE - no AWB algorithm\";\n> > > + << \"Could not set COLOUR_CORRECTION_MATRIX - AWB is active\";\n> > > break;\n> > > }\n> > >\n> > > - awb->setColourTemperature(temperatureK);\n> > > - /* This metadata will get reported back automatically. */\n> > > + /* We are guaranteed this control contains 9 values. Nevertheless: */\n> > > + assert(floats.size() == 9);\n> > > +\n> > > + Matrix matrix;\n> > > + for (std::size_t i = 0; i < 3; ++i)\n> > > + for (std::size_t j = 0; j < 3; ++j)\n> > > + matrix[i][j] = static_cast(floats[i * 3 + j]);\n> > > +\n> > > + ccm->setCcm(matrix);\n> > > +\n> > > + /*\n> > > + * But if AWB is running, go back to auto mode. The CCM gets remembered,\n> > > + * which avoids the race between setting the CCM and disabling AWB in\n> > > + * the same set of controls.\n> > > + */\n> > > + if (awbEnabled_)\n> > > + ccm->enableAuto();\n> > > +\n> > > + /* This metadata will be reported back automatically. */\n> > > break;\n> > > }\n> > >\n> > > diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h\n> > > index e2f6e330..5348f2ea 100644\n> > > --- a/src/ipa/rpi/common/ipa_base.h\n> > > +++ b/src/ipa/rpi/common/ipa_base.h\n> > > @@ -140,6 +140,8 @@ private:\n> > > int32_t mode;\n> > > utils::Duration manualPeriod;\n> > > } flickerState_;\n> > > +\n> > > + bool awbEnabled_;\n> > > };\n> > >\n> > > } /* namespace ipa::RPi */\n> > > diff --git a/src/ipa/rpi/controller/ccm_algorithm.h b/src/ipa/rpi/controller/ccm_algorithm.h\n> > > index 6678ba75..785c408a 100644\n> > > --- a/src/ipa/rpi/controller/ccm_algorithm.h\n> > > +++ b/src/ipa/rpi/controller/ccm_algorithm.h\n> > > @@ -6,16 +6,22 @@\n> > > */\n> > > #pragma once\n> > >\n> > > +#include \"libcamera/internal/matrix.h\"\n> > > +\n> > > #include \"algorithm.h\"\n> > >\n> > > namespace RPiController {\n> > >\n> > > +using Matrix3x3 = libcamera::Matrix;\n> > > +\n> > > class CcmAlgorithm : public Algorithm\n> > > {\n> > > public:\n> > > CcmAlgorithm(Controller *controller) : Algorithm(controller) {}\n> > > /* A CCM algorithm must provide the following: */\n> > > + virtual void enableAuto() = 0;\n> > > virtual void setSaturation(double saturation) = 0;\n> > > + virtual void setCcm(Matrix3x3 const &matrix) = 0;\n> > > };\n> > >\n> > > } /* namespace RPiController */\n> > > diff --git a/src/ipa/rpi/controller/rpi/ccm.cpp b/src/ipa/rpi/controller/rpi/ccm.cpp\n> > > index 8607f152..c42c0141 100644\n> > > --- a/src/ipa/rpi/controller/rpi/ccm.cpp\n> > > +++ b/src/ipa/rpi/controller/rpi/ccm.cpp\n> > > @@ -32,7 +32,10 @@ LOG_DEFINE_CATEGORY(RPiCcm)\n> > > using Matrix3x3 = Matrix;\n> > >\n> > > Ccm::Ccm(Controller *controller)\n> > > - : CcmAlgorithm(controller), saturation_(1.0) {}\n> > > + : CcmAlgorithm(controller), enableAuto_(true), saturation_(1.0),\n> > > + manualCcm_({ 1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0 })\n> > > +{\n> > > +}\n> > >\n> > > char const *Ccm::name() const\n> > > {\n> > > @@ -78,11 +81,22 @@ int Ccm::read(const libcamera::YamlObject ¶ms)\n> > > return 0;\n> > > }\n> > >\n> > > +void Ccm::enableAuto()\n> > > +{\n> > > + enableAuto_ = true;\n> > > +}\n> > > +\n> > > void Ccm::setSaturation(double saturation)\n> > > {\n> > > saturation_ = saturation;\n> > > }\n> > >\n> > > +void Ccm::setCcm(Matrix3x3 const &matrix)\n> > > +{\n> > > + enableAuto_ = false;\n> > > + manualCcm_ = matrix;\n> > > +}\n> > > +\n> > > void Ccm::initialise()\n> > > {\n> > > }\n> > > @@ -151,7 +165,13 @@ void Ccm::prepare(Metadata *imageMetadata)\n> > > LOG(RPiCcm, Warning) << \"no colour temperature found\";\n> > > if (!luxOk)\n> > > LOG(RPiCcm, Warning) << \"no lux value found\";\n> > > - Matrix3x3 ccm = calculateCcm(config_.ccms, awb.temperatureK);\n> > > +\n> > > + Matrix3x3 ccm;\n> > > + if (enableAuto_)\n> > > + ccm = calculateCcm(config_.ccms, awb.temperatureK);\n> > > + else\n> > > + ccm = manualCcm_;\n> > > +\n> > > double saturation = saturation_;\n> > > struct CcmStatus ccmStatus;\n> > > ccmStatus.saturation = saturation;\n> > > diff --git a/src/ipa/rpi/controller/rpi/ccm.h b/src/ipa/rpi/controller/rpi/ccm.h\n> > > index c05dbb17..70f28ed3 100644\n> > > --- a/src/ipa/rpi/controller/rpi/ccm.h\n> > > +++ b/src/ipa/rpi/controller/rpi/ccm.h\n> > > @@ -8,7 +8,6 @@\n> > >\n> > > #include \n> > >\n> > > -#include \"libcamera/internal/matrix.h\"\n> > > #include \n> > >\n> > > #include \"../ccm_algorithm.h\"\n> > > @@ -33,13 +32,17 @@ public:\n> > > Ccm(Controller *controller = NULL);\n> > > char const *name() const override;\n> > > int read(const libcamera::YamlObject ¶ms) override;\n> > > + void enableAuto() override;\n> > > void setSaturation(double saturation) override;\n> > > + void setCcm(Matrix3x3 const &matrix) override;\n> > > void initialise() override;\n> > > void prepare(Metadata *imageMetadata) override;\n> > >\n> > > private:\n> > > CcmConfig config_;\n> > > + bool enableAuto_;\n> > > double saturation_;\n> > > + Matrix3x3 manualCcm_;\n> > > };\n> > >\n> > > } /* namespace RPiController */\n> > > --\n> > > 2.39.5\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 C1D28BEFBE\n\tfor ;\n\tMon, 11 Aug 2025 10:55:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AB39D6921F;\n\tMon, 11 Aug 2025 12:55:34 +0200 (CEST)","from mail-qt1-x82e.google.com (mail-qt1-x82e.google.com\n\t[IPv6:2607:f8b0:4864:20::82e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8A7E66921A\n\tfor ;\n\tMon, 11 Aug 2025 12:55:32 +0200 (CEST)","by mail-qt1-x82e.google.com with SMTP id\n\td75a77b69052e-4b075cd3cdbso71143781cf.2\n\tfor ;\n\tMon, 11 Aug 2025 03:55:32 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"Mko5UMt6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1754909731; x=1755514531;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=7emoIXXli9WZLuVOKSvqcEcWlditnsbojnEeJp4e8FA=;\n\tb=Mko5UMt6OydB2yNfRkhVqhZHL3ZBA89G+hE2NfSpUPY4SMkdpueq9meiRalScXtCbJ\n\tHQFAzehLOXfuX8avtUTo5XKkBm6QSFCIjA5YLDFPvyKvVEvzg0HFxwFXH4dTXyaOukKw\n\tKqmpKBkke3TdZOKYS8aidUhDvHhvAkrSS+iHIb9eQx//UaXsfBohXlLekW2tb328YBBY\n\tefgannLbRTGhmAh3HrJz/+h5X8sBVr37/waRQ+7MGS6j143aBMnSMPX14pjlY6j58hhE\n\tkYsQp2+Nue0vCPQAxtVv21DaaJA9BYqGkkd/O4cNuRs/HbxycZOQERHvRdtYnGSDchqA\n\tQSKw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1754909731; x=1755514531;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=7emoIXXli9WZLuVOKSvqcEcWlditnsbojnEeJp4e8FA=;\n\tb=fMXlSVlJdZo/im0MMn09CgTERItCwRVok5vi0tGwWlHyM/3vc7M7vg/vxlwcc6ACdC\n\tvQZ28HcAYa0wnk+d1wet0rCpYcPtjsyUqzE10TmTIvGicLpv9umf4hiOIYL/sYY8QQW4\n\tlvUakIZCmxYbm5HjRagaziFAdNP3RfgYYEb4UkXxYtEYtkl3W+nxxWB6QkT2ukmZSuud\n\tOn+wZWD8mYBDRA//5+MhoG4lJTVimqRs8VcxZRrH9p/v1sbuGWiBGS80UbyjsW+j1+/M\n\tviQfX6g9UUWhtTgI1u1aRKw/ee6BuaB+xmAE8l/qFWGpINAHUjGdJsq1xHsXS4k2JyKZ\n\tdlrA==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCVVIIhJZr58Q2cQzEJ2E31hNVBowb3xj+jvV1d4Gvb7llEe4MNondJT12QPajGfLGwMhYt8fZJZHPQnvA/Ov4E=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YwBqt3XiMJs0ynjsV+Rv33uCUfulEqnYvmyBgiyrG8dwFhIfrRl\n\tC/Fg0q9pPfjaIAZuZBJ86iXRyHyuCaYnRwmIJcDCj7lb6EmTVyCCdCG98tlHi2OOIPKU+FKhNgJ\n\tQdlP6gqKITUI6puS4xSmSJOqrcBkDjRV3df7oqhwSRg==","X-Gm-Gg":"ASbGncswZi796iaAJBqCKIHOIIi/q5H6wQ48oRxoVek42oK78ANc2EMPa6Cccb2TeZH\n\tHSirBSRNg5xe2Ot5tD4YtHA7iBDEs5mmXJCai/aXz6oiKc7/ceWh3iG//iAb+rRDfzO7YPCmCaa\n\taSBLYXcYDmBgJ2p0zLrooiz6hFI269tfuU+/tnL9yLyWq+91ndbsfUonV39KdsaoR9cQWYeAMUi\n\tg9L+2omyJjagUfSj5xlGN1TDP+4mz+xsc+7gMw=","X-Google-Smtp-Source":"AGHT+IFcIzUz7vP3X6TjJe9ZRiusRCH8jibbPOEZJ8AGqN7+Jti8RVqhFEBhS0RsCU3QOsoGHD0DP9+VzMb/Kcq2SoY=","X-Received":"by 2002:a05:620a:19a7:b0:7e8:1bcb:399 with SMTP id\n\taf79cd13be357-7e82c7d1db8mr1726001085a.63.1754909730941;\n\tMon, 11 Aug 2025 03:55:30 -0700 (PDT)","MIME-Version":"1.0","References":"<20250804144804.16965-1-david.plowman@raspberrypi.com>\n\t\n\t<175490506503.560048.9977274206332933623@ping.linuxembedded.co.uk>","In-Reply-To":"<175490506503.560048.9977274206332933623@ping.linuxembedded.co.uk>","From":"David Plowman ","Date":"Mon, 11 Aug 2025 11:55:20 +0100","X-Gm-Features":"Ac12FXz9N0hkwCLNpFIreH7Xvf3ci_EUcO47PIf_ZPBSCS2EyuS432EXFDLY2Vs","Message-ID":"","Subject":"Re: [PATCH v2] ipa: rpi: ccm: Implement \"manual\" CCM mode","To":"Kieran Bingham ","Cc":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","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":35334,"web_url":"https://patchwork.libcamera.org/comment/35334/","msgid":"<175491114899.2641331.13553993291203285817@ping.linuxembedded.co.uk>","date":"2025-08-11T11:19:08","subject":"Re: [PATCH v2] ipa: rpi: ccm: Implement \"manual\" CCM mode","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting David Plowman (2025-08-11 11:55:20)\n> Hi Kieran\n> \n> Well, I don't think it's worth worrying about too much because I\n> believe I've implemented what the documentation describes.\n> \n> The point was merely that the documentation doesn't describe a\n> behaviour that's super convenient for us. In our world, the CCM and\n> AWB algorithm are separate things and don't actually know about each\n\nThat's the part I worry about ;-)\n\n> other, whereas the documented behaviour grabs the \"AWB enable\" and\n> uses that to put the CCM into automatic or manual mode too. TBH, I\n> can't see why it would be disallowed to set a manual CCM even when AWB\n> is running, but there you go.\n> \n> v1 of the patch made a bit of a pig's ear of the state that I'd added\n> to couple these two algorithms together. But whatever, I think v2 is\n> correct and I don't really feel motivated to change anything!!\n\nOk no problem! lets go with v2 as it is then!\n\n--\nKieran\n\n> \n> Thanks\n> David\n> \n> On Mon, 11 Aug 2025 at 10:37, Kieran Bingham\n> wrote:\n> >\n> > Quoting Naushir Patuck (2025-08-11 10:05:51)\n> > > Hi David,\n> > >\n> > > On Mon, 4 Aug 2025 at 15:48, David Plowman\n> > > wrote:\n> > > >\n> > > > The CCM algorithm will now let an explicit colour matrix be set when\n> > > > AWB is in manual mode.\n> > > >\n> > > > We must handle any controls that can cause the AWB to be enabled or\n> > > > disabled first, so that we know the AWB's state correctly when we come\n> > > > to set the CCM.\n> > > >\n> > > > Signed-off-by: David Plowman \n> > >\n> > > Looks reasonable to me.\n> > >\n> > > Reviewed-by: Naushir Patuck \n> >\n> > Happy to merge whenever you like here.\n> >\n> > I'm curious - v1 said something about the current documentation not\n> > giving the desired behavour.\n> >\n> > Should we change the documentation or update the expected behaviour? or\n> > are you ok with this one ?\n> >\n> > (I expect we can still merge this one and discuss separately)\n> > --\n> > Kieran\n> >\n> >\n> > >\n> > > > ---\n> > > > src/ipa/rpi/common/ipa_base.cpp | 180 +++++++++++++++++++------\n> > > > src/ipa/rpi/common/ipa_base.h | 2 +\n> > > > src/ipa/rpi/controller/ccm_algorithm.h | 6 +\n> > > > src/ipa/rpi/controller/rpi/ccm.cpp | 24 +++-\n> > > > src/ipa/rpi/controller/rpi/ccm.h | 5 +-\n> > > > 5 files changed, 170 insertions(+), 47 deletions(-)\n> > > >\n> > > > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> > > > index ce2343e9..6448e6ab 100644\n> > > > --- a/src/ipa/rpi/common/ipa_base.cpp\n> > > > +++ b/src/ipa/rpi/common/ipa_base.cpp\n> > > > @@ -94,6 +94,7 @@ const ControlInfoMap::Map ipaColourControls{\n> > > > { &controls::AwbEnable, ControlInfo(false, true) },\n> > > > { &controls::AwbMode, ControlInfo(controls::AwbModeValues) },\n> > > > { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },\n> > > > + { &controls::ColourCorrectionMatrix, ControlInfo(0.0f, 8.0f) },\n> > > > { &controls::ColourTemperature, ControlInfo(100, 100000) },\n> > > > { &controls::Saturation, ControlInfo(0.0f, 32.0f, 1.0f) },\n> > > > };\n> > > > @@ -126,7 +127,7 @@ namespace ipa::RPi {\n> > > > IpaBase::IpaBase()\n> > > > : controller_(), frameLengths_(FrameLengthsQueueSize, 0s), statsMetadataOutput_(false),\n> > > > stitchSwapBuffers_(false), frameCount_(0), mistrustCount_(0), lastRunTimestamp_(0),\n> > > > - firstStart_(true), flickerState_({ 0, 0s })\n> > > > + firstStart_(true), flickerState_({ 0, 0s }), awbEnabled_(true)\n> > > > {\n> > > > }\n> > > >\n> > > > @@ -821,6 +822,102 @@ void IpaBase::applyControls(const ControlList &controls)\n> > > > }\n> > > > }\n> > > >\n> > > > + /*\n> > > > + * We must also handle any AWB on/off changes first, so that the CCM algorithm\n> > > > + * knows its state correctly.\n> > > > + */\n> > > > + const auto awbEnable = controls.get(controls::AwbEnable);\n> > > > + if (awbEnable)\n> > > > + do {\n> > > > + /* Silently ignore this control for a mono sensor. */\n> > > > + if (monoSensor_)\n> > > > + break;\n> > > > +\n> > > > + RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > > > + controller_.getAlgorithm(\"awb\"));\n> > > > + if (!awb) {\n> > > > + LOG(IPARPI, Warning)\n> > > > + << \"Could not set AWB_ENABLE - no AWB algorithm\";\n> > > > + break;\n> > > > + }\n> > > > +\n> > > > + awbEnabled_ = *awbEnable;\n> > > > +\n> > > > + if (!awbEnabled_)\n> > > > + awb->disableAuto();\n> > > > + else {\n> > > > + awb->enableAuto();\n> > > > +\n> > > > + /* The CCM algorithm must go back to auto as well. */\n> > > > + RPiController::CcmAlgorithm *ccm = dynamic_cast(\n> > > > + controller_.getAlgorithm(\"ccm\"));\n> > > > + if (ccm)\n> > > > + ccm->enableAuto();\n> > > > + }\n> > > > +\n> > > > + libcameraMetadata_.set(controls::AwbEnable, awbEnabled_);\n> > > > + } while (false);\n> > > > +\n> > > > + const auto colourGains = controls.get(controls::ColourGains);\n> > > > + if (colourGains)\n> > > > + do {\n> > > > + /* Silently ignore this control for a mono sensor. */\n> > > > + if (monoSensor_)\n> > > > + break;\n> > > > +\n> > > > + auto gains = *colourGains;\n> > > > + RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > > > + controller_.getAlgorithm(\"awb\"));\n> > > > + if (!awb) {\n> > > > + LOG(IPARPI, Warning)\n> > > > + << \"Could not set COLOUR_GAINS - no AWB algorithm\";\n> > > > + break;\n> > > > + }\n> > > > +\n> > > > + awb->setManualGains(gains[0], gains[1]);\n> > > > + if (gains[0] != 0.0f && gains[1] != 0.0f) {\n> > > > + /* A gain of 0.0f will switch back to auto mode. */\n> > > > + libcameraMetadata_.set(controls::ColourGains,\n> > > > + { gains[0], gains[1] });\n> > > > +\n> > > > + awbEnabled_ = false; /* doing this puts AWB into manual mode */\n> > > > + } else {\n> > > > + awbEnabled_ = true; /* doing this puts AWB into auto mode */\n> > > > +\n> > > > + /* The CCM algorithm must go back to auto as well. */\n> > > > + RPiController::CcmAlgorithm *ccm = dynamic_cast(\n> > > > + controller_.getAlgorithm(\"ccm\"));\n> > > > + if (ccm)\n> > > > + ccm->enableAuto();\n> > > > + }\n> > > > +\n> > > > + /* This metadata will get reported back automatically. */\n> > > > + break;\n> > > > + } while (false);\n> > > > +\n> > > > + const auto colourTemperature = controls.get(controls::ColourTemperature);\n> > > > + if (colourTemperature)\n> > > > + do {\n> > > > + /* Silently ignore this control for a mono sensor. */\n> > > > + if (monoSensor_)\n> > > > + break;\n> > > > +\n> > > > + auto temperatureK = *colourTemperature;\n> > > > + RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > > > + controller_.getAlgorithm(\"awb\"));\n> > > > + if (!awb) {\n> > > > + LOG(IPARPI, Warning)\n> > > > + << \"Could not set COLOUR_TEMPERATURE - no AWB algorithm\";\n> > > > + break;\n> > > > + }\n> > > > +\n> > > > + awb->setColourTemperature(temperatureK);\n> > > > + awbEnabled_ = false; /* doing this puts AWB into manual mode */\n> > > > +\n> > > > + /* This metadata will get reported back automatically. */\n> > > > + break;\n> > > > + } while (false);\n> > > > +\n> > > > /* Iterate over controls */\n> > > > for (auto const &ctrl : controls) {\n> > > > LOG(IPARPI, Debug) << \"Request ctrl: \"\n> > > > @@ -1037,25 +1134,7 @@ void IpaBase::applyControls(const ControlList &controls)\n> > > > }\n> > > >\n> > > > case controls::AWB_ENABLE: {\n> > > > - /* Silently ignore this control for a mono sensor. */\n> > > > - if (monoSensor_)\n> > > > - break;\n> > > > -\n> > > > - RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > > > - controller_.getAlgorithm(\"awb\"));\n> > > > - if (!awb) {\n> > > > - LOG(IPARPI, Warning)\n> > > > - << \"Could not set AWB_ENABLE - no AWB algorithm\";\n> > > > - break;\n> > > > - }\n> > > > -\n> > > > - if (ctrl.second.get() == false)\n> > > > - awb->disableAuto();\n> > > > - else\n> > > > - awb->enableAuto();\n> > > > -\n> > > > - libcameraMetadata_.set(controls::AwbEnable,\n> > > > - ctrl.second.get());\n> > > > + /* We handled this one above. */\n> > > > break;\n> > > > }\n> > > >\n> > > > @@ -1080,47 +1159,60 @@ void IpaBase::applyControls(const ControlList &controls)\n> > > > LOG(IPARPI, Error) << \"AWB mode \" << idx\n> > > > << \" not recognised\";\n> > > > }\n> > > > +\n> > > > break;\n> > > > }\n> > > >\n> > > > case controls::COLOUR_GAINS: {\n> > > > - /* Silently ignore this control for a mono sensor. */\n> > > > - if (monoSensor_)\n> > > > - break;\n> > > > -\n> > > > - auto gains = ctrl.second.get>();\n> > > > - RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > > > - controller_.getAlgorithm(\"awb\"));\n> > > > - if (!awb) {\n> > > > - LOG(IPARPI, Warning)\n> > > > - << \"Could not set COLOUR_GAINS - no AWB algorithm\";\n> > > > - break;\n> > > > - }\n> > > > -\n> > > > - awb->setManualGains(gains[0], gains[1]);\n> > > > - if (gains[0] != 0.0f && gains[1] != 0.0f)\n> > > > - /* A gain of 0.0f will switch back to auto mode. */\n> > > > - libcameraMetadata_.set(controls::ColourGains,\n> > > > - { gains[0], gains[1] });\n> > > > + /* We handled this one above. */\n> > > > break;\n> > > > }\n> > > >\n> > > > case controls::COLOUR_TEMPERATURE: {\n> > > > - /* Silently ignore this control for a mono sensor. */\n> > > > + /* We handled this one above. */\n> > > > + break;\n> > > > + }\n> > > > +\n> > > > + case controls::COLOUR_CORRECTION_MATRIX: {\n> > > > if (monoSensor_)\n> > > > break;\n> > > >\n> > > > - auto temperatureK = ctrl.second.get();\n> > > > + auto floats = ctrl.second.get>();\n> > > > + RPiController::CcmAlgorithm *ccm = dynamic_cast(\n> > > > + controller_.getAlgorithm(\"ccm\"));\n> > > > + if (!ccm) {\n> > > > + LOG(IPARPI, Warning)\n> > > > + << \"Could not set COLOUR_CORRECTION_MATRIX - no CCM algorithm\";\n> > > > + break;\n> > > > + }\n> > > > +\n> > > > RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > > > controller_.getAlgorithm(\"awb\"));\n> > > > - if (!awb) {\n> > > > + if (awb && awbEnabled_) {\n> > > > LOG(IPARPI, Warning)\n> > > > - << \"Could not set COLOUR_TEMPERATURE - no AWB algorithm\";\n> > > > + << \"Could not set COLOUR_CORRECTION_MATRIX - AWB is active\";\n> > > > break;\n> > > > }\n> > > >\n> > > > - awb->setColourTemperature(temperatureK);\n> > > > - /* This metadata will get reported back automatically. */\n> > > > + /* We are guaranteed this control contains 9 values. Nevertheless: */\n> > > > + assert(floats.size() == 9);\n> > > > +\n> > > > + Matrix matrix;\n> > > > + for (std::size_t i = 0; i < 3; ++i)\n> > > > + for (std::size_t j = 0; j < 3; ++j)\n> > > > + matrix[i][j] = static_cast(floats[i * 3 + j]);\n> > > > +\n> > > > + ccm->setCcm(matrix);\n> > > > +\n> > > > + /*\n> > > > + * But if AWB is running, go back to auto mode. The CCM gets remembered,\n> > > > + * which avoids the race between setting the CCM and disabling AWB in\n> > > > + * the same set of controls.\n> > > > + */\n> > > > + if (awbEnabled_)\n> > > > + ccm->enableAuto();\n> > > > +\n> > > > + /* This metadata will be reported back automatically. */\n> > > > break;\n> > > > }\n> > > >\n> > > > diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h\n> > > > index e2f6e330..5348f2ea 100644\n> > > > --- a/src/ipa/rpi/common/ipa_base.h\n> > > > +++ b/src/ipa/rpi/common/ipa_base.h\n> > > > @@ -140,6 +140,8 @@ private:\n> > > > int32_t mode;\n> > > > utils::Duration manualPeriod;\n> > > > } flickerState_;\n> > > > +\n> > > > + bool awbEnabled_;\n> > > > };\n> > > >\n> > > > } /* namespace ipa::RPi */\n> > > > diff --git a/src/ipa/rpi/controller/ccm_algorithm.h b/src/ipa/rpi/controller/ccm_algorithm.h\n> > > > index 6678ba75..785c408a 100644\n> > > > --- a/src/ipa/rpi/controller/ccm_algorithm.h\n> > > > +++ b/src/ipa/rpi/controller/ccm_algorithm.h\n> > > > @@ -6,16 +6,22 @@\n> > > > */\n> > > > #pragma once\n> > > >\n> > > > +#include \"libcamera/internal/matrix.h\"\n> > > > +\n> > > > #include \"algorithm.h\"\n> > > >\n> > > > namespace RPiController {\n> > > >\n> > > > +using Matrix3x3 = libcamera::Matrix;\n> > > > +\n> > > > class CcmAlgorithm : public Algorithm\n> > > > {\n> > > > public:\n> > > > CcmAlgorithm(Controller *controller) : Algorithm(controller) {}\n> > > > /* A CCM algorithm must provide the following: */\n> > > > + virtual void enableAuto() = 0;\n> > > > virtual void setSaturation(double saturation) = 0;\n> > > > + virtual void setCcm(Matrix3x3 const &matrix) = 0;\n> > > > };\n> > > >\n> > > > } /* namespace RPiController */\n> > > > diff --git a/src/ipa/rpi/controller/rpi/ccm.cpp b/src/ipa/rpi/controller/rpi/ccm.cpp\n> > > > index 8607f152..c42c0141 100644\n> > > > --- a/src/ipa/rpi/controller/rpi/ccm.cpp\n> > > > +++ b/src/ipa/rpi/controller/rpi/ccm.cpp\n> > > > @@ -32,7 +32,10 @@ LOG_DEFINE_CATEGORY(RPiCcm)\n> > > > using Matrix3x3 = Matrix;\n> > > >\n> > > > Ccm::Ccm(Controller *controller)\n> > > > - : CcmAlgorithm(controller), saturation_(1.0) {}\n> > > > + : CcmAlgorithm(controller), enableAuto_(true), saturation_(1.0),\n> > > > + manualCcm_({ 1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0 })\n> > > > +{\n> > > > +}\n> > > >\n> > > > char const *Ccm::name() const\n> > > > {\n> > > > @@ -78,11 +81,22 @@ int Ccm::read(const libcamera::YamlObject ¶ms)\n> > > > return 0;\n> > > > }\n> > > >\n> > > > +void Ccm::enableAuto()\n> > > > +{\n> > > > + enableAuto_ = true;\n> > > > +}\n> > > > +\n> > > > void Ccm::setSaturation(double saturation)\n> > > > {\n> > > > saturation_ = saturation;\n> > > > }\n> > > >\n> > > > +void Ccm::setCcm(Matrix3x3 const &matrix)\n> > > > +{\n> > > > + enableAuto_ = false;\n> > > > + manualCcm_ = matrix;\n> > > > +}\n> > > > +\n> > > > void Ccm::initialise()\n> > > > {\n> > > > }\n> > > > @@ -151,7 +165,13 @@ void Ccm::prepare(Metadata *imageMetadata)\n> > > > LOG(RPiCcm, Warning) << \"no colour temperature found\";\n> > > > if (!luxOk)\n> > > > LOG(RPiCcm, Warning) << \"no lux value found\";\n> > > > - Matrix3x3 ccm = calculateCcm(config_.ccms, awb.temperatureK);\n> > > > +\n> > > > + Matrix3x3 ccm;\n> > > > + if (enableAuto_)\n> > > > + ccm = calculateCcm(config_.ccms, awb.temperatureK);\n> > > > + else\n> > > > + ccm = manualCcm_;\n> > > > +\n> > > > double saturation = saturation_;\n> > > > struct CcmStatus ccmStatus;\n> > > > ccmStatus.saturation = saturation;\n> > > > diff --git a/src/ipa/rpi/controller/rpi/ccm.h b/src/ipa/rpi/controller/rpi/ccm.h\n> > > > index c05dbb17..70f28ed3 100644\n> > > > --- a/src/ipa/rpi/controller/rpi/ccm.h\n> > > > +++ b/src/ipa/rpi/controller/rpi/ccm.h\n> > > > @@ -8,7 +8,6 @@\n> > > >\n> > > > #include \n> > > >\n> > > > -#include \"libcamera/internal/matrix.h\"\n> > > > #include \n> > > >\n> > > > #include \"../ccm_algorithm.h\"\n> > > > @@ -33,13 +32,17 @@ public:\n> > > > Ccm(Controller *controller = NULL);\n> > > > char const *name() const override;\n> > > > int read(const libcamera::YamlObject ¶ms) override;\n> > > > + void enableAuto() override;\n> > > > void setSaturation(double saturation) override;\n> > > > + void setCcm(Matrix3x3 const &matrix) override;\n> > > > void initialise() override;\n> > > > void prepare(Metadata *imageMetadata) override;\n> > > >\n> > > > private:\n> > > > CcmConfig config_;\n> > > > + bool enableAuto_;\n> > > > double saturation_;\n> > > > + Matrix3x3 manualCcm_;\n> > > > };\n> > > >\n> > > > } /* namespace RPiController */\n> > > > --\n> > > > 2.39.5\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 098FFBDCC1\n\tfor ;\n\tMon, 11 Aug 2025 11:19:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 13AE36921F;\n\tMon, 11 Aug 2025 13:19:13 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4437C6921A\n\tfor ;\n\tMon, 11 Aug 2025 13:19:11 +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 293BB82A;\n\tMon, 11 Aug 2025 13:18:19 +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=\"U0iBYZkT\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1754911099;\n\tbh=oaAzcKqKfTWW5TsjjoDPh8iEg+t6Vg2pP0jCNFs8tTY=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=U0iBYZkT0U+VxhIEVQo+KqEOuSiYrbwWF0lqRjZ7E1qxSWp4qVu/JFtaWH5PG1QmO\n\tDmVAATNI0vxtjD7thN0QUu6ZEazgscZkeQBt5gIPWcgG1vg1WvbphSnX7rZXLlST5O\n\tQb6RlX+xswbJI+9iAY85BhluYHM7rqKn66Yw2zzA=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20250804144804.16965-1-david.plowman@raspberrypi.com>\n\t\n\t<175490506503.560048.9977274206332933623@ping.linuxembedded.co.uk>\n\t","Subject":"Re: [PATCH v2] ipa: rpi: ccm: Implement \"manual\" CCM mode","From":"Kieran Bingham ","Cc":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","To":"David Plowman ","Date":"Mon, 11 Aug 2025 12:19:08 +0100","Message-ID":"<175491114899.2641331.13553993291203285817@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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":35653,"web_url":"https://patchwork.libcamera.org/comment/35653/","msgid":"<175671422696.1598056.14322765312609630863@ping.linuxembedded.co.uk>","date":"2025-09-01T08:10:26","subject":"Re: [PATCH v2] ipa: rpi: ccm: Implement \"manual\" CCM mode","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting David Plowman (2025-08-04 15:48:04)\n> The CCM algorithm will now let an explicit colour matrix be set when\n> AWB is in manual mode.\n> \n> We must handle any controls that can cause the AWB to be enabled or\n> disabled first, so that we know the AWB's state correctly when we come\n> to set the CCM.\n> \n> Signed-off-by: David Plowman \n\nUnfortunatly the CI blocked this patch.\n\nThe following addition gets it through.\n\n---------------------- src/ipa/rpi/controller/rpi/ccm.cpp ----------------------\nindex c42c014182ca..2806b4967158 100644\n@@ -27,12 +27,10 @@ LOG_DEFINE_CATEGORY(RPiCcm)\n * saturation setting that is exposed to applications.\n */\n\n #define NAME \"rpi.ccm\"\n\n-using Matrix3x3 = Matrix;\n-\n Ccm::Ccm(Controller *controller)\n \t: CcmAlgorithm(controller), enableAuto_(true), saturation_(1.0),\n \t manualCcm_({ 1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0 })\n {\n }\n\n\nAs the Matrix3x3 type is now defined in the header src/ipa/rpi/controller/ccm_algorithm.h\n\n\nIf that's fine - I'll merge with that squashed in.\n\n--\nKieran\n\n\n> ---\n> src/ipa/rpi/common/ipa_base.cpp | 180 +++++++++++++++++++------\n> src/ipa/rpi/common/ipa_base.h | 2 +\n> src/ipa/rpi/controller/ccm_algorithm.h | 6 +\n> src/ipa/rpi/controller/rpi/ccm.cpp | 24 +++-\n> src/ipa/rpi/controller/rpi/ccm.h | 5 +-\n> 5 files changed, 170 insertions(+), 47 deletions(-)\n> \n> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> index ce2343e9..6448e6ab 100644\n> --- a/src/ipa/rpi/common/ipa_base.cpp\n> +++ b/src/ipa/rpi/common/ipa_base.cpp\n> @@ -94,6 +94,7 @@ const ControlInfoMap::Map ipaColourControls{\n> { &controls::AwbEnable, ControlInfo(false, true) },\n> { &controls::AwbMode, ControlInfo(controls::AwbModeValues) },\n> { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },\n> + { &controls::ColourCorrectionMatrix, ControlInfo(0.0f, 8.0f) },\n> { &controls::ColourTemperature, ControlInfo(100, 100000) },\n> { &controls::Saturation, ControlInfo(0.0f, 32.0f, 1.0f) },\n> };\n> @@ -126,7 +127,7 @@ namespace ipa::RPi {\n> IpaBase::IpaBase()\n> : controller_(), frameLengths_(FrameLengthsQueueSize, 0s), statsMetadataOutput_(false),\n> stitchSwapBuffers_(false), frameCount_(0), mistrustCount_(0), lastRunTimestamp_(0),\n> - firstStart_(true), flickerState_({ 0, 0s })\n> + firstStart_(true), flickerState_({ 0, 0s }), awbEnabled_(true)\n> {\n> }\n> \n> @@ -821,6 +822,102 @@ void IpaBase::applyControls(const ControlList &controls)\n> }\n> }\n> \n> + /*\n> + * We must also handle any AWB on/off changes first, so that the CCM algorithm\n> + * knows its state correctly.\n> + */\n> + const auto awbEnable = controls.get(controls::AwbEnable);\n> + if (awbEnable)\n> + do {\n> + /* Silently ignore this control for a mono sensor. */\n> + if (monoSensor_)\n> + break;\n> +\n> + RPiController::AwbAlgorithm *awb = dynamic_cast(\n> + controller_.getAlgorithm(\"awb\"));\n> + if (!awb) {\n> + LOG(IPARPI, Warning)\n> + << \"Could not set AWB_ENABLE - no AWB algorithm\";\n> + break;\n> + }\n> +\n> + awbEnabled_ = *awbEnable;\n> +\n> + if (!awbEnabled_)\n> + awb->disableAuto();\n> + else {\n> + awb->enableAuto();\n> +\n> + /* The CCM algorithm must go back to auto as well. */\n> + RPiController::CcmAlgorithm *ccm = dynamic_cast(\n> + controller_.getAlgorithm(\"ccm\"));\n> + if (ccm)\n> + ccm->enableAuto();\n> + }\n> +\n> + libcameraMetadata_.set(controls::AwbEnable, awbEnabled_);\n> + } while (false);\n> +\n> + const auto colourGains = controls.get(controls::ColourGains);\n> + if (colourGains)\n> + do {\n> + /* Silently ignore this control for a mono sensor. */\n> + if (monoSensor_)\n> + break;\n> +\n> + auto gains = *colourGains;\n> + RPiController::AwbAlgorithm *awb = dynamic_cast(\n> + controller_.getAlgorithm(\"awb\"));\n> + if (!awb) {\n> + LOG(IPARPI, Warning)\n> + << \"Could not set COLOUR_GAINS - no AWB algorithm\";\n> + break;\n> + }\n> +\n> + awb->setManualGains(gains[0], gains[1]);\n> + if (gains[0] != 0.0f && gains[1] != 0.0f) {\n> + /* A gain of 0.0f will switch back to auto mode. */\n> + libcameraMetadata_.set(controls::ColourGains,\n> + { gains[0], gains[1] });\n> +\n> + awbEnabled_ = false; /* doing this puts AWB into manual mode */\n> + } else {\n> + awbEnabled_ = true; /* doing this puts AWB into auto mode */\n> +\n> + /* The CCM algorithm must go back to auto as well. */\n> + RPiController::CcmAlgorithm *ccm = dynamic_cast(\n> + controller_.getAlgorithm(\"ccm\"));\n> + if (ccm)\n> + ccm->enableAuto();\n> + }\n> +\n> + /* This metadata will get reported back automatically. */\n> + break;\n> + } while (false);\n> +\n> + const auto colourTemperature = controls.get(controls::ColourTemperature);\n> + if (colourTemperature)\n> + do {\n> + /* Silently ignore this control for a mono sensor. */\n> + if (monoSensor_)\n> + break;\n> +\n> + auto temperatureK = *colourTemperature;\n> + RPiController::AwbAlgorithm *awb = dynamic_cast(\n> + controller_.getAlgorithm(\"awb\"));\n> + if (!awb) {\n> + LOG(IPARPI, Warning)\n> + << \"Could not set COLOUR_TEMPERATURE - no AWB algorithm\";\n> + break;\n> + }\n> +\n> + awb->setColourTemperature(temperatureK);\n> + awbEnabled_ = false; /* doing this puts AWB into manual mode */\n> +\n> + /* This metadata will get reported back automatically. */\n> + break;\n> + } while (false);\n> +\n> /* Iterate over controls */\n> for (auto const &ctrl : controls) {\n> LOG(IPARPI, Debug) << \"Request ctrl: \"\n> @@ -1037,25 +1134,7 @@ void IpaBase::applyControls(const ControlList &controls)\n> }\n> \n> case controls::AWB_ENABLE: {\n> - /* Silently ignore this control for a mono sensor. */\n> - if (monoSensor_)\n> - break;\n> -\n> - RPiController::AwbAlgorithm *awb = dynamic_cast(\n> - controller_.getAlgorithm(\"awb\"));\n> - if (!awb) {\n> - LOG(IPARPI, Warning)\n> - << \"Could not set AWB_ENABLE - no AWB algorithm\";\n> - break;\n> - }\n> -\n> - if (ctrl.second.get() == false)\n> - awb->disableAuto();\n> - else\n> - awb->enableAuto();\n> -\n> - libcameraMetadata_.set(controls::AwbEnable,\n> - ctrl.second.get());\n> + /* We handled this one above. */\n> break;\n> }\n> \n> @@ -1080,47 +1159,60 @@ void IpaBase::applyControls(const ControlList &controls)\n> LOG(IPARPI, Error) << \"AWB mode \" << idx\n> << \" not recognised\";\n> }\n> +\n> break;\n> }\n> \n> case controls::COLOUR_GAINS: {\n> - /* Silently ignore this control for a mono sensor. */\n> - if (monoSensor_)\n> - break;\n> -\n> - auto gains = ctrl.second.get>();\n> - RPiController::AwbAlgorithm *awb = dynamic_cast(\n> - controller_.getAlgorithm(\"awb\"));\n> - if (!awb) {\n> - LOG(IPARPI, Warning)\n> - << \"Could not set COLOUR_GAINS - no AWB algorithm\";\n> - break;\n> - }\n> -\n> - awb->setManualGains(gains[0], gains[1]);\n> - if (gains[0] != 0.0f && gains[1] != 0.0f)\n> - /* A gain of 0.0f will switch back to auto mode. */\n> - libcameraMetadata_.set(controls::ColourGains,\n> - { gains[0], gains[1] });\n> + /* We handled this one above. */\n> break;\n> }\n> \n> case controls::COLOUR_TEMPERATURE: {\n> - /* Silently ignore this control for a mono sensor. */\n> + /* We handled this one above. */\n> + break;\n> + }\n> +\n> + case controls::COLOUR_CORRECTION_MATRIX: {\n> if (monoSensor_)\n> break;\n> \n> - auto temperatureK = ctrl.second.get();\n> + auto floats = ctrl.second.get>();\n> + RPiController::CcmAlgorithm *ccm = dynamic_cast(\n> + controller_.getAlgorithm(\"ccm\"));\n> + if (!ccm) {\n> + LOG(IPARPI, Warning)\n> + << \"Could not set COLOUR_CORRECTION_MATRIX - no CCM algorithm\";\n> + break;\n> + }\n> +\n> RPiController::AwbAlgorithm *awb = dynamic_cast(\n> controller_.getAlgorithm(\"awb\"));\n> - if (!awb) {\n> + if (awb && awbEnabled_) {\n> LOG(IPARPI, Warning)\n> - << \"Could not set COLOUR_TEMPERATURE - no AWB algorithm\";\n> + << \"Could not set COLOUR_CORRECTION_MATRIX - AWB is active\";\n> break;\n> }\n> \n> - awb->setColourTemperature(temperatureK);\n> - /* This metadata will get reported back automatically. */\n> + /* We are guaranteed this control contains 9 values. Nevertheless: */\n> + assert(floats.size() == 9);\n> +\n> + Matrix matrix;\n> + for (std::size_t i = 0; i < 3; ++i)\n> + for (std::size_t j = 0; j < 3; ++j)\n> + matrix[i][j] = static_cast(floats[i * 3 + j]);\n> +\n> + ccm->setCcm(matrix);\n> +\n> + /*\n> + * But if AWB is running, go back to auto mode. The CCM gets remembered,\n> + * which avoids the race between setting the CCM and disabling AWB in\n> + * the same set of controls.\n> + */\n> + if (awbEnabled_)\n> + ccm->enableAuto();\n> +\n> + /* This metadata will be reported back automatically. */\n> break;\n> }\n> \n> diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h\n> index e2f6e330..5348f2ea 100644\n> --- a/src/ipa/rpi/common/ipa_base.h\n> +++ b/src/ipa/rpi/common/ipa_base.h\n> @@ -140,6 +140,8 @@ private:\n> int32_t mode;\n> utils::Duration manualPeriod;\n> } flickerState_;\n> +\n> + bool awbEnabled_;\n> };\n> \n> } /* namespace ipa::RPi */\n> diff --git a/src/ipa/rpi/controller/ccm_algorithm.h b/src/ipa/rpi/controller/ccm_algorithm.h\n> index 6678ba75..785c408a 100644\n> --- a/src/ipa/rpi/controller/ccm_algorithm.h\n> +++ b/src/ipa/rpi/controller/ccm_algorithm.h\n> @@ -6,16 +6,22 @@\n> */\n> #pragma once\n> \n> +#include \"libcamera/internal/matrix.h\"\n> +\n> #include \"algorithm.h\"\n> \n> namespace RPiController {\n> \n> +using Matrix3x3 = libcamera::Matrix;\n> +\n> class CcmAlgorithm : public Algorithm\n> {\n> public:\n> CcmAlgorithm(Controller *controller) : Algorithm(controller) {}\n> /* A CCM algorithm must provide the following: */\n> + virtual void enableAuto() = 0;\n> virtual void setSaturation(double saturation) = 0;\n> + virtual void setCcm(Matrix3x3 const &matrix) = 0;\n> };\n> \n> } /* namespace RPiController */\n> diff --git a/src/ipa/rpi/controller/rpi/ccm.cpp b/src/ipa/rpi/controller/rpi/ccm.cpp\n> index 8607f152..c42c0141 100644\n> --- a/src/ipa/rpi/controller/rpi/ccm.cpp\n> +++ b/src/ipa/rpi/controller/rpi/ccm.cpp\n> @@ -32,7 +32,10 @@ LOG_DEFINE_CATEGORY(RPiCcm)\n> using Matrix3x3 = Matrix;\n> \n> Ccm::Ccm(Controller *controller)\n> - : CcmAlgorithm(controller), saturation_(1.0) {}\n> + : CcmAlgorithm(controller), enableAuto_(true), saturation_(1.0),\n> + manualCcm_({ 1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0 })\n> +{\n> +}\n> \n> char const *Ccm::name() const\n> {\n> @@ -78,11 +81,22 @@ int Ccm::read(const libcamera::YamlObject ¶ms)\n> return 0;\n> }\n> \n> +void Ccm::enableAuto()\n> +{\n> + enableAuto_ = true;\n> +}\n> +\n> void Ccm::setSaturation(double saturation)\n> {\n> saturation_ = saturation;\n> }\n> \n> +void Ccm::setCcm(Matrix3x3 const &matrix)\n> +{\n> + enableAuto_ = false;\n> + manualCcm_ = matrix;\n> +}\n> +\n> void Ccm::initialise()\n> {\n> }\n> @@ -151,7 +165,13 @@ void Ccm::prepare(Metadata *imageMetadata)\n> LOG(RPiCcm, Warning) << \"no colour temperature found\";\n> if (!luxOk)\n> LOG(RPiCcm, Warning) << \"no lux value found\";\n> - Matrix3x3 ccm = calculateCcm(config_.ccms, awb.temperatureK);\n> +\n> + Matrix3x3 ccm;\n> + if (enableAuto_)\n> + ccm = calculateCcm(config_.ccms, awb.temperatureK);\n> + else\n> + ccm = manualCcm_;\n> +\n> double saturation = saturation_;\n> struct CcmStatus ccmStatus;\n> ccmStatus.saturation = saturation;\n> diff --git a/src/ipa/rpi/controller/rpi/ccm.h b/src/ipa/rpi/controller/rpi/ccm.h\n> index c05dbb17..70f28ed3 100644\n> --- a/src/ipa/rpi/controller/rpi/ccm.h\n> +++ b/src/ipa/rpi/controller/rpi/ccm.h\n> @@ -8,7 +8,6 @@\n> \n> #include \n> \n> -#include \"libcamera/internal/matrix.h\"\n> #include \n> \n> #include \"../ccm_algorithm.h\"\n> @@ -33,13 +32,17 @@ public:\n> Ccm(Controller *controller = NULL);\n> char const *name() const override;\n> int read(const libcamera::YamlObject ¶ms) override;\n> + void enableAuto() override;\n> void setSaturation(double saturation) override;\n> + void setCcm(Matrix3x3 const &matrix) override;\n> void initialise() override;\n> void prepare(Metadata *imageMetadata) override;\n> \n> private:\n> CcmConfig config_;\n> + bool enableAuto_;\n> double saturation_;\n> + Matrix3x3 manualCcm_;\n> };\n> \n> } /* namespace RPiController */\n> -- \n> 2.39.5\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 AA421BD87C\n\tfor ;\n\tMon, 1 Sep 2025 08:10:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4D59069318;\n\tMon, 1 Sep 2025 10:10:32 +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 7097C69318\n\tfor ;\n\tMon, 1 Sep 2025 10:10:29 +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 29DF1E92;\n\tMon, 1 Sep 2025 10:09:22 +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=\"KJVJ6d4y\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1756714162;\n\tbh=3k037fwSP9oIziyZauMZkADh1V4ZxNNx2SNruQNBo00=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=KJVJ6d4yL5BKCiDRCSQdQEHMPS70nY0EvwXz64XT9SYEn1uSM88rLOMv+N5LwQHV6\n\tPhsCSvPRmtMVpjVujRpUl0sXuRIPeRW1PtgA4Vzo8m7+A4wt7OzgCLiNiU9sQuRLBV\n\tV7xNjJbH8HOCuMhv/IzdYtlpvEW5AeGIn6oH1lR0=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250804144804.16965-1-david.plowman@raspberrypi.com>","References":"<20250804144804.16965-1-david.plowman@raspberrypi.com>","Subject":"Re: [PATCH v2] ipa: rpi: ccm: Implement \"manual\" CCM mode","From":"Kieran Bingham ","Cc":"David Plowman ","To":"David Plowman ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 01 Sep 2025 09:10:26 +0100","Message-ID":"<175671422696.1598056.14322765312609630863@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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":35654,"web_url":"https://patchwork.libcamera.org/comment/35654/","msgid":"","date":"2025-09-01T08:26:27","subject":"Re: [PATCH v2] ipa: rpi: ccm: Implement \"manual\" CCM mode","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"On Mon, 1 Sept 2025 at 09:10, Kieran Bingham\n wrote:\n>\n> Quoting David Plowman (2025-08-04 15:48:04)\n> > The CCM algorithm will now let an explicit colour matrix be set when\n> > AWB is in manual mode.\n> >\n> > We must handle any controls that can cause the AWB to be enabled or\n> > disabled first, so that we know the AWB's state correctly when we come\n> > to set the CCM.\n> >\n> > Signed-off-by: David Plowman \n>\n> Unfortunatly the CI blocked this patch.\n>\n> The following addition gets it through.\n>\n> ---------------------- src/ipa/rpi/controller/rpi/ccm.cpp ----------------------\n> index c42c014182ca..2806b4967158 100644\n> @@ -27,12 +27,10 @@ LOG_DEFINE_CATEGORY(RPiCcm)\n> * saturation setting that is exposed to applications.\n> */\n>\n> #define NAME \"rpi.ccm\"\n>\n> -using Matrix3x3 = Matrix;\n> -\n> Ccm::Ccm(Controller *controller)\n> : CcmAlgorithm(controller), enableAuto_(true), saturation_(1.0),\n> manualCcm_({ 1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0 })\n> {\n> }\n>\n>\n> As the Matrix3x3 type is now defined in the header src/ipa/rpi/controller/ccm_algorithm.h\n>\n>\n> If that's fine - I'll merge with that squashed in.\n\nYup, that makes sense. Thanks Kieran!\n\n\n>\n> --\n> Kieran\n>\n>\n> > ---\n> > src/ipa/rpi/common/ipa_base.cpp | 180 +++++++++++++++++++------\n> > src/ipa/rpi/common/ipa_base.h | 2 +\n> > src/ipa/rpi/controller/ccm_algorithm.h | 6 +\n> > src/ipa/rpi/controller/rpi/ccm.cpp | 24 +++-\n> > src/ipa/rpi/controller/rpi/ccm.h | 5 +-\n> > 5 files changed, 170 insertions(+), 47 deletions(-)\n> >\n> > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> > index ce2343e9..6448e6ab 100644\n> > --- a/src/ipa/rpi/common/ipa_base.cpp\n> > +++ b/src/ipa/rpi/common/ipa_base.cpp\n> > @@ -94,6 +94,7 @@ const ControlInfoMap::Map ipaColourControls{\n> > { &controls::AwbEnable, ControlInfo(false, true) },\n> > { &controls::AwbMode, ControlInfo(controls::AwbModeValues) },\n> > { &controls::ColourGains, ControlInfo(0.0f, 32.0f) },\n> > + { &controls::ColourCorrectionMatrix, ControlInfo(0.0f, 8.0f) },\n> > { &controls::ColourTemperature, ControlInfo(100, 100000) },\n> > { &controls::Saturation, ControlInfo(0.0f, 32.0f, 1.0f) },\n> > };\n> > @@ -126,7 +127,7 @@ namespace ipa::RPi {\n> > IpaBase::IpaBase()\n> > : controller_(), frameLengths_(FrameLengthsQueueSize, 0s), statsMetadataOutput_(false),\n> > stitchSwapBuffers_(false), frameCount_(0), mistrustCount_(0), lastRunTimestamp_(0),\n> > - firstStart_(true), flickerState_({ 0, 0s })\n> > + firstStart_(true), flickerState_({ 0, 0s }), awbEnabled_(true)\n> > {\n> > }\n> >\n> > @@ -821,6 +822,102 @@ void IpaBase::applyControls(const ControlList &controls)\n> > }\n> > }\n> >\n> > + /*\n> > + * We must also handle any AWB on/off changes first, so that the CCM algorithm\n> > + * knows its state correctly.\n> > + */\n> > + const auto awbEnable = controls.get(controls::AwbEnable);\n> > + if (awbEnable)\n> > + do {\n> > + /* Silently ignore this control for a mono sensor. */\n> > + if (monoSensor_)\n> > + break;\n> > +\n> > + RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > + controller_.getAlgorithm(\"awb\"));\n> > + if (!awb) {\n> > + LOG(IPARPI, Warning)\n> > + << \"Could not set AWB_ENABLE - no AWB algorithm\";\n> > + break;\n> > + }\n> > +\n> > + awbEnabled_ = *awbEnable;\n> > +\n> > + if (!awbEnabled_)\n> > + awb->disableAuto();\n> > + else {\n> > + awb->enableAuto();\n> > +\n> > + /* The CCM algorithm must go back to auto as well. */\n> > + RPiController::CcmAlgorithm *ccm = dynamic_cast(\n> > + controller_.getAlgorithm(\"ccm\"));\n> > + if (ccm)\n> > + ccm->enableAuto();\n> > + }\n> > +\n> > + libcameraMetadata_.set(controls::AwbEnable, awbEnabled_);\n> > + } while (false);\n> > +\n> > + const auto colourGains = controls.get(controls::ColourGains);\n> > + if (colourGains)\n> > + do {\n> > + /* Silently ignore this control for a mono sensor. */\n> > + if (monoSensor_)\n> > + break;\n> > +\n> > + auto gains = *colourGains;\n> > + RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > + controller_.getAlgorithm(\"awb\"));\n> > + if (!awb) {\n> > + LOG(IPARPI, Warning)\n> > + << \"Could not set COLOUR_GAINS - no AWB algorithm\";\n> > + break;\n> > + }\n> > +\n> > + awb->setManualGains(gains[0], gains[1]);\n> > + if (gains[0] != 0.0f && gains[1] != 0.0f) {\n> > + /* A gain of 0.0f will switch back to auto mode. */\n> > + libcameraMetadata_.set(controls::ColourGains,\n> > + { gains[0], gains[1] });\n> > +\n> > + awbEnabled_ = false; /* doing this puts AWB into manual mode */\n> > + } else {\n> > + awbEnabled_ = true; /* doing this puts AWB into auto mode */\n> > +\n> > + /* The CCM algorithm must go back to auto as well. */\n> > + RPiController::CcmAlgorithm *ccm = dynamic_cast(\n> > + controller_.getAlgorithm(\"ccm\"));\n> > + if (ccm)\n> > + ccm->enableAuto();\n> > + }\n> > +\n> > + /* This metadata will get reported back automatically. */\n> > + break;\n> > + } while (false);\n> > +\n> > + const auto colourTemperature = controls.get(controls::ColourTemperature);\n> > + if (colourTemperature)\n> > + do {\n> > + /* Silently ignore this control for a mono sensor. */\n> > + if (monoSensor_)\n> > + break;\n> > +\n> > + auto temperatureK = *colourTemperature;\n> > + RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > + controller_.getAlgorithm(\"awb\"));\n> > + if (!awb) {\n> > + LOG(IPARPI, Warning)\n> > + << \"Could not set COLOUR_TEMPERATURE - no AWB algorithm\";\n> > + break;\n> > + }\n> > +\n> > + awb->setColourTemperature(temperatureK);\n> > + awbEnabled_ = false; /* doing this puts AWB into manual mode */\n> > +\n> > + /* This metadata will get reported back automatically. */\n> > + break;\n> > + } while (false);\n> > +\n> > /* Iterate over controls */\n> > for (auto const &ctrl : controls) {\n> > LOG(IPARPI, Debug) << \"Request ctrl: \"\n> > @@ -1037,25 +1134,7 @@ void IpaBase::applyControls(const ControlList &controls)\n> > }\n> >\n> > case controls::AWB_ENABLE: {\n> > - /* Silently ignore this control for a mono sensor. */\n> > - if (monoSensor_)\n> > - break;\n> > -\n> > - RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > - controller_.getAlgorithm(\"awb\"));\n> > - if (!awb) {\n> > - LOG(IPARPI, Warning)\n> > - << \"Could not set AWB_ENABLE - no AWB algorithm\";\n> > - break;\n> > - }\n> > -\n> > - if (ctrl.second.get() == false)\n> > - awb->disableAuto();\n> > - else\n> > - awb->enableAuto();\n> > -\n> > - libcameraMetadata_.set(controls::AwbEnable,\n> > - ctrl.second.get());\n> > + /* We handled this one above. */\n> > break;\n> > }\n> >\n> > @@ -1080,47 +1159,60 @@ void IpaBase::applyControls(const ControlList &controls)\n> > LOG(IPARPI, Error) << \"AWB mode \" << idx\n> > << \" not recognised\";\n> > }\n> > +\n> > break;\n> > }\n> >\n> > case controls::COLOUR_GAINS: {\n> > - /* Silently ignore this control for a mono sensor. */\n> > - if (monoSensor_)\n> > - break;\n> > -\n> > - auto gains = ctrl.second.get>();\n> > - RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > - controller_.getAlgorithm(\"awb\"));\n> > - if (!awb) {\n> > - LOG(IPARPI, Warning)\n> > - << \"Could not set COLOUR_GAINS - no AWB algorithm\";\n> > - break;\n> > - }\n> > -\n> > - awb->setManualGains(gains[0], gains[1]);\n> > - if (gains[0] != 0.0f && gains[1] != 0.0f)\n> > - /* A gain of 0.0f will switch back to auto mode. */\n> > - libcameraMetadata_.set(controls::ColourGains,\n> > - { gains[0], gains[1] });\n> > + /* We handled this one above. */\n> > break;\n> > }\n> >\n> > case controls::COLOUR_TEMPERATURE: {\n> > - /* Silently ignore this control for a mono sensor. */\n> > + /* We handled this one above. */\n> > + break;\n> > + }\n> > +\n> > + case controls::COLOUR_CORRECTION_MATRIX: {\n> > if (monoSensor_)\n> > break;\n> >\n> > - auto temperatureK = ctrl.second.get();\n> > + auto floats = ctrl.second.get>();\n> > + RPiController::CcmAlgorithm *ccm = dynamic_cast(\n> > + controller_.getAlgorithm(\"ccm\"));\n> > + if (!ccm) {\n> > + LOG(IPARPI, Warning)\n> > + << \"Could not set COLOUR_CORRECTION_MATRIX - no CCM algorithm\";\n> > + break;\n> > + }\n> > +\n> > RPiController::AwbAlgorithm *awb = dynamic_cast(\n> > controller_.getAlgorithm(\"awb\"));\n> > - if (!awb) {\n> > + if (awb && awbEnabled_) {\n> > LOG(IPARPI, Warning)\n> > - << \"Could not set COLOUR_TEMPERATURE - no AWB algorithm\";\n> > + << \"Could not set COLOUR_CORRECTION_MATRIX - AWB is active\";\n> > break;\n> > }\n> >\n> > - awb->setColourTemperature(temperatureK);\n> > - /* This metadata will get reported back automatically. */\n> > + /* We are guaranteed this control contains 9 values. Nevertheless: */\n> > + assert(floats.size() == 9);\n> > +\n> > + Matrix matrix;\n> > + for (std::size_t i = 0; i < 3; ++i)\n> > + for (std::size_t j = 0; j < 3; ++j)\n> > + matrix[i][j] = static_cast(floats[i * 3 + j]);\n> > +\n> > + ccm->setCcm(matrix);\n> > +\n> > + /*\n> > + * But if AWB is running, go back to auto mode. The CCM gets remembered,\n> > + * which avoids the race between setting the CCM and disabling AWB in\n> > + * the same set of controls.\n> > + */\n> > + if (awbEnabled_)\n> > + ccm->enableAuto();\n> > +\n> > + /* This metadata will be reported back automatically. */\n> > break;\n> > }\n> >\n> > diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h\n> > index e2f6e330..5348f2ea 100644\n> > --- a/src/ipa/rpi/common/ipa_base.h\n> > +++ b/src/ipa/rpi/common/ipa_base.h\n> > @@ -140,6 +140,8 @@ private:\n> > int32_t mode;\n> > utils::Duration manualPeriod;\n> > } flickerState_;\n> > +\n> > + bool awbEnabled_;\n> > };\n> >\n> > } /* namespace ipa::RPi */\n> > diff --git a/src/ipa/rpi/controller/ccm_algorithm.h b/src/ipa/rpi/controller/ccm_algorithm.h\n> > index 6678ba75..785c408a 100644\n> > --- a/src/ipa/rpi/controller/ccm_algorithm.h\n> > +++ b/src/ipa/rpi/controller/ccm_algorithm.h\n> > @@ -6,16 +6,22 @@\n> > */\n> > #pragma once\n> >\n> > +#include \"libcamera/internal/matrix.h\"\n> > +\n> > #include \"algorithm.h\"\n> >\n> > namespace RPiController {\n> >\n> > +using Matrix3x3 = libcamera::Matrix;\n> > +\n> > class CcmAlgorithm : public Algorithm\n> > {\n> > public:\n> > CcmAlgorithm(Controller *controller) : Algorithm(controller) {}\n> > /* A CCM algorithm must provide the following: */\n> > + virtual void enableAuto() = 0;\n> > virtual void setSaturation(double saturation) = 0;\n> > + virtual void setCcm(Matrix3x3 const &matrix) = 0;\n> > };\n> >\n> > } /* namespace RPiController */\n> > diff --git a/src/ipa/rpi/controller/rpi/ccm.cpp b/src/ipa/rpi/controller/rpi/ccm.cpp\n> > index 8607f152..c42c0141 100644\n> > --- a/src/ipa/rpi/controller/rpi/ccm.cpp\n> > +++ b/src/ipa/rpi/controller/rpi/ccm.cpp\n> > @@ -32,7 +32,10 @@ LOG_DEFINE_CATEGORY(RPiCcm)\n> > using Matrix3x3 = Matrix;\n> >\n> > Ccm::Ccm(Controller *controller)\n> > - : CcmAlgorithm(controller), saturation_(1.0) {}\n> > + : CcmAlgorithm(controller), enableAuto_(true), saturation_(1.0),\n> > + manualCcm_({ 1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0 })\n> > +{\n> > +}\n> >\n> > char const *Ccm::name() const\n> > {\n> > @@ -78,11 +81,22 @@ int Ccm::read(const libcamera::YamlObject ¶ms)\n> > return 0;\n> > }\n> >\n> > +void Ccm::enableAuto()\n> > +{\n> > + enableAuto_ = true;\n> > +}\n> > +\n> > void Ccm::setSaturation(double saturation)\n> > {\n> > saturation_ = saturation;\n> > }\n> >\n> > +void Ccm::setCcm(Matrix3x3 const &matrix)\n> > +{\n> > + enableAuto_ = false;\n> > + manualCcm_ = matrix;\n> > +}\n> > +\n> > void Ccm::initialise()\n> > {\n> > }\n> > @@ -151,7 +165,13 @@ void Ccm::prepare(Metadata *imageMetadata)\n> > LOG(RPiCcm, Warning) << \"no colour temperature found\";\n> > if (!luxOk)\n> > LOG(RPiCcm, Warning) << \"no lux value found\";\n> > - Matrix3x3 ccm = calculateCcm(config_.ccms, awb.temperatureK);\n> > +\n> > + Matrix3x3 ccm;\n> > + if (enableAuto_)\n> > + ccm = calculateCcm(config_.ccms, awb.temperatureK);\n> > + else\n> > + ccm = manualCcm_;\n> > +\n> > double saturation = saturation_;\n> > struct CcmStatus ccmStatus;\n> > ccmStatus.saturation = saturation;\n> > diff --git a/src/ipa/rpi/controller/rpi/ccm.h b/src/ipa/rpi/controller/rpi/ccm.h\n> > index c05dbb17..70f28ed3 100644\n> > --- a/src/ipa/rpi/controller/rpi/ccm.h\n> > +++ b/src/ipa/rpi/controller/rpi/ccm.h\n> > @@ -8,7 +8,6 @@\n> >\n> > #include \n> >\n> > -#include \"libcamera/internal/matrix.h\"\n> > #include \n> >\n> > #include \"../ccm_algorithm.h\"\n> > @@ -33,13 +32,17 @@ public:\n> > Ccm(Controller *controller = NULL);\n> > char const *name() const override;\n> > int read(const libcamera::YamlObject ¶ms) override;\n> > + void enableAuto() override;\n> > void setSaturation(double saturation) override;\n> > + void setCcm(Matrix3x3 const &matrix) override;\n> > void initialise() override;\n> > void prepare(Metadata *imageMetadata) override;\n> >\n> > private:\n> > CcmConfig config_;\n> > + bool enableAuto_;\n> > double saturation_;\n> > + Matrix3x3 manualCcm_;\n> > };\n> >\n> > } /* namespace RPiController */\n> > --\n> > 2.39.5\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 8978ABD87C\n\tfor ;\n\tMon, 1 Sep 2025 08:27:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6606369323;\n\tMon, 1 Sep 2025 10:27:07 +0200 (CEST)","from mail-vs1-xe2c.google.com (mail-vs1-xe2c.google.com\n\t[IPv6:2607:f8b0:4864:20::e2c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F1E1069318\n\tfor ;\n\tMon, 1 Sep 2025 10:27:04 +0200 (CEST)","by mail-vs1-xe2c.google.com with SMTP id\n\tada2fe7eead31-52fd5fc1f8aso1744137.2\n\tfor ;\n\tMon, 01 Sep 2025 01:27:04 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"cDD2NCuX\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1756715223; x=1757320023;\n\tdarn=lists.libcamera.org; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=ZnVNIOoXxTB26dngtEOD1ATfroJednE704llYjLYedY=;\n\tb=cDD2NCuXiftBAhEURTPf6wk6TzPklaLnULP1CC5OUQbK8KVZyqBE0yTUMbH8P+bwzt\n\tHEdMeNUcKiss3jK2+GYsxp/YOpHFxm+mLIo9LOZlkIEB52fZKdjdHKVmXrt+kgn5/kxb\n\t11XTCYrFntxZYs0e4gkB1fztRa1KfMLTeohNzQmvuifP3pEXnd0bWc6aVhSvAKNYfANo\n\tnQ8YSoFwVuj0wdzFXtyW+Bv96nnGgPxXObq+zCt+LYFE8oMDizofzRhkkHLrG3ZNBN6j\n\t6lGt3nGH0VdUjdTpORvAxUUGx+uN93fxa9qjUSAifw8vDhHSqW3/6YJa6ZSN2/fp+Kbg\n\teiqg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1756715223; x=1757320023;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=ZnVNIOoXxTB26dngtEOD1ATfroJednE704llYjLYedY=;\n\tb=KvMP6ZWTwcG3qZt5e79YygiBhmDBfIn/wJgEpMWsfpg8XKilYcxUgDbUp2UD9m88K7\n\tIDVuYmp6wpJjbGegaL/yoQx8/fjSCgvD15yr/zf7+ShCAV/mqO9Pl8HUvYMvjkGGXoI/\n\txQRbnWA9r77/FPMLk8cmYyeSgQEbSfknt/D74rJyA7kbLHDpS5rm3wkwsLr4rssILbZH\n\tjFqqVuzIB4PcyEMqoiCm6aE2aUcFIkk5eo8hfcek7pmkTS9qGRZfvOobOfQmDGB3b4v8\n\tzfkBwY2hYw//3cWljFd8+jAUaEgFbthzj+qXnH3jb5Av3zReMaM8Lf86Ji4I+zdkZBeu\n\tYztg==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCUqFVfmy0yLkCj2LtvyBbgW7rhTLG2vCajnm/y1pm9WWts/cfA0nRvrlATJr4tGO/SChyLdOIW+Znw+6Bu4CAE=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YzYWQr1UeNbPsfYxwwnl030l+wbgecteboU8dcW+fVbpo9m9sBG\n\tzgmCXHtebgKayIFaqmX86SsGoyIs2toxflFDEK/NowpGMKgNodMGglust6uT/Jva9HveBZfliiH\n\tM4OvvAHkkrQ+xEKvt6YO7s4fhu/W6WUHT/GtTA7wSzg==","X-Gm-Gg":"ASbGncuh3c127jGQYSFb5iBHWHZNw4axJgiT6S/gFJynaVrKcBoZfnXV3b6wXJPdyIu\n\tk1Dak+4J2sqrOoVRZ8GAEws5I9t+bpXJRKApCSGQfS3bSaSJvmtGdVU891dL4U3bOnncUl6Akk6\n\tkxJ4UawES5sg7SEFG6wR1znMz+5NT0hXdLzKlfa1yqiESXzPEgB048URDpMJPC8+4U028VncqWQ\n\tN/Km4g3CN6TSl9obQvMKuKP3UMbrzsogKKgfno=","X-Google-Smtp-Source":"AGHT+IEAFc8dLs65uW7bSUv3vK1N8Mji2SSwxuUo34gv9VTAcfcoY9XZdMaEpDGkVr/RmRROBx0ux66yd7ZL8HhcLus=","X-Received":"by 2002:a05:6102:8092:b0:528:b5cf:de26 with SMTP id\n\tada2fe7eead31-529c53ff03cmr1237705137.0.1756715223377;\n\tMon, 01 Sep 2025 01:27:03 -0700 (PDT)","MIME-Version":"1.0","References":"<20250804144804.16965-1-david.plowman@raspberrypi.com>\n\t<175671422696.1598056.14322765312609630863@ping.linuxembedded.co.uk>","In-Reply-To":"<175671422696.1598056.14322765312609630863@ping.linuxembedded.co.uk>","From":"Naushir Patuck ","Date":"Mon, 1 Sep 2025 09:26:27 +0100","X-Gm-Features":"Ac12FXy5Ia-nTZ7itB6syJ6yx-X6IxQOE5cA0wHm0YmqvH5bJ6sEspxIdVxloaI","Message-ID":"","Subject":"Re: [PATCH v2] ipa: rpi: ccm: Implement \"manual\" CCM mode","To":"Kieran Bingham ","Cc":"David Plowman ,\n\tlibcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","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\" "}}]