diff options
-rw-r--r-- | include/libcamera/internal/software_isp/debayer_params.h | 4 | ||||
-rw-r--r-- | include/libcamera/internal/software_isp/swisp_stats.h | 8 | ||||
-rw-r--r-- | src/ipa/simple/black_level.cpp | 88 | ||||
-rw-r--r-- | src/ipa/simple/black_level.h | 28 | ||||
-rw-r--r-- | src/ipa/simple/meson.build | 7 | ||||
-rw-r--r-- | src/ipa/simple/soft_simple.cpp | 30 | ||||
-rw-r--r-- | src/libcamera/software_isp/TODO | 10 | ||||
-rw-r--r-- | src/libcamera/software_isp/debayer_cpu.cpp | 13 | ||||
-rw-r--r-- | src/libcamera/software_isp/debayer_cpu.h | 1 | ||||
-rw-r--r-- | src/libcamera/software_isp/software_isp.cpp | 2 |
10 files changed, 173 insertions, 18 deletions
diff --git a/include/libcamera/internal/software_isp/debayer_params.h b/include/libcamera/internal/software_isp/debayer_params.h index c818ca3a..32cd448a 100644 --- a/include/libcamera/internal/software_isp/debayer_params.h +++ b/include/libcamera/internal/software_isp/debayer_params.h @@ -20,6 +20,10 @@ struct DebayerParams { unsigned int gainB; float gamma; + /** + * \brief Level of the black point, 0..255, 0 is no correction. + */ + unsigned int blackLevel; }; } /* namespace libcamera */ diff --git a/include/libcamera/internal/software_isp/swisp_stats.h b/include/libcamera/internal/software_isp/swisp_stats.h index dd5e207d..4ca8d647 100644 --- a/include/libcamera/internal/software_isp/swisp_stats.h +++ b/include/libcamera/internal/software_isp/swisp_stats.h @@ -35,11 +35,15 @@ struct SwIspStats { /** * \brief Number of bins in the yHistogram */ - static constexpr unsigned int kYHistogramSize = 16; + static constexpr unsigned int kYHistogramSize = 64; + /** + * \brief Type of the histogram. + */ + using Histogram = std::array<uint32_t, kYHistogramSize>; /** * \brief A histogram of luminance values */ - std::array<uint32_t, kYHistogramSize> yHistogram; + Histogram yHistogram; }; } /* namespace libcamera */ diff --git a/src/ipa/simple/black_level.cpp b/src/ipa/simple/black_level.cpp new file mode 100644 index 00000000..c7e8d8b7 --- /dev/null +++ b/src/ipa/simple/black_level.cpp @@ -0,0 +1,88 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2024, Red Hat Inc. + * + * black_level.cpp - black level handling + */ + +#include "black_level.h" + +#include <numeric> + +#include <libcamera/base/log.h> + +namespace libcamera { + +LOG_DEFINE_CATEGORY(IPASoftBL) + +/** + * \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. + */ +unsigned int 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 libcamera */ diff --git a/src/ipa/simple/black_level.h b/src/ipa/simple/black_level.h new file mode 100644 index 00000000..7e37757e --- /dev/null +++ b/src/ipa/simple/black_level.h @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2024, Red Hat Inc. + * + * black_level.h - black level handling + */ + +#pragma once + +#include <array> + +#include "libcamera/internal/software_isp/swisp_stats.h" + +namespace libcamera { + +class BlackLevel +{ +public: + BlackLevel(); + unsigned int get() const; + void update(SwIspStats::Histogram &yHistogram); + +private: + unsigned int blackLevel_; + bool blackLevelSet_; +}; + +} /* namespace libcamera */ diff --git a/src/ipa/simple/meson.build b/src/ipa/simple/meson.build index 3e863db7..44b5f1d7 100644 --- a/src/ipa/simple/meson.build +++ b/src/ipa/simple/meson.build @@ -2,8 +2,13 @@ ipa_name = 'ipa_soft_simple' +soft_simple_sources = files([ + 'soft_simple.cpp', + 'black_level.cpp', +]) + mod = shared_module(ipa_name, - ['soft_simple.cpp', libcamera_generated_ipa_headers], + [soft_simple_sources, libcamera_generated_ipa_headers], name_prefix : '', include_directories : [ipa_includes, libipa_includes], dependencies : libcamera_private, diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index ff1d8e0c..b9fb58b5 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -26,8 +26,9 @@ #include "libipa/camera_sensor_helper.h" -namespace libcamera { +#include "black_level.h" +namespace libcamera { LOG_DEFINE_CATEGORY(IPASoft) namespace ipa::soft { @@ -54,7 +55,8 @@ class IPASoftSimple : public ipa::soft::IPASoftInterface { public: IPASoftSimple() - : params_(nullptr), stats_(nullptr), ignoreUpdates_(0) + : params_(nullptr), stats_(nullptr), blackLevel_(BlackLevel()), + ignoreUpdates_(0) { } @@ -78,6 +80,7 @@ private: SwIspStats *stats_; std::unique_ptr<CameraSensorHelper> camHelper_; ControlInfoMap sensorInfoMap_; + BlackLevel blackLevel_; int32_t exposureMin_, exposureMax_; int32_t exposure_; @@ -255,6 +258,10 @@ void IPASoftSimple::processStats(const ControlList &sensorControls) params_->gainG = 256; params_->gamma = 0.5; + if (ignoreUpdates_ > 0) + blackLevel_.update(stats_->yHistogram); + params_->blackLevel = blackLevel_.get(); + setIspParams.emit(); /* \todo Switch to the libipa/algorithm.h API someday. */ @@ -273,18 +280,20 @@ void IPASoftSimple::processStats(const ControlList &sensorControls) * Calculate Mean Sample Value (MSV) according to formula from: * https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf */ - constexpr unsigned int yHistValsPerBin = - SwIspStats::kYHistogramSize / kExposureBinsCount; - constexpr unsigned int yHistValsPerBinMod = - SwIspStats::kYHistogramSize / - (SwIspStats::kYHistogramSize % kExposureBinsCount + 1); + const unsigned int blackLevelHistIdx = + params_->blackLevel / (256 / SwIspStats::kYHistogramSize); + const unsigned int histogramSize = + SwIspStats::kYHistogramSize - blackLevelHistIdx; + const unsigned int yHistValsPerBin = histogramSize / kExposureBinsCount; + const unsigned int yHistValsPerBinMod = + histogramSize / (histogramSize % kExposureBinsCount + 1); int exposureBins[kExposureBinsCount] = {}; unsigned int denom = 0; unsigned int num = 0; - for (unsigned int i = 0; i < SwIspStats::kYHistogramSize; i++) { + for (unsigned int i = 0; i < histogramSize; i++) { unsigned int idx = (i - (i / yHistValsPerBinMod)) / yHistValsPerBin; - exposureBins[idx] += stats_->yHistogram[i]; + exposureBins[idx] += stats_->yHistogram[blackLevelHistIdx + i]; } for (unsigned int i = 0; i < kExposureBinsCount; i++) { @@ -320,7 +329,8 @@ void IPASoftSimple::processStats(const ControlList &sensorControls) LOG(IPASoft, Debug) << "exposureMSV " << exposureMSV << " exp " << exposure_ << " again " << again_ - << " gain R/B " << params_->gainR << "/" << params_->gainB; + << " gain R/B " << params_->gainR << "/" << params_->gainB + << " black level " << params_->blackLevel; } void IPASoftSimple::updateExposure(double exposureMSV) diff --git a/src/libcamera/software_isp/TODO b/src/libcamera/software_isp/TODO index fcb02588..4fcee39b 100644 --- a/src/libcamera/software_isp/TODO +++ b/src/libcamera/software_isp/TODO @@ -267,3 +267,13 @@ This could be handled better with DelayedControls. > V4L2_CID_EXPOSURE })); You should use the DelayedControls class. + +--- + +13. Improve black level and colour gains application + +I think the black level should eventually be moved before debayering, and +ideally the colour gains as well. I understand the need for optimizations to +lower the CPU consumption, but at the same time I don't feel comfortable +building up on top of an implementation that may work a bit more by chance than +by correctness, as that's not very maintainable. diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp index 5b553162..88d6578b 100644 --- a/src/libcamera/software_isp/debayer_cpu.cpp +++ b/src/libcamera/software_isp/debayer_cpu.cpp @@ -35,7 +35,7 @@ namespace libcamera { * \param[in] stats Pointer to the stats object to use */ DebayerCpu::DebayerCpu(std::unique_ptr<SwStatsCpu> stats) - : stats_(std::move(stats)), gammaCorrection_(1.0) + : stats_(std::move(stats)), gammaCorrection_(1.0), blackLevel_(0) { /* * Reading from uncached buffers may be very slow. @@ -699,11 +699,16 @@ void DebayerCpu::process(FrameBuffer *input, FrameBuffer *output, DebayerParams } /* Apply DebayerParams */ - if (params.gamma != gammaCorrection_) { - for (unsigned int i = 0; i < kGammaLookupSize; i++) - gamma_[i] = UINT8_MAX * powf(i / (kGammaLookupSize - 1.0), params.gamma); + 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_) diff --git a/src/libcamera/software_isp/debayer_cpu.h b/src/libcamera/software_isp/debayer_cpu.h index e7a8ba74..689c1075 100644 --- a/src/libcamera/software_isp/debayer_cpu.h +++ b/src/libcamera/software_isp/debayer_cpu.h @@ -147,6 +147,7 @@ private: bool enableInputMemcpy_; bool swapRedBlueGains_; float gammaCorrection_; + unsigned int blackLevel_; unsigned int measuredFrames_; int64_t frameProcessTime_; /* Skip 30 frames for things to stabilize then measure 30 frames */ diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp index d746d893..e4e56086 100644 --- a/src/libcamera/software_isp/software_isp.cpp +++ b/src/libcamera/software_isp/software_isp.cpp @@ -64,7 +64,7 @@ LOG_DEFINE_CATEGORY(SoftwareIsp) */ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor) : debayerParams_{ DebayerParams::kGain10, DebayerParams::kGain10, - DebayerParams::kGain10, 0.5f }, + DebayerParams::kGain10, 0.5f, 0 }, dmaHeap_(DmaHeap::DmaHeapFlag::Cma | DmaHeap::DmaHeapFlag::System) { if (!dmaHeap_.isValid()) { |