[RFC,v7,3/6] ipa: simple: Add LSC algorithm

27348 diff mbox series

From: Xander Pronk <xander.c.pronk@gmail.com>

The algorithm is based on the common libipa lens shading correction
implementation.  The grid values obtained from the libipa algorithm are
passed to debayering in an array to be used as an RGB texture.

Notes to the implementation:

- The overall idea is to keep things simple, to not make the LSC
  computation unnecessarily expensive.

- 16 equally spaced grid positions are used for LscAlgorithm; for no
  particular reason other than that we already use the same number for
  the whole grid sizes.

- LscAlgorithm accepts only quantised types.  UQ<2,6> is used, to be
  converted to float for debayering.

- The limit of 100 to consider a temperature change noticeable is
  arbitrary.

Co-developed-by: Rick ten Wolde <rick_libcamera@wolde.info>
Signed-off-by: Rick ten Wolde <rick_libcamera@wolde.info>
Signed-off-by: Xander Pronk <xander.c.pronk@gmail.com>
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 src/ipa/simple/algorithms/lsc.cpp     | 89 +++++++++++++++++++++++++++
 src/ipa/simple/algorithms/lsc.h       | 50 +++++++++++++++
 src/ipa/simple/algorithms/meson.build |  1 +
 src/ipa/simple/ipa_context.h          |  5 ++
 4 files changed, 145 insertions(+)
 create mode 100644 src/ipa/simple/algorithms/lsc.cpp
 create mode 100644 src/ipa/simple/algorithms/lsc.h

On 08/07/2026 21:18, Milan Zamazal wrote:
> From: Xander Pronk <xander.c.pronk@gmail.com>
> 
> The algorithm is based on the common libipa lens shading correction
> implementation.  The grid values obtained from the libipa algorithm are
> passed to debayering in an array to be used as an RGB texture.

passed to the debayer algorithm as an array and used as an RGB(a) ? texture.

> 
> Notes to the implementation:

on the

> 
> - The overall idea is to keep things simple, to not make the LSC
>    computation unnecessarily expensive.
> 
> - 16 equally spaced grid positions are used for LscAlgorithm; for no
>    particular reason other than that we already use the same number for
>    the whole grid sizes.
> 
> - LscAlgorithm accepts only quantised types.  UQ<2,6> is used, to be
>    converted to float for debayering.
> 
> - The limit of 100 to consider a temperature change noticeable is
>    arbitrary.

100 what ? 100 counts, 100 lumens - what does the 100 represent ?

