[libcamera-devel,v5,3/6] ipa: ipu3: Inline parseStatistics() into processStatsBuffer()
diff mbox series

Message ID 20220406141709.164794-4-umang.jain@ideasonboard.com
State Accepted
Headers show
Series
  • ipa: ipu3: Replace event-based ops with dedicated functions
Related show

Commit Message

Umang Jain April 6, 2022, 2:17 p.m. UTC
Since we have moved away from switch/case on the operation ID,
there's little reason to split the operation in two functions.

Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/ipa/ipu3/ipu3.cpp | 54 +++++++++++++++----------------------------
 1 file changed, 19 insertions(+), 35 deletions(-)

Comments

Kieran Bingham April 7, 2022, 10:16 p.m. UTC | #1
Quoting Umang Jain via libcamera-devel (2022-04-06 15:17:06)
> Since we have moved away from switch/case on the operation ID,
> there's little reason to split the operation in two functions.
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  src/ipa/ipu3/ipu3.cpp | 54 +++++++++++++++----------------------------
>  1 file changed, 19 insertions(+), 35 deletions(-)
> 
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 8c9a20f5..569c1311 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -156,9 +156,6 @@ private:
>                             ControlInfoMap *ipaControls);
>         void updateSessionConfiguration(const ControlInfoMap &sensorControls);
>  
> -       void parseStatistics(unsigned int frame,
> -                            int64_t frameTimestamp,
> -                            const ipu3_uapi_stats_3a *stats);
>         bool validateSensorControls();
>  
>         void setControls(unsigned int frame);
> @@ -546,11 +543,15 @@ void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)
>  }
>  
>  /**
> - * \brief Process statistics after ISP completion
> + * \brief Process the statistics generated by the ImgU
>   * \param[in] frame The frame number
>   * \param[in] frameTimestamp Timestamp of the frame
>   * \param[in] bufferId ID of the statistics buffer
>   * \param[in] sensorControls Sensor controls
> + *
> + * Parse the most recently processed image statistics from the ImgU. The
> + * statistics are passed to each algorithm module to run their calculations and
> + * update their state accordingly.
>   */
>  void IPAIPU3::processStatsBuffer(const uint32_t frame, const int64_t frameTimestamp,
>                                  const uint32_t bufferId, const ControlList &sensorControls)
> @@ -568,37 +569,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame, const int64_t frameTimest
>         context_.frameContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
>         context_.frameContext.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
>  
> -       parseStatistics(frame, frameTimestamp, stats);
> -}
> -
> -/**
> - * \brief Queue a request and process the control list from the application
> - * \param[in] frame The number of the frame which will be processed next
> - * \param[in] controls The controls for the \a frame
> - *
> - * Parse the request to handle any IPA-managed controls that were set from the
> - * application such as manual sensor settings.
> - */
> -void IPAIPU3::queueRequest(const uint32_t frame,
> -                          [[maybe_unused]] const ControlList &controls)
> -{
> -       /* \todo Start processing for 'frame' based on 'controls'. */
> -}
> -
> -/**
> - * \brief Process the statistics generated by the ImgU
> - * \param[in] frame The number of the latest frame processed
> - * \param[in] frameTimestamp The current frame timestamp
> - * \param[in] stats The IPU3 statistics and ISP results
> - *
> - * Parse the most recently processed image statistics from the ImgU. The
> - * statistics are passed to each algorithm module to run their calculations and
> - * update their state accordingly.
> - */
> -void IPAIPU3::parseStatistics(unsigned int frame,
> -                             [[maybe_unused]] int64_t frameTimestamp,
> -                             const ipu3_uapi_stats_3a *stats)
> -{
>         double lineDuration = context_.configuration.sensor.lineDuration.get<std::micro>();
>         int32_t vBlank = context_.configuration.sensor.defVBlank;
>         ControlList ctrls(controls::controls);
> @@ -629,6 +599,20 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>         metadataReady.emit(frame, ctrls);
>  }
>  
> +/**
> + * \brief Queue a request and process the control list from the application
> + * \param[in] frame The number of the frame which will be processed next
> + * \param[in] controls The controls for the \a frame
> + *
> + * Parse the request to handle any IPA-managed controls that were set from the
> + * application such as manual sensor settings.
> + */
> +void IPAIPU3::queueRequest(const uint32_t frame,
> +                          [[maybe_unused]] const ControlList &controls)
> +{
> +       /* \todo Start processing for 'frame' based on 'controls'. */
> +}
> +
>  /**
>   * \brief Handle sensor controls for a given \a frame number
>   * \param[in] frame The frame on which the sensor controls should be set
> -- 
> 2.31.0
>
Nicolas Dufresne via libcamera-devel April 8, 2022, 7:07 a.m. UTC | #2
Hi Umang,

On Wed, Apr 06, 2022 at 07:47:06PM +0530, Umang Jain via libcamera-devel wrote:
> Since we have moved away from switch/case on the operation ID,
> there's little reason to split the operation in two functions.
> 
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/ipa/ipu3/ipu3.cpp | 54 +++++++++++++++----------------------------
>  1 file changed, 19 insertions(+), 35 deletions(-)
> 
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index 8c9a20f5..569c1311 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -156,9 +156,6 @@ private:
>  			    ControlInfoMap *ipaControls);
>  	void updateSessionConfiguration(const ControlInfoMap &sensorControls);
>  
> -	void parseStatistics(unsigned int frame,
> -			     int64_t frameTimestamp,
> -			     const ipu3_uapi_stats_3a *stats);
>  	bool validateSensorControls();
>  
>  	void setControls(unsigned int frame);
> @@ -546,11 +543,15 @@ void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)
>  }
>  
>  /**
> - * \brief Process statistics after ISP completion
> + * \brief Process the statistics generated by the ImgU
>   * \param[in] frame The frame number
>   * \param[in] frameTimestamp Timestamp of the frame
>   * \param[in] bufferId ID of the statistics buffer
>   * \param[in] sensorControls Sensor controls
> + *
> + * Parse the most recently processed image statistics from the ImgU. The
> + * statistics are passed to each algorithm module to run their calculations and
> + * update their state accordingly.
>   */
>  void IPAIPU3::processStatsBuffer(const uint32_t frame, const int64_t frameTimestamp,
>  				 const uint32_t bufferId, const ControlList &sensorControls)
> @@ -568,37 +569,6 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame, const int64_t frameTimest
>  	context_.frameContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
>  	context_.frameContext.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
>  
> -	parseStatistics(frame, frameTimestamp, stats);
> -}
> -
> -/**
> - * \brief Queue a request and process the control list from the application
> - * \param[in] frame The number of the frame which will be processed next
> - * \param[in] controls The controls for the \a frame
> - *
> - * Parse the request to handle any IPA-managed controls that were set from the
> - * application such as manual sensor settings.
> - */
> -void IPAIPU3::queueRequest(const uint32_t frame,
> -			   [[maybe_unused]] const ControlList &controls)
> -{
> -	/* \todo Start processing for 'frame' based on 'controls'. */
> -}
> -
> -/**
> - * \brief Process the statistics generated by the ImgU
> - * \param[in] frame The number of the latest frame processed
> - * \param[in] frameTimestamp The current frame timestamp
> - * \param[in] stats The IPU3 statistics and ISP results
> - *
> - * Parse the most recently processed image statistics from the ImgU. The
> - * statistics are passed to each algorithm module to run their calculations and
> - * update their state accordingly.
> - */
> -void IPAIPU3::parseStatistics(unsigned int frame,
> -			      [[maybe_unused]] int64_t frameTimestamp,
> -			      const ipu3_uapi_stats_3a *stats)
> -{
>  	double lineDuration = context_.configuration.sensor.lineDuration.get<std::micro>();
>  	int32_t vBlank = context_.configuration.sensor.defVBlank;
>  	ControlList ctrls(controls::controls);
> @@ -629,6 +599,20 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>  	metadataReady.emit(frame, ctrls);
>  }
>  
> +/**
> + * \brief Queue a request and process the control list from the application
> + * \param[in] frame The number of the frame which will be processed next
> + * \param[in] controls The controls for the \a frame
> + *
> + * Parse the request to handle any IPA-managed controls that were set from the
> + * application such as manual sensor settings.
> + */
> +void IPAIPU3::queueRequest(const uint32_t frame,
> +			   [[maybe_unused]] const ControlList &controls)
> +{
> +	/* \todo Start processing for 'frame' based on 'controls'. */
> +}
> +
>  /**
>   * \brief Handle sensor controls for a given \a frame number
>   * \param[in] frame The frame on which the sensor controls should be set
> -- 
> 2.31.0
>

Patch
diff mbox series

diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index 8c9a20f5..569c1311 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -156,9 +156,6 @@  private:
 			    ControlInfoMap *ipaControls);
 	void updateSessionConfiguration(const ControlInfoMap &sensorControls);
 
-	void parseStatistics(unsigned int frame,
-			     int64_t frameTimestamp,
-			     const ipu3_uapi_stats_3a *stats);
 	bool validateSensorControls();
 
 	void setControls(unsigned int frame);
@@ -546,11 +543,15 @@  void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)
 }
 
 /**
- * \brief Process statistics after ISP completion
+ * \brief Process the statistics generated by the ImgU
  * \param[in] frame The frame number
  * \param[in] frameTimestamp Timestamp of the frame
  * \param[in] bufferId ID of the statistics buffer
  * \param[in] sensorControls Sensor controls
+ *
+ * Parse the most recently processed image statistics from the ImgU. The
+ * statistics are passed to each algorithm module to run their calculations and
+ * update their state accordingly.
  */
 void IPAIPU3::processStatsBuffer(const uint32_t frame, const int64_t frameTimestamp,
 				 const uint32_t bufferId, const ControlList &sensorControls)
