[{"id":24540,"web_url":"https://patchwork.libcamera.org/comment/24540/","msgid":"","date":"2022-08-11T07:19:25","subject":"Re: [libcamera-devel] [PATCH v2 10/10] ipa: libipa: algorithm:\n\tqueueRequest(): Pass frame context","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Jacopo\n\nthank you for the patch.\n\nOn 8/5/22 19:23, Jacopo Mondi via libcamera-devel wrote:\n> From: Kieran Bingham via libcamera-devel \n>\n> Pass the current FrameContext for calls to queueRequest().\n>\n> Now that both the IPU3 and RkISP1 IPA modules comply with the same\n> interface, propagate the queueRequest() call to algorithms in the IPU3\n> IPA module.\n\n\nI feel a bit more expansion of rationale, on why do we need to propagate \nframeContexts to queueRequest() would be nice in the commit message.\n\nMaybe something on the lines:\n\n\"\"\"\nqueueRequest() already has access to incoming Request's controls and \nmight decide\nto populate frame context at initialization time (during parsing the \ncontrols).\nAs each algorithm is expected to have ownership of certain set of \ncontrols (),\nhence it should have access to the frame context to populate it for \nfuture usage.\n\"\"\"\n\n\n>\n> Signed-off-by: Kieran Bingham \n> Signed-off-by: Jacopo Mondi \n\n\nReviewed-by: Umang Jain \n\n> ---\n> src/ipa/ipu3/ipu3.cpp | 5 ++---\n> src/ipa/libipa/algorithm.cpp | 1 +\n> src/ipa/libipa/algorithm.h | 1 +\n> src/ipa/rkisp1/algorithms/cproc.cpp | 1 +\n> src/ipa/rkisp1/algorithms/cproc.h | 1 +\n> src/ipa/rkisp1/algorithms/filter.cpp | 1 +\n> src/ipa/rkisp1/algorithms/filter.h | 1 +\n> src/ipa/rkisp1/rkisp1.cpp | 4 +++-\n> 8 files changed, 11 insertions(+), 4 deletions(-)\n>\n> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp\n> index da4d416b6aef..e033891a9341 100644\n> --- a/src/ipa/ipu3/ipu3.cpp\n> +++ b/src/ipa/ipu3/ipu3.cpp\n> @@ -622,9 +622,8 @@ void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls)\n> \t/* \\todo Start processing for 'frame' based on 'controls'. */\n> \tIPU3FrameContext &frameContext = context_.frameContexts.initialise(frame);\n> \n> -\t/* \\todo Implement queueRequest to each algorithm. */\n> -\t(void)frameContext;\n> -\t(void)controls;\n> +\tfor (auto const &algo : algorithms_)\n> +\t\talgo->queueRequest(context_, frame, frameContext, controls);\n> }\n> \n> /**\n> diff --git a/src/ipa/libipa/algorithm.cpp b/src/ipa/libipa/algorithm.cpp\n> index 30eab67f71fc..c152b885aee1 100644\n> --- a/src/ipa/libipa/algorithm.cpp\n> +++ b/src/ipa/libipa/algorithm.cpp\n> @@ -88,6 +88,7 @@ namespace ipa {\n> * \\brief Provide control values to the algorithm\n> * \\param[in] context The shared IPA context\n> * \\param[in] frame The frame number to apply the control values\n> + * \\param[in] frameContext The current frame's context\n> * \\param[in] controls The list of user controls\n> *\n> * This function is called for each request queued to the camera. It provides\n> diff --git a/src/ipa/libipa/algorithm.h b/src/ipa/libipa/algorithm.h\n> index c2d990707b25..f92829ad7ac6 100644\n> --- a/src/ipa/libipa/algorithm.h\n> +++ b/src/ipa/libipa/algorithm.h\n> @@ -46,6 +46,7 @@ public:\n> \n> \tvirtual void queueRequest([[maybe_unused]] typename Module::Context &context,\n> \t\t\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t\t\t [[maybe_unused]] typename Module::FrameContext &frameContext,\n> \t\t\t\t [[maybe_unused]] const ControlList &controls)\n> \t{\n> \t}\n> diff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp\n> index edaf2d10e850..c0792926f9eb 100644\n> --- a/src/ipa/rkisp1/algorithms/cproc.cpp\n> +++ b/src/ipa/rkisp1/algorithms/cproc.cpp\n> @@ -38,6 +38,7 @@ LOG_DEFINE_CATEGORY(RkISP1CProc)\n> */\n> void ColorProcessing::queueRequest(IPAContext &context,\n> \t\t\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t\t\t [[maybe_unused]] RKISP1FrameContext &frameContext,\n> \t\t\t\t const ControlList &controls)\n> {\n> \tauto &cproc = context.activeState.cproc;\n> diff --git a/src/ipa/rkisp1/algorithms/cproc.h b/src/ipa/rkisp1/algorithms/cproc.h\n> index fde83f3c965c..13f8967e70a8 100644\n> --- a/src/ipa/rkisp1/algorithms/cproc.h\n> +++ b/src/ipa/rkisp1/algorithms/cproc.h\n> @@ -22,6 +22,7 @@ public:\n> \t~ColorProcessing() = default;\n> \n> \tvoid queueRequest(IPAContext &context, const uint32_t frame,\n> +\t\t\t RKISP1FrameContext &frameContext,\n> \t\t\t const ControlList &controls) override;\n> \tvoid prepare(IPAContext &context, unsigned int frame,\n> \t\t RKISP1FrameContext &frameContext,\n> diff --git a/src/ipa/rkisp1/algorithms/filter.cpp b/src/ipa/rkisp1/algorithms/filter.cpp\n> index f6577046442a..8c5af213d6d0 100644\n> --- a/src/ipa/rkisp1/algorithms/filter.cpp\n> +++ b/src/ipa/rkisp1/algorithms/filter.cpp\n> @@ -44,6 +44,7 @@ static constexpr uint32_t kFiltModeDefault = 0x000004f2;\n> */\n> void Filter::queueRequest(IPAContext &context,\n> \t\t\t [[maybe_unused]] const uint32_t frame,\n> +\t\t\t [[maybe_unused]] RKISP1FrameContext &frameContext,\n> \t\t\t const ControlList &controls)\n> {\n> \tauto &filter = context.activeState.filter;\n> diff --git a/src/ipa/rkisp1/algorithms/filter.h b/src/ipa/rkisp1/algorithms/filter.h\n> index 1fbe785a3cab..b10efa208a48 100644\n> --- a/src/ipa/rkisp1/algorithms/filter.h\n> +++ b/src/ipa/rkisp1/algorithms/filter.h\n> @@ -22,6 +22,7 @@ public:\n> \t~Filter() = default;\n> \n> \tvoid queueRequest(IPAContext &context, const uint32_t frame,\n> +\t\t\t RKISP1FrameContext &frameContext,\n> \t\t\t const ControlList &controls) override;\n> \tvoid prepare(IPAContext &context, unsigned int frame,\n> \t\t RKISP1FrameContext &frameContext,\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 9af8217c3b8a..548bbd202cfb 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -296,8 +296,10 @@ void IPARkISP1::unmapBuffers(const std::vector &ids)\n> \n> void IPARkISP1::queueRequest(const uint32_t frame, const ControlList &controls)\n> {\n> +\tRKISP1FrameContext &frameContext = context_.frameContexts.initialise(frame);\n> +\n> \tfor (auto const &algo : algorithms())\n> -\t\talgo->queueRequest(context_, frame, controls);\n> +\t\talgo->queueRequest(context_, frame, frameContext, controls);\n> }\n> \n> void IPARkISP1::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)","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 4B861C3272\n\tfor ;\n\tThu, 11 Aug 2022 07:19:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8D2166332B;\n\tThu, 11 Aug 2022 09:19: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 308CE63327\n\tfor ;\n\tThu, 11 Aug 2022 09:19:31 +0200 (CEST)","from [IPV6:2401:4900:1f3f:c7a1:27b3:9637:38a7:6084] (unknown\n\t[IPv6:2401:4900:1f3f:c7a1:27b3:9637:38a7:6084])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 38B493F1;\n\tThu, 11 Aug 2022 09:19:30 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660202372;\n\tbh=ztiBhH9kcTT03jGFbHtUDglcxlQwKCz1V7pRdZ640oQ=;\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=doG9yr5PlJcpU/ShMbFgez0iYaBC2indKl8I0i3gj40STynJbgMQRj+MIwZEMMVKX\n\tgxkR8Fyatp0k65OgdKQSpzlHcQ0XjG8XPQ9t8XedXG7CZ87MCFt2PuxCjP/OP7qjYI\n\to8rMdzSKHVlEvpAPgqVzySqTVqk5PbxVSqV4PPZNcPPAkD+v/kD5/slQgWTTCWFccM\n\t+Q+pUELYzpBjAEmPwMWJzoV4QJEYdpqGGJcmnfyTOkY13yNyaUfh/LCRX6dKNxz3sd\n\tOIazTQiTW6WomOdBU4aEEHlFOE1VVYn4McjIbP54KqQApTJg/ykwus+iGPdGXBzCq0\n\tckEERWvBtCcJw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1660202370;\n\tbh=ztiBhH9kcTT03jGFbHtUDglcxlQwKCz1V7pRdZ640oQ=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=b64EnLPNjmtdmP8PUY6FKnCvdjjEl+1OzISlljHWi5s9vBJ1xFiaJszgGY+uhtFzp\n\t4Im1RZLgkKJosj9ejQ1EV5uDUDOfyj1O0xQqjxLtQPceOHDweOJGFU67mV8IBmW6xD\n\tyUU2h1qiFz5eXcQw9J2HZG0SgyEcMQKc38PM1hlk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"b64EnLPN\"; dkim-atps=neutral","Message-ID":"","Date":"Thu, 11 Aug 2022 12:49:25 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","References":"<20220805135312.47497-1-jacopo@jmondi.org>\n\t<20220805135312.47497-11-jacopo@jmondi.org>","In-Reply-To":"<20220805135312.47497-11-jacopo@jmondi.org>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v2 10/10] ipa: libipa: algorithm:\n\tqueueRequest(): Pass frame context","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":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]