[PATCH v3 6/9] libcamera: software_isp: Add CCM algorithm

[Milan Zamazal]

Hi Kieran,

thank you for review.

Kieran Bingham <kieran.bingham at ideasonboard.com> writes:

> Quoting Milan Zamazal (2024-12-10 15:34:36)
>> This patch adds color correction matrix (CCM) algorithm to software ISP.
>> It is based on the corresponding algorithm in rkisp1.
>
>> 
>> The primary difference against hardware pipelines is that applying the
>> CCM is optional.  Applying CCM causes a significant slowdown, time
>> needed to process a frame raises by 40-90% on tested platforms.  If CCM
>
> Ouch... :-(

It's not that bad, considering how heavy-weight the operation it is.
It's usable on reasonable CPUs, especially for testing.  And as Laurent
pointed out, is likely to be much better with GPU.

>> is really needed, it can be applied, if not, it's better to stick
>> without it.  This can be configured by presence or omission of Ccm
>> algorithm in the tuning file.
>> 
>> CCM is changed only if the determined temperature changes by at least
>> 100 K (an arbitrarily selected value), to avoid recomputing the matrices
>
> I think I'd be comfortable with anything between 50 and 500 here, so I
> think this is a good starting point.
>
> The real answer will come from looking at how much the measured colour
> changes by in a given capture session!
>
>
>> and lookup tables all the time.  Some ugly unsigned->signed int
>> typecasting is needed to compare the temperatures.
>> 
>> The outputs of the algorithm are not used yet, they will be enabled in
>> followup patches.
>> 
>> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
>> ---
>>  src/ipa/simple/algorithms/ccm.cpp     | 88 +++++++++++++++++++++++++++
>>  src/ipa/simple/algorithms/ccm.h       | 45 ++++++++++++++
>>  src/ipa/simple/algorithms/meson.build |  1 +
>>  src/ipa/simple/ipa_context.h          | 13 ++++
>>  4 files changed, 147 insertions(+)
>>  create mode 100644 src/ipa/simple/algorithms/ccm.cpp
>>  create mode 100644 src/ipa/simple/algorithms/ccm.h
>> 
>> diff --git a/src/ipa/simple/algorithms/ccm.cpp b/src/ipa/simple/algorithms/ccm.cpp
>> new file mode 100644
>> index 00000000..40fe12c8
>> --- /dev/null
>> +++ b/src/ipa/simple/algorithms/ccm.cpp
>> @@ -0,0 +1,88 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2024, Ideas On Board
>> + * Copyright (C) 2024, Red Hat Inc.
>> + *
>> + * Color correction matrix
>> + */
>> +
>> +#include "ccm.h"
>> +
>> +#include <stdlib.h>
>> +
>> +#include <libcamera/base/log.h>
>> +
>> +#include <libcamera/control_ids.h>
>> +
>> +namespace libcamera {
>> +
>> +namespace ipa::soft::algorithms {
>> +
>> +LOG_DEFINE_CATEGORY(IPASoftCcm)
>> +
>> +unsigned int Ccm::kTemperatureThreshold = 100;
>> +
>> +int Ccm::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)
>> +{
>> +       int ret = ccm_.readYaml(tuningData["ccms"], "ct", "ccm");
>> +       if (ret < 0) {
>> +               LOG(IPASoftCcm, Warning)
>> +                       << "Failed to parse 'ccm' "
>> +                       << "parameter from tuning file; falling back to unit matrix";
>> +               ccmEnabled_ = false;
>> +       } else {
>> +               ccmEnabled_ = true;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +void Ccm::prepare(IPAContext &context, const uint32_t frame,
>> +                 IPAFrameContext &frameContext, [[maybe_unused]] DebayerParams *params)
>> +{
>> +       context.activeState.ccm.enabled = ccmEnabled_;
>> +
>> +       if (!ccmEnabled_)
>> +               return;
>> +
>> +       unsigned int ct = context.activeState.awb.temperatureK;
>> +
>> +       /* Change CCM only on bigger temperature changes. */
>> +       if (frame > 0 &&
>> +           abs(static_cast<int>(ct) - static_cast<int>(ct_)) <
>> +                   static_cast<int>(kTemperatureThreshold)) {
>
> Does utils::abs_diff() solve this ?

It does, thank you.

> Aside from that:
>
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
>> +               frameContext.ccm.ccm = context.activeState.ccm.ccm;
>> +               context.activeState.ccm.changed = false;
>> +               return;
>> +       }
>> +
>> +       ct_ = ct;
>> +       Matrix<double, 3, 3> ccm = ccm_.getInterpolated(ct);
>> +
>> +       context.activeState.ccm.ccm = ccm;
>> +       frameContext.ccm.ccm = ccm;
>> +       context.activeState.ccm.changed = true;
>> +}
>> +
>> +void Ccm::process([[maybe_unused]] IPAContext &context,
>> +                 [[maybe_unused]] const uint32_t frame,
>> +                 IPAFrameContext &frameContext,
>> +                 [[maybe_unused]] const SwIspStats *stats,
>> +                 ControlList &metadata)
>> +{
>> +       if (!ccmEnabled_)
>> +               return;
>> +
>> +       float m[9];
>> +       for (unsigned int i = 0; i < 3; i++) {
>> +               for (unsigned int j = 0; j < 3; j++)
>> +                       m[i * 3 + j] = frameContext.ccm.ccm[i][j];
>> +       }
>> +       metadata.set(controls::ColourCorrectionMatrix, m);
>> +}
>> +
>> +REGISTER_IPA_ALGORITHM(Ccm, "Ccm")
>> +
>> +} /* namespace ipa::soft::algorithms */
>> +
>> +} /* namespace libcamera */
>> diff --git a/src/ipa/simple/algorithms/ccm.h b/src/ipa/simple/algorithms/ccm.h
>> new file mode 100644
>> index 00000000..23481a08
>> --- /dev/null
>> +++ b/src/ipa/simple/algorithms/ccm.h
>> @@ -0,0 +1,45 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2024, Red Hat Inc.
>> + *
>> + * Color correction matrix
>> + */
>> +
>> +#pragma once
>> +
>> +#include "libcamera/internal/matrix.h"
>> +
>> +#include <libipa/interpolator.h>
>> +
>> +#include "algorithm.h"
>> +
>> +namespace libcamera {
>> +
>> +namespace ipa::soft::algorithms {
>> +
>> +class Ccm : public Algorithm
>> +{
>> +public:
>> +       Ccm() = default;
>> +       ~Ccm() = default;
>> +
>> +       int init(IPAContext &context, const YamlObject &tuningData) override;
>> +       void prepare(IPAContext &context,
>> +                    const uint32_t frame,
>> +                    IPAFrameContext &frameContext,
>> +                    DebayerParams *params) override;
>> +       void process(IPAContext &context, const uint32_t frame,
>> +                    IPAFrameContext &frameContext,
>> +                    const SwIspStats *stats,
>> +                    ControlList &metadata) override;
>> +
>> +private:
>> +       static unsigned int kTemperatureThreshold;
>> +       unsigned int ct_;
>> +       bool ccmEnabled_;
>> +       Interpolator<Matrix<double, 3, 3>> ccm_;
>> +};
>> +
>> +} /* namespace ipa::soft::algorithms */
>> +
>> +} /* namespace libcamera */
>> diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build
>> index 37a2eb53..2d0adb05 100644
>> --- a/src/ipa/simple/algorithms/meson.build
>> +++ b/src/ipa/simple/algorithms/meson.build
>> @@ -4,5 +4,6 @@ soft_simple_ipa_algorithms = files([
>>      'awb.cpp',
>>      'agc.cpp',
>>      'blc.cpp',
>> +    'ccm.cpp',
>>      'lut.cpp',
>>  ])
>> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h
>> index 607af45a..0def3eef 100644
>> --- a/src/ipa/simple/ipa_context.h
>> +++ b/src/ipa/simple/ipa_context.h
>> @@ -13,6 +13,8 @@
>>  
>>  #include <libcamera/controls.h>
>>  
>> +#include "libcamera/internal/matrix.h"
>> +
>>  #include <libipa/fc_queue.h>
>>  
>>  namespace libcamera {
>> @@ -50,6 +52,13 @@ struct IPAActiveState {
>>                 uint8_t blackLevel;
>>                 double contrast;
>>         } gamma;
>> +
>> +       struct {
>> +               Matrix<double, 3, 3> ccm;
>> +               bool enabled;
>> +               bool changed;
>> +       } ccm;
>> +
>>         struct {
>>                 /* 0..2 range, 1.0 = normal */
>>                 std::optional<double> contrast;
>> @@ -57,6 +66,10 @@ struct IPAActiveState {
>>  };
>>  
>>  struct IPAFrameContext : public FrameContext {
>> +       struct {
>> +               Matrix<double, 3, 3> ccm;
>> +       } ccm;
>> +
>>         struct {
>>                 int32_t exposure;
>>                 double gain;
>> -- 
>> 2.44.2
>>