[RFC,v1,7/8] ipa: libipa: agc: Rework frame duration limit calculation
diff mbox series

Message ID 20260827104108.1432632-8-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 Aug. 27, 2026, 10:41 a.m. UTC
The frame duration is calculated as frameHeight * lineDuration, but
it was effectively repeating the linu duration calculation, but with
microsecond precision. Instead, simply reuse the already calculated
line duration.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/ipa/libipa/agc.cpp | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

Comments

Jacopo Mondi Aug. 27, 2026, 3:22 p.m. UTC | #1
Hi Barnabás

On Thu, Aug 27, 2026 at 12:41:07PM +0200, Barnabás Pőcze wrote:
> The frame duration is calculated as frameHeight * lineDuration, but
> it was effectively repeating the linu duration calculation, but with

s/linu/line

> microsecond precision. Instead, simply reuse the already calculated
> line duration.
>
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

> ---
>  src/ipa/libipa/agc.cpp | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/src/ipa/libipa/agc.cpp b/src/ipa/libipa/agc.cpp
> index ce5b2ab6f6..6783055a5e 100644
> --- a/src/ipa/libipa/agc.cpp
> +++ b/src/ipa/libipa/agc.cpp
> @@ -396,18 +396,14 @@ int AgcAlgorithm::configure(agc::Session &session, agc::ActiveState &state,
>  	 */
>
>  	const ControlInfo &v4l2VBlank = config.sensorControls.find(V4L2_CID_VBLANK)->second;
> -	std::array<uint32_t, 3> frameHeights{
> -		v4l2VBlank.min().get<int32_t>() + config.sensorInfo.outputSize.height,
> -		v4l2VBlank.max().get<int32_t>() + config.sensorInfo.outputSize.height,
> -		v4l2VBlank.def().get<int32_t>() + config.sensorInfo.outputSize.height,
> +	const struct {
> +		uint32_t min;
> +		uint32_t max;
> +	} frameHeights = {
> +		.min = v4l2VBlank.min().get<int32_t>() + config.sensorInfo.outputSize.height,
> +		.max = v4l2VBlank.max().get<int32_t>() + config.sensorInfo.outputSize.height,
>  	};
>
> -	std::array<int64_t, 3> frameDurations;
> -	for (unsigned int i = 0; i < frameHeights.size(); ++i) {
> -		uint64_t frameSize = static_cast<uint64_t>(lineLength) * frameHeights[i];
> -		frameDurations[i] = frameSize * 1000000U / config.sensorInfo.pixelRate;
> -	}
> -
>  	/*
>  	 * When the AGC computes the new exposure values for a frame, it needs
>  	 * to know the limits for exposure time and analogue gain. As it depends
> @@ -420,8 +416,8 @@ int AgcAlgorithm::configure(agc::Session &session, agc::ActiveState &state,
>  	session.minAnalogueGain = minGain;
>  	session.maxAnalogueGain = maxGain;
>  	session.defAnalogueGain = defGain;
> -	session.minFrameDuration = std::chrono::microseconds(frameDurations[0]);
> -	session.maxFrameDuration = std::chrono::microseconds(frameDurations[1]);
> +	session.minFrameDuration = frameHeights.min * session.lineDuration;
> +	session.maxFrameDuration = frameHeights.max * session.lineDuration;
>
>  	/* Configure the default exposure and gain. */
>  	state = {};
> @@ -460,8 +456,12 @@ int AgcAlgorithm::configure(agc::Session &session, agc::ActiveState &state,
>  		static_cast<int32_t>(defExposure * lineDurationUs),
>  	};
>  	config.ctrlMap[&controls::FrameDurationLimits] = ControlInfo{
> -		frameDurations[0], frameDurations[1],
> -		Span<const int64_t, 2>{ { frameDurations[0], frameDurations[1] } },
> +		static_cast<int64_t>(session.minFrameDuration.get<std::micro>()),
> +		static_cast<int64_t>(session.maxFrameDuration.get<std::micro>()),
> +		Span<const int64_t, 2>{{
> +			static_cast<int64_t>(state.minFrameDuration.get<std::micro>()),
> +			static_cast<int64_t>(state.maxFrameDuration.get<std::micro>()),
> +		}},
>  	};
>
>  	const auto add = [&](const ControlId &cid, const auto &automatic, const auto &manual) {
> --
> 2.55.0
>

Patch
diff mbox series

diff --git a/src/ipa/libipa/agc.cpp b/src/ipa/libipa/agc.cpp
index ce5b2ab6f6..6783055a5e 100644
--- a/src/ipa/libipa/agc.cpp
+++ b/src/ipa/libipa/agc.cpp
@@ -396,18 +396,14 @@  int AgcAlgorithm::configure(agc::Session &session, agc::ActiveState &state,
 	 */
 
 	const ControlInfo &v4l2VBlank = config.sensorControls.find(V4L2_CID_VBLANK)->second;
-	std::array<uint32_t, 3> frameHeights{
-		v4l2VBlank.min().get<int32_t>() + config.sensorInfo.outputSize.height,
-		v4l2VBlank.max().get<int32_t>() + config.sensorInfo.outputSize.height,
-		v4l2VBlank.def().get<int32_t>() + config.sensorInfo.outputSize.height,
+	const struct {
+		uint32_t min;
+		uint32_t max;
+	} frameHeights = {
+		.min = v4l2VBlank.min().get<int32_t>() + config.sensorInfo.outputSize.height,
+		.max = v4l2VBlank.max().get<int32_t>() + config.sensorInfo.outputSize.height,
 	};
 
-	std::array<int64_t, 3> frameDurations;
-	for (unsigned int i = 0; i < frameHeights.size(); ++i) {
-		uint64_t frameSize = static_cast<uint64_t>(lineLength) * frameHeights[i];
-		frameDurations[i] = frameSize * 1000000U / config.sensorInfo.pixelRate;
-	}
-
 	/*
 	 * When the AGC computes the new exposure values for a frame, it needs
 	 * to know the limits for exposure time and analogue gain. As it depends
@@ -420,8 +416,8 @@  int AgcAlgorithm::configure(agc::Session &session, agc::ActiveState &state,
 	session.minAnalogueGain = minGain;
 	session.maxAnalogueGain = maxGain;
 	session.defAnalogueGain = defGain;
-	session.minFrameDuration = std::chrono::microseconds(frameDurations[0]);
-	session.maxFrameDuration = std::chrono::microseconds(frameDurations[1]);
+	session.minFrameDuration = frameHeights.min * session.lineDuration;
+	session.maxFrameDuration = frameHeights.max * session.lineDuration;
 
 	/* Configure the default exposure and gain. */
 	state = {};
@@ -460,8 +456,12 @@  int AgcAlgorithm::configure(agc::Session &session, agc::ActiveState &state,
 		static_cast<int32_t>(defExposure * lineDurationUs),
 	};
 	config.ctrlMap[&controls::FrameDurationLimits] = ControlInfo{
-		frameDurations[0], frameDurations[1],
-		Span<const int64_t, 2>{ { frameDurations[0], frameDurations[1] } },
+		static_cast<int64_t>(session.minFrameDuration.get<std::micro>()),
+		static_cast<int64_t>(session.maxFrameDuration.get<std::micro>()),
+		Span<const int64_t, 2>{{
+			static_cast<int64_t>(state.minFrameDuration.get<std::micro>()),
+			static_cast<int64_t>(state.maxFrameDuration.get<std::micro>()),
+		}},
 	};
 
 	const auto add = [&](const ControlId &cid, const auto &automatic, const auto &manual) {