[libcamera-devel] [PATCH v1 5/6] ipa: rkisp1: Introduce crosstalk correction

[Jean-Michel Hautbois]

Hi Laurent,

On 03/12/2021 03:40, Laurent Pinchart wrote:
> Hi Jean-Michel,
> 
> On Thu, Dec 02, 2021 at 07:04:09PM +0100, Jean-Michel Hautbois wrote:
>> Introduce the color correction matrix for the RkISP1 based on a simple
>> assumptions on the gains until we can tune the sensor properly.
>>
>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
>> ---
>>   src/ipa/rkisp1/algorithms/ctk.cpp     | 59 +++++++++++++++++++++++++++
>>   src/ipa/rkisp1/algorithms/ctk.h       | 30 ++++++++++++++
>>   src/ipa/rkisp1/algorithms/meson.build |  1 +
>>   src/ipa/rkisp1/rkisp1.cpp             |  2 +
>>   4 files changed, 92 insertions(+)
>>   create mode 100644 src/ipa/rkisp1/algorithms/ctk.cpp
>>   create mode 100644 src/ipa/rkisp1/algorithms/ctk.h
>>
>> diff --git a/src/ipa/rkisp1/algorithms/ctk.cpp b/src/ipa/rkisp1/algorithms/ctk.cpp
>> new file mode 100644
>> index 000000000..81600e776
>> --- /dev/null
>> +++ b/src/ipa/rkisp1/algorithms/ctk.cpp
>> @@ -0,0 +1,59 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2021, Ideas On Board
>> + *
>> + * ctk.cpp - RkISP1 Cross-Talk Correction control
>> + */
>> +
>> +#include "ctk.h"
>> +
>> +/**
>> + * \file ctk.h
>> + */
>> +
>> +namespace libcamera {
>> +
>> +namespace ipa::rkisp1::algorithms {
>> +
>> +/**
>> + * \class CrossTalkCorrection
>> + * \brief RkISP1 Cross-Talk Correction control (color correction matrix)
>> + *
>> + * The crosstalk in image sensor is an effect when signal from a specific pixel
>> + * is affected by its adjacent pixels. An image sensor can be considered as a
>> + * linear system, with linear crosstalk existing only between horizontal and
>> + * vertical neighboring pixels. This matrix should be calculated based on tuning
>> + * but a first approximation can be obtained by using the grey-world gains and
>> + * applying them to their respective channel.
>> + */
>> +
>> +/**
>> + * \copydoc libcamera::ipa::Algorithm::prepare
>> + */
>> +void CrossTalkCorrection::prepare([[maybe_unused]] IPAContext &context,
>> +				  rkisp1_params_cfg *params)
>> +{
>> +	params->others.ctk_config.coeff[0][0] = 128 * context.frameContext.awb.gains.red;
>> +	params->others.ctk_config.coeff[0][1] = 0;
>> +	params->others.ctk_config.coeff[0][2] = 0;
>> +
>> +	params->others.ctk_config.coeff[1][0] = 0;
>> +	params->others.ctk_config.coeff[1][1] = 128 * context.frameContext.awb.gains.green;
>> +	params->others.ctk_config.coeff[1][2] = 0;
>> +
>> +	params->others.ctk_config.coeff[2][0] = 0;
>> +	params->others.ctk_config.coeff[2][1] = 0;
>> +	params->others.ctk_config.coeff[2][2] = 128 * context.frameContext.awb.gains.blue;
> 
> I don't get it, won't this essentially apply the AWB gains a second time
> ?
> 

That's what I thought, but if you look closely to the CCM matrices in 
imx219 json data file, you will notice that the numbers are more than 1 
on the diagonals. For example, the CCM matrix for a color temperature of 
5716K is :
{
	 1.80439, -0.73699, -0.06739,
	-0.36073,  1.83327, -0.47255,
	-0.08378, -0.56403,  1.64781
}

The sums on the rows should be 1 (this is not the case here) to preserve 
grey. So, we could imagine having -64*gain for the other values in the 
rows. Until I can repair my SD connector I won't be able to go test it 
though... :-(

>> +
>> +	params->others.ctk_config.ct_offset[0] = 0;
>> +	params->others.ctk_config.ct_offset[1] = 0;
>> +	params->others.ctk_config.ct_offset[2] = 0;
>> +
>> +	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;
>> +}
>> +
>> +} /* namespace ipa::rkisp1::algorithms */
>> +
>> +} /* namespace libcamera */
>> diff --git a/src/ipa/rkisp1/algorithms/ctk.h b/src/ipa/rkisp1/algorithms/ctk.h
>> new file mode 100644
>> index 000000000..c4d240e2d
>> --- /dev/null
>> +++ b/src/ipa/rkisp1/algorithms/ctk.h
>> @@ -0,0 +1,30 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2021, Ideas On Board
>> + *
>> + * blc.h - RkISP1 Cross-Talk Correction control
>> + */
>> +
>> +#pragma once
>> +
>> +#include <linux/rkisp1-config.h>
>> +
>> +#include "algorithm.h"
>> +
>> +namespace libcamera {
>> +
>> +struct IPACameraSensorInfo;
>> +
>> +namespace ipa::rkisp1::algorithms {
>> +
>> +class CrossTalkCorrection : public Algorithm
>> +{
>> +public:
>> +	CrossTalkCorrection() = default;
>> +	~CrossTalkCorrection() = default;
>> +
>> +	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 faec0eb3e..482e88474 100644
>> --- a/src/ipa/rkisp1/algorithms/meson.build
>> +++ b/src/ipa/rkisp1/algorithms/meson.build
>> @@ -4,5 +4,6 @@ rkisp1_ipa_algorithms = files([
>>       'agc.cpp',
>>       'awb.cpp',
>>       'blc.cpp',
>> +    'ctk.cpp',
>>       'sdg.cpp',
>>   ])
>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
>> index 979f28420..21a9d15b9 100644
>> --- a/src/ipa/rkisp1/rkisp1.cpp
>> +++ b/src/ipa/rkisp1/rkisp1.cpp
>> @@ -29,6 +29,7 @@
>>   #include "algorithms/algorithm.h"
>>   #include "algorithms/awb.h"
>>   #include "algorithms/blc.h"
>> +#include "algorithms/ctk.h"
>>   #include "algorithms/sdg.h"
>>   #include "libipa/camera_sensor_helper.h"
>>   
>> @@ -130,6 +131,7 @@ int IPARkISP1::init(const IPASettings &settings, unsigned int hwRevision)
>>   	algorithms_.push_back(std::make_unique<algorithms::Agc>());
>>   	algorithms_.push_back(std::make_unique<algorithms::Awb>());
>>   	algorithms_.push_back(std::make_unique<algorithms::BlackLevelCorrection>());
>> +	algorithms_.push_back(std::make_unique<algorithms::CrossTalkCorrection>());
>>   	algorithms_.push_back(std::make_unique<algorithms::SensorDeGamma>());
>>   
>>   	return 0;
>