[{"id":37844,"web_url":"https://patchwork.libcamera.org/comment/37844/","msgid":"<176907331862.3882822.3912691518806357486@neptunite.rasen.tech>","date":"2026-01-22T09:15:18","subject":"Re: [PATCH v1 08/35] libipa: fc_queue: Add trailing underscore to\n\tprivate members of FrameContext","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Quoting Stefan Klug (2025-10-24 17:50:32)\n> It is not immediately obvious that FCQueue accesses private members of\n> the FrameContext class with the help of the friend declaration. This\n> gets even more confusing when such a member is shadowed by a declaration\n> in the actual IPA FrameContext class. Improve that by accessing the\n> FrameContext members via references to that type. Additionally add an\n> underscore to the variable names like we do on other members and which\n> allows us to create a frame() accessor without a name clash in an\n> upcoming commit.\n> \n> Signed-off-by: Stefan Klug \n\nLooks good to me.\n\nReviewed-by: Paul Elder \n\n> ---\n> src/ipa/libipa/fc_queue.cpp | 2 +-\n> src/ipa/libipa/fc_queue.h | 51 ++++++++++++++++++++-----------------\n> 2 files changed, 28 insertions(+), 25 deletions(-)\n> \n> diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp\n> index 0365e9197748..39222c2ed204 100644\n> --- a/src/ipa/libipa/fc_queue.cpp\n> +++ b/src/ipa/libipa/fc_queue.cpp\n> @@ -34,7 +34,7 @@ namespace ipa {\n> * update any specific action for this frame, and finally to update the metadata\n> * control lists when the frame is fully completed.\n> *\n> - * \\var FrameContext::frame\n> + * \\var FrameContext::frame_\n> * \\brief The frame number\n> */\n> \n> diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h\n> index 1f4f84c27fbc..1128e42f8ca6 100644\n> --- a/src/ipa/libipa/fc_queue.h\n> +++ b/src/ipa/libipa/fc_queue.h\n> @@ -24,8 +24,8 @@ class FCQueue;\n> struct FrameContext {\n> private:\n> template friend class FCQueue;\n> - uint32_t frame;\n> - bool initialised = false;\n> + uint32_t frame_;\n> + bool initialised_ = false;\n> };\n> \n> template\n> @@ -40,14 +40,15 @@ public:\n> void clear()\n> {\n> for (FC &ctx : contexts_) {\n> - ctx.initialised = false;\n> - ctx.frame = 0;\n> + ctx.initialised_ = false;\n> + ctx.frame_ = 0;\n> }\n> }\n> \n> FC &alloc(const uint32_t frame)\n> {\n> - FC &frameContext = contexts_[frame % contexts_.size()];\n> + FC &fc = contexts_[frame % contexts_.size()];\n> + FrameContext &frameContext = fc;\n> \n> /*\n> * Do not re-initialise if a get() call has already fetched this\n> @@ -60,18 +61,19 @@ public:\n> * time the application has queued a request. Does this deserve\n> * an error condition ?\n> */\n> - if (frame != 0 && frame <= frameContext.frame)\n> + if (frame != 0 && frame <= frameContext.frame_)\n> LOG(FCQueue, Warning)\n> << \"Frame \" << frame << \" already initialised\";\n> else\n> - init(frameContext, frame);\n> + init(fc, frame);\n> \n> - return frameContext;\n> + return fc;\n> }\n> \n> FC &get(uint32_t frame)\n> {\n> - FC &frameContext = contexts_[frame % contexts_.size()];\n> + FC &fc = contexts_[frame % contexts_.size()];\n> + FrameContext &frameContext = fc;\n> \n> /*\n> * If the IPA algorithms try to access a frame context slot which\n> @@ -81,28 +83,28 @@ public:\n> * queueing more requests to the IPA than the frame context\n> * queue size.\n> */\n> - if (frame < frameContext.frame)\n> + if (frame < frameContext.frame_)\n> LOG(FCQueue, Fatal) << \"Frame context for \" << frame\n> << \" has been overwritten by \"\n> - << frameContext.frame;\n> + << frameContext.frame_;\n> \n> - if (frame == 0 && !frameContext.initialised) {\n> + if (frame == 0 && !frameContext.initialised_) {\n> /*\n> * If the IPA calls get() at start() time it will get an\n> * un-intialized FrameContext as the below \"frame ==\n> - * frameContext.frame\" check will return success because\n> - * FrameContexts are zeroed at creation time.\n> + * frameContext.frame_\" check will return success\n> + * because FrameContexts are zeroed at creation time.\n> *\n> * Make sure the FrameContext gets initialised if get()\n> * is called before alloc() by the IPA for frame#0.\n> */\n> - init(frameContext, frame);\n> + init(fc, frame);\n> \n> - return frameContext;\n> + return fc;\n> }\n> \n> - if (frame == frameContext.frame)\n> - return frameContext;\n> + if (frame == frameContext.frame_)\n> + return fc;\n> \n> /*\n> * The frame context has been retrieved before it was\n> @@ -116,17 +118,18 @@ public:\n> LOG(FCQueue, Warning)\n> << \"Obtained an uninitialised FrameContext for \" << frame;\n> \n> - init(frameContext, frame);\n> + init(fc, frame);\n> \n> - return frameContext;\n> + return fc;\n> }\n> \n> private:\n> - void init(FC &frameContext, const uint32_t frame)\n> + void init(FC &fc, const uint32_t frame)\n> {\n> - frameContext = {};\n> - frameContext.frame = frame;\n> - frameContext.initialised = true;\n> + fc = {};\n> + FrameContext &frameContext = fc;\n> + frameContext.frame_ = frame;\n> + frameContext.initialised_ = true;\n> }\n> \n> std::vector contexts_;\n> -- \n> 2.48.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 323EEBDCBF\n\tfor ;\n\tThu, 22 Jan 2026 09:15:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D046A61FCA;\n\tThu, 22 Jan 2026 10:15:27 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 301AE61FBF\n\tfor ;\n\tThu, 22 Jan 2026 10:15:26 +0100 (CET)","from neptunite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:8816:a947:ebed:2ec7])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 35A8D2DD;\n\tThu, 22 Jan 2026 10:14:52 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"F1SEk9r1\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1769073293;\n\tbh=v3sELVDeSOe8VgsfadK+nd+nK9eP0bc3J/Pdrg/tWzU=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=F1SEk9r1xnAVscIWAAlxB6jW07INwifJy6zi+u+ZHJO6qplHynULYds7hxEl6e2gK\n\tYLAFLaGd6Fo01G0GkGhglC6y4xN0kpGDshq5vT2loKiMNHM7OP+/3SdVhzbgrh7oEv\n\tJ+Gdrwa7GRHzapLXKnC6qascJNHHHtOJT8oFm5y8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20251024085130.995967-9-stefan.klug@ideasonboard.com>","References":"<20251024085130.995967-1-stefan.klug@ideasonboard.com>\n\t<20251024085130.995967-9-stefan.klug@ideasonboard.com>","Subject":"Re: [PATCH v1 08/35] libipa: fc_queue: Add trailing underscore to\n\tprivate members of FrameContext","From":"Paul Elder ","Cc":"Stefan Klug ","To":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 22 Jan 2026 18:15:18 +0900","Message-ID":"<176907331862.3882822.3912691518806357486@neptunite.rasen.tech>","User-Agent":"alot/0.0.0","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\" "}}]