[{"id":36119,"web_url":"https://patchwork.libcamera.org/comment/36119/","msgid":"","date":"2025-10-03T16:20:04","subject":"Re: [PATCH v2 1/2] libcamera: ipa_manager: Create IPA by name","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 10. 03. 18:16 keltezéssel, Jacopo Mondi írta:\n> From: Hans de Goede \n> \n> Currently createIPA() / IPAManager::module() assume that there is a 1:1\n> relationship between pipeline handlers and IPAs and IPA matching is done\n> based on matching the pipe to ipaModuleInfo.pipelineName[].\n> \n> One way to allow using a single IPA with multiple pipelines would be to\n> allow the IPA to declare itself compatible with more than one pipeline,\n> turning ipaModuleInfo.pipelineName[] into e.g. a vector. But the way\n> ipaModuleInfo is loaded as an ELF symbol requires it to be a simple flat\n> C-struct.\n> \n> Instead, move the IPA creation procedure to be name-based, introducing\n> an overload to IPAManager::createIPA(pipe, name, minVer, maxVer) that\n> allows to specify the name of the IPA module to match. Pipeline handlers\n> that wants to use their name as matching criteria can continue doing so\n> using the already existing createIPA(pipe, minVer, maxVer) overload.\n> \n> Signed-off-by: Hans de Goede \n> Signed-off-by: Jacopo Mondi \n> Reviewed-by: Hans de Goede \n> Reviewed-by: Kieran Bingham \n> Tested-by: Niklas Söderlund \n> ---\n\nReviewed-by: Barnabás Pőcze \n\n\n> include/libcamera/internal/ipa_manager.h | 13 ++++++++++--\n> include/libcamera/internal/ipa_module.h | 4 ++--\n> src/libcamera/ipa_manager.cpp | 34 ++++++++++++++++++++++++++------\n> src/libcamera/ipa_module.cpp | 12 +++++------\n> 4 files changed, 47 insertions(+), 16 deletions(-)\n> \n> diff --git a/include/libcamera/internal/ipa_manager.h b/include/libcamera/internal/ipa_manager.h\n> index f8ce780169e617088557888d7c8dae2d3f10ec08..4c01e76f1800120f0baca25bc2e5e251f7cf80b5 100644\n> --- a/include/libcamera/internal/ipa_manager.h\n> +++ b/include/libcamera/internal/ipa_manager.h\n> @@ -34,12 +34,13 @@ public:\n> \n> \ttemplate\n> \tstatic std::unique_ptr createIPA(PipelineHandler *pipe,\n> +\t\t\t\t\t const char *name,\n> \t\t\t\t\t uint32_t minVersion,\n> \t\t\t\t\t uint32_t maxVersion)\n> \t{\n> \t\tCameraManager *cm = pipe->cameraManager();\n> \t\tIPAManager *self = cm->_d()->ipaManager();\n> -\t\tIPAModule *m = self->module(pipe, minVersion, maxVersion);\n> +\t\tIPAModule *m = self->module(name, minVersion, maxVersion);\n> \t\tif (!m)\n> \t\t\treturn nullptr;\n> \n> @@ -60,6 +61,14 @@ public:\n> \t\treturn proxy;\n> \t}\n> \n> +\ttemplate\n> +\tstatic std::unique_ptr createIPA(PipelineHandler *pipe,\n> +\t\t\t\t\t uint32_t minVersion,\n> +\t\t\t\t\t uint32_t maxVersion)\n> +\t{\n> +\t\treturn createIPA(pipe, pipe->name(), minVersion, maxVersion);\n> +\t}\n> +\n> #if HAVE_IPA_PUBKEY\n> \tstatic const PubKey &pubKey()\n> \t{\n> @@ -72,7 +81,7 @@ private:\n> \t\t std::vector &files);\n> \tunsigned int addDir(const char *libDir, unsigned int maxDepth = 0);\n> \n> -\tIPAModule *module(PipelineHandler *pipe, uint32_t minVersion,\n> +\tIPAModule *module(const char *name, uint32_t minVersion,\n> \t\t\t uint32_t maxVersion);\n> \n> \tbool isSignatureValid(IPAModule *ipa) const;\n> diff --git a/include/libcamera/internal/ipa_module.h b/include/libcamera/internal/ipa_module.h\n> index 15f19492c3a027a0bc4f9572188d13af41fcd450..a0a53764e1394abed3bab92cdde9f33a86441c5f 100644\n> --- a/include/libcamera/internal/ipa_module.h\n> +++ b/include/libcamera/internal/ipa_module.h\n> @@ -36,8 +36,8 @@ public:\n> \n> \tIPAInterface *createInterface();\n> \n> -\tbool match(PipelineHandler *pipe,\n> -\t\t uint32_t minVersion, uint32_t maxVersion) const;\n> +\tbool match(const char *name, uint32_t minVersion,\n> +\t\t uint32_t maxVersion) const;\n> \n> protected:\n> \tstd::string logPrefix() const override;\n> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> index 35171d097136a6d85b8f518c099f8228f9eacd6f..f62a4ee5fd012d23ce178d59f84c6ab49513376b 100644\n> --- a/src/libcamera/ipa_manager.cpp\n> +++ b/src/libcamera/ipa_manager.cpp\n> @@ -247,15 +247,15 @@ unsigned int IPAManager::addDir(const char *libDir, unsigned int maxDepth)\n> \n> /**\n> * \\brief Retrieve an IPA module that matches a given pipeline handler\n> - * \\param[in] pipe The pipeline handler\n> + * \\param[in] name The IPA module string identifier\n> * \\param[in] minVersion Minimum acceptable version of IPA module\n> * \\param[in] maxVersion Maximum acceptable version of IPA module\n> */\n> -IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion,\n> +IPAModule *IPAManager::module(const char *name, uint32_t minVersion,\n> \t\t\t uint32_t maxVersion)\n> {\n> \tfor (const auto &module : modules_) {\n> -\t\tif (module->match(pipe, minVersion, maxVersion))\n> +\t\tif (module->match(name, minVersion, maxVersion))\n> \t\t\treturn module.get();\n> \t}\n> \n> @@ -263,12 +263,34 @@ IPAModule *IPAManager::module(PipelineHandler *pipe, uint32_t minVersion,\n> }\n> \n> /**\n> - * \\fn IPAManager::createIPA()\n> - * \\brief Create an IPA proxy that matches a given pipeline handler\n> - * \\param[in] pipe The pipeline handler that wants a matching IPA proxy\n> + * \\fn IPAManager::createIPA(PipelineHandler *pipe, const char *ipaName, uint32_t minVersion, uint32_t maxVersion)\n> + * \\brief Create an IPA proxy that matches the requested name and version\n> + * \\param[in] pipe The pipeline handler that wants to create the IPA module\n> + * \\param[in] ipaName The IPA module name\n> * \\param[in] minVersion Minimum acceptable version of IPA module\n> * \\param[in] maxVersion Maximum acceptable version of IPA module\n> *\n> + * Create an IPA module using \\a name as the matching identifier. This overload\n> + * allows pipeline handlers to create an IPA module by specifying its name\n> + * instead of relying on the fact that the IPA module matches the pipeline\n> + * handler's one.\n> + *\n> + * \\return A newly created IPA proxy, or nullptr if no matching IPA module is\n> + * found or if the IPA proxy fails to initialize\n> + */\n> +\n> +/**\n> + * \\fn IPAManager::createIPA(PipelineHandler *pipe, uint32_t minVersion, uint32_t maxVersion)\n> + * \\brief Create an IPA proxy that matches the pipeline handler name and the\n> + * requested version\n> + * \\param[in] pipe The pipeline handler that wants to create the IPA module\n> + * \\param[in] minVersion Minimum acceptable version of IPA module\n> + * \\param[in] maxVersion Maximum acceptable version of IPA module\n> + *\n> + * Create an IPA module using the pipeline handler name as the matching\n> + * identifier. This overload allows pipeline handler to create an IPA module\n> + * whose name matches the pipeline handler one.\n> + *\n> * \\return A newly created IPA proxy, or nullptr if no matching IPA module is\n> * found or if the IPA proxy fails to initialize\n> */\n> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp\n> index e6ea61e4482983a83b4185c4309f2c514ed24fc2..0bd6f14626fe2038072f48b70ca4341b0eb8cef5 100644\n> --- a/src/libcamera/ipa_module.cpp\n> +++ b/src/libcamera/ipa_module.cpp\n> @@ -463,21 +463,21 @@ IPAInterface *IPAModule::createInterface()\n> \n> /**\n> * \\brief Verify if the IPA module matches a given pipeline handler\n> - * \\param[in] pipe Pipeline handler to match with\n> + * \\param[in] name The IPA module name\n> * \\param[in] minVersion Minimum acceptable version of IPA module\n> * \\param[in] maxVersion Maximum acceptable version of IPA module\n> *\n> - * This function checks if this IPA module matches the \\a pipe pipeline handler,\n> + * This function checks if this IPA module matches the requested \\a name\n> * and the input version range.\n> *\n> - * \\return True if the pipeline handler matches the IPA module, or false otherwise\n> + * \\return True if the IPA module matches, or false otherwise\n> */\n> -bool IPAModule::match(PipelineHandler *pipe,\n> -\t\t uint32_t minVersion, uint32_t maxVersion) const\n> +bool IPAModule::match(const char *name, uint32_t minVersion,\n> +\t\t uint32_t maxVersion) const\n> {\n> \treturn info_.pipelineVersion >= minVersion &&\n> \t info_.pipelineVersion <= maxVersion &&\n> -\t !strcmp(info_.pipelineName, pipe->name());\n> +\t !strcmp(info_.name, name);\n> }\n> \n> std::string IPAModule::logPrefix() const\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 35DBAC324C\n\tfor ;\n\tFri, 3 Oct 2025 16:20:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D699C6B5AA;\n\tFri, 3 Oct 2025 18:20:10 +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 9BC2469318\n\tfor ;\n\tFri, 3 Oct 2025 18:20:08 +0200 (CEST)","from [192.168.33.17] (185.182.214.142.nat.pool.zt.hu\n\t[185.182.214.142])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D2BB0192C;\n\tFri, 3 Oct 2025 18:18:37 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"MnRdPnXl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1759508318;\n\tbh=1XOiu79x3PSZe4DVk0EYnKbAMHiju+KzsSHMhYaFHvw=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=MnRdPnXlf3xa/7N/GMIlBRAf5QHrNpMVTUu1QX2MMq0g7U/ul6thdqo87IsOljj/7\n\totR2yHwIyf1/3MBrvOwJApt33/HQixd7HBtWbOjlO/l9odMXdDxjPEtNkkhgSu+6Jx\n\tMcqhz+mcshouVh/1bBdhNR2xnGvNf52pjKYnE23w=","Message-ID":"","Date":"Fri, 3 Oct 2025 18:20:04 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 1/2] libcamera: ipa_manager: Create IPA by name","To":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org, =?utf-8?q?Niklas_S=C3=B6derlund?=\n\t, Hans de Goede ","Cc":"Kieran Bingham ","References":"<20251003-ipa-match-by-name-v2-0-54f3cd828ddd@ideasonboard.com>\n\t<20251003-ipa-match-by-name-v2-1-54f3cd828ddd@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251003-ipa-match-by-name-v2-1-54f3cd828ddd@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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\" "}}]