[libcamera-devel] [PATCH v1 3/5] ipa: raspberrypi: awb: Delay release of the statistics buffer

[David Plowman]

Hi Naush

Thanks for the patch.

On Tue, 22 Nov 2022 at 11:22, Naushir Patuck via libcamera-devel
<libcamera-devel at lists.libcamera.org> wrote:
>
> Release the statistics buffer after running the through the AWB calculations.
> Only the "counted" statistics are copied out to a local structure, so keeping
> the statistics buffer allows the algorithm to see the "uncounted" statistics as
> well.

I guess I still feel a bit weird about this. The code used to copy out
everything it needed so that it could release the buffer as soon as
possible. Maybe that's not a meaningful concern any more...?

>
> This is currently handled by hard-coding the total number of statistics regions
> regions based on the structure definition in the bcm2835_isp_stats structure.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>

In spite of my probably unnecessary weird feelings, it's probably of
no consequence, so:

Reviewed-by: David Plowman <david.plowman at raspberrypi.com>

Thanks!
David

> ---
>  src/ipa/raspberrypi/controller/rpi/awb.cpp | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> index 8d8ddf0913f7..861022014896 100644
> --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> @@ -432,11 +432,6 @@ void Awb::prepareStats()
>          */
>         generateStats(zones_, statistics_->awb_stats, config_.minPixels,
>                       config_.minG);
> -       /*
> -        * we're done with these; we may as well relinquish our hold on the
> -        * pointer.
> -        */
> -       statistics_.reset();
>         /*
>          * apply sensitivities, so values appear to come from our "canonical"
>          * sensor.
> @@ -733,6 +728,11 @@ void Awb::doAwb()
>                         << " with gains r " << asyncResults_.gainR
>                         << " and b " << asyncResults_.gainB;
>         }
> +       /*
> +        * we're done with these; we may as well relinquish our hold on the
> +        * pointer.
> +        */
> +       statistics_.reset();
>  }
>
>  /* Register algorithm with the system. */
> --
> 2.25.1
>