[v3,5/9] ipa: rkisp1: Use interpolator in lsc

21300 diff mbox series

Now, that the generic interpolator is available, use it to do the
interpolation of the lens shading tables. This makes the algorithm
easier to read and remove some duplicate code.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

---

Changes in v3:
- Collected tags
- Fixed a comment
---
 src/ipa/rkisp1/algorithms/lsc.cpp | 166 +++++++++---------------------
 src/ipa/rkisp1/algorithms/lsc.h   |  13 +--
 2 files changed, 57 insertions(+), 122 deletions(-)

On Fri, Sep 20, 2024 at 03:39:20PM +0200, Stefan Klug wrote:
> Now, that the generic interpolator is available, use it to do the
> interpolation of the lens shading tables. This makes the algorithm
> easier to read and remove some duplicate code.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> ---
> 
> Changes in v3:
> - Collected tags
> - Fixed a comment
> ---
>  src/ipa/rkisp1/algorithms/lsc.cpp | 166 +++++++++---------------------
>  src/ipa/rkisp1/algorithms/lsc.h   |  13 +--
>  2 files changed, 57 insertions(+), 122 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
> index 5f3a0388075b..fe84062bbc70 100644
> --- a/src/ipa/rkisp1/algorithms/lsc.cpp
> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp
> @@ -24,6 +24,36 @@
>  
>  namespace libcamera {
>  
> +namespace ipa {
> +
> +constexpr int kColourTemperatureChangeThreshhold = 10;
> +
> +template<typename T>
> +void interpolateVector(const std::vector<T> &a, const std::vector<T> &b,
> +		       std::vector<T> &dest, double lambda)
> +{
> +	assert(a.size() == b.size());

Use ASSERT(), not assert(). Same for the whole patch series.

> +	dest.resize(a.size());
> +	for (size_t i = 0; i < a.size(); i++) {
> +		dest[i] = a[i] * (1.0 - lambda) + b[i] * lambda;
> +	}

No need for curly braces.

> +}
> +
> +template<>
> +void Interpolator<rkisp1::algorithms::LensShadingCorrection::Components>::
> +	interpolate(const rkisp1::algorithms::LensShadingCorrection::Components &a,
> +		    const rkisp1::algorithms::LensShadingCorrection::Components &b,
> +		    rkisp1::algorithms::LensShadingCorrection::Components &dest,
> +		    double lambda)
> +{
> +	interpolateVector(a.r, b.r, dest.r, lambda);
> +	interpolateVector(a.gr, b.gr, dest.gr, lambda);
> +	interpolateVector(a.gb, b.gb, dest.gb, lambda);
> +	interpolateVector(a.b, b.b, dest.b, lambda);
> +}

Sounds like there may be an opportunity to refactor the Components
structure, but that's probably for later.

> +
> +} /* namespace ipa */
> +
>  namespace ipa::rkisp1::algorithms {
>  
>  /**
> @@ -90,8 +120,9 @@ static std::vector<uint16_t> parseTable(const YamlObject &tuningData,
>  }
>  
>  LensShadingCorrection::LensShadingCorrection()
> -	: lastCt_({ 0, 0 })
> +	: lastAppliedCt_(0), lastAppliedQuantizedCt_(0)
>  {
> +	sets_.setQuantization(kColourTemperatureChangeThreshhold);
>  }
>  
>  /**
> @@ -115,17 +146,18 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
>  	}
>  
>  	const auto &sets = yamlSets.asList();
> +	std::map<unsigned int, Components> lscData;
>  	for (const auto &yamlSet : sets) {
>  		uint32_t ct = yamlSet["ct"].get<uint32_t>(0);
>  
> -		if (sets_.count(ct)) {
> +		if (lscData.count(ct)) {
>  			LOG(RkISP1Lsc, Error)
>  				<< "Multiple sets found for color temperature "
>  				<< ct;
>  			return -EINVAL;
>  		}
>  
> -		Components &set = sets_[ct];
> +		Components &set = lscData[ct];
>  
>  		set.ct = ct;
>  		set.r = parseTable(yamlSet, "r");
> @@ -142,11 +174,13 @@ int LensShadingCorrection::init([[maybe_unused]] IPAContext &context,
>  		}
>  	}
>  
> -	if (sets_.empty()) {
> +	if (lscData.empty()) {
>  		LOG(RkISP1Lsc, Error) << "Failed to load any sets";
>  		return -EINVAL;
>  	}
>  
> +	sets_.setData(std::move(lscData));
> +
>  	return 0;
>  }
>  
> @@ -202,134 +236,34 @@ void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config,
>  	std::copy(set.b.begin(), set.b.end(), &config.b_data_tbl[0][0]);
>  }
>  
> -/*
> - * Interpolate LSC parameters based on color temperature value.
> - */
> -void LensShadingCorrection::interpolateTable(rkisp1_cif_isp_lsc_config &config,
> -					     const Components &set0,
> -					     const Components &set1,
> -					     const uint32_t ct)
> -{
> -	double coeff0 = (set1.ct - ct) / static_cast<double>(set1.ct - set0.ct);
> -	double coeff1 = (ct - set0.ct) / static_cast<double>(set1.ct - set0.ct);
> -
> -	for (unsigned int i = 0; i < RKISP1_CIF_ISP_LSC_SAMPLES_MAX; ++i) {
> -		for (unsigned int j = 0; j < RKISP1_CIF_ISP_LSC_SAMPLES_MAX; ++j) {
> -			unsigned int sample = i * RKISP1_CIF_ISP_LSC_SAMPLES_MAX + j;
> -
> -			config.r_data_tbl[i][j] =
> -				set0.r[sample] * coeff0 +
> -				set1.r[sample] * coeff1;
> -
> -			config.gr_data_tbl[i][j] =
> -				set0.gr[sample] * coeff0 +
> -				set1.gr[sample] * coeff1;
> -
> -			config.gb_data_tbl[i][j] =
> -				set0.gb[sample] * coeff0 +
> -				set1.gb[sample] * coeff1;
> -
> -			config.b_data_tbl[i][j] =
> -				set0.b[sample] * coeff0 +
> -				set1.b[sample] * coeff1;
> -		}
> -	}
> -}
> -
>  /**
>   * \copydoc libcamera::ipa::Algorithm::prepare
>   */
>  void LensShadingCorrection::prepare(IPAContext &context,
> -				    const uint32_t frame,
> +				    [[maybe_unused]] const uint32_t frame,
>  				    [[maybe_unused]] IPAFrameContext &frameContext,
>  				    RkISP1Params *params)
>  {
> -	/*
> -	 * If there is only one set, the configuration has already been done
> -	 * for first frame.
> -	 */
> -	if (sets_.size() == 1 && frame > 0)
> -		return;
> -
> -	/*
> -	 * If there is only one set, pick it. We can ignore lastCt_, as it will
> -	 * never be relevant.
> -	 */
> -	if (sets_.size() == 1) {
> -		auto config = params->block<BlockType::Lsc>();
> -		config.setEnabled(true);
> -
> -		setParameters(*config);
> -		copyTable(*config, sets_.cbegin()->second);
> -		return;
> -	}
> -
>  	uint32_t ct = context.activeState.awb.temperatureK;
> -	ct = std::clamp(ct, sets_.cbegin()->first, sets_.crbegin()->first);
> -
> -	/*
> -	 * If the original is the same, then it means the same adjustment would
> -	 * be made. If the adjusted is the same, then it means that it's the
> -	 * same as what was actually applied. Thus in these cases we can skip
> -	 * reprogramming the LSC.
> -	 *
> -	 * original == adjusted can only happen if an interpolation
> -	 * happened, or if original has an exact entry in sets_. This means
> -	 * that if original != adjusted, then original was adjusted to
> -	 * the nearest available entry in sets_, resulting in adjusted.
> -	 * Clearly, any ct value that is in between original and adjusted
> -	 * will be adjusted to the same adjusted value, so we can skip
> -	 * reprogramming the LSC table.
> -	 *
> -	 * We also skip updating the original value, as the last one had a
> -	 * larger bound and thus a larger range of ct values that will be
> -	 * adjusted to the same adjusted.
> -	 */
> -	if ((lastCt_.original <= ct && ct <= lastCt_.adjusted) ||
> -	    (lastCt_.adjusted <= ct && ct <= lastCt_.original))
> +	if (std::abs(static_cast<int>(ct) - static_cast<int>(lastAppliedCt_)) <
> +	    kColourTemperatureChangeThreshhold)
> +		return;
> +	unsigned int quantizedCt;
> +	const Components &set = sets_.getInterpolated(ct, &quantizedCt);
> +	if (lastAppliedQuantizedCt_ == quantizedCt)
>  		return;
>  
>  	auto config = params->block<BlockType::Lsc>();
>  	config.setEnabled(true);
>  	setParameters(*config);
> +	copyTable(*config, set);
>  
> -	/*
> -	 * The color temperature matches exactly one of the available LSC tables.
> -	 */
> -	if (sets_.count(ct)) {
> -		copyTable(*config, sets_[ct]);
> -		lastCt_ = { ct, ct };
> -		return;
> -	}
> +	lastAppliedCt_ = ct;
> +	lastAppliedQuantizedCt_ = quantizedCt;
>  
> -	/* No shortcuts left; we need to round or interpolate */
> -	auto iter = sets_.upper_bound(ct);
> -	const Components &set1 = iter->second;
> -	const Components &set0 = (--iter)->second;
> -	uint32_t ct0 = set0.ct;
> -	uint32_t ct1 = set1.ct;
> -	uint32_t diff0 = ct - ct0;
> -	uint32_t diff1 = ct1 - ct;
> -	static constexpr double kThreshold = 0.1;
> -	float threshold = kThreshold * (ct1 - ct0);
> -
> -	if (diff0 < threshold || diff1 < threshold) {
> -		const Components &set = diff0 < diff1 ? set0 : set1;
> -		LOG(RkISP1Lsc, Debug) << "using LSC table for " << set.ct;
> -		copyTable(*config, set);
> -		lastCt_ = { ct, set.ct };
> -		return;
> -	}
> -
> -	/*
> -	 * ct is not within 10% of the difference between the neighbouring
> -	 * color temperatures, so we need to interpolate.
> -	 */
>  	LOG(RkISP1Lsc, Debug)
> -		<< "ct is " << ct << ", interpolating between "
> -		<< ct0 << " and " << ct1;
> -	interpolateTable(*config, set0, set1, ct);
> -	lastCt_ = { ct, ct };
> +		<< "ct is " << ct << ", quantized to "
> +		<< quantizedCt;
>  }
>  
>  REGISTER_IPA_ALGORITHM(LensShadingCorrection, "LensShadingCorrection")
> diff --git a/src/ipa/rkisp1/algorithms/lsc.h b/src/ipa/rkisp1/algorithms/lsc.h
> index a9c7a230e0fc..5a0824e36dd5 100644
> --- a/src/ipa/rkisp1/algorithms/lsc.h
> +++ b/src/ipa/rkisp1/algorithms/lsc.h
> @@ -9,6 +9,8 @@
>  
>  #include <map>
>  
> +#include "libipa/interpolator.h"
> +
>  #include "algorithm.h"
>  
>  namespace libcamera {
> @@ -27,7 +29,6 @@ public:
>  		     IPAFrameContext &frameContext,
>  		     RkISP1Params *params) override;
>  
> -private:
>  	struct Components {
>  		uint32_t ct;
>  		std::vector<uint16_t> r;
> @@ -36,23 +37,23 @@ private:
>  		std::vector<uint16_t> b;
>  	};
>  
> +private:
>  	void setParameters(rkisp1_cif_isp_lsc_config &config);
>  	void copyTable(rkisp1_cif_isp_lsc_config &config, const Components &set0);
>  	void interpolateTable(rkisp1_cif_isp_lsc_config &config,
>  			      const Components &set0, const Components &set1,
>  			      const uint32_t ct);
>  
> -	std::map<uint32_t, Components> sets_;
> +	ipa::Interpolator<Components> sets_;
>  	std::vector<double> xSize_;
>  	std::vector<double> ySize_;
>  	uint16_t xGrad_[RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE];
>  	uint16_t yGrad_[RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE];
>  	uint16_t xSizes_[RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE];
>  	uint16_t ySizes_[RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE];
> -	struct {
> -		uint32_t original;
> -		uint32_t adjusted;
> -	} lastCt_;
> +
> +	unsigned int lastAppliedCt_;
> +	unsigned int lastAppliedQuantizedCt_;
>  };
>  
>  } /* namespace ipa::rkisp1::algorithms */