| Message ID | 20240423182000.1527425-4-mzamazal@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Milan, Thank you for the patch. On Tue, Apr 23, 2024 at 08:19:58PM +0200, 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@redhat.com> > --- > .../internal/software_isp/debayer_params.h | 18 ++-- > src/ipa/simple/soft_simple.cpp | 84 ++++++++++++------- > 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, 87 insertions(+), 98 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@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 30956a19..c5085f71 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,40 +251,59 @@ 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. > * Clamp max gain at 4.0, this also avoids 0 division. > */ > - { > - const uint64_t nPixels = std::accumulate( > - histogram.begin(), histogram.end(), 0); > - auto subtractBlackLevel = [blackLevel, nPixels]( > - uint64_t sum, uint64_t div) > - -> uint64_t { > - uint64_t reducedSum = sum - blackLevel * nPixels / div; > - uint64_t spreadSum = reducedSum * 256 / (256 - blackLevel); > - return spreadSum; > - }; > - const uint64_t sumR = subtractBlackLevel(stats_->sumR_, 4); > - const uint64_t sumG = subtractBlackLevel(stats_->sumG_, 2); > - const uint64_t sumB = subtractBlackLevel(stats_->sumB_, 4); > - > - if (sumR <= sumG / 4) > - params_->gainR = 1024; > - else > - params_->gainR = 256 * sumG / sumR; > - > - if (sumB <= sumG / 4) > - params_->gainB = 1024; > - else > - params_->gainB = 256 * sumG / sumB; > + const uint64_t nPixels = std::accumulate( > + histogram.begin(), histogram.end(), 0); > + auto subtractBlackLevel = [blackLevel, nPixels]( > + uint64_t sum, uint64_t div) > + -> uint64_t { > + const uint64_t reducedSum = sum - blackLevel * nPixels / div; > + const uint64_t spreadSum = reducedSum * 256 / (256 - blackLevel); > + return spreadSum; > + }; > + const uint64_t sumR = subtractBlackLevel(stats_->sumR_, 4); > + const uint64_t sumG = subtractBlackLevel(stats_->sumG_, 2); > + const uint64_t sumB = subtractBlackLevel(stats_->sumB_, 4); The above belongs to the previous patch, including adding the const keyword to the local variables in subtractBlackLevel(). > + > + /* Gain: 128 = 0.5, 256 = 1.0, 512 = 2.0, etc. */ > + unsigned int gainR = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR; > + unsigned int gainB = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB; > + /* Green gain and gamma values are fixed */ > + 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; > } > > - /* Green gain and gamma values are fixed */ > - params_->gainG = 256; > - params_->gamma = 0.5; > + 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(); > > @@ -300,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; > @@ -348,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; > } > > 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@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";
Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: > Hi Milan, > > Thank you for the patch. Hi Laurent, thank you for the review. > On Tue, Apr 23, 2024 at 08:19:58PM +0200, 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@redhat.com> >> --- >> .../internal/software_isp/debayer_params.h | 18 ++-- >> src/ipa/simple/soft_simple.cpp | 84 ++++++++++++------- >> 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, 87 insertions(+), 98 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@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 30956a19..c5085f71 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,40 +251,59 @@ 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. >> * Clamp max gain at 4.0, this also avoids 0 division. >> */ >> - { >> - const uint64_t nPixels = std::accumulate( >> - histogram.begin(), histogram.end(), 0); >> - auto subtractBlackLevel = [blackLevel, nPixels]( >> - uint64_t sum, uint64_t div) >> - -> uint64_t { >> - uint64_t reducedSum = sum - blackLevel * nPixels / div; >> - uint64_t spreadSum = reducedSum * 256 / (256 - blackLevel); >> - return spreadSum; >> - }; >> - const uint64_t sumR = subtractBlackLevel(stats_->sumR_, 4); >> - const uint64_t sumG = subtractBlackLevel(stats_->sumG_, 2); >> - const uint64_t sumB = subtractBlackLevel(stats_->sumB_, 4); >> - >> - if (sumR <= sumG / 4) >> - params_->gainR = 1024; >> - else >> - params_->gainR = 256 * sumG / sumR; >> - >> - if (sumB <= sumG / 4) >> - params_->gainB = 1024; >> - else >> - params_->gainB = 256 * sumG / sumB; >> + const uint64_t nPixels = std::accumulate( >> + histogram.begin(), histogram.end(), 0); >> + auto subtractBlackLevel = [blackLevel, nPixels]( >> + uint64_t sum, uint64_t div) >> + -> uint64_t { >> + const uint64_t reducedSum = sum - blackLevel * nPixels / div; >> + const uint64_t spreadSum = reducedSum * 256 / (256 - blackLevel); >> + return spreadSum; >> + }; >> + const uint64_t sumR = subtractBlackLevel(stats_->sumR_, 4); >> + const uint64_t sumG = subtractBlackLevel(stats_->sumG_, 2); >> + const uint64_t sumB = subtractBlackLevel(stats_->sumB_, 4); > > The above belongs to the previous patch, Yes, I'll move it there. > including adding the const keyword to the local variables in > subtractBlackLevel(). Yes. Nevertheless, the variables will be dropped as we agreed on removing the superfluous multiplication. >> + >> + /* Gain: 128 = 0.5, 256 = 1.0, 512 = 2.0, etc. */ >> + unsigned int gainR = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR; >> + unsigned int gainB = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB; >> + /* Green gain and gamma values are fixed */ >> + 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; >> } >> >> - /* Green gain and gamma values are fixed */ >> - params_->gainG = 256; >> - params_->gamma = 0.5; >> + 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(); >> >> @@ -300,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; >> @@ -348,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; >> } >> >> 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@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";
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@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 30956a19..c5085f71 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,40 +251,59 @@ 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. * Clamp max gain at 4.0, this also avoids 0 division. */ - { - const uint64_t nPixels = std::accumulate( - histogram.begin(), histogram.end(), 0); - auto subtractBlackLevel = [blackLevel, nPixels]( - uint64_t sum, uint64_t div) - -> uint64_t { - uint64_t reducedSum = sum - blackLevel * nPixels / div; - uint64_t spreadSum = reducedSum * 256 / (256 - blackLevel); - return spreadSum; - }; - const uint64_t sumR = subtractBlackLevel(stats_->sumR_, 4); - const uint64_t sumG = subtractBlackLevel(stats_->sumG_, 2); - const uint64_t sumB = subtractBlackLevel(stats_->sumB_, 4); - - if (sumR <= sumG / 4) - params_->gainR = 1024; - else - params_->gainR = 256 * sumG / sumR; - - if (sumB <= sumG / 4) - params_->gainB = 1024; - else - params_->gainB = 256 * sumG / sumB; + const uint64_t nPixels = std::accumulate( + histogram.begin(), histogram.end(), 0); + auto subtractBlackLevel = [blackLevel, nPixels]( + uint64_t sum, uint64_t div) + -> uint64_t { + const uint64_t reducedSum = sum - blackLevel * nPixels / div; + const uint64_t spreadSum = reducedSum * 256 / (256 - blackLevel); + return spreadSum; + }; + const uint64_t sumR = subtractBlackLevel(stats_->sumR_, 4); + const uint64_t sumG = subtractBlackLevel(stats_->sumG_, 2); + const uint64_t sumB = subtractBlackLevel(stats_->sumB_, 4); + + /* Gain: 128 = 0.5, 256 = 1.0, 512 = 2.0, etc. */ + unsigned int gainR = sumR <= sumG / 4 ? 1024 : 256 * sumG / sumR; + unsigned int gainB = sumB <= sumG / 4 ? 1024 : 256 * sumG / sumB; + /* Green gain and gamma values are fixed */ + 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; } - /* Green gain and gamma values are fixed */ - params_->gainG = 256; - params_->gamma = 0.5; + 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(); @@ -300,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; @@ -348,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; } 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@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";
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@redhat.com> --- .../internal/software_isp/debayer_params.h | 18 ++-- src/ipa/simple/soft_simple.cpp | 84 ++++++++++++------- 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, 87 insertions(+), 98 deletions(-)