[{"id":23265,"web_url":"https://patchwork.libcamera.org/comment/23265/","msgid":"<20220531065938.GK2630765@pyrite.rasen.tech>","date":"2022-05-31T06:59:38","subject":"Re: [libcamera-devel] [RFC PATCH v2 1/3] ipa: ipu3: Separate out\n\tframe context queue as a distinct class","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Hi Umang,\n\nOn Fri, May 27, 2022 at 09:17:38PM +0200, Umang Jain via libcamera-devel wrote:\n> There are cases where we need more checks and balance to be carried out\n> by the frame context queue class. For that, separate it out as a\n> distinct class on which we can build upon.\n> \n> For now, a minimialistic implementation is provided with .get(frame)\n> helper which returns a IPAFrameContext for the required frame.\n> Going ahead more such helpers can be provided to access/modify the\n> frame context queue.\n> \n> Signed-off-by: Umang Jain \n\nReviewed-by: Paul Elder \n\n> ---\n> src/ipa/ipu3/ipa_context.cpp | 49 ++++++++++++++++++++++++++++++++++--\n> src/ipa/ipu3/ipa_context.h | 11 +++++++-\n> src/ipa/ipu3/ipu3.cpp | 6 ++---\n> 3 files changed, 60 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> index 13cdb835..e5010e3f 100644\n> --- a/src/ipa/ipu3/ipa_context.cpp\n> +++ b/src/ipa/ipu3/ipa_context.cpp\n> @@ -7,12 +7,18 @@\n> \n> #include \"ipa_context.h\"\n> \n> +#include \n> +\n> /**\n> * \\file ipa_context.h\n> * \\brief Context and state information shared between the algorithms\n> */\n> \n> -namespace libcamera::ipa::ipu3 {\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(IPAIPU3)\n> +\n> +namespace ipa::ipu3 {\n> \n> /**\n> * \\struct IPASessionConfiguration\n> @@ -211,4 +217,43 @@ IPAFrameContext::IPAFrameContext(uint32_t id, const ControlList &reqControls)\n> * \\brief Analogue gain multiplier\n> */\n> \n> -} /* namespace libcamera::ipa::ipu3 */\n> +/**\n> + * \\brief FCQueue constructor\n> + */\n> +FCQueue::FCQueue()\n> +{\n> +\tclear();\n> +}\n> +\n> +/**\n> + * Retrieve the IPAFrameContext for the frame\n> + * \\param[in] frame Frame number for which the IPAFrameContext needs to\n> + * retrieved\n> + *\n> + * \\return Reference to the IPAFrameContext\n> + */\n> +IPAFrameContext &FCQueue::get(uint32_t frame)\n> +{\n> +\tIPAFrameContext &frameContext = this->at(frame % kMaxFrameContexts);\n> +\n> +\tif (frame != frameContext.frame) {\n> +\t\tLOG(IPAIPU3, Warning)\n> +\t\t\t<< \"Got wrong frame context for frame\" << frame\n> +\t\t\t<< \" or frame context doesn't exist yet\";\n> +\t}\n> +\n> +\treturn frameContext;\n> +}\n> +\n> +/**\n> + * \\brief Clear the FCQueue by resetting all the entries in the ring-buffer\n> + */\n> +void FCQueue::clear()\n> +{\n> +\tIPAFrameContext initFrameContext;\n> +\tthis->fill(initFrameContext);\n> +}\n> +\n> +} /* namespace ipa::ipu3 */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> index 42e11141..61454b41 100644\n> --- a/src/ipa/ipu3/ipa_context.h\n> +++ b/src/ipa/ipu3/ipa_context.h\n> @@ -89,11 +89,20 @@ struct IPAFrameContext {\n> \tControlList frameControls;\n> };\n> \n> +class FCQueue : public std::array\n> +{\n> +public:\n> +\tFCQueue();\n> +\n> +\tvoid clear();\n> +\tIPAFrameContext &get(uint32_t frame);\n> +};\n> +\n> struct IPAContext {\n> \tIPASessionConfiguration configuration;\n> \tIPAActiveState activeState;\n> \n> -\tstd::array frameContexts;\n> +\tFCQueue frameContexts;\n> };\n> \n> } /* namespace ipa::ipu3 */\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 2f6bb672..c48d2f62 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -456,8 +456,8 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n> \n> \t/* Clean IPAActiveState at each reconfiguration. */\n> \tcontext_.activeState = {};\n> -\tIPAFrameContext initFrameContext;\n> -\tcontext_.frameContexts.fill(initFrameContext);\n> +\tcontext_.frameContexts.clear();\n> +\n> \n> \tif (!validateSensorControls()) {\n> \t\tLOG(IPAIPU3, Error) << \"Sensor control validation failed.\";\n> @@ -569,7 +569,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> \tconst ipu3_uapi_stats_3a *stats =\n> \t\treinterpret_cast(mem.data());\n> \n> -\tIPAFrameContext &frameContext = context_.frameContexts[frame % kMaxFrameContexts];\n> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> \n> \tif (frameContext.frame != frame)\n> \t\tLOG(IPAIPU3, Warning) << \"Frame \" << frame << \" does not match its frame context\";\n> -- \n> 2.31.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 2B6E2BD161\n\tfor ;\n\tTue, 31 May 2022 06:59:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6DEA965633;\n\tTue, 31 May 2022 08:59:48 +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 8B4866040A\n\tfor ;\n\tTue, 31 May 2022 08:59:46 +0200 (CEST)","from pyrite.rasen.tech (softbank036240126034.bbtec.net\n\t[36.240.126.34])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F2F386F0;\n\tTue, 31 May 2022 08:59:44 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1653980388;\n\tbh=fLxMUMXPcwbr4FaOTEpOdGK+krHgXH7lrh8IP0/tONk=;\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=w9OCd2wlJA3j37B5pgwgSGjulALq5q6GF9jVgIpQ4+y60/pYf7sG4alOiviaEJllp\n\tiHWCTRJWMvCLy5YRS4QZ/hLZYPn38dl7Irg6GZXgU5MspR41zq6w4EoztCKIWktTQ/\n\tvBmaH/QhPAsbtf7J/CDxQcoHJ8L/WgD7wd3d4FM5miyLxojTsHahKDL2TkZChoizgV\n\t/xITR4LaMBVT1Ffz43rY8om4AIzlgBG7Y+OhWonsMBH3K2YoCxQlCgTCEmDglsDYA4\n\tuvI86DQF/hOTlWoUAKzQ/31s0rnoZ6vvy8SsObasBR2ybyNRGz3ZZE6zPBlK/SOo0e\n\tOilxGIp4GmC8g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1653980386;\n\tbh=fLxMUMXPcwbr4FaOTEpOdGK+krHgXH7lrh8IP0/tONk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ocQITOHf2tkXaN9mCY6pWB3vYi5I/3jLOQy0NrRJWwpNs0eD2JH8vdLuoA0e5R+7J\n\tHpeJTrOcrZkaMC21f2pneCb9DPj015Rd1LwMtsuzIY+c4nDih5nXCG7kqAizwRli1N\n\tZXr0mfObu0Z5kxkjTj2Hmd9MFtV6XkghNpfnnsdw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ocQITOHf\"; dkim-atps=neutral","Date":"Tue, 31 May 2022 15:59:38 +0900","To":"Umang Jain ","Message-ID":"<20220531065938.GK2630765@pyrite.rasen.tech>","References":"<20220527191740.242300-1-umang.jain@ideasonboard.com>\n\t<20220527191740.242300-2-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20220527191740.242300-2-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH v2 1/3] ipa: ipu3: Separate out\n\tframe context queue as a distinct class","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":"Paul Elder via libcamera-devel ","Reply-To":"paul.elder@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23269,"web_url":"https://patchwork.libcamera.org/comment/23269/","msgid":"<02e38ea3-fa6e-90d5-c09b-2c9dd3052232@ideasonboard.com>","date":"2022-06-01T06:17:30","subject":"Re: [libcamera-devel] [RFC PATCH v2 1/3] ipa: ipu3: Separate out\n\tframe context queue as a distinct class","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Umang,\n\nThanks for the patch !\n\nOn 27/05/2022 21:17, Umang Jain via libcamera-devel wrote:\n> There are cases where we need more checks and balance to be carried out\n> by the frame context queue class. For that, separate it out as a\n> distinct class on which we can build upon.\n> \n> For now, a minimialistic implementation is provided with .get(frame)\n> helper which returns a IPAFrameContext for the required frame.\n> Going ahead more such helpers can be provided to access/modify the\n> frame context queue.\n> \n> Signed-off-by: Umang Jain \n> ---\n> src/ipa/ipu3/ipa_context.cpp | 49 ++++++++++++++++++++++++++++++++++--\n> src/ipa/ipu3/ipa_context.h | 11 +++++++-\n> src/ipa/ipu3/ipu3.cpp | 6 ++---\n> 3 files changed, 60 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp\n> index 13cdb835..e5010e3f 100644\n> --- a/src/ipa/ipu3/ipa_context.cpp\n> +++ b/src/ipa/ipu3/ipa_context.cpp\n> @@ -7,12 +7,18 @@\n> \n> #include \"ipa_context.h\"\n> \n> +#include \n> +\n> /**\n> * \\file ipa_context.h\n> * \\brief Context and state information shared between the algorithms\n> */\n> \n> -namespace libcamera::ipa::ipu3 {\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(IPAIPU3)\n> +\n> +namespace ipa::ipu3 {\n> \n> /**\n> * \\struct IPASessionConfiguration\n> @@ -211,4 +217,43 @@ IPAFrameContext::IPAFrameContext(uint32_t id, const ControlList &reqControls)\n> * \\brief Analogue gain multiplier\n> */\n> \n\nI think you missed the class documentation, and Doxygen warns it:\n'''\n/home/jm/libcamera/src/ipa/ipu3/ipa_context.h:92: warning: Compound \nlibcamera::ipa::ipu3::FCQueue is not documented.\n'''\n\nIt is in 2/3 but should be here I think.\n\nWith this:\nReviewed-by: Jean-Michel Hautbois \n\n> -} /* namespace libcamera::ipa::ipu3 */\n> +/**\n> + * \\brief FCQueue constructor\n> + */\n> +FCQueue::FCQueue()\n> +{\n> +\tclear();\n> +}\n> +\n> +/**\n> + * Retrieve the IPAFrameContext for the frame\n> + * \\param[in] frame Frame number for which the IPAFrameContext needs to\n> + * retrieved\n> + *\n> + * \\return Reference to the IPAFrameContext\n> + */\n> +IPAFrameContext &FCQueue::get(uint32_t frame)\n> +{\n> +\tIPAFrameContext &frameContext = this->at(frame % kMaxFrameContexts);\n> +\n> +\tif (frame != frameContext.frame) {\n> +\t\tLOG(IPAIPU3, Warning)\n> +\t\t\t<< \"Got wrong frame context for frame\" << frame\n> +\t\t\t<< \" or frame context doesn't exist yet\";\n> +\t}\n> +\n> +\treturn frameContext;\n> +}\n> +\n> +/**\n> + * \\brief Clear the FCQueue by resetting all the entries in the ring-buffer\n> + */\n> +void FCQueue::clear()\n> +{\n> +\tIPAFrameContext initFrameContext;\n> +\tthis->fill(initFrameContext);\n> +}\n> +\n> +} /* namespace ipa::ipu3 */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h\n> index 42e11141..61454b41 100644\n> --- a/src/ipa/ipu3/ipa_context.h\n> +++ b/src/ipa/ipu3/ipa_context.h\n> @@ -89,11 +89,20 @@ struct IPAFrameContext {\n> \tControlList frameControls;\n> };\n> \n> +class FCQueue : public std::array\n> +{\n> +public:\n> +\tFCQueue();\n> +\n> +\tvoid clear();\n> +\tIPAFrameContext &get(uint32_t frame);\n> +};\n> +\n> struct IPAContext {\n> \tIPASessionConfiguration configuration;\n> \tIPAActiveState activeState;\n> \n> -\tstd::array frameContexts;\n> +\tFCQueue frameContexts;\n> };\n> \n> } /* namespace ipa::ipu3 */\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index 2f6bb672..c48d2f62 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -456,8 +456,8 @@ int IPAIPU3::configure(const IPAConfigInfo &configInfo,\n> \n> \t/* Clean IPAActiveState at each reconfiguration. */\n> \tcontext_.activeState = {};\n> -\tIPAFrameContext initFrameContext;\n> -\tcontext_.frameContexts.fill(initFrameContext);\n> +\tcontext_.frameContexts.clear();\n> +\n> \n> \tif (!validateSensorControls()) {\n> \t\tLOG(IPAIPU3, Error) << \"Sensor control validation failed.\";\n> @@ -569,7 +569,7 @@ void IPAIPU3::processStatsBuffer(const uint32_t frame,\n> \tconst ipu3_uapi_stats_3a *stats =\n> \t\treinterpret_cast(mem.data());\n> \n> -\tIPAFrameContext &frameContext = context_.frameContexts[frame % kMaxFrameContexts];\n> +\tIPAFrameContext &frameContext = context_.frameContexts.get(frame);\n> \n> \tif (frameContext.frame != frame)\n> \t\tLOG(IPAIPU3, Warning) << \"Frame \" << frame << \" does not match its frame context\";","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 1D123BD160\n\tfor ;\n\tWed, 1 Jun 2022 06:17:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 37B4E60105;\n\tWed, 1 Jun 2022 08:17:36 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0158F60105\n\tfor ;\n\tWed, 1 Jun 2022 08:17:33 +0200 (CEST)","from [IPV6:2a01:e0a:169:7140:2d00:c89c:70b:c9ee] (unknown\n\t[IPv6:2a01:e0a:169:7140:2d00:c89c:70b:c9ee])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8363D6D1;\n\tWed, 1 Jun 2022 08:17:33 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1654064256;\n\tbh=gV00C/sxrrJJZZGkgYh9GFtWKVQ4a/SZfkzhpIsjeVE=;\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:\n\tFrom;\n\tb=GrzYSHSUmdA4Ptjf9XG7bNgg2DCbesIx47RAD/Y/kfJ4rUSO2ycp0J5sPRSyQ2AtE\n\t09SjBB8USJRKVGlnqTELNsH1N8r0PFwH8J17sWpf78HOS+1I/h1qVB/CZAIZ0sBYpm\n\tUxpu8oV1/NXH8lDXeQ8Zshv6KSe9iBVxsJ3XceS6agJStoBvtZEZPbSa73TRj/44/N\n\tzs9z/TBSm4YoEEOxlSVNPsKYdQHcCxBNM4tuSC6OSailqSMs1g9DF4ie9g3g6AC43R\n\tPzfC94gmrxoAaBs481AiJzyxyeZslt22FgLYPsrY/2HMWt4bGLn0h2YzYjxffRouK3\n\tA3AbDQLCtAFYQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1654064253;\n\tbh=gV00C/sxrrJJZZGkgYh9GFtWKVQ4a/SZfkzhpIsjeVE=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=hC4czmSOQXpcMBVHkhteYEFWmVdaMi0PPaUdn8r0jhLBCGGELkDnFDOgzT6jpFwn7\n\t2s9nWuPGquDgFuKYUlqfcOvNowpff7rgAyftP0tMSecEn1lwHyEN7iEoD+FnNE4JTA\n\txU89fE72dgFx83i4gIjCdmMPZDvMPUmVE4FyVtXk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"hC4czmSO\"; dkim-atps=neutral","Message-ID":"<02e38ea3-fa6e-90d5-c09b-2c9dd3052232@ideasonboard.com>","Date":"Wed, 1 Jun 2022 08:17:30 +0200","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.9.1","Content-Language":"en-US","To":"Umang Jain ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220527191740.242300-1-umang.jain@ideasonboard.com>\n\t<20220527191740.242300-2-umang.jain@ideasonboard.com>","In-Reply-To":"<20220527191740.242300-2-umang.jain@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [RFC PATCH v2 1/3] ipa: ipu3: Separate out\n\tframe context queue as a distinct class","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":"Jean-Michel Hautbois via libcamera-devel\n\t","Reply-To":"Jean-Michel Hautbois ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]