[{"id":27901,"web_url":"https://patchwork.libcamera.org/comment/27901/","msgid":"","date":"2023-09-28T09:52:09","subject":"Re: [libcamera-devel] [PATCH] ipa: rpi: Avoid skipping IPAs on the\n\tfirst frame after the drop frames","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 fixing this, quite a subtle bug!\n\nOn Thu, 28 Sept 2023 at 10:41, David Plowman via libcamera-devel\n wrote:\n>\n> We avoid skipping the IPAs while frameCount_ is less then\n\ns/then/than/\n\n> dropFrameCount_, indicating that these frames will not be sent to the\n> application. This means that when these numbers are equal then this is\n> the first frame the application will get, so again, we must avoid\n> skipping the IPAs. Consequently the test here must avoid the case of\n> equality.\n>\n> Signed-off-by: David Plowman \n\nPerhaps add a Fixes: 51533fecae8 (\"ipa: rpi: Fix frame count logic\nwhen running algorithms\")?\n\nReviewed-by: Naushir Patuck \n\n> ---\n> src/ipa/rpi/common/ipa_base.cpp | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> index 551adadf..5df1998c 100644\n> --- a/src/ipa/rpi/common/ipa_base.cpp\n> +++ b/src/ipa/rpi/common/ipa_base.cpp\n> @@ -409,7 +409,7 @@ void IpaBase::prepareIsp(const PrepareParams ¶ms)\n>\n> /* Allow a 10% margin on the comparison below. */\n> Duration delta = (frameTimestamp - lastRunTimestamp_) * 1.0ns;\n> - if (lastRunTimestamp_ && frameCount_ >= dropFrameCount_ &&\n> + if (lastRunTimestamp_ && frameCount_ > dropFrameCount_ &&\n> delta < controllerMinFrameDuration * 0.9) {\n> /*\n> * Ensure we merge the previous frame's metadata with the current\n> --\n> 2.39.2\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 B9519C326B\n\tfor ;\n\tThu, 28 Sep 2023 09:52:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 125D462964;\n\tThu, 28 Sep 2023 11:52:48 +0200 (CEST)","from mail-yw1-x1133.google.com (mail-yw1-x1133.google.com\n\t[IPv6:2607:f8b0:4864:20::1133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7328D61DE3\n\tfor ;\n\tThu, 28 Sep 2023 11:52:46 +0200 (CEST)","by mail-yw1-x1133.google.com with SMTP id\n\t00721157ae682-59f630612a4so121112807b3.1\n\tfor ;\n\tThu, 28 Sep 2023 02:52:46 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1695894768;\n\tbh=KWc2wbcgZf1ybiXYxo4sLy7UPRp5BDGmNryFng5IINE=;\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=KV1cnkznItVLPK7abC9wEyHwavDVPguu5AD2HXQip06sctpxkirrNJx+Hmm1kYL41\n\tORORQpIaCrnJkK/BLTmMGb1tN+KgYEmiXcjJHaeDtFbKOr93xCAXMdkb1YLLUmiBDx\n\tl/ZxENDq6U22l0fWN/y1blj5lKk7CO3s6mJ5+z4lPt7XI+AIlZ6RybdyK0UwBAF5XV\n\tbqbH+COJN10JzlMeDAx7nwYBoga8vZPXbM8EWNg/O2e3bfONSmztCbR8IXBYTjLHxs\n\tXPjuvQb3yDl4ybvV5hJlp3Ody5M0HGsaSAmNPkxwvRXwen2IuXwOohDBIWv4M+U20I\n\tz6vTYswwuiChQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1695894765; x=1696499565;\n\tdarn=lists.libcamera.org; \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=nU1JyKOgQUbKG53BD9psWuO/enpACWWqrJDy9uM8uYY=;\n\tb=cvtAW5yIC8t+0lWSQ35110BV3SD9Pl6D7wLpmYY4FPwSkJNt9xa+uRTDSOz9okvIE2\n\tfDbKCjSG44ifm73PmPcIBrSaF3obvEozIDvMoFed/4Cq8/lfv8uPBnqBj7uhv7QyoGD7\n\t5vAl4B2RXBgoITbLvPrXXHiN4f5skh9N8N2LDUzWyFKGA7C5pCbTRZLhTmOuutjtBuDT\n\tD2dxYwjUsiYM+6FlPexci2d6BSo2taCBxUZHEQHPseEDSptNpQx+bvr2/xMr/W/6/bmQ\n\tQjVkm/cdwy/CA1GW42u084NF/awTjVlZ6mReMA/wLTNVqeF3NfzhnmqFrEwTVUx7UZfz\n\tkQ4g=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"cvtAW5yI\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1695894765; x=1696499565;\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=nU1JyKOgQUbKG53BD9psWuO/enpACWWqrJDy9uM8uYY=;\n\tb=IwXwW4U5/JzoXDzZvGCNA0lqOXqLZqpZ5OOfSrb4UH1xLxq/Xff/wZsekzAQJAeHto\n\tBfobBGugjUSY0XwrSuyOJxbJ8aLflJNt421X+FSI1pGafw8RB8y4rJKtbD6zukjfBYL2\n\tY+8IWDPYJn5DgQYFe0PgoM0WMoY5VJXXmb3hMpY8WEKys+xPAPnnMQEqK2iay0L+hqmT\n\tzmENi8oGgh/xM5DmVc1petoq9RMh0r8ao0XhUJnqcPO2jJa5AgYobzcJzUfIN90lcNt2\n\tVGPqsKLoij6xa17Y9DSdywzcy0VH9IBcYXwX2CMrtySKPR+lGB6BZ3NPHPcDL4EgpuoZ\n\tZ/lQ==","X-Gm-Message-State":"AOJu0YyKQj/Xy45mxiJqCXG71yO3KR5RMW0fgY4nprCbjyQTWeDDdim5\n\tSsfpMM5BoF8EeBKMArSQ4t71twUthS56hBeiUE+/6pXiF3u3mip6UgxvDA==","X-Google-Smtp-Source":"AGHT+IGyHqAq8oxlf3me8UfhRNO/xmgG/NXmyqrqEi/dbIAsGXyqAqnIzs53xeBjAD6DcmEI/sxge5h5ZAhBQON6Q7c=","X-Received":"by 2002:a0d:d445:0:b0:59c:789:c848 with SMTP id\n\tw66-20020a0dd445000000b0059c0789c848mr574867ywd.7.1695894765127;\n\tThu, 28 Sep 2023 02:52:45 -0700 (PDT)","MIME-Version":"1.0","References":"<20230928094124.21801-1-david.plowman@raspberrypi.com>","In-Reply-To":"<20230928094124.21801-1-david.plowman@raspberrypi.com>","Date":"Thu, 28 Sep 2023 10:52:09 +0100","Message-ID":"","To":"David Plowman ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH] ipa: rpi: Avoid skipping IPAs on the\n\tfirst frame after the drop frames","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\" "}},{"id":27926,"web_url":"https://patchwork.libcamera.org/comment/27926/","msgid":"<169652162169.2915094.8481722298321842016@ping.linuxembedded.co.uk>","date":"2023-10-05T16:00:21","subject":"Re: [libcamera-devel] [PATCH] ipa: rpi: Avoid skipping IPAs on the\n\tfirst frame after the drop frames","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck via libcamera-devel (2023-09-28 10:52:09)\n> Hi David,\n> \n> Thank you for fixing this, quite a subtle bug!\n> \n> On Thu, 28 Sept 2023 at 10:41, David Plowman via libcamera-devel\n> wrote:\n> >\n> > We avoid skipping the IPAs while frameCount_ is less then\n> \n> s/then/than/\n> \n> > dropFrameCount_, indicating that these frames will not be sent to the\n> > application. This means that when these numbers are equal then this is\n> > the first frame the application will get, so again, we must avoid\n> > skipping the IPAs. Consequently the test here must avoid the case of\n> > equality.\n> >\n> > Signed-off-by: David Plowman \n> \n> Perhaps add a Fixes: 51533fecae8 (\"ipa: rpi: Fix frame count logic\n> when running algorithms\")?\n> \n> Reviewed-by: Naushir Patuck \n\nThis looks reasonable to me.\n\nI'll apply the fixups above while applying.\n\n\nReviewed-by: Kieran Bingham \n\n> \n> > ---\n> > src/ipa/rpi/common/ipa_base.cpp | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> >\n> > diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> > index 551adadf..5df1998c 100644\n> > --- a/src/ipa/rpi/common/ipa_base.cpp\n> > +++ b/src/ipa/rpi/common/ipa_base.cpp\n> > @@ -409,7 +409,7 @@ void IpaBase::prepareIsp(const PrepareParams ¶ms)\n> >\n> > /* Allow a 10% margin on the comparison below. */\n> > Duration delta = (frameTimestamp - lastRunTimestamp_) * 1.0ns;\n> > - if (lastRunTimestamp_ && frameCount_ >= dropFrameCount_ &&\n> > + if (lastRunTimestamp_ && frameCount_ > dropFrameCount_ &&\n> > delta < controllerMinFrameDuration * 0.9) {\n> > /*\n> > * Ensure we merge the previous frame's metadata with the current\n> > --\n> > 2.39.2\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 A1A27BD808\n\tfor ;\n\tThu, 5 Oct 2023 16:00:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B5CAE62963;\n\tThu, 5 Oct 2023 18:00:26 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E2F3061DE0\n\tfor ;\n\tThu, 5 Oct 2023 18:00:24 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 91F498C2;\n\tThu, 5 Oct 2023 17:58:37 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1696521626;\n\tbh=GL2LS+sAc14p3xFPT3Blh7nzgny6yDMhlY8zmnva1PI=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=EUKvX3CFDxFm4xDQAvqbjWbfgm1u5evaWB6B0wODAZyY6w7slolN+lZXRrwCW9CZu\n\t+GdwJIv7GiUiGAE13QP+KGV4LYB563+pOMC8CY1PZkk6zTymvJAAQSaz+a2/EWnrFi\n\tWUMPgbKZKYFsWFO66Rp30Pu3wsNU1dpOernu434CZI2l7XWYgHoAVmtpWLw7H445E/\n\tKv9WB4+cN/3ty+/vhvWtsU01InkURPC2PDB7e8Gox5y//tt5Zi838w5A6gI9sb9CLU\n\tiQKyhpRYqlr6NxLmhfdiBe8k6wP85M78MtwsEY53Qa31K75JXvH02/La+fzf3YDj7L\n\tdPMUDK4KJ+5VA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1696521517;\n\tbh=GL2LS+sAc14p3xFPT3Blh7nzgny6yDMhlY8zmnva1PI=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=aq9QnbgKQoctDMt5KgwAtY5d8sc+NyY9Z44S7lQOLZNlN/SM0oNQtITuuLwb/LR7H\n\tb8YFkUKWSq3orHivpctK1dkrIVwss1ScYJi7e82nIrqW5U+HYQJBCtDf7HQrZdMoFg\n\tViHj8z8eBEdCYLRrqi9kmBZntq4/ZEX+TmQPJz9g="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"aq9QnbgK\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20230928094124.21801-1-david.plowman@raspberrypi.com>\n\t","To":"David Plowman ,\n\tNaushir Patuck ,\n\tNaushir Patuck via libcamera-devel ","Date":"Thu, 05 Oct 2023 17:00:21 +0100","Message-ID":"<169652162169.2915094.8481722298321842016@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] ipa: rpi: Avoid skipping IPAs on the\n\tfirst frame after the drop frames","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":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]