[{"id":25011,"web_url":"https://patchwork.libcamera.org/comment/25011/","msgid":"<166368122820.3912877.9286121009516206496@Monstersaurus>","date":"2022-09-20T13:40:28","subject":"Re: [libcamera-devel] [PATCH v4 05/32] ipa: libipa: Provide a\n\tcommon base for frame contexts","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2022-09-08 02:41:33)\n> From: Kieran Bingham \n> \n> Provide a common FrameContext as a base for IPA modules to inherit from.\n> \n> This will allow having a common set of parameters for every frame\n> context managed by the FCQueue implementation.\n> \n> Signed-off-by: Kieran Bingham \n\nMissing SoB.\n\n> ---\n> Changes since v3:\n> \n> - Rename structure from IPAFrameContext to FrameContext\n> - Move changes to IPA modules to separate patches\n> - Make the FrameContext::frame member private\n> ---\n> src/ipa/libipa/fc_queue.cpp | 18 ++++++++++++++++++\n> src/ipa/libipa/fc_queue.h | 10 ++++++++++\n> 2 files changed, 28 insertions(+)\n> \n> diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp\n> index b81d497e255a..8f61c85131c9 100644\n> --- a/src/ipa/libipa/fc_queue.cpp\n> +++ b/src/ipa/libipa/fc_queue.cpp\n> @@ -20,6 +20,24 @@ namespace ipa {\n> * \\brief Queue to access per-frame Context\n> */\n> \n> +/**\n> + * \\struct FrameContext\n> + * \\brief Context for a frame\n> + *\n> + * The frame context stores data specific to a single frame processed by the\n> + * IPA. Each frame processed by the IPA has a context associated with it,\n> + * accessible through the Frame Context Queue.\n> + *\n> + * Fields in the frame context should reflect values and controls associated\n> + * with the specific frame as requested by the application, and as configured by\n> + * the hardware. Fields can be read by algorithms to determine if they should\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> + * \\brief The frame number\n> + */\n> +\n> /**\n> * \\class FCQueue\n> * \\brief A support class for queueing FrameContext instances through the IPA\n> diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h\n> index 0f3af0f3a189..d8a5b8297a70 100644\n> --- a/src/ipa/libipa/fc_queue.h\n> +++ b/src/ipa/libipa/fc_queue.h\n> @@ -8,6 +8,7 @@\n> #pragma once\n> \n> #include \n> +#include \n> #include \n> \n> #include \n> @@ -18,6 +19,15 @@ LOG_DECLARE_CATEGORY(FCQueue)\n> \n> namespace ipa {\n> \n> +template\n> +class FCQueue;\n> +\n> +struct FrameContext {\n> +private:\n> + template friend class FCQueue;\n> + uint32_t frame;\n\nSeems fine to me.\n\nI'm still not sure what else is going to end up in here. There's a\nchance that we might need to store the incoming Request ControlList (or\na copy of) ... so I still think there's sufficient requirement for the\ncommon base type.\n\n\nReviewed-by: Kieran Bingham \n\n> +};\n> +\n> template\n> class FCQueue\n> {\n> -- \n> Regards,\n> \n> Laurent Pinchart\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 A9E5BC3272\n\tfor ;\n\tTue, 20 Sep 2022 13:40:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1F7EC621AE;\n\tTue, 20 Sep 2022 15:40:32 +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 03DA06218B\n\tfor ;\n\tTue, 20 Sep 2022 15:40:31 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 829466BE;\n\tTue, 20 Sep 2022 15:40:30 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1663681232;\n\tbh=nEmXfbxUGBL14jx4olROvAHFeIiZsd6dDO2wx96ccB8=;\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:\n\tFrom;\n\tb=QriHRqCMuJFJ+TW01s7/WCPPB1rWCW6VshPv8+TTOYpFp9DPlfxcrTPNge9ebbB0j\n\ttUQTk2QisVDUJv1jzHOmfCo05ekmuGGCzYIfnGxDDztWrBYqHdUxTBhUOjwYiIu4JR\n\tx0oaFqL2tZW8ZLqxudI0/acNMNx0QKHIKH1TvP/mMeRA0sB6q2LDW5atVLZXdolDC/\n\tBP8DbC8NgVIiQ/OSRd8D0ZtNqStDGA1drQ/PwoM8FA6HXRraQc4pa5qJNePgxPJadt\n\tEp3pzjnHvcJPqwv0F0VN+syACK3U8wKLYBwWv+fvhcBccB1XBEyaWLAV2jBt/wBj1Q\n\tofoOvBgl2UuHg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1663681230;\n\tbh=nEmXfbxUGBL14jx4olROvAHFeIiZsd6dDO2wx96ccB8=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=dkWUwKsCU6S1CL3kyHIIH3oQ72ve1hntz99r8J3m2TYreNAkJ6vZx+db6dGlpZ1Ct\n\t5P7EQzFwTGIsYq9kEPrObeh8e7PPxNMvwn7r6jpuURH56mcAzKi7WnlNjQCn7QxZL3\n\tbnB0EeRJLXyAR6F10ZXUJBfedHtLt+Mo7w5deCCU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"dkWUwKsC\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220908014200.28728-6-laurent.pinchart@ideasonboard.com>","References":"<20220908014200.28728-1-laurent.pinchart@ideasonboard.com>\n\t<20220908014200.28728-6-laurent.pinchart@ideasonboard.com>","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 20 Sep 2022 14:40:28 +0100","Message-ID":"<166368122820.3912877.9286121009516206496@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v4 05/32] ipa: libipa: Provide a\n\tcommon base for frame contexts","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25027,"web_url":"https://patchwork.libcamera.org/comment/25027/","msgid":"","date":"2022-09-20T19:12:58","subject":"Re: [libcamera-devel] [PATCH v4 05/32] ipa: libipa: Provide a\n\tcommon base for frame contexts","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Sep 20, 2022 at 02:40:28PM +0100, Kieran Bingham wrote:\n> Quoting Laurent Pinchart (2022-09-08 02:41:33)\n> > From: Kieran Bingham \n> > \n> > Provide a common FrameContext as a base for IPA modules to inherit from.\n> > \n> > This will allow having a common set of parameters for every frame\n> > context managed by the FCQueue implementation.\n> > \n> > Signed-off-by: Kieran Bingham \n> \n> Missing SoB.\n> > ---\n> > Changes since v3:\n> > \n> > - Rename structure from IPAFrameContext to FrameContext\n> > - Move changes to IPA modules to separate patches\n> > - Make the FrameContext::frame member private\n> > ---\n> > src/ipa/libipa/fc_queue.cpp | 18 ++++++++++++++++++\n> > src/ipa/libipa/fc_queue.h | 10 ++++++++++\n> > 2 files changed, 28 insertions(+)\n> > \n> > diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp\n> > index b81d497e255a..8f61c85131c9 100644\n> > --- a/src/ipa/libipa/fc_queue.cpp\n> > +++ b/src/ipa/libipa/fc_queue.cpp\n> > @@ -20,6 +20,24 @@ namespace ipa {\n> > * \\brief Queue to access per-frame Context\n> > */\n> > \n> > +/**\n> > + * \\struct FrameContext\n> > + * \\brief Context for a frame\n> > + *\n> > + * The frame context stores data specific to a single frame processed by the\n> > + * IPA. Each frame processed by the IPA has a context associated with it,\n> > + * accessible through the Frame Context Queue.\n> > + *\n> > + * Fields in the frame context should reflect values and controls associated\n> > + * with the specific frame as requested by the application, and as configured by\n> > + * the hardware. Fields can be read by algorithms to determine if they should\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> > + * \\brief The frame number\n> > + */\n> > +\n> > /**\n> > * \\class FCQueue\n> > * \\brief A support class for queueing FrameContext instances through the IPA\n> > diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h\n> > index 0f3af0f3a189..d8a5b8297a70 100644\n> > --- a/src/ipa/libipa/fc_queue.h\n> > +++ b/src/ipa/libipa/fc_queue.h\n> > @@ -8,6 +8,7 @@\n> > #pragma once\n> > \n> > #include \n> > +#include \n> > #include \n> > \n> > #include \n> > @@ -18,6 +19,15 @@ LOG_DECLARE_CATEGORY(FCQueue)\n> > \n> > namespace ipa {\n> > \n> > +template\n> > +class FCQueue;\n> > +\n> > +struct FrameContext {\n> > +private:\n> > + template friend class FCQueue;\n> > + uint32_t frame;\n> \n> Seems fine to me.\n> \n> I'm still not sure what else is going to end up in here. There's a\n> chance that we might need to store the incoming Request ControlList (or\n> a copy of) ... so I still think there's sufficient requirement for the\n> common base type.\n\nI haven't found a need for storing additional data in the base class in\nthe RkISP1 IPA. We'll see.\n\n> Reviewed-by: Kieran Bingham \n> \n> > +};\n> > +\n> > template\n> > class FCQueue\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 AD2F0C3272\n\tfor ;\n\tTue, 20 Sep 2022 19:13:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 23E8B621C7;\n\tTue, 20 Sep 2022 21:13:16 +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 E23886218B\n\tfor ;\n\tTue, 20 Sep 2022 21:13:13 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3C5966BE;\n\tTue, 20 Sep 2022 21:13:13 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1663701196;\n\tbh=ghMCUXhmETiwJNd3s58C1e4f2DqjqHKKrZNktIm5Q9E=;\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=yaOyf/Z7XtT7TRFluQ86pcksVRoPQBkKlIHrMt+EztygwpYW+ORr4whhl2CMuENo8\n\tWXSNvotCJGwhkR2I789R9qS4xmnAFnypLaBRePeTtp8UeFAVBWyxtb+dCKsSzYLnma\n\tHqxmJsUMPFIATja6Eft73UJoGuMNqIY5rA5iK7OXfuLSIBNhFHo+44r2sNfG48E+6W\n\tGx4GHNS7PlzygmgGQDNRzFXhlgK0HtyYfZ1dstvkeOt4NF8OBXcEUwVL8MWgt1al4B\n\tlQg6NPXcvxk4J9LP8C7uIa9sok9gHNiAPpFU6XX93wbgI639ld5Q9YTAJ+lxVvkUgs\n\tKP6IsBbforIwA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1663701193;\n\tbh=ghMCUXhmETiwJNd3s58C1e4f2DqjqHKKrZNktIm5Q9E=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jbdVVnHeVFvutSl2oxV3eNoPOHd/IFLg32JNHcVFtisYJbQCCrQSxXiRe2dbKkX5m\n\tWLXOl/VII1U4ZMN/pfm66VoH5YGjmPJvBYaUS6uJm+PyZ2DS1VhuLLsb4KML8gKEjD\n\tNgaJ40oNiKVnBsguo4IyVwUhg7o9+1xj5nTedd6U="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"jbdVVnHe\"; dkim-atps=neutral","Date":"Tue, 20 Sep 2022 22:12:58 +0300","To":"Kieran Bingham ","Message-ID":"","References":"<20220908014200.28728-1-laurent.pinchart@ideasonboard.com>\n\t<20220908014200.28728-6-laurent.pinchart@ideasonboard.com>\n\t<166368122820.3912877.9286121009516206496@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<166368122820.3912877.9286121009516206496@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v4 05/32] ipa: libipa: Provide a\n\tcommon base for frame contexts","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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25052,"web_url":"https://patchwork.libcamera.org/comment/25052/","msgid":"<20220921175344.lewo4rxj4mdb4o76@uno.localdomain>","date":"2022-09-21T17:53:44","subject":"Re: [libcamera-devel] [PATCH v4 05/32] ipa: libipa: Provide a\n\tcommon base for frame contexts","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n\nOn Thu, Sep 08, 2022 at 04:41:33AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> From: Kieran Bingham \n>\n> Provide a common FrameContext as a base for IPA modules to inherit from.\n>\n> This will allow having a common set of parameters for every frame\n> context managed by the FCQueue implementation.\n>\n> Signed-off-by: Kieran Bingham \n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n> Changes since v3:\n>\n> - Rename structure from IPAFrameContext to FrameContext\n> - Move changes to IPA modules to separate patches\n> - Make the FrameContext::frame member private\n> ---\n> src/ipa/libipa/fc_queue.cpp | 18 ++++++++++++++++++\n> src/ipa/libipa/fc_queue.h | 10 ++++++++++\n> 2 files changed, 28 insertions(+)\n>\n> diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp\n> index b81d497e255a..8f61c85131c9 100644\n> --- a/src/ipa/libipa/fc_queue.cpp\n> +++ b/src/ipa/libipa/fc_queue.cpp\n> @@ -20,6 +20,24 @@ namespace ipa {\n> * \\brief Queue to access per-frame Context\n> */\n>\n> +/**\n> + * \\struct FrameContext\n> + * \\brief Context for a frame\n> + *\n> + * The frame context stores data specific to a single frame processed by the\n> + * IPA. Each frame processed by the IPA has a context associated with it,\n> + * accessible through the Frame Context Queue.\n> + *\n> + * Fields in the frame context should reflect values and controls associated\n> + * with the specific frame as requested by the application, and as configured by\n> + * the hardware. Fields can be read by algorithms to determine if they should\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> + * \\brief The frame number\n> + */\n> +\n> /**\n> * \\class FCQueue\n> * \\brief A support class for queueing FrameContext instances through the IPA\n> diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h\n> index 0f3af0f3a189..d8a5b8297a70 100644\n> --- a/src/ipa/libipa/fc_queue.h\n> +++ b/src/ipa/libipa/fc_queue.h\n> @@ -8,6 +8,7 @@\n> #pragma once\n>\n> #include \n> +#include \n> #include \n>\n> #include \n> @@ -18,6 +19,15 @@ LOG_DECLARE_CATEGORY(FCQueue)\n>\n> namespace ipa {\n>\n> +template\n> +class FCQueue;\n> +\n> +struct FrameContext {\n> +private:\n> +\ttemplate friend class FCQueue;\n> +\tuint32_t frame;\n> +};\n> +\n> template\n> class FCQueue\n> {\n> --\n> Regards,\n>\n> Laurent Pinchart\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 3AD25C3272\n\tfor ;\n\tWed, 21 Sep 2022 17:53:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7A799621E9;\n\tWed, 21 Sep 2022 19:53:48 +0200 (CEST)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F029A600AA\n\tfor ;\n\tWed, 21 Sep 2022 19:53:46 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 3E13A100003;\n\tWed, 21 Sep 2022 17:53:46 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1663782828;\n\tbh=D1amqrAfMv4jx41qJPRZv4vdUo0G3WzfzXecKB8/ga8=;\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=e5siz5Lmj6uqlaH95uLnLJwEjQNhEXaGQ/v3aM+U20bBpoF8H+GQMHXjSAwUCKpBV\n\taxUOiW+xSae8qgDoe8Txx7iUHiNPTXBCPv7A9OSMiAMJGr08WpQZmT1uEdSfHye/8g\n\tgpsexRLneQcWhDrjUJgPuGq008ePKQTQzZpIgJctYFRqU2uWPwacJldPw5M7e4rK5B\n\t5/6TBA/J7DZe1NGZW7ZO4sHJMkL6uC0v9xCIA/Bq6kMRzC3WoPPHbBH8Axo9pAX9qi\n\t/1TJ0bVytLctzuKnr+tvb3X+UvjOOpKM3psmeGOTdVgw8PLUuE1O6B+m9CY5G9ZbBS\n\t44V688iaAUnzA==","Date":"Wed, 21 Sep 2022 19:53:44 +0200","To":"Laurent Pinchart ","Message-ID":"<20220921175344.lewo4rxj4mdb4o76@uno.localdomain>","References":"<20220908014200.28728-1-laurent.pinchart@ideasonboard.com>\n\t<20220908014200.28728-6-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220908014200.28728-6-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 05/32] ipa: libipa: Provide a\n\tcommon base for frame contexts","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":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25073,"web_url":"https://patchwork.libcamera.org/comment/25073/","msgid":"<166383832008.56880.8240194557273446281@Monstersaurus>","date":"2022-09-22T09:18:40","subject":"Re: [libcamera-devel] [PATCH v4 05/32] ipa: libipa: Provide a\n\tcommon base for frame contexts","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2022-09-20 20:12:58)\n> On Tue, Sep 20, 2022 at 02:40:28PM +0100, Kieran Bingham wrote:\n> > Quoting Laurent Pinchart (2022-09-08 02:41:33)\n> > > From: Kieran Bingham \n> > > \n> > > Provide a common FrameContext as a base for IPA modules to inherit from.\n> > > \n> > > This will allow having a common set of parameters for every frame\n> > > context managed by the FCQueue implementation.\n> > > \n> > > Signed-off-by: Kieran Bingham \n> > \n> > Missing SoB.\n> > > ---\n> > > Changes since v3:\n> > > \n> > > - Rename structure from IPAFrameContext to FrameContext\n> > > - Move changes to IPA modules to separate patches\n> > > - Make the FrameContext::frame member private\n> > > ---\n> > > src/ipa/libipa/fc_queue.cpp | 18 ++++++++++++++++++\n> > > src/ipa/libipa/fc_queue.h | 10 ++++++++++\n> > > 2 files changed, 28 insertions(+)\n> > > \n> > > diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp\n> > > index b81d497e255a..8f61c85131c9 100644\n> > > --- a/src/ipa/libipa/fc_queue.cpp\n> > > +++ b/src/ipa/libipa/fc_queue.cpp\n> > > @@ -20,6 +20,24 @@ namespace ipa {\n> > > * \\brief Queue to access per-frame Context\n> > > */\n> > > \n> > > +/**\n> > > + * \\struct FrameContext\n> > > + * \\brief Context for a frame\n> > > + *\n> > > + * The frame context stores data specific to a single frame processed by the\n> > > + * IPA. Each frame processed by the IPA has a context associated with it,\n> > > + * accessible through the Frame Context Queue.\n> > > + *\n> > > + * Fields in the frame context should reflect values and controls associated\n> > > + * with the specific frame as requested by the application, and as configured by\n> > > + * the hardware. Fields can be read by algorithms to determine if they should\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> > > + * \\brief The frame number\n> > > + */\n> > > +\n> > > /**\n> > > * \\class FCQueue\n> > > * \\brief A support class for queueing FrameContext instances through the IPA\n> > > diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h\n> > > index 0f3af0f3a189..d8a5b8297a70 100644\n> > > --- a/src/ipa/libipa/fc_queue.h\n> > > +++ b/src/ipa/libipa/fc_queue.h\n> > > @@ -8,6 +8,7 @@\n> > > #pragma once\n> > > \n> > > #include \n> > > +#include \n> > > #include \n> > > \n> > > #include \n> > > @@ -18,6 +19,15 @@ LOG_DECLARE_CATEGORY(FCQueue)\n> > > \n> > > namespace ipa {\n> > > \n> > > +template\n> > > +class FCQueue;\n> > > +\n> > > +struct FrameContext {\n> > > +private:\n> > > + template friend class FCQueue;\n> > > + uint32_t frame;\n> > \n> > Seems fine to me.\n> > \n> > I'm still not sure what else is going to end up in here. There's a\n> > chance that we might need to store the incoming Request ControlList (or\n> > a copy of) ... so I still think there's sufficient requirement for the\n> > common base type.\n> \n> I haven't found a need for storing additional data in the base class in\n> the RkISP1 IPA. We'll see.\n\nWe should add a \n\tControlList metadata;\n\nhere in the base FrameContext. It can certainly be on top of this\nseries, and it should then get sent to the Pipeline handler to merge in\nto the Request::metadata().\n\nThat should be common to all algortihms using this design, and the\nalgorithms can use it to populate the Request metadata at the earliest\nopportunity.\n\n--\nKieran\n\n\n> \n> > Reviewed-by: Kieran Bingham \n> > \n> > > +};\n> > > +\n> > > template\n> > > class FCQueue\n> > > {\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 17124C0DA4\n\tfor ;\n\tThu, 22 Sep 2022 09:18:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 582B162202;\n\tThu, 22 Sep 2022 11:18:44 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E0C8E6219B\n\tfor ;\n\tThu, 22 Sep 2022 11:18:42 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 65D196BE;\n\tThu, 22 Sep 2022 11:18:42 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1663838324;\n\tbh=ErTY1Z0htUiXafTjCd2uzKKXznTIN876zVsdT7ohJ3M=;\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=3UnuBniZLC98RXtbnNLTt7fBZ+LdfvwWlg+mXCiprazKTLT7SLngvIZTHfar4eBHN\n\tMxkz8pi/muNR+f5aTN54IyQe3uW+GmLzfmeB7mGqLh9xkj4224Ltya1PQT7+pjlmer\n\tHGy9GIeS3mc/7gL6AclREW3f1VBAsq/QhqQIh8TLqz/I64lleI2sMGSKU0f+mS+AuG\n\tguTyOfrRC+rfCrUkwBf6Bie/94suRftczuSxGH3gI9xnMTdruvBxLdNEKeOnJj1emO\n\tTPe18pnzbVp+rhB895/Sp0hOj9uKdOWBHyuuM/tPxlth33Jz7+dkNMgQeNJk9DFi4R\n\tlIsLs5fg/fOpQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1663838322;\n\tbh=ErTY1Z0htUiXafTjCd2uzKKXznTIN876zVsdT7ohJ3M=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=uru8d4vWEbgK9QzpMmLHDVqW1S07HntF6sDujoLuzJJUTNXoU99tRnld92IxuQwLI\n\tkreNhWkazhC7Hvlfzwg11VOwUykPyGM3hlylR+oD2lxTkP6En1k9R6c81sF/fYdQPX\n\tYkBQ9xlqqc/kfG7rFXe3pNs88CgvWM1ArbDx0dAE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"uru8d4vW\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20220908014200.28728-1-laurent.pinchart@ideasonboard.com>\n\t<20220908014200.28728-6-laurent.pinchart@ideasonboard.com>\n\t<166368122820.3912877.9286121009516206496@Monstersaurus>\n\t","To":"Laurent Pinchart ","Date":"Thu, 22 Sep 2022 10:18:40 +0100","Message-ID":"<166383832008.56880.8240194557273446281@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v4 05/32] ipa: libipa: Provide a\n\tcommon base for frame contexts","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\" "}},{"id":25087,"web_url":"https://patchwork.libcamera.org/comment/25087/","msgid":"","date":"2022-09-22T19:37:44","subject":"Re: [libcamera-devel] [PATCH v4 05/32] ipa: libipa: Provide a\n\tcommon base for frame contexts","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Sep 22, 2022 at 10:18:40AM +0100, Kieran Bingham wrote:\n> Quoting Laurent Pinchart (2022-09-20 20:12:58)\n> > On Tue, Sep 20, 2022 at 02:40:28PM +0100, Kieran Bingham wrote:\n> > > Quoting Laurent Pinchart (2022-09-08 02:41:33)\n> > > > From: Kieran Bingham \n> > > > \n> > > > Provide a common FrameContext as a base for IPA modules to inherit from.\n> > > > \n> > > > This will allow having a common set of parameters for every frame\n> > > > context managed by the FCQueue implementation.\n> > > > \n> > > > Signed-off-by: Kieran Bingham \n> > > \n> > > Missing SoB.\n> > > \n> > > > ---\n> > > > Changes since v3:\n> > > > \n> > > > - Rename structure from IPAFrameContext to FrameContext\n> > > > - Move changes to IPA modules to separate patches\n> > > > - Make the FrameContext::frame member private\n> > > > ---\n> > > > src/ipa/libipa/fc_queue.cpp | 18 ++++++++++++++++++\n> > > > src/ipa/libipa/fc_queue.h | 10 ++++++++++\n> > > > 2 files changed, 28 insertions(+)\n> > > > \n> > > > diff --git a/src/ipa/libipa/fc_queue.cpp b/src/ipa/libipa/fc_queue.cpp\n> > > > index b81d497e255a..8f61c85131c9 100644\n> > > > --- a/src/ipa/libipa/fc_queue.cpp\n> > > > +++ b/src/ipa/libipa/fc_queue.cpp\n> > > > @@ -20,6 +20,24 @@ namespace ipa {\n> > > > * \\brief Queue to access per-frame Context\n> > > > */\n> > > > \n> > > > +/**\n> > > > + * \\struct FrameContext\n> > > > + * \\brief Context for a frame\n> > > > + *\n> > > > + * The frame context stores data specific to a single frame processed by the\n> > > > + * IPA. Each frame processed by the IPA has a context associated with it,\n> > > > + * accessible through the Frame Context Queue.\n> > > > + *\n> > > > + * Fields in the frame context should reflect values and controls associated\n> > > > + * with the specific frame as requested by the application, and as configured by\n> > > > + * the hardware. Fields can be read by algorithms to determine if they should\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> > > > + * \\brief The frame number\n> > > > + */\n> > > > +\n> > > > /**\n> > > > * \\class FCQueue\n> > > > * \\brief A support class for queueing FrameContext instances through the IPA\n> > > > diff --git a/src/ipa/libipa/fc_queue.h b/src/ipa/libipa/fc_queue.h\n> > > > index 0f3af0f3a189..d8a5b8297a70 100644\n> > > > --- a/src/ipa/libipa/fc_queue.h\n> > > > +++ b/src/ipa/libipa/fc_queue.h\n> > > > @@ -8,6 +8,7 @@\n> > > > #pragma once\n> > > > \n> > > > #include \n> > > > +#include \n> > > > #include \n> > > > \n> > > > #include \n> > > > @@ -18,6 +19,15 @@ LOG_DECLARE_CATEGORY(FCQueue)\n> > > > \n> > > > namespace ipa {\n> > > > \n> > > > +template\n> > > > +class FCQueue;\n> > > > +\n> > > > +struct FrameContext {\n> > > > +private:\n> > > > + template friend class FCQueue;\n> > > > + uint32_t frame;\n> > > \n> > > Seems fine to me.\n> > > \n> > > I'm still not sure what else is going to end up in here. There's a\n> > > chance that we might need to store the incoming Request ControlList (or\n> > > a copy of) ... so I still think there's sufficient requirement for the\n> > > common base type.\n> > \n> > I haven't found a need for storing additional data in the base class in\n> > the RkISP1 IPA. We'll see.\n> \n> We should add a \n> \tControlList metadata;\n> \n> here in the base FrameContext. It can certainly be on top of this\n> series, and it should then get sent to the Pipeline handler to merge in\n> to the Request::metadata().\n\nIt's a good idea, but I wonder how we can achieve that given that the\nControlList should be constructed with a camera-specific ControlInfoMap.\n\n> That should be common to all algortihms using this design, and the\n> algorithms can use it to populate the Request metadata at the earliest\n> opportunity.\n> \n> > > Reviewed-by: Kieran Bingham \n> > > \n> > > > +};\n> > > > +\n> > > > template\n> > > > class FCQueue\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 6FA8CC0DA4\n\tfor ;\n\tThu, 22 Sep 2022 19:38:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B14F06220C;\n\tThu, 22 Sep 2022 21:38:00 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CB5566219A\n\tfor ;\n\tThu, 22 Sep 2022 21:37:59 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3D8CA6BE;\n\tThu, 22 Sep 2022 21:37:59 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1663875480;\n\tbh=zovZ+4tGH+z1BK1+dNaMIQhmqO1uWqxLfJ9eodGA1nA=;\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=Xy5P7b1Xs30HSCDbDI3WrcLY1GpmPlV9dJBOY6UA9tbeKvR0ENu1od6+0ubaJwfuT\n\tHThP/wiJ0CNII59KP/qAiXR83tRoAyAZVUzfY0XIljiM3xua9K4Kvk/7hezYRDJkm8\n\tvay1TkW56uqjUl/DyzlaDYiVEHuwMm4r2Qa6cltMECkhDe4EYIaaTmz7RnlG+r21GP\n\tAEvRM8gqeMGrr5ahuQjwzTq/5KeQjh+D1v7qeWYpM8EQkRcdZcjSpx7M59ru9UEzNH\n\tOOwcetxJS+MEB8eCcglod9aG7x1tONW4IH/1EaiU4dHTgaqOE+/rDucmuyecs6t9Bw\n\tKhTZkZ4tN8M+w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1663875479;\n\tbh=zovZ+4tGH+z1BK1+dNaMIQhmqO1uWqxLfJ9eodGA1nA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=a6aZ9KGQlZRBRS3Fs4ACuuxDJJvThRPlZ8BGF8/FTS5FVTF+5lU2tfu3rWcflT1Ub\n\tujP0TgPOG3qufZ9YNfIQjdPWd5D+S59Pze9ZmUrTGxOHji6qehZkbilpuvQ6DXz/15\n\trNnf/rnqVa/uyjIU6c8ZuoEeoOzA5HhfxwVqc1Wg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"a6aZ9KGQ\"; dkim-atps=neutral","Date":"Thu, 22 Sep 2022 22:37:44 +0300","To":"Kieran Bingham ","Message-ID":"","References":"<20220908014200.28728-1-laurent.pinchart@ideasonboard.com>\n\t<20220908014200.28728-6-laurent.pinchart@ideasonboard.com>\n\t<166368122820.3912877.9286121009516206496@Monstersaurus>\n\t\n\t<166383832008.56880.8240194557273446281@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<166383832008.56880.8240194557273446281@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v4 05/32] ipa: libipa: Provide a\n\tcommon base for frame contexts","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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]