| Message ID | 20250804144804.16965-1-david.plowman@raspberrypi.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi David, On Mon, 4 Aug 2025 at 15:48, David Plowman <david.plowman@raspberrypi.com> wrote: > > The CCM algorithm will now let an explicit colour matrix be set when > AWB is in manual mode. > > We must handle any controls that can cause the AWB to be enabled or > disabled first, so that we know the AWB's state correctly when we come > to set the CCM. > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> Looks reasonable to me. Reviewed-by: Naushir Patuck <naush@raspberrypi.com> > --- > src/ipa/rpi/common/ipa_base.cpp | 180 +++++++++++++++++++------ > src/ipa/rpi/common/ipa_base.h | 2 + > src/ipa/rpi/controller/ccm_algorithm.h | 6 + > src/ipa/rpi/controller/rpi/ccm.cpp | 24 +++- > src/ipa/rpi/controller/rpi/ccm.h | 5 +- > 5 files changed, 170 insertions(+), 47 deletions(-) > > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp > index ce2343e9..6448e6ab 100644 > --- a/src/ipa/rpi/common/ipa_base.cpp > +++ b/src/ipa/rpi/common/ipa_base.cpp > @@ -94,6 +94,7 @@ const ControlInfoMap::Map ipaColourControls{ > { &controls::AwbEnable, ControlInfo(false, true) }, > { &controls::AwbMode, ControlInfo(controls::AwbModeValues) }, > { &controls::ColourGains, ControlInfo(0.0f, 32.0f) }, > + { &controls::ColourCorrectionMatrix, ControlInfo(0.0f, 8.0f) }, > { &controls::ColourTemperature, ControlInfo(100, 100000) }, > { &controls::Saturation, ControlInfo(0.0f, 32.0f, 1.0f) }, > }; > @@ -126,7 +127,7 @@ namespace ipa::RPi { > IpaBase::IpaBase() > : controller_(), frameLengths_(FrameLengthsQueueSize, 0s), statsMetadataOutput_(false), > stitchSwapBuffers_(false), frameCount_(0), mistrustCount_(0), lastRunTimestamp_(0), > - firstStart_(true), flickerState_({ 0, 0s }) > + firstStart_(true), flickerState_({ 0, 0s }), awbEnabled_(true) > { > } > > @@ -821,6 +822,102 @@ void IpaBase::applyControls(const ControlList &controls) > } > } > > + /* > + * We must also handle any AWB on/off changes first, so that the CCM algorithm > + * knows its state correctly. > + */ > + const auto awbEnable = controls.get(controls::AwbEnable); > + if (awbEnable) > + do { > + /* Silently ignore this control for a mono sensor. */ > + if (monoSensor_) > + break; > + > + RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > + controller_.getAlgorithm("awb")); > + if (!awb) { > + LOG(IPARPI, Warning) > + << "Could not set AWB_ENABLE - no AWB algorithm"; > + break; > + } > + > + awbEnabled_ = *awbEnable; > + > + if (!awbEnabled_) > + awb->disableAuto(); > + else { > + awb->enableAuto(); > + > + /* The CCM algorithm must go back to auto as well. */ > + RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>( > + controller_.getAlgorithm("ccm")); > + if (ccm) > + ccm->enableAuto(); > + } > + > + libcameraMetadata_.set(controls::AwbEnable, awbEnabled_); > + } while (false); > + > + const auto colourGains = controls.get(controls::ColourGains); > + if (colourGains) > + do { > + /* Silently ignore this control for a mono sensor. */ > + if (monoSensor_) > + break; > + > + auto gains = *colourGains; > + RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > + controller_.getAlgorithm("awb")); > + if (!awb) { > + LOG(IPARPI, Warning) > + << "Could not set COLOUR_GAINS - no AWB algorithm"; > + break; > + } > + > + awb->setManualGains(gains[0], gains[1]); > + if (gains[0] != 0.0f && gains[1] != 0.0f) { > + /* A gain of 0.0f will switch back to auto mode. */ > + libcameraMetadata_.set(controls::ColourGains, > + { gains[0], gains[1] }); > + > + awbEnabled_ = false; /* doing this puts AWB into manual mode */ > + } else { > + awbEnabled_ = true; /* doing this puts AWB into auto mode */ > + > + /* The CCM algorithm must go back to auto as well. */ > + RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>( > + controller_.getAlgorithm("ccm")); > + if (ccm) > + ccm->enableAuto(); > + } > + > + /* This metadata will get reported back automatically. */ > + break; > + } while (false); > + > + const auto colourTemperature = controls.get(controls::ColourTemperature); > + if (colourTemperature) > + do { > + /* Silently ignore this control for a mono sensor. */ > + if (monoSensor_) > + break; > + > + auto temperatureK = *colourTemperature; > + RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > + controller_.getAlgorithm("awb")); > + if (!awb) { > + LOG(IPARPI, Warning) > + << "Could not set COLOUR_TEMPERATURE - no AWB algorithm"; > + break; > + } > + > + awb->setColourTemperature(temperatureK); > + awbEnabled_ = false; /* doing this puts AWB into manual mode */ > + > + /* This metadata will get reported back automatically. */ > + break; > + } while (false); > + > /* Iterate over controls */ > for (auto const &ctrl : controls) { > LOG(IPARPI, Debug) << "Request ctrl: " > @@ -1037,25 +1134,7 @@ void IpaBase::applyControls(const ControlList &controls) > } > > case controls::AWB_ENABLE: { > - /* Silently ignore this control for a mono sensor. */ > - if (monoSensor_) > - break; > - > - RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > - controller_.getAlgorithm("awb")); > - if (!awb) { > - LOG(IPARPI, Warning) > - << "Could not set AWB_ENABLE - no AWB algorithm"; > - break; > - } > - > - if (ctrl.second.get<bool>() == false) > - awb->disableAuto(); > - else > - awb->enableAuto(); > - > - libcameraMetadata_.set(controls::AwbEnable, > - ctrl.second.get<bool>()); > + /* We handled this one above. */ > break; > } > > @@ -1080,47 +1159,60 @@ void IpaBase::applyControls(const ControlList &controls) > LOG(IPARPI, Error) << "AWB mode " << idx > << " not recognised"; > } > + > break; > } > > case controls::COLOUR_GAINS: { > - /* Silently ignore this control for a mono sensor. */ > - if (monoSensor_) > - break; > - > - auto gains = ctrl.second.get<Span<const float>>(); > - RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > - controller_.getAlgorithm("awb")); > - if (!awb) { > - LOG(IPARPI, Warning) > - << "Could not set COLOUR_GAINS - no AWB algorithm"; > - break; > - } > - > - awb->setManualGains(gains[0], gains[1]); > - if (gains[0] != 0.0f && gains[1] != 0.0f) > - /* A gain of 0.0f will switch back to auto mode. */ > - libcameraMetadata_.set(controls::ColourGains, > - { gains[0], gains[1] }); > + /* We handled this one above. */ > break; > } > > case controls::COLOUR_TEMPERATURE: { > - /* Silently ignore this control for a mono sensor. */ > + /* We handled this one above. */ > + break; > + } > + > + case controls::COLOUR_CORRECTION_MATRIX: { > if (monoSensor_) > break; > > - auto temperatureK = ctrl.second.get<int32_t>(); > + auto floats = ctrl.second.get<Span<const float>>(); > + RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>( > + controller_.getAlgorithm("ccm")); > + if (!ccm) { > + LOG(IPARPI, Warning) > + << "Could not set COLOUR_CORRECTION_MATRIX - no CCM algorithm"; > + break; > + } > + > RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > controller_.getAlgorithm("awb")); > - if (!awb) { > + if (awb && awbEnabled_) { > LOG(IPARPI, Warning) > - << "Could not set COLOUR_TEMPERATURE - no AWB algorithm"; > + << "Could not set COLOUR_CORRECTION_MATRIX - AWB is active"; > break; > } > > - awb->setColourTemperature(temperatureK); > - /* This metadata will get reported back automatically. */ > + /* We are guaranteed this control contains 9 values. Nevertheless: */ > + assert(floats.size() == 9); > + > + Matrix<double, 3, 3> matrix; > + for (std::size_t i = 0; i < 3; ++i) > + for (std::size_t j = 0; j < 3; ++j) > + matrix[i][j] = static_cast<double>(floats[i * 3 + j]); > + > + ccm->setCcm(matrix); > + > + /* > + * But if AWB is running, go back to auto mode. The CCM gets remembered, > + * which avoids the race between setting the CCM and disabling AWB in > + * the same set of controls. > + */ > + if (awbEnabled_) > + ccm->enableAuto(); > + > + /* This metadata will be reported back automatically. */ > break; > } > > diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h > index e2f6e330..5348f2ea 100644 > --- a/src/ipa/rpi/common/ipa_base.h > +++ b/src/ipa/rpi/common/ipa_base.h > @@ -140,6 +140,8 @@ private: > int32_t mode; > utils::Duration manualPeriod; > } flickerState_; > + > + bool awbEnabled_; > }; > > } /* namespace ipa::RPi */ > diff --git a/src/ipa/rpi/controller/ccm_algorithm.h b/src/ipa/rpi/controller/ccm_algorithm.h > index 6678ba75..785c408a 100644 > --- a/src/ipa/rpi/controller/ccm_algorithm.h > +++ b/src/ipa/rpi/controller/ccm_algorithm.h > @@ -6,16 +6,22 @@ > */ > #pragma once > > +#include "libcamera/internal/matrix.h" > + > #include "algorithm.h" > > namespace RPiController { > > +using Matrix3x3 = libcamera::Matrix<double, 3, 3>; > + > class CcmAlgorithm : public Algorithm > { > public: > CcmAlgorithm(Controller *controller) : Algorithm(controller) {} > /* A CCM algorithm must provide the following: */ > + virtual void enableAuto() = 0; > virtual void setSaturation(double saturation) = 0; > + virtual void setCcm(Matrix3x3 const &matrix) = 0; > }; > > } /* namespace RPiController */ > diff --git a/src/ipa/rpi/controller/rpi/ccm.cpp b/src/ipa/rpi/controller/rpi/ccm.cpp > index 8607f152..c42c0141 100644 > --- a/src/ipa/rpi/controller/rpi/ccm.cpp > +++ b/src/ipa/rpi/controller/rpi/ccm.cpp > @@ -32,7 +32,10 @@ LOG_DEFINE_CATEGORY(RPiCcm) > using Matrix3x3 = Matrix<double, 3, 3>; > > Ccm::Ccm(Controller *controller) > - : CcmAlgorithm(controller), saturation_(1.0) {} > + : CcmAlgorithm(controller), enableAuto_(true), saturation_(1.0), > + manualCcm_({ 1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0 }) > +{ > +} > > char const *Ccm::name() const > { > @@ -78,11 +81,22 @@ int Ccm::read(const libcamera::YamlObject ¶ms) > return 0; > } > > +void Ccm::enableAuto() > +{ > + enableAuto_ = true; > +} > + > void Ccm::setSaturation(double saturation) > { > saturation_ = saturation; > } > > +void Ccm::setCcm(Matrix3x3 const &matrix) > +{ > + enableAuto_ = false; > + manualCcm_ = matrix; > +} > + > void Ccm::initialise() > { > } > @@ -151,7 +165,13 @@ void Ccm::prepare(Metadata *imageMetadata) > LOG(RPiCcm, Warning) << "no colour temperature found"; > if (!luxOk) > LOG(RPiCcm, Warning) << "no lux value found"; > - Matrix3x3 ccm = calculateCcm(config_.ccms, awb.temperatureK); > + > + Matrix3x3 ccm; > + if (enableAuto_) > + ccm = calculateCcm(config_.ccms, awb.temperatureK); > + else > + ccm = manualCcm_; > + > double saturation = saturation_; > struct CcmStatus ccmStatus; > ccmStatus.saturation = saturation; > diff --git a/src/ipa/rpi/controller/rpi/ccm.h b/src/ipa/rpi/controller/rpi/ccm.h > index c05dbb17..70f28ed3 100644 > --- a/src/ipa/rpi/controller/rpi/ccm.h > +++ b/src/ipa/rpi/controller/rpi/ccm.h > @@ -8,7 +8,6 @@ > > #include <vector> > > -#include "libcamera/internal/matrix.h" > #include <libipa/pwl.h> > > #include "../ccm_algorithm.h" > @@ -33,13 +32,17 @@ public: > Ccm(Controller *controller = NULL); > char const *name() const override; > int read(const libcamera::YamlObject ¶ms) override; > + void enableAuto() override; > void setSaturation(double saturation) override; > + void setCcm(Matrix3x3 const &matrix) override; > void initialise() override; > void prepare(Metadata *imageMetadata) override; > > private: > CcmConfig config_; > + bool enableAuto_; > double saturation_; > + Matrix3x3 manualCcm_; > }; > > } /* namespace RPiController */ > -- > 2.39.5 >
Quoting Naushir Patuck (2025-08-11 10:05:51) > Hi David, > > On Mon, 4 Aug 2025 at 15:48, David Plowman > <david.plowman@raspberrypi.com> wrote: > > > > The CCM algorithm will now let an explicit colour matrix be set when > > AWB is in manual mode. > > > > We must handle any controls that can cause the AWB to be enabled or > > disabled first, so that we know the AWB's state correctly when we come > > to set the CCM. > > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > Looks reasonable to me. > > Reviewed-by: Naushir Patuck <naush@raspberrypi.com> Happy to merge whenever you like here. I'm curious - v1 said something about the current documentation not giving the desired behavour. Should we change the documentation or update the expected behaviour? or are you ok with this one ? (I expect we can still merge this one and discuss separately) -- Kieran > > > --- > > src/ipa/rpi/common/ipa_base.cpp | 180 +++++++++++++++++++------ > > src/ipa/rpi/common/ipa_base.h | 2 + > > src/ipa/rpi/controller/ccm_algorithm.h | 6 + > > src/ipa/rpi/controller/rpi/ccm.cpp | 24 +++- > > src/ipa/rpi/controller/rpi/ccm.h | 5 +- > > 5 files changed, 170 insertions(+), 47 deletions(-) > > > > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp > > index ce2343e9..6448e6ab 100644 > > --- a/src/ipa/rpi/common/ipa_base.cpp > > +++ b/src/ipa/rpi/common/ipa_base.cpp > > @@ -94,6 +94,7 @@ const ControlInfoMap::Map ipaColourControls{ > > { &controls::AwbEnable, ControlInfo(false, true) }, > > { &controls::AwbMode, ControlInfo(controls::AwbModeValues) }, > > { &controls::ColourGains, ControlInfo(0.0f, 32.0f) }, > > + { &controls::ColourCorrectionMatrix, ControlInfo(0.0f, 8.0f) }, > > { &controls::ColourTemperature, ControlInfo(100, 100000) }, > > { &controls::Saturation, ControlInfo(0.0f, 32.0f, 1.0f) }, > > }; > > @@ -126,7 +127,7 @@ namespace ipa::RPi { > > IpaBase::IpaBase() > > : controller_(), frameLengths_(FrameLengthsQueueSize, 0s), statsMetadataOutput_(false), > > stitchSwapBuffers_(false), frameCount_(0), mistrustCount_(0), lastRunTimestamp_(0), > > - firstStart_(true), flickerState_({ 0, 0s }) > > + firstStart_(true), flickerState_({ 0, 0s }), awbEnabled_(true) > > { > > } > > > > @@ -821,6 +822,102 @@ void IpaBase::applyControls(const ControlList &controls) > > } > > } > > > > + /* > > + * We must also handle any AWB on/off changes first, so that the CCM algorithm > > + * knows its state correctly. > > + */ > > + const auto awbEnable = controls.get(controls::AwbEnable); > > + if (awbEnable) > > + do { > > + /* Silently ignore this control for a mono sensor. */ > > + if (monoSensor_) > > + break; > > + > > + RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > > + controller_.getAlgorithm("awb")); > > + if (!awb) { > > + LOG(IPARPI, Warning) > > + << "Could not set AWB_ENABLE - no AWB algorithm"; > > + break; > > + } > > + > > + awbEnabled_ = *awbEnable; > > + > > + if (!awbEnabled_) > > + awb->disableAuto(); > > + else { > > + awb->enableAuto(); > > + > > + /* The CCM algorithm must go back to auto as well. */ > > + RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>( > > + controller_.getAlgorithm("ccm")); > > + if (ccm) > > + ccm->enableAuto(); > > + } > > + > > + libcameraMetadata_.set(controls::AwbEnable, awbEnabled_); > > + } while (false); > > + > > + const auto colourGains = controls.get(controls::ColourGains); > > + if (colourGains) > > + do { > > + /* Silently ignore this control for a mono sensor. */ > > + if (monoSensor_) > > + break; > > + > > + auto gains = *colourGains; > > + RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > > + controller_.getAlgorithm("awb")); > > + if (!awb) { > > + LOG(IPARPI, Warning) > > + << "Could not set COLOUR_GAINS - no AWB algorithm"; > > + break; > > + } > > + > > + awb->setManualGains(gains[0], gains[1]); > > + if (gains[0] != 0.0f && gains[1] != 0.0f) { > > + /* A gain of 0.0f will switch back to auto mode. */ > > + libcameraMetadata_.set(controls::ColourGains, > > + { gains[0], gains[1] }); > > + > > + awbEnabled_ = false; /* doing this puts AWB into manual mode */ > > + } else { > > + awbEnabled_ = true; /* doing this puts AWB into auto mode */ > > + > > + /* The CCM algorithm must go back to auto as well. */ > > + RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>( > > + controller_.getAlgorithm("ccm")); > > + if (ccm) > > + ccm->enableAuto(); > > + } > > + > > + /* This metadata will get reported back automatically. */ > > + break; > > + } while (false); > > + > > + const auto colourTemperature = controls.get(controls::ColourTemperature); > > + if (colourTemperature) > > + do { > > + /* Silently ignore this control for a mono sensor. */ > > + if (monoSensor_) > > + break; > > + > > + auto temperatureK = *colourTemperature; > > + RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > > + controller_.getAlgorithm("awb")); > > + if (!awb) { > > + LOG(IPARPI, Warning) > > + << "Could not set COLOUR_TEMPERATURE - no AWB algorithm"; > > + break; > > + } > > + > > + awb->setColourTemperature(temperatureK); > > + awbEnabled_ = false; /* doing this puts AWB into manual mode */ > > + > > + /* This metadata will get reported back automatically. */ > > + break; > > + } while (false); > > + > > /* Iterate over controls */ > > for (auto const &ctrl : controls) { > > LOG(IPARPI, Debug) << "Request ctrl: " > > @@ -1037,25 +1134,7 @@ void IpaBase::applyControls(const ControlList &controls) > > } > > > > case controls::AWB_ENABLE: { > > - /* Silently ignore this control for a mono sensor. */ > > - if (monoSensor_) > > - break; > > - > > - RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > > - controller_.getAlgorithm("awb")); > > - if (!awb) { > > - LOG(IPARPI, Warning) > > - << "Could not set AWB_ENABLE - no AWB algorithm"; > > - break; > > - } > > - > > - if (ctrl.second.get<bool>() == false) > > - awb->disableAuto(); > > - else > > - awb->enableAuto(); > > - > > - libcameraMetadata_.set(controls::AwbEnable, > > - ctrl.second.get<bool>()); > > + /* We handled this one above. */ > > break; > > } > > > > @@ -1080,47 +1159,60 @@ void IpaBase::applyControls(const ControlList &controls) > > LOG(IPARPI, Error) << "AWB mode " << idx > > << " not recognised"; > > } > > + > > break; > > } > > > > case controls::COLOUR_GAINS: { > > - /* Silently ignore this control for a mono sensor. */ > > - if (monoSensor_) > > - break; > > - > > - auto gains = ctrl.second.get<Span<const float>>(); > > - RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > > - controller_.getAlgorithm("awb")); > > - if (!awb) { > > - LOG(IPARPI, Warning) > > - << "Could not set COLOUR_GAINS - no AWB algorithm"; > > - break; > > - } > > - > > - awb->setManualGains(gains[0], gains[1]); > > - if (gains[0] != 0.0f && gains[1] != 0.0f) > > - /* A gain of 0.0f will switch back to auto mode. */ > > - libcameraMetadata_.set(controls::ColourGains, > > - { gains[0], gains[1] }); > > + /* We handled this one above. */ > > break; > > } > > > > case controls::COLOUR_TEMPERATURE: { > > - /* Silently ignore this control for a mono sensor. */ > > + /* We handled this one above. */ > > + break; > > + } > > + > > + case controls::COLOUR_CORRECTION_MATRIX: { > > if (monoSensor_) > > break; > > > > - auto temperatureK = ctrl.second.get<int32_t>(); > > + auto floats = ctrl.second.get<Span<const float>>(); > > + RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>( > > + controller_.getAlgorithm("ccm")); > > + if (!ccm) { > > + LOG(IPARPI, Warning) > > + << "Could not set COLOUR_CORRECTION_MATRIX - no CCM algorithm"; > > + break; > > + } > > + > > RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > > controller_.getAlgorithm("awb")); > > - if (!awb) { > > + if (awb && awbEnabled_) { > > LOG(IPARPI, Warning) > > - << "Could not set COLOUR_TEMPERATURE - no AWB algorithm"; > > + << "Could not set COLOUR_CORRECTION_MATRIX - AWB is active"; > > break; > > } > > > > - awb->setColourTemperature(temperatureK); > > - /* This metadata will get reported back automatically. */ > > + /* We are guaranteed this control contains 9 values. Nevertheless: */ > > + assert(floats.size() == 9); > > + > > + Matrix<double, 3, 3> matrix; > > + for (std::size_t i = 0; i < 3; ++i) > > + for (std::size_t j = 0; j < 3; ++j) > > + matrix[i][j] = static_cast<double>(floats[i * 3 + j]); > > + > > + ccm->setCcm(matrix); > > + > > + /* > > + * But if AWB is running, go back to auto mode. The CCM gets remembered, > > + * which avoids the race between setting the CCM and disabling AWB in > > + * the same set of controls. > > + */ > > + if (awbEnabled_) > > + ccm->enableAuto(); > > + > > + /* This metadata will be reported back automatically. */ > > break; > > } > > > > diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h > > index e2f6e330..5348f2ea 100644 > > --- a/src/ipa/rpi/common/ipa_base.h > > +++ b/src/ipa/rpi/common/ipa_base.h > > @@ -140,6 +140,8 @@ private: > > int32_t mode; > > utils::Duration manualPeriod; > > } flickerState_; > > + > > + bool awbEnabled_; > > }; > > > > } /* namespace ipa::RPi */ > > diff --git a/src/ipa/rpi/controller/ccm_algorithm.h b/src/ipa/rpi/controller/ccm_algorithm.h > > index 6678ba75..785c408a 100644 > > --- a/src/ipa/rpi/controller/ccm_algorithm.h > > +++ b/src/ipa/rpi/controller/ccm_algorithm.h > > @@ -6,16 +6,22 @@ > > */ > > #pragma once > > > > +#include "libcamera/internal/matrix.h" > > + > > #include "algorithm.h" > > > > namespace RPiController { > > > > +using Matrix3x3 = libcamera::Matrix<double, 3, 3>; > > + > > class CcmAlgorithm : public Algorithm > > { > > public: > > CcmAlgorithm(Controller *controller) : Algorithm(controller) {} > > /* A CCM algorithm must provide the following: */ > > + virtual void enableAuto() = 0; > > virtual void setSaturation(double saturation) = 0; > > + virtual void setCcm(Matrix3x3 const &matrix) = 0; > > }; > > > > } /* namespace RPiController */ > > diff --git a/src/ipa/rpi/controller/rpi/ccm.cpp b/src/ipa/rpi/controller/rpi/ccm.cpp > > index 8607f152..c42c0141 100644 > > --- a/src/ipa/rpi/controller/rpi/ccm.cpp > > +++ b/src/ipa/rpi/controller/rpi/ccm.cpp > > @@ -32,7 +32,10 @@ LOG_DEFINE_CATEGORY(RPiCcm) > > using Matrix3x3 = Matrix<double, 3, 3>; > > > > Ccm::Ccm(Controller *controller) > > - : CcmAlgorithm(controller), saturation_(1.0) {} > > + : CcmAlgorithm(controller), enableAuto_(true), saturation_(1.0), > > + manualCcm_({ 1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0 }) > > +{ > > +} > > > > char const *Ccm::name() const > > { > > @@ -78,11 +81,22 @@ int Ccm::read(const libcamera::YamlObject ¶ms) > > return 0; > > } > > > > +void Ccm::enableAuto() > > +{ > > + enableAuto_ = true; > > +} > > + > > void Ccm::setSaturation(double saturation) > > { > > saturation_ = saturation; > > } > > > > +void Ccm::setCcm(Matrix3x3 const &matrix) > > +{ > > + enableAuto_ = false; > > + manualCcm_ = matrix; > > +} > > + > > void Ccm::initialise() > > { > > } > > @@ -151,7 +165,13 @@ void Ccm::prepare(Metadata *imageMetadata) > > LOG(RPiCcm, Warning) << "no colour temperature found"; > > if (!luxOk) > > LOG(RPiCcm, Warning) << "no lux value found"; > > - Matrix3x3 ccm = calculateCcm(config_.ccms, awb.temperatureK); > > + > > + Matrix3x3 ccm; > > + if (enableAuto_) > > + ccm = calculateCcm(config_.ccms, awb.temperatureK); > > + else > > + ccm = manualCcm_; > > + > > double saturation = saturation_; > > struct CcmStatus ccmStatus; > > ccmStatus.saturation = saturation; > > diff --git a/src/ipa/rpi/controller/rpi/ccm.h b/src/ipa/rpi/controller/rpi/ccm.h > > index c05dbb17..70f28ed3 100644 > > --- a/src/ipa/rpi/controller/rpi/ccm.h > > +++ b/src/ipa/rpi/controller/rpi/ccm.h > > @@ -8,7 +8,6 @@ > > > > #include <vector> > > > > -#include "libcamera/internal/matrix.h" > > #include <libipa/pwl.h> > > > > #include "../ccm_algorithm.h" > > @@ -33,13 +32,17 @@ public: > > Ccm(Controller *controller = NULL); > > char const *name() const override; > > int read(const libcamera::YamlObject ¶ms) override; > > + void enableAuto() override; > > void setSaturation(double saturation) override; > > + void setCcm(Matrix3x3 const &matrix) override; > > void initialise() override; > > void prepare(Metadata *imageMetadata) override; > > > > private: > > CcmConfig config_; > > + bool enableAuto_; > > double saturation_; > > + Matrix3x3 manualCcm_; > > }; > > > > } /* namespace RPiController */ > > -- > > 2.39.5 > >
Hi Kieran Well, I don't think it's worth worrying about too much because I believe I've implemented what the documentation describes. The point was merely that the documentation doesn't describe a behaviour that's super convenient for us. In our world, the CCM and AWB algorithm are separate things and don't actually know about each other, whereas the documented behaviour grabs the "AWB enable" and uses that to put the CCM into automatic or manual mode too. TBH, I can't see why it would be disallowed to set a manual CCM even when AWB is running, but there you go. v1 of the patch made a bit of a pig's ear of the state that I'd added to couple these two algorithms together. But whatever, I think v2 is correct and I don't really feel motivated to change anything!! Thanks David On Mon, 11 Aug 2025 at 10:37, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > > Quoting Naushir Patuck (2025-08-11 10:05:51) > > Hi David, > > > > On Mon, 4 Aug 2025 at 15:48, David Plowman > > <david.plowman@raspberrypi.com> wrote: > > > > > > The CCM algorithm will now let an explicit colour matrix be set when > > > AWB is in manual mode. > > > > > > We must handle any controls that can cause the AWB to be enabled or > > > disabled first, so that we know the AWB's state correctly when we come > > > to set the CCM. > > > > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > > > Looks reasonable to me. > > > > Reviewed-by: Naushir Patuck <naush@raspberrypi.com> > > Happy to merge whenever you like here. > > I'm curious - v1 said something about the current documentation not > giving the desired behavour. > > Should we change the documentation or update the expected behaviour? or > are you ok with this one ? > > (I expect we can still merge this one and discuss separately) > -- > Kieran > > > > > > > --- > > > src/ipa/rpi/common/ipa_base.cpp | 180 +++++++++++++++++++------ > > > src/ipa/rpi/common/ipa_base.h | 2 + > > > src/ipa/rpi/controller/ccm_algorithm.h | 6 + > > > src/ipa/rpi/controller/rpi/ccm.cpp | 24 +++- > > > src/ipa/rpi/controller/rpi/ccm.h | 5 +- > > > 5 files changed, 170 insertions(+), 47 deletions(-) > > > > > > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp > > > index ce2343e9..6448e6ab 100644 > > > --- a/src/ipa/rpi/common/ipa_base.cpp > > > +++ b/src/ipa/rpi/common/ipa_base.cpp > > > @@ -94,6 +94,7 @@ const ControlInfoMap::Map ipaColourControls{ > > > { &controls::AwbEnable, ControlInfo(false, true) }, > > > { &controls::AwbMode, ControlInfo(controls::AwbModeValues) }, > > > { &controls::ColourGains, ControlInfo(0.0f, 32.0f) }, > > > + { &controls::ColourCorrectionMatrix, ControlInfo(0.0f, 8.0f) }, > > > { &controls::ColourTemperature, ControlInfo(100, 100000) }, > > > { &controls::Saturation, ControlInfo(0.0f, 32.0f, 1.0f) }, > > > }; > > > @@ -126,7 +127,7 @@ namespace ipa::RPi { > > > IpaBase::IpaBase() > > > : controller_(), frameLengths_(FrameLengthsQueueSize, 0s), statsMetadataOutput_(false), > > > stitchSwapBuffers_(false), frameCount_(0), mistrustCount_(0), lastRunTimestamp_(0), > > > - firstStart_(true), flickerState_({ 0, 0s }) > > > + firstStart_(true), flickerState_({ 0, 0s }), awbEnabled_(true) > > > { > > > } > > > > > > @@ -821,6 +822,102 @@ void IpaBase::applyControls(const ControlList &controls) > > > } > > > } > > > > > > + /* > > > + * We must also handle any AWB on/off changes first, so that the CCM algorithm > > > + * knows its state correctly. > > > + */ > > > + const auto awbEnable = controls.get(controls::AwbEnable); > > > + if (awbEnable) > > > + do { > > > + /* Silently ignore this control for a mono sensor. */ > > > + if (monoSensor_) > > > + break; > > > + > > > + RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > > > + controller_.getAlgorithm("awb")); > > > + if (!awb) { > > > + LOG(IPARPI, Warning) > > > + << "Could not set AWB_ENABLE - no AWB algorithm"; > > > + break; > > > + } > > > + > > > + awbEnabled_ = *awbEnable; > > > + > > > + if (!awbEnabled_) > > > + awb->disableAuto(); > > > + else { > > > + awb->enableAuto(); > > > + > > > + /* The CCM algorithm must go back to auto as well. */ > > > + RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>( > > > + controller_.getAlgorithm("ccm")); > > > + if (ccm) > > > + ccm->enableAuto(); > > > + } > > > + > > > + libcameraMetadata_.set(controls::AwbEnable, awbEnabled_); > > > + } while (false); > > > + > > > + const auto colourGains = controls.get(controls::ColourGains); > > > + if (colourGains) > > > + do { > > > + /* Silently ignore this control for a mono sensor. */ > > > + if (monoSensor_) > > > + break; > > > + > > > + auto gains = *colourGains; > > > + RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > > > + controller_.getAlgorithm("awb")); > > > + if (!awb) { > > > + LOG(IPARPI, Warning) > > > + << "Could not set COLOUR_GAINS - no AWB algorithm"; > > > + break; > > > + } > > > + > > > + awb->setManualGains(gains[0], gains[1]); > > > + if (gains[0] != 0.0f && gains[1] != 0.0f) { > > > + /* A gain of 0.0f will switch back to auto mode. */ > > > + libcameraMetadata_.set(controls::ColourGains, > > > + { gains[0], gains[1] }); > > > + > > > + awbEnabled_ = false; /* doing this puts AWB into manual mode */ > > > + } else { > > > + awbEnabled_ = true; /* doing this puts AWB into auto mode */ > > > + > > > + /* The CCM algorithm must go back to auto as well. */ > > > + RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>( > > > + controller_.getAlgorithm("ccm")); > > > + if (ccm) > > > + ccm->enableAuto(); > > > + } > > > + > > > + /* This metadata will get reported back automatically. */ > > > + break; > > > + } while (false); > > > + > > > + const auto colourTemperature = controls.get(controls::ColourTemperature); > > > + if (colourTemperature) > > > + do { > > > + /* Silently ignore this control for a mono sensor. */ > > > + if (monoSensor_) > > > + break; > > > + > > > + auto temperatureK = *colourTemperature; > > > + RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > > > + controller_.getAlgorithm("awb")); > > > + if (!awb) { > > > + LOG(IPARPI, Warning) > > > + << "Could not set COLOUR_TEMPERATURE - no AWB algorithm"; > > > + break; > > > + } > > > + > > > + awb->setColourTemperature(temperatureK); > > > + awbEnabled_ = false; /* doing this puts AWB into manual mode */ > > > + > > > + /* This metadata will get reported back automatically. */ > > > + break; > > > + } while (false); > > > + > > > /* Iterate over controls */ > > > for (auto const &ctrl : controls) { > > > LOG(IPARPI, Debug) << "Request ctrl: " > > > @@ -1037,25 +1134,7 @@ void IpaBase::applyControls(const ControlList &controls) > > > } > > > > > > case controls::AWB_ENABLE: { > > > - /* Silently ignore this control for a mono sensor. */ > > > - if (monoSensor_) > > > - break; > > > - > > > - RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > > > - controller_.getAlgorithm("awb")); > > > - if (!awb) { > > > - LOG(IPARPI, Warning) > > > - << "Could not set AWB_ENABLE - no AWB algorithm"; > > > - break; > > > - } > > > - > > > - if (ctrl.second.get<bool>() == false) > > > - awb->disableAuto(); > > > - else > > > - awb->enableAuto(); > > > - > > > - libcameraMetadata_.set(controls::AwbEnable, > > > - ctrl.second.get<bool>()); > > > + /* We handled this one above. */ > > > break; > > > } > > > > > > @@ -1080,47 +1159,60 @@ void IpaBase::applyControls(const ControlList &controls) > > > LOG(IPARPI, Error) << "AWB mode " << idx > > > << " not recognised"; > > > } > > > + > > > break; > > > } > > > > > > case controls::COLOUR_GAINS: { > > > - /* Silently ignore this control for a mono sensor. */ > > > - if (monoSensor_) > > > - break; > > > - > > > - auto gains = ctrl.second.get<Span<const float>>(); > > > - RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > > > - controller_.getAlgorithm("awb")); > > > - if (!awb) { > > > - LOG(IPARPI, Warning) > > > - << "Could not set COLOUR_GAINS - no AWB algorithm"; > > > - break; > > > - } > > > - > > > - awb->setManualGains(gains[0], gains[1]); > > > - if (gains[0] != 0.0f && gains[1] != 0.0f) > > > - /* A gain of 0.0f will switch back to auto mode. */ > > > - libcameraMetadata_.set(controls::ColourGains, > > > - { gains[0], gains[1] }); > > > + /* We handled this one above. */ > > > break; > > > } > > > > > > case controls::COLOUR_TEMPERATURE: { > > > - /* Silently ignore this control for a mono sensor. */ > > > + /* We handled this one above. */ > > > + break; > > > + } > > > + > > > + case controls::COLOUR_CORRECTION_MATRIX: { > > > if (monoSensor_) > > > break; > > > > > > - auto temperatureK = ctrl.second.get<int32_t>(); > > > + auto floats = ctrl.second.get<Span<const float>>(); > > > + RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>( > > > + controller_.getAlgorithm("ccm")); > > > + if (!ccm) { > > > + LOG(IPARPI, Warning) > > > + << "Could not set COLOUR_CORRECTION_MATRIX - no CCM algorithm"; > > > + break; > > > + } > > > + > > > RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > > > controller_.getAlgorithm("awb")); > > > - if (!awb) { > > > + if (awb && awbEnabled_) { > > > LOG(IPARPI, Warning) > > > - << "Could not set COLOUR_TEMPERATURE - no AWB algorithm"; > > > + << "Could not set COLOUR_CORRECTION_MATRIX - AWB is active"; > > > break; > > > } > > > > > > - awb->setColourTemperature(temperatureK); > > > - /* This metadata will get reported back automatically. */ > > > + /* We are guaranteed this control contains 9 values. Nevertheless: */ > > > + assert(floats.size() == 9); > > > + > > > + Matrix<double, 3, 3> matrix; > > > + for (std::size_t i = 0; i < 3; ++i) > > > + for (std::size_t j = 0; j < 3; ++j) > > > + matrix[i][j] = static_cast<double>(floats[i * 3 + j]); > > > + > > > + ccm->setCcm(matrix); > > > + > > > + /* > > > + * But if AWB is running, go back to auto mode. The CCM gets remembered, > > > + * which avoids the race between setting the CCM and disabling AWB in > > > + * the same set of controls. > > > + */ > > > + if (awbEnabled_) > > > + ccm->enableAuto(); > > > + > > > + /* This metadata will be reported back automatically. */ > > > break; > > > } > > > > > > diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h > > > index e2f6e330..5348f2ea 100644 > > > --- a/src/ipa/rpi/common/ipa_base.h > > > +++ b/src/ipa/rpi/common/ipa_base.h > > > @@ -140,6 +140,8 @@ private: > > > int32_t mode; > > > utils::Duration manualPeriod; > > > } flickerState_; > > > + > > > + bool awbEnabled_; > > > }; > > > > > > } /* namespace ipa::RPi */ > > > diff --git a/src/ipa/rpi/controller/ccm_algorithm.h b/src/ipa/rpi/controller/ccm_algorithm.h > > > index 6678ba75..785c408a 100644 > > > --- a/src/ipa/rpi/controller/ccm_algorithm.h > > > +++ b/src/ipa/rpi/controller/ccm_algorithm.h > > > @@ -6,16 +6,22 @@ > > > */ > > > #pragma once > > > > > > +#include "libcamera/internal/matrix.h" > > > + > > > #include "algorithm.h" > > > > > > namespace RPiController { > > > > > > +using Matrix3x3 = libcamera::Matrix<double, 3, 3>; > > > + > > > class CcmAlgorithm : public Algorithm > > > { > > > public: > > > CcmAlgorithm(Controller *controller) : Algorithm(controller) {} > > > /* A CCM algorithm must provide the following: */ > > > + virtual void enableAuto() = 0; > > > virtual void setSaturation(double saturation) = 0; > > > + virtual void setCcm(Matrix3x3 const &matrix) = 0; > > > }; > > > > > > } /* namespace RPiController */ > > > diff --git a/src/ipa/rpi/controller/rpi/ccm.cpp b/src/ipa/rpi/controller/rpi/ccm.cpp > > > index 8607f152..c42c0141 100644 > > > --- a/src/ipa/rpi/controller/rpi/ccm.cpp > > > +++ b/src/ipa/rpi/controller/rpi/ccm.cpp > > > @@ -32,7 +32,10 @@ LOG_DEFINE_CATEGORY(RPiCcm) > > > using Matrix3x3 = Matrix<double, 3, 3>; > > > > > > Ccm::Ccm(Controller *controller) > > > - : CcmAlgorithm(controller), saturation_(1.0) {} > > > + : CcmAlgorithm(controller), enableAuto_(true), saturation_(1.0), > > > + manualCcm_({ 1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0 }) > > > +{ > > > +} > > > > > > char const *Ccm::name() const > > > { > > > @@ -78,11 +81,22 @@ int Ccm::read(const libcamera::YamlObject ¶ms) > > > return 0; > > > } > > > > > > +void Ccm::enableAuto() > > > +{ > > > + enableAuto_ = true; > > > +} > > > + > > > void Ccm::setSaturation(double saturation) > > > { > > > saturation_ = saturation; > > > } > > > > > > +void Ccm::setCcm(Matrix3x3 const &matrix) > > > +{ > > > + enableAuto_ = false; > > > + manualCcm_ = matrix; > > > +} > > > + > > > void Ccm::initialise() > > > { > > > } > > > @@ -151,7 +165,13 @@ void Ccm::prepare(Metadata *imageMetadata) > > > LOG(RPiCcm, Warning) << "no colour temperature found"; > > > if (!luxOk) > > > LOG(RPiCcm, Warning) << "no lux value found"; > > > - Matrix3x3 ccm = calculateCcm(config_.ccms, awb.temperatureK); > > > + > > > + Matrix3x3 ccm; > > > + if (enableAuto_) > > > + ccm = calculateCcm(config_.ccms, awb.temperatureK); > > > + else > > > + ccm = manualCcm_; > > > + > > > double saturation = saturation_; > > > struct CcmStatus ccmStatus; > > > ccmStatus.saturation = saturation; > > > diff --git a/src/ipa/rpi/controller/rpi/ccm.h b/src/ipa/rpi/controller/rpi/ccm.h > > > index c05dbb17..70f28ed3 100644 > > > --- a/src/ipa/rpi/controller/rpi/ccm.h > > > +++ b/src/ipa/rpi/controller/rpi/ccm.h > > > @@ -8,7 +8,6 @@ > > > > > > #include <vector> > > > > > > -#include "libcamera/internal/matrix.h" > > > #include <libipa/pwl.h> > > > > > > #include "../ccm_algorithm.h" > > > @@ -33,13 +32,17 @@ public: > > > Ccm(Controller *controller = NULL); > > > char const *name() const override; > > > int read(const libcamera::YamlObject ¶ms) override; > > > + void enableAuto() override; > > > void setSaturation(double saturation) override; > > > + void setCcm(Matrix3x3 const &matrix) override; > > > void initialise() override; > > > void prepare(Metadata *imageMetadata) override; > > > > > > private: > > > CcmConfig config_; > > > + bool enableAuto_; > > > double saturation_; > > > + Matrix3x3 manualCcm_; > > > }; > > > > > > } /* namespace RPiController */ > > > -- > > > 2.39.5 > > >
Quoting David Plowman (2025-08-11 11:55:20) > Hi Kieran > > Well, I don't think it's worth worrying about too much because I > believe I've implemented what the documentation describes. > > The point was merely that the documentation doesn't describe a > behaviour that's super convenient for us. In our world, the CCM and > AWB algorithm are separate things and don't actually know about each That's the part I worry about ;-) > other, whereas the documented behaviour grabs the "AWB enable" and > uses that to put the CCM into automatic or manual mode too. TBH, I > can't see why it would be disallowed to set a manual CCM even when AWB > is running, but there you go. > > v1 of the patch made a bit of a pig's ear of the state that I'd added > to couple these two algorithms together. But whatever, I think v2 is > correct and I don't really feel motivated to change anything!! Ok no problem! lets go with v2 as it is then! -- Kieran > > Thanks > David > > On Mon, 11 Aug 2025 at 10:37, Kieran Bingham > <kieran.bingham@ideasonboard.com> wrote: > > > > Quoting Naushir Patuck (2025-08-11 10:05:51) > > > Hi David, > > > > > > On Mon, 4 Aug 2025 at 15:48, David Plowman > > > <david.plowman@raspberrypi.com> wrote: > > > > > > > > The CCM algorithm will now let an explicit colour matrix be set when > > > > AWB is in manual mode. > > > > > > > > We must handle any controls that can cause the AWB to be enabled or > > > > disabled first, so that we know the AWB's state correctly when we come > > > > to set the CCM. > > > > > > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > > > > > > Looks reasonable to me. > > > > > > Reviewed-by: Naushir Patuck <naush@raspberrypi.com> > > > > Happy to merge whenever you like here. > > > > I'm curious - v1 said something about the current documentation not > > giving the desired behavour. > > > > Should we change the documentation or update the expected behaviour? or > > are you ok with this one ? > > > > (I expect we can still merge this one and discuss separately) > > -- > > Kieran > > > > > > > > > > > --- > > > > src/ipa/rpi/common/ipa_base.cpp | 180 +++++++++++++++++++------ > > > > src/ipa/rpi/common/ipa_base.h | 2 + > > > > src/ipa/rpi/controller/ccm_algorithm.h | 6 + > > > > src/ipa/rpi/controller/rpi/ccm.cpp | 24 +++- > > > > src/ipa/rpi/controller/rpi/ccm.h | 5 +- > > > > 5 files changed, 170 insertions(+), 47 deletions(-) > > > > > > > > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp > > > > index ce2343e9..6448e6ab 100644 > > > > --- a/src/ipa/rpi/common/ipa_base.cpp > > > > +++ b/src/ipa/rpi/common/ipa_base.cpp > > > > @@ -94,6 +94,7 @@ const ControlInfoMap::Map ipaColourControls{ > > > > { &controls::AwbEnable, ControlInfo(false, true) }, > > > > { &controls::AwbMode, ControlInfo(controls::AwbModeValues) }, > > > > { &controls::ColourGains, ControlInfo(0.0f, 32.0f) }, > > > > + { &controls::ColourCorrectionMatrix, ControlInfo(0.0f, 8.0f) }, > > > > { &controls::ColourTemperature, ControlInfo(100, 100000) }, > > > > { &controls::Saturation, ControlInfo(0.0f, 32.0f, 1.0f) }, > > > > }; > > > > @@ -126,7 +127,7 @@ namespace ipa::RPi { > > > > IpaBase::IpaBase() > > > > : controller_(), frameLengths_(FrameLengthsQueueSize, 0s), statsMetadataOutput_(false), > > > > stitchSwapBuffers_(false), frameCount_(0), mistrustCount_(0), lastRunTimestamp_(0), > > > > - firstStart_(true), flickerState_({ 0, 0s }) > > > > + firstStart_(true), flickerState_({ 0, 0s }), awbEnabled_(true) > > > > { > > > > } > > > > > > > > @@ -821,6 +822,102 @@ void IpaBase::applyControls(const ControlList &controls) > > > > } > > > > } > > > > > > > > + /* > > > > + * We must also handle any AWB on/off changes first, so that the CCM algorithm > > > > + * knows its state correctly. > > > > + */ > > > > + const auto awbEnable = controls.get(controls::AwbEnable); > > > > + if (awbEnable) > > > > + do { > > > > + /* Silently ignore this control for a mono sensor. */ > > > > + if (monoSensor_) > > > > + break; > > > > + > > > > + RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > > > > + controller_.getAlgorithm("awb")); > > > > + if (!awb) { > > > > + LOG(IPARPI, Warning) > > > > + << "Could not set AWB_ENABLE - no AWB algorithm"; > > > > + break; > > > > + } > > > > + > > > > + awbEnabled_ = *awbEnable; > > > > + > > > > + if (!awbEnabled_) > > > > + awb->disableAuto(); > > > > + else { > > > > + awb->enableAuto(); > > > > + > > > > + /* The CCM algorithm must go back to auto as well. */ > > > > + RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>( > > > > + controller_.getAlgorithm("ccm")); > > > > + if (ccm) > > > > + ccm->enableAuto(); > > > > + } > > > > + > > > > + libcameraMetadata_.set(controls::AwbEnable, awbEnabled_); > > > > + } while (false); > > > > + > > > > + const auto colourGains = controls.get(controls::ColourGains); > > > > + if (colourGains) > > > > + do { > > > > + /* Silently ignore this control for a mono sensor. */ > > > > + if (monoSensor_) > > > > + break; > > > > + > > > > + auto gains = *colourGains; > > > > + RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > > > > + controller_.getAlgorithm("awb")); > > > > + if (!awb) { > > > > + LOG(IPARPI, Warning) > > > > + << "Could not set COLOUR_GAINS - no AWB algorithm"; > > > > + break; > > > > + } > > > > + > > > > + awb->setManualGains(gains[0], gains[1]); > > > > + if (gains[0] != 0.0f && gains[1] != 0.0f) { > > > > + /* A gain of 0.0f will switch back to auto mode. */ > > > > + libcameraMetadata_.set(controls::ColourGains, > > > > + { gains[0], gains[1] }); > > > > + > > > > + awbEnabled_ = false; /* doing this puts AWB into manual mode */ > > > > + } else { > > > > + awbEnabled_ = true; /* doing this puts AWB into auto mode */ > > > > + > > > > + /* The CCM algorithm must go back to auto as well. */ > > > > + RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>( > > > > + controller_.getAlgorithm("ccm")); > > > > + if (ccm) > > > > + ccm->enableAuto(); > > > > + } > > > > + > > > > + /* This metadata will get reported back automatically. */ > > > > + break; > > > > + } while (false); > > > > + > > > > + const auto colourTemperature = controls.get(controls::ColourTemperature); > > > > + if (colourTemperature) > > > > + do { > > > > + /* Silently ignore this control for a mono sensor. */ > > > > + if (monoSensor_) > > > > + break; > > > > + > > > > + auto temperatureK = *colourTemperature; > > > > + RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > > > > + controller_.getAlgorithm("awb")); > > > > + if (!awb) { > > > > + LOG(IPARPI, Warning) > > > > + << "Could not set COLOUR_TEMPERATURE - no AWB algorithm"; > > > > + break; > > > > + } > > > > + > > > > + awb->setColourTemperature(temperatureK); > > > > + awbEnabled_ = false; /* doing this puts AWB into manual mode */ > > > > + > > > > + /* This metadata will get reported back automatically. */ > > > > + break; > > > > + } while (false); > > > > + > > > > /* Iterate over controls */ > > > > for (auto const &ctrl : controls) { > > > > LOG(IPARPI, Debug) << "Request ctrl: " > > > > @@ -1037,25 +1134,7 @@ void IpaBase::applyControls(const ControlList &controls) > > > > } > > > > > > > > case controls::AWB_ENABLE: { > > > > - /* Silently ignore this control for a mono sensor. */ > > > > - if (monoSensor_) > > > > - break; > > > > - > > > > - RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > > > > - controller_.getAlgorithm("awb")); > > > > - if (!awb) { > > > > - LOG(IPARPI, Warning) > > > > - << "Could not set AWB_ENABLE - no AWB algorithm"; > > > > - break; > > > > - } > > > > - > > > > - if (ctrl.second.get<bool>() == false) > > > > - awb->disableAuto(); > > > > - else > > > > - awb->enableAuto(); > > > > - > > > > - libcameraMetadata_.set(controls::AwbEnable, > > > > - ctrl.second.get<bool>()); > > > > + /* We handled this one above. */ > > > > break; > > > > } > > > > > > > > @@ -1080,47 +1159,60 @@ void IpaBase::applyControls(const ControlList &controls) > > > > LOG(IPARPI, Error) << "AWB mode " << idx > > > > << " not recognised"; > > > > } > > > > + > > > > break; > > > > } > > > > > > > > case controls::COLOUR_GAINS: { > > > > - /* Silently ignore this control for a mono sensor. */ > > > > - if (monoSensor_) > > > > - break; > > > > - > > > > - auto gains = ctrl.second.get<Span<const float>>(); > > > > - RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > > > > - controller_.getAlgorithm("awb")); > > > > - if (!awb) { > > > > - LOG(IPARPI, Warning) > > > > - << "Could not set COLOUR_GAINS - no AWB algorithm"; > > > > - break; > > > > - } > > > > - > > > > - awb->setManualGains(gains[0], gains[1]); > > > > - if (gains[0] != 0.0f && gains[1] != 0.0f) > > > > - /* A gain of 0.0f will switch back to auto mode. */ > > > > - libcameraMetadata_.set(controls::ColourGains, > > > > - { gains[0], gains[1] }); > > > > + /* We handled this one above. */ > > > > break; > > > > } > > > > > > > > case controls::COLOUR_TEMPERATURE: { > > > > - /* Silently ignore this control for a mono sensor. */ > > > > + /* We handled this one above. */ > > > > + break; > > > > + } > > > > + > > > > + case controls::COLOUR_CORRECTION_MATRIX: { > > > > if (monoSensor_) > > > > break; > > > > > > > > - auto temperatureK = ctrl.second.get<int32_t>(); > > > > + auto floats = ctrl.second.get<Span<const float>>(); > > > > + RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>( > > > > + controller_.getAlgorithm("ccm")); > > > > + if (!ccm) { > > > > + LOG(IPARPI, Warning) > > > > + << "Could not set COLOUR_CORRECTION_MATRIX - no CCM algorithm"; > > > > + break; > > > > + } > > > > + > > > > RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( > > > > controller_.getAlgorithm("awb")); > > > > - if (!awb) { > > > > + if (awb && awbEnabled_) { > > > > LOG(IPARPI, Warning) > > > > - << "Could not set COLOUR_TEMPERATURE - no AWB algorithm"; > > > > + << "Could not set COLOUR_CORRECTION_MATRIX - AWB is active"; > > > > break; > > > > } > > > > > > > > - awb->setColourTemperature(temperatureK); > > > > - /* This metadata will get reported back automatically. */ > > > > + /* We are guaranteed this control contains 9 values. Nevertheless: */ > > > > + assert(floats.size() == 9); > > > > + > > > > + Matrix<double, 3, 3> matrix; > > > > + for (std::size_t i = 0; i < 3; ++i) > > > > + for (std::size_t j = 0; j < 3; ++j) > > > > + matrix[i][j] = static_cast<double>(floats[i * 3 + j]); > > > > + > > > > + ccm->setCcm(matrix); > > > > + > > > > + /* > > > > + * But if AWB is running, go back to auto mode. The CCM gets remembered, > > > > + * which avoids the race between setting the CCM and disabling AWB in > > > > + * the same set of controls. > > > > + */ > > > > + if (awbEnabled_) > > > > + ccm->enableAuto(); > > > > + > > > > + /* This metadata will be reported back automatically. */ > > > > break; > > > > } > > > > > > > > diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h > > > > index e2f6e330..5348f2ea 100644 > > > > --- a/src/ipa/rpi/common/ipa_base.h > > > > +++ b/src/ipa/rpi/common/ipa_base.h > > > > @@ -140,6 +140,8 @@ private: > > > > int32_t mode; > > > > utils::Duration manualPeriod; > > > > } flickerState_; > > > > + > > > > + bool awbEnabled_; > > > > }; > > > > > > > > } /* namespace ipa::RPi */ > > > > diff --git a/src/ipa/rpi/controller/ccm_algorithm.h b/src/ipa/rpi/controller/ccm_algorithm.h > > > > index 6678ba75..785c408a 100644 > > > > --- a/src/ipa/rpi/controller/ccm_algorithm.h > > > > +++ b/src/ipa/rpi/controller/ccm_algorithm.h > > > > @@ -6,16 +6,22 @@ > > > > */ > > > > #pragma once > > > > > > > > +#include "libcamera/internal/matrix.h" > > > > + > > > > #include "algorithm.h" > > > > > > > > namespace RPiController { > > > > > > > > +using Matrix3x3 = libcamera::Matrix<double, 3, 3>; > > > > + > > > > class CcmAlgorithm : public Algorithm > > > > { > > > > public: > > > > CcmAlgorithm(Controller *controller) : Algorithm(controller) {} > > > > /* A CCM algorithm must provide the following: */ > > > > + virtual void enableAuto() = 0; > > > > virtual void setSaturation(double saturation) = 0; > > > > + virtual void setCcm(Matrix3x3 const &matrix) = 0; > > > > }; > > > > > > > > } /* namespace RPiController */ > > > > diff --git a/src/ipa/rpi/controller/rpi/ccm.cpp b/src/ipa/rpi/controller/rpi/ccm.cpp > > > > index 8607f152..c42c0141 100644 > > > > --- a/src/ipa/rpi/controller/rpi/ccm.cpp > > > > +++ b/src/ipa/rpi/controller/rpi/ccm.cpp > > > > @@ -32,7 +32,10 @@ LOG_DEFINE_CATEGORY(RPiCcm) > > > > using Matrix3x3 = Matrix<double, 3, 3>; > > > > > > > > Ccm::Ccm(Controller *controller) > > > > - : CcmAlgorithm(controller), saturation_(1.0) {} > > > > + : CcmAlgorithm(controller), enableAuto_(true), saturation_(1.0), > > > > + manualCcm_({ 1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0 }) > > > > +{ > > > > +} > > > > > > > > char const *Ccm::name() const > > > > { > > > > @@ -78,11 +81,22 @@ int Ccm::read(const libcamera::YamlObject ¶ms) > > > > return 0; > > > > } > > > > > > > > +void Ccm::enableAuto() > > > > +{ > > > > + enableAuto_ = true; > > > > +} > > > > + > > > > void Ccm::setSaturation(double saturation) > > > > { > > > > saturation_ = saturation; > > > > } > > > > > > > > +void Ccm::setCcm(Matrix3x3 const &matrix) > > > > +{ > > > > + enableAuto_ = false; > > > > + manualCcm_ = matrix; > > > > +} > > > > + > > > > void Ccm::initialise() > > > > { > > > > } > > > > @@ -151,7 +165,13 @@ void Ccm::prepare(Metadata *imageMetadata) > > > > LOG(RPiCcm, Warning) << "no colour temperature found"; > > > > if (!luxOk) > > > > LOG(RPiCcm, Warning) << "no lux value found"; > > > > - Matrix3x3 ccm = calculateCcm(config_.ccms, awb.temperatureK); > > > > + > > > > + Matrix3x3 ccm; > > > > + if (enableAuto_) > > > > + ccm = calculateCcm(config_.ccms, awb.temperatureK); > > > > + else > > > > + ccm = manualCcm_; > > > > + > > > > double saturation = saturation_; > > > > struct CcmStatus ccmStatus; > > > > ccmStatus.saturation = saturation; > > > > diff --git a/src/ipa/rpi/controller/rpi/ccm.h b/src/ipa/rpi/controller/rpi/ccm.h > > > > index c05dbb17..70f28ed3 100644 > > > > --- a/src/ipa/rpi/controller/rpi/ccm.h > > > > +++ b/src/ipa/rpi/controller/rpi/ccm.h > > > > @@ -8,7 +8,6 @@ > > > > > > > > #include <vector> > > > > > > > > -#include "libcamera/internal/matrix.h" > > > > #include <libipa/pwl.h> > > > > > > > > #include "../ccm_algorithm.h" > > > > @@ -33,13 +32,17 @@ public: > > > > Ccm(Controller *controller = NULL); > > > > char const *name() const override; > > > > int read(const libcamera::YamlObject ¶ms) override; > > > > + void enableAuto() override; > > > > void setSaturation(double saturation) override; > > > > + void setCcm(Matrix3x3 const &matrix) override; > > > > void initialise() override; > > > > void prepare(Metadata *imageMetadata) override; > > > > > > > > private: > > > > CcmConfig config_; > > > > + bool enableAuto_; > > > > double saturation_; > > > > + Matrix3x3 manualCcm_; > > > > }; > > > > > > > > } /* namespace RPiController */ > > > > -- > > > > 2.39.5 > > > >
diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp index ce2343e9..6448e6ab 100644 --- a/src/ipa/rpi/common/ipa_base.cpp +++ b/src/ipa/rpi/common/ipa_base.cpp @@ -94,6 +94,7 @@ const ControlInfoMap::Map ipaColourControls{ { &controls::AwbEnable, ControlInfo(false, true) }, { &controls::AwbMode, ControlInfo(controls::AwbModeValues) }, { &controls::ColourGains, ControlInfo(0.0f, 32.0f) }, + { &controls::ColourCorrectionMatrix, ControlInfo(0.0f, 8.0f) }, { &controls::ColourTemperature, ControlInfo(100, 100000) }, { &controls::Saturation, ControlInfo(0.0f, 32.0f, 1.0f) }, }; @@ -126,7 +127,7 @@ namespace ipa::RPi { IpaBase::IpaBase() : controller_(), frameLengths_(FrameLengthsQueueSize, 0s), statsMetadataOutput_(false), stitchSwapBuffers_(false), frameCount_(0), mistrustCount_(0), lastRunTimestamp_(0), - firstStart_(true), flickerState_({ 0, 0s }) + firstStart_(true), flickerState_({ 0, 0s }), awbEnabled_(true) { } @@ -821,6 +822,102 @@ void IpaBase::applyControls(const ControlList &controls) } } + /* + * We must also handle any AWB on/off changes first, so that the CCM algorithm + * knows its state correctly. + */ + const auto awbEnable = controls.get(controls::AwbEnable); + if (awbEnable) + do { + /* Silently ignore this control for a mono sensor. */ + if (monoSensor_) + break; + + RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( + controller_.getAlgorithm("awb")); + if (!awb) { + LOG(IPARPI, Warning) + << "Could not set AWB_ENABLE - no AWB algorithm"; + break; + } + + awbEnabled_ = *awbEnable; + + if (!awbEnabled_) + awb->disableAuto(); + else { + awb->enableAuto(); + + /* The CCM algorithm must go back to auto as well. */ + RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>( + controller_.getAlgorithm("ccm")); + if (ccm) + ccm->enableAuto(); + } + + libcameraMetadata_.set(controls::AwbEnable, awbEnabled_); + } while (false); + + const auto colourGains = controls.get(controls::ColourGains); + if (colourGains) + do { + /* Silently ignore this control for a mono sensor. */ + if (monoSensor_) + break; + + auto gains = *colourGains; + RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( + controller_.getAlgorithm("awb")); + if (!awb) { + LOG(IPARPI, Warning) + << "Could not set COLOUR_GAINS - no AWB algorithm"; + break; + } + + awb->setManualGains(gains[0], gains[1]); + if (gains[0] != 0.0f && gains[1] != 0.0f) { + /* A gain of 0.0f will switch back to auto mode. */ + libcameraMetadata_.set(controls::ColourGains, + { gains[0], gains[1] }); + + awbEnabled_ = false; /* doing this puts AWB into manual mode */ + } else { + awbEnabled_ = true; /* doing this puts AWB into auto mode */ + + /* The CCM algorithm must go back to auto as well. */ + RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>( + controller_.getAlgorithm("ccm")); + if (ccm) + ccm->enableAuto(); + } + + /* This metadata will get reported back automatically. */ + break; + } while (false); + + const auto colourTemperature = controls.get(controls::ColourTemperature); + if (colourTemperature) + do { + /* Silently ignore this control for a mono sensor. */ + if (monoSensor_) + break; + + auto temperatureK = *colourTemperature; + RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( + controller_.getAlgorithm("awb")); + if (!awb) { + LOG(IPARPI, Warning) + << "Could not set COLOUR_TEMPERATURE - no AWB algorithm"; + break; + } + + awb->setColourTemperature(temperatureK); + awbEnabled_ = false; /* doing this puts AWB into manual mode */ + + /* This metadata will get reported back automatically. */ + break; + } while (false); + /* Iterate over controls */ for (auto const &ctrl : controls) { LOG(IPARPI, Debug) << "Request ctrl: " @@ -1037,25 +1134,7 @@ void IpaBase::applyControls(const ControlList &controls) } case controls::AWB_ENABLE: { - /* Silently ignore this control for a mono sensor. */ - if (monoSensor_) - break; - - RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( - controller_.getAlgorithm("awb")); - if (!awb) { - LOG(IPARPI, Warning) - << "Could not set AWB_ENABLE - no AWB algorithm"; - break; - } - - if (ctrl.second.get<bool>() == false) - awb->disableAuto(); - else - awb->enableAuto(); - - libcameraMetadata_.set(controls::AwbEnable, - ctrl.second.get<bool>()); + /* We handled this one above. */ break; } @@ -1080,47 +1159,60 @@ void IpaBase::applyControls(const ControlList &controls) LOG(IPARPI, Error) << "AWB mode " << idx << " not recognised"; } + break; } case controls::COLOUR_GAINS: { - /* Silently ignore this control for a mono sensor. */ - if (monoSensor_) - break; - - auto gains = ctrl.second.get<Span<const float>>(); - RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( - controller_.getAlgorithm("awb")); - if (!awb) { - LOG(IPARPI, Warning) - << "Could not set COLOUR_GAINS - no AWB algorithm"; - break; - } - - awb->setManualGains(gains[0], gains[1]); - if (gains[0] != 0.0f && gains[1] != 0.0f) - /* A gain of 0.0f will switch back to auto mode. */ - libcameraMetadata_.set(controls::ColourGains, - { gains[0], gains[1] }); + /* We handled this one above. */ break; } case controls::COLOUR_TEMPERATURE: { - /* Silently ignore this control for a mono sensor. */ + /* We handled this one above. */ + break; + } + + case controls::COLOUR_CORRECTION_MATRIX: { if (monoSensor_) break; - auto temperatureK = ctrl.second.get<int32_t>(); + auto floats = ctrl.second.get<Span<const float>>(); + RPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>( + controller_.getAlgorithm("ccm")); + if (!ccm) { + LOG(IPARPI, Warning) + << "Could not set COLOUR_CORRECTION_MATRIX - no CCM algorithm"; + break; + } + RPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>( controller_.getAlgorithm("awb")); - if (!awb) { + if (awb && awbEnabled_) { LOG(IPARPI, Warning) - << "Could not set COLOUR_TEMPERATURE - no AWB algorithm"; + << "Could not set COLOUR_CORRECTION_MATRIX - AWB is active"; break; } - awb->setColourTemperature(temperatureK); - /* This metadata will get reported back automatically. */ + /* We are guaranteed this control contains 9 values. Nevertheless: */ + assert(floats.size() == 9); + + Matrix<double, 3, 3> matrix; + for (std::size_t i = 0; i < 3; ++i) + for (std::size_t j = 0; j < 3; ++j) + matrix[i][j] = static_cast<double>(floats[i * 3 + j]); + + ccm->setCcm(matrix); + + /* + * But if AWB is running, go back to auto mode. The CCM gets remembered, + * which avoids the race between setting the CCM and disabling AWB in + * the same set of controls. + */ + if (awbEnabled_) + ccm->enableAuto(); + + /* This metadata will be reported back automatically. */ break; } diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h index e2f6e330..5348f2ea 100644 --- a/src/ipa/rpi/common/ipa_base.h +++ b/src/ipa/rpi/common/ipa_base.h @@ -140,6 +140,8 @@ private: int32_t mode; utils::Duration manualPeriod; } flickerState_; + + bool awbEnabled_; }; } /* namespace ipa::RPi */ diff --git a/src/ipa/rpi/controller/ccm_algorithm.h b/src/ipa/rpi/controller/ccm_algorithm.h index 6678ba75..785c408a 100644 --- a/src/ipa/rpi/controller/ccm_algorithm.h +++ b/src/ipa/rpi/controller/ccm_algorithm.h @@ -6,16 +6,22 @@ */ #pragma once +#include "libcamera/internal/matrix.h" + #include "algorithm.h" namespace RPiController { +using Matrix3x3 = libcamera::Matrix<double, 3, 3>; + class CcmAlgorithm : public Algorithm { public: CcmAlgorithm(Controller *controller) : Algorithm(controller) {} /* A CCM algorithm must provide the following: */ + virtual void enableAuto() = 0; virtual void setSaturation(double saturation) = 0; + virtual void setCcm(Matrix3x3 const &matrix) = 0; }; } /* namespace RPiController */ diff --git a/src/ipa/rpi/controller/rpi/ccm.cpp b/src/ipa/rpi/controller/rpi/ccm.cpp index 8607f152..c42c0141 100644 --- a/src/ipa/rpi/controller/rpi/ccm.cpp +++ b/src/ipa/rpi/controller/rpi/ccm.cpp @@ -32,7 +32,10 @@ LOG_DEFINE_CATEGORY(RPiCcm) using Matrix3x3 = Matrix<double, 3, 3>; Ccm::Ccm(Controller *controller) - : CcmAlgorithm(controller), saturation_(1.0) {} + : CcmAlgorithm(controller), enableAuto_(true), saturation_(1.0), + manualCcm_({ 1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0 }) +{ +} char const *Ccm::name() const { @@ -78,11 +81,22 @@ int Ccm::read(const libcamera::YamlObject ¶ms) return 0; } +void Ccm::enableAuto() +{ + enableAuto_ = true; +} + void Ccm::setSaturation(double saturation) { saturation_ = saturation; } +void Ccm::setCcm(Matrix3x3 const &matrix) +{ + enableAuto_ = false; + manualCcm_ = matrix; +} + void Ccm::initialise() { } @@ -151,7 +165,13 @@ void Ccm::prepare(Metadata *imageMetadata) LOG(RPiCcm, Warning) << "no colour temperature found"; if (!luxOk) LOG(RPiCcm, Warning) << "no lux value found"; - Matrix3x3 ccm = calculateCcm(config_.ccms, awb.temperatureK); + + Matrix3x3 ccm; + if (enableAuto_) + ccm = calculateCcm(config_.ccms, awb.temperatureK); + else + ccm = manualCcm_; + double saturation = saturation_; struct CcmStatus ccmStatus; ccmStatus.saturation = saturation; diff --git a/src/ipa/rpi/controller/rpi/ccm.h b/src/ipa/rpi/controller/rpi/ccm.h index c05dbb17..70f28ed3 100644 --- a/src/ipa/rpi/controller/rpi/ccm.h +++ b/src/ipa/rpi/controller/rpi/ccm.h @@ -8,7 +8,6 @@ #include <vector> -#include "libcamera/internal/matrix.h" #include <libipa/pwl.h> #include "../ccm_algorithm.h" @@ -33,13 +32,17 @@ public: Ccm(Controller *controller = NULL); char const *name() const override; int read(const libcamera::YamlObject ¶ms) override; + void enableAuto() override; void setSaturation(double saturation) override; + void setCcm(Matrix3x3 const &matrix) override; void initialise() override; void prepare(Metadata *imageMetadata) override; private: CcmConfig config_; + bool enableAuto_; double saturation_; + Matrix3x3 manualCcm_; }; } /* namespace RPiController */
The CCM algorithm will now let an explicit colour matrix be set when AWB is in manual mode. We must handle any controls that can cause the AWB to be enabled or disabled first, so that we know the AWB's state correctly when we come to set the CCM. Signed-off-by: David Plowman <david.plowman@raspberrypi.com> --- src/ipa/rpi/common/ipa_base.cpp | 180 +++++++++++++++++++------ src/ipa/rpi/common/ipa_base.h | 2 + src/ipa/rpi/controller/ccm_algorithm.h | 6 + src/ipa/rpi/controller/rpi/ccm.cpp | 24 +++- src/ipa/rpi/controller/rpi/ccm.h | 5 +- 5 files changed, 170 insertions(+), 47 deletions(-)