[v2,05/16] libipa: exposure_mode_helper: Remove double calculation of lastStageGain
diff mbox series

Message ID 20250808141315.413839-6-stefan.klug@ideasonboard.com
State Superseded
Headers show
Series
  • Implement WDR algorithm
Related show

Commit Message

Stefan Klug Aug. 8, 2025, 2:12 p.m. UTC
lastStageGain gets recalculated unconditionally even though it is the
stageGain of the last stage. Refactor for increased simplicity and
efficiency.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 src/ipa/libipa/exposure_mode_helper.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Kieran Bingham Aug. 8, 2025, 5:56 p.m. UTC | #1
Quoting Stefan Klug (2025-08-08 15:12:43)
> lastStageGain gets recalculated unconditionally even though it is the
> stageGain of the last stage. Refactor for increased simplicity and
> efficiency.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>  src/ipa/libipa/exposure_mode_helper.cpp | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp
> index d0a61908c966..3c758851679f 100644
> --- a/src/ipa/libipa/exposure_mode_helper.cpp
> +++ b/src/ipa/libipa/exposure_mode_helper.cpp
> @@ -198,10 +198,10 @@ ExposureModeHelper::splitExposure(utils::Duration exposure) const
>  
>         utils::Duration exposureTime;
>         double stageGain = 1.0;
> +       double lastStageGain = 1.0;
>         double gain;
>  
>         for (unsigned int stage = 0; stage < gains_.size(); stage++) {
> -               double lastStageGain = stage == 0 ? 1.0 : clampGain(gains_[stage - 1]);
>                 utils::Duration stageExposureTime = clampExposureTime(exposureTimes_[stage]);
>                 stageGain = clampGain(gains_[stage]);
>  
> @@ -228,6 +228,8 @@ ExposureModeHelper::splitExposure(utils::Duration exposure) const
>  
>                         return { exposureTime, gain, exposure / (exposureTime * gain) };
>                 }
> +
> +               lastStageGain = stageGain;

Looks good ...

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

>         }
>  
>         /*
> -- 
> 2.48.1
>
Dan Scally Aug. 11, 2025, 2:38 p.m. UTC | #2
Hi Stefan

On 08/08/2025 15:12, Stefan Klug wrote:
> lastStageGain gets recalculated unconditionally even though it is the
> stageGain of the last stage. Refactor for increased simplicity and
> efficiency.
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
Much nicer:

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

>   src/ipa/libipa/exposure_mode_helper.cpp | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp
> index d0a61908c966..3c758851679f 100644
> --- a/src/ipa/libipa/exposure_mode_helper.cpp
> +++ b/src/ipa/libipa/exposure_mode_helper.cpp
> @@ -198,10 +198,10 @@ ExposureModeHelper::splitExposure(utils::Duration exposure) const
>   
>   	utils::Duration exposureTime;
>   	double stageGain = 1.0;
> +	double lastStageGain = 1.0;
>   	double gain;
>   
>   	for (unsigned int stage = 0; stage < gains_.size(); stage++) {
> -		double lastStageGain = stage == 0 ? 1.0 : clampGain(gains_[stage - 1]);
>   		utils::Duration stageExposureTime = clampExposureTime(exposureTimes_[stage]);
>   		stageGain = clampGain(gains_[stage]);
>   
> @@ -228,6 +228,8 @@ ExposureModeHelper::splitExposure(utils::Duration exposure) const
>   
>   			return { exposureTime, gain, exposure / (exposureTime * gain) };
>   		}
> +
> +		lastStageGain = stageGain;
>   	}
>   
>   	/*

Patch
diff mbox series

diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp
index d0a61908c966..3c758851679f 100644
--- a/src/ipa/libipa/exposure_mode_helper.cpp
+++ b/src/ipa/libipa/exposure_mode_helper.cpp
@@ -198,10 +198,10 @@  ExposureModeHelper::splitExposure(utils::Duration exposure) const
 
 	utils::Duration exposureTime;
 	double stageGain = 1.0;
+	double lastStageGain = 1.0;
 	double gain;
 
 	for (unsigned int stage = 0; stage < gains_.size(); stage++) {
-		double lastStageGain = stage == 0 ? 1.0 : clampGain(gains_[stage - 1]);
 		utils::Duration stageExposureTime = clampExposureTime(exposureTimes_[stage]);
 		stageGain = clampGain(gains_[stage]);
 
@@ -228,6 +228,8 @@  ExposureModeHelper::splitExposure(utils::Duration exposure) const
 
 			return { exposureTime, gain, exposure / (exposureTime * gain) };
 		}
+
+		lastStageGain = stageGain;
 	}
 
 	/*