> 
> Co-developed-by: Rick ten Wolde <rick_libcamera@wolde.info>
> Signed-off-by: Rick ten Wolde <rick_libcamera@wolde.info>
> Signed-off-by: Xander Pronk <xander.c.pronk@gmail.com>
> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
> ---
>   src/ipa/simple/algorithms/lsc.cpp     | 89 +++++++++++++++++++++++++++
>   src/ipa/simple/algorithms/lsc.h       | 50 +++++++++++++++
>   src/ipa/simple/algorithms/meson.build |  1 +
>   src/ipa/simple/ipa_context.h          |  5 ++
>   4 files changed, 145 insertions(+)
>   create mode 100644 src/ipa/simple/algorithms/lsc.cpp
>   create mode 100644 src/ipa/simple/algorithms/lsc.h
> 
> diff --git a/src/ipa/simple/algorithms/lsc.cpp b/src/ipa/simple/algorithms/lsc.cpp
> new file mode 100644
> index 000000000..815ffb911
> --- /dev/null
> +++ b/src/ipa/simple/algorithms/lsc.cpp
> @@ -0,0 +1,89 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Lens shading correction
> + */
> +
> +#include "lsc.h"
> +
> +#include <libcamera/base/log.h>
> +
> +namespace libcamera {
> +
> +namespace ipa::soft::algorithms {
> +
> +LOG_DEFINE_CATEGORY(IPASoftLsc)
> +
> +int Lsc::init(IPAContext &context, const ValueNode &tuningData)
> +{
> +	static constexpr unsigned int kGridSize = DebayerParams::kLscGridSize;
> +
> +	for (unsigned int i = 0; i < kGridSize; i++)
> +		gridPos_.push_back(static_cast<double>(i) / (kGridSize - 1));
> +
> +	return lscAlgo_.init(tuningData, context.ctrlMap,
> +			     { .keys = { "r", "g", "b" },
> +			       .numHCells = kGridSize,
> +			       .numVCells = kGridSize,
> +			       .sensorSize = context.sensorInfo.activeAreaSize });
> +}
> +
> +int Lsc::configure(IPAContext &context,
> +		   [[maybe_unused]] const IPAConfigInfo &configInfo)
> +{
> +	return lscAlgo_.configure(context.activeState.lsc,
> +				  context.sensorInfo.analogCrop,
> +				  gridPos_, gridPos_);
> +}
> +
> +void Lsc::prepare([[maybe_unused]] IPAContext &context,
> +		  [[maybe_unused]] const uint32_t frame,
> +		  IPAFrameContext &frameContext,
> +		  DebayerParams *params)
> +{
> +	unsigned int ct = frameContext.awb.colourTemperature;
> +
> +	params->lscEnabled = frameContext.lsc.enabled;
> +
> +	if (!frameContext.lsc.enabled ||
> +	    utils::abs_diff(ct, lastAppliedCt_) < 100)
> +		return;

The 100 should be a define with a meaningful name

> +
> +	const lsc::Components<uint8_t> &set = lscAlgo_.interpolateComponents(ct);
> +
> +	const auto &red = set.at("r");
> +	const auto &green = set.at("g");
> +	const auto &blue = set.at("b");
> +
> +	DebayerParams::LscLookupTable lut;
> +	constexpr unsigned int gridSize = DebayerParams::kLscGridSize;
> +	for (unsigned int i = 0, j = 0; i < gridSize * gridSize; i++) {
> +		lut[j++] = red[i] / 64.0;
> +		lut[j++] = green[i] / 64.0;
> +		lut[j++] = blue[i] / 64.0;
> +	}

Does the commit log make this / 64 make sense ? Anyway a comment would 
be appreciated.

> +	params->lscLut = lut;
> +
> +	lastAppliedCt_ = ct;
> +}
> +
> +void Lsc::queueRequest(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> +		       IPAFrameContext &frameContext, const ControlList &controls)
> +{
> +	lscAlgo_.queueRequest(context.activeState.lsc, frameContext.lsc,
> +			      controls);
> +}
> +
> +void Lsc::process([[maybe_unused]] IPAContext &context,
> +		  [[maybe_unused]] const uint32_t frame,
> +		  IPAFrameContext &frameContext,
> +		  [[maybe_unused]] const SwIspStats *stats,
> +		  ControlList &metadata)
> +{
> +	lscAlgo_.process(frameContext.lsc, metadata);
> +}
> +
> +REGISTER_IPA_ALGORITHM(Lsc, "Lsc")
> +
> +} /* namespace ipa::soft::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/simple/algorithms/lsc.h b/src/ipa/simple/algorithms/lsc.h
> new file mode 100644
> index 000000000..9418fac39
> --- /dev/null
> +++ b/src/ipa/simple/algorithms/lsc.h
> @@ -0,0 +1,50 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Lens shading correction
> + */
> +
> +#pragma once
> +
> +#include <libipa/interpolator.h>
> +
> +#include "libipa/fixedpoint.h"
> +#include "libipa/lsc.h"
> +
> +#include "algorithm.h"
> +#include "ipa_context.h"
> +
> +namespace libcamera {
> +
> +namespace ipa::soft::algorithms {
> +
> +class Lsc : public Algorithm
> +{
> +public:
> +	Lsc() = default;
> +	~Lsc() = default;
> +
> +	int init(IPAContext &context, const ValueNode &tuningData) override;
> +	int configure(IPAContext &context,
> +		      const IPAConfigInfo &configInfo) override;
> +	void queueRequest(IPAContext &context, [[maybe_unused]] const uint32_t frame,
> +			  IPAFrameContext &frameContext, const ControlList &controls) override;
> +	void prepare(IPAContext &context,
> +		     const uint32_t frame,
> +		     IPAFrameContext &frameContext,
> +		     DebayerParams *params) override;
> +	void process([[maybe_unused]] IPAContext &context,
> +		     [[maybe_unused]] const uint32_t frame,
> +		     IPAFrameContext &frameContext,
> +		     [[maybe_unused]] const SwIspStats *stats,
> +		     ControlList &metadata) override;
> +
> +private:
> +	LscAlgorithm<UQ<2, 6>> lscAlgo_;
> +	std::vector<double> gridPos_;
> +
> +	unsigned int lastAppliedCt_ = 0;
> +};
> +
> +} /* namespace ipa::soft::algorithms */
> +
> +} /* namespace libcamera */
> diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build
> index 73c637220..c9f6e5590 100644
> --- a/src/ipa/simple/algorithms/meson.build
> +++ b/src/ipa/simple/algorithms/meson.build
> @@ -6,4 +6,5 @@ soft_simple_ipa_algorithms = files([
>       'agc.cpp',
>       'blc.cpp',
>       'ccm.cpp',
> +    'lsc.cpp',
>   ])
> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h
> index ff312ae8f..23c2cfd0a 100644
> --- a/src/ipa/simple/ipa_context.h
> +++ b/src/ipa/simple/ipa_context.h
> @@ -19,6 +19,7 @@
>   #include <libipa/awb.h>
>   #include <libipa/ccm.h>
>   #include <libipa/fc_queue.h>
> +#include "libipa/lsc.h"
> 
>   #include "core_ipa_interface.h"
> 
> @@ -61,6 +62,8 @@ struct IPAActiveState {
>   		std::optional<float> contrast;
>   		std::optional<float> saturation;
>   	} knobs;
> +
> +	ipa::lsc::ActiveState lsc;
>   };
> 
>   struct IPAFrameContext : public FrameContext {
> @@ -75,6 +78,7 @@ struct IPAFrameContext : public FrameContext {
>   	float gamma;
>   	std::optional<float> contrast;
>   	std::optional<float> saturation;
> +	ipa::lsc::FrameContext lsc;
>   };
> 
>   struct IPAContext {
> @@ -89,6 +93,7 @@ struct IPAContext {
>   	FCQueue<IPAFrameContext> frameContexts;
>   	ControlInfoMap::Map ctrlMap;
>   	bool ccmEnabled = false;
> +	ipa::lsc::ActiveState lsc;
>   };
> 
>   } /* namespace ipa::soft */
> --
> 2.55.0
> 

Code seems fine but, I think it should be more self-describing - 
comments I mean.

---
bod

Bryan O'Donoghue <bod.linux@nxsw.ie> writes:

> On 08/07/2026 21:18, Milan Zamazal wrote:
>> From: Xander Pronk <xander.c.pronk@gmail.com>
>> The algorithm is based on the common libipa lens shading correction
>> implementation.  The grid values obtained from the libipa algorithm are
>> passed to debayering in an array to be used as an RGB texture.
>
> passed to the debayer algorithm as an array and used as an 

OK

> RGB(a) ? texture.

It's an RGB texture, no alpha correction.

>> Notes to the implementation:
>
> on the

OK

>> - The overall idea is to keep things simple, to not make the LSC
>>    computation unnecessarily expensive.
>> - 16 equally spaced grid positions are used for LscAlgorithm; for no
>>    particular reason other than that we already use the same number for
>>    the whole grid sizes.
>> - LscAlgorithm accepts only quantised types.  UQ<2,6> is used, to be
>>    converted to float for debayering.
>> - The limit of 100 to consider a temperature change noticeable is
>>    arbitrary.
>
> 100 what ? 100 counts, 100 lumens - what does the 100 represent ?

Degrees, will add.

>> Co-developed-by: Rick ten Wolde <rick_libcamera@wolde.info>
>> Signed-off-by: Rick ten Wolde <rick_libcamera@wolde.info>
>> Signed-off-by: Xander Pronk <xander.c.pronk@gmail.com>
>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
>> ---
>>   src/ipa/simple/algorithms/lsc.cpp     | 89 +++++++++++++++++++++++++++
>>   src/ipa/simple/algorithms/lsc.h       | 50 +++++++++++++++
>>   src/ipa/simple/algorithms/meson.build |  1 +
>>   src/ipa/simple/ipa_context.h          |  5 ++
>>   4 files changed, 145 insertions(+)
>>   create mode 100644 src/ipa/simple/algorithms/lsc.cpp
>>   create mode 100644 src/ipa/simple/algorithms/lsc.h
>> diff --git a/src/ipa/simple/algorithms/lsc.cpp b/src/ipa/simple/algorithms/lsc.cpp
>> new file mode 100644
>> index 000000000..815ffb911
>> --- /dev/null
>> +++ b/src/ipa/simple/algorithms/lsc.cpp
>> @@ -0,0 +1,89 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Lens shading correction
>> + */
>> +
>> +#include "lsc.h"
>> +
>> +#include <libcamera/base/log.h>
>> +
>> +namespace libcamera {
>> +
>> +namespace ipa::soft::algorithms {
>> +
>> +LOG_DEFINE_CATEGORY(IPASoftLsc)
>> +
>> +int Lsc::init(IPAContext &context, const ValueNode &tuningData)
>> +{
>> +	static constexpr unsigned int kGridSize = DebayerParams::kLscGridSize;
>> +
>> +	for (unsigned int i = 0; i < kGridSize; i++)
>> +		gridPos_.push_back(static_cast<double>(i) / (kGridSize - 1));
>> +
>> +	return lscAlgo_.init(tuningData, context.ctrlMap,
>> +			     { .keys = { "r", "g", "b" },
>> +			       .numHCells = kGridSize,
>> +			       .numVCells = kGridSize,
>> +			       .sensorSize = context.sensorInfo.activeAreaSize });
>> +}
>> +
>> +int Lsc::configure(IPAContext &context,
>> +		   [[maybe_unused]] const IPAConfigInfo &configInfo)
>> +{
>> +	return lscAlgo_.configure(context.activeState.lsc,
>> +				  context.sensorInfo.analogCrop,
>> +				  gridPos_, gridPos_);
>> +}
>> +
>> +void Lsc::prepare([[maybe_unused]] IPAContext &context,
>> +		  [[maybe_unused]] const uint32_t frame,
>> +		  IPAFrameContext &frameContext,
>> +		  DebayerParams *params)
>> +{
>> +	unsigned int ct = frameContext.awb.colourTemperature;
>> +
>> +	params->lscEnabled = frameContext.lsc.enabled;
>> +
>> +	if (!frameContext.lsc.enabled ||
>> +	    utils::abs_diff(ct, lastAppliedCt_) < 100)
>> +		return;
>
> The 100 should be a define with a meaningful name

OK

>> +
>> +	const lsc::Components<uint8_t> &set = lscAlgo_.interpolateComponents(ct);
>> +
>> +	const auto &red = set.at("r");
>> +	const auto &green = set.at("g");
>> +	const auto &blue = set.at("b");
>> +
>> +	DebayerParams::LscLookupTable lut;
>> +	constexpr unsigned int gridSize = DebayerParams::kLscGridSize;
>> +	for (unsigned int i = 0, j = 0; i < gridSize * gridSize; i++) {
>> +		lut[j++] = red[i] / 64.0;
>> +		lut[j++] = green[i] / 64.0;
>> +		lut[j++] = blue[i] / 64.0;
>> +	}
>
> Does the commit log make this / 64 make sense ? Anyway a comment would be appreciated.

I'll add a source code comment.

>> +	params->lscLut = lut;
>> +
>> +	lastAppliedCt_ = ct;
>> +}
>> +
>> +void Lsc::queueRequest(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>> +		       IPAFrameContext &frameContext, const ControlList &controls)
>> +{
>> +	lscAlgo_.queueRequest(context.activeState.lsc, frameContext.lsc,
>> +			      controls);
>> +}
>> +
>> +void Lsc::process([[maybe_unused]] IPAContext &context,
>> +		  [[maybe_unused]] const uint32_t frame,
>> +		  IPAFrameContext &frameContext,
>> +		  [[maybe_unused]] const SwIspStats *stats,
>> +		  ControlList &metadata)
>> +{
>> +	lscAlgo_.process(frameContext.lsc, metadata);
>> +}
>> +
>> +REGISTER_IPA_ALGORITHM(Lsc, "Lsc")
>> +
>> +} /* namespace ipa::soft::algorithms */
>> +
>> +} /* namespace libcamera */
>> diff --git a/src/ipa/simple/algorithms/lsc.h b/src/ipa/simple/algorithms/lsc.h
>> new file mode 100644
>> index 000000000..9418fac39
>> --- /dev/null
>> +++ b/src/ipa/simple/algorithms/lsc.h
>> @@ -0,0 +1,50 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Lens shading correction
>> + */
>> +
>> +#pragma once
>> +
>> +#include <libipa/interpolator.h>
>> +
>> +#include "libipa/fixedpoint.h"
>> +#include "libipa/lsc.h"
>> +
>> +#include "algorithm.h"
>> +#include "ipa_context.h"
>> +
>> +namespace libcamera {
>> +
>> +namespace ipa::soft::algorithms {
>> +
>> +class Lsc : public Algorithm
>> +{
>> +public:
>> +	Lsc() = default;
>> +	~Lsc() = default;
>> +
>> +	int init(IPAContext &context, const ValueNode &tuningData) override;
>> +	int configure(IPAContext &context,
>> +		      const IPAConfigInfo &configInfo) override;
>> +	void queueRequest(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>> +			  IPAFrameContext &frameContext, const ControlList &controls) override;
>> +	void prepare(IPAContext &context,
>> +		     const uint32_t frame,
>> +		     IPAFrameContext &frameContext,
>> +		     DebayerParams *params) override;
>> +	void process([[maybe_unused]] IPAContext &context,
>> +		     [[maybe_unused]] const uint32_t frame,
>> +		     IPAFrameContext &frameContext,
>> +		     [[maybe_unused]] const SwIspStats *stats,
>> +		     ControlList &metadata) override;
>> +
>> +private:
>> +	LscAlgorithm<UQ<2, 6>> lscAlgo_;
>> +	std::vector<double> gridPos_;
>> +
>> +	unsigned int lastAppliedCt_ = 0;
>> +};
>> +
>> +} /* namespace ipa::soft::algorithms */
>> +
>> +} /* namespace libcamera */
>> diff --git a/src/ipa/simple/algorithms/meson.build b/src/ipa/simple/algorithms/meson.build
>> index 73c637220..c9f6e5590 100644
>> --- a/src/ipa/simple/algorithms/meson.build
>> +++ b/src/ipa/simple/algorithms/meson.build
>> @@ -6,4 +6,5 @@ soft_simple_ipa_algorithms = files([
>>       'agc.cpp',
>>       'blc.cpp',
>>       'ccm.cpp',
>> +    'lsc.cpp',
>>   ])
>> diff --git a/src/ipa/simple/ipa_context.h b/src/ipa/simple/ipa_context.h
>> index ff312ae8f..23c2cfd0a 100644
>> --- a/src/ipa/simple/ipa_context.h
>> +++ b/src/ipa/simple/ipa_context.h
>> @@ -19,6 +19,7 @@
>>   #include <libipa/awb.h>
>>   #include <libipa/ccm.h>
>>   #include <libipa/fc_queue.h>
>> +#include "libipa/lsc.h"
>>   #include "core_ipa_interface.h"
>> @@ -61,6 +62,8 @@ struct IPAActiveState {
>>   		std::optional<float> contrast;
>>   		std::optional<float> saturation;
>>   	} knobs;
>> +
>> +	ipa::lsc::ActiveState lsc;
>>   };
>>   struct IPAFrameContext : public FrameContext {
>> @@ -75,6 +78,7 @@ struct IPAFrameContext : public FrameContext {
>>   	float gamma;
>>   	std::optional<float> contrast;
>>   	std::optional<float> saturation;
>> +	ipa::lsc::FrameContext lsc;
>>   };
>>   struct IPAContext {
>> @@ -89,6 +93,7 @@ struct IPAContext {
>>   	FCQueue<IPAFrameContext> frameContexts;
>>   	ControlInfoMap::Map ctrlMap;
>>   	bool ccmEnabled = false;
>> +	ipa::lsc::ActiveState lsc;
>>   };
>>   } /* namespace ipa::soft */
>> --
>> 2.55.0
>> 
>
> Code seems fine but, I think it should be more self-describing - comments I mean.
>
> ---
> bod