[libcamera-devel] [PATCH v5 8/9] ipa: rkisp1: Add support of Filter control

Laurent Pinchart laurent.pinchart at ideasonboard.com
Thu Jul 28 12:29:08 CEST 2022


Hi Jacopo,

On Thu, Jul 28, 2022 at 09:51:49AM +0200, Jacopo Mondi wrote:
> On Thu, Jul 28, 2022 at 01:21:48AM +0300, Laurent Pinchart via libcamera-devel wrote:
> > From: Florian Sylvestre <fsylvestre at baylibre.com>
> >
> > Denoise and Sharpness filters will be applied by RkISP1 during the
> > demosaicing step. The denoise filter is responsible for removing noise from
> > the image, while the sharpness filter will enhance its acutance.
> >
> > Add filter algorithm with denoise and sharpness values based on user controls.
> >
> > Signed-off-by: Florian Sylvestre <fsylvestre at baylibre.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  src/ipa/rkisp1/algorithms/filter.cpp     | 201 +++++++++++++++++++++++
> >  src/ipa/rkisp1/algorithms/filter.h       |  30 ++++
> >  src/ipa/rkisp1/algorithms/meson.build    |   1 +
> >  src/ipa/rkisp1/data/ov5640.yaml          |   1 +
> >  src/ipa/rkisp1/ipa_context.cpp           |  14 ++
> >  src/ipa/rkisp1/ipa_context.h             |   6 +
> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp |   8 +
> >  7 files changed, 261 insertions(+)
> >  create mode 100644 src/ipa/rkisp1/algorithms/filter.cpp
> >  create mode 100644 src/ipa/rkisp1/algorithms/filter.h
> >
> > diff --git a/src/ipa/rkisp1/algorithms/filter.cpp b/src/ipa/rkisp1/algorithms/filter.cpp
> > new file mode 100644
> > index 000000000000..8ca10fd1ee9d
> > --- /dev/null
> > +++ b/src/ipa/rkisp1/algorithms/filter.cpp
> > @@ -0,0 +1,201 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2021-2022, Ideas On Board
> > + *
> > + * filter.cpp - RkISP1 Filter control
> > + */
> > +
> > +#include "filter.h"
> > +
> > +#include <cmath>
> > +
> > +#include <libcamera/base/log.h>
> > +
> > +#include <libcamera/control_ids.h>
> > +
> > +/**
> > + * \file filter.h
> > + */
> > +
> > +namespace libcamera {
> > +
> > +namespace ipa::rkisp1::algorithms {
> > +
> > +/**
> > + * \class Filter
> > + * \brief RkISP1 Filter control
> > + *
> > + * Denoise and Sharpness filters will be applied by RkISP1 during the
> > + * demosaicing step. The denoise filter is responsible for removing noise from
> > + * the image, while the sharpness filter will enhance its acutance.
> > + *
> > + * \todo In current version the denoise and sharpness control is based on user
> > + * controls. In a future version it should be controlled automatically by the
> > + * algorithm.
> > + */
> > +
> > +LOG_DEFINE_CATEGORY(RkISP1Filter)
> > +
> > +static constexpr uint32_t kFiltLumWeightDefault = 0x00022040;
> > +static constexpr uint32_t kFiltModeDefault = 0x000004f2;
> > +
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::queueRequest
> > + */
> > +void Filter::queueRequest(IPAContext &context,
> > +			  [[maybe_unused]] const uint32_t frame,
> > +			  const ControlList &controls)
> > +{
> > +	auto &filter = context.frameContext.filter;
> > +
> > +	const auto &sharpness = controls.get(controls::Sharpness);
> > +	if (sharpness) {
> > +		filter.sharpness = std::round(std::clamp(*sharpness, 0.0f, 10.0f));
> > +		filter.updateParams = true;
> > +
> > +		LOG(RkISP1Filter, Debug) << "Set sharpness to " << *sharpness;
> > +	}
> > +
> > +	const auto &denoise = controls.get(controls::draft::NoiseReductionMode);
> > +	if (denoise) {
> > +		LOG(RkISP1Filter, Debug) << "Set denoise to " << *denoise;
> > +
> > +		switch (*denoise) {
> > +		case controls::draft::NoiseReductionModeOff:
> > +			filter.denoise = 0;
> > +			filter.updateParams = true;
> > +			break;
> > +		case controls::draft::NoiseReductionModeMinimal:
> > +			filter.denoise = 1;
> > +			filter.updateParams = true;
> > +			break;
> > +		case controls::draft::NoiseReductionModeHighQuality:
> > +		case controls::draft::NoiseReductionModeFast:
> > +			filter.denoise = 3;
> > +			filter.updateParams = true;
> > +			break;
> > +		default:
> > +			LOG(RkISP1Filter, Error)
> > +				<< "Unsupported denoise value "
> > +				<< *denoise;
> > +		}
> > +	}
> > +}
> > +
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::prepare
> > + */
> > +void Filter::prepare(IPAContext &context, rkisp1_params_cfg *params)
> > +{
> > +	auto &filter = context.frameContext.filter;
> > +
> > +	/* Check if the algorithm configuration has been updated. */
> > +	if (!filter.updateParams)
> > +		return;
> > +
> > +	filter.updateParams = false;
> > +
> > +	static constexpr uint16_t filt_fac_sh0[] = {
> > +		0x04, 0x07, 0x0a, 0x0c, 0x10, 0x14, 0x1a, 0x1e, 0x24, 0x2a, 0x30
> > +	};
> > +
> > +	static constexpr uint16_t filt_fac_sh1[] = {
> > +		0x04, 0x08, 0x0c, 0x10, 0x16, 0x1b, 0x20, 0x26, 0x2c, 0x30, 0x3f
> > +	};
> > +
> > +	static constexpr uint16_t filt_fac_mid[] = {
> > +		0x04, 0x06, 0x08, 0x0a, 0x0c, 0x10, 0x13, 0x17, 0x1d, 0x22, 0x28
> > +	};
> > +
> > +	static constexpr uint16_t filt_fac_bl0[] = {
> > +		0x02, 0x02, 0x04, 0x06, 0x08, 0x0a, 0x0c, 0x10, 0x15, 0x1a, 0x24
> > +	};
> > +
> > +	static constexpr uint16_t filt_fac_bl1[] = {
> > +		0x00, 0x00, 0x00, 0x02, 0x04, 0x04, 0x06, 0x08, 0x0d, 0x14, 0x20
> > +	};
> > +
> > +	static constexpr uint16_t filt_thresh_sh0[] = {
> > +		0, 18, 26, 36, 41, 75, 90, 120, 170, 250, 1023
> > +	};
> > +
> > +	static constexpr uint16_t filt_thresh_sh1[] = {
> > +		0, 33, 44, 51, 67, 100, 120, 150, 200, 300, 1023
> > +	};
> > +
> > +	static constexpr uint16_t filt_thresh_bl0[] = {
> > +		0, 8, 13, 23, 26, 50, 60, 80, 140, 180, 1023
> > +	};
> > +
> > +	static constexpr uint16_t filt_thresh_bl1[] = {
> > +		0, 2, 5, 10, 15, 20, 26, 51, 100, 150, 1023
> > +	};
> > +
> > +	static constexpr uint16_t stage1_select[] = {
> > +		6, 6, 4, 4, 3, 3, 2, 2, 2, 1, 0
> > +	};
> > +
> > +	static constexpr uint16_t filt_chr_v_mode[] = {
> > +		1, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3
> > +	};
> > +
> > +	static constexpr uint16_t filt_chr_h_mode[] = {
> > +		0, 3, 3, 3, 3, 3, 3, 3, 3, 3, 3
> > +	};
> > +
> > +	uint8_t denoise = filter.denoise;
> > +	uint8_t sharpness = filter.sharpness;
> > +	auto &flt_config = params->others.flt_config;
> > +
> > +	flt_config.fac_sh0 = filt_fac_sh0[sharpness];
> > +	flt_config.fac_sh1 = filt_fac_sh1[sharpness];
> > +	flt_config.fac_mid = filt_fac_mid[sharpness];
> > +	flt_config.fac_bl0 = filt_fac_bl0[sharpness];
> > +	flt_config.fac_bl1 = filt_fac_bl1[sharpness];
> > +
> > +	flt_config.lum_weight = kFiltLumWeightDefault;
> > +	flt_config.mode = kFiltModeDefault;
> > +	flt_config.thresh_sh0 = filt_thresh_sh0[denoise];
> > +	flt_config.thresh_sh1 = filt_thresh_sh1[denoise];
> > +	flt_config.thresh_bl0 = filt_thresh_bl0[denoise];
> > +	flt_config.thresh_bl1 = filt_thresh_bl1[denoise];
> > +	flt_config.grn_stage1 = stage1_select[denoise];
> > +	flt_config.chr_v_mode = filt_chr_v_mode[denoise];
> > +	flt_config.chr_h_mode = filt_chr_h_mode[denoise];
> > +
> > +	/*
> > +	 * Combined high denoising and high sharpening requires some
> > +	 * adjustments to the configuration of the filters. A first stage
> > +	 * filter with a lower strength must be selected, and the blur factors
> > +	 * must be decreased.
> > +	 */
> > +	if (denoise == 9) {
> > +		if (sharpness > 3)
> > +			flt_config.grn_stage1 = 2;
> > +	} else if (denoise == 10) {
> > +		if (sharpness > 5)
> > +			flt_config.grn_stage1 = 2;
> > +		else if (sharpness > 3)
> > +			flt_config.grn_stage1 = 1;
> > +	}
> > +
> > +	if (denoise > 7) {
> > +		if (sharpness > 7) {
> > +			flt_config.fac_bl0 /= 2;
> > +			flt_config.fac_bl1 /= 4;
> > +		} else if (sharpness > 4) {
> > +			flt_config.fac_bl0 = flt_config.fac_bl0 * 3 / 4;
> > +			flt_config.fac_bl1 /= 2;
> > +		}
> > +	}
> > +
> > +	params->module_en_update |= RKISP1_CIF_ISP_MODULE_FLT;
> > +	params->module_ens |= RKISP1_CIF_ISP_MODULE_FLT;
> > +	params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_FLT;
> > +}
> > +
> > +REGISTER_IPA_ALGORITHM(Filter, "Filter")
> > +
> > +} /* namespace ipa::rkisp1::algorithms */
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/ipa/rkisp1/algorithms/filter.h b/src/ipa/rkisp1/algorithms/filter.h
> > new file mode 100644
> > index 000000000000..9eb170eb7da1
> > --- /dev/null
> > +++ b/src/ipa/rkisp1/algorithms/filter.h
> > @@ -0,0 +1,30 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2021-2022, Ideas On Board
> > + *
> > + * filter.h - RkISP1 Filter control
> > + */
> > +
> > +#pragma once
> > +
> > +#include <sys/types.h>
> > +
> > +#include "algorithm.h"
> > +
> > +namespace libcamera {
> > +
> > +namespace ipa::rkisp1::algorithms {
> > +
> > +class Filter : public Algorithm
> > +{
> > +public:
> > +	Filter() = default;
> > +	~Filter() = default;
> > +
> > +	void queueRequest(IPAContext &context, const uint32_t frame,
> > +			  const ControlList &controls) override;
> > +	void prepare(IPAContext &context, rkisp1_params_cfg *params) override;
> > +};
> > +
> > +} /* namespace ipa::rkisp1::algorithms */
> > +} /* namespace libcamera */
> > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build
> > index 870074939d6a..dcd24fe07d23 100644
> > --- a/src/ipa/rkisp1/algorithms/meson.build
> > +++ b/src/ipa/rkisp1/algorithms/meson.build
> > @@ -5,6 +5,7 @@ rkisp1_ipa_algorithms = files([
> >      'awb.cpp',
> >      'blc.cpp',
> >      'dpcc.cpp',
> > +    'filter.cpp',
> >      'gsl.cpp',
> >      'lsc.cpp',
> >  ])
> > diff --git a/src/ipa/rkisp1/data/ov5640.yaml b/src/ipa/rkisp1/data/ov5640.yaml
> > index 2315ec437f77..99529481f593 100644
> > --- a/src/ipa/rkisp1/data/ov5640.yaml
> > +++ b/src/ipa/rkisp1/data/ov5640.yaml
> > @@ -155,4 +155,5 @@ algorithms:
> >            rnd-offsets:
> >              green: 2
> >              red-blue: 2
> > +  - Filter:
> >  ...
> > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> > index 30bb87a967bd..4b1171865f1f 100644
> > --- a/src/ipa/rkisp1/ipa_context.cpp
> > +++ b/src/ipa/rkisp1/ipa_context.cpp
> > @@ -136,6 +136,20 @@ namespace libcamera::ipa::rkisp1 {
> >   * \brief Estimated color temperature
> >   */
> >
> > +/**
> > + * \var IPAFrameContext::filter
> > + * \brief Context for the Filter algorithm
> > + *
> > + * \struct IPAFrameContext::filter.denoise
> > + * \brief Denoising level
> > + *
> > + * \var IPAFrameContext::filter.sharpness
> > + * \brief Sharpness level
> > + *
> > + * \var IPAFrameContext::filter.updateParams
> > + * \brief Indicates if ISP parameters need to be updated
> > + */
> > +
> >  /**
> >   * \var IPAFrameContext::sensor
> >   * \brief Effective sensor values
> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> > index 3bfb262c8eb3..3b2f6af1276f 100644
> > --- a/src/ipa/rkisp1/ipa_context.h
> > +++ b/src/ipa/rkisp1/ipa_context.h
> > @@ -57,6 +57,12 @@ struct IPAFrameContext {
> >  		double temperatureK;
> >  	} awb;
> >
> > +	struct {
> > +		uint8_t denoise;
> > +		uint8_t sharpness;
> > +		bool updateParams;
> > +	} filter;
> > +
> >  	struct {
> >  		uint32_t exposure;
> >  		double gain;
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index 99eecd444f95..4e000d3122fb 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -968,6 +968,14 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)
> >  						   hasSelfPath_ ? &selfPath_ : nullptr);
> >
> >  	ControlInfoMap::Map ctrls;
> > +	ctrls.emplace(std::piecewise_construct,
> > +		      std::forward_as_tuple(&controls::Sharpness),
> > +		      std::forward_as_tuple(0.0f, 10.0f, 1.0f));
> > +
> > +	ctrls.emplace(std::piecewise_construct,
> > +		      std::forward_as_tuple(&controls::draft::NoiseReductionMode),
> > +		      std::forward_as_tuple(controls::draft::NoiseReductionModeValues));
> > +
> 
> Shouldn't these be registered by the IPA ?
> 
> I see below AeEnable is also registered here and it should be moved
> there as well, so I guess it will have to be done on top ?

Yes, they both should move to the IPA. I've told Florian I would handle
this on top indeed, to avoid delaying the implementation of the
algorithms. I'll get to it shortly.

> >  	ctrls.emplace(std::piecewise_construct,
> >  		      std::forward_as_tuple(&controls::AeEnable),
> >  		      std::forward_as_tuple(false, true));

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list