[libcamera-devel] [PATCH 2/5] ipa: rkisp1: Add support of Gamma Sensor Linearization control
paul.elder at ideasonboard.com
paul.elder at ideasonboard.com
Tue Jul 12 09:18:01 CEST 2022
Hi Florian,
On Wed, Jun 22, 2022 at 05:19:15PM +0200, Florian Sylvestre via libcamera-devel wrote:
> The GammaSensorLinearization algorithm improves the image dynamic using
> a coefficient table in the YAML tuning file.
>
> Signed-off-by: Florian Sylvestre <fsylvestre at baylibre.com>
> ---
> src/ipa/rkisp1/algorithms/gsl.cpp | 142 ++++++++++++++++++++++++++
> src/ipa/rkisp1/algorithms/gsl.h | 38 +++++++
> src/ipa/rkisp1/algorithms/meson.build | 1 +
> src/ipa/rkisp1/data/ov5640.yaml | 6 ++
> src/ipa/rkisp1/rkisp1.cpp | 1 +
> 5 files changed, 188 insertions(+)
> create mode 100644 src/ipa/rkisp1/algorithms/gsl.cpp
> create mode 100644 src/ipa/rkisp1/algorithms/gsl.h
>
> diff --git a/src/ipa/rkisp1/algorithms/gsl.cpp b/src/ipa/rkisp1/algorithms/gsl.cpp
> new file mode 100644
> index 00000000..7ff3d360
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/gsl.cpp
> @@ -0,0 +1,142 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021-2022, Ideas On Board
> + *
> + * gsl.cpp - RkISP1 Gamma Sensor Linearization control
> + */
> +
> +#include "gsl.h"
> +
> +#include <libcamera/base/log.h>
> +
> +#include "libcamera/internal/yaml_parser.h"
> +#include "linux/rkisp1-config.h"
> +
> +/**
> + * \file gsl.h
> + */
> +
> +namespace libcamera {
> +
> +namespace ipa::rkisp1::algorithms {
> +
> +/**
> + * \class GammaSensorLinearization
> + * \brief RkISP1 Gamma Sensor Linearization control
> + *
> + * This algorithm improves the image dynamic using a coefficient table in the
> + * Yaml tuning file, the table contains:
> + * - 'x-intervals': These values corresponds to GAMMA_DX_n values (from
> + * GAMMA_DX_0 to GAMMA_DX_16 fields in RKISP1_CIF_ISP_GAMMA_DX_LO and
s/DX_0/DX_1/ ?
> + * RKISP1_CIF_ISP_GAMMA_DX_HI rkisp1 registers).
> + * Note: rkisp1 use each value to compute a X interval equal to 2^(value+4)
s/use/uses/
s/a X/an X/
> + * - 'y' values for each component (red, green , blue) on 12 bits resolution.
> + */
> +
> +LOG_DEFINE_CATEGORY(RkISP1Gsl)
> +
> +#define DEGAMMA_X_INTERVALS 16
> +
> +GammaSensorLinearization::GammaSensorLinearization()
> + : tuningParameters_(false)
I would call this tuned_ or something along this lines, as this variable
signifies if the tuning parameters have been loaded correctly or not,
and does not actually contain the tuning parameters as the varaible name
suggests.
Also, what happens if no tuning file is provided? Does the algorithm
just... not work (as in it doesn't do anything)? Is that better than
running on some default tuning values? (I feel like I saw discussion on
this topic elsewhere but I'm just asking it again :S)
> +{
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::init
> + */
> +int GammaSensorLinearization::init([[maybe_unused]] IPAContext &context,
> + const YamlObject &tuningData)
> +{
> + std::vector<uint16_t> xIntervals_ = tuningData["x-intervals"].getList<uint16_t>();
> + if (xIntervals_.size() != DEGAMMA_X_INTERVALS) {
> + LOG(RkISP1Gsl, Error)
> + << "Issue while parsing 'x'"
> + << "in tuning file (list size:"
> + << xIntervals_.size() << "/"
> + << DEGAMMA_X_INTERVALS << ")";
I would prefer slightly more verbosity, like printing which/where the
tuning file is, and saying explicitly "expected". Also I think you're
missing spaces in the string concatenation.
Same for the other error messages below.
> + return -EINVAL;
> + }
> +
> + /* Compute gammaDx_ intervals from xIntervals_ values */
> + gammaDx_[0] = 0;
> + gammaDx_[1] = 0;
> + for (int i = 0; i < DEGAMMA_X_INTERVALS; ++i) {
> + gammaDx_[i / 8] |= (xIntervals_[i] & 0x07) << ((i % 8) * 4);
> + }
I don't think you need the braces.
Neat math trick :)
Paul
> +
> + const YamlObject &yObject = tuningData["y"];
> +
> + if (!yObject.isDictionary()) {
> + LOG(RkISP1Gsl, Error)
> + << "Issue while parsing 'y' in tuning file: "
> + << "entry must be a dictionary";
> + return -EINVAL;
> + }
> +
> + curveYr_ = yObject["red"].getList<uint16_t>();
> + if (curveYr_.size() != RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE) {
> + LOG(RkISP1Gsl, Error)
> + << "Issue while parsing 'y:red'"
> + << "in tuning file (list size: "
> + << curveYr_.size() << "/"
> + << RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE << ")";
> + return -EINVAL;
> + }
> +
> + curveYg_ = yObject["green"].getList<uint16_t>();
> + if (curveYg_.size() != RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE) {
> + LOG(RkISP1Gsl, Error)
> + << "Issue while parsing 'y:green'"
> + << "in tuning file (list size: "
> + << curveYg_.size() << "/"
> + << RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE << ")";
> + return -EINVAL;
> + }
> +
> + curveYb_ = yObject["blue"].getList<uint16_t>();
> + if (curveYb_.size() != RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE) {
> + LOG(RkISP1Gsl, Error)
> + << "Issue while parsing 'y:blue'"
> + << "in tuning file (list size: "
> + << curveYb_.size() << "/"
> + << RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE << ")";
> + return -EINVAL;
> + }
> +
> + tuningParameters_ = true;
> + return 0;
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::prepare
> + */
> +void GammaSensorLinearization::prepare(IPAContext &context,
> + rkisp1_params_cfg *params)
> +{
> + if (context.frameContext.frameCount > 0)
> + return;
> +
> + if (!tuningParameters_)
> + return;
> +
> + params->others.sdg_config.xa_pnts.gamma_dx0 = gammaDx_[0];
> + params->others.sdg_config.xa_pnts.gamma_dx1 = gammaDx_[1];
> +
> + std::copy(curveYr_.begin(), curveYr_.end(),
> + params->others.sdg_config.curve_r.gamma_y);
> + std::copy(curveYg_.begin(), curveYg_.end(),
> + params->others.sdg_config.curve_g.gamma_y);
> + std::copy(curveYb_.begin(), curveYb_.end(),
> + params->others.sdg_config.curve_b.gamma_y);
> +
> + params->module_en_update |= RKISP1_CIF_ISP_MODULE_SDG;
> + params->module_ens |= RKISP1_CIF_ISP_MODULE_SDG;
> + params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_SDG;
> +}
> +
> +REGISTER_IPA_ALGORITHM(GammaSensorLinearization)
> +
> +} /* namespace ipa::rkisp1::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/algorithms/gsl.h b/src/ipa/rkisp1/algorithms/gsl.h
> new file mode 100644
> index 00000000..df16a89b
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/gsl.h
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021-2022, Ideas On Board
> + *
> + * gsl.h - RkISP1 Gamma Sensor Linearization control
> + */
> +
> +#pragma once
> +
> +#include <linux/rkisp1-config.h>
> +
> +#include "algorithm.h"
> +
> +namespace libcamera {
> +
> +struct IPACameraSensorInfo;
> +
> +namespace ipa::rkisp1::algorithms {
> +
> +class GammaSensorLinearization : public Algorithm
> +{
> +public:
> + GammaSensorLinearization();
> + ~GammaSensorLinearization() = default;
> +
> + int init(IPAContext &context, const YamlObject &tuningData) override;
> + void prepare(IPAContext &context, rkisp1_params_cfg *params) override;
> +
> +private:
> + bool tuningParameters_;
> + uint32_t gammaDx_[2];
> + std::vector<uint16_t> curveYr_;
> + std::vector<uint16_t> curveYg_;
> + std::vector<uint16_t> curveYb_;
> +};
> +
> +} /* namespace ipa::rkisp1::algorithms */
> +} /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build
> index 7ec53d89..0597c353 100644
> --- a/src/ipa/rkisp1/algorithms/meson.build
> +++ b/src/ipa/rkisp1/algorithms/meson.build
> @@ -4,4 +4,5 @@ rkisp1_ipa_algorithms = files([
> 'agc.cpp',
> 'awb.cpp',
> 'blc.cpp',
> + 'gsl.cpp',
> ])
> diff --git a/src/ipa/rkisp1/data/ov5640.yaml b/src/ipa/rkisp1/data/ov5640.yaml
> index 232d8ae8..6cb84ed9 100644
> --- a/src/ipa/rkisp1/data/ov5640.yaml
> +++ b/src/ipa/rkisp1/data/ov5640.yaml
> @@ -10,4 +10,10 @@ algorithms:
> Gr: 256
> Gb: 256
> B: 256
> + - GammaSensorLinearization:
> + x-intervals: [ 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2 ]
> + y:
> + red: [ 0, 256, 512, 768, 1024, 1280, 1536, 1792, 2048, 2304, 2560, 2816, 3072, 3328, 3584, 3840, 4096 ]
> + green: [ 0, 256, 512, 768, 1024, 1280, 1536, 1792, 2048, 2304, 2560, 2816, 3072, 3328, 3584, 3840, 4096 ]
> + blue: [ 0, 256, 512, 768, 1024, 1280, 1536, 1792, 2048, 2304, 2560, 2816, 3072, 3328, 3584, 3840, 4096 ]
> ...
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index d052954e..622a0d61 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/gsl.h"
> #include "libipa/camera_sensor_helper.h"
>
> #include "ipa_context.h"
> --
> 2.34.1
>
More information about the libcamera-devel
mailing list