[10/12] ipa: rkisp1: agc: Rename maxShutterSpeed to maxFrameDuration
diff mbox series

Message ID 20240616163910.5506-11-laurent.pinchart@ideasonboard.com
State Accepted
Commit 14056bceb536e952f745704a4bc3856bad4686ea
Headers show
Series
  • ipa: rkisp1: Miscellaneous AGC fixes
Related show

Commit Message

Laurent Pinchart June 16, 2024, 4:39 p.m. UTC 
The AGC active state and frame context both contain a variable named
maxShutterSpeed. The variable is used to limit the maximum shutter speed
when computing the exposure time and gains, but stores the maximum frame
duration, not clamped by the sensor's maximum shutter speed. Rename it
to maxFrameDuration.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/ipa/rkisp1/algorithms/agc.cpp | 10 +++++-----
 src/ipa/rkisp1/ipa_context.cpp    |  4 ++--
 src/ipa/rkisp1/ipa_context.h      |  4 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)

Comments

Kieran Bingham June 17, 2024, 9:31 a.m. UTC | #1 
Quoting Laurent Pinchart (2024-06-16 17:39:08)
> The AGC active state and frame context both contain a variable named
> maxShutterSpeed. The variable is used to limit the maximum shutter speed
> when computing the exposure time and gains, but stores the maximum frame
> duration, not clamped by the sensor's maximum shutter speed. Rename it
> to maxFrameDuration.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

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

