[PATCH v3 11/16] ipa: rkisp1: ccm/lsc: Fix CCM/LSC based on manual color temperature

Paul Elder paul.elder at ideasonboard.com
Wed May 7 19:00:57 CEST 2025


On Thu, Apr 03, 2025 at 05:49:16PM +0200, Stefan Klug wrote:
> In RkISP1Awb::process(), the color temperature in the active state is
> updated every time new statistics are available.  The CCM/LSC algorithms
> use that value in prepare() to update the CCM/LSC. This is not correct
> if the color temperature was specified manually and leads to visible
> flicker even when AwbEnable is set to false.
> 
> To fix that, track the auto and manual color temperature separately in
> active state. In Awb::prepare() the current frame context is updated
> with the corresponding value from active state. Change the algorithms to
> fetch the color temperature from the frame context instead of the active
> state in prepare().
> 
> Fixes: 02308809548d ("ipa: rkisp1: awb: Implement ColourTemperature control")
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder at ideasonboard.com>

> 
> ---
> 
> Changes in v2:
> - None
> 
> Changes in v3:
> - Move incorrect colour temperature metadata to separate fixup patch
> - Updated documentation
> ---
>  src/ipa/rkisp1/algorithms/awb.cpp | 16 +++++++++-------
>  src/ipa/rkisp1/algorithms/ccm.cpp |  2 +-
>  src/ipa/rkisp1/algorithms/lsc.cpp |  6 +++---
>  src/ipa/rkisp1/ipa_context.cpp    |  6 +++---
>  src/ipa/rkisp1/ipa_context.h      |  2 +-
>  5 files changed, 17 insertions(+), 15 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> index 79c4c658406d..0795b8e5b1e1 100644
> --- a/src/ipa/rkisp1/algorithms/awb.cpp
> +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> @@ -128,7 +128,8 @@ int Awb::configure(IPAContext &context,
>  	context.activeState.awb.automatic.gains =
>  		awbAlgo_->gainsFromColourTemperature(kDefaultColourTemperature);
>  	context.activeState.awb.autoEnabled = true;
> -	context.activeState.awb.temperatureK = kDefaultColourTemperature;
> +	context.activeState.awb.manual.temperatureK = kDefaultColourTemperature;
> +	context.activeState.awb.automatic.temperatureK = kDefaultColourTemperature;
>  
>  	/*
>  	 * Define the measurement window for AWB as a centered rectangle
> @@ -185,7 +186,7 @@ void Awb::queueRequest(IPAContext &context,
>  		const auto &gains = awbAlgo_->gainsFromColourTemperature(*colourTemperature);
>  		awb.manual.gains.r() = gains.r();
>  		awb.manual.gains.b() = gains.b();
> -		awb.temperatureK = *colourTemperature;
> +		awb.manual.temperatureK = *colourTemperature;
>  		update = true;
>  	}
>  
> @@ -194,7 +195,7 @@ void Awb::queueRequest(IPAContext &context,
>  			<< "Set colour gains to " << awb.manual.gains;
>  
>  	frameContext.awb.gains = awb.manual.gains;
> -	frameContext.awb.temperatureK = awb.temperatureK;
> +	frameContext.awb.temperatureK = awb.manual.temperatureK;
>  }
>  
>  /**
> @@ -208,8 +209,9 @@ void Awb::prepare(IPAContext &context, const uint32_t frame,
>  	 * most up-to-date automatic values we can read.
>  	 */
>  	if (frameContext.awb.autoEnabled) {
> -		frameContext.awb.gains = context.activeState.awb.automatic.gains;
> -		frameContext.awb.temperatureK = context.activeState.awb.temperatureK;
> +		auto &awb = context.activeState.awb;
> +		frameContext.awb.gains = awb.automatic.gains;
> +		frameContext.awb.temperatureK = awb.automatic.temperatureK;
>  	}
>  
>  	auto gainConfig = params->block<BlockType::AwbGain>();
> @@ -309,7 +311,7 @@ void Awb::process(IPAContext &context,
>  	RkISP1AwbStats awbStats{ rgbMeans };
>  	AwbResult awbResult = awbAlgo_->calculateAwb(awbStats, frameContext.lux.lux);
>  
> -	activeState.awb.temperatureK = awbResult.colourTemperature;
> +	activeState.awb.automatic.temperatureK = awbResult.colourTemperature;
>  
>  	/*
>  	 * Clamp the gain values to the hardware, which expresses gains as Q2.8
> @@ -330,7 +332,7 @@ void Awb::process(IPAContext &context,
>  		<< std::showpoint
>  		<< "Means " << rgbMeans << ", gains "
>  		<< activeState.awb.automatic.gains << ", temp "
> -		<< activeState.awb.temperatureK << "K";
> +		<< activeState.awb.automatic.temperatureK << "K";
>  }
>  
>  RGB<double> Awb::calculateRgbMeans(const IPAFrameContext &frameContext, const rkisp1_cif_isp_awb_stat *awb) const
> diff --git a/src/ipa/rkisp1/algorithms/ccm.cpp b/src/ipa/rkisp1/algorithms/ccm.cpp
> index eb8ca39e56a8..2e5e91006b55 100644
> --- a/src/ipa/rkisp1/algorithms/ccm.cpp
> +++ b/src/ipa/rkisp1/algorithms/ccm.cpp
> @@ -88,7 +88,7 @@ void Ccm::setParameters(struct rkisp1_cif_isp_ctk_config &config,
>  void Ccm::prepare(IPAContext &context, const uint32_t frame,
>  		  IPAFrameContext &frameContext, RkISP1Params *params)
>  {
> -	uint32_t ct = context.activeState.awb.temperatureK;
> +	uint32_t ct = frameContext.awb.temperatureK;
>  
>  	/*
>  	 * \todo The colour temperature will likely be noisy, add filtering to
> diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
> index e47aa2f0727e..e7301bfec863 100644
> --- a/src/ipa/rkisp1/algorithms/lsc.cpp
> +++ b/src/ipa/rkisp1/algorithms/lsc.cpp
> @@ -404,12 +404,12 @@ void LensShadingCorrection::copyTable(rkisp1_cif_isp_lsc_config &config,
>  /**
>   * \copydoc libcamera::ipa::Algorithm::prepare
>   */
> -void LensShadingCorrection::prepare(IPAContext &context,
> +void LensShadingCorrection::prepare([[maybe_unused]] IPAContext &context,
>  				    [[maybe_unused]] const uint32_t frame,
> -				    [[maybe_unused]] IPAFrameContext &frameContext,
> +				    IPAFrameContext &frameContext,
>  				    RkISP1Params *params)
>  {
> -	uint32_t ct = context.activeState.awb.temperatureK;
> +	uint32_t ct = frameContext.awb.temperatureK;
>  	if (std::abs(static_cast<int>(ct) - static_cast<int>(lastAppliedCt_)) <
>  	    kColourTemperatureChangeThreshhold)
>  		return;
> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> index 39b97d143e95..7bc42e6de415 100644
> --- a/src/ipa/rkisp1/ipa_context.cpp
> +++ b/src/ipa/rkisp1/ipa_context.cpp
> @@ -197,15 +197,15 @@ namespace libcamera::ipa::rkisp1 {
>   * \struct IPAActiveState::awb::AwbState.gains
>   * \brief White balance gains
>   *
> + * \var IPAActiveState::awb::AwbState.temperatureK
> + * \brief Estimated color temperature
> + *
>   * \var IPAActiveState::awb.manual
>   * \brief Manual regulation state (set through requests)
>   *
>   * \var IPAActiveState::awb.automatic
>   * \brief Automatic regulation state (computed by the algorithm)
>   *
> - * \var IPAActiveState::awb.temperatureK
> - * \brief Estimated color temperature
> - *
>   * \var IPAActiveState::awb.autoEnabled
>   * \brief Whether the Auto White Balance algorithm is enabled
>   */
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index 6bc922a82971..769e9f114e23 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -91,12 +91,12 @@ struct IPAActiveState {
>  	struct {
>  		struct AwbState {
>  			RGB<double> gains;
> +			unsigned int temperatureK;
>  		};
>  
>  		AwbState manual;
>  		AwbState automatic;
>  
> -		unsigned int temperatureK;
>  		bool autoEnabled;
>  	} awb;
>  
> -- 
> 2.43.0
> 


More information about the libcamera-devel mailing list