[libcamera-devel] [RFC PATCH 05/12] ipa: rkisp1: Rename frameContext to activeState

Umang Jain umang.jain at ideasonboard.com
Fri Jul 22 16:55:22 CEST 2022


Hi Kieran,

Thank you for the patch

On 7/21/22 17:43, Kieran Bingham via libcamera-devel wrote:
> Move the existing frame context structure to the IPAActiveState.
> This structure should store the most up to date results for the IPA
> which may be shared to other algorithms that operate on the data.
>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>


Looks good!

Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>

> ---
>   src/ipa/rkisp1/algorithms/agc.cpp | 20 +++++++++---------
>   src/ipa/rkisp1/algorithms/awb.cpp | 34 +++++++++++++++----------------
>   src/ipa/rkisp1/algorithms/blc.cpp |  2 +-
>   src/ipa/rkisp1/ipa_context.h      |  7 +++++--
>   src/ipa/rkisp1/rkisp1.cpp         | 12 +++++------
>   5 files changed, 39 insertions(+), 36 deletions(-)
>
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index a1bb7d972926..483e941fe427 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -73,8 +73,8 @@ Agc::Agc()
>   int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)
>   {
>   	/* Configure the default exposure and gain. */
> -	context.frameContext.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);
> -	context.frameContext.agc.exposure = 10ms / context.configuration.sensor.lineDuration;
> +	context.activeState.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain);
> +	context.activeState.agc.exposure = 10ms / context.configuration.sensor.lineDuration;
>   
>   	/*
>   	 * According to the RkISP1 documentation:
> @@ -98,7 +98,7 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)
>   	context.configuration.agc.measureWindow.h_size = 3 * configInfo.outputSize.width / 4;
>   	context.configuration.agc.measureWindow.v_size = 3 * configInfo.outputSize.height / 4;
>   
> -	/* \todo Use actual frame index by populating it in the frameContext. */
> +	/* \todo Use actual frame index by populating it in the activeState. */
>   	frameCount_ = 0;
>   	return 0;
>   }
> @@ -140,18 +140,18 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue)
>   
>   /**
>    * \brief Estimate the new exposure and gain values
> - * \param[inout] frameContext The shared IPA frame Context
> + * \param[inout] context The shared IPA Context
>    * \param[in] yGain The gain calculated on the current brightness level
>    * \param[in] iqMeanGain The gain calculated based on the relative luminance target
>    */
>   void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain)
>   {
>   	IPASessionConfiguration &configuration = context.configuration;
> -	IPAFrameContext &frameContext = context.frameContext;
> +	IPAActiveState &activeState = context.activeState;
>   
>   	/* Get the effective exposure and gain applied on the sensor. */
> -	uint32_t exposure = frameContext.sensor.exposure;
> -	double analogueGain = frameContext.sensor.gain;
> +	uint32_t exposure = activeState.sensor.exposure;
> +	double analogueGain = activeState.sensor.gain;
>   
>   	/* Use the highest of the two gain estimates. */
>   	double evGain = std::max(yGain, iqMeanGain);
> @@ -216,8 +216,8 @@ void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain)
>   			      << stepGain;
>   
>   	/* Update the estimated exposure and gain. */
> -	frameContext.agc.exposure = shutterTime / configuration.sensor.lineDuration;
> -	frameContext.agc.gain = stepGain;
> +	activeState.agc.exposure = shutterTime / configuration.sensor.lineDuration;
> +	activeState.agc.gain = stepGain;
>   }
>   
>   /**
> @@ -324,7 +324,7 @@ void Agc::process(IPAContext &context,
>    */
>   void Agc::prepare(IPAContext &context, rkisp1_params_cfg *params)
>   {
> -	if (context.frameContext.frameCount > 0)
> +	if (context.activeState.frameCount > 0)
>   		return;
>   
>   	/* Configure the measurement window. */
> diff --git a/src/ipa/rkisp1/algorithms/awb.cpp b/src/ipa/rkisp1/algorithms/awb.cpp
> index 9f00364d12b1..427aaeb1e955 100644
> --- a/src/ipa/rkisp1/algorithms/awb.cpp
> +++ b/src/ipa/rkisp1/algorithms/awb.cpp
> @@ -35,9 +35,9 @@ LOG_DEFINE_CATEGORY(RkISP1Awb)
>   int Awb::configure(IPAContext &context,
>   		   const IPACameraSensorInfo &configInfo)
>   {
> -	context.frameContext.awb.gains.red = 1.0;
> -	context.frameContext.awb.gains.blue = 1.0;
> -	context.frameContext.awb.gains.green = 1.0;
> +	context.activeState.awb.gains.red = 1.0;
> +	context.activeState.awb.gains.blue = 1.0;
> +	context.activeState.awb.gains.green = 1.0;
>   
>   	/*
>   	 * Define the measurement window for AWB as a centered rectangle
> @@ -72,16 +72,16 @@ uint32_t Awb::estimateCCT(double red, double green, double blue)
>    */
>   void Awb::prepare(IPAContext &context, rkisp1_params_cfg *params)
>   {
> -	params->others.awb_gain_config.gain_green_b = 256 * context.frameContext.awb.gains.green;
> -	params->others.awb_gain_config.gain_blue = 256 * context.frameContext.awb.gains.blue;
> -	params->others.awb_gain_config.gain_red = 256 * context.frameContext.awb.gains.red;
> -	params->others.awb_gain_config.gain_green_r = 256 * context.frameContext.awb.gains.green;
> +	params->others.awb_gain_config.gain_green_b = 256 * context.activeState.awb.gains.green;
> +	params->others.awb_gain_config.gain_blue = 256 * context.activeState.awb.gains.blue;
> +	params->others.awb_gain_config.gain_red = 256 * context.activeState.awb.gains.red;
> +	params->others.awb_gain_config.gain_green_r = 256 * context.activeState.awb.gains.green;
>   
>   	/* Update the gains. */
>   	params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AWB_GAIN;
>   
>   	/* If we already have configured the gains and window, return. */
> -	if (context.frameContext.frameCount > 0)
> +	if (context.activeState.frameCount > 0)
>   		return;
>   
>   	/* Configure the gains to apply. */
> @@ -125,7 +125,7 @@ void Awb::process([[maybe_unused]] IPAContext &context,
>   {
>   	const rkisp1_cif_isp_stat *params = &stats->params;
>   	const rkisp1_cif_isp_awb_stat *awb = &params->awb;
> -	IPAFrameContext &frameContext = context.frameContext;
> +	IPAActiveState &activeState = context.activeState;
>   
>   	/* Get the YCbCr mean values */
>   	double yMean = awb->awb_mean[0].mean_y_or_g;
> @@ -157,22 +157,22 @@ void Awb::process([[maybe_unused]] IPAContext &context,
>   
>   	/* Filter the values to avoid oscillations. */
>   	double speed = 0.2;
> -	redGain = speed * redGain + (1 - speed) * frameContext.awb.gains.red;
> -	blueGain = speed * blueGain + (1 - speed) * frameContext.awb.gains.blue;
> +	redGain = speed * redGain + (1 - speed) * activeState.awb.gains.red;
> +	blueGain = speed * blueGain + (1 - speed) * activeState.awb.gains.blue;
>   
>   	/*
>   	 * Gain values are unsigned integer value, range 0 to 4 with 8 bit
>   	 * fractional part.
>   	 */
> -	frameContext.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256);
> -	frameContext.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256);
> +	activeState.awb.gains.red = std::clamp(redGain, 0.0, 1023.0 / 256);
> +	activeState.awb.gains.blue = std::clamp(blueGain, 0.0, 1023.0 / 256);
>   	/* Hardcode the green gain to 1.0. */
> -	frameContext.awb.gains.green = 1.0;
> +	activeState.awb.gains.green = 1.0;
>   
> -	frameContext.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);
> +	activeState.awb.temperatureK = estimateCCT(redMean, greenMean, blueMean);
>   
> -	LOG(RkISP1Awb, Debug) << "Gain found for red: " << context.frameContext.awb.gains.red
> -			      << " and for blue: " << context.frameContext.awb.gains.blue;
> +	LOG(RkISP1Awb, Debug) << "Gain found for red: " << context.activeState.awb.gains.red
> +			      << " and for blue: " << context.activeState.awb.gains.blue;
>   }
>   
>   REGISTER_IPA_ALGORITHM(Awb, "Awb")
> diff --git a/src/ipa/rkisp1/algorithms/blc.cpp b/src/ipa/rkisp1/algorithms/blc.cpp
> index a58569fa2dc2..4d55a2d529cb 100644
> --- a/src/ipa/rkisp1/algorithms/blc.cpp
> +++ b/src/ipa/rkisp1/algorithms/blc.cpp
> @@ -68,7 +68,7 @@ int BlackLevelCorrection::init([[maybe_unused]] IPAContext &context,
>   void BlackLevelCorrection::prepare(IPAContext &context,
>   				   rkisp1_params_cfg *params)
>   {
> -	if (context.frameContext.frameCount > 0)
> +	if (context.activeState.frameCount > 0)
>   		return;
>   
>   	if (!tuningParameters_)
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index f387cacea363..341fef2e68fe 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -40,7 +40,7 @@ struct IPASessionConfiguration {
>   	} hw;
>   };
>   
> -struct IPAFrameContext {
> +struct IPAActiveState {
>   	struct {
>   		uint32_t exposure;
>   		double gain;
> @@ -64,9 +64,12 @@ struct IPAFrameContext {
>   	unsigned int frameCount;
>   };
>   
> +struct IPAFrameContext {
> +};
> +
>   struct IPAContext {
>   	IPASessionConfiguration configuration;
> -	IPAFrameContext frameContext;
> +	IPAActiveState activeState;
>   };
>   
>   } /* namespace ipa::rkisp1 */
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 21166b0f1ba6..9629ccbf4247 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -224,7 +224,7 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,
>   	context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain);
>   	context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain);
>   
> -	context_.frameContext.frameCount = 0;
> +	context_.activeState.frameCount = 0;
>   
>   	for (auto const &algo : algorithms()) {
>   		int ret = algo->configure(context_, info);
> @@ -284,7 +284,7 @@ void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)
>   		algo->prepare(context_, params);
>   
>   	paramsBufferReady.emit(frame);
> -	context_.frameContext.frameCount++;
> +	context_.activeState.frameCount++;
>   }
>   
>   void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId,
> @@ -294,9 +294,9 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId
>   		reinterpret_cast<rkisp1_stat_buffer *>(
>   			mappedBuffers_.at(bufferId).planes()[0].data());
>   
> -	context_.frameContext.sensor.exposure =
> +	context_.activeState.sensor.exposure =
>   		sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> -	context_.frameContext.sensor.gain =
> +	context_.activeState.sensor.gain =
>   		camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
>   
>   	unsigned int aeState = 0;
> @@ -311,8 +311,8 @@ void IPARkISP1::processStatsBuffer(const uint32_t frame, const uint32_t bufferId
>   
>   void IPARkISP1::setControls(unsigned int frame)
>   {
> -	uint32_t exposure = context_.frameContext.agc.exposure;
> -	uint32_t gain = camHelper_->gainCode(context_.frameContext.agc.gain);
> +	uint32_t exposure = context_.activeState.agc.exposure;
> +	uint32_t gain = camHelper_->gainCode(context_.activeState.agc.gain);
>   
>   	ControlList ctrls(ctrls_);
>   	ctrls.set(V4L2_CID_EXPOSURE, static_cast<int32_t>(exposure));


More information about the libcamera-devel mailing list