[{"id":23545,"web_url":"https://patchwork.libcamera.org/comment/23545/","msgid":"<20220623122907.GH3309559@pyrite.rasen.tech>","date":"2022-06-23T12:29:07","subject":"Re: [libcamera-devel] [PATCH v4 05/12] ipa: libipa: module: Add\n\tsupport for instantiation from YAML","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 Laurent,\n\nOn Mon, Jun 20, 2022 at 04:42:58AM +0300, Laurent Pinchart wrote:\n> Add a Module::createAlgorithms() function to instantiate algorithms from\n> a YamlObject. The instantiated algorithms are stored in a private member\n> variable list, exposed through the Module::algorithms() function.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Paul Elder \n\n> ---\n> src/ipa/libipa/module.cpp | 42 ++++++++++++++++------\n> src/ipa/libipa/module.h | 73 +++++++++++++++++++++++++++++++++++----\n> 2 files changed, 99 insertions(+), 16 deletions(-)\n> \n> diff --git a/src/ipa/libipa/module.cpp b/src/ipa/libipa/module.cpp\n> index 451614fd04da..7735210444b6 100644\n> --- a/src/ipa/libipa/module.cpp\n> +++ b/src/ipa/libipa/module.cpp\n> @@ -14,6 +14,8 @@\n> \n> namespace libcamera {\n> \n> +LOG_DEFINE_CATEGORY(IPAModuleAlgo)\n> +\n> /**\n> * \\brief The IPA namespace\n> *\n> @@ -76,7 +78,36 @@ namespace ipa {\n> */\n> \n> /**\n> - * \\fn Module::createAlgorithm()\n> + * \\fn Module::algorithms()\n> + * \\brief Retrieve the list of instantiated algorithms\n> + * \\return The list of instantiated algorithms\n> + */\n> +\n> +/**\n> + * \\fn Module::createAlgorithms()\n> + * \\brief Create algorithms from YAML configuration data\n> + * \\param[in] context The IPA context\n> + * \\param[in] algorithms Algorithms configuration data as a parsed YamlObject\n> + *\n> + * This function iterates over the list of \\a algorithms parsed from the YAML\n> + * configuration file, and instantiates and initializes the corresponding\n> + * algorithms. The configuration data is expected to be correct, any error\n> + * causes the function to fail and return immediately.\n> + *\n> + * \\return 0 on success, or a negative error code on failure\n> + */\n> +\n> +/**\n> + * \\fn Module::registerAlgorithm()\n> + * \\brief Add an algorithm factory class to the list of available algorithms\n> + * \\param[in] factory Factory to use to construct the algorithm\n> + *\n> + * This function registers an algorithm factory. It is meant to be called by the\n> + * AlgorithmFactory constructor only.\n> + */\n> +\n> +/**\n> + * \\fn Module::createAlgorithm(const std::string &name)\n> * \\brief Create an instance of an Algorithm by name\n> * \\param[in] name The algorithm name\n> *\n> @@ -90,15 +121,6 @@ namespace ipa {\n> * \\return A new instance of the Algorithm subclass corresponding to the \\a name\n> */\n> \n> -/**\n> - * \\fn Module::registerAlgorithm()\n> - * \\brief Add an algorithm factory class to the list of available algorithms\n> - * \\param[in] factory Factory to use to construct the algorithm\n> - *\n> - * This function registers an algorithm factory. It is meant to be called by the\n> - * AlgorithmFactory constructor only.\n> - */\n> -\n> } /* namespace ipa */\n> \n> } /* namespace libcamera */\n> diff --git a/src/ipa/libipa/module.h b/src/ipa/libipa/module.h\n> index f30fc33711bb..00d5785e1aa0 100644\n> --- a/src/ipa/libipa/module.h\n> +++ b/src/ipa/libipa/module.h\n> @@ -7,14 +7,22 @@\n> \n> #pragma once\n> \n> +#include \n> #include \n> #include \n> #include \n> \n> +#include \n> +#include \n> +\n> +#include \"libcamera/internal/yaml_parser.h\"\n> +\n> #include \"algorithm.h\"\n> \n> namespace libcamera {\n> \n> +LOG_DECLARE_CATEGORY(IPAModuleAlgo)\n> +\n> namespace ipa {\n> \n> template @@ -30,6 +38,63 @@ public:\n> \n> \tvirtual ~Module() {}\n> \n> +\tconst std::list>> &algorithms() const\n> +\t{\n> +\t\treturn algorithms_;\n> +\t}\n> +\n> +\tint createAlgorithms(Context &context, const YamlObject &algorithms)\n> +\t{\n> +\t\tconst auto &list = algorithms.asList();\n> +\n> +\t\tfor (const auto &[i, algo] : utils::enumerate(list)) {\n> +\t\t\tif (!algo.isDictionary()) {\n> +\t\t\t\tLOG(IPAModuleAlgo, Error)\n> +\t\t\t\t\t<< \"Invalid YAML syntax for algorithm \" << i;\n> +\t\t\t\talgorithms_.clear();\n> +\t\t\t\treturn -EINVAL;\n> +\t\t\t}\n> +\n> +\t\t\tint ret = createAlgorithm(context, algo);\n> +\t\t\tif (ret) {\n> +\t\t\t\talgorithms_.clear();\n> +\t\t\t\treturn ret;\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tstatic void registerAlgorithm(AlgorithmFactory *factory)\n> +\t{\n> +\t\tfactories().push_back(factory);\n> +\t}\n> +\n> +private:\n> +\tint createAlgorithm(Context &context, const YamlObject &data)\n> +\t{\n> +\t\tconst auto &[name, algoData] = *data.asDict().begin();\n> +\t\tstd::unique_ptr> algo = createAlgorithm(name);\n> +\t\tif (!algo) {\n> +\t\t\tLOG(IPAModuleAlgo, Error)\n> +\t\t\t\t<< \"Algorithm '\" << name << \"' not found\";\n> +\t\t\treturn -EINVAL;\n> +\t\t}\n> +\n> +\t\tint ret = algo->init(context, algoData);\n> +\t\tif (ret) {\n> +\t\t\tLOG(IPAModuleAlgo, Error)\n> +\t\t\t\t<< \"Algorithm '\" << name << \"' failed to initialize\";\n> +\t\t\treturn ret;\n> +\t\t}\n> +\n> +\t\tLOG(IPAModuleAlgo, Debug)\n> +\t\t\t<< \"Instantiated algorithm '\" << name << \"'\";\n> +\n> +\t\talgorithms_.push_back(std::move(algo));\n> +\t\treturn 0;\n> +\t}\n> +\n> \tstatic std::unique_ptr> createAlgorithm(const std::string &name)\n> \t{\n> \t\tfor (const AlgorithmFactory *factory : factories()) {\n> @@ -40,12 +105,6 @@ public:\n> \t\treturn nullptr;\n> \t}\n> \n> -\tstatic void registerAlgorithm(AlgorithmFactory *factory)\n> -\t{\n> -\t\tfactories().push_back(factory);\n> -\t}\n> -\n> -private:\n> \tstatic std::vector *> &factories()\n> \t{\n> \t\t/*\n> @@ -56,6 +115,8 @@ private:\n> \t\tstatic std::vector *> factories;\n> \t\treturn factories;\n> \t}\n> +\n> +\tstd::list>> algorithms_;\n> };\n> \n> } /* namespace ipa */","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 61018BE173\n\tfor ;\n\tThu, 23 Jun 2022 12:29:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 70E7365635;\n\tThu, 23 Jun 2022 14:29:17 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 621EF6048F\n\tfor ;\n\tThu, 23 Jun 2022 14:29:15 +0200 (CEST)","from pyrite.rasen.tech (softbank114048060123.bbtec.net\n\t[114.48.60.123])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 75C809DA;\n\tThu, 23 Jun 2022 14:29:13 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655987357;\n\tbh=R3LbAfSxbK4h0GZandxr1/VeqfSveJjiBcXnbtLPjpk=;\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=ZZLEiIPrUJpuTuITr9kgJtP6IK0Dihxq1Z+fmXJFBsbPx4d2beaZYa5oSNQJd32fE\n\thTK7/IweU0Sj2Ik9dBQy0GZ64YBe84DrfEw8z+c5emCK0HeQLOSEuD7TWQI/8g7v8U\n\t65b8PlQ3kr5wcaU7fYVhkDEc7hFlzaAwjY26hnLtlH2vai4bQF8splXt13+SPvdAeV\n\t0b1eIOKy/n0Uk8oDY7r0q3CS+xZ0uiunlqRfkXc7q8yQvCemHQCbqeggRaVTGQYsfs\n\t+sfvNQTWVXCME8RmMa6myHwtRAzeJ0hlgHxML+CpjxPPCHUSeQTvjcnNdfKwi90IR8\n\tFhrslNGor3h7g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1655987354;\n\tbh=R3LbAfSxbK4h0GZandxr1/VeqfSveJjiBcXnbtLPjpk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RsrvHIQtFkRiW6zjozMFAB8Bs9N4Rgaq7YbncH5xQjgQ7HTxTHN5qj6p486wxilaG\n\thv/wWAnjUSxZzpmGe/tpgnJ9MIZrCBmv1jG0kjQhLFeZzX2ZNrfi8gIVutn03Tpapm\n\tvBUv31rsOiwmDlraZ6C0RG30iNY9cUzmYxF0jcz8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"RsrvHIQt\"; dkim-atps=neutral","Date":"Thu, 23 Jun 2022 21:29:07 +0900","To":"Laurent Pinchart ","Message-ID":"<20220623122907.GH3309559@pyrite.rasen.tech>","References":"<20220620014305.26778-1-laurent.pinchart@ideasonboard.com>\n\t<20220620014305.26778-6-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20220620014305.26778-6-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 05/12] ipa: libipa: module: Add\n\tsupport for instantiation from YAML","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\" "}}]