[PATCH v2 11/17] ipa: rkisp1: Implement manual ColourCorrectionMatrix control
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Apr 1 23:30:05 CEST 2025
On Mon, Mar 31, 2025 at 06:39:48PM +0100, Kieran Bingham wrote:
> Quoting Stefan Klug (2025-03-19 16:11:16)
> > Add a manual ColourCorrectionMatrix control. This was already discussed
> > while implementing manual colour temperature but was never implemented.
> > The control allows to manually specify the CCM when AwbEnable is false.
> >
> > Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>
> >
> > ---
> >
> > Changes in v2:
> > - None
> > ---
> > src/ipa/rkisp1/algorithms/ccm.cpp | 59 ++++++++++++++++++++++++++++---
> > src/ipa/rkisp1/algorithms/ccm.h | 6 ++++
> > src/ipa/rkisp1/ipa_context.h | 3 +-
> > 3 files changed, 62 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp
> > index 2e5e91006b55..303ac3dd2fe2 100644
> > --- a/src/ipa/rkisp1/algorithms/ccm.cpp
> > +++ b/src/ipa/rkisp1/algorithms/ccm.cpp
> > @@ -36,17 +36,25 @@ namespace ipa::rkisp1::algorithms {
> >
> > LOG_DEFINE_CATEGORY(RkISP1Ccm)
> >
> > +constexpr Matrix<float, 3, 3> kIdentity3x3 = Matrix<float, 3, 3>::identity();
> > +
> > /**
> > * \copydoc libcamera::ipa::Algorithm::init
> > */
> > int Ccm::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)
> > {
> > + auto &cmap = context.ctrlMap;
> > + cmap[&controls::ColourCorrectionMatrix] = ControlInfo(
> > + ControlValue(-8.0f),
> > + ControlValue(7.993f),
> > + ControlValue(kIdentity3x3.data()));
> > +
> > int ret = ccm_.readYaml(tuningData["ccms"], "ct", "ccm");
> > if (ret < 0) {
> > LOG(RkISP1Ccm, Warning)
> > << "Failed to parse 'ccm' "
> > << "parameter from tuning file; falling back to unit matrix";
> > - ccm_.setData({ { 0, Matrix<float, 3, 3>::identity() } });
> > + ccm_.setData({ { 0, kIdentity3x3 } });
> > }
> >
> > ret = offsets_.readYaml(tuningData["ccms"], "ct", "offsets");
> > @@ -61,13 +69,48 @@ int Ccm::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData
> > return 0;
> > }
> >
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::configure
> > + */
> > +int Ccm::configure(IPAContext &context,
> > + [[maybe_unused]] const IPACameraSensorInfo &configInfo)
> > +{
> > + auto &as = context.activeState;
> > + as.ccm.manual = kIdentity3x3;
> > + as.ccm.automatic = ccm_.getInterpolated(as.awb.automatic.temperatureK);
> > + LOG(RkISP1Ccm, Debug) << "init matrix " << as.ccm.manual;
>
> I'm not sure that debug line adds much value...
>
> But aside from that, it seems to match the control documentation.
>
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
>
> > + return 0;
> > +}
> > +
> > +void Ccm::queueRequest(IPAContext &context,
> > + [[maybe_unused]] const uint32_t frame,
> > + IPAFrameContext &frameContext,
> > + const ControlList &controls)
> > +{
> > + /* Nothing to do here, the ccm will be calculated in prepare() */
> > + if (frameContext.awb.autoEnabled)
> > + return;
> > +
> > + auto &ccm = context.activeState.ccm;
> > +
> > + const auto &colourTemperature = controls.get(controls::ColourTemperature);
> > + const auto &ccmMatrix = controls.get(controls::ColourCorrectionMatrix);
> > + if (ccmMatrix)
> > + ccm.manual = Matrix<float, 3, 3>(*ccmMatrix);
> > + else if (colourTemperature)
> > + ccm.manual = ccm_.getInterpolated(*colourTemperature);
> > +
> > + LOG(RkISP1Ccm, Debug) << "queueRequest matrix " << ccm.manual;
To make the log less noisy, should this be restricted to when a CT or
CCM matrix control was set ? Or maybe also indicate where the CCM came
from ?
if (ccmMatrix) {
ccm.manual = Matrix<float, 3, 3>(*ccmMatrix);
LOG(RkISP1Ccm, Debug)
<< "Setting manual CCM from CCM control to " << ccm.manual;
} else if (colourTemperature) {
ccm.manual = ccm_.getInterpolated(*colourTemperature);
LOG(RkISP1Ccm, Debug)
<< "Setting manual CCM from CT control to " << ccm.manual;
}
or something similar.
> > + frameContext.ccm.ccm = ccm.manual;
> > +}
> > +
> > void Ccm::setParameters(struct rkisp1_cif_isp_ctk_config &config,
> > const Matrix<float, 3, 3> &matrix,
> > const Matrix<int16_t, 3, 1> &offsets)
> > {
> > /*
> > * 4 bit integer and 7 bit fractional, ranging from -8 (0x400) to
> > - * +7.992 (0x3ff)
> > + * +7.9921875 (0x3ff)
Someone likes precision :-)
> > */
> > for (unsigned int i = 0; i < 3; i++) {
> > for (unsigned int j = 0; j < 3; j++)
> > @@ -88,14 +131,20 @@ void Ccm::setParameters(struct rkisp1_cif_isp_ctk_config &config,
> > void Ccm::prepare(IPAContext &context, const uint32_t frame,
> > IPAFrameContext &frameContext, RkISP1Params *params)
> > {
> > - uint32_t ct = frameContext.awb.temperatureK;
> > + if (!frameContext.awb.autoEnabled) {
> > + auto config = params->block<BlockType::Ctk>();
> > + config.setEnabled(true);
> > + setParameters(*config, frameContext.ccm.ccm, Matrix<int16_t, 3, 1>());
> > + return;
> > + }
> >
> > + uint32_t ct = frameContext.awb.temperatureK;
> > /*
> > * \todo The colour temperature will likely be noisy, add filtering to
> > * avoid updating the CCM matrix all the time.
> > */
> > if (frame > 0 && ct == ct_) {
> > - frameContext.ccm.ccm = context.activeState.ccm.ccm;
> > + frameContext.ccm.ccm = context.activeState.ccm.automatic;
> > return;
> > }
> >
> > @@ -103,7 +152,7 @@ void Ccm::prepare(IPAContext &context, const uint32_t frame,
> > Matrix<float, 3, 3> ccm = ccm_.getInterpolated(ct);
> > Matrix<int16_t, 3, 1> offsets = offsets_.getInterpolated(ct);
> >
> > - context.activeState.ccm.ccm = ccm;
> > + context.activeState.ccm.automatic = ccm;
> > frameContext.ccm.ccm = ccm;
> >
> > auto config = params->block<BlockType::Ctk>();
> > diff --git a/src/ipa/rkisp1/algorithms/ccm.h b/src/ipa/rkisp1/algorithms/ccm.h
> > index a5d9a9a45e5d..c301e6e531c8 100644
> > --- a/src/ipa/rkisp1/algorithms/ccm.h
> > +++ b/src/ipa/rkisp1/algorithms/ccm.h
> > @@ -26,6 +26,12 @@ public:
> > ~Ccm() = default;
> >
> > int init(IPAContext &context, const YamlObject &tuningData) override;
> > + int configure(IPAContext &context,
> > + const IPACameraSensorInfo &configInfo) override;
> > + void queueRequest(IPAContext &context,
> > + const uint32_t frame,
> > + IPAFrameContext &frameContext,
> > + const ControlList &controls) override;
> > void prepare(IPAContext &context, const uint32_t frame,
> > IPAFrameContext &frameContext,
> > RkISP1Params *params) override;
> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> > index 769e9f114e23..f0d504215d34 100644
> > --- a/src/ipa/rkisp1/ipa_context.h
> > +++ b/src/ipa/rkisp1/ipa_context.h
> > @@ -101,7 +101,8 @@ struct IPAActiveState {
> > } awb;
> >
> > struct {
> > - Matrix<float, 3, 3> ccm;
> > + Matrix<float, 3, 3> manual;
> > + Matrix<float, 3, 3> automatic;
Missing documentation updates.
> > } ccm;
> >
> > struct {
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list