@@ -568,37 +569,6 @@  void IPAIPU3::processStatsBuffer(const uint32_t frame, const int64_t frameTimest
 	context_.frameContext.sensor.exposure = sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
 	context_.frameContext.sensor.gain = camHelper_->gain(sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>());
 
-	parseStatistics(frame, frameTimestamp, stats);
-}
-
-/**
- * \brief Queue a request and process the control list from the application
- * \param[in] frame The number of the frame which will be processed next
- * \param[in] controls The controls for the \a frame
- *
- * Parse the request to handle any IPA-managed controls that were set from the
- * application such as manual sensor settings.
- */
-void IPAIPU3::queueRequest(const uint32_t frame,
-			   [[maybe_unused]] const ControlList &controls)
-{
-	/* \todo Start processing for 'frame' based on 'controls'. */
-}
-
-/**
- * \brief Process the statistics generated by the ImgU
- * \param[in] frame The number of the latest frame processed
- * \param[in] frameTimestamp The current frame timestamp
- * \param[in] stats The IPU3 statistics and ISP results
- *
- * Parse the most recently processed image statistics from the ImgU. The
- * statistics are passed to each algorithm module to run their calculations and
- * update their state accordingly.
- */
-void IPAIPU3::parseStatistics(unsigned int frame,
-			      [[maybe_unused]] int64_t frameTimestamp,
-			      const ipu3_uapi_stats_3a *stats)
-{
 	double lineDuration = context_.configuration.sensor.lineDuration.get<std::micro>();
 	int32_t vBlank = context_.configuration.sensor.defVBlank;
 	ControlList ctrls(controls::controls);
@@ -629,6 +599,20 @@  void IPAIPU3::parseStatistics(unsigned int frame,
 	metadataReady.emit(frame, ctrls);
 }
 
+/**
+ * \brief Queue a request and process the control list from the application
+ * \param[in] frame The number of the frame which will be processed next
+ * \param[in] controls The controls for the \a frame
+ *
+ * Parse the request to handle any IPA-managed controls that were set from the
+ * application such as manual sensor settings.
+ */
+void IPAIPU3::queueRequest(const uint32_t frame,
+			   [[maybe_unused]] const ControlList &controls)
+{
+	/* \todo Start processing for 'frame' based on 'controls'. */
+}
+
 /**
  * \brief Handle sensor controls for a given \a frame number
  * \param[in] frame The frame on which the sensor controls should be set