[PATCH v9 3/3] ipa: rkisp1: algorithms: Add crosstalk algorithm
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Jun 16 19:35:16 CEST 2024
Hi Paul,
Thank you for the patch.
On Fri, Jun 14, 2024 at 08:37:57PM +0900, Paul Elder wrote:
> Add an algorithm module to the rkisp1 IPA for crosstalk correction.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Reviewed-by: Stefan Klug <stefan.klug at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> ---
> Changes in v9:
> - compilation fixes
>
> Changes in v8:
> - cleanups
>
> Changes in v7:
> - make offsets_ default to zero-matrices as opposed to identity matrices
> - checkstyle
> - populate metadata
> - add ccm to IPAFrameContext
> - don't update the ccm if the color temperature didn't change
>
> No change in v6
>
> Changes in v5:
> - clean up documentation
> - coalesce parseYaml into init
>
> Changes in v4:
> - remove stray semicolons
> - use the new matrix interpolator readYaml
> - use the new matrix operator[] getter
>
> Changes in v3:
> - read ccm offsets from tuning data, and write these offsets to the
> parameters buffer
> - make parseYaml return void, as it should fill in default data if
> unable to read, thus never failing
>
> Changes in v2:
> - rename ctk to ccm
> - reset the matrix interpolator to identity matrix if failed to read
> from tuning file
> ---
> src/ipa/rkisp1/algorithms/ccm.cpp | 142 ++++++++++++++++++++++++++
> src/ipa/rkisp1/algorithms/ccm.h | 49 +++++++++
> src/ipa/rkisp1/algorithms/meson.build | 1 +
> src/ipa/rkisp1/ipa_context.h | 5 +
> 4 files changed, 197 insertions(+)
> create mode 100644 src/ipa/rkisp1/algorithms/ccm.cpp
> create mode 100644 src/ipa/rkisp1/algorithms/ccm.h
>
> diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp
> new file mode 100644
> index 000000000000..2690bd699e41
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/ccm.cpp
> @@ -0,0 +1,142 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Ideas On Board
> + *
> + * RkISP1 Color Correction Matrix control algorithm
> + */
> +
> +#include "ccm.h"
> +
> +#include <algorithm>
> +#include <chrono>
> +#include <cmath>
> +#include <tuple>
> +#include <vector>
> +
> +#include <libcamera/base/log.h>
> +#include <libcamera/base/utils.h>
> +
> +#include <libcamera/control_ids.h>
> +
> +#include <libcamera/ipa/core_ipa_interface.h>
> +
> +#include "libcamera/internal/yaml_parser.h"
> +
> +#include "../utils.h"
> +#include "libipa/matrix_interpolator.h"
> +
> +/**
> + * \file ccm.h
> + */
> +
> +namespace libcamera {
> +
> +namespace ipa::rkisp1::algorithms {
> +
> +/**
> + * \class Ccm
> + * \brief A color correction matrix algorithm
> + */
> +
> +LOG_DEFINE_CATEGORY(RkISP1Ccm)
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::init
> + */
> +int Ccm::init([[maybe_unused]] IPAContext &context, const YamlObject &tuningData)
> +{
> + int ret = ccm_.readYaml(tuningData["ccms"], "ct", "ccm");
> + if (ret < 0) {
> + LOG(RkISP1Ccm, Warning)
> + << "Failed to parse 'ccm' "
> + << "parameter from tuning file; falling back to unit matrix";
> + ccm_.reset();
> + }
> +
> + ret = offsets_.readYaml(tuningData["ccms"], "ct", "offsets");
> + if (ret < 0) {
> + LOG(RkISP1Ccm, Warning)
> + << "Failed to parse 'offsets' "
> + << "parameter from tuning file; falling back to zero offsets";
> + /*
> + * MatrixInterpolator::reset() resets to identity matrices
> + * while here we need zero matrices so we need to construct it
> + * ourselves.
> + */
> + Matrix<int16_t, 3, 1> m({ 0, 0, 0 });
> + std::map<unsigned int, Matrix<int16_t, 3, 1>> matrices = { { 0, m } };
> + offsets_ = MatrixInterpolator<int16_t, 3, 1>(matrices);
> + }
> +
> + return 0;
> +}
> +
> +void Ccm::setParameters(rkisp1_params_cfg *params,
> + const Matrix<float, 3, 3> &matrix,
> + const Matrix<int16_t, 3, 1> &offsets)
> +{
> + struct rkisp1_cif_isp_ctk_config &config = params->others.ctk_config;
> +
> + /*
> + * 4 bit integer and 7 bit fractional, ranging from -8 (0x400) to
> + * +7.992 (0x3ff)
> + */
> + for (unsigned int i = 0; i < 3; i++) {
> + for (unsigned int j = 0; j < 3; j++)
> + config.coeff[i][j] =
> + utils::floatingToFixedPoint<4, 7, uint16_t, double>(matrix[i][j]);
> + }
> +
> + for (unsigned int i = 0; i < 3; i++)
> + config.ct_offset[i] = offsets[i][0] & 0xfff;
> +
> + LOG(RkISP1Ccm, Debug) << "Setting matrix " << matrix;
> + LOG(RkISP1Ccm, Debug) << "Setting offsets " << offsets;
> +
> + params->module_en_update |= RKISP1_CIF_ISP_MODULE_CTK;
> + params->module_ens |= RKISP1_CIF_ISP_MODULE_CTK;
> + params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_CTK;
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::prepare
> + */
> +void Ccm::prepare(IPAContext &context, const uint32_t frame,
> + IPAFrameContext &frameContext,
> + rkisp1_params_cfg *params)
> +{
> + uint32_t ct = context.activeState.awb.temperatureK;
> + if (frame > 0 && ct == ct_)
> + return;
Repeating a comment from v7: I expect temperatureK to be fairly noisy,
should we add a threshold to the comparison to avoid updating the CCM
parameters on every frame in practice ?
It can be done on top if we add a \todo comment here:
uint32_t ct = context.activeState.awb.temperatureK;
/*
* \todo The colour temperature will likely be noisy, add filtering to
* avoid updating the CCM matrix all the time.
*/
if (frame > 0 && ct == ct_)
return;
With that,
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> +
> + ct_ = ct;
> + Matrix<float, 3, 3> ccm = ccm_.get(ct);
> + Matrix<int16_t, 3, 1> offsets = offsets_.get(ct);
> +
> + frameContext.ccm.ccm = ccm;
> +
> + setParameters(params, ccm, offsets);
> +}
> +
> +/**
> + * \copydoc libcamera::ipa::Algorithm::process
> + */
> +void Ccm::process([[maybe_unused]] IPAContext &context,
> + [[maybe_unused]] const uint32_t frame,
> + IPAFrameContext &frameContext,
> + [[maybe_unused]] const rkisp1_stat_buffer *stats,
> + ControlList &metadata)
> +{
> + float m[9];
> + for (unsigned int i = 0; i < 3; i++) {
> + for (unsigned int j = 0; j < 3; j++)
> + m[i] = frameContext.ccm.ccm[i][j];
> + }
> + metadata.set(controls::ColourCorrectionMatrix, m);
> +}
> +
> +REGISTER_IPA_ALGORITHM(Ccm, "Ccm")
> +
> +} /* namespace ipa::rkisp1::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/algorithms/ccm.h b/src/ipa/rkisp1/algorithms/ccm.h
> new file mode 100644
> index 000000000000..30cb882180cc
> --- /dev/null
> +++ b/src/ipa/rkisp1/algorithms/ccm.h
> @@ -0,0 +1,49 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Ideas On Board
> + *
> + * RkISP1 Color Correction Matrix control algorithm
> + */
> +
> +#pragma once
> +
> +#include <linux/rkisp1-config.h>
> +
> +#include "libipa/matrix.h"
> +#include "libipa/matrix_interpolator.h"
> +
> +#include "algorithm.h"
> +
> +namespace libcamera {
> +
> +namespace ipa::rkisp1::algorithms {
> +
> +class Ccm : public Algorithm
> +{
> +public:
> + Ccm() {}
> + ~Ccm() = default;
> +
> + int init(IPAContext &context, const YamlObject &tuningData) override;
> + void prepare(IPAContext &context, const uint32_t frame,
> + IPAFrameContext &frameContext,
> + rkisp1_params_cfg *params) override;
> + void process(IPAContext &context, const uint32_t frame,
> + IPAFrameContext &frameContext,
> + const rkisp1_stat_buffer *stats,
> + ControlList &metadata) override;
> +
> +private:
> + void parseYaml(const YamlObject &tuningData);
> + void setParameters(rkisp1_params_cfg *params,
> + const Matrix<float, 3, 3> &matrix,
> + const Matrix<int16_t, 3, 1> &offsets);
> +
> + unsigned int ct_;
> + MatrixInterpolator<float, 3, 3> ccm_;
> + MatrixInterpolator<int16_t, 3, 1> offsets_;
> +};
> +
> +} /* namespace ipa::rkisp1::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/rkisp1/algorithms/meson.build b/src/ipa/rkisp1/algorithms/meson.build
> index 6ee71a9b5da3..1734a6675f78 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',
> + 'ccm.cpp',
> 'cproc.cpp',
> 'dpcc.cpp',
> 'dpf.cpp',
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index 6022480d4fd2..4f01a7f8ed33 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -17,6 +17,7 @@
> #include <libcamera/geometry.h>
>
> #include <libipa/fc_queue.h>
> +#include <libipa/matrix.h>
>
> namespace libcamera {
>
> @@ -163,6 +164,10 @@ struct IPAFrameContext : public FrameContext {
> uint32_t exposure;
> double gain;
> } sensor;
> +
> + struct {
> + Matrix<float, 3, 3> ccm;
> + } ccm;
> };
>
> struct IPAContext {
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list