[libcamera-devel,v2,3/4] ipa: rkisp1: agc: Introduce histogram calculation

15374 diff mbox series

As for the IPU3, we can estimate the histogram of the luminance. The
RkISP1 can estimate multiple ones, the R, G and B ones, the Y only one
and a combination of RGB. The one we are interested by in AGC is the Y
histogram.

Use the hardware revision to determine the number of bins of the
produced histogram, and use it to populate a vector passed down to the
libipa::Histogram class.

As the sensor deGamma and AWB are applied, we also need to get back to
the relative luminance value of 0.16, as for the IPU3.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Tested-by: Peter Griffin <peter.griffin@linaro.org>
---
 src/ipa/rkisp1/algorithms/agc.cpp | 97 ++++++++++++++++++++++++++-----
 src/ipa/rkisp1/algorithms/agc.h   |  4 +-
 src/ipa/rkisp1/ipa_context.cpp    |  3 +
 src/ipa/rkisp1/ipa_context.h      |  1 +
 4 files changed, 89 insertions(+), 16 deletions(-)

Hi Jean-Michel,

Thank you for the patch.

On Wed, Feb 23, 2022 at 11:48:23AM +0100, Jean-Michel Hautbois wrote:
> As for the IPU3, we can estimate the histogram of the luminance. The
> RkISP1 can estimate multiple ones, the R, G and B ones, the Y only one
> and a combination of RGB. The one we are interested by in AGC is the Y
> histogram.

When reading the commit message (including the subject) I thought you
were only calculating a histogram, possibly to be used by AWB in patch
4/4. Then I realized the main purpose of the patch is to introduce a
second gain calculation method. The commit message should explain this.

> Use the hardware revision to determine the number of bins of the
> produced histogram, and use it to populate a vector passed down to the
> libipa::Histogram class.

The second part will need to be updated, see below.

> As the sensor deGamma and AWB are applied, we also need to get back to
> the relative luminance value of 0.16, as for the IPU3.

They're not yet in this patch, and degamma is actually dropped from this
series compared to v1. Does AWB result in a need for the relative
luminance target to go from 0.4 to 0.16 ? That's a big change. I'm not
saying the new value is wrong, but I'm not sure we understand the
reason.

> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Tested-by: Peter Griffin <peter.griffin@linaro.org>
> ---
>  src/ipa/rkisp1/algorithms/agc.cpp | 97 ++++++++++++++++++++++++++-----
>  src/ipa/rkisp1/algorithms/agc.h   |  4 +-
>  src/ipa/rkisp1/ipa_context.cpp    |  3 +
>  src/ipa/rkisp1/ipa_context.h      |  1 +
>  4 files changed, 89 insertions(+), 16 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 50980835..0311ecf4 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -16,6 +16,8 @@
>  
>  #include <libcamera/ipa/core_ipa_interface.h>
>  
> +#include "libipa/histogram.h"
> +
>  /**
>   * \file agc.h
>   */
> @@ -43,6 +45,9 @@ static constexpr utils::Duration kMaxShutterSpeed = 60ms;
>  /* Number of frames to wait before calculating stats on minimum exposure */
>  static constexpr uint32_t kNumStartupFrames = 10;
>  
> +/* Target value to reach for the top 2% of the histogram */
> +static constexpr double kEvGainTarget = 0.5;
> +
>  /*
>   * Relative luminance target.
>   *
> @@ -51,10 +56,10 @@ static constexpr uint32_t kNumStartupFrames = 10;
>   *
>   * \todo Why is the value different between IPU3 and RkISP1 ?
>   */
> -static constexpr double kRelativeLuminanceTarget = 0.4;
> +static constexpr double kRelativeLuminanceTarget = 0.16;
>  
>  Agc::Agc()
> -	: frameCount_(0), numCells_(0), filteredExposure_(0s)
> +	: frameCount_(0), numCells_(0), numHistBins_(0), filteredExposure_(0s)
>  {
>  }
>  
> @@ -65,8 +70,7 @@ Agc::Agc()
>   *
>   * \return 0
>   */
> -int Agc::configure(IPAContext &context,
> -		   [[maybe_unused]] const IPACameraSensorInfo &configInfo)
> +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);
> @@ -77,10 +81,19 @@ int Agc::configure(IPAContext &context,
>  	 * - versions < V12 have RKISP1_CIF_ISP_AE_MEAN_MAX_V10 entries,
>  	 * - versions >= V12 have RKISP1_CIF_ISP_AE_MEAN_MAX_V12 entries.
>  	 */
> -	if (context.configuration.hw.revision < RKISP1_V12)
> +	if (context.configuration.hw.revision < RKISP1_V12) {
>  		numCells_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V10;
> -	else
> +		numHistBins_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V10;
> +	} else {
>  		numCells_ = RKISP1_CIF_ISP_AE_MEAN_MAX_V12;
> +		numHistBins_ = RKISP1_CIF_ISP_HIST_BIN_N_MAX_V12;
> +	}
> +
> +	/* Define the measurement window for AGC. */

Let's expand this:

	/*
	 * Define the measurement window for AGC as a centered rectangle
	 * covering 3/4 of the image width and height.
	 */

(assuming I understand correctly what this does :-))

