[RFC,v3,28/50] ipa: libipa: agc: Fix multiplication type
diff mbox series

Message ID 20260803131435.153927-29-barnabas.pocze@ideasonboard.com
State Superseded
Headers show
Series
  • ipa: libipa: agc rework
Related show

Commit Message

Barnabás Pőcze Aug. 3, 2026, 1:14 p.m. UTC
The type of `frameSize` is `uint64_t`, but it is assigned the result
of a 32-bit multiplication. So make one of the terms a 64-bit integer
so that the result will be actually 64-bit.

Furthermore, also adjust the calculation to do the multiplication first
just to be as accurate as possible. No wraparound will happen as long
as the frame size is less than about 16.7 TiB.

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

Comments

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

On Mon, Aug 03, 2026 at 03:14:13PM +0200, Barnabás Pőcze wrote:
> The type of `frameSize` is `uint64_t`, but it is assigned the result
> of a 32-bit multiplication. So make one of the terms a 64-bit integer
> so that the result will be actually 64-bit.
>
> Furthermore, also adjust the calculation to do the multiplication first
> just to be as accurate as possible. No wraparound will happen as long
> as the frame size is less than about 16.7 TiB.

We -should- be safe

>
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  src/ipa/libipa/agc.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/ipa/libipa/agc.cpp b/src/ipa/libipa/agc.cpp
> index 0f787925cb..f420dfdce4 100644
> --- a/src/ipa/libipa/agc.cpp
> +++ b/src/ipa/libipa/agc.cpp
> @@ -297,8 +297,8 @@ int AgcAlgorithm::configure(agc::Session &session, agc::ActiveState &state,
>
>  	std::array<int64_t, 3> frameDurations;
>  	for (unsigned int i = 0; i < frameHeights.size(); ++i) {
> -		uint64_t frameSize = lineLength * frameHeights[i];
> -		frameDurations[i] = frameSize / (config.sensorInfo.pixelRate / 1000000U);
> +		uint64_t frameSize = uint64_t(lineLength) * frameHeights[i];

static_cast<> is preferred over plain C casts

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

Thanks
  j

> +		frameDurations[i] = frameSize * 1000000U / config.sensorInfo.pixelRate;
>  	}
>
>  	/*
> --
> 2.55.0
>

Patch
diff mbox series

diff --git a/src/ipa/libipa/agc.cpp b/src/ipa/libipa/agc.cpp
index 0f787925cb..f420dfdce4 100644
--- a/src/ipa/libipa/agc.cpp
+++ b/src/ipa/libipa/agc.cpp
@@ -297,8 +297,8 @@  int AgcAlgorithm::configure(agc::Session &session, agc::ActiveState &state,
 
 	std::array<int64_t, 3> frameDurations;
 	for (unsigned int i = 0; i < frameHeights.size(); ++i) {
-		uint64_t frameSize = lineLength * frameHeights[i];
-		frameDurations[i] = frameSize / (config.sensorInfo.pixelRate / 1000000U);
+		uint64_t frameSize = uint64_t(lineLength) * frameHeights[i];
+		frameDurations[i] = frameSize * 1000000U / config.sensorInfo.pixelRate;
 	}
 
 	/*