[v7,06/19] ipa: libipa: exposure_mode_helper: Consider last stage as hard-limit
diff mbox series

Message ID 20261109094954.341480-7-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • ipa: libipa: agc: Take exposure margin into account
Related show

Commit Message

Barnabás Pőcze Nov. 9, 2026, 9:49 a.m. UTC 
Currently `splitExposure()` will raise the exposure and analogue gain above
what is specified in the last stage, if the desired exposure could not be
reached. This behaviour is different from that of the raspberry pi ipa,
and also could be surprising to the user / tuning file creator.

So make sure that the exposure time and analogue gain is limited by the
last stage, in addition to the runtime controlled upper bound.

With this change, at least one stage is required, so an assertions are added.
And the general agc algorithm is adjusted to provide stages for the fallback
mode.

Note that the `stageGain` initialization has to be adjusted to retrieve the
quantization gain, so that at the end of the loop

  * stageExposureTime
  * stageGain
  * quantGain
  * quantGain2

are all up to date for the last stage.

Closes: https://gitlab.freedesktop.org/camera/libcamera/-/work_items/362
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
---
 src/ipa/libipa/agc.cpp                  |  7 ++++++-
 src/ipa/libipa/exposure_mode_helper.cpp | 27 ++++++++++++-------------
 2 files changed, 19 insertions(+), 15 deletions(-)

Comments

Stefan Klug Nov. 9, 2026, 11:58 a.m. UTC | #1 
Hi Barnabás,

Quoting Barnabás Pőcze (2026-11-09 10:49:41)
> Currently `splitExposure()` will raise the exposure and analogue gain above
> what is specified in the last stage, if the desired exposure could not be
> reached. This behaviour is different from that of the raspberry pi ipa,
> and also could be surprising to the user / tuning file creator.
> 
> So make sure that the exposure time and analogue gain is limited by the
> last stage, in addition to the runtime controlled upper bound.
> 
> With this change, at least one stage is required, so an assertions are added.
s/an assertions are/add an assertion/ ?
> And the general agc algorithm is adjusted to provide stages for the fallback
> mode.
> 
> Note that the `stageGain` initialization has to be adjusted to retrieve the
> quantization gain, so that at the end of the loop
> 
>   * stageExposureTime
>   * stageGain
>   * quantGain
>   * quantGain2
> 
> are all up to date for the last stage.
> 
> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/work_items/362
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Looks good to me.

Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>

Best regards,
Stefan

> ---
>  src/ipa/libipa/agc.cpp                  |  7 ++++++-
>  src/ipa/libipa/exposure_mode_helper.cpp | 27 ++++++++++++-------------
>  2 files changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/src/ipa/libipa/agc.cpp b/src/ipa/libipa/agc.cpp
> index 6127859d0f..c1345787f7 100644
> --- a/src/ipa/libipa/agc.cpp
> +++ b/src/ipa/libipa/agc.cpp
> @@ -463,7 +463,12 @@ void addFallbacks(AgcMeanLuminance::Configuration &config)
>         if (config.exposureModeHelpers.empty())
>                 config.exposureModeHelpers.try_emplace(
>                         controls::AeExposureModeNormal,
> -                       std::vector<ExposureModeHelper::Stage>{});
> +                       std::vector{
> +                               ExposureModeHelper::Stage{
> +                                       .exposureTime = utils::Duration::max(),
> +                                       .gain = std::numeric_limits<double>::max(),
> +                               },
> +                       });
>  }
>  
>  } /* namespace */
> diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp
> index 4ec31f54c3..5c4242519b 100644
> --- a/src/ipa/libipa/exposure_mode_helper.cpp
> +++ b/src/ipa/libipa/exposure_mode_helper.cpp
> @@ -84,6 +84,7 @@ ExposureModeHelper::ExposureModeHelper(std::vector<Stage> &&stages)
>         : stages_(std::move(stages)), lineDuration_(1us), minExposureTime_(0us),
>           maxExposureTime_(0us), minGain_(0), maxGain_(0), sensorHelper_(nullptr)
>  {
> +       ASSERT(!stages_.empty());
>  }
>  
>  /**
> @@ -203,8 +204,8 @@ double ExposureModeHelper::clampGain(double gain, double *quantizationGain) cons
>   * value.
>   *
>   * If no combination of exposure time and gain limits is found that meets the
> - * required exposure, the helper falls-back to simply maximising the exposure
> - * time first, followed by analogue gain, followed by digital gain.
> + * required exposure, then the exposure and gain of the last stage will be used,
> + * and the digital gain will be increased as necessary.
>   *
>   * During the calculations the gain missed due to quantization is recorded and
>   * returned as quantization gain. The quantization gain is not included in the
> @@ -219,6 +220,7 @@ ExposureModeHelper::splitExposure(utils::Duration exposure) const
>  {
>         ASSERT(maxExposureTime_);
>         ASSERT(maxGain_);
> +       ASSERT(!stages_.empty());
>  
>         utils::Duration exposureTime;
>         double gain;
> @@ -240,13 +242,13 @@ ExposureModeHelper::splitExposure(utils::Duration exposure) const
>                          exposure / (exposureTime * gain * quantGain) };
>         }
>  
> -       double stageGain = clampGain(1.0);
> -       double lastStageGain = stageGain;
> +       utils::Duration stageExposureTime = {};
> +       double lastStageGain = clampGain(1.0);
> +       double stageGain = {};
>  
>         for (const auto &stage : stages_) {
> -               utils::Duration stageExposureTime = clampExposureTime(stage.exposureTime,
> -                                                                     &quantGain);
> -               stageGain = clampGain(stage.gain);
> +               stageExposureTime = clampExposureTime(stage.exposureTime, &quantGain);
> +               stageGain = clampGain(stage.gain, &quantGain2);
>  
>                 /*
>                  * We perform the clamping on both exposure time and gain in
> @@ -280,14 +282,11 @@ ExposureModeHelper::splitExposure(utils::Duration exposure) const
>         }
>  
>         /*
> -        * From here on all we can do is max out the exposure time, followed by
> -        * the analogue gain. If we still haven't achieved the target we send
> -        * the rest of the exposure time to digital gain. If we were given no
> -        * stages to use then the default stageGain of 1.0 is used so that
> -        * exposure time is maxed before gain is touched at all.
> +        * If no stages could satisfy the desired exposure, then use the last
> +        * stage's exposure time and gain, which is the maximum allowed.
>          */
> -       exposureTime = clampExposureTime(exposure / stageGain, &quantGain);
> -       gain = clampGain(exposure / exposureTime, &quantGain2);
> +       exposureTime = stageExposureTime;
> +       gain = stageGain;
>         quantGain *= quantGain2;
>  
>         return { exposureTime, gain, quantGain,
> -- 
> 2.56.0
>

Patch
diff mbox series 

diff --git a/src/ipa/libipa/agc.cpp b/src/ipa/libipa/agc.cpp
index 6127859d0f..c1345787f7 100644
--- a/src/ipa/libipa/agc.cpp
+++ b/src/ipa/libipa/agc.cpp
@@ -463,7 +463,12 @@  void addFallbacks(AgcMeanLuminance::Configuration &config)
 	if (config.exposureModeHelpers.empty())
 		config.exposureModeHelpers.try_emplace(
 			controls::AeExposureModeNormal,
-			std::vector<ExposureModeHelper::Stage>{});
+			std::vector{
+				ExposureModeHelper::Stage{
+					.exposureTime = utils::Duration::max(),
+					.gain = std::numeric_limits<double>::max(),
+				},
+			});
 }
 
 } /* namespace */
