[3/5] ipa: rpi: pisp: Use a floating region to get whole image Y statistics

24700 diff mbox series

Floating regions are currently unused on the PiSP platform, so we can
use one of them to get an average Y value for the whole image.

If an algorithm (such as the "lux" algorithm) wants a scene-referred
estimate of absolute brightness, then this would be a better choice
than a value from Y histogram as the latter is not invariant to the
metering mode (e.g. centre-weighted, spot etc.). (So note that for the
AGC/AEC algorithm, where the metering mode is relevant, the Y
histogram is the appropriate choice.)

We also fix the loop that copies the hardware's floating region values
into our statistics structure; it was using the wrong limit which was
causing it not to do anything.

A future commit will update the lux algorithm to use this feature.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/ipa/rpi/pisp/pisp.cpp | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Hi David,

Quoting David Plowman (2025-10-17 12:05:39)
> Floating regions are currently unused on the PiSP platform, so we can
> use one of them to get an average Y value for the whole image.
> 
> If an algorithm (such as the "lux" algorithm) wants a scene-referred
> estimate of absolute brightness, then this would be a better choice
> than a value from Y histogram as the latter is not invariant to the
> metering mode (e.g. centre-weighted, spot etc.). (So note that for the
> AGC/AEC algorithm, where the metering mode is relevant, the Y
> histogram is the appropriate choice.)
> 
> We also fix the loop that copies the hardware's floating region values
> into our statistics structure; it was using the wrong limit which was
> causing it not to do anything.
> 
> A future commit will update the lux algorithm to use this feature.
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>

I can't really comment on the hardware details. But it looks good to me
:-)

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

Cheers,
Stefan

> ---
>  src/ipa/rpi/pisp/pisp.cpp | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/rpi/pisp/pisp.cpp b/src/ipa/rpi/pisp/pisp.cpp
> index bb50a9e0..2ba879c1 100644
> --- a/src/ipa/rpi/pisp/pisp.cpp
> +++ b/src/ipa/rpi/pisp/pisp.cpp
> @@ -467,7 +467,7 @@ RPiController::StatisticsPtr IpaPiSP::platformProcessStats(Span<uint8_t> mem)
>  
>         /* AGC region sums only get collected on floating zones. */
>         statistics->agcRegions.init({ 0, 0 }, PISP_FLOATING_STATS_NUM_ZONES);
> -       for (i = 0; i < statistics->agcRegions.numRegions(); i++)
> +       for (i = 0; i < PISP_FLOATING_STATS_NUM_ZONES; i++)
>                 statistics->agcRegions.setFloating(i,
>                                                    { { 0, 0, 0, stats->agc.floating[i].Y_sum },
>                                                      stats->agc.floating[i].counted, 0 });
> @@ -985,6 +985,12 @@ void IpaPiSP::setStatsAndDebin()
>          */
>         setHistogramWeights();
>  
> +       /* Configure the first AGC floating region to cover the whole image, for the lux algo. */
> +       pisp_fe_floating_stats_config floatingStatsConfig = {};
> +       floatingStatsConfig.regions[0].size_x = mode_.width;
> +       floatingStatsConfig.regions[0].size_y = mode_.height;
> +       fe_->SetFloatingStats(floatingStatsConfig);
> +
>         pisp_be_global_config beGlobal;
>         be_->GetGlobal(beGlobal);
>  
> -- 
> 2.47.3
>

Hi David,

On Fri, 17 Oct 2025 at 11:27, David Plowman <david.plowman@raspberrypi.com>
wrote:

> Floating regions are currently unused on the PiSP platform, so we can
> use one of them to get an average Y value for the whole image.
>
> If an algorithm (such as the "lux" algorithm) wants a scene-referred
> estimate of absolute brightness, then this would be a better choice
> than a value from Y histogram as the latter is not invariant to the
> metering mode (e.g. centre-weighted, spot etc.). (So note that for the
> AGC/AEC algorithm, where the metering mode is relevant, the Y
> histogram is the appropriate choice.)
>
> We also fix the loop that copies the hardware's floating region values
> into our statistics structure; it was using the wrong limit which was
> causing it not to do anything.
>
> A future commit will update the lux algorithm to use this feature.
>
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
>

Reviewed-by: Naushir Patuck <naush@raspberrypi.com>


> ---
>  src/ipa/rpi/pisp/pisp.cpp | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/src/ipa/rpi/pisp/pisp.cpp b/src/ipa/rpi/pisp/pisp.cpp
> index bb50a9e0..2ba879c1 100644
> --- a/src/ipa/rpi/pisp/pisp.cpp
> +++ b/src/ipa/rpi/pisp/pisp.cpp
> @@ -467,7 +467,7 @@ RPiController::StatisticsPtr
> IpaPiSP::platformProcessStats(Span<uint8_t> mem)
>
>         /* AGC region sums only get collected on floating zones. */
>         statistics->agcRegions.init({ 0, 0 },
> PISP_FLOATING_STATS_NUM_ZONES);
> -       for (i = 0; i < statistics->agcRegions.numRegions(); i++)
> +       for (i = 0; i < PISP_FLOATING_STATS_NUM_ZONES; i++)
>                 statistics->agcRegions.setFloating(i,
>                                                    { { 0, 0, 0,
> stats->agc.floating[i].Y_sum },
>
>  stats->agc.floating[i].counted, 0 });
> @@ -985,6 +985,12 @@ void IpaPiSP::setStatsAndDebin()
>          */
>         setHistogramWeights();
>
> +       /* Configure the first AGC floating region to cover the whole
> image, for the lux algo. */
> +       pisp_fe_floating_stats_config floatingStatsConfig = {};
> +       floatingStatsConfig.regions[0].size_x = mode_.width;
> +       floatingStatsConfig.regions[0].size_y = mode_.height;
> +       fe_->SetFloatingStats(floatingStatsConfig);
> +
>         pisp_be_global_config beGlobal;
>         be_->GetGlobal(beGlobal);
>
> --
> 2.47.3
>
>