| Message ID | 20250911135144.56586-5-mzamazal@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
On 9/11/25 15:51, Milan Zamazal wrote: > The R/G/B sums computed in AWB simple IPA may be zero or perhaps even > negative. Let's make sure the sums are always positive, to prevent > division by zero or completely nonsense results. > > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > --- > src/ipa/simple/algorithms/awb.cpp | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp > index cf567e894..9509129e9 100644 > --- a/src/ipa/simple/algorithms/awb.cpp > +++ b/src/ipa/simple/algorithms/awb.cpp > @@ -7,6 +7,7 @@ > > #include "awb.h" > > +#include <algorithm> > #include <numeric> > #include <stdint.h> > > @@ -69,9 +70,10 @@ void Awb::process(IPAContext &context, > const uint64_t nPixels = std::accumulate( > histogram.begin(), histogram.end(), 0); > const uint64_t offset = blackLevel * nPixels; > - const uint64_t sumR = stats->sumR_ - offset / 4; > - const uint64_t sumG = stats->sumG_ - offset / 2; > - const uint64_t sumB = stats->sumB_ - offset / 4; > + const uint64_t minValid = 1; > + const uint64_t sumR = std::max(stats->sumR_ - offset / 4, minValid); > + const uint64_t sumG = std::max(stats->sumG_ - offset / 2, minValid); > + const uint64_t sumB = std::max(stats->sumB_ - offset / 4, minValid); It looks like stats->sumR_ and offset are both unsigned integers so I guess they won't ever go negative? In this case if (offset / 4) > stats->sumR_ the result will be a large positive value. Thanks, Macoej
Hi Maciej, thank you for review. "Maciej S. Szmigiero" <mail@maciej.szmigiero.name> writes: > On 9/11/25 15:51, Milan Zamazal wrote: >> The R/G/B sums computed in AWB simple IPA may be zero or perhaps even >> negative. Let's make sure the sums are always positive, to prevent > >> division by zero or completely nonsense results. >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com> >> --- >> src/ipa/simple/algorithms/awb.cpp | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp >> index cf567e894..9509129e9 100644 >> --- a/src/ipa/simple/algorithms/awb.cpp >> +++ b/src/ipa/simple/algorithms/awb.cpp >> @@ -7,6 +7,7 @@ >> #include "awb.h" >> +#include <algorithm> >> #include <numeric> >> #include <stdint.h> >> @@ -69,9 +70,10 @@ void Awb::process(IPAContext &context, >> const uint64_t nPixels = std::accumulate( >> histogram.begin(), histogram.end(), 0); >> const uint64_t offset = blackLevel * nPixels; >> - const uint64_t sumR = stats->sumR_ - offset / 4; >> - const uint64_t sumG = stats->sumG_ - offset / 2; >> - const uint64_t sumB = stats->sumB_ - offset / 4; >> + const uint64_t minValid = 1; >> + const uint64_t sumR = std::max(stats->sumR_ - offset / 4, minValid); >> + const uint64_t sumG = std::max(stats->sumG_ - offset / 2, minValid); >> + const uint64_t sumB = std::max(stats->sumB_ - offset / 4, minValid); > > It looks like stats->sumR_ and offset are both unsigned integers so > I guess they won't ever go negative? > > In this case if (offset / 4) > stats->sumR_ the result will be a large > positive value. Good catch, I'll fix it in v3. (In theory, the sums shouldn't be below the black level offset, but in practice they can.) > Thanks, > Macoej
diff --git a/src/ipa/simple/algorithms/awb.cpp b/src/ipa/simple/algorithms/awb.cpp index cf567e894..9509129e9 100644 --- a/src/ipa/simple/algorithms/awb.cpp +++ b/src/ipa/simple/algorithms/awb.cpp @@ -7,6 +7,7 @@ #include "awb.h" +#include <algorithm> #include <numeric> #include <stdint.h> @@ -69,9 +70,10 @@ void Awb::process(IPAContext &context, const uint64_t nPixels = std::accumulate( histogram.begin(), histogram.end(), 0); const uint64_t offset = blackLevel * nPixels; - const uint64_t sumR = stats->sumR_ - offset / 4; - const uint64_t sumG = stats->sumG_ - offset / 2; - const uint64_t sumB = stats->sumB_ - offset / 4; + const uint64_t minValid = 1; + const uint64_t sumR = std::max(stats->sumR_ - offset / 4, minValid); + const uint64_t sumG = std::max(stats->sumG_ - offset / 2, minValid); + const uint64_t sumB = std::max(stats->sumB_ - offset / 4, minValid); /* * Calculate red and blue gains for AWB.
The R/G/B sums computed in AWB simple IPA may be zero or perhaps even negative. Let's make sure the sums are always positive, to prevent division by zero or completely nonsense results. Signed-off-by: Milan Zamazal <mzamazal@redhat.com> --- src/ipa/simple/algorithms/awb.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)