[RFC,v2,3/3] ipa: mali-c55: agc: Ignore invalid statistics
diff mbox series

Message ID 20261106125807.1238919-3-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • [RFC,v2,1/3] ipa: libipa: histogram: Add `empty()`
Related show

Commit Message

Barnabás Pőcze Nov. 6, 2026, 12:58 p.m. UTC
It is possible that, for example, the first frame has no valid statistics
yet. In that case the histograms are empty. This is problematic because
e.g. `interQuantileMean()` may return NaN, which causes issues in the
calculations (this affects both the color temperature calculation and
`AgcTraits::estimanteLuminance()` as well).

Previously this has not manifested itself because implementation details in
`AgcMeanLuminance` prevented the NaNs from propagating, but 25cb6cd274d3
("libipa: agc_mean_luminance: Improve inital gain estimation") has changed that.

So ignore statistics if any of the color histograms are empty.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/ipa/mali-c55/algorithms/agc.cpp | 19 +++++++++++++------
 src/ipa/mali-c55/algorithms/agc.h   |  5 +++++
 2 files changed, 18 insertions(+), 6 deletions(-)

Comments

Kieran Bingham Nov. 6, 2026, 3:01 p.m. UTC | #1
Quoting Barnabás Pőcze (2026-11-06 12:58:07)
> It is possible that, for example, the first frame has no valid statistics
> yet. In that case the histograms are empty. This is problematic because
> e.g. `interQuantileMean()` may return NaN, which causes issues in the
> calculations (this affects both the color temperature calculation and
> `AgcTraits::estimanteLuminance()` as well).
> 
> Previously this has not manifested itself because implementation details in
> `AgcMeanLuminance` prevented the NaNs from propagating, but 25cb6cd274d3
> ("libipa: agc_mean_luminance: Improve inital gain estimation") has changed that.
> 
> So ignore statistics if any of the color histograms are empty.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

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

> ---
>  src/ipa/mali-c55/algorithms/agc.cpp | 19 +++++++++++++------
>  src/ipa/mali-c55/algorithms/agc.h   |  5 +++++
>  2 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp
> index bf9d3f7d4a..1b7687e723 100644
> --- a/src/ipa/mali-c55/algorithms/agc.cpp
> +++ b/src/ipa/mali-c55/algorithms/agc.cpp
> @@ -257,13 +257,20 @@ void Agc::process(IPAContext &context,
>         }
>  
>         statistics_.parseStatistics(stats);
> +       if (!statistics_.isValid()) {
> +               LOG(MaliC55Agc, Debug) << "Ignoring invalid statistics";
> +               agc_.process(context.configuration.agc, context.activeState.agc, frameContext.agc, {}, metadata);
> +               return;
> +       }
>  
> -       agc_.process(context.configuration.agc, context.activeState.agc, frameContext.agc, {{
> -               .traits = AgcTraits(statistics_),
> -               .yHist = statistics_.yHist,
> -               .exposure = frameContext.sensor.exposure,
> -               .gain = frameContext.sensor.gain,
> -       }}, metadata);
> +       agc_.process(context.configuration.agc, context.activeState.agc, frameContext.agc,
> +                    { {
> +                            .traits = AgcTraits(statistics_),
> +                            .yHist = statistics_.yHist,
> +                            .exposure = frameContext.sensor.exposure,
> +                            .gain = frameContext.sensor.gain,
> +                    } },
> +                    metadata);
>  }
>  
>  REGISTER_IPA_ALGORITHM(Agc, "Agc")
> diff --git a/src/ipa/mali-c55/algorithms/agc.h b/src/ipa/mali-c55/algorithms/agc.h
> index 809a57c53a..998ef57210 100644
> --- a/src/ipa/mali-c55/algorithms/agc.h
> +++ b/src/ipa/mali-c55/algorithms/agc.h
> @@ -32,6 +32,11 @@ public:
>         uint32_t decodeBinValue(uint16_t binVal);
>         void parseStatistics(const mali_c55_stats_buffer *stats);
>  
> +       bool isValid() const
> +       {
> +               return !rHist.empty() && !gHist.empty() && !bHist.empty();
> +       }
> +
>         Histogram rHist;
>         Histogram gHist;
>         Histogram bHist;
> -- 
> 2.56.0
>

Patch
diff mbox series

diff --git a/src/ipa/mali-c55/algorithms/agc.cpp b/src/ipa/mali-c55/algorithms/agc.cpp
index bf9d3f7d4a..1b7687e723 100644
--- a/src/ipa/mali-c55/algorithms/agc.cpp
+++ b/src/ipa/mali-c55/algorithms/agc.cpp
@@ -257,13 +257,20 @@  void Agc::process(IPAContext &context,
 	}
 
 	statistics_.parseStatistics(stats);
+	if (!statistics_.isValid()) {
+		LOG(MaliC55Agc, Debug) << "Ignoring invalid statistics";
+		agc_.process(context.configuration.agc, context.activeState.agc, frameContext.agc, {}, metadata);
+		return;
+	}
 
-	agc_.process(context.configuration.agc, context.activeState.agc, frameContext.agc, {{
-		.traits = AgcTraits(statistics_),
-		.yHist = statistics_.yHist,
-		.exposure = frameContext.sensor.exposure,
-		.gain = frameContext.sensor.gain,
-	}}, metadata);
+	agc_.process(context.configuration.agc, context.activeState.agc, frameContext.agc,
+		     { {
+			     .traits = AgcTraits(statistics_),
+			     .yHist = statistics_.yHist,
+			     .exposure = frameContext.sensor.exposure,
+			     .gain = frameContext.sensor.gain,
+		     } },
+		     metadata);
 }
 
 REGISTER_IPA_ALGORITHM(Agc, "Agc")
diff --git a/src/ipa/mali-c55/algorithms/agc.h b/src/ipa/mali-c55/algorithms/agc.h
index 809a57c53a..998ef57210 100644
--- a/src/ipa/mali-c55/algorithms/agc.h
+++ b/src/ipa/mali-c55/algorithms/agc.h
@@ -32,6 +32,11 @@  public:
 	uint32_t decodeBinValue(uint16_t binVal);
 	void parseStatistics(const mali_c55_stats_buffer *stats);
 
+	bool isValid() const
+	{
+		return !rHist.empty() && !gHist.empty() && !bHist.empty();
+	}
+
 	Histogram rHist;
 	Histogram gHist;
 	Histogram bHist;