[libcamera-devel] [PATCH v4 03/14] ipa: ipu3: Use sensor controls to update frameContext

[Umang Jain]

Hi JM,

Thank you for the pathc.

On 11/11/21 7:39 PM, Jean-Michel Hautbois wrote:
> The pipeline handler populates the new sensorControls ControlList, to
> have the effective exposure and gain values for the current frame. This
> is done when a statistics buffer is received.
>
> Make those values the frameContext::sensor values for the frame when the
> EventStatReady event is received.
>
> AGC also needs to use frameContext.sensor as its input values and
> frameContext.agc as its output values. Modify computeExposure by passing
> it the frameContext instead of individual exposure and gain values.
>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>


Looks good to me individually as per the rationale written in the commit 
message

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

> ---
>   src/ipa/ipu3/algorithms/agc.cpp | 19 ++++++++++---------
>   src/ipa/ipu3/algorithms/agc.h   |  2 +-
>   src/ipa/ipu3/ipa_context.cpp    | 11 +++++++++++
>   src/ipa/ipu3/ipa_context.h      |  5 +++++
>   src/ipa/ipu3/ipu3.cpp           |  3 +++
>   5 files changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp
> index b5d736c1..5723f6f3 100644
> --- a/src/ipa/ipu3/algorithms/agc.cpp
> +++ b/src/ipa/ipu3/algorithms/agc.cpp
> @@ -169,10 +169,9 @@ void Agc::filterExposure()
>   
>   /**
>    * \brief Estimate the new exposure and gain values
> - * \param[inout] exposure The exposure value reference as a number of lines
> - * \param[inout] gain The gain reference to be updated
> + * \param[inout] frameContext The shared IPA frame Context
>    */
> -void Agc::computeExposure(uint32_t &exposure, double &analogueGain)
> +void Agc::computeExposure(IPAFrameContext &frameContext)
>   {
>   	/* Algorithm initialization should wait for first valid frames */
>   	/* \todo - have a number of frames given by DelayedControls ?
> @@ -189,6 +188,10 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain)
>   		return;
>   	}
>   
> +	/* Get the effective exposure and gain applied on the sensor. */
> +	uint32_t exposure = frameContext.sensor.exposure;
> +	double analogueGain = frameContext.sensor.gain;
> +
>   	/* Estimate the gain needed to have the proportion wanted */
>   	double evGain = kEvGainTarget * knumHistogramBins / iqMean_;
>   
> @@ -233,8 +236,9 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain)
>   			    << shutterTime << " and "
>   			    << stepGain;
>   
> -	exposure = shutterTime / lineDuration_;
> -	analogueGain = stepGain;
> +	/* Update the estimated exposure and gain. */
> +	frameContext.agc.exposure = shutterTime / lineDuration_;
> +	frameContext.agc.gain = stepGain;
>   
>   	/*
>   	 * Update the exposure value for the next process call.
> @@ -257,11 +261,8 @@ void Agc::computeExposure(uint32_t &exposure, double &analogueGain)
>    */
>   void Agc::process(IPAContext &context, const ipu3_uapi_stats_3a *stats)
>   {
> -	/* Get the latest exposure and gain applied */
> -	uint32_t &exposure = context.frameContext.agc.exposure;
> -	double &analogueGain = context.frameContext.agc.gain;
>   	measureBrightness(stats, context.configuration.grid.bdsGrid);
> -	computeExposure(exposure, analogueGain);
> +	computeExposure(context.frameContext);
>   	frameCount_++;
>   }
>   
> diff --git a/src/ipa/ipu3/algorithms/agc.h b/src/ipa/ipu3/algorithms/agc.h
> index 69e0b831..f0db25ee 100644
> --- a/src/ipa/ipu3/algorithms/agc.h
> +++ b/src/ipa/ipu3/algorithms/agc.h
> @@ -34,7 +34,7 @@ private:
>   	void measureBrightness(const ipu3_uapi_stats_3a *stats,
>   			       const ipu3_uapi_grid_config &grid);
>   	void filterExposure();
> -	void computeExposure(uint32_t &exposure, double &gain);
> +	void computeExposure(IPAFrameContext &frameContext);
>   
>   	uint64_t frameCount_;
>   	uint64_t lastFrame_;
> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp
> index 2355a9c7..a7ff957d 100644
> --- a/src/ipa/ipu3/ipa_context.cpp
> +++ b/src/ipa/ipu3/ipa_context.cpp
> @@ -119,6 +119,17 @@ namespace libcamera::ipa::ipu3 {
>    * \brief White balance gain for B channel
>    */
>   
> +/**
> + * \var IPAFrameContext::sensor
> + * \brief Effective sensor values
> + *
> + * \var IPAFrameContext::sensor.exposure
> + * \brief Exposure time expressed as a number of lines
> + *
> + * \var IPAFrameContext::sensor.gain
> + * \brief Analogue gain multiplier
> + */
> +
>   /**
>    * \var IPAFrameContext::toneMapping
>    * \brief Context for ToneMapping and Gamma control
> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h
> index 1e46c61a..a5a19800 100644
> --- a/src/ipa/ipu3/ipa_context.h
> +++ b/src/ipa/ipu3/ipa_context.h
> @@ -47,6 +47,11 @@ struct IPAFrameContext {
>   		} gains;
>   	} awb;
>   
> +	struct {
> +		uint32_t exposure;
> +		double gain;
> +	} sensor;
> +
>   	struct {
>   		double gamma;
>   		struct ipu3_uapi_gamma_corr_lut gammaCorrection;
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index bcc3863b..38e86e58 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -549,6 +549,9 @@ void IPAIPU3::processEvent(const IPU3Event &event)
>   		const ipu3_uapi_stats_3a *stats =
>   			reinterpret_cast<ipu3_uapi_stats_3a *>(mem.data());
>   
> +		context_.frameContext.sensor.exposure = event.sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
> +		context_.frameContext.sensor.gain = camHelper_->gain(event.sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
> +
>   		parseStatistics(event.frame, event.frameTimestamp, stats);
>   		break;
>   	}