[{"id":27961,"web_url":"https://patchwork.libcamera.org/comment/27961/","msgid":"<2cjpnawqfsrs5lgmntsvbbe7xpw2ncmca6w2ezn3qqetl662to@nm3admwgench>","date":"2023-10-12T10:54:43","subject":"Re: [libcamera-devel] [PATCH 19/20] ipa: rpi: agc: Avoid\n\toverwriting caller's statistics pointer","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Fri, Oct 06, 2023 at 02:19:59PM +0100, Naushir Patuck via libcamera-devel wrote:\n> From: David Plowman <david.plowman@raspberrypi.com>\n>\n> The code was inadvertently overwriting the caller's StatisticsPtr,\n> meaning that subsequent algorithms would get the wrong image\n> statistics when AGC channels changed.\n>\n> This could be fix using std::ref, though I find the C-style pointer\n> fix easier to understand!\n>\n> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>\n> Reviewed-by: Naushir Patuck <naush@raspberrypi.com>\n\nReviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>\n\n> ---\n>  src/ipa/rpi/controller/rpi/agc.cpp | 5 +++--\n>  1 file changed, 3 insertions(+), 2 deletions(-)\n>\n> diff --git a/src/ipa/rpi/controller/rpi/agc.cpp b/src/ipa/rpi/controller/rpi/agc.cpp\n> index 870cb4315c42..758da0719b9b 100644\n> --- a/src/ipa/rpi/controller/rpi/agc.cpp\n> +++ b/src/ipa/rpi/controller/rpi/agc.cpp\n> @@ -311,15 +311,16 @@ void Agc::process(StatisticsPtr &stats, Metadata *imageMetadata)\n>  \t * exist, and call process(). We must make the agc.status metadata record correctly\n>  \t * which channel this is.\n>  \t */\n> +\tStatisticsPtr *statsPtr = &stats;\n>  \tif (channelData.statistics && channelData.deviceStatus) {\n>  \t\tdeviceStatus = *channelData.deviceStatus;\n> -\t\tstats = channelData.statistics;\n> +\t\tstatsPtr = &channelData.statistics;\n>  \t} else {\n>  \t\t/* Can also happen when new channels start. */\n>  \t\tLOG(RPiAgc, Debug) << \"process: channel \" << channelIndex << \" not seen yet\";\n>  \t}\n>\n> -\tchannelData.channel.process(stats, deviceStatus, imageMetadata, channelTotalExposures_);\n> +\tchannelData.channel.process(*statsPtr, deviceStatus, imageMetadata, channelTotalExposures_);\n>  \tauto dur = setCurrentChannelIndexGetExposure(imageMetadata, \"process: no AGC status found\",\n>  \t\t\t\t\t\t     channelIndex);\n>  \tif (dur)\n> --\n> 2.34.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8A985C3272\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 12 Oct 2023 10:54:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 114DA6297C;\n\tThu, 12 Oct 2023 12:54:48 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 42AEB6296D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 12 Oct 2023 12:54:46 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0EE3C7FC;\n\tThu, 12 Oct 2023 12:54:43 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1697108088;\n\tbh=3yQ1z12dqMDJA0vV3aM6/PymnHrtE7+4fzHYUYo2zuE=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=nk21XDJopeGoD/BYnMkMNiBskZgToSXuNX36z5kgaCSrJdQ54HEpklnd7/AWgXC9H\n\tAUXWUO14G5QaDsR/33KDmh4u/k8/4/L7XGm6orrieL+E4Aw5Yig80skwak4m+D0UWy\n\tOZrpx8g8+r27bA61MKSn/cvqK2i+8BIKhQyTCsjf9biOaHFYG0pdJJzugvkT3cWffG\n\tZbDlOHQwwqmIaq7BvRJZIwixl7vhm2qu6fwJSXyI9CYrKCl5L+GfQW1d9Pjv0/4KUf\n\tIN9Uud45b2zdytJH1KapGysiyaBcerCmsgVTb2W1myhYmTqQDFuxWhrj3NTjiZni9i\n\t7BLoz3SpFN44g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1697108083;\n\tbh=3yQ1z12dqMDJA0vV3aM6/PymnHrtE7+4fzHYUYo2zuE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Bpyd95e1W52Gx1RmWwIU2iGj9XgVpHNy7RWzAK7ZXoJCiaGMz2MNwZC22sdbLIKCr\n\t+h1OTXtQGhld8Yt0veAP7q0gu8X05wc90SREp9mjHIm8CE8ykBHhrN5oxV4l0Eh3In\n\tA7xHj9ndOWLAWZB0Mg8GsUw1fMXJGCi4o4LvhIhM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Bpyd95e1\"; dkim-atps=neutral","Date":"Thu, 12 Oct 2023 12:54:43 +0200","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<2cjpnawqfsrs5lgmntsvbbe7xpw2ncmca6w2ezn3qqetl662to@nm3admwgench>","References":"<20231006132000.23504-1-naush@raspberrypi.com>\n\t<20231006132000.23504-20-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20231006132000.23504-20-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 19/20] ipa: rpi: agc: Avoid\n\toverwriting caller's statistics pointer","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo.mondi@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]