[RFC,v2,17/43] ipa: libipa: agc_mean_luminance: calculateNewEv(): Return y target
diff mbox series

Message ID 20260723154327.1357866-18-barnabas.pocze@ideasonboard.com
State Superseded
Headers show
Series
  • ipa: libipa: agc rework
Related show

Commit Message

Barnabás Pőcze July 23, 2026, 3:43 p.m. UTC 
Return the current applicable y target as part of the result set.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/ipa/libipa/agc_mean_luminance.cpp | 13 ++++++++-----
 src/ipa/libipa/agc_mean_luminance.h   |  3 ++-
 src/ipa/rkisp1/algorithms/agc.cpp     |  2 +-
 3 files changed, 11 insertions(+), 7 deletions(-)

Comments

Jacopo Mondi July 24, 2026, 12:51 p.m. UTC | #1 
Hi Barnabás

On Thu, Jul 23, 2026 at 05:43:00PM +0200, Barnabás Pőcze wrote:
> Return the current applicable y target as part of the result set.

It would be nice to tell what it will be used for

Apart from that
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

>
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  src/ipa/libipa/agc_mean_luminance.cpp | 13 ++++++++-----
>  src/ipa/libipa/agc_mean_luminance.h   |  3 ++-
>  src/ipa/rkisp1/algorithms/agc.cpp     |  2 +-
>  3 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
> index 5e12c4b475..d25b1e8322 100644
> --- a/src/ipa/libipa/agc_mean_luminance.cpp
> +++ b/src/ipa/libipa/agc_mean_luminance.cpp
> @@ -502,9 +502,8 @@ void AgcMeanLuminance::setLimits(utils::Duration minExposureTime,
>   * target
>   * \return The calculated initial gain
>   */
> -double AgcMeanLuminance::estimateInitialGain(const Traits &traits) const
> +double AgcMeanLuminance::estimateInitialGain(const Traits &traits, double yTarget) const
>  {
> -	double yTarget = effectiveYTarget();
>  	double yGain = 1.0;
>
>  	/*
> @@ -671,6 +670,9 @@ utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue)
>  /**
>   * \struct AgcMeanLuminance::Result
>   * \brief Collection of results of the mean luminance AGC algorithm
> + *
> + * \var AgcMeanLuminance::Result::yTarget
> + * \brief The current y target including exposure compensation
>   */
>
>  /**
> @@ -694,6 +696,7 @@ AgcMeanLuminance::calculateNewEv(const Params &params)
>  	 */
>  	ExposureModeHelper &exposureModeHelper =
>  		exposureModeHelpers_.at(params.exposureModeIndex);
> +	double yTarget = effectiveYTarget();
>
>  	if (params.effectiveExposureValue == 0s) {
>  		LOG(AgcMeanLuminance, Error)
> @@ -704,10 +707,10 @@ AgcMeanLuminance::calculateNewEv(const Params &params)
>  		 * doesn't get stuck with 0 in case the sensor driver allows a
>  		 * min exposure of 0.
>  		 */
> -		return { exposureModeHelper.splitExposure(10ms) };
> +		return { exposureModeHelper.splitExposure(10ms), yTarget };
>  	}
>
> -	double gain = estimateInitialGain(params.traits);
> +	double gain = estimateInitialGain(params.traits, yTarget);
>  	gain = constraintClampGain(params.constraintModeIndex, params.yHist, gain);
>
>  	/*
> @@ -726,7 +729,7 @@ AgcMeanLuminance::calculateNewEv(const Params &params)
>  	newExposureValue = filterExposure(newExposureValue);
>
>  	frameCount_++;
> -	return { exposureModeHelper.splitExposure(newExposureValue) };
> +	return { exposureModeHelper.splitExposure(newExposureValue), yTarget };
>  }
>
>  /**
> diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h
> index aa4c0369ac..6418fe2e65 100644
> --- a/src/ipa/libipa/agc_mean_luminance.h
> +++ b/src/ipa/libipa/agc_mean_luminance.h
> @@ -87,6 +87,7 @@ public:
>  	};
>
>  	struct Result : ExposureModeHelper::Result {
> +		double yTarget;
>  	};
>
>  	[[nodiscard]] Result calculateNewEv(const Params &params);
> @@ -103,7 +104,7 @@ private:
>  	int parseConstraint(const ValueNode &modeDict, int32_t id);
>  	int parseConstraintModes(const ValueNode &tuningData);
>  	int parseExposureModes(const ValueNode &tuningData);
> -	double estimateInitialGain(const Traits &traits) const;
> +	double estimateInitialGain(const Traits &traits, double yTarget) const;
>  	double constraintClampGain(uint32_t constraintModeIndex,
>  				   const Histogram &hist,
>  				   double gain) const;
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index a4567c8258..8ed9d7ea6d 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -741,7 +741,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>  	activeState.agc.automatic.exposure = newEv.exposureTime / lineDuration;
>  	activeState.agc.automatic.gain = newEv.analogueGain;
>  	activeState.agc.automatic.quantizationGain = newEv.quantizationGain;
> -	activeState.agc.automatic.yTarget = agc_.effectiveYTarget();
> +	activeState.agc.automatic.yTarget = newEv.yTarget;
>  	/*
>  	 * Expand the target frame duration so that we do not run faster than
>  	 * the minimum frame duration when we have short exposures.
> --
> 2.55.0
>
Barnabás Pőcze July 24, 2026, 1:08 p.m. UTC | #2 
2026. 07. 24. 14:51 keltezéssel, Jacopo Mondi írta:
> Hi Barnabás
> 
> On Thu, Jul 23, 2026 at 05:43:00PM +0200, Barnabás Pőcze wrote:
>> Return the current applicable y target as part of the result set.
> 
> It would be nice to tell what it will be used for

I have adjusted the text:

   The rkisp1 agc algorithms needs to save the current y target for the
   wdr algorithm. Instead of having to recalculate it, just return it
   as part of the result set.


> 
> Apart from that
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> 
>>
>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>> ---
>>   src/ipa/libipa/agc_mean_luminance.cpp | 13 ++++++++-----
>>   src/ipa/libipa/agc_mean_luminance.h   |  3 ++-
>>   src/ipa/rkisp1/algorithms/agc.cpp     |  2 +-
>>   3 files changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
>> index 5e12c4b475..d25b1e8322 100644
>> --- a/src/ipa/libipa/agc_mean_luminance.cpp
>> +++ b/src/ipa/libipa/agc_mean_luminance.cpp
>> @@ -502,9 +502,8 @@ void AgcMeanLuminance::setLimits(utils::Duration minExposureTime,
>>    * target
>>    * \return The calculated initial gain
>>    */
>> -double AgcMeanLuminance::estimateInitialGain(const Traits &traits) const
>> +double AgcMeanLuminance::estimateInitialGain(const Traits &traits, double yTarget) const
>>   {
>> -	double yTarget = effectiveYTarget();
>>   	double yGain = 1.0;
>>
>>   	/*
>> @@ -671,6 +670,9 @@ utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue)
>>   /**
>>    * \struct AgcMeanLuminance::Result
>>    * \brief Collection of results of the mean luminance AGC algorithm
>> + *
>> + * \var AgcMeanLuminance::Result::yTarget
>> + * \brief The current y target including exposure compensation
>>    */
>>
>>   /**
>> @@ -694,6 +696,7 @@ AgcMeanLuminance::calculateNewEv(const Params &params)
>>   	 */
>>   	ExposureModeHelper &exposureModeHelper =
>>   		exposureModeHelpers_.at(params.exposureModeIndex);
>> +	double yTarget = effectiveYTarget();
>>
>>   	if (params.effectiveExposureValue == 0s) {
>>   		LOG(AgcMeanLuminance, Error)
>> @@ -704,10 +707,10 @@ AgcMeanLuminance::calculateNewEv(const Params &params)
>>   		 * doesn't get stuck with 0 in case the sensor driver allows a
>>   		 * min exposure of 0.
>>   		 */
>> -		return { exposureModeHelper.splitExposure(10ms) };
>> +		return { exposureModeHelper.splitExposure(10ms), yTarget };
>>   	}
>>
>> -	double gain = estimateInitialGain(params.traits);
>> +	double gain = estimateInitialGain(params.traits, yTarget);
>>   	gain = constraintClampGain(params.constraintModeIndex, params.yHist, gain);
>>
>>   	/*
>> @@ -726,7 +729,7 @@ AgcMeanLuminance::calculateNewEv(const Params &params)
>>   	newExposureValue = filterExposure(newExposureValue);
>>
>>   	frameCount_++;
>> -	return { exposureModeHelper.splitExposure(newExposureValue) };
>> +	return { exposureModeHelper.splitExposure(newExposureValue), yTarget };
>>   }
>>
>>   /**
>> diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h
>> index aa4c0369ac..6418fe2e65 100644
>> --- a/src/ipa/libipa/agc_mean_luminance.h
>> +++ b/src/ipa/libipa/agc_mean_luminance.h
>> @@ -87,6 +87,7 @@ public:
>>   	};
>>
>>   	struct Result : ExposureModeHelper::Result {
>> +		double yTarget;
>>   	};
>>
>>   	[[nodiscard]] Result calculateNewEv(const Params &params);
>> @@ -103,7 +104,7 @@ private:
>>   	int parseConstraint(const ValueNode &modeDict, int32_t id);
>>   	int parseConstraintModes(const ValueNode &tuningData);
>>   	int parseExposureModes(const ValueNode &tuningData);
>> -	double estimateInitialGain(const Traits &traits) const;
>> +	double estimateInitialGain(const Traits &traits, double yTarget) const;
>>   	double constraintClampGain(uint32_t constraintModeIndex,
>>   				   const Histogram &hist,
>>   				   double gain) const;
>> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
>> index a4567c8258..8ed9d7ea6d 100644
>> --- a/src/ipa/rkisp1/algorithms/agc.cpp
>> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
>> @@ -741,7 +741,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>>   	activeState.agc.automatic.exposure = newEv.exposureTime / lineDuration;
>>   	activeState.agc.automatic.gain = newEv.analogueGain;
>>   	activeState.agc.automatic.quantizationGain = newEv.quantizationGain;
>> -	activeState.agc.automatic.yTarget = agc_.effectiveYTarget();
>> +	activeState.agc.automatic.yTarget = newEv.yTarget;
>>   	/*
>>   	 * Expand the target frame duration so that we do not run faster than
>>   	 * the minimum frame duration when we have short exposures.
>> --
>> 2.55.0
>>

Patch
diff mbox series 

diff --git a/src/ipa/libipa/agc_mean_luminance.cpp b/src/ipa/libipa/agc_mean_luminance.cpp
index 5e12c4b475..d25b1e8322 100644
--- a/src/ipa/libipa/agc_mean_luminance.cpp
+++ b/src/ipa/libipa/agc_mean_luminance.cpp
@@ -502,9 +502,8 @@  void AgcMeanLuminance::setLimits(utils::Duration minExposureTime,
  * target
  * \return The calculated initial gain
  */
-double AgcMeanLuminance::estimateInitialGain(const Traits &traits) const
+double AgcMeanLuminance::estimateInitialGain(const Traits &traits, double yTarget) const
 {
-	double yTarget = effectiveYTarget();
 	double yGain = 1.0;
 
 	/*
@@ -671,6 +670,9 @@  utils::Duration AgcMeanLuminance::filterExposure(utils::Duration exposureValue)
 /**
  * \struct AgcMeanLuminance::Result
  * \brief Collection of results of the mean luminance AGC algorithm
+ *
+ * \var AgcMeanLuminance::Result::yTarget
+ * \brief The current y target including exposure compensation
  */
 
 /**
@@ -694,6 +696,7 @@  AgcMeanLuminance::calculateNewEv(const Params &params)
 	 */
 	ExposureModeHelper &exposureModeHelper =
 		exposureModeHelpers_.at(params.exposureModeIndex);
+	double yTarget = effectiveYTarget();
 
 	if (params.effectiveExposureValue == 0s) {
 		LOG(AgcMeanLuminance, Error)
@@ -704,10 +707,10 @@  AgcMeanLuminance::calculateNewEv(const Params &params)
 		 * doesn't get stuck with 0 in case the sensor driver allows a
 		 * min exposure of 0.
 		 */
-		return { exposureModeHelper.splitExposure(10ms) };
+		return { exposureModeHelper.splitExposure(10ms), yTarget };
 	}
 
-	double gain = estimateInitialGain(params.traits);
+	double gain = estimateInitialGain(params.traits, yTarget);
 	gain = constraintClampGain(params.constraintModeIndex, params.yHist, gain);
 
 	/*
@@ -726,7 +729,7 @@  AgcMeanLuminance::calculateNewEv(const Params &params)
 	newExposureValue = filterExposure(newExposureValue);
 
 	frameCount_++;
-	return { exposureModeHelper.splitExposure(newExposureValue) };
+	return { exposureModeHelper.splitExposure(newExposureValue), yTarget };
 }
 
 /**
diff --git a/src/ipa/libipa/agc_mean_luminance.h b/src/ipa/libipa/agc_mean_luminance.h
index aa4c0369ac..6418fe2e65 100644
--- a/src/ipa/libipa/agc_mean_luminance.h
+++ b/src/ipa/libipa/agc_mean_luminance.h
@@ -87,6 +87,7 @@  public:
 	};
 
 	struct Result : ExposureModeHelper::Result {
+		double yTarget;
 	};
 
 	[[nodiscard]] Result calculateNewEv(const Params &params);
@@ -103,7 +104,7 @@  private:
 	int parseConstraint(const ValueNode &modeDict, int32_t id);
 	int parseConstraintModes(const ValueNode &tuningData);
 	int parseExposureModes(const ValueNode &tuningData);
-	double estimateInitialGain(const Traits &traits) const;
+	double estimateInitialGain(const Traits &traits, double yTarget) const;
 	double constraintClampGain(uint32_t constraintModeIndex,
 				   const Histogram &hist,
 				   double gain) const;
diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
index a4567c8258..8ed9d7ea6d 100644
--- a/src/ipa/rkisp1/algorithms/agc.cpp
+++ b/src/ipa/rkisp1/algorithms/agc.cpp
@@ -741,7 +741,7 @@  void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
 	activeState.agc.automatic.exposure = newEv.exposureTime / lineDuration;
 	activeState.agc.automatic.gain = newEv.analogueGain;
 	activeState.agc.automatic.quantizationGain = newEv.quantizationGain;
-	activeState.agc.automatic.yTarget = agc_.effectiveYTarget();
+	activeState.agc.automatic.yTarget = newEv.yTarget;
 	/*
 	 * Expand the target frame duration so that we do not run faster than
 	 * the minimum frame duration when we have short exposures.