@@ -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 */
@@ -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 */
new file mode 100644
@@ -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 */
new file mode 100644
@@ -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(std::array<unsigned int, SwIspStats::kYHistogramSize> &yHistogram);
+
+private:
+ unsigned int blackLevel_;
+ bool blackLevelSet_;
+};
+
+} /* namespace libcamera */
@@ -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,
@@ -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(0);
/* \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)
@@ -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.
@@ -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_)
@@ -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 */
@@ -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()) {