[libcamera-devel] [PATCH v2 5/5] ipa: rkisp1: Add support of Defect Pixel Cluster Correction control
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Jul 24 23:55:43 CEST 2022
Hi Florian,
Thank you for the patch.
On Fri, Jul 22, 2022 at 05:16:35PM +0200, Florian Sylvestre via libcamera-devel wrote:
> The Defect Pixel Cluster Correction algorithm is responsible to minimize
> the impact of defective pixels. The on-the-fly method is actually used,
> based on coefficient provided by the tuning file.
>
> Signed-off-by: Florian Sylvestre <fsylvestre at baylibre.com>
> ---
> src/ipa/rkisp1/algorithms/dpcc.cpp | 262 ++++++++++++++++++++++++++
> src/ipa/rkisp1/algorithms/dpcc.h | 31 +++
> src/ipa/rkisp1/algorithms/meson.build | 1 +
> src/ipa/rkisp1/data/ov5640.yaml | 60 ++++++
> src/ipa/rkisp1/rkisp1.cpp | 1 +
> 5 files changed, 355 insertions(+)
> create mode 100644 src/ipa/rkisp1/algorithms/dpcc.cpp
> create mode 100644 src/ipa/rkisp1/algorithms/dpcc.h
>
> diff --git a/src/ipa/rkisp1/algorithms/dpcc.cpp b/src/ipa/rkisp1/algorithms/dpcc.cpp
> new file mode 100644
> index 00000000..c1598b6c
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/dpcc.cpp
> @@ -0,0 +1,262 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021-2022, Ideas On Board
> + *
> + * dpcc.cpp - RkISP1 Defect Pixel Cluster Correction control
> + */
> +
> +#include "dpcc.h"
> +
> +#include <libcamera/base/log.h>
> +
> +#include "libcamera/internal/yaml_parser.h"
> +
> +#include "linux/rkisp1-config.h"
> +
> +/**
> + * \file dpcc.h
> + */
> +
> +namespace libcamera {
> +
> +namespace ipa::rkisp1::algorithms {
> +
> +/**
> + * \class DefectPixelClusterCorrection
> + * \brief RkISP1 Defect Pixel Cluster Correction control
> + *
> + * Depending of the sensor quality, some pixels can be defective and then
> + * appear significantly brighter or darker than the other pixels.
> + *
> + * The Defect Pixel Cluster Correction algorithms is responsible to minimize
> + * the impact of the pixels.
> + * This can be done with algorithms applied at run time (on-the-fly method) or
> + * with a table of defective pixels. Only first method is supported for the
s/first/the first/
> + * moment.
Nitpicking, if you intended this to be two separate paragraphs, they
should be separated by a blank line. Otherwise there should be no line
break after the first sentence.
> + */
> +
> +LOG_DEFINE_CATEGORY(RkISP1Dpcc)
> +
> +DefectPixelClusterCorrection::DefectPixelClusterCorrection()
> + : initialized_(false), config_({})
> +{
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::init
> + */
> +int DefectPixelClusterCorrection::init([[maybe_unused]] IPAContext &context,
> + const YamlObject &tuningData)
> +{
> + config_.mode =
> + RKISP1_CIF_ISP_DPCC_MODE_STAGE1_ENABLE;
> + config_.output_mode =
> + RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_STAGE1_INCL_G_CENTER |
> + RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_STAGE1_INCL_RB_CENTER;
You can avoid the line breaks:
config_.mode = RKISP1_CIF_ISP_DPCC_MODE_STAGE1_ENABLE;
config_.output_mode = RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_STAGE1_INCL_G_CENTER
| RKISP1_CIF_ISP_DPCC_OUTPUT_MODE_STAGE1_INCL_RB_CENTER;
> +
> + config_.set_use = tuningData["fixed-set"].get<bool>(false)
> + ? RKISP1_CIF_ISP_DPCC_SET_USE_STAGE1_USE_FIX_SET
> + : 0;
I haven't found a way yet to get clang-format to accept our usual style:
config_.set_use = tuningData["fixed-set"].get<bool>(false)
? RKISP1_CIF_ISP_DPCC_SET_USE_STAGE1_USE_FIX_SET : 0;
> +
> + /* Get all defined sets to apply (up to 3). */
> + const YamlObject &setsObject = tuningData["sets"];
> + if (!setsObject.isList()) {
> + LOG(RkISP1Dpcc, Error)
> + << "'sets' parameter not found in tuning file";
> + return -EINVAL;
> + }
> +
> + if (setsObject.size() > RKISP1_CIF_ISP_DPCC_METHODS_MAX) {
> + LOG(RkISP1Dpcc, Error)
> + << "'sets' size in tuning file (" << setsObject.size()
> + << ") exceeds the maximum hardware capacity (3)";
> + return -EINVAL;
> + }
> +
> + for (std::size_t i = 0; i < setsObject.size(); ++i) {
> + struct rkisp1_cif_isp_dpcc_methods_config &method = config_.methods[i];
> + const YamlObject &set = setsObject[i];
> + uint16_t value;
> +
> + /* Enable set if described in Yaml tuning file. */
s/Yaml/YAML/
> + config_.set_use |= 1 << i;
> +
> + /* PG Method */
> + const YamlObject &pgObject = set["pg-factor"];
> +
> + if (pgObject.contains("green")) {
> + method.method |=
> + RKISP1_CIF_ISP_DPCC_METHODS_SET_PG_GREEN_ENABLE;
> +
> + value = pgObject["green"].get<uint16_t>(0);
> + method.pg_fac |=
> + RKISP1_CIF_ISP_DPCC_PG_FAC_G(value);
Same here and where applicable below for line breaks.
> + }
> +
> + if (pgObject.contains("red-blue")) {
> + method.method |=
> + RKISP1_CIF_ISP_DPCC_METHODS_SET_PG_RED_BLUE_ENABLE;
> +
> + value = pgObject["red-blue"].get<uint16_t>(0);
> + method.pg_fac |=
> + RKISP1_CIF_ISP_DPCC_PG_FAC_RB(value);
> + }
> +
> + /* RO Method */
> + const YamlObject &roObject = set["ro-limits"];
> +
> + if (roObject.contains("green")) {
> + method.method |=
> + RKISP1_CIF_ISP_DPCC_METHODS_SET_RO_GREEN_ENABLE;
> +
> + value = roObject["green"].get<uint16_t>(0);
> + config_.ro_limits |=
> + RKISP1_CIF_ISP_DPCC_RO_LIMITS_n_G(i, value);
> + }
> +
> + if (roObject.contains("red-blue")) {
> + method.method |=
> + RKISP1_CIF_ISP_DPCC_METHODS_SET_RO_RED_BLUE_ENABLE;
> +
> + value = roObject["red-blue"].get<uint16_t>(0);
> + config_.ro_limits |=
> + RKISP1_CIF_ISP_DPCC_RO_LIMITS_n_RB(i, value);
> + }
> +
> + /* RG Method */
> + const YamlObject &rgObject = set["rg-factor"];
> + method.rg_fac = 0;
> +
> + if (rgObject.contains("green")) {
> + method.method |=
> + RKISP1_CIF_ISP_DPCC_METHODS_SET_RG_GREEN_ENABLE;
> +
> + value = rgObject["green"].get<uint16_t>(0);
> + method.rg_fac |=
> + RKISP1_CIF_ISP_DPCC_RG_FAC_G(value);
> + }
> +
> + if (rgObject.contains("red-blue")) {
> + method.method |=
> + RKISP1_CIF_ISP_DPCC_METHODS_SET_RG_RED_BLUE_ENABLE;
> +
> + value = rgObject["red-blue"].get<uint16_t>(0);
> + method.rg_fac |=
> + RKISP1_CIF_ISP_DPCC_RG_FAC_RB(value);
> + }
> +
> + /* RND Method */
> + const YamlObject &rndOffsetsObject = set["rnd-offsets"];
> +
> + if (rndOffsetsObject.contains("green")) {
> + method.method |=
> + RKISP1_CIF_ISP_DPCC_METHODS_SET_RND_GREEN_ENABLE;
> +
> + value = rndOffsetsObject["green"].get<uint16_t>(0);
> + config_.rnd_offs |=
> + RKISP1_CIF_ISP_DPCC_RND_OFFS_n_G(i, value);
> + }
> +
> + if (rndOffsetsObject.contains("red-blue")) {
> + method.method |=
> + RKISP1_CIF_ISP_DPCC_METHODS_SET_RND_RED_BLUE_ENABLE;
> +
> + value = rndOffsetsObject["red-blue"].get<uint16_t>(0);
> + config_.rnd_offs |=
> + RKISP1_CIF_ISP_DPCC_RND_OFFS_n_RB(i, value);
> + }
> +
> + const YamlObject &rndThresholdObject = set["rnd-threshold"];
> + method.rnd_thresh = 0;
> +
> + if (rndThresholdObject.contains("green")) {
> + method.method |=
> + RKISP1_CIF_ISP_DPCC_METHODS_SET_RND_GREEN_ENABLE;
> +
> + value = rndThresholdObject["green"].get<uint16_t>(0);
> + method.rnd_thresh |=
> + RKISP1_CIF_ISP_DPCC_RND_THRESH_G(value);
> + }
> +
> + if (rndThresholdObject.contains("red-blue")) {
> + method.method |=
> + RKISP1_CIF_ISP_DPCC_METHODS_SET_RND_RED_BLUE_ENABLE;
> +
> + value = rndThresholdObject["red-blue"].get<uint16_t>(0);
> + method.rnd_thresh |=
> + RKISP1_CIF_ISP_DPCC_RND_THRESH_RB(value);
> + }
> +
> + /* LC Method */
> + const YamlObject &lcThresholdObject = set["line-threshold"];
> + method.line_thresh = 0;
> +
> + if (lcThresholdObject.contains("green")) {
> + method.method |=
> + RKISP1_CIF_ISP_DPCC_METHODS_SET_LC_GREEN_ENABLE;
> +
> + value = lcThresholdObject["green"].get<uint16_t>(0);
> + method.line_thresh |=
> + RKISP1_CIF_ISP_DPCC_LINE_THRESH_G(value);
> + }
> +
> + if (lcThresholdObject.contains("red-blue")) {
> + method.method |=
> + RKISP1_CIF_ISP_DPCC_METHODS_SET_LC_RED_BLUE_ENABLE;
> +
> + value = lcThresholdObject["red-blue"].get<uint16_t>(0);
> + method.line_thresh |=
> + RKISP1_CIF_ISP_DPCC_LINE_THRESH_RB(value);
> + }
> +
> + const YamlObject &lcTMadFactorObject = set["line-mad-factor"];
> + method.line_mad_fac = 0;
> +
> + if (lcTMadFactorObject.contains("green")) {
> + method.method |=
> + RKISP1_CIF_ISP_DPCC_METHODS_SET_LC_GREEN_ENABLE;
> +
> + value = lcTMadFactorObject["green"].get<uint16_t>(0);
> + method.line_mad_fac |=
> + RKISP1_CIF_ISP_DPCC_LINE_MAD_FAC_G(value);
> + }
> +
> + if (lcTMadFactorObject.contains("red-blue")) {
> + method.method |=
> + RKISP1_CIF_ISP_DPCC_METHODS_SET_LC_RED_BLUE_ENABLE;
> +
> + value = lcTMadFactorObject["red-blue"].get<uint16_t>(0);
> + method.line_mad_fac |=
> + RKISP1_CIF_ISP_DPCC_LINE_MAD_FAC_RB(value);
> + }
> + }
> +
> + initialized_ = true;
> +
> + return 0;
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::prepare
> + */
> +void DefectPixelClusterCorrection::prepare(IPAContext &context,
> + rkisp1_params_cfg *params)
> +{
> + if (context.frameContext.frameCount > 0)
> + return;
> +
> + if (!initialized_)
> + return;
> +
> + params->others.dpcc_config = config_;
> +
> + params->module_en_update |= RKISP1_CIF_ISP_MODULE_DPCC;
> + params->module_ens |= RKISP1_CIF_ISP_MODULE_DPCC;
> + params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_DPCC;
> +}
> +
> +REGISTER_IPA_ALGORITHM(DefectPixelClusterCorrection, "DefectPixelClusterCorrection")
> +
> +} /* namespace ipa::rkisp1::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/algorithms/dpcc.h b/src/ipa/rkisp1/algorithms/dpcc.h
> new file mode 100644
> index 00000000..ce332c3a
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/dpcc.h
> @@ -0,0 +1,31 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021-2022, Ideas On Board
> + *
> + * dpcc.h - RkISP1 Defect Pixel Cluster Correction control
s/ / /
> + */
> +
> +#pragma once
> +
> +#include "algorithm.h"
> +
> +namespace libcamera {
> +
> +namespace ipa::rkisp1::algorithms {
> +
> +class DefectPixelClusterCorrection : public Algorithm
> +{
> +public:
> + DefectPixelClusterCorrection();
> + ~DefectPixelClusterCorrection() = default;
> +
> + int init(IPAContext &context, const YamlObject &tuningData) override;
> + void prepare(IPAContext &context, rkisp1_params_cfg *params) override;
> +
> +private:
> + bool initialized_;
> + rkisp1_cif_isp_dpcc_config config_;
> +};
> +
> +} /* namespace ipa::rkisp1::algorithms */
> +} /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build
> index 64e11dce..87007493 100644
> --- a/src/ipa/rkisp1/algorithms/meson.build
> +++ b/src/ipa/rkisp1/algorithms/meson.build
> @@ -4,6 +4,7 @@ rkisp1_ipa_algorithms = files([
> 'agc.cpp',
> 'awb.cpp',
> 'blc.cpp',
> + 'dpcc.cpp',
> 'gsl.cpp',
> 'lsc.cpp',
> ])
> diff --git a/src/ipa/rkisp1/data/ov5640.yaml b/src/ipa/rkisp1/data/ov5640.yaml
> index fa2ae436..2315ec43 100644
> --- a/src/ipa/rkisp1/data/ov5640.yaml
> +++ b/src/ipa/rkisp1/data/ov5640.yaml
> @@ -95,4 +95,64 @@ algorithms:
> 1024, 1024, 1024, 1024, 1024, 1024, 1024, 1024, 1024, 1024, 1024, 1024, 1024, 1024, 1024, 1024, 1024,
> 1024, 1024, 1024, 1024, 1024, 1024, 1024, 1024, 1024, 1024, 1024, 1024, 1024, 1024, 1024, 1024, 1024,
> ]
> + - DefectPixelClusterCorrection:
> + fixed-set: false
> + sets:
> + # PG, LC, RO, RND, RG
> + - line-threshold:
> + green: 8
> + red-blue: 8
> + line-mad-factor:
> + green: 4
> + red-blue: 4
> + pg-factor:
> + green: 8
> + red-blue: 8
> + rnd-threshold:
> + green: 10
> + red-blue: 10
> + rg-factor:
> + green: 32
> + red-blue: 32
> + ro-limits:
> + green: 1
> + red-blue: 1
> + rnd-offsets:
> + green: 2
> + red-blue: 2
> + # PG, LC, RO
> + - line-threshold:
> + green: 24
> + red-blue: 32
> + line-mad-factor:
> + green: 16
> + red-blue: 24
> + pg-factor:
> + green: 6
> + red-blue: 8
> + ro-limits:
> + green: 2
> + red-blue: 2
> + # PG, LC, RO, RND, RG
> + - line-threshold:
> + green: 32
> + red-blue: 32
> + line-mad-factor:
> + green: 4
> + red-blue: 4
> + pg-factor:
> + green: 10
> + red-blue: 10
> + rnd-threshold:
> + green: 6
> + red-blue: 8
> + rg-factor:
> + green: 4
> + red-blue: 4
> + ro-limits:
> + green: 1
> + red-blue: 2
> + rnd-offsets:
> + green: 2
> + red-blue: 2
> ...
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index af0f43f2..caa67cf0 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -31,6 +31,7 @@
> #include "algorithms/algorithm.h"
> #include "algorithms/awb.h"
> #include "algorithms/blc.h"
> +#include "algorithms/dpcc.h"
This can be dropped.
> #include "algorithms/gsl.h"
> #include "algorithms/lsc.h"
> #include "libipa/camera_sensor_helper.h"
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list