[libcamera-devel,v3,1/5] ipa: rkisp1: Use frame counter for the request queued
diff mbox series

Message ID 20220224113347.80001-2-jeanmichel.hautbois@ideasonboard.com
State Accepted
Headers show
Series
  • IPA RkISP1 awb and misc improvements
Related show

Commit Message

Jean-Michel Hautbois Feb. 24, 2022, 11:33 a.m. UTC 
Introduce a frameCount variable in the IPAFrameContext which increments
each time a request is queued. It is reset at configure call, when the
camera is started.

This will allow the frameCount to be used by other algorithms, without
having to keep multiple private frame counters.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>

---
v3: - replace frameId by a frame counter
    - Change the date of the copyright
---
 src/ipa/rkisp1/ipa_context.cpp | 9 ++++++++-
 src/ipa/rkisp1/ipa_context.h   | 4 +++-
 src/ipa/rkisp1/rkisp1.cpp      | 3 +++
 3 files changed, 14 insertions(+), 2 deletions(-)

Comments

Nicolas Dufresne via libcamera-devel March 8, 2022, 8:51 a.m. UTC | #1 
Hi JM

Thank you for the patch.

On 2/24/22 17:03, Jean-Michel Hautbois wrote:
> Introduce a frameCount variable in the IPAFrameContext which increments
> each time a request is queued. It is reset at configure call, when the
> camera is started.
>
> This will allow the frameCount to be used by other algorithms, without
> having to keep multiple private frame counters.
>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>


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

>
> ---
> v3: - replace frameId by a frame counter
>      - Change the date of the copyright
> ---
>   src/ipa/rkisp1/ipa_context.cpp | 9 ++++++++-
>   src/ipa/rkisp1/ipa_context.h   | 4 +++-
>   src/ipa/rkisp1/rkisp1.cpp      | 3 +++
>   3 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> index 9cb2a9fd..664f572f 100644
> --- a/src/ipa/rkisp1/ipa_context.cpp
> +++ b/src/ipa/rkisp1/ipa_context.cpp
> @@ -1,6 +1,6 @@
>   /* SPDX-License-Identifier: LGPL-2.1-or-later */
>   /*
> - * Copyright (C) 2021, Ideas On Board
> + * Copyright (C) 2021-2022, Ideas On Board
>    *
>    * ipa_context.cpp - RkISP1 IPA Context
>    */
> @@ -113,4 +113,11 @@ namespace libcamera::ipa::rkisp1 {
>    * \brief Analogue gain multiplier
>    */
>   
> +/**
> + * \var IPAFrameContext::frameCount
> + * \brief Number of queued requests for this frame context
> + *
> + * It restarts from 0 when the camera is started.
> + */
> +
>   } /* namespace libcamera::ipa::rkisp1 */
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index b94ade0c..212fa052 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -1,6 +1,6 @@
>   /* SPDX-License-Identifier: LGPL-2.1-or-later */
>   /*
> - * Copyright (C) 2021, Ideas On Board
> + * Copyright (C) 2021-2022, Ideas On Board
>    *
>    * ipa_context.h - RkISP1 IPA Context
>    *
> @@ -43,6 +43,8 @@ struct IPAFrameContext {
>   		uint32_t exposure;
>   		double gain;
>   	} sensor;
> +
> +	unsigned int frameCount;
>   };
>   
>   struct IPAContext {
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 2d79f15f..f119b3f3 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -196,6 +196,8 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,
>   	context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain_);
>   	context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain_);
>   
> +	context_.frameContext.frameCount = 0;
> +
>   	for (auto const &algo : algorithms_) {
>   		int ret = algo->configure(context_, info);
>   		if (ret)
> @@ -284,6 +286,7 @@ void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,
>   	op.op = ActionParamFilled;
>   
>   	queueFrameAction.emit(frame, op);
> +	context_.frameContext.frameCount++;
>   }
>   
>   void IPARkISP1::updateStatistics(unsigned int frame,
Laurent Pinchart March 27, 2022, 8:21 p.m. UTC | #2 
Hi Jean-Michel,

Thank you for the patch.

On Thu, Feb 24, 2022 at 12:33:43PM +0100, Jean-Michel Hautbois wrote:
> Introduce a frameCount variable in the IPAFrameContext which increments
> each time a request is queued. It is reset at configure call, when the
> camera is started.
> 
> This will allow the frameCount to be used by other algorithms, without
> having to keep multiple private frame counters.
> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> 
> ---
> v3: - replace frameId by a frame counter
>     - Change the date of the copyright
> ---
>  src/ipa/rkisp1/ipa_context.cpp | 9 ++++++++-
>  src/ipa/rkisp1/ipa_context.h   | 4 +++-
>  src/ipa/rkisp1/rkisp1.cpp      | 3 +++
>  3 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> index 9cb2a9fd..664f572f 100644
> --- a/src/ipa/rkisp1/ipa_context.cpp
> +++ b/src/ipa/rkisp1/ipa_context.cpp
> @@ -1,6 +1,6 @@
>  /* SPDX-License-Identifier: LGPL-2.1-or-later */
>  /*
> - * Copyright (C) 2021, Ideas On Board
> + * Copyright (C) 2021-2022, Ideas On Board
>   *
>   * ipa_context.cpp - RkISP1 IPA Context
>   */
> @@ -113,4 +113,11 @@ namespace libcamera::ipa::rkisp1 {
>   * \brief Analogue gain multiplier
>   */
>  
> +/**
> + * \var IPAFrameContext::frameCount
> + * \brief Number of queued requests for this frame context

"Number of queued requests" sounds a bit like the number of requests
currently queued.

/**
 * \var IPAFrameContext::frameCount
 * \brief Counter of requests queued to the IPA module
 *
 * The counter is reset to 0 when the IPA module is configured, and is
 * incremented for each request being queued, after calling the
 * Algorithm::prepare() function of all algorithms.
 */

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> + *
> + * It restarts from 0 when the camera is started.
> + */
> +
>  } /* namespace libcamera::ipa::rkisp1 */
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index b94ade0c..212fa052 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -1,6 +1,6 @@
>  /* SPDX-License-Identifier: LGPL-2.1-or-later */
>  /*
> - * Copyright (C) 2021, Ideas On Board
> + * Copyright (C) 2021-2022, Ideas On Board
>   *
>   * ipa_context.h - RkISP1 IPA Context
>   *
> @@ -43,6 +43,8 @@ struct IPAFrameContext {
>  		uint32_t exposure;
>  		double gain;
>  	} sensor;
> +
> +	unsigned int frameCount;
>  };
>  
>  struct IPAContext {
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 2d79f15f..f119b3f3 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -196,6 +196,8 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,
>  	context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain_);
>  	context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain_);
>  
> +	context_.frameContext.frameCount = 0;
> +
>  	for (auto const &algo : algorithms_) {
>  		int ret = algo->configure(context_, info);
>  		if (ret)
> @@ -284,6 +286,7 @@ void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,
>  	op.op = ActionParamFilled;
>  
>  	queueFrameAction.emit(frame, op);
> +	context_.frameContext.frameCount++;
>  }
>  
>  void IPARkISP1::updateStatistics(unsigned int frame,

Patch
diff mbox series 

diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
index 9cb2a9fd..664f572f 100644
--- a/src/ipa/rkisp1/ipa_context.cpp
+++ b/src/ipa/rkisp1/ipa_context.cpp
@@ -1,6 +1,6 @@ 
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 /*
- * Copyright (C) 2021, Ideas On Board
+ * Copyright (C) 2021-2022, Ideas On Board
  *
  * ipa_context.cpp - RkISP1 IPA Context
  */
