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

[Laurent Pinchart]

On Tue, Dec 03, 2024 at 06:19:41PM +0100, Hans de Goede wrote:
> 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.

They do, but they typically apply colour gains before CFA interpolation,
not after.

> 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

-- 
Regards,

Laurent Pinchart