[libcamera-devel] [PATCH 19/20] ipa: rpi: agc: Avoid overwriting caller's statistics pointer

[Jacopo Mondi]

Hi Naush

On Fri, Oct 06, 2023 at 02:19:59PM +0100, Naushir Patuck via libcamera-devel wrote:
> From: David Plowman <david.plowman at raspberrypi.com>
>
> The code was inadvertently overwriting the caller's StatisticsPtr,
> meaning that subsequent algorithms would get the wrong image
> statistics when AGC channels changed.
>
> This could be fix using std::ref, though I find the C-style pointer
> fix easier to understand!
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> Reviewed-by: Naushir Patuck <naush at raspberrypi.com>

Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>

> ---
>  src/ipa/rpi/controller/rpi/agc.cpp | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp
> index 870cb4315c42..758da0719b9b 100644
> --- a/src/ipa/rpi/controller/rpi/agc.cpp
> +++ b/src/ipa/rpi/controller/rpi/agc.cpp
> @@ -311,15 +311,16 @@ void Agc::process(StatisticsPtr &stats, Metadata *imageMetadata)
>  	 * exist, and call process(). We must make the agc.status metadata record correctly
>  	 * which channel this is.
>  	 */
> +	StatisticsPtr *statsPtr = &stats;
>  	if (channelData.statistics && channelData.deviceStatus) {
>  		deviceStatus = *channelData.deviceStatus;
> -		stats = channelData.statistics;
> +		statsPtr = &channelData.statistics;
>  	} else {
>  		/* Can also happen when new channels start. */
>  		LOG(RPiAgc, Debug) << "process: channel " << channelIndex << " not seen yet";
>  	}
>
> -	channelData.channel.process(stats, deviceStatus, imageMetadata, channelTotalExposures_);
> +	channelData.channel.process(*statsPtr, deviceStatus, imageMetadata, channelTotalExposures_);
>  	auto dur = setCurrentChannelIndexGetExposure(imageMetadata, "process: no AGC status found",
>  						     channelIndex);
>  	if (dur)
> --
> 2.34.1
>