[{"id":25878,"web_url":"https://patchwork.libcamera.org/comment/25878/","msgid":"","date":"2022-11-23T13:51:53","subject":"Re: [libcamera-devel] [PATCH v1 3/5] ipa: raspberrypi: awb: Delay\n\trelease of the statistics buffer","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for the patch.\n\nOn Tue, 22 Nov 2022 at 11:22, Naushir Patuck via libcamera-devel\n wrote:\n>\n> Release the statistics buffer after running the through the AWB calculations.\n> Only the \"counted\" statistics are copied out to a local structure, so keeping\n> the statistics buffer allows the algorithm to see the \"uncounted\" statistics as\n> well.\n\nI guess I still feel a bit weird about this. The code used to copy out\neverything it needed so that it could release the buffer as soon as\npossible. Maybe that's not a meaningful concern any more...?\n\n>\n> This is currently handled by hard-coding the total number of statistics regions\n> regions based on the structure definition in the bcm2835_isp_stats structure.\n>\n> Signed-off-by: Naushir Patuck \n\nIn spite of my probably unnecessary weird feelings, it's probably of\nno consequence, so:\n\nReviewed-by: David Plowman \n\nThanks!\nDavid\n\n> ---\n> src/ipa/raspberrypi/controller/rpi/awb.cpp | 10 +++++-----\n> 1 file changed, 5 insertions(+), 5 deletions(-)\n>\n> diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> index 8d8ddf0913f7..861022014896 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> @@ -432,11 +432,6 @@ void Awb::prepareStats()\n> */\n> generateStats(zones_, statistics_->awb_stats, config_.minPixels,\n> config_.minG);\n> - /*\n> - * we're done with these; we may as well relinquish our hold on the\n> - * pointer.\n> - */\n> - statistics_.reset();\n> /*\n> * apply sensitivities, so values appear to come from our \"canonical\"\n> * sensor.\n> @@ -733,6 +728,11 @@ void Awb::doAwb()\n> << \" with gains r \" << asyncResults_.gainR\n> << \" and b \" << asyncResults_.gainB;\n> }\n> + /*\n> + * we're done with these; we may as well relinquish our hold on the\n> + * pointer.\n> + */\n> + statistics_.reset();\n> }\n>\n> /* Register algorithm with the system. */\n> --\n> 2.25.1\n>","headers":{"Return-Path":"","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 228EBBDE6B\n\tfor ;\n\tWed, 23 Nov 2022 13:52:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8E2646331A;\n\tWed, 23 Nov 2022 14:52:07 +0100 (CET)","from mail-pl1-x636.google.com (mail-pl1-x636.google.com\n\t[IPv6:2607:f8b0:4864:20::636])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5DDD963311\n\tfor ;\n\tWed, 23 Nov 2022 14:52:06 +0100 (CET)","by mail-pl1-x636.google.com with SMTP id w23so16661297ply.12\n\tfor ;\n\tWed, 23 Nov 2022 05:52:06 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669211527;\n\tbh=6bzHV2hEjF6izoaXQWZE3ygaG1jadZHkhf1nJaK6oII=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=UWcmi0+18nhh2/JzYS9Piahw6Q8jeYjRaQVKOGPBvLoxDac0g1INW8DQNsDJwyaqS\n\ttdNGwroAGTxaLfsk0too9f7s42NNvonktpk6nrdky0FskpuZjtpedCWyVcbuplz/fl\n\tV20JAWwCRvNWk2pnlHrqEcogSt90vi+DqpiDPCzd6kyINiMDWuEhtDFqp52Rhx2KCh\n\t1RC0x5L7vdpBKqjfRqvQq7aXeJZTM35bdaDKlLRcH0HjiZWFDZSYBMeoQ7/NHMaCK4\n\teJl9BDovebasTXTvG3V9gcMPlRLjPmH0tP85KLVo+0kJ2Q7jc4yBYC8EMeyh2I/Kap\n\tK17OXsbSMsdBw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=cmMSDxc2Km1XHZ5LgRGRuDJaSSWoe0iPAuUkbsBtKW8=;\n\tb=nzlse9scUY2sQWiRSz1NcUdhJEfnoQdeBbwWPDUNlSghDNIOXlBaJXgzIRCXvCKa0F\n\tDEHgqsiKztW09jx6xZgpCjpb+4TTnJtL+2c+aK5mILbUAusPnAcU3uQ9inl8XZEWQPlm\n\tc7sDGVnQTqF8iwIP/ZYca4OAVxSBwvvjPmBu/wapwtB5Q9k4pv/SrOMrgTeDcW/vX3Fe\n\tpQN9JcS1Me/4ODBjpGbawmhRhYYA+vW0WqHzdp+cfysuf4K9hlfHXC996Kohed4E28Tz\n\tTmIt33lTausFiXKxcRF+ekWzUkrREW2fZ157fPX6LSDU2yQdyc9s4UA8d+T9erd22cSh\n\txf7Q=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"nzlse9sc\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=cmMSDxc2Km1XHZ5LgRGRuDJaSSWoe0iPAuUkbsBtKW8=;\n\tb=tNv9YAH5EIL2MH6gZbMPvHDvJ054cnNEjLx5wlSEHPg4YlqooP7xQLT9H9qXpUGFXA\n\tT64t3sNLnAlkJe+/3eZXf4WnnPXKPiLMZFT4pQIJpXzduEUvd+AoAc3dCC18nir8kXmE\n\tgShI57TiIlVOYRN3gTZZpCtfrWJiN2OpAFKYZnMlwMpqPVqqBEjPijY1BcsjLlgB1IBC\n\ttIVqB08VZOYZb5+R3pR8cBtpRrqfh9Jk1Q/FMowlF6mM1EBRsP9bR79H3pyKm0Ojj6xf\n\trXwlwzJ5208+LdoDK5bXjNdTOtpviUwSuE6g7rdWtDAoaJsJ36EQdnDdzdqt9thGe4e5\n\t5LVA==","X-Gm-Message-State":"ANoB5pmqSFQeztNJ1A/m502boQT4u0Mb1jUBPuTqti7LyGtfcABfvg6j\n\tU4k0dLZqv4q8EHGJUyPpWkV3fRDCXc4OmPZ4vAUlT7lrzEg=","X-Google-Smtp-Source":"AA0mqf5FHD5H1ESoL021gZR4kXtLTX68zlsrSyYtcAaHRxsaN/tpuVwxrYHdzg49fpd1COQv+L7SGAJGfImQcrYQPCk=","X-Received":"by 2002:a17:902:7d93:b0:186:9cf4:e53b with SMTP id\n\ta19-20020a1709027d9300b001869cf4e53bmr13086340plm.50.1669211524837;\n\tWed, 23 Nov 2022 05:52:04 -0800 (PST)","MIME-Version":"1.0","References":"<20221122112224.31691-1-naush@raspberrypi.com>\n\t<20221122112224.31691-4-naush@raspberrypi.com>","In-Reply-To":"<20221122112224.31691-4-naush@raspberrypi.com>","Date":"Wed, 23 Nov 2022 13:51:53 +0000","Message-ID":"","To":"Naushir Patuck ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v1 3/5] ipa: raspberrypi: awb: Delay\n\trelease of the statistics buffer","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"David Plowman via libcamera-devel ","Reply-To":"David Plowman ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25879,"web_url":"https://patchwork.libcamera.org/comment/25879/","msgid":"","date":"2022-11-23T14:05:36","subject":"Re: [libcamera-devel] [PATCH v1 3/5] ipa: raspberrypi: awb: Delay\n\trelease of the statistics buffer","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nThank you for the review!\n\nOn Wed, 23 Nov 2022 at 13:52, David Plowman \nwrote:\n\n> Hi Naush\n>\n> Thanks for the patch.\n>\n> On Tue, 22 Nov 2022 at 11:22, Naushir Patuck via libcamera-devel\n> wrote:\n> >\n> > Release the statistics buffer after running the through the AWB\n> calculations.\n> > Only the \"counted\" statistics are copied out to a local structure, so\n> keeping\n> > the statistics buffer allows the algorithm to see the \"uncounted\"\n> statistics as\n> > well.\n>\n> I guess I still feel a bit weird about this. The code used to copy out\n> everything it needed so that it could release the buffer as soon as\n> possible. Maybe that's not a meaningful concern any more...?\n>\n\nIn the original code, we used to make a copy of the stats dmabuf, and this\ncopy (through a shared_ptr) was what the controller algorithms would get.\nWe now copy values out of the stats dmabuf into the new stats structure,\nand pass this in as a shared_ptr.\n\nAs such, moving the reset() to release the buffer to later does not have any\nimpact on recycling the stats dmabuf back to the ISP. The alternative to\nthis\nchange would be to cache the stats grid sizes (that's what we really need)\ninstead of keeping the shared_ptr referenced for a bit longer?\n\nNaush\n\n\n>\n> >\n> > This is currently handled by hard-coding the total number of statistics\n> regions\n> > regions based on the structure definition in the bcm2835_isp_stats\n> structure.\n> >\n> > Signed-off-by: Naushir Patuck \n>\n> In spite of my probably unnecessary weird feelings, it's probably of\n> no consequence, so:\n>\n> Reviewed-by: David Plowman \n>\n> Thanks!\n> David\n>\n> > ---\n> > src/ipa/raspberrypi/controller/rpi/awb.cpp | 10 +++++-----\n> > 1 file changed, 5 insertions(+), 5 deletions(-)\n> >\n> > diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> b/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> > index 8d8ddf0913f7..861022014896 100644\n> > --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> > +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp\n> > @@ -432,11 +432,6 @@ void Awb::prepareStats()\n> > */\n> > generateStats(zones_, statistics_->awb_stats, config_.minPixels,\n> > config_.minG);\n> > - /*\n> > - * we're done with these; we may as well relinquish our hold on\n> the\n> > - * pointer.\n> > - */\n> > - statistics_.reset();\n> > /*\n> > * apply sensitivities, so values appear to come from our\n> \"canonical\"\n> > * sensor.\n> > @@ -733,6 +728,11 @@ void Awb::doAwb()\n> > << \" with gains r \" << asyncResults_.gainR\n> > << \" and b \" << asyncResults_.gainB;\n> > }\n> > + /*\n> > + * we're done with these; we may as well relinquish our hold on\n> the\n> > + * pointer.\n> > + */\n> > + statistics_.reset();\n> > }\n> >\n> > /* Register algorithm with the system. */\n> > --\n> > 2.25.1\n> >\n>","headers":{"Return-Path":"","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 9BB9DBE08B\n\tfor ;\n\tWed, 23 Nov 2022 14:05:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1F1E263313;\n\tWed, 23 Nov 2022 15:05:54 +0100 (CET)","from mail-io1-xd33.google.com (mail-io1-xd33.google.com\n\t[IPv6:2607:f8b0:4864:20::d33])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3E67D63311\n\tfor ;\n\tWed, 23 Nov 2022 15:05:52 +0100 (CET)","by mail-io1-xd33.google.com with SMTP id e189so13210680iof.1\n\tfor ;\n\tWed, 23 Nov 2022 06:05:52 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669212354;\n\tbh=QW54SamZtxLQ8/vrEUZCTBAATi2wWogTc5miiAjdRrY=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=BWUdR2G/lLhsMNtSDFwJTbslNJ23kL3g8MkM7tEB/Qq/JO9xSUQ1cv8zyBqrS482O\n\tRfSvY5beIzSpIi9mECAaeSZ90My7HCQ+hSb0NLs8LFe/ehgL2tVpQCkWacKhggYCsb\n\t+tq1oLRlYXZPCL5KV+9ih/TnAemaoWN6IDW7MR1d9ZnnZSUdCFbT4P1T00xEIlPBGs\n\tCMkRqbuP5yz17nJDHEtlOirsnhTpNJZpuSVog80eBIYnl9yfP99aQCD98dYw+0Yoqf\n\tP5w52Q0W1u9UMqEzF9HddNwLCMz80U/SkdDATOefLC2GfefcXFpssA40LFm+26k1fK\n\tOM8bSxvD9rIUg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=bKt90cdWB+dC5iDY1MWf2QjkikUoy4+PQ7W+NY7lMbw=;\n\tb=RrONAmKHXHUa673qoVIHOBgjKCO8Lymu6AOSEtpbvUEVnOvHcoXfh76wbeQFgHSnNw\n\tnYPurE1jTudcCcxktzZxSe9u/24oC4ouUnzx75mHP0Z3LkAUWBsXMy7XtF0JgF2R8S4a\n\tWc4UDMJEFO+pizUyGqgdd8bBaGcY3dKXFHFhHluL647WwjrteZwMaDsJ908iT1iiklg5\n\twMLR70keuzMWzdVsJKfCyOGYzijVZbcQKJp/nHkAHVX6BAET4doOnzm75MzaZJywBLXa\n\tFDJRwnH+osubyMTAYp64pfz0VdpJQMxQsJQxY8Ue4di/OY9EC9HcHp4ITRnQXJnVziqe\n\tosBQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"RrONAmKH\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=bKt90cdWB+dC5iDY1MWf2QjkikUoy4+PQ7W+NY7lMbw=;\n\tb=c3tL81QMw0vpzcPOkVi2w7I9kRXOFtLA6TR3bXfXjfNQMMHAP5MfbgZrV+VOVmHFOZ\n\t0Vpu1bNWAjSVPjbx5dqyAvs7f1dE9NpmdFj1HlseuZYYfHD3R7mr/Gk+s3yruI8WIoGI\n\tYCdTVkVuF8u4OS6Rrlw3V89JMSh8ckfRUGBYkrQ3sZfHwQ3inAZUfDHwH1xQn8DzMAWf\n\t2F0eD9m0OQA6HJ/vqxHjLBR4kWdJukdB15tjYlzBGXdt862lPB/VOqDXteTgfTKac9lF\n\tyQxR3n1whrZPtnwJqPx813zswKc5dCB4DaR82TrCJr1uDgE2vjk804/clBUM9y6SqlAf\n\tcA3w==","X-Gm-Message-State":"ANoB5plDmiXYvObffUbElSvXdgxbrnTH2LZJX2/K1jaqYdFoxNzp4Egx\n\tFajaIRhU5M6so9DcDT/IUSb0v90+7BAzU5pQK8Jy13cHBmc=","X-Google-Smtp-Source":"AA0mqf4GPoQiJvoQ6RskmzVeQRuQDIvDD9Kihk9zAkGMXp+4mSyFGSsm31UBBxT3wrSHobKEXA0fwAlQSgxLEiTOAWo=","X-Received":"by 2002:a5d:88c3:0:b0:6d6:5fe4:8212 with SMTP id\n\ti3-20020a5d88c3000000b006d65fe48212mr13216714iol.180.1669212350979;\n\tWed, 23 Nov 2022 06:05:50 -0800 (PST)","MIME-Version":"1.0","References":"<20221122112224.31691-1-naush@raspberrypi.com>\n\t<20221122112224.31691-4-naush@raspberrypi.com>\n\t","In-Reply-To":"","Date":"Wed, 23 Nov 2022 14:05:36 +0000","Message-ID":"","To":"David Plowman ","Content-Type":"multipart/alternative; boundary=\"0000000000003c3bcf05ee23c9a9\"","Subject":"Re: [libcamera-devel] [PATCH v1 3/5] ipa: raspberrypi: awb: Delay\n\trelease of the statistics buffer","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]