[libcamera-devel] [PATCH v3 3/3] ipa: rkisp1: Add support of Denoise Pre-Filter control
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Aug 15 20:43:52 CEST 2022
Hi Umang,
On Mon, Aug 15, 2022 at 09:22:12PM +0530, Umang Jain wrote:
> On 8/13/22 04:03, Laurent Pinchart via libcamera-devel wrote:
> > From: Florian Sylvestre <fsylvestre at baylibre.com>
> >
> > The denoise pre-filter algorithm is a bilateral filter which combines a range
> > filter and a domain filter. The denoise pre-filter is applied before
> > demosaicing.
> >
> > Signed-off-by: Florian Sylvestre <fsylvestre at baylibre.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > Changes since v2:
> >
> > - Change the scope of some variables in Dpf::prepare()
> > - Line-wrap coefficients in tuning file
> > ---
> > src/ipa/rkisp1/algorithms/dpf.cpp | 249 ++++++++++++++++++++++++++
> > src/ipa/rkisp1/algorithms/dpf.h | 36 ++++
> > src/ipa/rkisp1/algorithms/meson.build | 1 +
> > src/ipa/rkisp1/data/ov5640.yaml | 15 ++
> > src/ipa/rkisp1/ipa_context.cpp | 11 ++
> > src/ipa/rkisp1/ipa_context.h | 5 +
> > 6 files changed, 317 insertions(+)
> > create mode 100644 src/ipa/rkisp1/algorithms/dpf.cpp
> > create mode 100644 src/ipa/rkisp1/algorithms/dpf.h
> >
> > diff --git a/src/ipa/rkisp1/algorithms/dpf.cpp b/src/ipa/rkisp1/algorithms/dpf.cpp
> > new file mode 100644
> > index 000000000000..33fe3066b9e3
> > --- /dev/null
> > +++ b/src/ipa/rkisp1/algorithms/dpf.cpp
> > @@ -0,0 +1,249 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2021-2022, Ideas On Board
> > + *
> > + * dpf.cpp - RkISP1 Denoise Pre-Filter control
> > + */
> > +
> > +#include "dpf.h"
> > +
> > +#include <cmath>
> > +
> > +#include <libcamera/base/log.h>
> > +
> > +#include <libcamera/control_ids.h>
> > +
> > +#include "linux/rkisp1-config.h"
> > +
> > +/**
> > + * \file dpf.h
> > + */
> > +
> > +namespace libcamera {
> > +
> > +namespace ipa::rkisp1::algorithms {
> > +
> > +/**
> > + * \class Dpf
> > + * \brief RkISP1 Denoise Pre-Filter control
> > + *
> > + * The denoise pre-filter algorithm is a bilateral filter which combines a
> > + * range filter and a domain filter. The denoise pre-filter is applied before
> > + * demosaicing.
> > + */
> > +
> > +LOG_DEFINE_CATEGORY(RkISP1Dpf)
> > +
> > +Dpf::Dpf()
> > + : initialized_(false), config_({}), strengthConfig_({})
> > +{
> > +}
> > +
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::init
> > + */
> > +int Dpf::init([[maybe_unused]] IPAContext &context,
> > + const YamlObject &tuningData)
> > +{
> > + std::vector<uint16_t> values;
> > +
> > + /*
> > + * The domain kernel is configured with a 9x9 kernel for the green
> > + * pixels, and a 13x9 or 9x9 kernel for red and blue pixels.
> > + */
> > + const YamlObject &dFObject = tuningData["DomainFilter"];
> > +
> > + /*
> > + * For the green component, we have the 9x9 kernel specified
> > + * as 6 coefficients:
> > + * Y
> > + * ^
> > + * 4 | 6 5 4 5 6
> > + * 3 | 5 3 3 5
> > + * 2 | 5 3 2 3 5
> > + * 1 | 3 1 1 3
> > + * 0 - 4 2 0 2 4
> > + * -1 | 3 1 1 3
> > + * -2 | 5 3 2 3 5
> > + * -3 | 5 3 3 5
> > + * -4 | 6 5 4 5 6
> > + * +---------|--------> X
> > + * -4....-1 0 1 2 3 4
> > + */
> > + values = dFObject["g"].getList<uint16_t>().value_or(utils::defopt);
> > + if (values.size() != RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS) {
> > + LOG(RkISP1Dpf, Error)
> > + << "Invalid 'DomainFilter:g': expected "
> > + << RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS
> > + << " elements, got " << values.size();
> > + return -EINVAL;
> > + }
> > +
> > + std::copy_n(values.begin(), values.size(),
> > + std::begin(config_.g_flt.spatial_coeff));
> > +
> > + config_.g_flt.gr_enable = true;
> > + config_.g_flt.gb_enable = true;
> > +
> > + /*
> > + * For the red and blue components, we have the 13x9 kernel specified
> > + * as 6 coefficients:
> > + *
> > + * Y
> > + * ^
> > + * 4 | 6 5 4 3 4 5 6
> > + * |
> > + * 2 | 5 4 2 1 2 4 5
> > + * |
> > + * 0 - 5 3 1 0 1 3 5
> > + * |
> > + * -2 | 5 4 2 1 2 4 5
> > + * |
> > + * -4 | 6 5 4 3 4 5 6
> > + * +-------------|------------> X
> > + * -6 -4 -2 0 2 4 6
> > + *
> > + * For a 9x9 kernel, columns -6 and 6 are dropped, so coefficient
> > + * number 6 is not used.
> > + */
> > + values = dFObject["rb"].getList<uint16_t>().value_or(utils::defopt);
> > + if (values.size() != RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS &&
> > + values.size() != RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS - 1) {
> > + LOG(RkISP1Dpf, Error)
> > + << "Invalid 'DomainFilter:rb': expected "
> > + << RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS - 1
> > + << " or " << RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS
> > + << " elements, got " << values.size();
> > + return -EINVAL;
> > + }
> > +
> > + config_.rb_flt.fltsize = values.size() == RKISP1_CIF_ISP_DPF_MAX_SPATIAL_COEFFS
> > + ? RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_13x9
> > + : RKISP1_CIF_ISP_DPF_RB_FILTERSIZE_9x9;
> > +
> > + std::copy_n(values.begin(), values.size(),
> > + std::begin(config_.rb_flt.spatial_coeff));
> > +
> > + config_.rb_flt.r_enable = true;
> > + config_.rb_flt.b_enable = true;
> > +
> > + /*
> > + * The range kernel is configured with a noise level lookup table (NLL)
> > + * which stores a piecewise linear function that characterizes the
> > + * sensor noise profile as a noise level function curve (NLF).
> > + */
> > + const YamlObject &rFObject = tuningData["NoiseLevelFunction"];
> > +
> > + values = rFObject["coeff"].getList<uint16_t>().value_or(utils::defopt);
> > + if (values.size() != RKISP1_CIF_ISP_DPF_MAX_NLF_COEFFS) {
> > + LOG(RkISP1Dpf, Error)
> > + << "Invalid 'RangeFilter:coeff': expected "
> > + << RKISP1_CIF_ISP_DPF_MAX_NLF_COEFFS
> > + << " elements, got " << values.size();
> > + return -EINVAL;
> > + }
> > +
> > + std::copy_n(values.begin(), values.size(),
> > + std::begin(config_.nll.coeff));
> > +
> > + std::string scaleMode = rFObject["scale-mode"].get<std::string>("");
> > + if (scaleMode == "linear") {
> > + config_.nll.scale_mode = RKISP1_CIF_ISP_NLL_SCALE_LINEAR;
> > + } else if (scaleMode == "logarithmic") {
> > + config_.nll.scale_mode = RKISP1_CIF_ISP_NLL_SCALE_LOGARITHMIC;
> > + } else {
> > + LOG(RkISP1Dpf, Error)
> > + << "Invalid 'RangeFilter:scale-mode': expected "
> > + << "'linear' or 'logarithmic' value, got "
> > + << scaleMode;
> > + return -EINVAL;
> > + }
> > +
> > + const YamlObject &fSObject = tuningData["FilterStrength"];
> > +
> > + strengthConfig_.r = fSObject["r"].get<uint16_t>(64);
> > + strengthConfig_.g = fSObject["g"].get<uint16_t>(64);
> > + strengthConfig_.b = fSObject["b"].get<uint16_t>(64);
> > +
> > + initialized_ = true;
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::queueRequest
> > + */
> > +void Dpf::queueRequest(IPAContext &context,
> > + [[maybe_unused]] const uint32_t frame,
> > + const ControlList &controls)
> > +{
> > + auto &dpf = context.frameContext.dpf;
> > +
> > + const auto &denoise = controls.get(controls::draft::NoiseReductionMode);
> > + if (denoise) {
> > + LOG(RkISP1Dpf, Debug) << "Set denoise to " << *denoise;
> > +
> > + switch (*denoise) {
> > + case controls::draft::NoiseReductionModeOff:
> > + dpf.denoise = false;
> > + dpf.updateParams = true;
> > + break;
> > + case controls::draft::NoiseReductionModeMinimal:
> > + case controls::draft::NoiseReductionModeHighQuality:
> > + case controls::draft::NoiseReductionModeFast:
> > + dpf.denoise = true;
> > + dpf.updateParams = true;
> > + break;
> > + default:
> > + LOG(RkISP1Dpf, Error)
> > + << "Unsupported denoise value "
> > + << *denoise;
> > + }
> > + }
> > +}
> > +
> > +/**
> > + * \copydoc libcamera::ipa::Algorithm::prepare
> > + */
> > +void Dpf::prepare(IPAContext &context, rkisp1_params_cfg *params)
> > +{
> > + if (!initialized_)
> > + return;
> > +
> > + auto &dpf = context.frameContext.dpf;
> > +
> > + if (context.frameContext.frameCount == 0) {
> > + params->others.dpf_config = config_;
> > + params->others.dpf_strength_config = strengthConfig_;
> > +
> > + const auto &awb = context.configuration.awb;
> > + const auto &lsc = context.configuration.lsc;
> > + auto &mode = params->others.dpf_config.gain.mode;
> > +
> > + if (awb.enabled && lsc.enabled)
> > + mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_AWB_LSC_GAINS;
> > + else if (awb.enabled)
> > + mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_AWB_GAINS;
> > + else if (lsc.enabled)
> > + mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_LSC_GAINS;
> > + else
> > + mode = RKISP1_CIF_ISP_DPF_GAIN_USAGE_DISABLED;
> > +
>
> It might be worth to mention as a comment that usage of *_GAIN_USAGE_*
> mode will inherently supply AWB/LSC gain values to the DPF h/w block.
I'll add this:
/*
* The DPF needs to take into account the total amount of
* digital gain, which comes from the AWB and LSC modules. The
* DPF hardware can be programmed with a digital gain value
* manually, but can also use the gains supplied by the AWB and
* LSC modules automatically when they are enabled. Use that
* mode of operation as it simplifies control of the DPF.
*/
> Also a question on whether the _GAIN_USAGE_ is allowed to change between
> frames somehow? Or once it's set to AWB's or LSC's - it should continue
> to use that for the entire session.
>
> hmm, now this is looking like something at configure() time operation -
> ah but we won't have rkisp1_params_cfg* at configure().
It could change from frame to frame if we were to enable/disable the AWB
and LSC modules per frame, but I don't think we will ever do that. Note
that in this case, "enabled AWB" doesn't meant that white balance is
automatic, it means that the hardware block applies colour gains.
> > + params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_DPF |
> > + RKISP1_CIF_ISP_MODULE_DPF_STRENGTH;
> > + }
> > +
> > + if (dpf.updateParams) {
> > + params->module_en_update |= RKISP1_CIF_ISP_MODULE_DPF;
> > + if (dpf.denoise)
> > + params->module_ens |= RKISP1_CIF_ISP_MODULE_DPF;
> > +
> > + dpf.updateParams = false;
> > + }
> > +}
> > +
> > +REGISTER_IPA_ALGORITHM(Dpf, "Dpf")
> > +
> > +} /* namespace ipa::rkisp1::algorithms */
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/ipa/rkisp1/algorithms/dpf.h b/src/ipa/rkisp1/algorithms/dpf.h
> > new file mode 100644
> > index 000000000000..128ebd5e02e3
> > --- /dev/null
> > +++ b/src/ipa/rkisp1/algorithms/dpf.h
> > @@ -0,0 +1,36 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2021-2022, Ideas On Board
> > + *
> > + * dpf.h - RkISP1 Denoise Pre-Filter control
> > + */
> > +
> > +#pragma once
> > +
> > +#include <sys/types.h>
> > +
> > +#include "algorithm.h"
> > +
> > +namespace libcamera {
> > +
> > +namespace ipa::rkisp1::algorithms {
> > +
> > +class Dpf : public Algorithm
> > +{
> > +public:
> > + Dpf();
> > + ~Dpf() = default;
> > +
> > + int init(IPAContext &context, const YamlObject &tuningData) override;
> > + void queueRequest(IPAContext &context, const uint32_t frame,
> > + const ControlList &controls) override;
> > + void prepare(IPAContext &context, rkisp1_params_cfg *params) override;
> > +
> > +private:
> > + bool initialized_;
> > + struct rkisp1_cif_isp_dpf_config config_;
> > + struct rkisp1_cif_isp_dpf_strength_config strengthConfig_;
> > +};
> > +
> > +} /* namespace ipa::rkisp1::algorithms */
> > +} /* namespace libcamera */
> > diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build
> > index e48974b454b5..93a483292753 100644
> > --- a/src/ipa/rkisp1/algorithms/meson.build
> > +++ b/src/ipa/rkisp1/algorithms/meson.build
> > @@ -6,6 +6,7 @@ rkisp1_ipa_algorithms = files([
> > 'blc.cpp',
> > 'cproc.cpp',
> > 'dpcc.cpp',
> > + 'dpf.cpp',
> > 'filter.cpp',
> > 'gsl.cpp',
> > 'lsc.cpp',
> > diff --git a/src/ipa/rkisp1/data/ov5640.yaml b/src/ipa/rkisp1/data/ov5640.yaml
> > index 45d4bb77f8ca..3dc369ac3681 100644
> > --- a/src/ipa/rkisp1/data/ov5640.yaml
> > +++ b/src/ipa/rkisp1/data/ov5640.yaml
> > @@ -156,5 +156,20 @@ algorithms:
> > rnd-offsets:
> > green: 2
> > red-blue: 2
> > + - Dpf:
> > + DomainFilter:
> > + g: [ 16, 16, 16, 16, 16, 16 ]
> > + rb: [ 16, 16, 16, 16, 16, 16 ]
> > + NoiseLevelFunction:
> > + coeff: [
> > + 1023, 1023, 1023, 1023, 1023, 1023, 1023, 1023,
> > + 1023, 1023, 1023, 1023, 1023, 1023, 1023, 1023,
> > + 1023
> > + ]
>
> I am not not sure, where these Domain filter coefficients and Noise
> level function lookup table comes from... Other than that,
All those are the hardware defaults. They will likely result in a pretty
blurred image, but I think that's fine for now. The goal is to implement
manual control of the hardware block, the right values should be
computed by a tuning tool (and possibly by the IPA module for some of
them) later.
> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
>
> > + scale-mode: "linear"
> > + FilterStrength:
> > + r: 64
> > + g: 64
> > + b: 64
> > - Filter:
> > ...
> > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> > index 1a549c092d73..17f15fd4e6b5 100644
> > --- a/src/ipa/rkisp1/ipa_context.cpp
> > +++ b/src/ipa/rkisp1/ipa_context.cpp
> > @@ -164,6 +164,17 @@ namespace libcamera::ipa::rkisp1 {
> > * \brief Indicates if ISP parameters need to be updated
> > */
> >
> > +/**
> > + * \var IPAFrameContext::dpf
> > + * \brief Context for the Denoise Pre-Filter algorithm
> > + *
> > + * \var IPAFrameContext::dpf.denoise
> > + * \brief Indicates if denoise is activated
> > + *
> > + * \var IPAFrameContext::dpf.updateParams
> > + * \brief Indicates if ISP parameters need to be updated
> > + */
> > +
> > /**
> > * \var IPAFrameContext::filter
> > * \brief Context for the Filter algorithm
> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> > index 0cd6aadb83ed..3a743ac325da 100644
> > --- a/src/ipa/rkisp1/ipa_context.h
> > +++ b/src/ipa/rkisp1/ipa_context.h
> > @@ -69,6 +69,11 @@ struct IPAFrameContext {
> > bool updateParams;
> > } cproc;
> >
> > + struct {
> > + bool denoise;
> > + bool updateParams;
> > + } dpf;
> > +
> > struct {
> > uint8_t denoise;
> > uint8_t sharpness;
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list