| Message ID | 20240430173430.200392-4-mzamazal@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Milan, Thank you for the patch! 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@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@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. With this minor issue fixed, Reviewed-by: Andrei Konovalov <andrey.konovalov.ynk@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@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";
Andrei Konovalov <andrey.konovalov.ynk@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@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@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@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@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 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; } 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 | 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(-)