[PATCH 4/5] ipa: libipa: agc: Change luminance target to piecewise linear function

[Stefan Klug]

Hi Paul,

thanks for the patch.

On Fri, Apr 05, 2024 at 11:47:28PM +0900, Paul Elder wrote:
> Change the relative luminance target from a scalar valur to a piecewise
> linear function that needs to be sampled by the estimate lux value.
> 
> Also chagne the rkisp1 and ipu3 IPAs according, as they use the libipa

s/chagne/change/

> agc. As they both don't yet have lux modules, hardcode them to a single
> lux value for now.
> 
> This affects the format of the tuning files, but as there aren't yet any
> this shouldn't be an issue.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>  src/ipa/ipu3/algorithms/agc.cpp   |  5 ++++-
>  src/ipa/libipa/agc.cpp            | 22 +++++++++++++++-------
>  src/ipa/libipa/agc.h              |  7 ++++---
>  src/ipa/rkisp1/algorithms/agc.cpp |  5 ++++-
>  4 files changed, 27 insertions(+), 12 deletions(-)
> 
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index 08deff0c..8e07c89e 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -228,12 +228,15 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>  	double analogueGain = frameContext.sensor.gain;
>  	utils::Duration effectiveExposureValue = exposureTime * analogueGain;
>  
> +	/* \todo Plumb in the lux value. Requires a lux algo + tuning */
> +	double lux = 400;
> +
>  	utils::Duration shutterTime;
>  	double aGain, dGain;
>  	std::tie(shutterTime, aGain, dGain) =
>  		calculateNewEv(context.activeState.agc.constraintMode,
>  			       context.activeState.agc.exposureMode, hist_,
> -			       effectiveExposureValue);
> +			       effectiveExposureValue, lux);
>  
>  	LOG(IPU3Agc, Debug)
>  		<< "Divided up shutter, analogue gain and digital gain are "
> diff --git a/src/ipa/libipa/agc.cpp b/src/ipa/libipa/agc.cpp
> index af57a571..bcb036e6 100644
> --- a/src/ipa/libipa/agc.cpp
> +++ b/src/ipa/libipa/agc.cpp
> @@ -110,7 +110,7 @@ static constexpr double kDefaultRelativeLuminanceTarget = 0.16;
>   */
>  
>  MeanLuminanceAgc::MeanLuminanceAgc()
> -	: frameCount_(0), filteredExposure_(0s), relativeLuminanceTarget_(0)
> +	: frameCount_(0), filteredExposure_(0s)
>  {
>  }
>  
> @@ -120,8 +120,12 @@ MeanLuminanceAgc::MeanLuminanceAgc()
>   */
>  void MeanLuminanceAgc::parseRelativeLuminanceTarget(const YamlObject &tuningData)
>  {
> -	relativeLuminanceTarget_ =
> -		tuningData["relativeLuminanceTarget"].get<double>(kDefaultRelativeLuminanceTarget);
> +	int ret = relativeLuminanceTarget_.readYaml(tuningData["relativeLuminanceTarget"]);
> +	if (ret == 0)
> +		return;

Maybe a warning would be nice, that we fall back to the default value.

> +
> +	std::vector<FPoint> points = { { 0, kDefaultRelativeLuminanceTarget } };
> +	relativeLuminanceTarget_ = Pwl(points);
>  }
>  
>  /**
> @@ -378,6 +382,7 @@ int MeanLuminanceAgc::parseExposureModes(const YamlObject &tuningData)
>  /**
>   * \brief Estimate the initial gain needed to achieve a relative luminance
>   *        target
> + * \param[in] lux The lux value at which to sample the luminance target pwl
>   *
>   * To account for non-linearity caused by saturation, the value needs to be
>   * estimated in an iterative process, as multiplying by a gain will not increase
> @@ -385,9 +390,10 @@ int MeanLuminanceAgc::parseExposureModes(const YamlObject &tuningData)
>   *
>   * \return The calculated initial gain
>   */
> -double MeanLuminanceAgc::estimateInitialGain()
> +double MeanLuminanceAgc::estimateInitialGain(double lux)
>  {
> -	double yTarget = relativeLuminanceTarget_;
> +	double yTarget =
> +		relativeLuminanceTarget_.eval(relativeLuminanceTarget_.domain().clamp(lux));
>  	double yGain = 1.0;
>  
>  	for (unsigned int i = 0; i < 8; i++) {
> @@ -476,6 +482,7 @@ utils::Duration MeanLuminanceAgc::filterExposure(utils::Duration exposureValue)
>   *	      the calculated gain
>   * \param[in] effectiveExposureValue The EV applied to the frame from which the
>   *	      statistics in use derive
> + * \param[in] lux The lux value at which to sample the luminance target pwl
>   *
>   * Calculate a new exposure value to try to obtain the target. The calculated
>   * exposure value is filtered to prevent rapid changes from frame to frame, and
> @@ -487,7 +494,8 @@ std::tuple<utils::Duration, double, double>
>  MeanLuminanceAgc::calculateNewEv(uint32_t constraintModeIndex,
>  				 uint32_t exposureModeIndex,
>  				 const Histogram &yHist,
> -				 utils::Duration effectiveExposureValue)
> +				 utils::Duration effectiveExposureValue,
> +				 double lux)
>  {
>  	/*
>  	 * The pipeline handler should validate that we have received an allowed
> @@ -496,7 +504,7 @@ MeanLuminanceAgc::calculateNewEv(uint32_t constraintModeIndex,
>  	std::shared_ptr<ExposureModeHelper> exposureModeHelper =
>  		exposureModeHelpers_.at(exposureModeIndex);
>  
> -	double gain = estimateInitialGain();
> +	double gain = estimateInitialGain(lux);
>  	gain = constraintClampGain(constraintModeIndex, yHist, gain);
>  
>  	/*
> diff --git a/src/ipa/libipa/agc.h b/src/ipa/libipa/agc.h
> index 902a359a..f187dbc8 100644
> --- a/src/ipa/libipa/agc.h
> +++ b/src/ipa/libipa/agc.h
> @@ -16,6 +16,7 @@
>  
>  #include "exposure_mode_helper.h"
>  #include "histogram.h"
> +#include "pwl.h"
>  
>  namespace libcamera {
>  
> @@ -59,18 +60,18 @@ public:
>  	}
>  
>  	virtual double estimateLuminance(const double gain) = 0;
> -	double estimateInitialGain();
> +	double estimateInitialGain(double lux);
>  	double constraintClampGain(uint32_t constraintModeIndex,
>  				   const Histogram &hist,
>  				   double gain);
>  	utils::Duration filterExposure(utils::Duration exposureValue);
>  	std::tuple<utils::Duration, double, double>
>  	calculateNewEv(uint32_t constraintModeIndex, uint32_t exposureModeIndex,
> -		       const Histogram &yHist, utils::Duration effectiveExposureValue);
> +		       const Histogram &yHist, utils::Duration effectiveExposureValue, double lux);
>  private:
>  	uint64_t frameCount_;
>  	utils::Duration filteredExposure_;
> -	double relativeLuminanceTarget_;
> +	Pwl relativeLuminanceTarget_;
>  
>  	std::map<int32_t, std::vector<AgcConstraint>> constraintModes_;
>  	std::map<int32_t, std::shared_ptr<ExposureModeHelper>> exposureModeHelpers_;
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 1dfc4aaa..a1b6eb39 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -389,12 +389,15 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>  	double analogueGain = frameContext.sensor.gain;
>  	utils::Duration effectiveExposureValue = exposureTime * analogueGain;
>  
> +	/* \todo Plumb in the lux value. Requires a lux algo + tuning */
> +	double lux = 400;
> +
>  	utils::Duration shutterTime;
>  	double aGain, dGain;
>  	std::tie(shutterTime, aGain, dGain) =
>  		calculateNewEv(context.activeState.agc.constraintMode,
>  			       context.activeState.agc.exposureMode, hist_,
> -			       effectiveExposureValue);
> +			       effectiveExposureValue, lux);

I don't yet fully understand the benefits of the lux algo. But otherwise
it looks reasonable to me.

Reviewed-by: Stefan Klug <stefan.klug at ideasonboard.com> 

Cheers,
Stefan

>  
>  	LOG(RkISP1Agc, Debug)
>  		<< "Divided up shutter, analogue gain and digital gain are "
> -- 
> 2.39.2
>