[PATCH v3 09/16] libcamera: ipa: add Soft IPA

Milan Zamazal mzamazal at redhat.com
Thu Feb 15 17:31:07 CET 2024


Hans de Goede <hdegoede at redhat.com> writes:

> From: Andrey Konovalov <andrey.konovalov at 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
>
> Tested-by: Bryan O'Donoghue <bryan.odonoghue at linaro.org> # sc8280xp Lenovo x13s
> Tested-by: Pavel Machek <pavel at ucw.cz>
> Reviewed-by: Pavel Machek <pavel at ucw.cz>
> Signed-off-by: Andrey Konovalov <andrey.konovalov at linaro.org>
> Co-developed-by: Dennis Bonke <admin at dennisbonke.com>
> Signed-off-by: Dennis Bonke <admin at dennisbonke.com>
> Co-developed-by: Marttico <g.martti at gmail.com>
> Signed-off-by: Marttico <g.martti at gmail.com>
> Co-developed-by: Toon Langendam <t.langendam at gmail.com>
> Signed-off-by: Toon Langendam <t.langendam at gmail.com>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
> Changes in v3:
> - Use new SwIspStats::kYHistogramSize constexpr and adjust
>   the auto-exposure/-gain code so that it can deal with
>   that having a different value then 16 (modify the loop
>   to divide the histogram in 5 bins to not have hardcoded
>   values)
> - Rename a few foo_bar symbols to fooBar
> ---
>  Documentation/Doxyfile.in         |   1 +
>  include/libcamera/ipa/meson.build |   1 +
>  include/libcamera/ipa/soft.mojom  |  28 +++
>  meson_options.txt                 |   2 +-
>  src/ipa/simple/data/meson.build   |   9 +
>  src/ipa/simple/data/soft.conf     |   3 +
>  src/ipa/simple/meson.build        |  25 +++
>  src/ipa/simple/soft_simple.cpp    | 308 ++++++++++++++++++++++++++++++
>  8 files changed, 376 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/soft.conf
>  create mode 100644 src/ipa/simple/meson.build
>  create mode 100644 src/ipa/simple/soft_simple.cpp
>
> 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..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.
> + */
> +
> +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(int32 dummy);
> +};
> diff --git a/meson_options.txt b/meson_options.txt
> index 5fdc7be8..94372e47 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/simple/data/meson.build b/src/ipa/simple/data/meson.build
> new file mode 100644
> index 00000000..33548cc6
> --- /dev/null
> +++ b/src/ipa/simple/data/meson.build
> @@ -0,0 +1,9 @@
> +# SPDX-License-Identifier: CC0-1.0
> +
> +conf_files = files([
> +    'soft.conf',
> +])
> +
> +install_data(conf_files,
> +             install_dir : ipa_data_dir / 'soft',
> +             install_tag : 'runtime')
> diff --git a/src/ipa/simple/data/soft.conf b/src/ipa/simple/data/soft.conf
> new file mode 100644
> index 00000000..0c70e7c0
> --- /dev/null
> +++ b/src/ipa/simple/data/soft.conf
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: LGPL-2.1-or-later
> +#
> +# Dummy configuration file for the soft IPA.
> 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..f7ac02f9
> --- /dev/null
> +++ b/src/ipa/simple/soft_simple.cpp
> @@ -0,0 +1,308 @@
> +/* 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 <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/camera_sensor.h"
> +#include "libcamera/internal/software_isp/debayer_params.h"
> +#include "libcamera/internal/software_isp/swisp_stats.h"
> +
> +#define EXPOSURE_OPTIMAL_VALUE 2.5
> +#define EXPOSURE_SATISFACTORY_OFFSET 0.2

Do both the values come from the cited paper or do they deserve some
explanation?

And maybe constexpr rather than #define?

> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(IPASoft)
> +
> +namespace ipa::soft {
> +
> +class IPASoftSimple : public ipa::soft::IPASoftInterface
> +{
> +public:
> +	IPASoftSimple()
> +		: ignore_updates_(0)
> +	{
> +	}
> +
> +	~IPASoftSimple()
> +	{
> +		if (stats_)
> +			munmap(stats_, sizeof(SwIspStats));
> +		if (params_)
> +			munmap(params_, sizeof(DebayerParams));
> +	}
> +
> +	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);
> +
> +	SharedFD fdStats_;
> +	SharedFD fdParams_;
> +	DebayerParams *params_;
> +	SwIspStats *stats_;
> +	int exposure_min_, exposure_max_;
> +	int again_min_, again_max_;
> +	int again_, exposure_;
> +	int ignore_updates_;
> +};
> +
> +int IPASoftSimple::init([[maybe_unused]] const IPASettings &settings,
> +			const SharedFD &fdStats,
> +			const SharedFD &fdParams,
> +			const ControlInfoMap &sensorInfoMap)
> +{
> +	fdStats_ = std::move(fdStats);
> +	if (!fdStats_.isValid()) {
> +		LOG(IPASoft, Error) << "Invalid Statistics handle";
> +		return -ENODEV;
> +	}
> +
> +	fdParams_ = std::move(fdParams);
> +	if (!fdParams_.isValid()) {
> +		LOG(IPASoft, Error) << "Invalid Parameters handle";
> +		return -ENODEV;
> +	}
> +
> +	params_ = static_cast<DebayerParams *>(mmap(nullptr, sizeof(DebayerParams),
> +						    PROT_WRITE, MAP_SHARED,
> +						    fdParams_.get(), 0));
> +	if (!params_) {
> +		LOG(IPASoft, Error) << "Unable to map Parameters";
> +		return -ENODEV;
> +	}
> +
> +	stats_ = static_cast<SwIspStats *>(mmap(nullptr, sizeof(SwIspStats),
> +						PROT_READ, MAP_SHARED,
> +						fdStats_.get(), 0));
> +	if (!stats_) {
> +		LOG(IPASoft, Error) << "Unable to map Statistics";
> +		return -ENODEV;
> +	}
> +
> +	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)
> +{
> +	const ControlInfo &exposure_info = sensorInfoMap.find(V4L2_CID_EXPOSURE)->second;
> +	const ControlInfo &gain_info = sensorInfoMap.find(V4L2_CID_ANALOGUE_GAIN)->second;
> +
> +	exposure_min_ = exposure_info.min().get<int>();
> +	if (!exposure_min_) {
> +		LOG(IPASoft, Warning) << "Minimum exposure is zero, that can't be linear";
> +		exposure_min_ = 1;
> +	}
> +	exposure_max_ = exposure_info.max().get<int>();
> +	again_min_ = gain_info.min().get<int>();
> +	if (!again_min_) {
> +		LOG(IPASoft, Warning) << "Minimum gain is zero, that can't be linear";
> +		again_min_ = 100;
> +	}

I wonder what "can't be linear" means here exactly. 

> +	again_max_ = gain_info.max().get<int>();

Can it happen that again_max_ is lower than the artificially set again_min_ =
100 above?

> +
> +	LOG(IPASoft, Info) << "Exposure " << exposure_min_ << "-" << exposure_max_
> +			   << ", gain " << again_min_ << "-" << again_max_;
> +
> +	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(0);
> +
> +	/*
> +	 * 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;
> +	}
> +
> +	/*
> +	 * Calculate Mean Sample Value (MSV) according to formula from:
> +	 * https://www.araa.asn.au/acra/acra2007/papers/paper84final.pdf
> +	 */
> +	constexpr unsigned int ExposureBinsCount = 5;
> +	constexpr unsigned int yHistValsPerBin =
> +		SwIspStats::kYHistogramSize / ExposureBinsCount;
> +	constexpr unsigned int yHistValsPerBinMod =
> +		SwIspStats::kYHistogramSize /
> +		(SwIspStats::kYHistogramSize % ExposureBinsCount + 1);
> +	int ExposureBins[ExposureBinsCount] = {};
> +	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 < ExposureBinsCount; i++) {
> +		LOG(IPASoft, Debug) << i << ": " << ExposureBins[i];
> +		denom += ExposureBins[i];
> +		num += ExposureBins[i] * (i + 1);
> +	}
> +
> +	float exposureMSV = (float)num / denom;
> +
> +	/* sanity check */
> +	if (!sensorControls.contains(V4L2_CID_EXPOSURE) ||
> +	    !sensorControls.contains(V4L2_CID_ANALOGUE_GAIN)) {
> +		LOG(IPASoft, Error) << "Control(s) missing";
> +		return;
> +	}
> +
> +	ControlList ctrls(sensorControls);
> +
> +	exposure_ = ctrls.get(V4L2_CID_EXPOSURE).get<int>();
> +	again_ = ctrls.get(V4L2_CID_ANALOGUE_GAIN).get<int>();
> +
> +	updateExposure(exposureMSV);
> +
> +	ctrls.set(V4L2_CID_EXPOSURE, exposure_);
> +	ctrls.set(V4L2_CID_ANALOGUE_GAIN, again_);
> +
> +	ignore_updates_ = 2;
> +
> +	setSensorControls.emit(ctrls);
> +
> +	LOG(IPASoft, Debug) << "exposureMSV " << exposureMSV
> +			    << " exp " << exposure_ << " again " << again_
> +			    << " gain R/B " << params_->gainR << "/" << params_->gainB;
> +}
> +
> +/* DENOMINATOR of 10 gives ~10% increment/decrement; DENOMINATOR of 5 - about ~20% */
> +#define DENOMINATOR 10
> +#define UP_NUMERATOR (DENOMINATOR + 1)
> +#define DOWN_NUMERATOR (DENOMINATOR - 1)
> +
> +void IPASoftSimple::updateExposure(double exposureMSV)
> +{
> +	int next;
> +
> +	if (exposureMSV < EXPOSURE_OPTIMAL_VALUE - EXPOSURE_SATISFACTORY_OFFSET) {
> +		next = exposure_ * UP_NUMERATOR / DENOMINATOR;
> +		if (next - exposure_ < 1)
> +			exposure_ += 1;
> +		else
> +			exposure_ = next;
> +		if (exposure_ >= exposure_max_) {
> +			next = again_ * UP_NUMERATOR / DENOMINATOR;
> +			if (next - again_ < 1)
> +				again_ += 1;
> +			else
> +				again_ = next;
> +		}
> +	}
> +
> +	if (exposureMSV > EXPOSURE_OPTIMAL_VALUE + EXPOSURE_SATISFACTORY_OFFSET) {
> +		if (exposure_ == exposure_max_ && again_ != again_min_) {
> +			next = again_ * DOWN_NUMERATOR / DENOMINATOR;
> +			if (again_ - next < 1)
> +				again_ -= 1;
> +			else
> +				again_ = next;
> +		} else {
> +			next = exposure_ * DOWN_NUMERATOR / DENOMINATOR;
> +			if (exposure_ - next < 1)
> +				exposure_ -= 1;
> +			else
> +				exposure_ = next;
> +		}
> +	}
> +
> +	if (exposure_ > exposure_max_)
> +		exposure_ = exposure_max_;
> +	else if (exposure_ < exposure_min_)
> +		exposure_ = exposure_min_;
> +
> +	if (again_ > again_max_)
> +		again_ = again_max_;
> +	else if (again_ < again_min_)
> +		again_ = again_min_;
> +}
> +
> +} /* 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 */



More information about the libcamera-devel mailing list