diff options
| -rw-r--r-- | Documentation/Doxyfile.in | 1 | ||||
| -rw-r--r-- | include/libcamera/ipa/meson.build | 1 | ||||
| -rw-r--r-- | include/libcamera/ipa/soft.mojom | 28 | ||||
| -rw-r--r-- | meson_options.txt | 2 | ||||
| -rw-r--r-- | src/ipa/meson.build | 3 | ||||
| -rw-r--r-- | src/ipa/simple/data/meson.build | 10 | ||||
| -rw-r--r-- | src/ipa/simple/data/uncalibrated.yaml | 5 | ||||
| -rw-r--r-- | src/ipa/simple/meson.build | 25 | ||||
| -rw-r--r-- | src/ipa/simple/soft_simple.cpp | 393 | ||||
| -rw-r--r-- | src/libcamera/software_isp/TODO | 83 |
10 files changed, 550 insertions, 1 deletions
diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in index a86ea6c1..2be8d47b 100644 --- a/Documentation/Doxyfile.in +++ b/Documentation/Doxyfile.in @@ -44,6 +44,7 @@ EXCLUDE = @TOP_SRCDIR@/include/libcamera/base/span.h \ @TOP_SRCDIR@/src/libcamera/pipeline/ \ @TOP_SRCDIR@/src/libcamera/tracepoints.cpp \ @TOP_BUILDDIR@/include/libcamera/internal/tracepoints.h \ + @TOP_BUILDDIR@/include/libcamera/ipa/soft_ipa_interface.h \ @TOP_BUILDDIR@/src/libcamera/proxy/ EXCLUDE_PATTERNS = @TOP_BUILDDIR@/include/libcamera/ipa/*_serializer.h \ diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build index f3b4881c..3352d08f 100644 --- a/include/libcamera/ipa/meson.build +++ b/include/libcamera/ipa/meson.build @@ -65,6 +65,7 @@ pipeline_ipa_mojom_mapping = { 'ipu3': 'ipu3.mojom', 'rkisp1': 'rkisp1.mojom', 'rpi/vc4': 'raspberrypi.mojom', + 'simple': 'soft.mojom', 'vimc': 'vimc.mojom', } diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom new file mode 100644 index 00000000..3aa2066e --- /dev/null +++ b/include/libcamera/ipa/soft.mojom @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +/* + * \todo Document the interface and remove the related EXCLUDE_PATTERNS entry. + */ + +module ipa.soft; + +import "include/libcamera/ipa/core.mojom"; + +interface IPASoftInterface { + init(libcamera.IPASettings settings, + libcamera.SharedFD fdStats, + libcamera.SharedFD fdParams, + libcamera.ControlInfoMap sensorCtrlInfoMap) + => (int32 ret); + start() => (int32 ret); + stop(); + configure(libcamera.ControlInfoMap sensorCtrlInfoMap) + => (int32 ret); + + [async] processStats(libcamera.ControlList sensorControls); +}; + +interface IPASoftEventInterface { + setSensorControls(libcamera.ControlList sensorControls); + setIspParams(); +}; diff --git a/meson_options.txt b/meson_options.txt index 7c4f6d3a..c61eb555 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -27,7 +27,7 @@ option('gstreamer', option('ipas', type : 'array', - choices : ['ipu3', 'rkisp1', 'rpi/vc4', 'vimc'], + choices : ['ipu3', 'rkisp1', 'rpi/vc4', 'simple', 'vimc'], description : 'Select which IPA modules to build') option('lc-compliance', diff --git a/src/ipa/meson.build b/src/ipa/meson.build index 48793e07..0ad4631d 100644 --- a/src/ipa/meson.build +++ b/src/ipa/meson.build @@ -41,6 +41,9 @@ ipa_names = [] subdirs = [] foreach pipeline : pipelines + # The current implementation expects the IPA module name to match the + # pipeline name. + # \todo Make the IPA naming scheme more flexible. if not ipa_modules.contains(pipeline) continue endif diff --git a/src/ipa/simple/data/meson.build b/src/ipa/simple/data/meson.build new file mode 100644 index 00000000..92795ee4 --- /dev/null +++ b/src/ipa/simple/data/meson.build @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: CC0-1.0 + +conf_files = files([ + 'uncalibrated.yaml', +]) + +# The install_dir must match the name from the IPAModuleInfo +install_data(conf_files, + install_dir : ipa_data_dir / 'simple', + install_tag : 'runtime') diff --git a/src/ipa/simple/data/uncalibrated.yaml b/src/ipa/simple/data/uncalibrated.yaml new file mode 100644 index 00000000..ff981a1a --- /dev/null +++ b/src/ipa/simple/data/uncalibrated.yaml @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: CC0-1.0 +%YAML 1.1 +--- +version: 1 +... diff --git a/src/ipa/simple/meson.build b/src/ipa/simple/meson.build new file mode 100644 index 00000000..3e863db7 --- /dev/null +++ b/src/ipa/simple/meson.build @@ -0,0 +1,25 @@ +# SPDX-License-Identifier: CC0-1.0 + +ipa_name = 'ipa_soft_simple' + +mod = shared_module(ipa_name, + ['soft_simple.cpp', libcamera_generated_ipa_headers], + name_prefix : '', + include_directories : [ipa_includes, libipa_includes], + dependencies : libcamera_private, + link_with : libipa, + install : true, + install_dir : ipa_install_dir) + +if ipa_sign_module + custom_target(ipa_name + '.so.sign', + input : mod, + output : ipa_name + '.so.sign', + command : [ipa_sign, ipa_priv_key, '@INPUT@', '@OUTPUT@'], + install : false, + build_by_default : true) +endif + +subdir('data') + +ipa_names += ipa_name diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp new file mode 100644 index 00000000..ff1d8e0c --- /dev/null +++ b/src/ipa/simple/soft_simple.cpp @@ -0,0 +1,393 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2023, Linaro Ltd + * + * soft_simple.cpp - Simple Software Image Processing Algorithm module + */ + +#include <sys/mman.h> + +#include <linux/v4l2-controls.h> + +#include <libcamera/base/file.h> +#include <libcamera/base/log.h> +#include <libcamera/base/shared_fd.h> + +#include <libcamera/control_ids.h> +#include <libcamera/controls.h> + +#include <libcamera/ipa/ipa_interface.h> +#include <libcamera/ipa/ipa_module_info.h> +#include <libcamera/ipa/soft_ipa_interface.h> + +#include "libcamera/internal/software_isp/debayer_params.h" +#include "libcamera/internal/software_isp/swisp_stats.h" +#include "libcamera/internal/yaml_parser.h" + +#include "libipa/camera_sensor_helper.h" + +namespace libcamera { + +LOG_DEFINE_CATEGORY(IPASoft) + +namespace ipa::soft { + +/* + * The number of bins to use for the optimal exposure calculations. + */ +static constexpr unsigned int kExposureBinsCount = 5; + +/* + * The exposure is optimal when the mean sample value of the histogram is + * in the middle of the range. + */ +static constexpr float kExposureOptimal = kExposureBinsCount / 2.0; + +/* + * The below value implements the hysteresis for the exposure adjustment. + * It is small enough to have the exposure close to the optimal, and is big + * enough to prevent the exposure from wobbling around the optimal value. + */ +static constexpr float kExposureSatisfactory = 0.2; + +class IPASoftSimple : public ipa::soft::IPASoftInterface +{ +public: + IPASoftSimple() + : params_(nullptr), stats_(nullptr), ignoreUpdates_(0) + { + } + + ~IPASoftSimple(); + + int init(const IPASettings &settings, + const SharedFD &fdStats, + const SharedFD &fdParams, + const ControlInfoMap &sensorInfoMap) override; + int configure(const ControlInfoMap &sensorInfoMap) override; + + int start() override; + void stop() override; + + void processStats(const ControlList &sensorControls) override; + +private: + void updateExposure(double exposureMSV); + + DebayerParams *params_; + SwIspStats *stats_; + std::unique_ptr<CameraSensorHelper> camHelper_; + ControlInfoMap sensorInfoMap_; + + int32_t exposureMin_, exposureMax_; + int32_t exposure_; + double againMin_, againMax_, againMinStep_; + double again_; + unsigned int ignoreUpdates_; +}; + +IPASoftSimple::~IPASoftSimple() +{ + if (stats_) + munmap(stats_, sizeof(SwIspStats)); + if (params_) + munmap(params_, sizeof(DebayerParams)); +} + +int IPASoftSimple::init(const IPASettings &settings, + const SharedFD &fdStats, + const SharedFD &fdParams, + const ControlInfoMap &sensorInfoMap) +{ + camHelper_ = CameraSensorHelperFactoryBase::create(settings.sensorModel); + if (!camHelper_) { + LOG(IPASoft, Warning) + << "Failed to create camera sensor helper for " + << settings.sensorModel; + } + + /* Load the tuning data file */ + File file(settings.configurationFile); + if (!file.open(File::OpenModeFlag::ReadOnly)) { + int ret = file.error(); + LOG(IPASoft, Error) + << "Failed to open configuration file " + << settings.configurationFile << ": " << strerror(-ret); + return ret; + } + + std::unique_ptr<libcamera::YamlObject> data = YamlParser::parse(file); + if (!data) + return -EINVAL; + + /* \todo Use the IPA configuration file for real. */ + unsigned int version = (*data)["version"].get<uint32_t>(0); + LOG(IPASoft, Debug) << "Tuning file version " << version; + + params_ = nullptr; + stats_ = nullptr; + + if (!fdStats.isValid()) { + LOG(IPASoft, Error) << "Invalid Statistics handle"; + return -ENODEV; + } + + if (!fdParams.isValid()) { + LOG(IPASoft, Error) << "Invalid Parameters handle"; + return -ENODEV; + } + + { + void *mem = mmap(nullptr, sizeof(DebayerParams), PROT_WRITE, + MAP_SHARED, fdParams.get(), 0); + if (mem == MAP_FAILED) { + LOG(IPASoft, Error) << "Unable to map Parameters"; + return -errno; + } + + params_ = static_cast<DebayerParams *>(mem); + } + + { + void *mem = mmap(nullptr, sizeof(SwIspStats), PROT_READ, + MAP_SHARED, fdStats.get(), 0); + if (mem == MAP_FAILED) { + LOG(IPASoft, Error) << "Unable to map Statistics"; + return -errno; + } + + stats_ = static_cast<SwIspStats *>(mem); + } + + /* + * Check if the sensor driver supports the controls required by the + * Soft IPA. + * Don't save the min and max control values yet, as e.g. the limits + * for V4L2_CID_EXPOSURE depend on the configured sensor resolution. + */ + if (sensorInfoMap.find(V4L2_CID_EXPOSURE) == sensorInfoMap.end()) { + LOG(IPASoft, Error) << "Don't have exposure control"; + return -EINVAL; + } + + if (sensorInfoMap.find(V4L2_CID_ANALOGUE_GAIN) == sensorInfoMap.end()) { + LOG(IPASoft, Error) << "Don't have gain control"; + return -EINVAL; + } + + return 0; +} + +int IPASoftSimple::configure(const ControlInfoMap &sensorInfoMap) +{ + sensorInfoMap_ = sensorInfoMap; + + const ControlInfo &exposureInfo = sensorInfoMap_.find(V4L2_CID_EXPOSURE)->second; + const ControlInfo &gainInfo = sensorInfoMap_.find(V4L2_CID_ANALOGUE_GAIN)->second; + + exposureMin_ = exposureInfo.min().get<int32_t>(); + exposureMax_ = exposureInfo.max().get<int32_t>(); + if (!exposureMin_) { + LOG(IPASoft, Warning) << "Minimum exposure is zero, that can't be linear"; + exposureMin_ = 1; + } + + int32_t againMin = gainInfo.min().get<int32_t>(); + int32_t againMax = gainInfo.max().get<int32_t>(); + + if (camHelper_) { + againMin_ = camHelper_->gain(againMin); + againMax_ = camHelper_->gain(againMax); + againMinStep_ = (againMax_ - againMin_) / 100.0; + } else { + /* + * The camera sensor gain (g) is usually not equal to the value written + * into the gain register (x). But the way how the AGC algorithm changes + * the gain value to make the total exposure closer to the optimum + * assumes that g(x) is not too far from linear function. If the minimal + * gain is 0, the g(x) is likely to be far from the linear, like + * g(x) = a / (b * x + c). To avoid unexpected changes to the gain by + * the AGC algorithm (abrupt near one edge, and very small near the + * other) we limit the range of the gain values used. + */ + againMax_ = againMax; + if (!againMin) { + LOG(IPASoft, Warning) + << "Minimum gain is zero, that can't be linear"; + againMin_ = std::min(100, againMin / 2 + againMax / 2); + } + againMinStep_ = 1.0; + } + + LOG(IPASoft, Info) << "Exposure " << exposureMin_ << "-" << exposureMax_ + << ", gain " << againMin_ << "-" << againMax_ + << " (" << againMinStep_ << ")"; + + return 0; +} + +int IPASoftSimple::start() +{ + return 0; +} + +void IPASoftSimple::stop() +{ +} + +void IPASoftSimple::processStats(const ControlList &sensorControls) +{ + /* + * Calculate red and blue gains for AWB. + * Clamp max gain at 4.0, this also avoids 0 division. + */ + if (stats_->sumR_ <= stats_->sumG_ / 4) + params_->gainR = 1024; + else + params_->gainR = 256 * stats_->sumG_ / stats_->sumR_; + + if (stats_->sumB_ <= stats_->sumG_ / 4) + params_->gainB = 1024; + else + params_->gainB = 256 * stats_->sumG_ / stats_->sumB_; + + /* Green gain and gamma values are fixed */ + params_->gainG = 256; + params_->gamma = 0.5; + + setIspParams.emit(); + + /* \todo Switch to the libipa/algorithm.h API someday. */ + + /* + * AE / AGC, use 2 frames delay to make sure that the exposure and + * the gain set have applied to the camera sensor. + * \todo This could be handled better with DelayedControls. + */ + if (ignoreUpdates_ > 0) { + --ignoreUpdates_; + return; + } + + /* + * 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); + int exposureBins[kExposureBinsCount] = {}; + unsigned int denom = 0; + unsigned int num = 0; + + for (unsigned int i = 0; i < SwIspStats::kYHistogramSize; i++) { + unsigned int idx = (i - (i / yHistValsPerBinMod)) / yHistValsPerBin; + exposureBins[idx] += stats_->yHistogram[i]; + } + + for (unsigned int i = 0; i < kExposureBinsCount; i++) { + LOG(IPASoft, Debug) << i << ": " << exposureBins[i]; + denom += exposureBins[i]; + num += exposureBins[i] * (i + 1); + } + + float exposureMSV = static_cast<float>(num) / denom; + + /* Sanity check */ + if (!sensorControls.contains(V4L2_CID_EXPOSURE) || + !sensorControls.contains(V4L2_CID_ANALOGUE_GAIN)) { + LOG(IPASoft, Error) << "Control(s) missing"; + return; + } + + exposure_ = sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>(); + int32_t again = sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>(); + again_ = camHelper_ ? camHelper_->gain(again) : again; + + updateExposure(exposureMSV); + + ControlList ctrls(sensorInfoMap_); + + ctrls.set(V4L2_CID_EXPOSURE, exposure_); + ctrls.set(V4L2_CID_ANALOGUE_GAIN, + static_cast<int32_t>(camHelper_ ? camHelper_->gainCode(again_) : again_)); + + ignoreUpdates_ = 2; + + setSensorControls.emit(ctrls); + + LOG(IPASoft, Debug) << "exposureMSV " << exposureMSV + << " exp " << exposure_ << " again " << again_ + << " gain R/B " << params_->gainR << "/" << params_->gainB; +} + +void IPASoftSimple::updateExposure(double exposureMSV) +{ + /* + * kExpDenominator of 10 gives ~10% increment/decrement; + * kExpDenominator of 5 - about ~20% + */ + static constexpr uint8_t kExpDenominator = 10; + static constexpr uint8_t kExpNumeratorUp = kExpDenominator + 1; + static constexpr uint8_t kExpNumeratorDown = kExpDenominator - 1; + + double next; + + if (exposureMSV < kExposureOptimal - kExposureSatisfactory) { + next = exposure_ * kExpNumeratorUp / kExpDenominator; + if (next - exposure_ < 1) + exposure_ += 1; + else + exposure_ = next; + if (exposure_ >= exposureMax_) { + next = again_ * kExpNumeratorUp / kExpDenominator; + if (next - again_ < againMinStep_) + again_ += againMinStep_; + else + again_ = next; + } + } + + if (exposureMSV > kExposureOptimal + kExposureSatisfactory) { + if (exposure_ == exposureMax_ && again_ > againMin_) { + next = again_ * kExpNumeratorDown / kExpDenominator; + if (again_ - next < againMinStep_) + again_ -= againMinStep_; + else + again_ = next; + } else { + next = exposure_ * kExpNumeratorDown / kExpDenominator; + if (exposure_ - next < 1) + exposure_ -= 1; + else + exposure_ = next; + } + } + + exposure_ = std::clamp(exposure_, exposureMin_, exposureMax_); + again_ = std::clamp(again_, againMin_, againMax_); +} + +} /* namespace ipa::soft */ + +/* + * External IPA module interface + */ +extern "C" { +const struct IPAModuleInfo ipaModuleInfo = { + IPA_MODULE_API_VERSION, + 0, + "SimplePipelineHandler", + "simple", +}; + +IPAInterface *ipaCreate() +{ + return new ipa::soft::IPASoftSimple(); +} + +} /* extern "C" */ + +} /* namespace libcamera */ diff --git a/src/libcamera/software_isp/TODO b/src/libcamera/software_isp/TODO index 29be5386..ae0af25b 100644 --- a/src/libcamera/software_isp/TODO +++ b/src/libcamera/software_isp/TODO @@ -173,3 +173,86 @@ the need for performances and the need for a maintainable architecture. > I think this falls under the lets wait until we have a GPU > based SoftISP MVP/POC and then do some refactoring to see which > bits should go where. + +--- + +8. Decouple pipeline and IPA naming + +> The current src/ipa/meson.build assumes the IPA name to match the +> pipeline name. For this reason "-Dipas=simple" is used for the +> Soft IPA module. + +This should be addressed. + +--- + +9. Doxyfile cleanup + +>> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in +>> index a86ea6c1..2be8d47b 100644 +>> --- a/Documentation/Doxyfile.in +>> +++ b/Documentation/Doxyfile.in +>> @@ -44,6 +44,7 @@ EXCLUDE = @TOP_SRCDIR@/include/libcamera/base/span.h \ +>> @TOP_SRCDIR@/src/libcamera/pipeline/ \ +>> @TOP_SRCDIR@/src/libcamera/tracepoints.cpp \ +>> @TOP_BUILDDIR@/include/libcamera/internal/tracepoints.h \ +>> + @TOP_BUILDDIR@/include/libcamera/ipa/soft_ipa_interface.h \ +> Why is this needed ? +> +>> @TOP_BUILDDIR@/src/libcamera/proxy/ +>> EXCLUDE_PATTERNS = @TOP_BUILDDIR@/include/libcamera/ipa/*_serializer.h \ +>> diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build +>> index f3b4881c..3352d08f 100644 +>> --- a/include/libcamera/ipa/meson.build +>> +++ b/include/libcamera/ipa/meson.build +>> @@ -65,6 +65,7 @@ pipeline_ipa_mojom_mapping = { +>> 'ipu3': 'ipu3.mojom', +>> 'rkisp1': 'rkisp1.mojom', +>> 'rpi/vc4': 'raspberrypi.mojom', +>> + 'simple': 'soft.mojom', +>> 'vimc': 'vimc.mojom', +>> } +>> diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom +>> new file mode 100644 +>> index 00000000..c249bd75 +>> --- /dev/null +>> +++ b/include/libcamera/ipa/soft.mojom +>> @@ -0,0 +1,28 @@ +>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +>> + +>> +/* +>> + * \todo Document the interface and remove the related EXCLUDE_PATTERNS entry. +> Ah that's why. + +Yes, because, well... all the other IPAs were doing that... + +> It doesn't have to be done before merging, but could you +> address this sooner than later ? + +--- + +10. Switch to libipa/algorithm.h API in processStats + +>> void IPASoftSimple::processStats(const ControlList &sensorControls) +>> +> Do you envision switching to the libipa/algorithm.h API at some point ? + +At some point, yes. + +--- + +11. Improve handling the sensor controls which take effect with a delay + +> void IPASoftSimple::processStats(const ControlList &sensorControls) +> { +> ... +> /* +> * AE / AGC, use 2 frames delay to make sure that the exposure and +> * the gain set have applied to the camera sensor. +> */ +> if (ignore_updates_ > 0) { +> --ignore_updates_; +> return; +> } + +This could be handled better with DelayedControls. |