[PATCH v7 10/10] libcamera: software_isp: Apply CCM in debayering
Milan Zamazal
mzamazal at redhat.com
Wed Mar 26 10:11:34 CET 2025
Hi Laurent,
thank you for review.
Laurent Pinchart <laurent.pinchart at ideasonboard.com> writes:
> Hi Milan,
>
> Thank you for the patch.
>
> On Fri, Feb 07, 2025 at 11:17:02AM +0100, Milan Zamazal wrote:
>> This patch applies color correction matrix (CCM) in debayering if the
>> CCM is specified. Not using CCM must still be supported for performance
>> reasons.
>>
>> The CCM is applied as follows:
>>
>> [r1 g1 b1] [r]
>> [r2 g2 b2] * [g]
>> [r3 g3 b3] [b]
>>
>> The CCM matrix (the left side of the multiplication) is constant during
>> single frame processing, while the input pixel (the right side) changes.
>> Because each of the color channels is only 8-bit in software ISP, we can
>> make 9 lookup tables with 256 input values for multiplications of each
>> of the r_i, g_i, b_i values. This way we don't have to multiply each
>> pixel, we can use table lookups and additions instead. Gamma (which is
>> non-linear and thus cannot be a part of the 9 lookup tables values) is
>> applied on the final values rounded to integers using another lookup
>> table.
>>
>> Because the changing part is the pixel value with three color elements,
>> only three dynamic table lookups are needed. We use three lookup tables
>> to represent the multiplied matrix values, each of the tables
>> corresponding to the given matrix column and pixel color.
>>
>> We use int16_t to store the precomputed multiplications. This seems to
>> be noticeably (>10%) faster than `float' for the price of slightly less
>> accuracy and it covers the range of values that sane CCMs produce. The
>> selection and structure of data is performance critical, for example
>> using bytes would add significant (>10%) speedup but would be too short
>> to cover the value range.
>>
>> The color lookup tables can be represented either as unions,
>> accommodating tables for both the CCM and non-CCM cases, or as separate
>> tables for each of the cases, leaving the tables for the other case
>> unused. The latter is selected as a matter of preference.
>>
>> The tables are copied (as before), which is not elegant but also not a
>> big problem. There are patches posted that use shared buffers for
>> parameters passing in software ISP (see software ISP TODO #5) and they
>> can be adjusted for the new parameter format.
>>
>> Color gains from white balance are supposed not to be a part of the
>> specified CCM. They are applied on it using matrix multiplication,
>> which is simple and in correspondence with future additions in the form
>> of matrix multiplication, like saturation adjustment.
>>
>> With this patch, the reported per-frame slowdown when applying CCM is
>> about 45% on Debix Model A and about 75% on TI AM69 SK.
>>
>> Using std::clamp in debayering adds some performance penalty (a few
>> percent). The clamping is necessary to eliminate out of range values
>> possibly produced by the CCM. If it could be avoided by adjusting the
>> precomputed tables some way then performance could be improved a bit.
>>
>> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
>> ---
>> .../internal/software_isp/debayer_params.h | 34 ++++++++++--
>> src/ipa/simple/algorithms/lut.cpp | 53 ++++++++++++++-----
>> src/ipa/simple/algorithms/lut.h | 1 +
>> src/libcamera/software_isp/debayer.cpp | 51 ++++++++++++++++--
>> src/libcamera/software_isp/debayer_cpu.cpp | 53 +++++++++++++++----
>> src/libcamera/software_isp/debayer_cpu.h | 10 ++--
>> 6 files changed, 169 insertions(+), 33 deletions(-)
>>
>> diff --git a/include/libcamera/internal/software_isp/debayer_params.h b/include/libcamera/internal/software_isp/debayer_params.h
>> index 7d8fdd48..e85b5c60 100644
>> --- a/include/libcamera/internal/software_isp/debayer_params.h
>> +++ b/include/libcamera/internal/software_isp/debayer_params.h
>> @@ -1,6 +1,6 @@
>> /* SPDX-License-Identifier: LGPL-2.1-or-later */
>> /*
>> - * Copyright (C) 2023, 2024 Red Hat Inc.
>> + * Copyright (C) 2023-2025 Red Hat Inc.
>> *
>> * Authors:
>> * Hans de Goede <hdegoede at redhat.com>
>> @@ -18,11 +18,35 @@ namespace libcamera {
>> struct DebayerParams {
>> static constexpr unsigned int kRGBLookupSize = 256;
>>
>> - using ColorLookupTable = std::array<uint8_t, kRGBLookupSize>;
>> + struct CcmColumn {
>> + int16_t r;
>> + int16_t g;
>> + int16_t b;
>> + };
>>
>> - ColorLookupTable red;
>> - ColorLookupTable green;
>> - ColorLookupTable blue;
>> + using LookupTable = std::array<uint8_t, kRGBLookupSize>;
>> + using CcmLookupTable = std::array<CcmColumn, kRGBLookupSize>;
>> +
>> + /*
>> + * Color lookup tables when CCM is not used.
>
> Add a blank line, or remove the line break.
>
>> + * Each color of a debayered pixel is amended by the corresponding
>> + * value in the given table.
>> + */
>> + LookupTable red;
>> + LookupTable green;
>> + LookupTable blue;
>> +
>> + /*
>> + * Color and gamma lookup tables when CCM is used.
>
> Same here.
>
>> + * Each of the CcmLookupTable's corresponds to a CCM column; together they
>> + * make a complete 3x3 CCM lookup table. The CCM is applied on debayered
>> + * pixels and then the gamma lookup table is used to set the resulting
>> + * values of all the three colors.
>> + */
>> + CcmLookupTable redCcm;
>> + CcmLookupTable greenCcm;
>> + CcmLookupTable blueCcm;
>> + LookupTable gammaLut;
>> };
>>
>> } /* namespace libcamera */
>> diff --git a/src/ipa/simple/algorithms/lut.cpp b/src/ipa/simple/algorithms/lut.cpp
>> index 352bbf57..1eaa2b5e 100644
>> --- a/src/ipa/simple/algorithms/lut.cpp
>> +++ b/src/ipa/simple/algorithms/lut.cpp
>> @@ -1,6 +1,6 @@
>> /* SPDX-License-Identifier: LGPL-2.1-or-later */
>> /*
>> - * Copyright (C) 2024, Red Hat Inc.
>> + * Copyright (C) 2024-2025, Red Hat Inc.
>> *
>> * Color lookup tables construction
>> */
>> @@ -80,6 +80,11 @@ void Lut::updateGammaTable(IPAContext &context)
>> context.activeState.gamma.contrast = contrast;
>> }
>>
>> +int16_t Lut::ccmValue(unsigned int i, float ccm) const
>> +{
>> + return std::round(i * ccm);
>> +}
>> +
>> void Lut::prepare(IPAContext &context,
>> [[maybe_unused]] const uint32_t frame,
>> [[maybe_unused]] IPAFrameContext &frameContext,
>> @@ -91,22 +96,46 @@ void Lut::prepare(IPAContext &context,
>> * observed, it's not permanently prone to minor fluctuations or
>> * rounding errors.
>> */
>> - if (context.activeState.gamma.blackLevel != context.activeState.blc.level ||
>> - context.activeState.gamma.contrast != context.activeState.knobs.contrast)
>> + const bool gammaUpdateNeeded =
>> + context.activeState.gamma.blackLevel != context.activeState.blc.level ||
>> + context.activeState.gamma.contrast != context.activeState.knobs.contrast;
>> + if (gammaUpdateNeeded)
>> updateGammaTable(context);
>>
>> auto &gains = context.activeState.awb.gains;
>> auto &gammaTable = context.activeState.gamma.gammaTable;
>> const unsigned int gammaTableSize = gammaTable.size();
>> -
>> - for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {
>> - const double div = static_cast<double>(DebayerParams::kRGBLookupSize) /
>> - gammaTableSize;
>> - /* Apply gamma after gain! */
>> - const RGB<float> lutGains = (gains * i / div).min(gammaTableSize - 1);
>> - params->red[i] = gammaTable[static_cast<unsigned int>(lutGains.r())];
>> - params->green[i] = gammaTable[static_cast<unsigned int>(lutGains.g())];
>> - params->blue[i] = gammaTable[static_cast<unsigned int>(lutGains.b())];
>> + const double div = static_cast<double>(DebayerParams::kRGBLookupSize) /
>> + gammaTableSize;
>> +
>> + if (!context.ccmEnabled) {
>> + for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {
>> + /* Apply gamma after gain! */
>> + const RGB<float> lutGains = (gains * i / div).min(gammaTableSize - 1);
>> + params->red[i] = gammaTable[static_cast<unsigned int>(lutGains.r())];
>> + params->green[i] = gammaTable[static_cast<unsigned int>(lutGains.g())];
>> + params->blue[i] = gammaTable[static_cast<unsigned int>(lutGains.b())];
>> + }
>> + } else if (context.activeState.ccm.changed || gammaUpdateNeeded) {
>> + Matrix<float, 3, 3> gainCcm = { { gains.r(), 0, 0,
>> + 0, gains.g(), 0,
>> + 0, 0, gains.b() } };
>> + auto ccm = gainCcm * context.activeState.ccm.ccm;
>> + auto &red = params->redCcm;
>> + auto &green = params->greenCcm;
>> + auto &blue = params->blueCcm;
>> + for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {
>> + red[i].r = ccmValue(i, ccm[0][0]);
>> + red[i].g = ccmValue(i, ccm[1][0]);
>> + red[i].b = ccmValue(i, ccm[2][0]);
>> + green[i].r = ccmValue(i, ccm[0][1]);
>> + green[i].g = ccmValue(i, ccm[1][1]);
>> + green[i].b = ccmValue(i, ccm[2][1]);
>> + blue[i].r = ccmValue(i, ccm[0][2]);
>> + blue[i].g = ccmValue(i, ccm[1][2]);
>> + blue[i].b = ccmValue(i, ccm[2][2]);
>> + params->gammaLut[i] = gammaTable[i / div];
>> + }
>> }
>> }
>>
>> diff --git a/src/ipa/simple/algorithms/lut.h b/src/ipa/simple/algorithms/lut.h
>> index 889f864b..77324800 100644
>> --- a/src/ipa/simple/algorithms/lut.h
>> +++ b/src/ipa/simple/algorithms/lut.h
>> @@ -33,6 +33,7 @@ public:
>>
>> private:
>> void updateGammaTable(IPAContext &context);
>> + int16_t ccmValue(unsigned int i, float ccm) const;
>> };
>>
>> } /* namespace ipa::soft::algorithms */
>> diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp
>> index 34e42201..ca81d2e4 100644
>> --- a/src/libcamera/software_isp/debayer.cpp
>> +++ b/src/libcamera/software_isp/debayer.cpp
>> @@ -1,7 +1,7 @@
>> /* SPDX-License-Identifier: LGPL-2.1-or-later */
>> /*
>> * Copyright (C) 2023, Linaro Ltd
>> - * Copyright (C) 2023, 2024 Red Hat Inc.
>> + * Copyright (C) 2023-2025 Red Hat Inc.
>> *
>> * Authors:
>> * Hans de Goede <hdegoede at redhat.com>
>> @@ -24,8 +24,33 @@ namespace libcamera {
>> */
>>
>> /**
>> - * \typedef DebayerParams::ColorLookupTable
>> - * \brief Type of the lookup tables for red, green, blue values
>> + * \struct DebayerParams::CcmColumn
>> + * \brief Type of a single column of a color correction matrix (CCM)
>> + */
>> +
>> +/**
>> + * \var DebayerParams::CcmColumn::r
>> + * \brief Red (first) component of a CCM column
>> + */
>> +
>> +/**
>> + * \var DebayerParams::CcmColumn::g
>> + * \brief Green (second) component of a CCM column
>> + */
>> +
>> +/**
>> + * \var DebayerParams::CcmColumn::b
>> + * \brief Blue (third) component of a CCM column
>
> This is slightly confusing, given the structure of the CCM described in
> the commit message. I understand that r, g and b here refer to the rows
> in the CCM that produce the red, green and blue components of the output
> value, not the columns that are multiplied by the red, green and blue
> components of the input value. Maybe expanding the documentation of
> DebayerParams::CcmColumn would make it clearer:
>
> * \struct DebayerParams::CcmColumn
> * \brief Type of a single column of a color correction matrix (CCM)
> *
> * When multiplying an input pixel, columns in the CCM corresponds to the red,
> * green or blue component of input pixel values, while rows corresponds to the
> * red, green or blue components of the output pixel values. The members of the
> * CcmColumn structure are named after the colour components of the output pixel
> * values they correspond to.
>
>> + */
>> +
>> +/**
>> + * \typedef DebayerParams::LookupTable
>> + * \brief Type of the lookup tables for single lookup values
>> + */
>> +
>> +/**
>> + * \typedef DebayerParams::CcmLookupTable
>> + * \brief Type of the CCM lookup tables for red, green, blue values
>> */
>>
>> /**
>> @@ -43,6 +68,26 @@ namespace libcamera {
>> * \brief Lookup table for blue color, mapping input values to output values
>> */
>>
>> +/**
>> + * \var DebayerParams::redCcm
>> + * \brief CCM lookup table for red color, mapping input values to output values
>
> For the same reason, I would write here
>
> * \brief Lookup table for the CCM red column, mapping input values to output values
>
> Same below.
>
>> + */
>> +
>> +/**
>> + * \var DebayerParams::greenCcm
>> + * \brief CCM lookup table for green color, mapping input values to output values
>> + */
>> +
>> +/**
>> + * \var DebayerParams::blueCcm
>> + * \brief CCM lookup table for blue color, mapping input values to output values
>> + */
>> +
>> +/**
>> + * \var DebayerParams::gammaLut
>> + * \brief Gamma lookup table used with color correction matrix
>> + */
>> +
>> /**
>> * \class Debayer
>> * \brief Base debayering class
>> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
>> index 0cd03a8f..83a171d8 100644
>> --- a/src/libcamera/software_isp/debayer_cpu.cpp
>> +++ b/src/libcamera/software_isp/debayer_cpu.cpp
>> @@ -1,7 +1,7 @@
>> /* SPDX-License-Identifier: LGPL-2.1-or-later */
>> /*
>> * Copyright (C) 2023, Linaro Ltd
>> - * Copyright (C) 2023, Red Hat Inc.
>> + * Copyright (C) 2023-2025 Red Hat Inc.
>> *
>> * Authors:
>> * Hans de Goede <hdegoede at redhat.com>
>> @@ -11,9 +11,11 @@
>>
>> #include "debayer_cpu.h"
>>
>> +#include <algorithm>
>> #include <stdlib.h>
>> #include <sys/ioctl.h>
>> #include <time.h>
>> +#include <utility>
>>
>> #include <linux/dma-buf.h>
>>
>> @@ -51,8 +53,12 @@ DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats)
>> enableInputMemcpy_ = true;
>>
>> /* Initialize color lookup tables */
>> - for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++)
>> + for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {
>> red_[i] = green_[i] = blue_[i] = i;
>> + redCcm_[i] = { 1, 0, 0 };
>> + greenCcm_[i] = { 0, 1, 0 };
>> + blueCcm_[i] = { 0, 0, 1 };
>
> Shouldn't the diagonal elements be 'i', not '1' ?
Indeed, they should.
Posted v8 with all these changes.
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
>> + }
>> }
>>
>> DebayerCpu::~DebayerCpu() = default;
>> @@ -62,12 +68,24 @@ DebayerCpu::~DebayerCpu() = default;
>> const pixel_t *curr = (const pixel_t *)src[1] + xShift_; \
>> const pixel_t *next = (const pixel_t *)src[2] + xShift_;
>>
>> -#define STORE_PIXEL(b, g, r) \
>> - *dst++ = blue_[b]; \
>> - *dst++ = green_[g]; \
>> - *dst++ = red_[r]; \
>> - if constexpr (addAlphaByte) \
>> - *dst++ = 255; \
>> +#define GAMMA(value) \
>> + *dst++ = gammaLut_[std::clamp(value, 0, static_cast<int>(gammaLut_.size()) - 1)]
>> +
>> +#define STORE_PIXEL(b_, g_, r_) \
>> + if constexpr (ccmEnabled) { \
>> + const DebayerParams::CcmColumn &blue = blueCcm_[b_]; \
>> + const DebayerParams::CcmColumn &green = greenCcm_[g_]; \
>> + const DebayerParams::CcmColumn &red = redCcm_[r_]; \
>> + GAMMA(blue.b + green.b + red.b); \
>> + GAMMA(blue.g + green.g + red.g); \
>> + GAMMA(blue.r + green.r + red.r); \
>> + } else { \
>> + *dst++ = blue_[b_]; \
>> + *dst++ = green_[g_]; \
>> + *dst++ = red_[r_]; \
>> + } \
>> + if constexpr (addAlphaByte) \
>> + *dst++ = 255; \
>> x++;
>>
>> /*
>> @@ -755,8 +773,23 @@ void DebayerCpu::process(uint32_t frame, FrameBuffer *input, FrameBuffer *output
>> dmaSyncers.emplace_back(plane.fd, DmaSyncer::SyncType::Write);
>>
>> green_ = params.green;
>> - red_ = swapRedBlueGains_ ? params.blue : params.red;
>> - blue_ = swapRedBlueGains_ ? params.red : params.blue;
>> + greenCcm_ = params.greenCcm;
>> + if (swapRedBlueGains_) {
>> + red_ = params.blue;
>> + blue_ = params.red;
>> + redCcm_ = params.blueCcm;
>> + blueCcm_ = params.redCcm;
>> + for (unsigned int i = 0; i < 256; i++) {
>> + std::swap(redCcm_[i].r, redCcm_[i].b);
>> + std::swap(blueCcm_[i].r, blueCcm_[i].b);
>> + }
>> + } else {
>> + red_ = params.red;
>> + blue_ = params.blue;
>> + redCcm_ = params.redCcm;
>> + blueCcm_ = params.blueCcm;
>> + }
>> + gammaLut_ = params.gammaLut;
>>
>> /* Copy metadata from the input buffer */
>> FrameMetadata &metadata = output->_d()->metadata();
>> diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h
>> index 21c08a2d..926195e9 100644
>> --- a/src/libcamera/software_isp/debayer_cpu.h
>> +++ b/src/libcamera/software_isp/debayer_cpu.h
>> @@ -138,9 +138,13 @@ private:
>> /* Max. supported Bayer pattern height is 4, debayering this requires 5 lines */
>> static constexpr unsigned int kMaxLineBuffers = 5;
>>
>> - DebayerParams::ColorLookupTable red_;
>> - DebayerParams::ColorLookupTable green_;
>> - DebayerParams::ColorLookupTable blue_;
>> + DebayerParams::LookupTable red_;
>> + DebayerParams::LookupTable green_;
>> + DebayerParams::LookupTable blue_;
>> + DebayerParams::CcmLookupTable redCcm_;
>> + DebayerParams::CcmLookupTable greenCcm_;
>> + DebayerParams::CcmLookupTable blueCcm_;
>> + DebayerParams::LookupTable gammaLut_;
>> debayerFn debayer0_;
>> debayerFn debayer1_;
>> debayerFn debayer2_;
More information about the libcamera-devel
mailing list