[{"id":34424,"web_url":"https://patchwork.libcamera.org/comment/34424/","msgid":"<2poglfe4mhmgyjbwnwdvqinai247qcamyytk3ojxbca25upqnu@c65vaom3xlzt>","date":"2025-06-05T07:30:19","subject":"Re: [PATCH v2 6/6] ipa: rpi: Rename dropFrameCount_ to startupCount_","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Thu, May 22, 2025 at 08:48:22AM +0100, Naushir Patuck wrote:\n> Rename dropFrameCount_ to startupCount_ to better reflect its use as\n> frames are no longer dropped by the pipeline handler.\n\nMakes sense\n\n>\n> Signed-off-by: Naushir Patuck \n> Reviewed-by: David Plowman \n> ---\n> src/ipa/rpi/common/ipa_base.cpp | 10 +++++-----\n> src/ipa/rpi/common/ipa_base.h | 4 ++--\n> 2 files changed, 7 insertions(+), 7 deletions(-)\n>\n> diff --git a/src/ipa/rpi/common/ipa_base.cpp b/src/ipa/rpi/common/ipa_base.cpp\n> index c15f8a7bf71e..8d591faeceaa 100644\n> --- a/src/ipa/rpi/common/ipa_base.cpp\n> +++ b/src/ipa/rpi/common/ipa_base.cpp\n> @@ -327,7 +327,7 @@ void IpaBase::start(const ControlList &controls, StartResult *result)\n> \tunsigned int agcConvergenceFrames = 0, awbConvergenceFrames = 0;\n> \tframeCount_ = 0;\n> \tif (firstStart_) {\n> -\t\tdropFrameCount_ = helper_->hideFramesStartup();\n> +\t\tstartupCount_ = helper_->hideFramesStartup();\n> \t\tmistrustCount_ = helper_->mistrustFramesStartup();\n>\n> \t\t/*\n> @@ -353,14 +353,14 @@ void IpaBase::start(const ControlList &controls, StartResult *result)\n> \t\t\t\tawbConvergenceFrames += mistrustCount_;\n> \t\t}\n> \t} else {\n> -\t\tdropFrameCount_ = helper_->hideFramesModeSwitch();\n> +\t\tstartupCount_ = helper_->hideFramesModeSwitch();\n> \t\tmistrustCount_ = helper_->mistrustFramesModeSwitch();\n> \t}\n>\n> \tresult->startupFrameCount = std::max({ agcConvergenceFrames, awbConvergenceFrames });\n> -\tresult->invalidFrameCount = dropFrameCount_;\n> +\tresult->invalidFrameCount = startupCount_;\n\nBut here it makese\n\n result->startupFrameCount = .. convergence frames;\n result->invalidFrameCount = startupCount_;\n\nIt's your IPA, so if it's fine with you, fine with me!\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n>\n> -\tdropFrameCount_ = std::max({ dropFrameCount_, agcConvergenceFrames, awbConvergenceFrames });\n> +\tstartupCount_ = std::max({ startupCount_, agcConvergenceFrames, awbConvergenceFrames });\n>\n> \tLOG(IPARPI, Debug) << \"Startup frames: \" << result->startupFrameCount\n> \t\t\t << \" Invalid frames: \" << result->invalidFrameCount;\n> @@ -443,7 +443,7 @@ void IpaBase::prepareIsp(const PrepareParams ¶ms)\n>\n> \t/* Allow a 10% margin on the comparison below. */\n> \tDuration delta = (frameTimestamp - lastRunTimestamp_) * 1.0ns;\n> -\tif (lastRunTimestamp_ && frameCount_ > dropFrameCount_ &&\n> +\tif (lastRunTimestamp_ && frameCount_ > startupCount_ &&\n> \t delta < controllerMinFrameDuration * 0.9 && !hdrChange) {\n> \t\t/*\n> \t\t * Ensure we merge the previous frame's metadata with the current\n> diff --git a/src/ipa/rpi/common/ipa_base.h b/src/ipa/rpi/common/ipa_base.h\n> index 1a811beb31f2..a51afc156a8f 100644\n> --- a/src/ipa/rpi/common/ipa_base.h\n> +++ b/src/ipa/rpi/common/ipa_base.h\n> @@ -115,8 +115,8 @@ private:\n> \t/* How many frames we should avoid running control algos on. */\n> \tunsigned int mistrustCount_;\n>\n> -\t/* Number of frames that need to be dropped on startup. */\n> -\tunsigned int dropFrameCount_;\n> +\t/* Number of frames that need to be marked as dropped on startup. */\n> +\tunsigned int startupCount_;\n>\n> \t/* Frame timestamp for the last run of the controller. */\n> \tuint64_t lastRunTimestamp_;\n> --\n> 2.43.0\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 2863BC3237\n\tfor ;\n\tThu, 5 Jun 2025 07:30:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2797A68DB3;\n\tThu, 5 Jun 2025 09:30:24 +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 9B6AC68DB2\n\tfor ;\n\tThu, 5 Jun 2025 09:30:22 +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 0A5C06AF;\n\tThu, 5 Jun 2025 09:30:18 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"mtv6y8Nw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1749108619;\n\tbh=te+fxUtnRvcWe5B50g7a1iA+Nwkhw9PQfuhpfxfZ06E=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mtv6y8NwEQlbvkYiMeE7r2/sbu8tiMBectxepAIfJXLYSLzGKYr9jEkG3B3hbYdpm\n\tf44eYbRKtBSAtCRgckvzzUGtWMMLoa0QxDWTZcZWBVx5psz7J0lKat/GqJjzkz+2+t\n\ttJxZkN/rDjTzDQKIm7pD3KoUpIjmssuVr0gbrHtE=","Date":"Thu, 5 Jun 2025 09:30:19 +0200","From":"Jacopo Mondi ","To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org, \n\tDavid Plowman ","Subject":"Re: [PATCH v2 6/6] ipa: rpi: Rename dropFrameCount_ to startupCount_","Message-ID":"<2poglfe4mhmgyjbwnwdvqinai247qcamyytk3ojxbca25upqnu@c65vaom3xlzt>","References":"<20250522075244.1198110-1-naush@raspberrypi.com>\n\t<20250522075244.1198110-7-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250522075244.1198110-7-naush@raspberrypi.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":34430,"web_url":"https://patchwork.libcamera.org/comment/34430/","msgid":"","date":"2025-06-05T11:34:32","subject":"Re: [PATCH v2 6/6] ipa: rpi: Rename dropFrameCount_ to startupCount_","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\n\nOn Thu, 5 Jun 2025 at 08:30, Jacopo Mondi \nwrote:\n\n> Hi Naush\n>\n> On Thu, May 22, 2025 at 08:48:22AM +0100, Naushir Patuck wrote:\n> > Rename dropFrameCount_ to startupCount_ to better reflect its use as\n> > frames are no longer dropped by the pipeline handler.\n>\n> Makes sense\n>\n> >\n> > Signed-off-by: Naushir Patuck \n> > Reviewed-by: David Plowman \n> > ---\n> > src/ipa/rpi/common/ipa_base.cpp | 10 +++++-----\n> > src/ipa/rpi/common/ipa_base.h | 4 ++--\n> > 2 files changed, 7 insertions(+), 7 deletions(-)\n> >\n> > diff --git a/src/ipa/rpi/common/ipa_base.cpp\n> b/src/ipa/rpi/common/ipa_base.cpp\n> > index c15f8a7bf71e..8d591faeceaa 100644\n> > --- a/src/ipa/rpi/common/ipa_base.cpp\n> > +++ b/src/ipa/rpi/common/ipa_base.cpp\n> > @@ -327,7 +327,7 @@ void IpaBase::start(const ControlList &controls,\n> StartResult *result)\n> > unsigned int agcConvergenceFrames = 0, awbConvergenceFrames = 0;\n> > frameCount_ = 0;\n> > if (firstStart_) {\n> > - dropFrameCount_ = helper_->hideFramesStartup();\n> > + startupCount_ = helper_->hideFramesStartup();\n> > mistrustCount_ = helper_->mistrustFramesStartup();\n> >\n> > /*\n> > @@ -353,14 +353,14 @@ void IpaBase::start(const ControlList &controls,\n> StartResult *result)\n> > awbConvergenceFrames += mistrustCount_;\n> > }\n> > } else {\n> > - dropFrameCount_ = helper_->hideFramesModeSwitch();\n> > + startupCount_ = helper_->hideFramesModeSwitch();\n> > mistrustCount_ = helper_->mistrustFramesModeSwitch();\n> > }\n> >\n> > result->startupFrameCount = std::max({ agcConvergenceFrames,\n> awbConvergenceFrames });\n> > - result->invalidFrameCount = dropFrameCount_;\n> > + result->invalidFrameCount = startupCount_;\n>\n> But here it makese\n>\n> result->startupFrameCount = .. convergence frames;\n> result->invalidFrameCount = startupCount_;\n>\n> It's your IPA, so if it's fine with you, fine with me!\n>\n> Reviewed-by: Jacopo Mondi \n>\n\nIt took me a while to realise what was wrong here, and indeed the naming is\ncompletely opposite between the IPA and PH. I will rename to make these\nvariables consistent.\n\nRegards,\nNaush\n\n\n>\n> Thanks\n> j\n>\n> >\n> > - dropFrameCount_ = std::max({ dropFrameCount_,\n> agcConvergenceFrames, awbConvergenceFrames });\n> > + startupCount_ = std::max({ startupCount_, agcConvergenceFrames,\n> awbConvergenceFrames });\n> >\n> > LOG(IPARPI, Debug) << \"Startup frames: \" <<\n> result->startupFrameCount\n> > << \" Invalid frames: \" <<\n> result->invalidFrameCount;\n> > @@ -443,7 +443,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_ > startupCount_ &&\n> > delta < controllerMinFrameDuration * 0.9 && !hdrChange) {\n> > /*\n> > * Ensure we merge the previous frame's metadata with the\n> current\n> > diff --git a/src/ipa/rpi/common/ipa_base.h\n> b/src/ipa/rpi/common/ipa_base.h\n> > index 1a811beb31f2..a51afc156a8f 100644\n> > --- a/src/ipa/rpi/common/ipa_base.h\n> > +++ b/src/ipa/rpi/common/ipa_base.h\n> > @@ -115,8 +115,8 @@ private:\n> > /* How many frames we should avoid running control algos on. */\n> > unsigned int mistrustCount_;\n> >\n> > - /* Number of frames that need to be dropped on startup. */\n> > - unsigned int dropFrameCount_;\n> > + /* Number of frames that need to be marked as dropped on startup.\n> */\n> > + unsigned int startupCount_;\n> >\n> > /* Frame timestamp for the last run of the controller. */\n> > uint64_t lastRunTimestamp_;\n> > --\n> > 2.43.0\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 6003BC31E9\n\tfor ;\n\tThu, 5 Jun 2025 11:35:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0F26F68DB3;\n\tThu, 5 Jun 2025 13:35:11 +0200 (CEST)","from mail-ua1-x92b.google.com (mail-ua1-x92b.google.com\n\t[IPv6:2607:f8b0:4864:20::92b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8670068D96\n\tfor ;\n\tThu, 5 Jun 2025 13:35:08 +0200 (CEST)","by mail-ua1-x92b.google.com with SMTP id\n\ta1e0cc1a2514c-87e27d264a1so20289241.2\n\tfor ;\n\tThu, 05 Jun 2025 04:35:08 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"a3PINg5F\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1749123307; x=1749728107;\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=bSxgRoO6thCtnyWX0NTA/nBag/Ndg7KKlXOdUOc81ZA=;\n\tb=a3PINg5FBq3k461WvBx7uhNGuhbMOXFIcnR1npyllsQIrgjOzn2fPCe52XLmBS/ETi\n\tz2y0plsNnfV/gT7DUBg8UgGTrUkRqXa564fjoSmDarHkB3PNo7tK47c2T+MHbaXX299u\n\tEoy6yvLttsCg6FeHdFiguU8Hy11JJjkw3/Ui28qPVkf7j0ODwVtOppQG6/Oy5JlvdlTu\n\tmGLNI5gon4jJwwevCcFLVYvF8q/c34BHDAPJEQrlV20cX6NNc9JYrU9PXQUWYIdAFlgF\n\tPcmEOyB35fCwxIJbTzfm6DiX6Z6oC9R2ypsactEVweEETuH1Axbxb/wxndpIDgM5rnYZ\n\tfEjQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1749123307; x=1749728107;\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=bSxgRoO6thCtnyWX0NTA/nBag/Ndg7KKlXOdUOc81ZA=;\n\tb=oBUlr+jw/YAwfzpq53c+i64oXQIOhf/A00P/ePvsrbJg3FiWV+K7CbTaUAHBNTZDlb\n\tYN1iJfoxg2Sg77RCoGU+LNXfc3YmsRfvK0NRAhmBgpOIQgovZskbO4pSKxB2tVxq8AF+\n\tubzaPW1z3zOZNDF//K4oEUsEQSg0UX2KiyZxcVSswm2TgxNOmAWvSaSvplmbAwfd7BKm\n\tHkpvbbve+mzWjefagakI5/Hpob4qxxmAT0WCnSwKW4fohosR/jXP33RGbi7aLEtYKzBj\n\twacX2lrno2TQ32obBvDp17QAx2iicV+0rWIjODU5ycO2IBYvBOvDPE1FB/+NaVKgYTx5\n\tSz6A==","X-Gm-Message-State":"AOJu0YzAmifZRhTO5TYiljBmlFCcdmwrt3te2RhBB0LYZdgjhFcx4ID7\n\tcKZCTUYgOsSqUi2X61ejd+e4cZ2YWfimzarostB1D73qo/ZkqW+AXZbBnqcUvhihJ11c+senHr7\n\tKLSZOFndQZeeBpwLNVBMC7fzV+eRVNqRrOS2yyrUrTw==","X-Gm-Gg":"ASbGncvjbUoMlp3BGsyJQHlvKeqfF+GnDSKoMV9AC2ss3C7uvX1aBfiSohY2CbGnkAK\n\ttJ9AMSm9YYmjUTPtH4oF7KIMzvgF0mnxTP+3F/K9/FWQn8TxZe2w7WgLYg/nYXFs5mXqiOxbWFS\n\tFTrjhhYCzTpqMhQlnwvCGSuH7SvXVSu1dn/vHfudfsbD33bM+GAHMbAFWfN/7lXnQJRiiMiYPrc\n\tA==","X-Google-Smtp-Source":"AGHT+IE1QGhCzwGUEbttYX20cmgCqEstpolk+hNdXCRH/2Yh1anlAkf2nUlgg8drQJhamSFgNstuX/HyNqW1FFJ5D1c=","X-Received":"by 2002:a05:6102:b0b:b0:4df:4ef8:8624 with SMTP id\n\tada2fe7eead31-4e746e767d3mr1581243137.7.1749123307131;\n\tThu, 05 Jun 2025 04:35:07 -0700 (PDT)","MIME-Version":"1.0","References":"<20250522075244.1198110-1-naush@raspberrypi.com>\n\t<20250522075244.1198110-7-naush@raspberrypi.com>\n\t<2poglfe4mhmgyjbwnwdvqinai247qcamyytk3ojxbca25upqnu@c65vaom3xlzt>","In-Reply-To":"<2poglfe4mhmgyjbwnwdvqinai247qcamyytk3ojxbca25upqnu@c65vaom3xlzt>","From":"Naushir Patuck ","Date":"Thu, 5 Jun 2025 12:34:32 +0100","X-Gm-Features":"AX0GCFu9GRQuDtpo9sJ47bOFKzrGtW48i2DEKQZe-gDuBfO7sjWoyN3Du4Vg6r4","Message-ID":"","Subject":"Re: [PATCH v2 6/6] ipa: rpi: Rename dropFrameCount_ to startupCount_","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org, \n\tDavid Plowman ","Content-Type":"multipart/alternative; boundary=\"00000000000063f39a0636d18142\"","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]