[libcamera-devel,v4,03/14] ipa: ipu3: Use sensor controls to update frameContext
diff mbox series

Message ID 20211111140928.136111-4-jeanmichel.hautbois@ideasonboard.com
State Accepted
Headers show
Series
  • IPA: IPU3: Introduce per-frame controls
Related show

Commit Message

Jean-Michel Hautbois Nov. 11, 2021, 2:09 p.m. UTC 
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@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@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(-)

Comments

Umang Jain Nov. 12, 2021, 8:10 a.m. UTC | #1 
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@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


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

Reviewed-by: Umang Jain <umang.jain@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;
>   	}
Paul Elder Nov. 12, 2021, 10:55 p.m. UTC | #2 
Hi Jean-Michel,

On Thu, Nov 11, 2021 at 03:09:17PM +0100, 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@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@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;
>  	}
> -- 
> 2.32.0
>

Patch
diff mbox series 

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;
 	}