> ---
>  src/ipa/rkisp1/algorithms/agc.cpp | 10 +++++-----
>  src/ipa/rkisp1/ipa_context.cpp    |  4 ++--
>  src/ipa/rkisp1/ipa_context.h      |  4 ++--
>  3 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 6a199b47c9ad..5b917557b887 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -182,7 +182,7 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)
>          * except it's computed in the IPA and not here so we'd have to
>          * recompute it.
>          */
> -       context.activeState.agc.maxShutterSpeed = context.configuration.sensor.maxShutterSpeed;
> +       context.activeState.agc.maxFrameDuration = context.configuration.sensor.maxShutterSpeed;
>  
>         /*
>          * Define the measurement window for AGC as a centered rectangle
> @@ -269,11 +269,11 @@ void Agc::queueRequest(IPAContext &context,
>  
>         const auto &frameDurationLimits = controls.get(controls::FrameDurationLimits);
>         if (frameDurationLimits) {
> -               utils::Duration maxShutterSpeed =
> +               utils::Duration maxFrameDuration =
>                         std::chrono::milliseconds((*frameDurationLimits).back());
> -               agc.maxShutterSpeed = maxShutterSpeed;
> +               agc.maxFrameDuration = maxFrameDuration;
>         }
> -       frameContext.agc.maxShutterSpeed = agc.maxShutterSpeed;
> +       frameContext.agc.maxFrameDuration = agc.maxFrameDuration;
>  }
>  
>  /**
> @@ -421,7 +421,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>         expMeans_ = { params->ae.exp_mean, context.hw->numAeCells };
>  
>         utils::Duration maxShutterSpeed = std::min(context.configuration.sensor.maxShutterSpeed,
> -                                                  frameContext.agc.maxShutterSpeed);
> +                                                  frameContext.agc.maxFrameDuration);
>         setLimits(context.configuration.sensor.minShutterSpeed,
>                   maxShutterSpeed,
>                   context.configuration.sensor.minAnalogueGain,
> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> index 9e445dcca933..ab6cfae17feb 100644
> --- a/src/ipa/rkisp1/ipa_context.cpp
> +++ b/src/ipa/rkisp1/ipa_context.cpp
> @@ -172,7 +172,7 @@ namespace libcamera::ipa::rkisp1 {
>   * \var IPAActiveState::agc.meteringMode
>   * \brief Metering mode as set by the AeMeteringMode control
>   *
> - * \var IPAActiveState::agc.maxShutterSpeed
> + * \var IPAActiveState::agc.maxFrameDuration
>   * \brief Maximum frame duration as set by the FrameDurationLimits control
>   */
>  
> @@ -314,7 +314,7 @@ namespace libcamera::ipa::rkisp1 {
>   * \var IPAFrameContext::agc.meteringMode
>   * \brief Metering mode as set by the AeMeteringMode control
>   *
> - * \var IPAFrameContext::agc.maxShutterSpeed
> + * \var IPAFrameContext::agc.maxFrameDuration
>   * \brief Maximum frame duration as set by the FrameDurationLimits control
>   *
>   * \var IPAFrameContext::agc.update
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index 6022480d4fd2..734856cdb199 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -72,7 +72,7 @@ struct IPAActiveState {
>                 controls::AeConstraintModeEnum constraintMode;
>                 controls::AeExposureModeEnum exposureMode;
>                 controls::AeMeteringModeEnum meteringMode;
> -               utils::Duration maxShutterSpeed;
> +               utils::Duration maxFrameDuration;
>         } agc;
>  
>         struct {
> @@ -121,7 +121,7 @@ struct IPAFrameContext : public FrameContext {
>                 controls::AeConstraintModeEnum constraintMode;
>                 controls::AeExposureModeEnum exposureMode;
>                 controls::AeMeteringModeEnum meteringMode;
> -               utils::Duration maxShutterSpeed;
> +               utils::Duration maxFrameDuration;
>                 bool update;
>         } agc;
>  
> -- 
> Regards,
> 
> Laurent Pinchart
>
Paul Elder June 17, 2024, 11:52 a.m. UTC | #2 
On Sun, Jun 16, 2024 at 07:39:08PM +0300, Laurent Pinchart wrote:
> The AGC active state and frame context both contain a variable named
> maxShutterSpeed. The variable is used to limit the maximum shutter speed
> when computing the exposure time and gains, but stores the maximum frame
> duration, not clamped by the sensor's maximum shutter speed. Rename it
> to maxFrameDuration.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

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

> ---
>  src/ipa/rkisp1/algorithms/agc.cpp | 10 +++++-----
>  src/ipa/rkisp1/ipa_context.cpp    |  4 ++--
>  src/ipa/rkisp1/ipa_context.h      |  4 ++--
>  3 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
> index 6a199b47c9ad..5b917557b887 100644
> --- a/src/ipa/rkisp1/algorithms/agc.cpp
> +++ b/src/ipa/rkisp1/algorithms/agc.cpp
> @@ -182,7 +182,7 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)
>  	 * except it's computed in the IPA and not here so we'd have to
>  	 * recompute it.
>  	 */
> -	context.activeState.agc.maxShutterSpeed = context.configuration.sensor.maxShutterSpeed;
> +	context.activeState.agc.maxFrameDuration = context.configuration.sensor.maxShutterSpeed;
>  
>  	/*
>  	 * Define the measurement window for AGC as a centered rectangle
> @@ -269,11 +269,11 @@ void Agc::queueRequest(IPAContext &context,
>  
>  	const auto &frameDurationLimits = controls.get(controls::FrameDurationLimits);
>  	if (frameDurationLimits) {
> -		utils::Duration maxShutterSpeed =
> +		utils::Duration maxFrameDuration =
>  			std::chrono::milliseconds((*frameDurationLimits).back());
> -		agc.maxShutterSpeed = maxShutterSpeed;
> +		agc.maxFrameDuration = maxFrameDuration;
>  	}
> -	frameContext.agc.maxShutterSpeed = agc.maxShutterSpeed;
> +	frameContext.agc.maxFrameDuration = agc.maxFrameDuration;
>  }
>  
>  /**
> @@ -421,7 +421,7 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
>  	expMeans_ = { params->ae.exp_mean, context.hw->numAeCells };
>  
>  	utils::Duration maxShutterSpeed = std::min(context.configuration.sensor.maxShutterSpeed,
> -						   frameContext.agc.maxShutterSpeed);
> +						   frameContext.agc.maxFrameDuration);
>  	setLimits(context.configuration.sensor.minShutterSpeed,
>  		  maxShutterSpeed,
>  		  context.configuration.sensor.minAnalogueGain,
> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
> index 9e445dcca933..ab6cfae17feb 100644
> --- a/src/ipa/rkisp1/ipa_context.cpp
> +++ b/src/ipa/rkisp1/ipa_context.cpp
> @@ -172,7 +172,7 @@ namespace libcamera::ipa::rkisp1 {
>   * \var IPAActiveState::agc.meteringMode
>   * \brief Metering mode as set by the AeMeteringMode control
>   *
> - * \var IPAActiveState::agc.maxShutterSpeed
> + * \var IPAActiveState::agc.maxFrameDuration
>   * \brief Maximum frame duration as set by the FrameDurationLimits control
>   */
>  
> @@ -314,7 +314,7 @@ namespace libcamera::ipa::rkisp1 {
>   * \var IPAFrameContext::agc.meteringMode
>   * \brief Metering mode as set by the AeMeteringMode control
>   *
> - * \var IPAFrameContext::agc.maxShutterSpeed
> + * \var IPAFrameContext::agc.maxFrameDuration
>   * \brief Maximum frame duration as set by the FrameDurationLimits control
>   *
>   * \var IPAFrameContext::agc.update
> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
> index 6022480d4fd2..734856cdb199 100644
> --- a/src/ipa/rkisp1/ipa_context.h
> +++ b/src/ipa/rkisp1/ipa_context.h
> @@ -72,7 +72,7 @@ struct IPAActiveState {
>  		controls::AeConstraintModeEnum constraintMode;
>  		controls::AeExposureModeEnum exposureMode;
>  		controls::AeMeteringModeEnum meteringMode;
> -		utils::Duration maxShutterSpeed;
> +		utils::Duration maxFrameDuration;
>  	} agc;
>  
>  	struct {
> @@ -121,7 +121,7 @@ struct IPAFrameContext : public FrameContext {
>  		controls::AeConstraintModeEnum constraintMode;
>  		controls::AeExposureModeEnum exposureMode;
>  		controls::AeMeteringModeEnum meteringMode;
> -		utils::Duration maxShutterSpeed;
> +		utils::Duration maxFrameDuration;
>  		bool update;
>  	} agc;
>

Patch
diff mbox series 

diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp
index 6a199b47c9ad..5b917557b887 100644
--- a/src/ipa/rkisp1/algorithms/agc.cpp
+++ b/src/ipa/rkisp1/algorithms/agc.cpp
@@ -182,7 +182,7 @@  int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo)
 	 * except it's computed in the IPA and not here so we'd have to
 	 * recompute it.
 	 */
-	context.activeState.agc.maxShutterSpeed = context.configuration.sensor.maxShutterSpeed;
+	context.activeState.agc.maxFrameDuration = context.configuration.sensor.maxShutterSpeed;
 
 	/*
 	 * Define the measurement window for AGC as a centered rectangle
@@ -269,11 +269,11 @@  void Agc::queueRequest(IPAContext &context,
 
 	const auto &frameDurationLimits = controls.get(controls::FrameDurationLimits);
 	if (frameDurationLimits) {
-		utils::Duration maxShutterSpeed =
+		utils::Duration maxFrameDuration =
 			std::chrono::milliseconds((*frameDurationLimits).back());
-		agc.maxShutterSpeed = maxShutterSpeed;
+		agc.maxFrameDuration = maxFrameDuration;
 	}
-	frameContext.agc.maxShutterSpeed = agc.maxShutterSpeed;
+	frameContext.agc.maxFrameDuration = agc.maxFrameDuration;
 }
 
 /**
@@ -421,7 +421,7 @@  void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,
 	expMeans_ = { params->ae.exp_mean, context.hw->numAeCells };
 
 	utils::Duration maxShutterSpeed = std::min(context.configuration.sensor.maxShutterSpeed,
-						   frameContext.agc.maxShutterSpeed);
+						   frameContext.agc.maxFrameDuration);
 	setLimits(context.configuration.sensor.minShutterSpeed,
 		  maxShutterSpeed,
 		  context.configuration.sensor.minAnalogueGain,
diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
index 9e445dcca933..ab6cfae17feb 100644
--- a/src/ipa/rkisp1/ipa_context.cpp
+++ b/src/ipa/rkisp1/ipa_context.cpp
@@ -172,7 +172,7 @@  namespace libcamera::ipa::rkisp1 {
  * \var IPAActiveState::agc.meteringMode
  * \brief Metering mode as set by the AeMeteringMode control
  *
- * \var IPAActiveState::agc.maxShutterSpeed
+ * \var IPAActiveState::agc.maxFrameDuration
  * \brief Maximum frame duration as set by the FrameDurationLimits control
  */
 
@@ -314,7 +314,7 @@  namespace libcamera::ipa::rkisp1 {
  * \var IPAFrameContext::agc.meteringMode
  * \brief Metering mode as set by the AeMeteringMode control
  *
- * \var IPAFrameContext::agc.maxShutterSpeed
+ * \var IPAFrameContext::agc.maxFrameDuration
  * \brief Maximum frame duration as set by the FrameDurationLimits control
  *
  * \var IPAFrameContext::agc.update
diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
index 6022480d4fd2..734856cdb199 100644
--- a/src/ipa/rkisp1/ipa_context.h
+++ b/src/ipa/rkisp1/ipa_context.h
@@ -72,7 +72,7 @@  struct IPAActiveState {
 		controls::AeConstraintModeEnum constraintMode;
 		controls::AeExposureModeEnum exposureMode;
 		controls::AeMeteringModeEnum meteringMode;
-		utils::Duration maxShutterSpeed;
+		utils::Duration maxFrameDuration;
 	} agc;
 
 	struct {
@@ -121,7 +121,7 @@  struct IPAFrameContext : public FrameContext {
 		controls::AeConstraintModeEnum constraintMode;
 		controls::AeExposureModeEnum exposureMode;
 		controls::AeMeteringModeEnum meteringMode;
-		utils::Duration maxShutterSpeed;
+		utils::Duration maxFrameDuration;
 		bool update;
 	} agc;