diff --git a/src/ipa/libipa/exposure_mode_helper.cpp b/src/ipa/libipa/exposure_mode_helper.cpp
index 4ec31f54c3..5c4242519b 100644
--- a/src/ipa/libipa/exposure_mode_helper.cpp
+++ b/src/ipa/libipa/exposure_mode_helper.cpp
@@ -84,6 +84,7 @@  ExposureModeHelper::ExposureModeHelper(std::vector<Stage> &&stages)
 	: stages_(std::move(stages)), lineDuration_(1us), minExposureTime_(0us),
 	  maxExposureTime_(0us), minGain_(0), maxGain_(0), sensorHelper_(nullptr)
 {
+	ASSERT(!stages_.empty());
 }
 
 /**
@@ -203,8 +204,8 @@  double ExposureModeHelper::clampGain(double gain, double *quantizationGain) cons
  * value.
  *
  * If no combination of exposure time and gain limits is found that meets the
- * required exposure, the helper falls-back to simply maximising the exposure
- * time first, followed by analogue gain, followed by digital gain.
+ * required exposure, then the exposure and gain of the last stage will be used,
+ * and the digital gain will be increased as necessary.
  *
  * During the calculations the gain missed due to quantization is recorded and
  * returned as quantization gain. The quantization gain is not included in the
@@ -219,6 +220,7 @@  ExposureModeHelper::splitExposure(utils::Duration exposure) const
 {
 	ASSERT(maxExposureTime_);
 	ASSERT(maxGain_);
+	ASSERT(!stages_.empty());
 
 	utils::Duration exposureTime;
 	double gain;
@@ -240,13 +242,13 @@  ExposureModeHelper::splitExposure(utils::Duration exposure) const
 			 exposure / (exposureTime * gain * quantGain) };
 	}
 
-	double stageGain = clampGain(1.0);
-	double lastStageGain = stageGain;
+	utils::Duration stageExposureTime = {};
+	double lastStageGain = clampGain(1.0);
+	double stageGain = {};
 
 	for (const auto &stage : stages_) {
-		utils::Duration stageExposureTime = clampExposureTime(stage.exposureTime,
-								      &quantGain);
-		stageGain = clampGain(stage.gain);
+		stageExposureTime = clampExposureTime(stage.exposureTime, &quantGain);
+		stageGain = clampGain(stage.gain, &quantGain2);
 
 		/*
 		 * We perform the clamping on both exposure time and gain in
@@ -280,14 +282,11 @@  ExposureModeHelper::splitExposure(utils::Duration exposure) const
 	}
 
 	/*
-	 * From here on all we can do is max out the exposure time, followed by
-	 * the analogue gain. If we still haven't achieved the target we send
-	 * the rest of the exposure time to digital gain. If we were given no
-	 * stages to use then the default stageGain of 1.0 is used so that
-	 * exposure time is maxed before gain is touched at all.
+	 * If no stages could satisfy the desired exposure, then use the last
+	 * stage's exposure time and gain, which is the maximum allowed.
 	 */
-	exposureTime = clampExposureTime(exposure / stageGain, &quantGain);
-	gain = clampGain(exposure / exposureTime, &quantGain2);
+	exposureTime = stageExposureTime;
+	gain = stageGain;
 	quantGain *= quantGain2;
 
 	return { exposureTime, gain, quantGain,