[PATCH v2 0/9] Software ISP support for CCM

[Hans de Goede]

Hi Milan,

On 25-Nov-24 7:03 PM, Milan Zamazal wrote:
> This series implements application of color correction matrices in
> software ISP.
> 
> Support for color temperature is added to auto white balance algorithm,
> this is needed to obtain the right CCM.  A new Ccm algorithm is added to
> determine the matrix.  Both roughly follow what the hardware pipelines
> do.
> 
> Lut algorithm is modified to incorporate the CCM and perform some
> precomputations in the form of 10 lookup tables (one per each matrix
> element and one for gamma).
> 
> A tricky part is to support both CCM and the original (no-CCM)
> transformations.  CCM obviously adds a significant overhead; per-frame
> time increases by ~45% on Debix Model A and ~85% on TI AM69 SK.  This
> means CCM must be optional, which is determined by presence of Ccm
> algorithm in the tuning file.
> 
> The performance critical debayering code must not include extra
> conditionals and must work efficiently for both the cases.  The macros
> in debayering code are refactored and restructured for this.  An added
> bonus is that this also removes some annoying code duplication.  And
> some more, partially already present, templating is used to select
> between CCM and non-CCM processing.
> 
> Performance is much influenced by the data structures used in
> debayering.  I tried to define something reasonable and performed only
> basic performance testing.  Suggestions for possible improvements are
> welcome.

Thank you for writing this. I have tested this on a ThinkPad X1 Yoga
with a 13th Gen Intel(R) CoreT i7-1370P.

Pinning qcam and all its threads to only the 6 performance cores /
12 threads:

taskset 0xfff ./redhat-linux-build/src/apps/qcam/qcam

I consistently get 6.3 ms / frame processing time without the CCM
both with and without your patches and 9.5 ms / frame with the CCM
snippet in uncalibrated.yaml activated. So it seems that we should
be able to activate the CCM with the software ISP in the IPU6 case.

I did notice that the image becomes quite green with the default
1 - 1 - 1  CCM, so it seems that currently using the CCM ignores
the color-gains calculated by the simple IPA.

I'm pretty sure that real ISP-s have both color gains and a CCM,
so I think that we should apply the color gains to the CCM
when calculating the lookup tables.

E.g. multiply CcmColumn.r by red-gain before using it to fill
the lookup-table, etc.

And then it would be up to the IPA code to either do
whitebalance through the gains or directly in the ccm,
e.g. when using a ccm to enhance saturation then awb
could still be done through the gains.

Regards,

Hans



> 
> See the commit messages for more details.
> 
> Changes in v2:
> - Fix of the gamma table index upper boundary.
> - Fix of red<->blue being temporarily swapped in an intermediate patch.
> - Fix of CCM transformation when swapRedBlueGains_ is true.
> - A related minor lookup data structures rearrangement, with an
>   unintended but welcome effect of ~10% speedup with CCM in my
>   environment. 
> 
> Milan Zamazal (9):
>   libcamera: software_isp: Determine color temperature
>   libcamera: software_isp: Store color temperature to metadata
>   libcamera: software_isp: lut: Remove maybe_unused on a used argument
>   libcamera: software_isp: Use common code to store debayered pixels
>   libcamera: software_isp: Use a macro to assign debayering methods
>   libcamera: software_isp: Add CCM algorithm
>   libcamera: software_isp: Add an example CCM to uncalibrated.yaml
>   libcamera: software_isp: Track whether CCM is enabled
>   libcamera: software_isp: Apply CCM in debayering
> 
>  .../internal/software_isp/debayer_params.h    |  20 ++-
>  include/libcamera/ipa/soft.mojom              |   2 +-
>  src/ipa/simple/algorithms/awb.cpp             |  18 ++-
>  src/ipa/simple/algorithms/ccm.cpp             |  86 ++++++++++++
>  src/ipa/simple/algorithms/ccm.h               |  45 ++++++
>  src/ipa/simple/algorithms/lut.cpp             |  62 +++++---
>  src/ipa/simple/algorithms/lut.h               |   1 +
>  src/ipa/simple/algorithms/meson.build         |   1 +
>  src/ipa/simple/data/uncalibrated.yaml         |   7 +
>  src/ipa/simple/ipa_context.h                  |  22 ++-
>  src/ipa/simple/soft_simple.cpp                |   8 +-
>  src/libcamera/software_isp/debayer.cpp        |  53 ++++++-
>  src/libcamera/software_isp/debayer.h          |   3 +-
>  src/libcamera/software_isp/debayer_cpu.cpp    | 132 ++++++++++--------
>  src/libcamera/software_isp/debayer_cpu.h      |  31 ++--
>  src/libcamera/software_isp/software_isp.cpp   |  11 +-
>  16 files changed, 393 insertions(+), 109 deletions(-)
>  create mode 100644 src/ipa/simple/algorithms/ccm.cpp
>  create mode 100644 src/ipa/simple/algorithms/ccm.h
>