[PATCH v2 3/5] libcamera: software_isp: Move color mappings out of debayering
Milan Zamazal
mzamazal at redhat.com
Thu May 23 13:25:07 CEST 2024
Andrei Konovalov <andrey.konovalov.ynk at gmail.com> writes:
> Hi Milan,
>
> Thank you for the patch!
Hi Andrei,
thank you for your reviews!
> On 30.04.2024 20:34, Milan Zamazal wrote:
>> Constructing the color mapping tables is related to stats rather than
>> debayering, where they are applied. Let's move the corresponding code
>> to stats processing.
>> This is a preliminary step towards building this functionality on top of
>> libipa/algorithm.h, which should follow.
>> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
>> ---
>> .../internal/software_isp/debayer_params.h | 18 +++----
>> src/ipa/simple/soft_simple.cpp | 51 +++++++++++++++----
>> src/libcamera/software_isp/debayer.cpp | 29 +++++------
>> src/libcamera/software_isp/debayer_cpu.cpp | 41 +++------------
>> src/libcamera/software_isp/debayer_cpu.h | 9 ++--
>> src/libcamera/software_isp/software_isp.cpp | 4 +-
>> 6 files changed, 75 insertions(+), 77 deletions(-)
>> diff --git a/include/libcamera/internal/software_isp/debayer_params.h
>> b/include/libcamera/internal/software_isp/debayer_params.h
>> index 32cd448a..09f4ff00 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, Red Hat Inc.
>> + * Copyright (C) 2023, 2024 Red Hat Inc.
>> *
>> * Authors:
>> * Hans de Goede <hdegoede at redhat.com>
>> @@ -10,20 +10,20 @@
>> #pragma once
>> +#include <array>
>> +#include <stdint.h>
>> +
>> namespace libcamera {
>> struct DebayerParams {
>> static constexpr unsigned int kGain10 = 256;
>> + static constexpr unsigned int kRGBLookupSize = 256;
>> - unsigned int gainR;
>> - unsigned int gainG;
>> - unsigned int gainB;
>> + using ColorLookupTable = std::array<uint8_t, kRGBLookupSize>;
>> - float gamma;
>> - /**
>> - * \brief Level of the black point, 0..255, 0 is no correction.
>> - */
>> - unsigned int blackLevel;
>> + ColorLookupTable red;
>> + ColorLookupTable green;
>> + ColorLookupTable blue;
>> };
>> } /* namespace libcamera */
>> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
>> index f595abc2..981e1ae0 100644
>> --- a/src/ipa/simple/soft_simple.cpp
>> +++ b/src/ipa/simple/soft_simple.cpp
>> @@ -5,6 +5,7 @@
>> * soft_simple.cpp - Simple Software Image Processing Algorithm module
>> */
>> +#include <math.h>
>> #include <numeric>
>> #include <stdint.h>
>> #include <sys/mman.h>
>> @@ -84,6 +85,10 @@ private:
>> ControlInfoMap sensorInfoMap_;
>> BlackLevel blackLevel_;
>> + static constexpr unsigned int kGammaLookupSize = 1024;
>> + std::array<uint8_t, kGammaLookupSize> gammaTable_;
>> + int lastBlackLevel_ = -1;
>> +
>> int32_t exposureMin_, exposureMax_;
>> int32_t exposure_;
>> double againMin_, againMax_, againMinStep_;
>> @@ -246,7 +251,6 @@ void IPASoftSimple::processStats(const ControlList &sensorControls)
>> if (ignoreUpdates_ > 0)
>> blackLevel_.update(histogram);
>> const uint8_t blackLevel = blackLevel_.get();
>> - params_->blackLevel = blackLevel;
>> /*
>> * Calculate red and blue gains for AWB.
>> @@ -265,12 +269,41 @@ void IPASoftSimple::processStats(const ControlList &sensorControls)
>> const uint64_t sumG = subtractBlackLevel(stats_->sumG_, 2);
>> const uint64_t sumB = subtractBlackLevel(stats_->sumB_, 4);
>> - params_->gainR = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR;
>> - params_->gainB = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB;
>> -
>> + /* Gain: 128 = 0.5, 256 = 1.0, 512 = 2.0, etc. */
>> + const unsigned int gainR = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR;
>> + const unsigned int gainB = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB;
>> /* Green gain and gamma values are fixed */
>> - params_->gainG = 256;
>> - params_->gamma = 0.5;
>> + constexpr unsigned int gainG = 256;
>> + /* gamma == 1.0 means no correction */
>> + constexpr float gamma = 0.5;
>> +
>> + /* Update the gamma table if needed */
>> + if (blackLevel != lastBlackLevel_) {
>> + const unsigned int blackIndex = blackLevel * kGammaLookupSize / 256;
>> + std::fill(gammaTable_.begin(), gammaTable_.begin() + blackIndex, 0);
>> + const float divisor = kGammaLookupSize - blackIndex - 1.0;
>> + for (unsigned int i = blackIndex; i < kGammaLookupSize; i++)
>> + gammaTable_[i] = UINT8_MAX * powf((i - blackIndex) / divisor, gamma);
>> +
>> + lastBlackLevel_ = blackLevel;
>> + }
>> +
>> + for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {
>> + constexpr unsigned int div =
>> + DebayerParams::kRGBLookupSize * DebayerParams::kGain10 /
>> + kGammaLookupSize;
>> + unsigned int idx;
>> +
>> + /* Apply gamma after gain! */
>> + idx = std::min({ i * gainR / div, (kGammaLookupSize - 1) });
>> + params_->red[i] = gammaTable_[idx];
>> +
>> + idx = std::min({ i * gainG / div, (kGammaLookupSize - 1) });
>> + params_->green[i] = gammaTable_[idx];
>> +
>> + idx = std::min({ i * gainB / div, (kGammaLookupSize - 1) });
>> + params_->blue[i] = gammaTable_[idx];
>> + }
>> setIspParams.emit();
>> @@ -291,7 +324,7 @@ void IPASoftSimple::processStats(const ControlList
>> &sensorControls)
>> * https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf
>> */
>> const unsigned int blackLevelHistIdx =
>> - params_->blackLevel / (256 / SwIspStats::kYHistogramSize);
>> + blackLevel / (256 / SwIspStats::kYHistogramSize);
>> const unsigned int histogramSize =
>> SwIspStats::kYHistogramSize - blackLevelHistIdx;
>> const unsigned int yHistValsPerBin = histogramSize / kExposureBinsCount;
>> @@ -339,8 +372,8 @@ void IPASoftSimple::processStats(const ControlList &sensorControls)
>> LOG(IPASoft, Debug) << "exposureMSV " << exposureMSV
>> << " exp " << exposure_ << " again " << again_
>> - << " gain R/B " << params_->gainR << "/" << params_->gainB
>> - << " black level " << params_->blackLevel;
>> + << " gain R/B " << gainR << "/" << gainB
>> + << " black level " << blackLevel;
>
> Here blackLevel needs a type cast as otherwise uint8_t is printed as a
> character, not a number.
Ah, I see, thank you, will fix it.
> With this minor issue fixed,
>
> Reviewed-by: Andrei Konovalov <andrey.konovalov.ynk at gmail.com>
>
>> }
>> void IPASoftSimple::updateExposure(double exposureMSV)
>> diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp
>> index 1c035e9b..ac438a33 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, Red Hat Inc.
>> + * Copyright (C) 2023, 2024 Red Hat Inc.
>> *
>> * Authors:
>> * Hans de Goede <hdegoede at redhat.com>
>> @@ -24,29 +24,28 @@ namespace libcamera {
>> */
>> /**
>> - * \var DebayerParams::gainR
>> - * \brief Red gain
>> - *
>> - * 128 = 0.5, 256 = 1.0, 512 = 2.0, etc.
>> + * \var DebayerParams::kRGBLookupSize
>> + * \brief Size of a color lookup table
>> */
>> /**
>> - * \var DebayerParams::gainG
>> - * \brief Green gain
>> - *
>> - * 128 = 0.5, 256 = 1.0, 512 = 2.0, etc.
>> + * \typedef DebayerParams::ColorLookupTable
>> + * \brief Type of the lookup tables for red, green, blue values
>> */
>> /**
>> - * \var DebayerParams::gainB
>> - * \brief Blue gain
>> - *
>> - * 128 = 0.5, 256 = 1.0, 512 = 2.0, etc.
>> + * \var DebayerParams::red
>> + * \brief Lookup table for red color, mapping input values to output values
>> + */
>> +
>> +/**
>> + * \var DebayerParams::green
>> + * \brief Lookup table for green color, mapping input values to output values
>> */
>> /**
>> - * \var DebayerParams::gamma
>> - * \brief Gamma correction, 1.0 is no correction
>> + * \var DebayerParams::blue
>> + * \brief Lookup table for blue color, mapping input values to output values
>> */
>> /**
>> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
>> index 88d6578b..8b2b2f40 100644
>> --- a/src/libcamera/software_isp/debayer_cpu.cpp
>> +++ b/src/libcamera/software_isp/debayer_cpu.cpp
>> @@ -11,7 +11,6 @@
>> #include "debayer_cpu.h"
>> -#include <math.h>
>> #include <stdlib.h>
>> #include <time.h>
>> @@ -47,9 +46,9 @@ DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats)
>> */
>> enableInputMemcpy_ = true;
>> - /* Initialize gamma to 1.0 curve */
>> - for (unsigned int i = 0; i < kGammaLookupSize; i++)
>> - gamma_[i] = i / (kGammaLookupSize / kRGBLookupSize);
>> + /* Initialize color lookup tables */
>> + for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++)
>> + red_[i] = green_[i] = blue_[i] = i;
>> for (unsigned int i = 0; i < kMaxLineBuffers; i++)
>> lineBuffers_[i] = nullptr;
>> @@ -698,37 +697,9 @@ void DebayerCpu::process(FrameBuffer *input, FrameBuffer *output, DebayerParams
>> clock_gettime(CLOCK_MONOTONIC_RAW, &frameStartTime);
>> }
>> - /* Apply DebayerParams */
>> - if (params.gamma != gammaCorrection_ || params.blackLevel != blackLevel_) {
>> - const unsigned int blackIndex =
>> - params.blackLevel * kGammaLookupSize / 256;
>> - std::fill(gamma_.begin(), gamma_.begin() + blackIndex, 0);
>> - const float divisor = kGammaLookupSize - blackIndex - 1.0;
>> - for (unsigned int i = blackIndex; i < kGammaLookupSize; i++)
>> - gamma_[i] = UINT8_MAX * powf((i - blackIndex) / divisor, params.gamma);
>> -
>> - gammaCorrection_ = params.gamma;
>> - blackLevel_ = params.blackLevel;
>> - }
>> -
>> - if (swapRedBlueGains_)
>> - std::swap(params.gainR, params.gainB);
>> -
>> - for (unsigned int i = 0; i < kRGBLookupSize; i++) {
>> - constexpr unsigned int div =
>> - kRGBLookupSize * DebayerParams::kGain10 / kGammaLookupSize;
>> - unsigned int idx;
>> -
>> - /* Apply gamma after gain! */
>> - idx = std::min({ i * params.gainR / div, (kGammaLookupSize - 1) });
>> - red_[i] = gamma_[idx];
>> -
>> - idx = std::min({ i * params.gainG / div, (kGammaLookupSize - 1) });
>> - green_[i] = gamma_[idx];
>> -
>> - idx = std::min({ i * params.gainB / div, (kGammaLookupSize - 1) });
>> - blue_[i] = gamma_[idx];
>> - }
>> + green_ = params.green;
>> + red_ = swapRedBlueGains_ ? params.blue : params.red;
>> + blue_ = swapRedBlueGains_ ? params.red : params.blue;
>> /* 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 689c1075..47373426 100644
>> --- a/src/libcamera/software_isp/debayer_cpu.h
>> +++ b/src/libcamera/software_isp/debayer_cpu.h
>> @@ -122,15 +122,12 @@ private:
>> void process2(const uint8_t *src, uint8_t *dst);
>> void process4(const uint8_t *src, uint8_t *dst);
>> - static constexpr unsigned int kGammaLookupSize = 1024;
>> - static constexpr unsigned int kRGBLookupSize = 256;
>> /* Max. supported Bayer pattern height is 4, debayering this requires 5 lines */
>> static constexpr unsigned int kMaxLineBuffers = 5;
>> - std::array<uint8_t, kGammaLookupSize> gamma_;
>> - std::array<uint8_t, kRGBLookupSize> red_;
>> - std::array<uint8_t, kRGBLookupSize> green_;
>> - std::array<uint8_t, kRGBLookupSize> blue_;
>> + DebayerParams::ColorLookupTable red_;
>> + DebayerParams::ColorLookupTable green_;
>> + DebayerParams::ColorLookupTable blue_;
>> debayerFn debayer0_;
>> debayerFn debayer1_;
>> debayerFn debayer2_;
>> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp
>> index e4e56086..3e07453d 100644
>> --- a/src/libcamera/software_isp/software_isp.cpp
>> +++ b/src/libcamera/software_isp/software_isp.cpp
>> @@ -63,9 +63,7 @@ LOG_DEFINE_CATEGORY(SoftwareIsp)
>> * handler
>> */
>> SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor)
>> - : debayerParams_{ DebayerParams::kGain10, DebayerParams::kGain10,
>> - DebayerParams::kGain10, 0.5f, 0 },
>> - dmaHeap_(DmaHeap::DmaHeapFlag::Cma | DmaHeap::DmaHeapFlag::System)
>> + : dmaHeap_(DmaHeap::DmaHeapFlag::Cma | DmaHeap::DmaHeapFlag::System)
>> {
>> if (!dmaHeap_.isValid()) {
>> LOG(SoftwareIsp, Error) << "Failed to create DmaHeap object";
More information about the libcamera-devel
mailing list