[{"id":21053,"web_url":"https://patchwork.libcamera.org/comment/21053/","msgid":"<163733313807.1089182.15933382009619073695@Monstersaurus>","date":"2021-11-19T14:45:38","subject":"Re: [libcamera-devel] [PATCH v1 3/8] ipa: rkisp1: Introduce\n\tIPAContext","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jean-Michel Hautbois (2021-11-19 11:16:49)\n> Before using any algorithm, we want the IPAContext to be ready for\n> those. Introduce the IPAContext as in IPA::IPU3 but empty. Each\n\nI might say \"Introduce the IPAContext following the existing design from\nIPA::IPU3.\"\n\nThere's plenty of duplication from the IPU3 here, so maybe Laurent's\nclass template idea might help - but this is going to quickly diverge\nand have only RKISP specifics added, so I dont' know how easy it will be\nto have a common base...\n\nSo given that - a simple start makes sense here - we can always refactor\nlater.\n\n\n\n> algorithm will then introduce the needed fields.\n> \n> Signed-off-by: Jean-Michel Hautbois \n> ---\n> src/ipa/rkisp1/ipa_context.cpp | 58 ++++++++++++++++++++++++++++++++++\n> src/ipa/rkisp1/ipa_context.h | 36 +++++++++++++++++++++\n> src/ipa/rkisp1/meson.build | 7 +++-\n> src/ipa/rkisp1/rkisp1.cpp | 7 ++++\n> 4 files changed, 107 insertions(+), 1 deletion(-)\n> create mode 100644 src/ipa/rkisp1/ipa_context.cpp\n> create mode 100644 src/ipa/rkisp1/ipa_context.h\n> \n> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> new file mode 100644\n> index 00000000..819b2c73\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/ipa_context.cpp\n> @@ -0,0 +1,58 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Ideas On Board\n> + *\n> + * ipa_context.cpp - RkISP1 IPA Context\n> + */\n> +\n> +#include \"ipa_context.h\"\n> +\n> +/**\n> + * \\file ipa_context.h\n> + * \\brief Context and state information shared between the algorithms\n> + */\n> +\n> +namespace libcamera::ipa::rkisp1 {\n> +\n> +/**\n> + * \\struct IPASessionConfiguration\n> + * \\brief Session configuration for the IPA module\n> + *\n> + * The session configuration contains all IPA configuration parameters that\n> + * remain constant during the capture session, from IPA module start to stop.\n> + * It is typically set during the configure() operation of the IPA module, but\n> + * may also be updated in the start() operation.\n> + */\n> +\n> +/**\n> + * \\struct IPAFrameContext\n> + * \\brief Per-frame context for algorithms\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 IPAContext structure.\n> + *\n> + * \\todo Detail how to access contexts for a particular frame\n> + *\n> + * Each of the fields in the frame context belongs to either a specific\n> + * algorithm, or to the top-level IPA module. A field may be read by any\n> + * algorithm, but should only be written by its owner.\n> + */\n> +\n> +/**\n> + * \\struct IPAContext\n> + * \\brief Global IPA context data shared between all algorithms\n> + *\n> + * \\var IPAContext::configuration\n> + * \\brief The IPA session configuration, immutable during the session\n> + *\n> + * \\var IPAContext::frameContext\n> + * \\brief The frame context for the frame being processed\n> + *\n> + * \\todo While the frame context is supposed to be per-frame, this\n> + * single frame context stores data related to both the current frame\n> + * and the previous frames, with fields being updated as the algorithms\n> + * are run. This needs to be turned into real per-frame data storage.\n> + */\n> +\n> +} /* namespace libcamera::ipa::rkisp1 */\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> new file mode 100644\n> index 00000000..a1dc39fe\n> --- /dev/null\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -0,0 +1,36 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Ideas On Board\n> + *\n> + * ipa_context.h - RkISP1 IPA Context\n> + *\n> + */\n> +#ifndef __LIBCAMERA_RKISP1_IPA_CONTEXT_H__\n> +#define __LIBCAMERA_RKISP1_IPA_CONTEXT_H__\n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \n\nOnly add these headers in the patch that introduces the need.\n\nwith those removed:\n\nReviewed-by: Kieran Bingham \n\n\n> +\n> +namespace libcamera {\n> +\n> +namespace ipa::rkisp1 {\n> +\n> +struct IPASessionConfiguration {\n> +};\n> +\n> +struct IPAFrameContext {\n> +};\n> +\n> +struct IPAContext {\n> + IPASessionConfiguration configuration;\n> + IPAFrameContext frameContext;\n> +};\n> +\n> +} /* namespace ipa::rkisp1 */\n> +\n> +} /* namespace libcamera*/\n> +\n> +#endif /* __LIBCAMERA_RKISP1_IPA_CONTEXT_H__ */\n> diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build\n> index f76b37f5..3683c922 100644\n> --- a/src/ipa/rkisp1/meson.build\n> +++ b/src/ipa/rkisp1/meson.build\n> @@ -2,8 +2,13 @@\n> \n> ipa_name = 'ipa_rkisp1'\n> \n> +rkisp1_ipa_sources = files([\n> + 'ipa_context.cpp',\n> + 'rkisp1.cpp',\n> +])\n> +\n> mod = shared_module(ipa_name,\n> - ['rkisp1.cpp', libcamera_generated_ipa_headers],\n> + [rkisp1_ipa_sources, libcamera_generated_ipa_headers],\n> name_prefix : '',\n> include_directories : [ipa_includes, libipa_includes],\n> dependencies : libcamera_private,\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index ff8d8712..e0933e22 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -25,6 +25,7 @@\n> \n> #include \n> \n> +#include \"ipa_context.h\"\n> #include \"libipa/camera_sensor_helper.h\"\n> \n> namespace libcamera {\n> @@ -78,6 +79,9 @@ private:\n> \n> /* Interface to the Camera Helper */\n> std::unique_ptr camHelper_;\n> +\n> + /* Local parameter storage */\n> + struct IPAContext context_;\n> };\n> \n> int IPARkISP1::init([[maybe_unused]] const IPASettings &settings,\n> @@ -165,6 +169,9 @@ int IPARkISP1::configure([[maybe_unused]] const IPACameraSensorInfo &info,\n> << \"Exposure: \" << minExposure_ << \"-\" << maxExposure_\n> << \" Gain: \" << minGain_ << \"-\" << maxGain_;\n> \n> + /* Clean context at configuration */\n> + context_ = {};\n> +\n> return 0;\n> }\n> \n> -- \n> 2.32.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 48512BF415\n\tfor ;\n\tFri, 19 Nov 2021 14:45:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 74D6060371;\n\tFri, 19 Nov 2021 15:45:42 +0100 (CET)","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 7F0D5600B5\n\tfor ;\n\tFri, 19 Nov 2021 15:45:41 +0100 (CET)","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 1E2811959;\n\tFri, 19 Nov 2021 15:45:41 +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=\"dfH+t4rJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637333141;\n\tbh=RD7YQ6XgX5v9TxP5P+zGMCnzAH2grBUsAibN6qG/Lkc=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=dfH+t4rJU4r9nYwGthuUXEGdC6N44lTLFUCLD4zzYZd9abthCvT4BtCByB2Mf1tXx\n\th1enRg+VgqEDYsQ8C0ygwQVfoj3p5BgLXj7wSB508AiT5GBtU/wIbqoo4zcYmRkhNB\n\tisxSLq55YGHbXgi/YsiZS36fg8J+iTd3qp/5aMs0=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211119111654.68445-4-jeanmichel.hautbois@ideasonboard.com>","References":"<20211119111654.68445-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20211119111654.68445-4-jeanmichel.hautbois@ideasonboard.com>","From":"Kieran Bingham ","To":"Jean-Michel Hautbois ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 19 Nov 2021 14:45:38 +0000","Message-ID":"<163733313807.1089182.15933382009619073695@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v1 3/8] ipa: rkisp1: Introduce\n\tIPAContext","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\" "}}]