Message ID | 20240717085444.289997-19-mzamazal@redhat.com |
---|---|
State | Superseded |
Headers | show |
Series |
|
Related | show |
Hi Milan On 17/07/2024 09:54, Milan Zamazal wrote: > The black level determination, already present as a separate class, can > be moved to the prepared Algorithm processing structure. It is the > first of the current software ISP algorithms that is called in the stats > processing sequence, which means it is also the first one that should be > moved to the new structure. Stats processing starts with calling > Algorithm-based processing so the call order of the algorithms is > retained. > > Movement of this algorithm is relatively straightforward because all it > does is processing stats. > > Black level is now recomputed on each stats processing. In a future > patch, after DelayedControls are used, this will be changed to recompute > the black level only after exposure/gain changes. > > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > --- > src/ipa/simple/algorithms/blc.cpp | 71 ++++++++++++++++++++ > src/ipa/simple/algorithms/blc.h | 37 +++++++++++ > src/ipa/simple/algorithms/meson.build | 1 + > src/ipa/simple/black_level.cpp | 93 --------------------------- > src/ipa/simple/black_level.h | 33 ---------- > src/ipa/simple/data/uncalibrated.yaml | 1 + > src/ipa/simple/ipa_context.cpp | 8 +++ > src/ipa/simple/ipa_context.h | 5 ++ > src/ipa/simple/meson.build | 1 - > src/ipa/simple/soft_simple.cpp | 8 +-- > 10 files changed, 125 insertions(+), 133 deletions(-) > create mode 100644 src/ipa/simple/algorithms/blc.cpp > create mode 100644 src/ipa/simple/algorithms/blc.h > delete mode 100644 src/ipa/simple/black_level.cpp > delete mode 100644 src/ipa/simple/black_level.h > > diff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp > new file mode 100644 > index 00000000..3b73d830 > --- /dev/null > +++ b/src/ipa/simple/algorithms/blc.cpp > @@ -0,0 +1,71 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2024, Red Hat Inc. > + * > + * Black level handling > + */ > + > +#include "blc.h" > + > +#include <numeric> > + > +#include <libcamera/base/log.h> > + > +namespace libcamera { > + > +namespace ipa::soft::algorithms { > + > +LOG_DEFINE_CATEGORY(IPASoftBL) > + > +BlackLevel::BlackLevel() > +{ > +} > + > +int BlackLevel::init(IPAContext &context, > + [[maybe_unused]] const YamlObject &tuningData) > +{ > + context.activeState.black.level = 255; > + return 0; > +} The removed comment from the defunct BlackLevel class says: - * Black level can be provided in hardware tuning files or, if no tuning file is - * available for the given hardware, guessed automatically, with less accuracy. - * As tuning files are not yet implemented for software ISP, BlackLevel - * currently provides only guessed black levels. Since moving to the Algorithm based code includes tuning file support, can we take the opportunity to fix that? > + > +void BlackLevel::process(IPAContext &context, > + [[maybe_unused]] const uint32_t frame, > + [[maybe_unused]] IPAFrameContext &frameContext, > + const SwIspStats *stats, > + [[maybe_unused]] ControlList &metadata) > +{ > + const SwIspStats::Histogram &histogram = stats->yHistogram; > + > + /* > + * The constant is selected to be "good enough", not overly conservative or > + * aggressive. There is no magic about the given value. > + */ > + constexpr float ignoredPercentage_ = 0.02; > + const unsigned int total = > + std::accumulate(begin(histogram), end(histogram), 0); > + const unsigned int pixelThreshold = ignoredPercentage_ * total; > + const unsigned int histogramRatio = 256 / SwIspStats::kYHistogramSize; > + const unsigned int currentBlackIdx = > + context.activeState.black.level / histogramRatio; > + > + for (unsigned int i = 0, seen = 0; > + i < currentBlackIdx && i < SwIspStats::kYHistogramSize; > + i++) { > + seen += histogram[i]; > + if (seen >= pixelThreshold) { > + context.activeState.black.level = i * histogramRatio; > + LOG(IPASoftBL, Debug) > + << "Auto-set black level: " > + << i << "/" << SwIspStats::kYHistogramSize > + << " (" << 100 * (seen - histogram[i]) / total << "% below, " > + << 100 * seen / total << "% at or below)"; > + break; > + } > + }; > +} > + > +REGISTER_IPA_ALGORITHM(BlackLevel, "BlackLevel") > + > +} /* namespace ipa::soft::algorithms */ > + > +} /* namespace libcamera */ > diff --git a/src/ipa/simple/algorithms/blc.h b/src/ipa/simple/algorithms/blc.h > new file mode 100644 > index 00000000..2f73db52 > --- /dev/null > +++ b/src/ipa/simple/algorithms/blc.h > @@ -0,0 +1,37 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2024, Red Hat Inc. > + * > + * Black level handling > + */ > + > +#pragma once > + > +#include <libcamera/controls.h> > + > +#include "libcamera/internal/software_isp/swisp_stats.h" > + > +#include "algorithm.h" > +#include "ipa_context.h" > + > +namespace libcamera { > + > +namespace ipa::soft::algorithms { > + > +class BlackLevel : public Algorithm > +{ > +public: > + BlackLevel(); > + ~BlackLevel() = default; > + > + int init(IPAContext &context, const YamlObject &tuningData) > + override; > + void process(IPAContext &context, const uint32_t frame, > + IPAFrameContext &frameContext, > + const SwIspStats *stats, > + ControlList &metadata) override; > +}; > + > +} /* namespace ipa::soft::algorithms */ > + > +} /* namespace libcamera */ > diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build > index 31d26e43..1f63c220 100644 > --- a/src/ipa/simple/algorithms/meson.build > +++ b/src/ipa/simple/algorithms/meson.build > @@ -1,4 +1,5 @@ > # SPDX-License-Identifier: CC0-1.0 > > soft_simple_ipa_algorithms = files([ > + 'blc.cpp', > ]) > diff --git a/src/ipa/simple/black_level.cpp b/src/ipa/simple/black_level.cpp > deleted file mode 100644 > index 37e0109c..00000000 > --- a/src/ipa/simple/black_level.cpp > +++ /dev/null > @@ -1,93 +0,0 @@ > -/* SPDX-License-Identifier: LGPL-2.1-or-later */ > -/* > - * Copyright (C) 2024, Red Hat Inc. > - * > - * black level handling > - */ > - > -#include "black_level.h" > - > -#include <numeric> > - > -#include <libcamera/base/log.h> > - > -namespace libcamera { > - > -LOG_DEFINE_CATEGORY(IPASoftBL) > - > -namespace ipa::soft { > - > -/** > - * \class BlackLevel > - * \brief Object providing black point level for software ISP > - * > - * Black level can be provided in hardware tuning files or, if no tuning file is > - * available for the given hardware, guessed automatically, with less accuracy. > - * As tuning files are not yet implemented for software ISP, BlackLevel > - * currently provides only guessed black levels. > - * > - * This class serves for tracking black level as a property of the underlying > - * hardware, not as means of enhancing a particular scene or image. > - * > - * The class is supposed to be instantiated for the given camera stream. > - * The black level can be retrieved using BlackLevel::get() method. It is > - * initially 0 and may change when updated using BlackLevel::update() method. > - */ > - > -BlackLevel::BlackLevel() > - : blackLevel_(255), blackLevelSet_(false) > -{ > -} > - > -/** > - * \brief Return the current black level > - * > - * \return The black level, in the range from 0 (minimum) to 255 (maximum). > - * If the black level couldn't be determined yet, return 0. > - */ > -uint8_t BlackLevel::get() const > -{ > - return blackLevelSet_ ? blackLevel_ : 0; > -} > - > -/** > - * \brief Update black level from the provided histogram > - * \param[in] yHistogram The histogram to be used for updating black level > - * > - * The black level is property of the given hardware, not image. It is updated > - * only if it has not been yet set or if it is lower than the lowest value seen > - * so far. > - */ > -void BlackLevel::update(SwIspStats::Histogram &yHistogram) > -{ > - /* > - * The constant is selected to be "good enough", not overly conservative or > - * aggressive. There is no magic about the given value. > - */ > - constexpr float ignoredPercentage_ = 0.02; > - const unsigned int total = > - std::accumulate(begin(yHistogram), end(yHistogram), 0); > - const unsigned int pixelThreshold = ignoredPercentage_ * total; > - const unsigned int histogramRatio = 256 / SwIspStats::kYHistogramSize; > - const unsigned int currentBlackIdx = blackLevel_ / histogramRatio; > - > - for (unsigned int i = 0, seen = 0; > - i < currentBlackIdx && i < SwIspStats::kYHistogramSize; > - i++) { > - seen += yHistogram[i]; > - if (seen >= pixelThreshold) { > - blackLevel_ = i * histogramRatio; > - blackLevelSet_ = true; > - LOG(IPASoftBL, Debug) > - << "Auto-set black level: " > - << i << "/" << SwIspStats::kYHistogramSize > - << " (" << 100 * (seen - yHistogram[i]) / total << "% below, " > - << 100 * seen / total << "% at or below)"; > - break; > - } > - }; > -} > - > -} /* namespace ipa::soft */ > - > -} /* namespace libcamera */ > diff --git a/src/ipa/simple/black_level.h b/src/ipa/simple/black_level.h > deleted file mode 100644 > index a04230c9..00000000 > --- a/src/ipa/simple/black_level.h > +++ /dev/null > @@ -1,33 +0,0 @@ > -/* SPDX-License-Identifier: LGPL-2.1-or-later */ > -/* > - * Copyright (C) 2024, Red Hat Inc. > - * > - * black level handling > - */ > - > -#pragma once > - > -#include <array> > -#include <stdint.h> > - > -#include "libcamera/internal/software_isp/swisp_stats.h" > - > -namespace libcamera { > - > -namespace ipa::soft { > - > -class BlackLevel > -{ > -public: > - BlackLevel(); > - uint8_t get() const; > - void update(SwIspStats::Histogram &yHistogram); > - > -private: > - uint8_t blackLevel_; > - bool blackLevelSet_; > -}; > - > -} /* namespace ipa::soft */ > - > -} /* namespace libcamera */ > diff --git a/src/ipa/simple/data/uncalibrated.yaml b/src/ipa/simple/data/uncalibrated.yaml > index 2cdc39a8..e0d03d96 100644 > --- a/src/ipa/simple/data/uncalibrated.yaml > +++ b/src/ipa/simple/data/uncalibrated.yaml > @@ -3,4 +3,5 @@ > --- > version: 1 > algorithms: > + - BlackLevel: > ... > diff --git a/src/ipa/simple/ipa_context.cpp b/src/ipa/simple/ipa_context.cpp > index 3c1c7262..268cff32 100644 > --- a/src/ipa/simple/ipa_context.cpp > +++ b/src/ipa/simple/ipa_context.cpp > @@ -50,4 +50,12 @@ namespace libcamera::ipa::soft { > * \brief The current state of IPA algorithms > */ > > +/** > + * \var IPAActiveState::black > + * \brief Context for the Black Level algorithm > + * > + * \var IPAActiveState::black.level > + * \brief Current determined black level > + */ > + > } /* namespace libcamera::ipa::soft */ > diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h > index bc1235b6..ed07dbb8 100644 > --- a/src/ipa/simple/ipa_context.h > +++ b/src/ipa/simple/ipa_context.h > @@ -8,6 +8,8 @@ > > #pragma once > > +#include <stdint.h> > + > #include <libipa/fc_queue.h> > > namespace libcamera { > @@ -18,6 +20,9 @@ struct IPASessionConfiguration { > }; > > struct IPAActiveState { > + struct { > + uint8_t level; > + } black; > }; > > struct IPAFrameContext : public FrameContext { > diff --git a/src/ipa/simple/meson.build b/src/ipa/simple/meson.build > index 7515a8d8..92aa5744 100644 > --- a/src/ipa/simple/meson.build > +++ b/src/ipa/simple/meson.build > @@ -8,7 +8,6 @@ ipa_name = 'ipa_soft_simple' > soft_simple_sources = files([ > 'ipa_context.cpp', > 'soft_simple.cpp', > - 'black_level.cpp', > ]) > > soft_simple_sources += soft_simple_ipa_algorithms > diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp > index a8499f29..2fb3a473 100644 > --- a/src/ipa/simple/soft_simple.cpp > +++ b/src/ipa/simple/soft_simple.cpp > @@ -28,7 +28,6 @@ > > #include "libipa/camera_sensor_helper.h" > > -#include "black_level.h" > #include "module.h" > > namespace libcamera { > @@ -60,7 +59,7 @@ class IPASoftSimple : public ipa::soft::IPASoftInterface, public Module > { > public: > IPASoftSimple() > - : params_(nullptr), stats_(nullptr), blackLevel_(BlackLevel()), > + : params_(nullptr), stats_(nullptr), > context_({ {}, {}, { kMaxFrameContexts } }), > ignoreUpdates_(0) > { > @@ -92,7 +91,6 @@ private: > SwIspStats *stats_; > std::unique_ptr<CameraSensorHelper> camHelper_; > ControlInfoMap sensorInfoMap_; > - BlackLevel blackLevel_; > > static constexpr unsigned int kGammaLookupSize = 1024; > std::array<uint8_t, kGammaLookupSize> gammaTable_; > @@ -303,9 +301,7 @@ void IPASoftSimple::processStats( > algo->process(context_, frame, frameContext, stats_, metadata); > > SwIspStats::Histogram histogram = stats_->yHistogram; > - if (ignoreUpdates_ > 0) > - blackLevel_.update(histogram); > - const uint8_t blackLevel = blackLevel_.get(); > + const uint8_t blackLevel = context_.activeState.black.level; > > /* > * Black level must be subtracted to get the correct AWB ratios, they
Hi Dan, thank you for review. Dan Scally <dan.scally@ideasonboard.com> writes: > Hi Milan > > On 17/07/2024 09:54, Milan Zamazal wrote: >> The black level determination, already present as a separate class, can >> be moved to the prepared Algorithm processing structure. It is the >> first of the current software ISP algorithms that is called in the stats >> processing sequence, which means it is also the first one that should be >> moved to the new structure. Stats processing starts with calling >> Algorithm-based processing so the call order of the algorithms is >> retained. >> >> Movement of this algorithm is relatively straightforward because all it >> does is processing stats. >> >> Black level is now recomputed on each stats processing. In a future >> patch, after DelayedControls are used, this will be changed to recompute >> the black level only after exposure/gain changes. >> >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com> >> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> >> --- >> src/ipa/simple/algorithms/blc.cpp | 71 ++++++++++++++++++++ >> src/ipa/simple/algorithms/blc.h | 37 +++++++++++ >> src/ipa/simple/algorithms/meson.build | 1 + >> src/ipa/simple/black_level.cpp | 93 --------------------------- >> src/ipa/simple/black_level.h | 33 ---------- >> src/ipa/simple/data/uncalibrated.yaml | 1 + >> src/ipa/simple/ipa_context.cpp | 8 +++ >> src/ipa/simple/ipa_context.h | 5 ++ >> src/ipa/simple/meson.build | 1 - >> src/ipa/simple/soft_simple.cpp | 8 +-- >> 10 files changed, 125 insertions(+), 133 deletions(-) >> create mode 100644 src/ipa/simple/algorithms/blc.cpp >> create mode 100644 src/ipa/simple/algorithms/blc.h >> delete mode 100644 src/ipa/simple/black_level.cpp >> delete mode 100644 src/ipa/simple/black_level.h >> >> diff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp >> new file mode 100644 >> index 00000000..3b73d830 >> --- /dev/null >> +++ b/src/ipa/simple/algorithms/blc.cpp >> @@ -0,0 +1,71 @@ >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */ >> +/* >> + * Copyright (C) 2024, Red Hat Inc. >> + * >> + * Black level handling >> + */ >> + >> +#include "blc.h" >> + >> +#include <numeric> >> + >> +#include <libcamera/base/log.h> >> + >> +namespace libcamera { >> + >> +namespace ipa::soft::algorithms { >> + >> +LOG_DEFINE_CATEGORY(IPASoftBL) >> + >> +BlackLevel::BlackLevel() >> +{ >> +} >> + >> +int BlackLevel::init(IPAContext &context, >> + [[maybe_unused]] const YamlObject &tuningData) >> +{ >> + context.activeState.black.level = 255; >> + return 0; >> +} > > The removed comment from the defunct BlackLevel class says: > > - * Black level can be provided in hardware tuning files or, if no tuning file is - * available for the given hardware, guessed automatically, > with less accuracy. - * As tuning files are not yet implemented for software ISP, BlackLevel - * currently provides only guessed black > levels. Since moving to the Algorithm based code includes tuning > file support, can we take the opportunity to fix that? CameraSensorHelper has been enhanced with blackLevel() recently, which as I understand it is preferred to reading this information from tuning files. I have a patch that uses CameraSensorHelper in the sense described above and I can post it on top of v4 or as a part of it. I think the former is preferable as this series is already long enough. I'll mention it in the updated commit message. >> + >> +void BlackLevel::process(IPAContext &context, >> + [[maybe_unused]] const uint32_t frame, >> + [[maybe_unused]] IPAFrameContext &frameContext, >> + const SwIspStats *stats, >> + [[maybe_unused]] ControlList &metadata) >> +{ >> + const SwIspStats::Histogram &histogram = stats->yHistogram; >> + >> + /* >> + * The constant is selected to be "good enough", not overly conservative or >> + * aggressive. There is no magic about the given value. >> + */ >> + constexpr float ignoredPercentage_ = 0.02; >> + const unsigned int total = >> + std::accumulate(begin(histogram), end(histogram), 0); >> + const unsigned int pixelThreshold = ignoredPercentage_ * total; >> + const unsigned int histogramRatio = 256 / SwIspStats::kYHistogramSize; >> + const unsigned int currentBlackIdx = >> + context.activeState.black.level / histogramRatio; >> + >> + for (unsigned int i = 0, seen = 0; >> + i < currentBlackIdx && i < SwIspStats::kYHistogramSize; >> + i++) { >> + seen += histogram[i]; >> + if (seen >= pixelThreshold) { >> + context.activeState.black.level = i * histogramRatio; >> + LOG(IPASoftBL, Debug) >> + << "Auto-set black level: " >> + << i << "/" << SwIspStats::kYHistogramSize >> + << " (" << 100 * (seen - histogram[i]) / total << "% below, " >> + << 100 * seen / total << "% at or below)"; >> + break; >> + } >> + }; >> +} >> + >> +REGISTER_IPA_ALGORITHM(BlackLevel, "BlackLevel") >> + >> +} /* namespace ipa::soft::algorithms */ >> + >> +} /* namespace libcamera */ >> diff --git a/src/ipa/simple/algorithms/blc.h b/src/ipa/simple/algorithms/blc.h >> new file mode 100644 >> index 00000000..2f73db52 >> --- /dev/null >> +++ b/src/ipa/simple/algorithms/blc.h >> @@ -0,0 +1,37 @@ >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */ >> +/* >> + * Copyright (C) 2024, Red Hat Inc. >> + * >> + * Black level handling >> + */ >> + >> +#pragma once >> + >> +#include <libcamera/controls.h> >> + >> +#include "libcamera/internal/software_isp/swisp_stats.h" >> + >> +#include "algorithm.h" >> +#include "ipa_context.h" >> + >> +namespace libcamera { >> + >> +namespace ipa::soft::algorithms { >> + >> +class BlackLevel : public Algorithm >> +{ >> +public: >> + BlackLevel(); >> + ~BlackLevel() = default; >> + >> + int init(IPAContext &context, const YamlObject &tuningData) >> + override; >> + void process(IPAContext &context, const uint32_t frame, >> + IPAFrameContext &frameContext, >> + const SwIspStats *stats, >> + ControlList &metadata) override; >> +}; >> + >> +} /* namespace ipa::soft::algorithms */ >> + >> +} /* namespace libcamera */ >> diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build >> index 31d26e43..1f63c220 100644 >> --- a/src/ipa/simple/algorithms/meson.build >> +++ b/src/ipa/simple/algorithms/meson.build >> @@ -1,4 +1,5 @@ >> # SPDX-License-Identifier: CC0-1.0 >> soft_simple_ipa_algorithms = files([ >> + 'blc.cpp', >> ]) >> diff --git a/src/ipa/simple/black_level.cpp b/src/ipa/simple/black_level.cpp >> deleted file mode 100644 >> index 37e0109c..00000000 >> --- a/src/ipa/simple/black_level.cpp >> +++ /dev/null >> @@ -1,93 +0,0 @@ >> -/* SPDX-License-Identifier: LGPL-2.1-or-later */ >> -/* >> - * Copyright (C) 2024, Red Hat Inc. >> - * >> - * black level handling >> - */ >> - >> -#include "black_level.h" >> - >> -#include <numeric> >> - >> -#include <libcamera/base/log.h> >> - >> -namespace libcamera { >> - >> -LOG_DEFINE_CATEGORY(IPASoftBL) >> - >> -namespace ipa::soft { >> - >> -/** >> - * \class BlackLevel >> - * \brief Object providing black point level for software ISP >> - * >> - * Black level can be provided in hardware tuning files or, if no tuning file is >> - * available for the given hardware, guessed automatically, with less accuracy. >> - * As tuning files are not yet implemented for software ISP, BlackLevel >> - * currently provides only guessed black levels. >> - * >> - * This class serves for tracking black level as a property of the underlying >> - * hardware, not as means of enhancing a particular scene or image. >> - * >> - * The class is supposed to be instantiated for the given camera stream. >> - * The black level can be retrieved using BlackLevel::get() method. It is >> - * initially 0 and may change when updated using BlackLevel::update() method. >> - */ >> - >> -BlackLevel::BlackLevel() >> - : blackLevel_(255), blackLevelSet_(false) >> -{ >> -} >> - >> -/** >> - * \brief Return the current black level >> - * >> - * \return The black level, in the range from 0 (minimum) to 255 (maximum). >> - * If the black level couldn't be determined yet, return 0. >> - */ >> -uint8_t BlackLevel::get() const >> -{ >> - return blackLevelSet_ ? blackLevel_ : 0; >> -} >> - >> -/** >> - * \brief Update black level from the provided histogram >> - * \param[in] yHistogram The histogram to be used for updating black level >> - * >> - * The black level is property of the given hardware, not image. It is updated >> - * only if it has not been yet set or if it is lower than the lowest value seen >> - * so far. >> - */ >> -void BlackLevel::update(SwIspStats::Histogram &yHistogram) >> -{ >> - /* >> - * The constant is selected to be "good enough", not overly conservative or >> - * aggressive. There is no magic about the given value. >> - */ >> - constexpr float ignoredPercentage_ = 0.02; >> - const unsigned int total = >> - std::accumulate(begin(yHistogram), end(yHistogram), 0); >> - const unsigned int pixelThreshold = ignoredPercentage_ * total; >> - const unsigned int histogramRatio = 256 / SwIspStats::kYHistogramSize; >> - const unsigned int currentBlackIdx = blackLevel_ / histogramRatio; >> - >> - for (unsigned int i = 0, seen = 0; >> - i < currentBlackIdx && i < SwIspStats::kYHistogramSize; >> - i++) { >> - seen += yHistogram[i]; >> - if (seen >= pixelThreshold) { >> - blackLevel_ = i * histogramRatio; >> - blackLevelSet_ = true; >> - LOG(IPASoftBL, Debug) >> - << "Auto-set black level: " >> - << i << "/" << SwIspStats::kYHistogramSize >> - << " (" << 100 * (seen - yHistogram[i]) / total << "% below, " >> - << 100 * seen / total << "% at or below)"; >> - break; >> - } >> - }; >> -} >> - >> -} /* namespace ipa::soft */ >> - >> -} /* namespace libcamera */ >> diff --git a/src/ipa/simple/black_level.h b/src/ipa/simple/black_level.h >> deleted file mode 100644 >> index a04230c9..00000000 >> --- a/src/ipa/simple/black_level.h >> +++ /dev/null >> @@ -1,33 +0,0 @@ >> -/* SPDX-License-Identifier: LGPL-2.1-or-later */ >> -/* >> - * Copyright (C) 2024, Red Hat Inc. >> - * >> - * black level handling >> - */ >> - >> -#pragma once >> - >> -#include <array> >> -#include <stdint.h> >> - >> -#include "libcamera/internal/software_isp/swisp_stats.h" >> - >> -namespace libcamera { >> - >> -namespace ipa::soft { >> - >> -class BlackLevel >> -{ >> -public: >> - BlackLevel(); >> - uint8_t get() const; >> - void update(SwIspStats::Histogram &yHistogram); >> - >> -private: >> - uint8_t blackLevel_; >> - bool blackLevelSet_; >> -}; >> - >> -} /* namespace ipa::soft */ >> - >> -} /* namespace libcamera */ >> diff --git a/src/ipa/simple/data/uncalibrated.yaml b/src/ipa/simple/data/uncalibrated.yaml >> index 2cdc39a8..e0d03d96 100644 >> --- a/src/ipa/simple/data/uncalibrated.yaml >> +++ b/src/ipa/simple/data/uncalibrated.yaml >> @@ -3,4 +3,5 @@ >> --- >> version: 1 >> algorithms: >> + - BlackLevel: >> ... >> diff --git a/src/ipa/simple/ipa_context.cpp b/src/ipa/simple/ipa_context.cpp >> index 3c1c7262..268cff32 100644 >> --- a/src/ipa/simple/ipa_context.cpp >> +++ b/src/ipa/simple/ipa_context.cpp >> @@ -50,4 +50,12 @@ namespace libcamera::ipa::soft { >> * \brief The current state of IPA algorithms >> */ >> +/** >> + * \var IPAActiveState::black >> + * \brief Context for the Black Level algorithm >> + * >> + * \var IPAActiveState::black.level >> + * \brief Current determined black level >> + */ >> + >> } /* namespace libcamera::ipa::soft */ >> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h >> index bc1235b6..ed07dbb8 100644 >> --- a/src/ipa/simple/ipa_context.h >> +++ b/src/ipa/simple/ipa_context.h >> @@ -8,6 +8,8 @@ >> #pragma once >> +#include <stdint.h> >> + >> #include <libipa/fc_queue.h> >> namespace libcamera { >> @@ -18,6 +20,9 @@ struct IPASessionConfiguration { >> }; >> struct IPAActiveState { >> + struct { >> + uint8_t level; >> + } black; >> }; >> struct IPAFrameContext : public FrameContext { >> diff --git a/src/ipa/simple/meson.build b/src/ipa/simple/meson.build >> index 7515a8d8..92aa5744 100644 >> --- a/src/ipa/simple/meson.build >> +++ b/src/ipa/simple/meson.build >> @@ -8,7 +8,6 @@ ipa_name = 'ipa_soft_simple' >> soft_simple_sources = files([ >> 'ipa_context.cpp', >> 'soft_simple.cpp', >> - 'black_level.cpp', >> ]) >> soft_simple_sources += soft_simple_ipa_algorithms >> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp >> index a8499f29..2fb3a473 100644 >> --- a/src/ipa/simple/soft_simple.cpp >> +++ b/src/ipa/simple/soft_simple.cpp >> @@ -28,7 +28,6 @@ >> #include "libipa/camera_sensor_helper.h" >> -#include "black_level.h" >> #include "module.h" >> namespace libcamera { >> @@ -60,7 +59,7 @@ class IPASoftSimple : public ipa::soft::IPASoftInterface, public Module >> { >> public: >> IPASoftSimple() >> - : params_(nullptr), stats_(nullptr), blackLevel_(BlackLevel()), >> + : params_(nullptr), stats_(nullptr), >> context_({ {}, {}, { kMaxFrameContexts } }), >> ignoreUpdates_(0) >> { >> @@ -92,7 +91,6 @@ private: >> SwIspStats *stats_; >> std::unique_ptr<CameraSensorHelper> camHelper_; >> ControlInfoMap sensorInfoMap_; >> - BlackLevel blackLevel_; >> static constexpr unsigned int kGammaLookupSize = 1024; >> std::array<uint8_t, kGammaLookupSize> gammaTable_; >> @@ -303,9 +301,7 @@ void IPASoftSimple::processStats( >> algo->process(context_, frame, frameContext, stats_, metadata); >> SwIspStats::Histogram histogram = stats_->yHistogram; >> - if (ignoreUpdates_ > 0) >> - blackLevel_.update(histogram); >> - const uint8_t blackLevel = blackLevel_.get(); >> + const uint8_t blackLevel = context_.activeState.black.level; >> /* >> * Black level must be subtracted to get the correct AWB ratios, they
diff --git a/src/ipa/simple/algorithms/blc.cpp b/src/ipa/simple/algorithms/blc.cpp new file mode 100644 index 00000000..3b73d830 --- /dev/null +++ b/src/ipa/simple/algorithms/blc.cpp @@ -0,0 +1,71 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2024, Red Hat Inc. + * + * Black level handling + */ + +#include "blc.h" + +#include <numeric> + +#include <libcamera/base/log.h> + +namespace libcamera { + +namespace ipa::soft::algorithms { + +LOG_DEFINE_CATEGORY(IPASoftBL) + +BlackLevel::BlackLevel() +{ +} + +int BlackLevel::init(IPAContext &context, + [[maybe_unused]] const YamlObject &tuningData) +{ + context.activeState.black.level = 255; + return 0; +} + +void BlackLevel::process(IPAContext &context, + [[maybe_unused]] const uint32_t frame, + [[maybe_unused]] IPAFrameContext &frameContext, + const SwIspStats *stats, + [[maybe_unused]] ControlList &metadata) +{ + const SwIspStats::Histogram &histogram = stats->yHistogram; + + /* + * The constant is selected to be "good enough", not overly conservative or + * aggressive. There is no magic about the given value. + */ + constexpr float ignoredPercentage_ = 0.02; + const unsigned int total = + std::accumulate(begin(histogram), end(histogram), 0); + const unsigned int pixelThreshold = ignoredPercentage_ * total; + const unsigned int histogramRatio = 256 / SwIspStats::kYHistogramSize; + const unsigned int currentBlackIdx = + context.activeState.black.level / histogramRatio; + + for (unsigned int i = 0, seen = 0; + i < currentBlackIdx && i < SwIspStats::kYHistogramSize; + i++) { + seen += histogram[i]; + if (seen >= pixelThreshold) { + context.activeState.black.level = i * histogramRatio; + LOG(IPASoftBL, Debug) + << "Auto-set black level: " + << i << "/" << SwIspStats::kYHistogramSize + << " (" << 100 * (seen - histogram[i]) / total << "% below, " + << 100 * seen / total << "% at or below)"; + break; + } + }; +} + +REGISTER_IPA_ALGORITHM(BlackLevel, "BlackLevel") + +} /* namespace ipa::soft::algorithms */ + +} /* namespace libcamera */ diff --git a/src/ipa/simple/algorithms/blc.h b/src/ipa/simple/algorithms/blc.h new file mode 100644 index 00000000..2f73db52 --- /dev/null +++ b/src/ipa/simple/algorithms/blc.h @@ -0,0 +1,37 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2024, Red Hat Inc. + * + * Black level handling + */ + +#pragma once + +#include <libcamera/controls.h> + +#include "libcamera/internal/software_isp/swisp_stats.h" + +#include "algorithm.h" +#include "ipa_context.h" + +namespace libcamera { + +namespace ipa::soft::algorithms { + +class BlackLevel : public Algorithm +{ +public: + BlackLevel(); + ~BlackLevel() = default; + + int init(IPAContext &context, const YamlObject &tuningData) + override; + void process(IPAContext &context, const uint32_t frame, + IPAFrameContext &frameContext, + const SwIspStats *stats, + ControlList &metadata) override; +}; + +} /* namespace ipa::soft::algorithms */ + +} /* namespace libcamera */ diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build index 31d26e43..1f63c220 100644 --- a/src/ipa/simple/algorithms/meson.build +++ b/src/ipa/simple/algorithms/meson.build @@ -1,4 +1,5 @@ # SPDX-License-Identifier: CC0-1.0 soft_simple_ipa_algorithms = files([ + 'blc.cpp', ]) diff --git a/src/ipa/simple/black_level.cpp b/src/ipa/simple/black_level.cpp deleted file mode 100644 index 37e0109c..00000000 --- a/src/ipa/simple/black_level.cpp +++ /dev/null @@ -1,93 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -/* - * Copyright (C) 2024, Red Hat Inc. - * - * black level handling - */ - -#include "black_level.h" - -#include <numeric> - -#include <libcamera/base/log.h> - -namespace libcamera { - -LOG_DEFINE_CATEGORY(IPASoftBL) - -namespace ipa::soft { - -/** - * \class BlackLevel - * \brief Object providing black point level for software ISP - * - * Black level can be provided in hardware tuning files or, if no tuning file is - * available for the given hardware, guessed automatically, with less accuracy. - * As tuning files are not yet implemented for software ISP, BlackLevel - * currently provides only guessed black levels. - * - * This class serves for tracking black level as a property of the underlying - * hardware, not as means of enhancing a particular scene or image. - * - * The class is supposed to be instantiated for the given camera stream. - * The black level can be retrieved using BlackLevel::get() method. It is - * initially 0 and may change when updated using BlackLevel::update() method. - */ - -BlackLevel::BlackLevel() - : blackLevel_(255), blackLevelSet_(false) -{ -} - -/** - * \brief Return the current black level - * - * \return The black level, in the range from 0 (minimum) to 255 (maximum). - * If the black level couldn't be determined yet, return 0. - */ -uint8_t BlackLevel::get() const -{ - return blackLevelSet_ ? blackLevel_ : 0; -} - -/** - * \brief Update black level from the provided histogram - * \param[in] yHistogram The histogram to be used for updating black level - * - * The black level is property of the given hardware, not image. It is updated - * only if it has not been yet set or if it is lower than the lowest value seen - * so far. - */ -void BlackLevel::update(SwIspStats::Histogram &yHistogram) -{ - /* - * The constant is selected to be "good enough", not overly conservative or - * aggressive. There is no magic about the given value. - */ - constexpr float ignoredPercentage_ = 0.02; - const unsigned int total = - std::accumulate(begin(yHistogram), end(yHistogram), 0); - const unsigned int pixelThreshold = ignoredPercentage_ * total; - const unsigned int histogramRatio = 256 / SwIspStats::kYHistogramSize; - const unsigned int currentBlackIdx = blackLevel_ / histogramRatio; - - for (unsigned int i = 0, seen = 0; - i < currentBlackIdx && i < SwIspStats::kYHistogramSize; - i++) { - seen += yHistogram[i]; - if (seen >= pixelThreshold) { - blackLevel_ = i * histogramRatio; - blackLevelSet_ = true; - LOG(IPASoftBL, Debug) - << "Auto-set black level: " - << i << "/" << SwIspStats::kYHistogramSize - << " (" << 100 * (seen - yHistogram[i]) / total << "% below, " - << 100 * seen / total << "% at or below)"; - break; - } - }; -} - -} /* namespace ipa::soft */ - -} /* namespace libcamera */ diff --git a/src/ipa/simple/black_level.h b/src/ipa/simple/black_level.h deleted file mode 100644 index a04230c9..00000000 --- a/src/ipa/simple/black_level.h +++ /dev/null @@ -1,33 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -/* - * Copyright (C) 2024, Red Hat Inc. - * - * black level handling - */ - -#pragma once - -#include <array> -#include <stdint.h> - -#include "libcamera/internal/software_isp/swisp_stats.h" - -namespace libcamera { - -namespace ipa::soft { - -class BlackLevel -{ -public: - BlackLevel(); - uint8_t get() const; - void update(SwIspStats::Histogram &yHistogram); - -private: - uint8_t blackLevel_; - bool blackLevelSet_; -}; - -} /* namespace ipa::soft */ - -} /* namespace libcamera */ diff --git a/src/ipa/simple/data/uncalibrated.yaml b/src/ipa/simple/data/uncalibrated.yaml index 2cdc39a8..e0d03d96 100644 --- a/src/ipa/simple/data/uncalibrated.yaml +++ b/src/ipa/simple/data/uncalibrated.yaml @@ -3,4 +3,5 @@ --- version: 1 algorithms: + - BlackLevel: ... diff --git a/src/ipa/simple/ipa_context.cpp b/src/ipa/simple/ipa_context.cpp index 3c1c7262..268cff32 100644 --- a/src/ipa/simple/ipa_context.cpp +++ b/src/ipa/simple/ipa_context.cpp @@ -50,4 +50,12 @@ namespace libcamera::ipa::soft { * \brief The current state of IPA algorithms */ +/** + * \var IPAActiveState::black + * \brief Context for the Black Level algorithm + * + * \var IPAActiveState::black.level + * \brief Current determined black level + */ + } /* namespace libcamera::ipa::soft */ diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h index bc1235b6..ed07dbb8 100644 --- a/src/ipa/simple/ipa_context.h +++ b/src/ipa/simple/ipa_context.h @@ -8,6 +8,8 @@ #pragma once +#include <stdint.h> + #include <libipa/fc_queue.h> namespace libcamera { @@ -18,6 +20,9 @@ struct IPASessionConfiguration { }; struct IPAActiveState { + struct { + uint8_t level; + } black; }; struct IPAFrameContext : public FrameContext { diff --git a/src/ipa/simple/meson.build b/src/ipa/simple/meson.build index 7515a8d8..92aa5744 100644 --- a/src/ipa/simple/meson.build +++ b/src/ipa/simple/meson.build @@ -8,7 +8,6 @@ ipa_name = 'ipa_soft_simple' soft_simple_sources = files([ 'ipa_context.cpp', 'soft_simple.cpp', - 'black_level.cpp', ]) soft_simple_sources += soft_simple_ipa_algorithms diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index a8499f29..2fb3a473 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -28,7 +28,6 @@ #include "libipa/camera_sensor_helper.h" -#include "black_level.h" #include "module.h" namespace libcamera { @@ -60,7 +59,7 @@ class IPASoftSimple : public ipa::soft::IPASoftInterface, public Module { public: IPASoftSimple() - : params_(nullptr), stats_(nullptr), blackLevel_(BlackLevel()), + : params_(nullptr), stats_(nullptr), context_({ {}, {}, { kMaxFrameContexts } }), ignoreUpdates_(0) { @@ -92,7 +91,6 @@ private: SwIspStats *stats_; std::unique_ptr<CameraSensorHelper> camHelper_; ControlInfoMap sensorInfoMap_; - BlackLevel blackLevel_; static constexpr unsigned int kGammaLookupSize = 1024; std::array<uint8_t, kGammaLookupSize> gammaTable_; @@ -303,9 +301,7 @@ void IPASoftSimple::processStats( algo->process(context_, frame, frameContext, stats_, metadata); SwIspStats::Histogram histogram = stats_->yHistogram; - if (ignoreUpdates_ > 0) - blackLevel_.update(histogram); - const uint8_t blackLevel = blackLevel_.get(); + const uint8_t blackLevel = context_.activeState.black.level; /* * Black level must be subtracted to get the correct AWB ratios, they