> +	context.configuration.agc.measureWindow.h_offs = configInfo.outputSize.width / 8;
> +	context.configuration.agc.measureWindow.v_offs = configInfo.outputSize.height / 8;
> +	context.configuration.agc.measureWindow.h_size = 3 * configInfo.outputSize.width / 4;
> +	context.configuration.agc.measureWindow.v_size = 3 * configInfo.outputSize.height / 4;
>  
>  	return 0;
>  }
> @@ -125,7 +138,7 @@ utils::Duration Agc::filterExposure(utils::Duration exposureValue)
>   * \param[inout] frameContext The shared IPA frame Context
>   * \param[in] yGain The gain calculated on the current brightness level

Missing documentation for iqMeanGain. You can copy it from the IPU3 IPA.

>   */
> -void Agc::computeExposure(IPAContext &context, double yGain)
> +void Agc::computeExposure(IPAContext &context, double yGain, double iqMeanGain)
>  {
>  	IPASessionConfiguration &configuration = context.configuration;
>  	IPAFrameContext &frameContext = context.frameContext;
> @@ -134,6 +147,9 @@ void Agc::computeExposure(IPAContext &context, double yGain)
>  	uint32_t exposure = frameContext.sensor.exposure;
>  	double analogueGain = frameContext.sensor.gain;
>  
> +	/* Use the highest of the two gain estimates. */
> +	double evGain = std::max(yGain, iqMeanGain);
> +
>  	utils::Duration minShutterSpeed = configuration.agc.minShutterSpeed;
>  	utils::Duration maxShutterSpeed = std::min(configuration.agc.maxShutterSpeed,
>  						   kMaxShutterSpeed);
> @@ -144,7 +160,7 @@ void Agc::computeExposure(IPAContext &context, double yGain)
>  					  kMaxAnalogueGain);
>  
>  	/* Consider within 1% of the target as correctly exposed. */
> -	if (utils::abs_diff(yGain, 1.0) < 0.01)
> +	if (utils::abs_diff(evGain, 1.0) < 0.01)
>  		return;
>  
>  	/* extracted from Rpi::Agc::computeTargetExposure. */
> @@ -161,13 +177,13 @@ void Agc::computeExposure(IPAContext &context, double yGain)
>  	LOG(RkISP1Agc, Debug) << "Actual total exposure " << currentShutter * analogueGain
>  			      << " Shutter speed " << currentShutter
>  			      << " Gain " << analogueGain
> -			      << " Needed ev gain " << yGain;
> +			      << " Needed ev gain " << evGain;
>  
>  	/*
>  	 * Calculate the current exposure value for the scene as the latest
>  	 * exposure value applied multiplied by the new estimated gain.
>  	 */
> -	utils::Duration exposureValue = effectiveExposureValue * yGain;
> +	utils::Duration exposureValue = effectiveExposureValue * evGain;
>  
>  	/* Clamp the exposure value to the min and max authorized. */
>  	utils::Duration maxTotalExposure = maxShutterSpeed * maxAnalogueGain;
> @@ -238,6 +254,23 @@ double Agc::estimateLuminance(const rkisp1_cif_isp_ae_stat *ae,
>  	return ySum / numCells_ / 255;
>  }
>  
> +/**
> + * \brief Estimate the mean value of the top 2% of the histogram
> + * \param[in] hist The histogram statistics computed by the ImgU
> + * \return The mean value of the top 2% of the histogram
> + */
> +double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const
> +{
> +	std::vector<uint32_t> measHist(numHistBins_);
> +
> +	/* Initialise the histogram array using the maximum available size */
> +	for (unsigned int histBin = 0; histBin < numHistBins_; histBin++)
> +		measHist.push_back(hist->hist_bins[histBin]);

hist->hist_bins is a __u32 array, I think you can skip the copy and do

	/* Estimate the quantile mean of the top 2% of the histogram. */
	Histogram histogram{ Span<uint32_t>(hist->hist_bins, numHistBins_) };
	return histogram.interQuantileMean(0.98, 1.0);

> +
> +	/* Estimate the quantile mean of the top 2% of the histogram. */
> +	return Histogram(Span<uint32_t>(measHist)).interQuantileMean(0.98, 1.0);
> +}
> +
>  /**
>   * \brief Process RkISP1 statistics, and run AGC operations
>   * \param[in] context The shared IPA context
> @@ -252,9 +285,13 @@ void Agc::process(IPAContext &context, const rkisp1_stat_buffer *stats)
>  	ASSERT(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP);
>  
>  	const rkisp1_cif_isp_ae_stat *ae = &params->ae;
> +	const rkisp1_cif_isp_hist_stat *hist = &params->hist;
>  
>  	frameCount_ = context.frameContext.frameId;
>  
> +	double iqMean = measureBrightness(hist);
> +	double iqMeanGain = kEvGainTarget * numHistBins_ / iqMean;
> +
>  	/*
>  	 * Estimate the gain needed to achieve a relative luminance target. To
>  	 * account for non-linearity caused by saturation, the value needs to be
> @@ -277,14 +314,44 @@ void Agc::process(IPAContext &context, const rkisp1_stat_buffer *stats)
>  			break;
>  	}
>  
> -	computeExposure(context, yGain);
> +	computeExposure(context, yGain, iqMeanGain);
>  }
>  
> -void Agc::prepare([[maybe_unused]] IPAContext &context,
> -		  rkisp1_params_cfg *params)
> +/**
> + * \copydoc libcamera::ipa::Algorithm::prepare
> + */
> +void Agc::prepare(IPAContext &context, rkisp1_params_cfg *params)
>  {
> -	params->module_ens |= RKISP1_CIF_ISP_MODULE_AEC;
> -	params->module_en_update |= RKISP1_CIF_ISP_MODULE_AEC;
> +	/* Configure the measurement window. */
> +	params->meas.aec_config.meas_window = context.configuration.agc.measureWindow;
> +	/* Use a continuous methode for measure. */

s/methode/method/

> +	params->meas.aec_config.autostop = RKISP1_CIF_ISP_EXP_CTRL_AUTOSTOP_0;
> +	/* Estimate Y as (R + G + B) x (85/256). */
> +	params->meas.aec_config.mode = RKISP1_CIF_ISP_EXP_MEASURING_MODE_1;
> +
> +	if (context.frameContext.frameId == 0) {

As for the BLC, I think you can have

	if (context.frameContext.frameId > 0)
		return;

at the beginning of the function instead.

> +		params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_AEC;
> +		params->module_ens |= RKISP1_CIF_ISP_MODULE_AEC;
> +		params->module_en_update |= RKISP1_CIF_ISP_MODULE_AEC;
> +	}
> +
> +	/* Configure histogram. */
> +	params->meas.hst_config.meas_window = context.configuration.agc.measureWindow;
> +	/* Produce the luminance histogram. */
> +	params->meas.hst_config.mode = RKISP1_CIF_ISP_HISTOGRAM_MODE_Y_HISTOGRAM;
> +	/* Set an average weighted histogram. */
> +	for (unsigned int histBin = 0; histBin < numHistBins_; histBin++)
> +		params->meas.hst_config.hist_weight[histBin] = 1;
> +	/* Step size can't be less than 3. */
> +	params->meas.hst_config.histogram_predivider = 4;
> +
> +	if (context.frameContext.frameId == 0) {
> +		/* Update the configuration for histogram. */
> +		params->module_cfg_update |= RKISP1_CIF_ISP_MODULE_HST;
> +		/* Enable the histogram measure unit. */
> +		params->module_ens |= RKISP1_CIF_ISP_MODULE_HST;
> +		params->module_en_update |= RKISP1_CIF_ISP_MODULE_HST;
> +	}
>  }
>  
>  } /* namespace ipa::rkisp1::algorithms */
> diff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h
> index 942c9d7a..872776d0 100644
> --- a/src/ipa/rkisp1/algorithms/agc.h
> +++ b/src/ipa/rkisp1/algorithms/agc.h
> @@ -32,13 +32,15 @@ public:
>  	void process(IPAContext &context, const rkisp1_stat_buffer *stats) override;
>  
>  private:
> -	void computeExposure(IPAContext &Context, double yGain);
> +	void computeExposure(IPAContext &Context, double yGain, double iqMeanGain);
>  	utils::Duration filterExposure(utils::Duration exposureValue);
>  	double estimateLuminance(const rkisp1_cif_isp_ae_stat *ae, double gain);
> +	double measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const;
>  
>  	uint64_t frameCount_;
>  
>  	uint32_t numCells_;
> +	uint32_t numHistBins_;
>  
>  	utils::Duration filteredExposure_;
>  };
> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> index 992c9225..39acef47 100644
> --- a/src/ipa/rkisp1/ipa_context.cpp
> +++ b/src/ipa/rkisp1/ipa_context.cpp
> @@ -71,6 +71,9 @@ namespace libcamera::ipa::rkisp1 {
>   * \var IPASessionConfiguration::agc.maxAnalogueGain
>   * \brief Maximum analogue gain supported with the configured sensor
>   *
> + * \var IPASessionConfiguration::agc.measureWindow
> + * \brief AGC measure window
> + *
>   * \var IPASessionConfiguration::hw
>   * \brief RkISP1-specific hardware information
>   *
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index c447369f..9ac3b40c 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -22,6 +22,7 @@ struct IPASessionConfiguration {
>  		utils::Duration maxShutterSpeed;
>  		double minAnalogueGain;
>  		double maxAnalogueGain;
> +		struct rkisp1_cif_isp_window measureWindow;
>  	} agc;
>  
>  	struct {