@@ -113,4 +113,11 @@  namespace libcamera::ipa::rkisp1 {
  * \brief Analogue gain multiplier
  */
 
+/**
+ * \var IPAFrameContext::frameCount
+ * \brief Number of queued requests for this frame context
+ *
+ * It restarts from 0 when the camera is started.
+ */
+
 } /* namespace libcamera::ipa::rkisp1 */
diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
index b94ade0c..212fa052 100644
--- a/src/ipa/rkisp1/ipa_context.h
+++ b/src/ipa/rkisp1/ipa_context.h
@@ -1,6 +1,6 @@ 
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 /*
- * Copyright (C) 2021, Ideas On Board
+ * Copyright (C) 2021-2022, Ideas On Board
  *
  * ipa_context.h - RkISP1 IPA Context
  *
@@ -43,6 +43,8 @@  struct IPAFrameContext {
 		uint32_t exposure;
 		double gain;
 	} sensor;
+
+	unsigned int frameCount;
 };
 
 struct IPAContext {
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index 2d79f15f..f119b3f3 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -196,6 +196,8 @@  int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,
 	context_.configuration.agc.minAnalogueGain = camHelper_->gain(minGain_);
 	context_.configuration.agc.maxAnalogueGain = camHelper_->gain(maxGain_);
 
+	context_.frameContext.frameCount = 0;
+
 	for (auto const &algo : algorithms_) {
 		int ret = algo->configure(context_, info);
 		if (ret)
@@ -284,6 +286,7 @@  void IPARkISP1::queueRequest(unsigned int frame, rkisp1_params_cfg *params,
 	op.op = ActionParamFilled;
 
 	queueFrameAction.emit(frame, op);
+	context_.frameContext.frameCount++;
 }
 
 void IPARkISP1::updateStatistics(unsigned int frame,