[v1,07/12] ipa: rksip1: lsc: Move function definitions out of class
diff mbox series

Message ID 20251014075252.2876485-8-stefan.klug@ideasonboard.com
State New
Headers show
Series
  • Add resampling support for polynomial LSC data
Related show

Commit Message

Stefan Klug Oct. 14, 2025, 7:52 a.m. UTC 
Move the function definitions out of the related classes. This was noted
in review after the series was already merged. After trying it out I
must admit that it really improves readability and reduces the
indentation level by one.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>

---

Note: This patch contains no functional changes. `git diff
--ignore-space-change` helps in verifying that.
---
 src/ipa/rkisp1/algorithms/lsc.cpp | 314 ++++++++++++++++--------------
 1 file changed, 163 insertions(+), 151 deletions(-)

Comments

Barnabás Pőcze Oct. 24, 2025, 4:12 p.m. UTC | #1 
2025. 10. 14. 9:52 keltezéssel, Stefan Klug írta:
> Move the function definitions out of the related classes. This was noted
> in review after the series was already merged. After trying it out I
> must admit that it really improves readability and reduces the
> indentation level by one.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> 
> ---
> 
> Note: This patch contains no functional changes. `git diff
> --ignore-space-change` helps in verifying that.
> ---

Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>


>   src/ipa/rkisp1/algorithms/lsc.cpp | 314 ++++++++++++++++--------------
>   1 file changed, 163 insertions(+), 151 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
> index e46943f4bae0..0c1bb470c346 100644
> --- a/src/ipa/rkisp1/algorithms/lsc.cpp
> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp
> @@ -85,184 +85,196 @@ public:
>   	}
>   
>   	int parseLscData(const YamlObject &yamlSets,
> -			 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> -	{
> -		const auto &sets = yamlSets.asList();
> -		for (const auto &yamlSet : sets) {
> -			std::optional<LscPolynomial> pr, pgr, pgb, pb;
> -			uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> -
> -			if (lscData.count(ct)) {
> -				LOG(RkISP1Lsc, Error)
> -					<< "Multiple sets found for "
> -					<< "color temperature " << ct;
> -				return -EINVAL;
> -			}
> -
> -			LensShadingCorrection::Components &set = lscData[ct];
> -			pr = yamlSet["r"].get<LscPolynomial>();
> -			pgr = yamlSet["gr"].get<LscPolynomial>();
> -			pgb = yamlSet["gb"].get<LscPolynomial>();
> -			pb = yamlSet["b"].get<LscPolynomial>();
> -
> -			if (!(pr || pgr || pgb || pb)) {
> -				LOG(RkISP1Lsc, Error)
> -					<< "Failed to parse polynomial for "
> -					<< "colour temperature " << ct;
> -				return -EINVAL;
> -			}
> -
> -			pr->setReferenceImageSize(sensorSize_);
> -			pgr->setReferenceImageSize(sensorSize_);
> -			pgb->setReferenceImageSize(sensorSize_);
> -			pb->setReferenceImageSize(sensorSize_);
> -			set.r = samplePolynomial(*pr);
> -			set.gr = samplePolynomial(*pgr);
> -			set.gb = samplePolynomial(*pgb);
> -			set.b = samplePolynomial(*pb);
> +			 std::map<unsigned int, LensShadingCorrection::Components> &lscData);
> +
> +private:
> +	std::vector<double> sizesListToPositions(const std::vector<double> &sizes);
> +	std::vector<uint16_t> samplePolynomial(const LscPolynomial &poly);
> +
> +	Size sensorSize_;
> +	Rectangle cropRectangle_;
> +	const std::vector<double> &xSizes_;
> +	const std::vector<double> &ySizes_;
> +};
> +
> +int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,
> +				      std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> +{
> +	const auto &sets = yamlSets.asList();
> +	for (const auto &yamlSet : sets) {
> +		std::optional<LscPolynomial> pr, pgr, pgb, pb;
> +		uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> +
> +		if (lscData.count(ct)) {
> +			LOG(RkISP1Lsc, Error)
> +				<< "Multiple sets found for "
> +				<< "color temperature " << ct;
> +			return -EINVAL;
>   		}
>   
> -		if (lscData.empty()) {
> -			LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> +		LensShadingCorrection::Components &set = lscData[ct];
> +		pr = yamlSet["r"].get<LscPolynomial>();
> +		pgr = yamlSet["gr"].get<LscPolynomial>();
> +		pgb = yamlSet["gb"].get<LscPolynomial>();
> +		pb = yamlSet["b"].get<LscPolynomial>();
> +
> +		if (!(pr || pgr || pgb || pb)) {
> +			LOG(RkISP1Lsc, Error)
> +				<< "Failed to parse polynomial for "
> +				<< "colour temperature " << ct;
>   			return -EINVAL;
>   		}
>   
> -		return 0;
> +		pr->setReferenceImageSize(sensorSize_);
> +		pgr->setReferenceImageSize(sensorSize_);
> +		pgb->setReferenceImageSize(sensorSize_);
> +		pb->setReferenceImageSize(sensorSize_);
> +		set.r = samplePolynomial(*pr);
> +		set.gr = samplePolynomial(*pgr);
> +		set.gb = samplePolynomial(*pgb);
> +		set.b = samplePolynomial(*pb);
>   	}
>   
> -private:
> -	/*
> -	 * The rkisp1 LSC grid spacing is defined by the cell sizes on the first
> -	 * half of the grid. This is then mirrored in hardware to the other
> -	 * half. See parseSizes() for further details. For easier handling, this
> -	 * function converts the cell sizes of half the grid to a list of
> -	 * position of the whole grid (on one axis). Example:
> -	 *
> -	 * input:   | 0.2 | 0.3 |
> -	 * output: 0.0   0.2   0.5   0.8   1.0
> -	 */
> -	std::vector<double> sizesListToPositions(const std::vector<double> &sizes)
> -	{
> -		const int half = sizes.size();
> -		std::vector<double> positions(half * 2 + 1);
> -		double x = 0.0;
> -
> -		positions[half] = 0.5;
> -		for (int i = 1; i <= half; i++) {
> -			x += sizes[half - i];
> -			positions[half - i] = 0.5 - x;
> -			positions[half + i] = 0.5 + x;
> -		}
> +	if (lscData.empty()) {
> +		LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> +		return -EINVAL;
> +	}
>   
> -		return positions;
> +	return 0;
> +}
> +
> +/*
> + * The rkisp1 LSC grid spacing is defined by the cell sizes on the first half of
> + * the grid. This is then mirrored in hardware to the other half. See
> + * parseSizes() for further details. For easier handling, this function converts
> + * the cell sizes of half the grid to a list of position of the whole grid (on
> + * one axis). Example:
> + *
> + * input:   | 0.2 | 0.3 |
> + * output: 0.0   0.2   0.5   0.8   1.0
> + */
> +std::vector<double> LscPolynomialLoader::sizesListToPositions(const std::vector<double> &sizes)
> +{
> +	const int half = sizes.size();
> +	std::vector<double> positions(half * 2 + 1);
> +	double x = 0.0;
> +
> +	positions[half] = 0.5;
> +	for (int i = 1; i <= half; i++) {
> +		x += sizes[half - i];
> +		positions[half - i] = 0.5 - x;
> +		positions[half + i] = 0.5 + x;
>   	}
>   
> -	std::vector<uint16_t> samplePolynomial(const LscPolynomial &poly)
> -	{
> -		constexpr int k = RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> -
> -		double m = poly.getM();
> -		double x0 = cropRectangle_.x / m;
> -		double y0 = cropRectangle_.y / m;
> -		double w = cropRectangle_.width / m;
> -		double h = cropRectangle_.height / m;
> -		std::vector<uint16_t> samples;
> -
> -		ASSERT(xSizes_.size() * 2 + 1 == k);
> -		ASSERT(ySizes_.size() * 2 + 1 == k);
> -
> -		samples.reserve(k * k);
> -
> -		std::vector<double> xPos(sizesListToPositions(xSizes_));
> -		std::vector<double> yPos(sizesListToPositions(ySizes_));
> -
> -		for (int y = 0; y < k; y++) {
> -			for (int x = 0; x < k; x++) {
> -				double xp = x0 + xPos[x] * w;
> -				double yp = y0 + yPos[y] * h;
> -				/*
> -				 * The hardware uses 2.10 fixed point format and
> -				 * limits the legal values to [1..3.999]. Scale
> -				 * and clamp the sampled value accordingly.
> -				 */
> -				int v = static_cast<int>(
> -					poly.sampleAtNormalizedPixelPos(xp, yp) *
> -					1024);
> -				v = std::min(std::max(v, 1024), 4095);
> -				samples.push_back(v);
> -			}
> +	return positions;
> +}
> +
> +std::vector<uint16_t> LscPolynomialLoader::samplePolynomial(const LscPolynomial &poly)
> +{
> +	constexpr int k = RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> +
> +	double m = poly.getM();
> +	double x0 = cropRectangle_.x / m;
> +	double y0 = cropRectangle_.y / m;
> +	double w = cropRectangle_.width / m;
> +	double h = cropRectangle_.height / m;
> +	std::vector<uint16_t> samples;
> +
> +	ASSERT(xSizes_.size() * 2 + 1 == k);
> +	ASSERT(ySizes_.size() * 2 + 1 == k);
> +
> +	samples.reserve(k * k);
> +
> +	std::vector<double> xPos(sizesListToPositions(xSizes_));
> +	std::vector<double> yPos(sizesListToPositions(ySizes_));
> +
> +	for (int y = 0; y < k; y++) {
> +		for (int x = 0; x < k; x++) {
> +			double xp = x0 + xPos[x] * w;
> +			double yp = y0 + yPos[y] * h;
> +			/*
> +			 * The hardware uses 2.10 fixed point format and limits
> +			 * the legal values to [1..3.999]. Scale and clamp the
> +			 * sampled value accordingly.
> +			 */
> +			int v = static_cast<int>(
> +				poly.sampleAtNormalizedPixelPos(xp, yp) *
> +				1024);
> +			v = std::min(std::max(v, 1024), 4095);
> +			samples.push_back(v);
>   		}
> -		return samples;
>   	}
> -
> -	Size sensorSize_;
> -	Rectangle cropRectangle_;
> -	const std::vector<double> &xSizes_;
> -	const std::vector<double> &ySizes_;
> -};
> +	return samples;
> +}
>   
>   class LscTableLoader
>   {
>   public:
>   	int parseLscData(const YamlObject &yamlSets,
> -			 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> -	{
> -		const auto &sets = yamlSets.asList();
> -
> -		for (const auto &yamlSet : sets) {
> -			uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> -
> -			if (lscData.count(ct)) {
> -				LOG(RkISP1Lsc, Error)
> -					<< "Multiple sets found for color temperature "
> -					<< ct;
> -				return -EINVAL;
> -			}
> -
> -			LensShadingCorrection::Components &set = lscData[ct];
> -
> -			set.r = parseTable(yamlSet, "r");
> -			set.gr = parseTable(yamlSet, "gr");
> -			set.gb = parseTable(yamlSet, "gb");
> -			set.b = parseTable(yamlSet, "b");
> -
> -			if (set.r.empty() || set.gr.empty() ||
> -			    set.gb.empty() || set.b.empty()) {
> -				LOG(RkISP1Lsc, Error)
> -					<< "Set for color temperature " << ct
> -					<< " is missing tables";
> -				return -EINVAL;
> -			}
> -		}
> +			 std::map<unsigned int, LensShadingCorrection::Components> &lscData);
> +
> +private:
> +	std::vector<uint16_t> parseTable(const YamlObject &tuningData,
> +					 const char *prop);
> +};
> +
> +int LscTableLoader::parseLscData(const YamlObject &yamlSets,
> +				 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> +{
> +	const auto &sets = yamlSets.asList();
> +
> +	for (const auto &yamlSet : sets) {
> +		uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
>   
> -		if (lscData.empty()) {
> -			LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> +		if (lscData.count(ct)) {
> +			LOG(RkISP1Lsc, Error)
> +				<< "Multiple sets found for color temperature "
> +				<< ct;
>   			return -EINVAL;
>   		}
>   
> -		return 0;
> -	}
> +		LensShadingCorrection::Components &set = lscData[ct];
>   
> -private:
> -	std::vector<uint16_t> parseTable(const YamlObject &tuningData,
> -					 const char *prop)
> -	{
> -		static constexpr unsigned int kLscNumSamples =
> -			RKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> +		set.r = parseTable(yamlSet, "r");
> +		set.gr = parseTable(yamlSet, "gr");
> +		set.gb = parseTable(yamlSet, "gb");
> +		set.b = parseTable(yamlSet, "b");
>   
> -		std::vector<uint16_t> table =
> -			tuningData[prop].getList<uint16_t>().value_or(std::vector<uint16_t>{});
> -		if (table.size() != kLscNumSamples) {
> +		if (set.r.empty() || set.gr.empty() ||
> +		    set.gb.empty() || set.b.empty()) {
>   			LOG(RkISP1Lsc, Error)
> -				<< "Invalid '" << prop << "' values: expected "
> -				<< kLscNumSamples
> -				<< " elements, got " << table.size();
> -			return {};
> +				<< "Set for color temperature " << ct
> +				<< " is missing tables";
> +			return -EINVAL;
>   		}
> +	}
>   
> -		return table;
> +	if (lscData.empty()) {
> +		LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> +		return -EINVAL;
>   	}
> -};
> +
> +	return 0;
> +}
> +
> +std::vector<uint16_t> LscTableLoader::parseTable(const YamlObject &tuningData,
> +						 const char *prop)
> +{
> +	static constexpr unsigned int kLscNumSamples =
> +		RKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> +
> +	std::vector<uint16_t> table =
> +		tuningData[prop].getList<uint16_t>().value_or(std::vector<uint16_t>{});
> +	if (table.size() != kLscNumSamples) {
> +		LOG(RkISP1Lsc, Error)
> +			<< "Invalid '" << prop << "' values: expected "
> +			<< kLscNumSamples
> +			<< " elements, got " << table.size();
> +		return {};
> +	}
> +
> +	return table;
> +}
>   
>   static std::vector<double> parseSizes(const YamlObject &tuningData,
>   				      const char *prop)
Rui Wang Oct. 26, 2025, 1:28 a.m. UTC | #2 
On 2025-10-14 03:52, Stefan Klug wrote:
> Move the function definitions out of the related classes. This was noted
> in review after the series was already merged. After trying it out I
> must admit that it really improves readability and reduces the
> indentation level by one.
>
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
>
> ---
>
> Note: This patch contains no functional changes. `git diff
> --ignore-space-change` helps in verifying that.
> ---
>   src/ipa/rkisp1/algorithms/lsc.cpp | 314 ++++++++++++++++--------------
>   1 file changed, 163 insertions(+), 151 deletions(-)
>
> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
> index e46943f4bae0..0c1bb470c346 100644
> --- a/src/ipa/rkisp1/algorithms/lsc.cpp
> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp
> @@ -85,184 +85,196 @@ public:
>   	}
>   
>   	int parseLscData(const YamlObject &yamlSets,
> -			 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> -	{
> -		const auto &sets = yamlSets.asList();
> -		for (const auto &yamlSet : sets) {
> -			std::optional<LscPolynomial> pr, pgr, pgb, pb;
> -			uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> -
> -			if (lscData.count(ct)) {
> -				LOG(RkISP1Lsc, Error)
> -					<< "Multiple sets found for "
> -					<< "color temperature " << ct;
> -				return -EINVAL;
> -			}
> -
> -			LensShadingCorrection::Components &set = lscData[ct];
> -			pr = yamlSet["r"].get<LscPolynomial>();
> -			pgr = yamlSet["gr"].get<LscPolynomial>();
> -			pgb = yamlSet["gb"].get<LscPolynomial>();
> -			pb = yamlSet["b"].get<LscPolynomial>();
> -
> -			if (!(pr || pgr || pgb || pb)) {
> -				LOG(RkISP1Lsc, Error)
> -					<< "Failed to parse polynomial for "
> -					<< "colour temperature " << ct;
> -				return -EINVAL;
> -			}
> -
> -			pr->setReferenceImageSize(sensorSize_);
> -			pgr->setReferenceImageSize(sensorSize_);
> -			pgb->setReferenceImageSize(sensorSize_);
> -			pb->setReferenceImageSize(sensorSize_);
> -			set.r = samplePolynomial(*pr);
> -			set.gr = samplePolynomial(*pgr);
> -			set.gb = samplePolynomial(*pgb);
> -			set.b = samplePolynomial(*pb);
> +			 std::map<unsigned int, LensShadingCorrection::Components> &lscData);
> +
> +private:
> +	std::vector<double> sizesListToPositions(const std::vector<double> &sizes);
> +	std::vector<uint16_t> samplePolynomial(const LscPolynomial &poly);
> +
> +	Size sensorSize_;
> +	Rectangle cropRectangle_;
> +	const std::vector<double> &xSizes_;
> +	const std::vector<double> &ySizes_;
> +};
> +
> +int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,
> +				      std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> +{
> +	const auto &sets = yamlSets.asList();
> +	for (const auto &yamlSet : sets) {
> +		std::optional<LscPolynomial> pr, pgr, pgb, pb;
> +		uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> +
> +		if (lscData.count(ct)) {
> +			LOG(RkISP1Lsc, Error)
> +				<< "Multiple sets found for "
> +				<< "color temperature " << ct;
> +			return -EINVAL;
>   		}
>   
> -		if (lscData.empty()) {
> -			LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> +		LensShadingCorrection::Components &set = lscData[ct];
> +		pr = yamlSet["r"].get<LscPolynomial>();
> +		pgr = yamlSet["gr"].get<LscPolynomial>();
> +		pgb = yamlSet["gb"].get<LscPolynomial>();
> +		pb = yamlSet["b"].get<LscPolynomial>();
> +
> +		if (!(pr || pgr || pgb || pb)) {
> +			LOG(RkISP1Lsc, Error)
> +				<< "Failed to parse polynomial for "
> +				<< "colour temperature " << ct;
>   			return -EINVAL;
>   		}
>   
> -		return 0;
> +		pr->setReferenceImageSize(sensorSize_);
> +		pgr->setReferenceImageSize(sensorSize_);
> +		pgb->setReferenceImageSize(sensorSize_);
> +		pb->setReferenceImageSize(sensorSize_);
> +		set.r = samplePolynomial(*pr);
> +		set.gr = samplePolynomial(*pgr);
> +		set.gb = samplePolynomial(*pgb);
> +		set.b = samplePolynomial(*pb);
>   	}
>   
> -private:
> -	/*
> -	 * The rkisp1 LSC grid spacing is defined by the cell sizes on the first
> -	 * half of the grid. This is then mirrored in hardware to the other
> -	 * half. See parseSizes() for further details. For easier handling, this
> -	 * function converts the cell sizes of half the grid to a list of
> -	 * position of the whole grid (on one axis). Example:
> -	 *
> -	 * input:   | 0.2 | 0.3 |
> -	 * output: 0.0   0.2   0.5   0.8   1.0
> -	 */
> -	std::vector<double> sizesListToPositions(const std::vector<double> &sizes)
> -	{
> -		const int half = sizes.size();
> -		std::vector<double> positions(half * 2 + 1);
> -		double x = 0.0;
> -
> -		positions[half] = 0.5;
> -		for (int i = 1; i <= half; i++) {
> -			x += sizes[half - i];
> -			positions[half - i] = 0.5 - x;
> -			positions[half + i] = 0.5 + x;
> -		}
> +	if (lscData.empty()) {
> +		LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> +		return -EINVAL;
> +	}
>   
> -		return positions;
> +	return 0;
> +}
> +
> +/*
> + * The rkisp1 LSC grid spacing is defined by the cell sizes on the first half of
> + * the grid. This is then mirrored in hardware to the other half. See
> + * parseSizes() for further details. For easier handling, this function converts
> + * the cell sizes of half the grid to a list of position of the whole grid (on
> + * one axis). Example:
> + *
> + * input:   | 0.2 | 0.3 |
> + * output: 0.0   0.2   0.5   0.8   1.0
> + */
> +std::vector<double> LscPolynomialLoader::sizesListToPositions(const std::vector<double> &sizes)
> +{
> +	const int half = sizes.size();
> +	std::vector<double> positions(half * 2 + 1);
> +	double x = 0.0;
> +
> +	positions[half] = 0.5;
> +	for (int i = 1; i <= half; i++) {
> +		x += sizes[half - i];
> +		positions[half - i] = 0.5 - x;
> +		positions[half + i] = 0.5 + x;
>   	}
>   
> -	std::vector<uint16_t> samplePolynomial(const LscPolynomial &poly)
> -	{
> -		constexpr int k = RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> -
> -		double m = poly.getM();
> -		double x0 = cropRectangle_.x / m;
> -		double y0 = cropRectangle_.y / m;
> -		double w = cropRectangle_.width / m;
> -		double h = cropRectangle_.height / m;
> -		std::vector<uint16_t> samples;
> -
> -		ASSERT(xSizes_.size() * 2 + 1 == k);
> -		ASSERT(ySizes_.size() * 2 + 1 == k);
> -
> -		samples.reserve(k * k);
> -
> -		std::vector<double> xPos(sizesListToPositions(xSizes_));
> -		std::vector<double> yPos(sizesListToPositions(ySizes_));
> -
> -		for (int y = 0; y < k; y++) {
> -			for (int x = 0; x < k; x++) {
> -				double xp = x0 + xPos[x] * w;
> -				double yp = y0 + yPos[y] * h;
> -				/*
> -				 * The hardware uses 2.10 fixed point format and
> -				 * limits the legal values to [1..3.999]. Scale
> -				 * and clamp the sampled value accordingly.
> -				 */
> -				int v = static_cast<int>(
> -					poly.sampleAtNormalizedPixelPos(xp, yp) *
> -					1024);
> -				v = std::min(std::max(v, 1024), 4095);
> -				samples.push_back(v);
> -			}
> +	return positions;
> +}
> +
> +std::vector<uint16_t> LscPolynomialLoader::samplePolynomial(const LscPolynomial &poly)
> +{
> +	constexpr int k = RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> +
> +	double m = poly.getM();
> +	double x0 = cropRectangle_.x / m;
> +	double y0 = cropRectangle_.y / m;
> +	double w = cropRectangle_.width / m;
> +	double h = cropRectangle_.height / m;
> +	std::vector<uint16_t> samples;
> +
> +	ASSERT(xSizes_.size() * 2 + 1 == k);
> +	ASSERT(ySizes_.size() * 2 + 1 == k);
> +
> +	samples.reserve(k * k);
> +
> +	std::vector<double> xPos(sizesListToPositions(xSizes_));
> +	std::vector<double> yPos(sizesListToPositions(ySizes_));
> +
> +	for (int y = 0; y < k; y++) {
> +		for (int x = 0; x < k; x++) {
> +			double xp = x0 + xPos[x] * w;
> +			double yp = y0 + yPos[y] * h;
> +			/*
> +			 * The hardware uses 2.10 fixed point format and limits
> +			 * the legal values to [1..3.999]. Scale and clamp the
> +			 * sampled value accordingly.
> +			 */
> +			int v = static_cast<int>(
> +				poly.sampleAtNormalizedPixelPos(xp, yp) *
> +				1024);
> +			v = std::min(std::max(v, 1024), 4095);
> +			samples.push_back(v);
>   		}
> -		return samples;
>   	}
> -
> -	Size sensorSize_;
> -	Rectangle cropRectangle_;
> -	const std::vector<double> &xSizes_;
> -	const std::vector<double> &ySizes_;
> -};
> +	return samples;
> +}
>   
>   class LscTableLoader
>   {
>   public:
>   	int parseLscData(const YamlObject &yamlSets,
> -			 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> -	{
> -		const auto &sets = yamlSets.asList();
> -
> -		for (const auto &yamlSet : sets) {
> -			uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> -
> -			if (lscData.count(ct)) {
> -				LOG(RkISP1Lsc, Error)
> -					<< "Multiple sets found for color temperature "
> -					<< ct;
> -				return -EINVAL;
> -			}
> -
> -			LensShadingCorrection::Components &set = lscData[ct];
> -
> -			set.r = parseTable(yamlSet, "r");
> -			set.gr = parseTable(yamlSet, "gr");
> -			set.gb = parseTable(yamlSet, "gb");
> -			set.b = parseTable(yamlSet, "b");
> -
> -			if (set.r.empty() || set.gr.empty() ||
> -			    set.gb.empty() || set.b.empty()) {
> -				LOG(RkISP1Lsc, Error)
> -					<< "Set for color temperature " << ct
> -					<< " is missing tables";
> -				return -EINVAL;
> -			}
> -		}
> +			 std::map<unsigned int, LensShadingCorrection::Components> &lscData);
> +
> +private:
> +	std::vector<uint16_t> parseTable(const YamlObject &tuningData,
> +					 const char *prop);
> +};
> +
> +int LscTableLoader::parseLscData(const YamlObject &yamlSets,
> +				 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> +{
> +	const auto &sets = yamlSets.asList();
> +
> +	for (const auto &yamlSet : sets) {
> +		uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
>   
> -		if (lscData.empty()) {
> -			LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> +		if (lscData.count(ct)) {
> +			LOG(RkISP1Lsc, Error)
> +				<< "Multiple sets found for color temperature "
> +				<< ct;
>   			return -EINVAL;
>   		}
>   
> -		return 0;
> -	}
> +		LensShadingCorrection::Components &set = lscData[ct];
>   
> -private:
> -	std::vector<uint16_t> parseTable(const YamlObject &tuningData,
> -					 const char *prop)
> -	{
> -		static constexpr unsigned int kLscNumSamples =
> -			RKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> +		set.r = parseTable(yamlSet, "r");
> +		set.gr = parseTable(yamlSet, "gr");
> +		set.gb = parseTable(yamlSet, "gb");
> +		set.b = parseTable(yamlSet, "b");
>   
> -		std::vector<uint16_t> table =
> -			tuningData[prop].getList<uint16_t>().value_or(std::vector<uint16_t>{});
> -		if (table.size() != kLscNumSamples) {
> +		if (set.r.empty() || set.gr.empty() ||
> +		    set.gb.empty() || set.b.empty()) {
>   			LOG(RkISP1Lsc, Error)
> -				<< "Invalid '" << prop << "' values: expected "
> -				<< kLscNumSamples
> -				<< " elements, got " << table.size();
> -			return {};
> +				<< "Set for color temperature " << ct
> +				<< " is missing tables";
> +			return -EINVAL;
>   		}
> +	}
>   
> -		return table;
> +	if (lscData.empty()) {
> +		LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> +		return -EINVAL;
>   	}
> -};
> +
> +	return 0;
> +}
> +
> +std::vector<uint16_t> LscTableLoader::parseTable(const YamlObject &tuningData,
> +						 const char *prop)
> +{
> +	static constexpr unsigned int kLscNumSamples =
> +		RKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> +
> +	std::vector<uint16_t> table =
> +		tuningData[prop].getList<uint16_t>().value_or(std::vector<uint16_t>{});
> +	if (table.size() != kLscNumSamples) {
> +		LOG(RkISP1Lsc, Error)
> +			<< "Invalid '" << prop << "' values: expected "
> +			<< kLscNumSamples
> +			<< " elements, got " << table.size();
> +		return {};
> +	}
> +
> +	return table;
> +}
>   
>   static std::vector<double> parseSizes(const YamlObject &tuningData,
>   				      const char *prop)
Reviewed by Rui Wang
Rui Wang Oct. 26, 2025, 2:23 a.m. UTC | #3 
On 2025-10-14 03:52, Stefan Klug wrote:
> Move the function definitions out of the related classes. This was noted
> in review after the series was already merged. After trying it out I
> must admit that it really improves readability and reduces the
> indentation level by one.
>
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
>
> ---
>
> Note: This patch contains no functional changes. `git diff
> --ignore-space-change` helps in verifying that.
> ---
>   src/ipa/rkisp1/algorithms/lsc.cpp | 314 ++++++++++++++++--------------
>   1 file changed, 163 insertions(+), 151 deletions(-)
>
> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
> index e46943f4bae0..0c1bb470c346 100644
> --- a/src/ipa/rkisp1/algorithms/lsc.cpp
> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp
> @@ -85,184 +85,196 @@ public:
>   	}
>   
>   	int parseLscData(const YamlObject &yamlSets,
> -			 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> -	{
> -		const auto &sets = yamlSets.asList();
> -		for (const auto &yamlSet : sets) {
> -			std::optional<LscPolynomial> pr, pgr, pgb, pb;
> -			uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> -
> -			if (lscData.count(ct)) {
> -				LOG(RkISP1Lsc, Error)
> -					<< "Multiple sets found for "
> -					<< "color temperature " << ct;
> -				return -EINVAL;
> -			}
> -
> -			LensShadingCorrection::Components &set = lscData[ct];
> -			pr = yamlSet["r"].get<LscPolynomial>();
> -			pgr = yamlSet["gr"].get<LscPolynomial>();
> -			pgb = yamlSet["gb"].get<LscPolynomial>();
> -			pb = yamlSet["b"].get<LscPolynomial>();
> -
> -			if (!(pr || pgr || pgb || pb)) {
> -				LOG(RkISP1Lsc, Error)
> -					<< "Failed to parse polynomial for "
> -					<< "colour temperature " << ct;
> -				return -EINVAL;
> -			}
> -
> -			pr->setReferenceImageSize(sensorSize_);
> -			pgr->setReferenceImageSize(sensorSize_);
> -			pgb->setReferenceImageSize(sensorSize_);
> -			pb->setReferenceImageSize(sensorSize_);
> -			set.r = samplePolynomial(*pr);
> -			set.gr = samplePolynomial(*pgr);
> -			set.gb = samplePolynomial(*pgb);
> -			set.b = samplePolynomial(*pb);
> +			 std::map<unsigned int, LensShadingCorrection::Components> &lscData);
> +
> +private:
> +	std::vector<double> sizesListToPositions(const std::vector<double> &sizes);
> +	std::vector<uint16_t> samplePolynomial(const LscPolynomial &poly);
> +
> +	Size sensorSize_;
> +	Rectangle cropRectangle_;
> +	const std::vector<double> &xSizes_;
> +	const std::vector<double> &ySizes_;
> +};
> +
> +int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,
> +				      std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> +{
> +	const auto &sets = yamlSets.asList();
> +	for (const auto &yamlSet : sets) {
> +		std::optional<LscPolynomial> pr, pgr, pgb, pb;
> +		uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> +
> +		if (lscData.count(ct)) {
> +			LOG(RkISP1Lsc, Error)
> +				<< "Multiple sets found for "
> +				<< "color temperature " << ct;
> +			return -EINVAL;
>   		}
>   
> -		if (lscData.empty()) {
> -			LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> +		LensShadingCorrection::Components &set = lscData[ct];
> +		pr = yamlSet["r"].get<LscPolynomial>();
> +		pgr = yamlSet["gr"].get<LscPolynomial>();
> +		pgb = yamlSet["gb"].get<LscPolynomial>();
> +		pb = yamlSet["b"].get<LscPolynomial>();
> +
> +		if (!(pr || pgr || pgb || pb)) {
> +			LOG(RkISP1Lsc, Error)
> +				<< "Failed to parse polynomial for "
> +				<< "colour temperature " << ct;
>   			return -EINVAL;
>   		}
>   
> -		return 0;
> +		pr->setReferenceImageSize(sensorSize_);
> +		pgr->setReferenceImageSize(sensorSize_);
> +		pgb->setReferenceImageSize(sensorSize_);
> +		pb->setReferenceImageSize(sensorSize_);
> +		set.r = samplePolynomial(*pr);
> +		set.gr = samplePolynomial(*pgr);
> +		set.gb = samplePolynomial(*pgb);
> +		set.b = samplePolynomial(*pb);

   those 4 <LscPolynomial> need to calculate each channel ,but some of 
information are sharing :

lsc table size /center position/ mapping location/ coefs , all those 
were calculated 4 times.

can we thinking to provide base structure to save all those and can 
improve the time complexity

>   	}
>   
> -private:
> -	/*
> -	 * The rkisp1 LSC grid spacing is defined by the cell sizes on the first
> -	 * half of the grid. This is then mirrored in hardware to the other
> -	 * half. See parseSizes() for further details. For easier handling, this
> -	 * function converts the cell sizes of half the grid to a list of
> -	 * position of the whole grid (on one axis). Example:
> -	 *
> -	 * input:   | 0.2 | 0.3 |
> -	 * output: 0.0   0.2   0.5   0.8   1.0
> -	 */
> -	std::vector<double> sizesListToPositions(const std::vector<double> &sizes)
> -	{
> -		const int half = sizes.size();
> -		std::vector<double> positions(half * 2 + 1);
> -		double x = 0.0;
> -
> -		positions[half] = 0.5;
> -		for (int i = 1; i <= half; i++) {
> -			x += sizes[half - i];
> -			positions[half - i] = 0.5 - x;
> -			positions[half + i] = 0.5 + x;
> -		}
> +	if (lscData.empty()) {
> +		LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> +		return -EINVAL;
> +	}
>   
> -		return positions;
> +	return 0;
> +}
> +
> +/*
> + * The rkisp1 LSC grid spacing is defined by the cell sizes on the first half of
> + * the grid. This is then mirrored in hardware to the other half. See
> + * parseSizes() for further details. For easier handling, this function converts
> + * the cell sizes of half the grid to a list of position of the whole grid (on
> + * one axis). Example:
> + *
> + * input:   | 0.2 | 0.3 |
> + * output: 0.0   0.2   0.5   0.8   1.0
> + */
> +std::vector<double> LscPolynomialLoader::sizesListToPositions(const std::vector<double> &sizes)
> +{
> +	const int half = sizes.size();
> +	std::vector<double> positions(half * 2 + 1);
> +	double x = 0.0;
> +
> +	positions[half] = 0.5;
> +	for (int i = 1; i <= half; i++) {
> +		x += sizes[half - i];
> +		positions[half - i] = 0.5 - x;
> +		positions[half + i] = 0.5 + x;
>   	}
>   
> -	std::vector<uint16_t> samplePolynomial(const LscPolynomial &poly)
> -	{
> -		constexpr int k = RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> -
> -		double m = poly.getM();
> -		double x0 = cropRectangle_.x / m;
> -		double y0 = cropRectangle_.y / m;
> -		double w = cropRectangle_.width / m;
> -		double h = cropRectangle_.height / m;
> -		std::vector<uint16_t> samples;
> -
> -		ASSERT(xSizes_.size() * 2 + 1 == k);
> -		ASSERT(ySizes_.size() * 2 + 1 == k);
> -
> -		samples.reserve(k * k);
> -
> -		std::vector<double> xPos(sizesListToPositions(xSizes_));
> -		std::vector<double> yPos(sizesListToPositions(ySizes_));
> -
> -		for (int y = 0; y < k; y++) {
> -			for (int x = 0; x < k; x++) {
> -				double xp = x0 + xPos[x] * w;
> -				double yp = y0 + yPos[y] * h;
> -				/*
> -				 * The hardware uses 2.10 fixed point format and
> -				 * limits the legal values to [1..3.999]. Scale
> -				 * and clamp the sampled value accordingly.
> -				 */
> -				int v = static_cast<int>(
> -					poly.sampleAtNormalizedPixelPos(xp, yp) *
> -					1024);
> -				v = std::min(std::max(v, 1024), 4095);
> -				samples.push_back(v);
> -			}
> +	return positions;
> +}
> +
> +std::vector<uint16_t> LscPolynomialLoader::samplePolynomial(const LscPolynomial &poly)
> +{
> +	constexpr int k = RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> +
> +	double m = poly.getM();
> +	double x0 = cropRectangle_.x / m;
> +	double y0 = cropRectangle_.y / m;
> +	double w = cropRectangle_.width / m;
> +	double h = cropRectangle_.height / m;
> +	std::vector<uint16_t> samples;
> +
> +	ASSERT(xSizes_.size() * 2 + 1 == k);
> +	ASSERT(ySizes_.size() * 2 + 1 == k);
> +
> +	samples.reserve(k * k);
> +
> +	std::vector<double> xPos(sizesListToPositions(xSizes_));
> +	std::vector<double> yPos(sizesListToPositions(ySizes_));
> +
> +	for (int y = 0; y < k; y++) {
> +		for (int x = 0; x < k; x++) {
> +			double xp = x0 + xPos[x] * w;
> +			double yp = y0 + yPos[y] * h;
> +			/*
> +			 * The hardware uses 2.10 fixed point format and limits
> +			 * the legal values to [1..3.999]. Scale and clamp the
> +			 * sampled value accordingly.
> +			 */
> +			int v = static_cast<int>(
> +				poly.sampleAtNormalizedPixelPos(xp, yp) *
> +				1024);
> +			v = std::min(std::max(v, 1024), 4095);
> +			samples.push_back(v);
>   		}
> -		return samples;
>   	}
> -
> -	Size sensorSize_;
> -	Rectangle cropRectangle_;
> -	const std::vector<double> &xSizes_;
> -	const std::vector<double> &ySizes_;
> -};
> +	return samples;
> +}
>   
>   class LscTableLoader
>   {
>   public:
>   	int parseLscData(const YamlObject &yamlSets,
> -			 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> -	{
> -		const auto &sets = yamlSets.asList();
> -
> -		for (const auto &yamlSet : sets) {
> -			uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> -
> -			if (lscData.count(ct)) {
> -				LOG(RkISP1Lsc, Error)
> -					<< "Multiple sets found for color temperature "
> -					<< ct;
> -				return -EINVAL;
> -			}
> -
> -			LensShadingCorrection::Components &set = lscData[ct];
> -
> -			set.r = parseTable(yamlSet, "r");
> -			set.gr = parseTable(yamlSet, "gr");
> -			set.gb = parseTable(yamlSet, "gb");
> -			set.b = parseTable(yamlSet, "b");
> -
> -			if (set.r.empty() || set.gr.empty() ||
> -			    set.gb.empty() || set.b.empty()) {
> -				LOG(RkISP1Lsc, Error)
> -					<< "Set for color temperature " << ct
> -					<< " is missing tables";
> -				return -EINVAL;
> -			}
> -		}
> +			 std::map<unsigned int, LensShadingCorrection::Components> &lscData);
> +
> +private:
> +	std::vector<uint16_t> parseTable(const YamlObject &tuningData,
> +					 const char *prop);
> +};
> +
> +int LscTableLoader::parseLscData(const YamlObject &yamlSets,
> +				 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> +{
> +	const auto &sets = yamlSets.asList();
> +
> +	for (const auto &yamlSet : sets) {
> +		uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
>   
> -		if (lscData.empty()) {
> -			LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> +		if (lscData.count(ct)) {
> +			LOG(RkISP1Lsc, Error)
> +				<< "Multiple sets found for color temperature "
> +				<< ct;
>   			return -EINVAL;
>   		}
>   
> -		return 0;
> -	}
> +		LensShadingCorrection::Components &set = lscData[ct];
>   
> -private:
> -	std::vector<uint16_t> parseTable(const YamlObject &tuningData,
> -					 const char *prop)
> -	{
> -		static constexpr unsigned int kLscNumSamples =
> -			RKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> +		set.r = parseTable(yamlSet, "r");
> +		set.gr = parseTable(yamlSet, "gr");
> +		set.gb = parseTable(yamlSet, "gb");
> +		set.b = parseTable(yamlSet, "b");
>   
> -		std::vector<uint16_t> table =
> -			tuningData[prop].getList<uint16_t>().value_or(std::vector<uint16_t>{});
> -		if (table.size() != kLscNumSamples) {
> +		if (set.r.empty() || set.gr.empty() ||
> +		    set.gb.empty() || set.b.empty()) {
>   			LOG(RkISP1Lsc, Error)
> -				<< "Invalid '" << prop << "' values: expected "
> -				<< kLscNumSamples
> -				<< " elements, got " << table.size();
> -			return {};
> +				<< "Set for color temperature " << ct
> +				<< " is missing tables";
> +			return -EINVAL;
>   		}
> +	}
>   
> -		return table;
> +	if (lscData.empty()) {
> +		LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> +		return -EINVAL;
>   	}
> -};
> +
> +	return 0;
> +}
> +
> +std::vector<uint16_t> LscTableLoader::parseTable(const YamlObject &tuningData,
> +						 const char *prop)
> +{
> +	static constexpr unsigned int kLscNumSamples =
> +		RKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> +
> +	std::vector<uint16_t> table =
> +		tuningData[prop].getList<uint16_t>().value_or(std::vector<uint16_t>{});
> +	if (table.size() != kLscNumSamples) {
> +		LOG(RkISP1Lsc, Error)
> +			<< "Invalid '" << prop << "' values: expected "
> +			<< kLscNumSamples
> +			<< " elements, got " << table.size();
> +		return {};
> +	}
> +
> +	return table;
> +}
>   
>   static std::vector<double> parseSizes(const YamlObject &tuningData,
>   				      const char *prop)
Rui Wang Oct. 27, 2025, 9:29 p.m. UTC | #4 
On 2025-10-14 03:52, Stefan Klug wrote:
> Move the function definitions out of the related classes. This was noted
> in review after the series was already merged. After trying it out I
> must admit that it really improves readability and reduces the
> indentation level by one.
>
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
>
> ---
>
> Note: This patch contains no functional changes. `git diff
> --ignore-space-change` helps in verifying that.
> ---
>   src/ipa/rkisp1/algorithms/lsc.cpp | 314 ++++++++++++++++--------------
>   1 file changed, 163 insertions(+), 151 deletions(-)
>
> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
> index e46943f4bae0..0c1bb470c346 100644
> --- a/src/ipa/rkisp1/algorithms/lsc.cpp
> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp
> @@ -85,184 +85,196 @@ public:
>   	}
>   
>   	int parseLscData(const YamlObject &yamlSets,
> -			 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> -	{
> -		const auto &sets = yamlSets.asList();
> -		for (const auto &yamlSet : sets) {
> -			std::optional<LscPolynomial> pr, pgr, pgb, pb;
> -			uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> -
> -			if (lscData.count(ct)) {
> -				LOG(RkISP1Lsc, Error)
> -					<< "Multiple sets found for "
> -					<< "color temperature " << ct;
> -				return -EINVAL;
> -			}
> -
> -			LensShadingCorrection::Components &set = lscData[ct];
> -			pr = yamlSet["r"].get<LscPolynomial>();
> -			pgr = yamlSet["gr"].get<LscPolynomial>();
> -			pgb = yamlSet["gb"].get<LscPolynomial>();
> -			pb = yamlSet["b"].get<LscPolynomial>();
> -
> -			if (!(pr || pgr || pgb || pb)) {
> -				LOG(RkISP1Lsc, Error)
> -					<< "Failed to parse polynomial for "
> -					<< "colour temperature " << ct;
> -				return -EINVAL;
> -			}
> -
> -			pr->setReferenceImageSize(sensorSize_);
> -			pgr->setReferenceImageSize(sensorSize_);
> -			pgb->setReferenceImageSize(sensorSize_);
> -			pb->setReferenceImageSize(sensorSize_);
> -			set.r = samplePolynomial(*pr);
> -			set.gr = samplePolynomial(*pgr);
> -			set.gb = samplePolynomial(*pgb);
> -			set.b = samplePolynomial(*pb);
> +			 std::map<unsigned int, LensShadingCorrection::Components> &lscData);
> +
> +private:
> +	std::vector<double> sizesListToPositions(const std::vector<double> &sizes);
> +	std::vector<uint16_t> samplePolynomial(const LscPolynomial &poly);
> +
> +	Size sensorSize_;
> +	Rectangle cropRectangle_;
> +	const std::vector<double> &xSizes_;
> +	const std::vector<double> &ySizes_;
> +};
> +
> +int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,
> +				      std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> +{
> +	const auto &sets = yamlSets.asList();
> +	for (const auto &yamlSet : sets) {
> +		std::optional<LscPolynomial> pr, pgr, pgb, pb;
> +		uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> +
> +		if (lscData.count(ct)) {
> +			LOG(RkISP1Lsc, Error)
> +				<< "Multiple sets found for "
> +				<< "color temperature " << ct;
> +			return -EINVAL;
>   		}
>   
> -		if (lscData.empty()) {
> -			LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> +		LensShadingCorrection::Components &set = lscData[ct];
> +		pr = yamlSet["r"].get<LscPolynomial>();
> +		pgr = yamlSet["gr"].get<LscPolynomial>();
> +		pgb = yamlSet["gb"].get<LscPolynomial>();
> +		pb = yamlSet["b"].get<LscPolynomial>();
> +
> +		if (!(pr || pgr || pgb || pb)) {
> +			LOG(RkISP1Lsc, Error)
> +				<< "Failed to parse polynomial for "
> +				<< "colour temperature " << ct;
>   			return -EINVAL;
>   		}
>   
> -		return 0;
> +		pr->setReferenceImageSize(sensorSize_);
> +		pgr->setReferenceImageSize(sensorSize_);
> +		pgb->setReferenceImageSize(sensorSize_);
> +		pb->setReferenceImageSize(sensorSize_);
> +		set.r = samplePolynomial(*pr);
> +		set.gr = samplePolynomial(*pgr);
> +		set.gb = samplePolynomial(*pgb);
> +		set.b = samplePolynomial(*pb);
>   	}
>   
> -private:
> -	/*
> -	 * The rkisp1 LSC grid spacing is defined by the cell sizes on the first
> -	 * half of the grid. This is then mirrored in hardware to the other
> -	 * half. See parseSizes() for further details. For easier handling, this
> -	 * function converts the cell sizes of half the grid to a list of
> -	 * position of the whole grid (on one axis). Example:
> -	 *
> -	 * input:   | 0.2 | 0.3 |
> -	 * output: 0.0   0.2   0.5   0.8   1.0
> -	 */
> -	std::vector<double> sizesListToPositions(const std::vector<double> &sizes)
> -	{
> -		const int half = sizes.size();
> -		std::vector<double> positions(half * 2 + 1);
> -		double x = 0.0;
> -
> -		positions[half] = 0.5;
> -		for (int i = 1; i <= half; i++) {
> -			x += sizes[half - i];
> -			positions[half - i] = 0.5 - x;
> -			positions[half + i] = 0.5 + x;
> -		}
> +	if (lscData.empty()) {
> +		LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> +		return -EINVAL;
> +	}
>   
> -		return positions;
> +	return 0;
> +}
> +
> +/*
> + * The rkisp1 LSC grid spacing is defined by the cell sizes on the first half of
> + * the grid. This is then mirrored in hardware to the other half. See
> + * parseSizes() for further details. For easier handling, this function converts
> + * the cell sizes of half the grid to a list of position of the whole grid (on
> + * one axis). Example:
> + *
> + * input:   | 0.2 | 0.3 |
> + * output: 0.0   0.2   0.5   0.8   1.0
> + */
> +std::vector<double> LscPolynomialLoader::sizesListToPositions(const std::vector<double> &sizes)
> +{
> +	const int half = sizes.size();
> +	std::vector<double> positions(half * 2 + 1);
> +	double x = 0.0;
> +
> +	positions[half] = 0.5;
> +	for (int i = 1; i <= half; i++) {
> +		x += sizes[half - i];
> +		positions[half - i] = 0.5 - x;
> +		positions[half + i] = 0.5 + x;
>   	}
>   
> -	std::vector<uint16_t> samplePolynomial(const LscPolynomial &poly)
> -	{
> -		constexpr int k = RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> -
> -		double m = poly.getM();
> -		double x0 = cropRectangle_.x / m;
> -		double y0 = cropRectangle_.y / m;
> -		double w = cropRectangle_.width / m;
> -		double h = cropRectangle_.height / m;
> -		std::vector<uint16_t> samples;
> -
> -		ASSERT(xSizes_.size() * 2 + 1 == k);
> -		ASSERT(ySizes_.size() * 2 + 1 == k);
> -
> -		samples.reserve(k * k);
> -
> -		std::vector<double> xPos(sizesListToPositions(xSizes_));
> -		std::vector<double> yPos(sizesListToPositions(ySizes_));
> -
> -		for (int y = 0; y < k; y++) {
> -			for (int x = 0; x < k; x++) {
> -				double xp = x0 + xPos[x] * w;
> -				double yp = y0 + yPos[y] * h;
> -				/*
> -				 * The hardware uses 2.10 fixed point format and
> -				 * limits the legal values to [1..3.999]. Scale
> -				 * and clamp the sampled value accordingly.
> -				 */
> -				int v = static_cast<int>(
> -					poly.sampleAtNormalizedPixelPos(xp, yp) *
> -					1024);
> -				v = std::min(std::max(v, 1024), 4095);
> -				samples.push_back(v);
> -			}
> +	return positions;
> +}
> +
> +std::vector<uint16_t> LscPolynomialLoader::samplePolynomial(const LscPolynomial &poly)
> +{
> +	constexpr int k = RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> +
> +	double m = poly.getM();
> +	double x0 = cropRectangle_.x / m;
> +	double y0 = cropRectangle_.y / m;
> +	double w = cropRectangle_.width / m;
> +	double h = cropRectangle_.height / m;
> +	std::vector<uint16_t> samples;
> +
> +	ASSERT(xSizes_.size() * 2 + 1 == k);
> +	ASSERT(ySizes_.size() * 2 + 1 == k);
> +
> +	samples.reserve(k * k);
> +
> +	std::vector<double> xPos(sizesListToPositions(xSizes_));
> +	std::vector<double> yPos(sizesListToPositions(ySizes_));
> +
> +	for (int y = 0; y < k; y++) {
> +		for (int x = 0; x < k; x++) {
> +			double xp = x0 + xPos[x] * w;
> +			double yp = y0 + yPos[y] * h;
> +			/*
> +			 * The hardware uses 2.10 fixed point format and limits
> +			 * the legal values to [1..3.999]. Scale and clamp the
> +			 * sampled value accordingly.
> +			 */
> +			int v = static_cast<int>(
> +				poly.sampleAtNormalizedPixelPos(xp, yp) *
> +				1024);
> +			v = std::min(std::max(v, 1024), 4095);
> +			samples.push_back(v);
>   		}
> -		return samples;
>   	}
> -
> -	Size sensorSize_;
> -	Rectangle cropRectangle_;
> -	const std::vector<double> &xSizes_;
> -	const std::vector<double> &ySizes_;
> -};
> +	return samples;
> +}
>   
>   class LscTableLoader
>   {
>   public:
>   	int parseLscData(const YamlObject &yamlSets,
> -			 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> -	{
> -		const auto &sets = yamlSets.asList();
> -
> -		for (const auto &yamlSet : sets) {
> -			uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> -
> -			if (lscData.count(ct)) {
> -				LOG(RkISP1Lsc, Error)
> -					<< "Multiple sets found for color temperature "
> -					<< ct;
> -				return -EINVAL;
> -			}
> -
> -			LensShadingCorrection::Components &set = lscData[ct];
> -
> -			set.r = parseTable(yamlSet, "r");
> -			set.gr = parseTable(yamlSet, "gr");
> -			set.gb = parseTable(yamlSet, "gb");
> -			set.b = parseTable(yamlSet, "b");
> -
> -			if (set.r.empty() || set.gr.empty() ||
> -			    set.gb.empty() || set.b.empty()) {
> -				LOG(RkISP1Lsc, Error)
> -					<< "Set for color temperature " << ct
> -					<< " is missing tables";
> -				return -EINVAL;
> -			}
> -		}
> +			 std::map<unsigned int, LensShadingCorrection::Components> &lscData);
> +
> +private:
> +	std::vector<uint16_t> parseTable(const YamlObject &tuningData,
> +					 const char *prop);
> +};
> +
> +int LscTableLoader::parseLscData(const YamlObject &yamlSets,
> +				 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> +{
> +	const auto &sets = yamlSets.asList();
> +
> +	for (const auto &yamlSet : sets) {
> +		uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
>   
> -		if (lscData.empty()) {
> -			LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> +		if (lscData.count(ct)) {
> +			LOG(RkISP1Lsc, Error)
> +				<< "Multiple sets found for color temperature "
> +				<< ct;
>   			return -EINVAL;
>   		}
>   
> -		return 0;
> -	}
> +		LensShadingCorrection::Components &set = lscData[ct];
>   
> -private:
> -	std::vector<uint16_t> parseTable(const YamlObject &tuningData,
> -					 const char *prop)
> -	{
> -		static constexpr unsigned int kLscNumSamples =
> -			RKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> +		set.r = parseTable(yamlSet, "r");
> +		set.gr = parseTable(yamlSet, "gr");
> +		set.gb = parseTable(yamlSet, "gb");
> +		set.b = parseTable(yamlSet, "b");
>   
> -		std::vector<uint16_t> table =
> -			tuningData[prop].getList<uint16_t>().value_or(std::vector<uint16_t>{});
> -		if (table.size() != kLscNumSamples) {
> +		if (set.r.empty() || set.gr.empty() ||
> +		    set.gb.empty() || set.b.empty()) {
>   			LOG(RkISP1Lsc, Error)
> -				<< "Invalid '" << prop << "' values: expected "
> -				<< kLscNumSamples
> -				<< " elements, got " << table.size();
> -			return {};
> +				<< "Set for color temperature " << ct
> +				<< " is missing tables";
> +			return -EINVAL;
>   		}
> +	}
>   
> -		return table;
> +	if (lscData.empty()) {
> +		LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> +		return -EINVAL;
>   	}
> -};
> +
> +	return 0;
> +}
> +
> +std::vector<uint16_t> LscTableLoader::parseTable(const YamlObject &tuningData,
> +						 const char *prop)
> +{
> +	static constexpr unsigned int kLscNumSamples =
> +		RKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> +
> +	std::vector<uint16_t> table =
> +		tuningData[prop].getList<uint16_t>().value_or(std::vector<uint16_t>{});
> +	if (table.size() != kLscNumSamples) {
> +		LOG(RkISP1Lsc, Error)
> +			<< "Invalid '" << prop << "' values: expected "
> +			<< kLscNumSamples
> +			<< " elements, got " << table.size();
> +		return {};
> +	}
> +
> +	return table;
> +}
>   
>   static std::vector<double> parseSizes(const YamlObject &tuningData,
>   				      const char *prop)


Reviewed-by: Rui Wang <rui.wang@ideasonboard.com>
Laurent Pinchart Oct. 28, 2025, 9:37 a.m. UTC | #5 
Hi Rui,

On Sat, Oct 25, 2025 at 10:23:57PM -0400, Rui Wang wrote:
> On 2025-10-14 03:52, Stefan Klug wrote:
> > Move the function definitions out of the related classes. This was noted
> > in review after the series was already merged. After trying it out I
> > must admit that it really improves readability and reduces the
> > indentation level by one.
> >
> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> >
> > ---
> >
> > Note: This patch contains no functional changes. `git diff
> > --ignore-space-change` helps in verifying that.
> > ---
> >   src/ipa/rkisp1/algorithms/lsc.cpp | 314 ++++++++++++++++--------------
> >   1 file changed, 163 insertions(+), 151 deletions(-)
> >
> > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
> > index e46943f4bae0..0c1bb470c346 100644
> > --- a/src/ipa/rkisp1/algorithms/lsc.cpp
> > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp
> > @@ -85,184 +85,196 @@ public:
> >   	}
> >   
> >   	int parseLscData(const YamlObject &yamlSets,
> > -			 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> > -	{
> > -		const auto &sets = yamlSets.asList();
> > -		for (const auto &yamlSet : sets) {
> > -			std::optional<LscPolynomial> pr, pgr, pgb, pb;
> > -			uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> > -
> > -			if (lscData.count(ct)) {
> > -				LOG(RkISP1Lsc, Error)
> > -					<< "Multiple sets found for "
> > -					<< "color temperature " << ct;
> > -				return -EINVAL;
> > -			}
> > -
> > -			LensShadingCorrection::Components &set = lscData[ct];
> > -			pr = yamlSet["r"].get<LscPolynomial>();
> > -			pgr = yamlSet["gr"].get<LscPolynomial>();
> > -			pgb = yamlSet["gb"].get<LscPolynomial>();
> > -			pb = yamlSet["b"].get<LscPolynomial>();
> > -
> > -			if (!(pr || pgr || pgb || pb)) {
> > -				LOG(RkISP1Lsc, Error)
> > -					<< "Failed to parse polynomial for "
> > -					<< "colour temperature " << ct;
> > -				return -EINVAL;
> > -			}
> > -
> > -			pr->setReferenceImageSize(sensorSize_);
> > -			pgr->setReferenceImageSize(sensorSize_);
> > -			pgb->setReferenceImageSize(sensorSize_);
> > -			pb->setReferenceImageSize(sensorSize_);
> > -			set.r = samplePolynomial(*pr);
> > -			set.gr = samplePolynomial(*pgr);
> > -			set.gb = samplePolynomial(*pgb);
> > -			set.b = samplePolynomial(*pb);
> > +			 std::map<unsigned int, LensShadingCorrection::Components> &lscData);
> > +
> > +private:
> > +	std::vector<double> sizesListToPositions(const std::vector<double> &sizes);
> > +	std::vector<uint16_t> samplePolynomial(const LscPolynomial &poly);
> > +
> > +	Size sensorSize_;
> > +	Rectangle cropRectangle_;
> > +	const std::vector<double> &xSizes_;
> > +	const std::vector<double> &ySizes_;
> > +};
> > +
> > +int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,
> > +				      std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> > +{
> > +	const auto &sets = yamlSets.asList();
> > +	for (const auto &yamlSet : sets) {
> > +		std::optional<LscPolynomial> pr, pgr, pgb, pb;
> > +		uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> > +
> > +		if (lscData.count(ct)) {
> > +			LOG(RkISP1Lsc, Error)
> > +				<< "Multiple sets found for "
> > +				<< "color temperature " << ct;
> > +			return -EINVAL;
> >   		}
> >   
> > -		if (lscData.empty()) {
> > -			LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> > +		LensShadingCorrection::Components &set = lscData[ct];
> > +		pr = yamlSet["r"].get<LscPolynomial>();
> > +		pgr = yamlSet["gr"].get<LscPolynomial>();
> > +		pgb = yamlSet["gb"].get<LscPolynomial>();
> > +		pb = yamlSet["b"].get<LscPolynomial>();
> > +
> > +		if (!(pr || pgr || pgb || pb)) {
> > +			LOG(RkISP1Lsc, Error)
> > +				<< "Failed to parse polynomial for "
> > +				<< "colour temperature " << ct;
> >   			return -EINVAL;
> >   		}
> >   
> > -		return 0;
> > +		pr->setReferenceImageSize(sensorSize_);
> > +		pgr->setReferenceImageSize(sensorSize_);
> > +		pgb->setReferenceImageSize(sensorSize_);
> > +		pb->setReferenceImageSize(sensorSize_);
> > +		set.r = samplePolynomial(*pr);
> > +		set.gr = samplePolynomial(*pgr);
> > +		set.gb = samplePolynomial(*pgb);
> > +		set.b = samplePolynomial(*pb);
> 
>    those 4 <LscPolynomial> need to calculate each channel ,but some of 
> information are sharing :
> 
> lsc table size /center position/ mapping location/ coefs , all those 
> were calculated 4 times.
> 
> can we thinking to provide base structure to save all those and can 
> improve the time complexity

That sounds like a good improvement to explore. It's not an issue
introduced by this patch though, which only moves code around. I think
we can merge this, and then optimize on top.

> >   	}
> >   
> > -private:
> > -	/*
> > -	 * The rkisp1 LSC grid spacing is defined by the cell sizes on the first
> > -	 * half of the grid. This is then mirrored in hardware to the other
> > -	 * half. See parseSizes() for further details. For easier handling, this
> > -	 * function converts the cell sizes of half the grid to a list of
> > -	 * position of the whole grid (on one axis). Example:
> > -	 *
> > -	 * input:   | 0.2 | 0.3 |
> > -	 * output: 0.0   0.2   0.5   0.8   1.0
> > -	 */
> > -	std::vector<double> sizesListToPositions(const std::vector<double> &sizes)
> > -	{
> > -		const int half = sizes.size();
> > -		std::vector<double> positions(half * 2 + 1);
> > -		double x = 0.0;
> > -
> > -		positions[half] = 0.5;
> > -		for (int i = 1; i <= half; i++) {
> > -			x += sizes[half - i];
> > -			positions[half - i] = 0.5 - x;
> > -			positions[half + i] = 0.5 + x;
> > -		}
> > +	if (lscData.empty()) {
> > +		LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> > +		return -EINVAL;
> > +	}
> >   
> > -		return positions;
> > +	return 0;
> > +}
> > +
> > +/*
> > + * The rkisp1 LSC grid spacing is defined by the cell sizes on the first half of
> > + * the grid. This is then mirrored in hardware to the other half. See
> > + * parseSizes() for further details. For easier handling, this function converts
> > + * the cell sizes of half the grid to a list of position of the whole grid (on
> > + * one axis). Example:
> > + *
> > + * input:   | 0.2 | 0.3 |
> > + * output: 0.0   0.2   0.5   0.8   1.0
> > + */
> > +std::vector<double> LscPolynomialLoader::sizesListToPositions(const std::vector<double> &sizes)
> > +{
> > +	const int half = sizes.size();
> > +	std::vector<double> positions(half * 2 + 1);
> > +	double x = 0.0;
> > +
> > +	positions[half] = 0.5;
> > +	for (int i = 1; i <= half; i++) {
> > +		x += sizes[half - i];
> > +		positions[half - i] = 0.5 - x;
> > +		positions[half + i] = 0.5 + x;
> >   	}
> >   
> > -	std::vector<uint16_t> samplePolynomial(const LscPolynomial &poly)
> > -	{
> > -		constexpr int k = RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> > -
> > -		double m = poly.getM();
> > -		double x0 = cropRectangle_.x / m;
> > -		double y0 = cropRectangle_.y / m;
> > -		double w = cropRectangle_.width / m;
> > -		double h = cropRectangle_.height / m;
> > -		std::vector<uint16_t> samples;
> > -
> > -		ASSERT(xSizes_.size() * 2 + 1 == k);
> > -		ASSERT(ySizes_.size() * 2 + 1 == k);
> > -
> > -		samples.reserve(k * k);
> > -
> > -		std::vector<double> xPos(sizesListToPositions(xSizes_));
> > -		std::vector<double> yPos(sizesListToPositions(ySizes_));
> > -
> > -		for (int y = 0; y < k; y++) {
> > -			for (int x = 0; x < k; x++) {
> > -				double xp = x0 + xPos[x] * w;
> > -				double yp = y0 + yPos[y] * h;
> > -				/*
> > -				 * The hardware uses 2.10 fixed point format and
> > -				 * limits the legal values to [1..3.999]. Scale
> > -				 * and clamp the sampled value accordingly.
> > -				 */
> > -				int v = static_cast<int>(
> > -					poly.sampleAtNormalizedPixelPos(xp, yp) *
> > -					1024);
> > -				v = std::min(std::max(v, 1024), 4095);
> > -				samples.push_back(v);
> > -			}
> > +	return positions;
> > +}
> > +
> > +std::vector<uint16_t> LscPolynomialLoader::samplePolynomial(const LscPolynomial &poly)
> > +{
> > +	constexpr int k = RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> > +
> > +	double m = poly.getM();
> > +	double x0 = cropRectangle_.x / m;
> > +	double y0 = cropRectangle_.y / m;
> > +	double w = cropRectangle_.width / m;
> > +	double h = cropRectangle_.height / m;
> > +	std::vector<uint16_t> samples;
> > +
> > +	ASSERT(xSizes_.size() * 2 + 1 == k);
> > +	ASSERT(ySizes_.size() * 2 + 1 == k);
> > +
> > +	samples.reserve(k * k);
> > +
> > +	std::vector<double> xPos(sizesListToPositions(xSizes_));
> > +	std::vector<double> yPos(sizesListToPositions(ySizes_));
> > +
> > +	for (int y = 0; y < k; y++) {
> > +		for (int x = 0; x < k; x++) {
> > +			double xp = x0 + xPos[x] * w;
> > +			double yp = y0 + yPos[y] * h;
> > +			/*
> > +			 * The hardware uses 2.10 fixed point format and limits
> > +			 * the legal values to [1..3.999]. Scale and clamp the
> > +			 * sampled value accordingly.
> > +			 */
> > +			int v = static_cast<int>(
> > +				poly.sampleAtNormalizedPixelPos(xp, yp) *
> > +				1024);
> > +			v = std::min(std::max(v, 1024), 4095);
> > +			samples.push_back(v);
> >   		}
> > -		return samples;
> >   	}
> > -
> > -	Size sensorSize_;
> > -	Rectangle cropRectangle_;
> > -	const std::vector<double> &xSizes_;
> > -	const std::vector<double> &ySizes_;
> > -};
> > +	return samples;
> > +}
> >   
> >   class LscTableLoader
> >   {
> >   public:
> >   	int parseLscData(const YamlObject &yamlSets,
> > -			 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> > -	{
> > -		const auto &sets = yamlSets.asList();
> > -
> > -		for (const auto &yamlSet : sets) {
> > -			uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> > -
> > -			if (lscData.count(ct)) {
> > -				LOG(RkISP1Lsc, Error)
> > -					<< "Multiple sets found for color temperature "
> > -					<< ct;
> > -				return -EINVAL;
> > -			}
> > -
> > -			LensShadingCorrection::Components &set = lscData[ct];
> > -
> > -			set.r = parseTable(yamlSet, "r");
> > -			set.gr = parseTable(yamlSet, "gr");
> > -			set.gb = parseTable(yamlSet, "gb");
> > -			set.b = parseTable(yamlSet, "b");
> > -
> > -			if (set.r.empty() || set.gr.empty() ||
> > -			    set.gb.empty() || set.b.empty()) {
> > -				LOG(RkISP1Lsc, Error)
> > -					<< "Set for color temperature " << ct
> > -					<< " is missing tables";
> > -				return -EINVAL;
> > -			}
> > -		}
> > +			 std::map<unsigned int, LensShadingCorrection::Components> &lscData);
> > +
> > +private:
> > +	std::vector<uint16_t> parseTable(const YamlObject &tuningData,
> > +					 const char *prop);
> > +};
> > +
> > +int LscTableLoader::parseLscData(const YamlObject &yamlSets,
> > +				 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> > +{
> > +	const auto &sets = yamlSets.asList();
> > +
> > +	for (const auto &yamlSet : sets) {
> > +		uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> >   
> > -		if (lscData.empty()) {
> > -			LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> > +		if (lscData.count(ct)) {
> > +			LOG(RkISP1Lsc, Error)
> > +				<< "Multiple sets found for color temperature "
> > +				<< ct;
> >   			return -EINVAL;
> >   		}
> >   
> > -		return 0;
> > -	}
> > +		LensShadingCorrection::Components &set = lscData[ct];
> >   
> > -private:
> > -	std::vector<uint16_t> parseTable(const YamlObject &tuningData,
> > -					 const char *prop)
> > -	{
> > -		static constexpr unsigned int kLscNumSamples =
> > -			RKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> > +		set.r = parseTable(yamlSet, "r");
> > +		set.gr = parseTable(yamlSet, "gr");
> > +		set.gb = parseTable(yamlSet, "gb");
> > +		set.b = parseTable(yamlSet, "b");
> >   
> > -		std::vector<uint16_t> table =
> > -			tuningData[prop].getList<uint16_t>().value_or(std::vector<uint16_t>{});
> > -		if (table.size() != kLscNumSamples) {
> > +		if (set.r.empty() || set.gr.empty() ||
> > +		    set.gb.empty() || set.b.empty()) {
> >   			LOG(RkISP1Lsc, Error)
> > -				<< "Invalid '" << prop << "' values: expected "
> > -				<< kLscNumSamples
> > -				<< " elements, got " << table.size();
> > -			return {};
> > +				<< "Set for color temperature " << ct
> > +				<< " is missing tables";
> > +			return -EINVAL;
> >   		}
> > +	}
> >   
> > -		return table;
> > +	if (lscData.empty()) {
> > +		LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> > +		return -EINVAL;
> >   	}
> > -};
> > +
> > +	return 0;
> > +}
> > +
> > +std::vector<uint16_t> LscTableLoader::parseTable(const YamlObject &tuningData,
> > +						 const char *prop)
> > +{
> > +	static constexpr unsigned int kLscNumSamples =
> > +		RKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> > +
> > +	std::vector<uint16_t> table =
> > +		tuningData[prop].getList<uint16_t>().value_or(std::vector<uint16_t>{});
> > +	if (table.size() != kLscNumSamples) {
> > +		LOG(RkISP1Lsc, Error)
> > +			<< "Invalid '" << prop << "' values: expected "
> > +			<< kLscNumSamples
> > +			<< " elements, got " << table.size();
> > +		return {};
> > +	}
> > +
> > +	return table;
> > +}
> >   
> >   static std::vector<double> parseSizes(const YamlObject &tuningData,
> >   				      const char *prop)
Laurent Pinchart Oct. 28, 2025, 9:44 a.m. UTC | #6 
On Tue, Oct 28, 2025 at 11:37:56AM +0200, Laurent Pinchart wrote:
> On Sat, Oct 25, 2025 at 10:23:57PM -0400, Rui Wang wrote:
> > On 2025-10-14 03:52, Stefan Klug wrote:
> > > Move the function definitions out of the related classes. This was noted
> > > in review after the series was already merged. After trying it out I
> > > must admit that it really improves readability and reduces the
> > > indentation level by one.
> > >
> > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > >
> > > ---
> > >
> > > Note: This patch contains no functional changes. `git diff
> > > --ignore-space-change` helps in verifying that.
> > > ---
> > >   src/ipa/rkisp1/algorithms/lsc.cpp | 314 ++++++++++++++++--------------
> > >   1 file changed, 163 insertions(+), 151 deletions(-)
> > >
> > > diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
> > > index e46943f4bae0..0c1bb470c346 100644
> > > --- a/src/ipa/rkisp1/algorithms/lsc.cpp
> > > +++ b/src/ipa/rkisp1/algorithms/lsc.cpp
> > > @@ -85,184 +85,196 @@ public:
> > >   	}
> > >   
> > >   	int parseLscData(const YamlObject &yamlSets,
> > > -			 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> > > -	{
> > > -		const auto &sets = yamlSets.asList();
> > > -		for (const auto &yamlSet : sets) {
> > > -			std::optional<LscPolynomial> pr, pgr, pgb, pb;
> > > -			uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> > > -
> > > -			if (lscData.count(ct)) {
> > > -				LOG(RkISP1Lsc, Error)
> > > -					<< "Multiple sets found for "
> > > -					<< "color temperature " << ct;
> > > -				return -EINVAL;
> > > -			}
> > > -
> > > -			LensShadingCorrection::Components &set = lscData[ct];
> > > -			pr = yamlSet["r"].get<LscPolynomial>();
> > > -			pgr = yamlSet["gr"].get<LscPolynomial>();
> > > -			pgb = yamlSet["gb"].get<LscPolynomial>();
> > > -			pb = yamlSet["b"].get<LscPolynomial>();
> > > -
> > > -			if (!(pr || pgr || pgb || pb)) {
> > > -				LOG(RkISP1Lsc, Error)
> > > -					<< "Failed to parse polynomial for "
> > > -					<< "colour temperature " << ct;
> > > -				return -EINVAL;
> > > -			}
> > > -
> > > -			pr->setReferenceImageSize(sensorSize_);
> > > -			pgr->setReferenceImageSize(sensorSize_);
> > > -			pgb->setReferenceImageSize(sensorSize_);
> > > -			pb->setReferenceImageSize(sensorSize_);
> > > -			set.r = samplePolynomial(*pr);
> > > -			set.gr = samplePolynomial(*pgr);
> > > -			set.gb = samplePolynomial(*pgb);
> > > -			set.b = samplePolynomial(*pb);
> > > +			 std::map<unsigned int, LensShadingCorrection::Components> &lscData);
> > > +
> > > +private:
> > > +	std::vector<double> sizesListToPositions(const std::vector<double> &sizes);
> > > +	std::vector<uint16_t> samplePolynomial(const LscPolynomial &poly);
> > > +
> > > +	Size sensorSize_;
> > > +	Rectangle cropRectangle_;
> > > +	const std::vector<double> &xSizes_;
> > > +	const std::vector<double> &ySizes_;
> > > +};
> > > +
> > > +int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,
> > > +				      std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> > > +{
> > > +	const auto &sets = yamlSets.asList();
> > > +	for (const auto &yamlSet : sets) {
> > > +		std::optional<LscPolynomial> pr, pgr, pgb, pb;
> > > +		uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> > > +
> > > +		if (lscData.count(ct)) {
> > > +			LOG(RkISP1Lsc, Error)
> > > +				<< "Multiple sets found for "
> > > +				<< "color temperature " << ct;
> > > +			return -EINVAL;
> > >   		}
> > >   
> > > -		if (lscData.empty()) {
> > > -			LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> > > +		LensShadingCorrection::Components &set = lscData[ct];
> > > +		pr = yamlSet["r"].get<LscPolynomial>();
> > > +		pgr = yamlSet["gr"].get<LscPolynomial>();
> > > +		pgb = yamlSet["gb"].get<LscPolynomial>();
> > > +		pb = yamlSet["b"].get<LscPolynomial>();
> > > +
> > > +		if (!(pr || pgr || pgb || pb)) {
> > > +			LOG(RkISP1Lsc, Error)
> > > +				<< "Failed to parse polynomial for "
> > > +				<< "colour temperature " << ct;
> > >   			return -EINVAL;
> > >   		}
> > >   
> > > -		return 0;
> > > +		pr->setReferenceImageSize(sensorSize_);
> > > +		pgr->setReferenceImageSize(sensorSize_);
> > > +		pgb->setReferenceImageSize(sensorSize_);
> > > +		pb->setReferenceImageSize(sensorSize_);
> > > +		set.r = samplePolynomial(*pr);
> > > +		set.gr = samplePolynomial(*pgr);
> > > +		set.gb = samplePolynomial(*pgb);
> > > +		set.b = samplePolynomial(*pb);
> > 
> >    those 4 <LscPolynomial> need to calculate each channel ,but some of 
> > information are sharing :
> > 
> > lsc table size /center position/ mapping location/ coefs , all those 
> > were calculated 4 times.
> > 
> > can we thinking to provide base structure to save all those and can 
> > improve the time complexity
> 
> That sounds like a good improvement to explore. It's not an issue
> introduced by this patch though, which only moves code around. I think
> we can merge this, and then optimize on top.

Which the rest of the series does :-)

> > >   	}
> > >   
> > > -private:
> > > -	/*
> > > -	 * The rkisp1 LSC grid spacing is defined by the cell sizes on the first
> > > -	 * half of the grid. This is then mirrored in hardware to the other
> > > -	 * half. See parseSizes() for further details. For easier handling, this
> > > -	 * function converts the cell sizes of half the grid to a list of
> > > -	 * position of the whole grid (on one axis). Example:
> > > -	 *
> > > -	 * input:   | 0.2 | 0.3 |
> > > -	 * output: 0.0   0.2   0.5   0.8   1.0
> > > -	 */
> > > -	std::vector<double> sizesListToPositions(const std::vector<double> &sizes)
> > > -	{
> > > -		const int half = sizes.size();
> > > -		std::vector<double> positions(half * 2 + 1);
> > > -		double x = 0.0;
> > > -
> > > -		positions[half] = 0.5;
> > > -		for (int i = 1; i <= half; i++) {
> > > -			x += sizes[half - i];
> > > -			positions[half - i] = 0.5 - x;
> > > -			positions[half + i] = 0.5 + x;
> > > -		}
> > > +	if (lscData.empty()) {
> > > +		LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> > > +		return -EINVAL;
> > > +	}
> > >   
> > > -		return positions;
> > > +	return 0;
> > > +}
> > > +
> > > +/*
> > > + * The rkisp1 LSC grid spacing is defined by the cell sizes on the first half of
> > > + * the grid. This is then mirrored in hardware to the other half. See
> > > + * parseSizes() for further details. For easier handling, this function converts
> > > + * the cell sizes of half the grid to a list of position of the whole grid (on
> > > + * one axis). Example:
> > > + *
> > > + * input:   | 0.2 | 0.3 |
> > > + * output: 0.0   0.2   0.5   0.8   1.0
> > > + */
> > > +std::vector<double> LscPolynomialLoader::sizesListToPositions(const std::vector<double> &sizes)
> > > +{
> > > +	const int half = sizes.size();
> > > +	std::vector<double> positions(half * 2 + 1);
> > > +	double x = 0.0;
> > > +
> > > +	positions[half] = 0.5;
> > > +	for (int i = 1; i <= half; i++) {
> > > +		x += sizes[half - i];
> > > +		positions[half - i] = 0.5 - x;
> > > +		positions[half + i] = 0.5 + x;
> > >   	}
> > >   
> > > -	std::vector<uint16_t> samplePolynomial(const LscPolynomial &poly)
> > > -	{
> > > -		constexpr int k = RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> > > -
> > > -		double m = poly.getM();
> > > -		double x0 = cropRectangle_.x / m;
> > > -		double y0 = cropRectangle_.y / m;
> > > -		double w = cropRectangle_.width / m;
> > > -		double h = cropRectangle_.height / m;
> > > -		std::vector<uint16_t> samples;
> > > -
> > > -		ASSERT(xSizes_.size() * 2 + 1 == k);
> > > -		ASSERT(ySizes_.size() * 2 + 1 == k);
> > > -
> > > -		samples.reserve(k * k);
> > > -
> > > -		std::vector<double> xPos(sizesListToPositions(xSizes_));
> > > -		std::vector<double> yPos(sizesListToPositions(ySizes_));
> > > -
> > > -		for (int y = 0; y < k; y++) {
> > > -			for (int x = 0; x < k; x++) {
> > > -				double xp = x0 + xPos[x] * w;
> > > -				double yp = y0 + yPos[y] * h;
> > > -				/*
> > > -				 * The hardware uses 2.10 fixed point format and
> > > -				 * limits the legal values to [1..3.999]. Scale
> > > -				 * and clamp the sampled value accordingly.
> > > -				 */
> > > -				int v = static_cast<int>(
> > > -					poly.sampleAtNormalizedPixelPos(xp, yp) *
> > > -					1024);
> > > -				v = std::min(std::max(v, 1024), 4095);
> > > -				samples.push_back(v);
> > > -			}
> > > +	return positions;
> > > +}
> > > +
> > > +std::vector<uint16_t> LscPolynomialLoader::samplePolynomial(const LscPolynomial &poly)
> > > +{
> > > +	constexpr int k = RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> > > +
> > > +	double m = poly.getM();
> > > +	double x0 = cropRectangle_.x / m;
> > > +	double y0 = cropRectangle_.y / m;
> > > +	double w = cropRectangle_.width / m;
> > > +	double h = cropRectangle_.height / m;
> > > +	std::vector<uint16_t> samples;
> > > +
> > > +	ASSERT(xSizes_.size() * 2 + 1 == k);
> > > +	ASSERT(ySizes_.size() * 2 + 1 == k);
> > > +
> > > +	samples.reserve(k * k);
> > > +
> > > +	std::vector<double> xPos(sizesListToPositions(xSizes_));
> > > +	std::vector<double> yPos(sizesListToPositions(ySizes_));
> > > +
> > > +	for (int y = 0; y < k; y++) {
> > > +		for (int x = 0; x < k; x++) {
> > > +			double xp = x0 + xPos[x] * w;
> > > +			double yp = y0 + yPos[y] * h;
> > > +			/*
> > > +			 * The hardware uses 2.10 fixed point format and limits
> > > +			 * the legal values to [1..3.999]. Scale and clamp the
> > > +			 * sampled value accordingly.
> > > +			 */
> > > +			int v = static_cast<int>(
> > > +				poly.sampleAtNormalizedPixelPos(xp, yp) *
> > > +				1024);
> > > +			v = std::min(std::max(v, 1024), 4095);
> > > +			samples.push_back(v);
> > >   		}
> > > -		return samples;
> > >   	}
> > > -
> > > -	Size sensorSize_;
> > > -	Rectangle cropRectangle_;
> > > -	const std::vector<double> &xSizes_;
> > > -	const std::vector<double> &ySizes_;
> > > -};
> > > +	return samples;
> > > +}
> > >   
> > >   class LscTableLoader
> > >   {
> > >   public:
> > >   	int parseLscData(const YamlObject &yamlSets,
> > > -			 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> > > -	{
> > > -		const auto &sets = yamlSets.asList();
> > > -
> > > -		for (const auto &yamlSet : sets) {
> > > -			uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> > > -
> > > -			if (lscData.count(ct)) {
> > > -				LOG(RkISP1Lsc, Error)
> > > -					<< "Multiple sets found for color temperature "
> > > -					<< ct;
> > > -				return -EINVAL;
> > > -			}
> > > -
> > > -			LensShadingCorrection::Components &set = lscData[ct];
> > > -
> > > -			set.r = parseTable(yamlSet, "r");
> > > -			set.gr = parseTable(yamlSet, "gr");
> > > -			set.gb = parseTable(yamlSet, "gb");
> > > -			set.b = parseTable(yamlSet, "b");
> > > -
> > > -			if (set.r.empty() || set.gr.empty() ||
> > > -			    set.gb.empty() || set.b.empty()) {
> > > -				LOG(RkISP1Lsc, Error)
> > > -					<< "Set for color temperature " << ct
> > > -					<< " is missing tables";
> > > -				return -EINVAL;
> > > -			}
> > > -		}
> > > +			 std::map<unsigned int, LensShadingCorrection::Components> &lscData);
> > > +
> > > +private:
> > > +	std::vector<uint16_t> parseTable(const YamlObject &tuningData,
> > > +					 const char *prop);
> > > +};
> > > +
> > > +int LscTableLoader::parseLscData(const YamlObject &yamlSets,
> > > +				 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
> > > +{
> > > +	const auto &sets = yamlSets.asList();
> > > +
> > > +	for (const auto &yamlSet : sets) {
> > > +		uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
> > >   
> > > -		if (lscData.empty()) {
> > > -			LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> > > +		if (lscData.count(ct)) {
> > > +			LOG(RkISP1Lsc, Error)
> > > +				<< "Multiple sets found for color temperature "
> > > +				<< ct;
> > >   			return -EINVAL;
> > >   		}
> > >   
> > > -		return 0;
> > > -	}
> > > +		LensShadingCorrection::Components &set = lscData[ct];
> > >   
> > > -private:
> > > -	std::vector<uint16_t> parseTable(const YamlObject &tuningData,
> > > -					 const char *prop)
> > > -	{
> > > -		static constexpr unsigned int kLscNumSamples =
> > > -			RKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> > > +		set.r = parseTable(yamlSet, "r");
> > > +		set.gr = parseTable(yamlSet, "gr");
> > > +		set.gb = parseTable(yamlSet, "gb");
> > > +		set.b = parseTable(yamlSet, "b");
> > >   
> > > -		std::vector<uint16_t> table =
> > > -			tuningData[prop].getList<uint16_t>().value_or(std::vector<uint16_t>{});
> > > -		if (table.size() != kLscNumSamples) {
> > > +		if (set.r.empty() || set.gr.empty() ||
> > > +		    set.gb.empty() || set.b.empty()) {
> > >   			LOG(RkISP1Lsc, Error)
> > > -				<< "Invalid '" << prop << "' values: expected "
> > > -				<< kLscNumSamples
> > > -				<< " elements, got " << table.size();
> > > -			return {};
> > > +				<< "Set for color temperature " << ct
> > > +				<< " is missing tables";
> > > +			return -EINVAL;
> > >   		}
> > > +	}
> > >   
> > > -		return table;
> > > +	if (lscData.empty()) {
> > > +		LOG(RkISP1Lsc, Error) << "Failed to load any sets";
> > > +		return -EINVAL;
> > >   	}
> > > -};
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +std::vector<uint16_t> LscTableLoader::parseTable(const YamlObject &tuningData,
> > > +						 const char *prop)
> > > +{
> > > +	static constexpr unsigned int kLscNumSamples =
> > > +		RKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
> > > +
> > > +	std::vector<uint16_t> table =
> > > +		tuningData[prop].getList<uint16_t>().value_or(std::vector<uint16_t>{});
> > > +	if (table.size() != kLscNumSamples) {
> > > +		LOG(RkISP1Lsc, Error)
> > > +			<< "Invalid '" << prop << "' values: expected "
> > > +			<< kLscNumSamples
> > > +			<< " elements, got " << table.size();
> > > +		return {};
> > > +	}
> > > +
> > > +	return table;
> > > +}
> > >   
> > >   static std::vector<double> parseSizes(const YamlObject &tuningData,
> > >   				      const char *prop)

Patch
diff mbox series 

diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
index e46943f4bae0..0c1bb470c346 100644
--- a/src/ipa/rkisp1/algorithms/lsc.cpp
+++ b/src/ipa/rkisp1/algorithms/lsc.cpp
@@ -85,184 +85,196 @@  public:
 	}
 
 	int parseLscData(const YamlObject &yamlSets,
-			 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
-	{
-		const auto &sets = yamlSets.asList();
-		for (const auto &yamlSet : sets) {
-			std::optional<LscPolynomial> pr, pgr, pgb, pb;
-			uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
-
-			if (lscData.count(ct)) {
-				LOG(RkISP1Lsc, Error)
-					<< "Multiple sets found for "
-					<< "color temperature " << ct;
-				return -EINVAL;
-			}
-
-			LensShadingCorrection::Components &set = lscData[ct];
-			pr = yamlSet["r"].get<LscPolynomial>();
-			pgr = yamlSet["gr"].get<LscPolynomial>();
-			pgb = yamlSet["gb"].get<LscPolynomial>();
-			pb = yamlSet["b"].get<LscPolynomial>();
-
-			if (!(pr || pgr || pgb || pb)) {
-				LOG(RkISP1Lsc, Error)
-					<< "Failed to parse polynomial for "
-					<< "colour temperature " << ct;
-				return -EINVAL;
-			}
-
-			pr->setReferenceImageSize(sensorSize_);
-			pgr->setReferenceImageSize(sensorSize_);
-			pgb->setReferenceImageSize(sensorSize_);
-			pb->setReferenceImageSize(sensorSize_);
-			set.r = samplePolynomial(*pr);
-			set.gr = samplePolynomial(*pgr);
-			set.gb = samplePolynomial(*pgb);
-			set.b = samplePolynomial(*pb);
+			 std::map<unsigned int, LensShadingCorrection::Components> &lscData);
+
+private:
+	std::vector<double> sizesListToPositions(const std::vector<double> &sizes);
+	std::vector<uint16_t> samplePolynomial(const LscPolynomial &poly);
+
+	Size sensorSize_;
+	Rectangle cropRectangle_;
+	const std::vector<double> &xSizes_;
+	const std::vector<double> &ySizes_;
+};
+
+int LscPolynomialLoader::parseLscData(const YamlObject &yamlSets,
+				      std::map<unsigned int, LensShadingCorrection::Components> &lscData)
+{
+	const auto &sets = yamlSets.asList();
+	for (const auto &yamlSet : sets) {
+		std::optional<LscPolynomial> pr, pgr, pgb, pb;
+		uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
+
+		if (lscData.count(ct)) {
+			LOG(RkISP1Lsc, Error)
+				<< "Multiple sets found for "
+				<< "color temperature " << ct;
+			return -EINVAL;
 		}
 
-		if (lscData.empty()) {
-			LOG(RkISP1Lsc, Error) << "Failed to load any sets";
+		LensShadingCorrection::Components &set = lscData[ct];
+		pr = yamlSet["r"].get<LscPolynomial>();
+		pgr = yamlSet["gr"].get<LscPolynomial>();
+		pgb = yamlSet["gb"].get<LscPolynomial>();
+		pb = yamlSet["b"].get<LscPolynomial>();
+
+		if (!(pr || pgr || pgb || pb)) {
+			LOG(RkISP1Lsc, Error)
+				<< "Failed to parse polynomial for "
+				<< "colour temperature " << ct;
 			return -EINVAL;
 		}
 
-		return 0;
+		pr->setReferenceImageSize(sensorSize_);
+		pgr->setReferenceImageSize(sensorSize_);
+		pgb->setReferenceImageSize(sensorSize_);
+		pb->setReferenceImageSize(sensorSize_);
+		set.r = samplePolynomial(*pr);
+		set.gr = samplePolynomial(*pgr);
+		set.gb = samplePolynomial(*pgb);
+		set.b = samplePolynomial(*pb);
 	}
 
-private:
-	/*
-	 * The rkisp1 LSC grid spacing is defined by the cell sizes on the first
-	 * half of the grid. This is then mirrored in hardware to the other
-	 * half. See parseSizes() for further details. For easier handling, this
-	 * function converts the cell sizes of half the grid to a list of
-	 * position of the whole grid (on one axis). Example:
-	 *
-	 * input:   | 0.2 | 0.3 |
-	 * output: 0.0   0.2   0.5   0.8   1.0
-	 */
-	std::vector<double> sizesListToPositions(const std::vector<double> &sizes)
-	{
-		const int half = sizes.size();
-		std::vector<double> positions(half * 2 + 1);
-		double x = 0.0;
-
-		positions[half] = 0.5;
-		for (int i = 1; i <= half; i++) {
-			x += sizes[half - i];
-			positions[half - i] = 0.5 - x;
-			positions[half + i] = 0.5 + x;
-		}
+	if (lscData.empty()) {
+		LOG(RkISP1Lsc, Error) << "Failed to load any sets";
+		return -EINVAL;
+	}
 
-		return positions;
+	return 0;
+}
+
+/*
+ * The rkisp1 LSC grid spacing is defined by the cell sizes on the first half of
+ * the grid. This is then mirrored in hardware to the other half. See
+ * parseSizes() for further details. For easier handling, this function converts
+ * the cell sizes of half the grid to a list of position of the whole grid (on
+ * one axis). Example:
+ *
+ * input:   | 0.2 | 0.3 |
+ * output: 0.0   0.2   0.5   0.8   1.0
+ */
+std::vector<double> LscPolynomialLoader::sizesListToPositions(const std::vector<double> &sizes)
+{
+	const int half = sizes.size();
+	std::vector<double> positions(half * 2 + 1);
+	double x = 0.0;
+
+	positions[half] = 0.5;
+	for (int i = 1; i <= half; i++) {
+		x += sizes[half - i];
+		positions[half - i] = 0.5 - x;
+		positions[half + i] = 0.5 + x;
 	}
 
-	std::vector<uint16_t> samplePolynomial(const LscPolynomial &poly)
-	{
-		constexpr int k = RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
-
-		double m = poly.getM();
-		double x0 = cropRectangle_.x / m;
-		double y0 = cropRectangle_.y / m;
-		double w = cropRectangle_.width / m;
-		double h = cropRectangle_.height / m;
-		std::vector<uint16_t> samples;
-
-		ASSERT(xSizes_.size() * 2 + 1 == k);
-		ASSERT(ySizes_.size() * 2 + 1 == k);
-
-		samples.reserve(k * k);
-
-		std::vector<double> xPos(sizesListToPositions(xSizes_));
-		std::vector<double> yPos(sizesListToPositions(ySizes_));
-
-		for (int y = 0; y < k; y++) {
-			for (int x = 0; x < k; x++) {
-				double xp = x0 + xPos[x] * w;
-				double yp = y0 + yPos[y] * h;
-				/*
-				 * The hardware uses 2.10 fixed point format and
-				 * limits the legal values to [1..3.999]. Scale
-				 * and clamp the sampled value accordingly.
-				 */
-				int v = static_cast<int>(
-					poly.sampleAtNormalizedPixelPos(xp, yp) *
-					1024);
-				v = std::min(std::max(v, 1024), 4095);
-				samples.push_back(v);
-			}
+	return positions;
+}
+
+std::vector<uint16_t> LscPolynomialLoader::samplePolynomial(const LscPolynomial &poly)
+{
+	constexpr int k = RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
+
+	double m = poly.getM();
+	double x0 = cropRectangle_.x / m;
+	double y0 = cropRectangle_.y / m;
+	double w = cropRectangle_.width / m;
+	double h = cropRectangle_.height / m;
+	std::vector<uint16_t> samples;
+
+	ASSERT(xSizes_.size() * 2 + 1 == k);
+	ASSERT(ySizes_.size() * 2 + 1 == k);
+
+	samples.reserve(k * k);
+
+	std::vector<double> xPos(sizesListToPositions(xSizes_));
+	std::vector<double> yPos(sizesListToPositions(ySizes_));
+
+	for (int y = 0; y < k; y++) {
+		for (int x = 0; x < k; x++) {
+			double xp = x0 + xPos[x] * w;
+			double yp = y0 + yPos[y] * h;
+			/*
+			 * The hardware uses 2.10 fixed point format and limits
+			 * the legal values to [1..3.999]. Scale and clamp the
+			 * sampled value accordingly.
+			 */
+			int v = static_cast<int>(
+				poly.sampleAtNormalizedPixelPos(xp, yp) *
+				1024);
+			v = std::min(std::max(v, 1024), 4095);
+			samples.push_back(v);
 		}
-		return samples;
 	}
-
-	Size sensorSize_;
-	Rectangle cropRectangle_;
-	const std::vector<double> &xSizes_;
-	const std::vector<double> &ySizes_;
-};
+	return samples;
+}
 
 class LscTableLoader
 {
 public:
 	int parseLscData(const YamlObject &yamlSets,
-			 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
-	{
-		const auto &sets = yamlSets.asList();
-
-		for (const auto &yamlSet : sets) {
-			uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
-
-			if (lscData.count(ct)) {
-				LOG(RkISP1Lsc, Error)
-					<< "Multiple sets found for color temperature "
-					<< ct;
-				return -EINVAL;
-			}
-
-			LensShadingCorrection::Components &set = lscData[ct];
-
-			set.r = parseTable(yamlSet, "r");
-			set.gr = parseTable(yamlSet, "gr");
-			set.gb = parseTable(yamlSet, "gb");
-			set.b = parseTable(yamlSet, "b");
-
-			if (set.r.empty() || set.gr.empty() ||
-			    set.gb.empty() || set.b.empty()) {
-				LOG(RkISP1Lsc, Error)
-					<< "Set for color temperature " << ct
-					<< " is missing tables";
-				return -EINVAL;
-			}
-		}
+			 std::map<unsigned int, LensShadingCorrection::Components> &lscData);
+
+private:
+	std::vector<uint16_t> parseTable(const YamlObject &tuningData,
+					 const char *prop);
+};
+
+int LscTableLoader::parseLscData(const YamlObject &yamlSets,
+				 std::map<unsigned int, LensShadingCorrection::Components> &lscData)
+{
+	const auto &sets = yamlSets.asList();
+
+	for (const auto &yamlSet : sets) {
+		uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
 
-		if (lscData.empty()) {
-			LOG(RkISP1Lsc, Error) << "Failed to load any sets";
+		if (lscData.count(ct)) {
+			LOG(RkISP1Lsc, Error)
+				<< "Multiple sets found for color temperature "
+				<< ct;
 			return -EINVAL;
 		}
 
-		return 0;
-	}
+		LensShadingCorrection::Components &set = lscData[ct];
 
-private:
-	std::vector<uint16_t> parseTable(const YamlObject &tuningData,
-					 const char *prop)
-	{
-		static constexpr unsigned int kLscNumSamples =
-			RKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
+		set.r = parseTable(yamlSet, "r");
+		set.gr = parseTable(yamlSet, "gr");
+		set.gb = parseTable(yamlSet, "gb");
+		set.b = parseTable(yamlSet, "b");
 
-		std::vector<uint16_t> table =
-			tuningData[prop].getList<uint16_t>().value_or(std::vector<uint16_t>{});
-		if (table.size() != kLscNumSamples) {
+		if (set.r.empty() || set.gr.empty() ||
+		    set.gb.empty() || set.b.empty()) {
 			LOG(RkISP1Lsc, Error)
-				<< "Invalid '" << prop << "' values: expected "
-				<< kLscNumSamples
-				<< " elements, got " << table.size();
-			return {};
+				<< "Set for color temperature " << ct
+				<< " is missing tables";
+			return -EINVAL;
 		}
+	}
 
-		return table;
+	if (lscData.empty()) {
+		LOG(RkISP1Lsc, Error) << "Failed to load any sets";
+		return -EINVAL;
 	}
-};
+
+	return 0;
+}
+
+std::vector<uint16_t> LscTableLoader::parseTable(const YamlObject &tuningData,
+						 const char *prop)
+{
+	static constexpr unsigned int kLscNumSamples =
+		RKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;
+
+	std::vector<uint16_t> table =
+		tuningData[prop].getList<uint16_t>().value_or(std::vector<uint16_t>{});
+	if (table.size() != kLscNumSamples) {
+		LOG(RkISP1Lsc, Error)
+			<< "Invalid '" << prop << "' values: expected "
+			<< kLscNumSamples
+			<< " elements, got " << table.size();
+		return {};
+	}
+
+	return table;
+}
 
 static std::vector<double> parseSizes(const YamlObject &tuningData,
 				      const char *prop)