[v8,10/18] libcamera: ipa: Add Soft IPA
diff mbox series

Message ID 20240416091357.211951-11-mzamazal@redhat.com
State Accepted
Headers show
Series
  • libcamera: introduce Software ISP and Software IPA
Related show

Commit Message

Milan Zamazal April 16, 2024, 9:13 a.m. UTC
From: Andrey Konovalov <andrey.konovalov@linaro.org>

Define the Soft IPA main and event interfaces, add the Soft IPA
implementation.

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.

Auto exposure/gain and AWB implementation by Dennis, Toon and Martti.

Auto exposure/gain targets a Mean Sample Value of 2.5 following
the MSV calculation algorithm from:
https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf

Use CameraSensorHelper to convert the analogue gain code read from the
camera sensor into real analogue gain value. In the future this makes
it possible to use faster AE/AGC algorithm. Right now the CameraSensorHelper
lets us use the full range of analogue gain values.

If there is no CameraSensorHelper for the camera sensor in use, a
warning log message is printed.

Tested-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> # sc8280xp Lenovo x13s
Tested-by: Pavel Machek <pavel@ucw.cz>
Reviewed-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Andrey Konovalov <andrey.konovalov@linaro.org>
Co-developed-by: Dennis Bonke <admin@dennisbonke.com>
Signed-off-by: Dennis Bonke <admin@dennisbonke.com>
Co-developed-by: Marttico <g.martti@gmail.com>
Signed-off-by: Marttico <g.martti@gmail.com>
Co-developed-by: Toon Langendam <t.langendam@gmail.com>
Signed-off-by: Toon Langendam <t.langendam@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 Documentation/Doxyfile.in             |   1 +
 include/libcamera/ipa/meson.build     |   1 +
 include/libcamera/ipa/soft.mojom      |  28 ++
 meson_options.txt                     |   2 +-
 src/ipa/meson.build                   |   3 +
 src/ipa/simple/data/meson.build       |  10 +
 src/ipa/simple/data/uncalibrated.yaml |   5 +
 src/ipa/simple/meson.build            |  25 ++
 src/ipa/simple/soft_simple.cpp        | 393 ++++++++++++++++++++++++++
 src/libcamera/software_isp/TODO       |  83 ++++++
 10 files changed, 550 insertions(+), 1 deletion(-)
 create mode 100644 include/libcamera/ipa/soft.mojom
 create mode 100644 src/ipa/simple/data/meson.build
 create mode 100644 src/ipa/simple/data/uncalibrated.yaml
 create mode 100644 src/ipa/simple/meson.build
 create mode 100644 src/ipa/simple/soft_simple.cpp

Patch
diff mbox series

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.