[{"id":1770,"web_url":"https://patchwork.libcamera.org/comment/1770/","msgid":"<20190605133246.GB4784@pendragon.ideasonboard.com>","date":"2019-06-05T13:32:46","subject":"Re: [libcamera-devel] [PATCH v3 03/10] libcamera: ipa_module_info:\n\tupdate struct to allow IPA matching","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the match.\n\nOn Tue, Jun 04, 2019 at 08:53:09PM -0400, Paul Elder wrote:\n> We need a way to match pipelines with IPA modules, so add fields in\n> IPAModuleInfo to hold the IPA module API version number, the pipeline\n> name, and the pipeline version.\n> \n> The module API version is used to determine the layout of struct\n> IPAModuleInfo.\n> \n> Also update IPA module tests and Doxygen accordingly. Doxygen needs to\n> be updated to accomodate __attribute__((packed)).\n> \n> Signed-off-by: Paul Elder \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> Changes in v3:\n> - remove PIPELINE_VERSION documentation (since the macro was removed,\n> since pipeline versions are now a single number)\n> \n> Changes in v2:\n> - combine pipeline major and minor versions into one using the\n> Documentation/Doxyfile.in | 7 +++---\n> \n> include/libcamera/ipa/ipa_module_info.h | 10 ++++++--\n> src/libcamera/ipa_module.cpp | 32 ++++++++++++++++++++-----\n> test/ipa/ipa_test.cpp | 27 +++++++++++----------\n> test/ipa/shared_test.cpp | 4 +++-\n> 5 files changed, 55 insertions(+), 25 deletions(-)\n> \n> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in\n> index 67eaded..ac70efb 100644\n> --- a/Documentation/Doxyfile.in\n> +++ b/Documentation/Doxyfile.in\n> @@ -2016,7 +2016,7 @@ ENABLE_PREPROCESSING = YES\n> # The default value is: NO.\n> # This tag requires that the tag ENABLE_PREPROCESSING is set to YES.\n> \n> -MACRO_EXPANSION = NO\n> +MACRO_EXPANSION = YES\n> \n> # If the EXPAND_ONLY_PREDEF and MACRO_EXPANSION tags are both set to YES then\n> # the macro expansion is limited to the macros specified with the PREDEFINED and\n> @@ -2024,7 +2024,7 @@ MACRO_EXPANSION = NO\n> # The default value is: NO.\n> # This tag requires that the tag ENABLE_PREPROCESSING is set to YES.\n> \n> -EXPAND_ONLY_PREDEF = NO\n> +EXPAND_ONLY_PREDEF = YES\n> \n> # If the SEARCH_INCLUDES tag is set to YES, the include files in the\n> # INCLUDE_PATH will be searched if a #include is found.\n> @@ -2057,7 +2057,8 @@ INCLUDE_FILE_PATTERNS = *.h\n> # This tag requires that the tag ENABLE_PREPROCESSING is set to YES.\n> \n> PREDEFINED = __DOXYGEN__ \\\n> - __cplusplus\n> + __cplusplus \\\n> + __attribute__(x)=\n> \n> # If the MACRO_EXPANSION and EXPAND_ONLY_PREDEF tags are set to YES then this\n> # tag can be used to specify a list of macro names that should be expanded. The\n> diff --git a/include/libcamera/ipa/ipa_module_info.h b/include/libcamera/ipa/ipa_module_info.h\n> index 4e0d668..803b5d3 100644\n> --- a/include/libcamera/ipa/ipa_module_info.h\n> +++ b/include/libcamera/ipa/ipa_module_info.h\n> @@ -7,14 +7,20 @@\n> #ifndef __LIBCAMERA_IPA_MODULE_INFO_H__\n> #define __LIBCAMERA_IPA_MODULE_INFO_H__\n> \n> +#include \n> +\n> +#define IPA_MODULE_API_VERSION 1\n> +\n> #ifdef __cplusplus\n> namespace libcamera {\n> #endif\n> \n> struct IPAModuleInfo {\n> +\tint moduleAPIVersion;\n> +\tuint32_t pipelineVersion;\n> +\tchar pipelineName[256];\n> \tchar name[256];\n> -\tunsigned int version;\n> -};\n> +} __attribute__((packed));\n> \n> #ifdef __cplusplus\n> extern \"C\" {\n> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp\n> index 86cbe71..f79a44e 100644\n> --- a/src/libcamera/ipa_module.cpp\n> +++ b/src/libcamera/ipa_module.cpp\n> @@ -169,6 +169,17 @@ int elfLoadSymbol(void *dst, size_t size, void *map, size_t soSize,\n> \n> } /* namespace */\n> \n> +/**\n> + * \\def IPA_MODULE_API_VERSION\n> + * \\brief The IPA module API version\n> + *\n> + * This version number specifies the version for the layout of\n> + * struct IPAModuleInfo. The IPA module shall use this macro to\n> + * set its moduleAPIVersion field.\n> + *\n> + * \\sa IPAModuleInfo::moduleAPIVersion\n> + */\n> +\n> /**\n> * \\struct IPAModuleInfo\n> * \\brief Information of an IPA module\n> @@ -176,14 +187,23 @@ int elfLoadSymbol(void *dst, size_t size, void *map, size_t soSize,\n> * This structure contains the information of an IPA module. It is loaded,\n> * read, and validated before anything else is loaded from the shared object.\n> *\n> - * \\var IPAModuleInfo::name\n> - * \\brief The name of the IPA module\n> + * \\var IPAModuleInfo::moduleAPIVersion\n> + * \\brief The IPA module API version that the IPA module implements\n> + *\n> + * This version number specifies the version for the layout of\n> + * struct IPAModuleInfo. The IPA module shall report here the version that\n> + * it was built for, using the macro IPA_MODULE_API_VERSION.\n> + *\n> + * \\var IPAModuleInfo::pipelineVersion\n> + * \\brief The pipeline handler version that the IPA module is for\n> *\n> - * \\var IPAModuleInfo::version\n> - * \\brief The version of the IPA module\n> + * \\var IPAModuleInfo::pipelineName\n> + * \\brief The name of the pipeline handler that the IPA module is for\n> *\n> - * \\todo abi compatability version\n> - * \\todo pipeline compatability matcher\n> + * This name is used to match a pipeline handler with the module.\n> + *\n> + * \\var IPAModuleInfo::name\n> + * \\brief The name of the IPA module\n> */\n> \n> /**\n> diff --git a/test/ipa/ipa_test.cpp b/test/ipa/ipa_test.cpp\n> index 2dbc702..63e4243 100644\n> --- a/test/ipa/ipa_test.cpp\n> +++ b/test/ipa/ipa_test.cpp\n> @@ -33,18 +33,17 @@ protected:\n> \n> \t\tconst struct IPAModuleInfo &info = ll->info();\n> \n> -\t\tif (strcmp(info.name, testInfo.name)) {\n> -\t\t\tcerr << \"test IPA module has incorrect name\" << endl;\n> -\t\t\tcerr << \"expected \\\"\" << testInfo.name << \"\\\", got \\\"\"\n> -\t\t\t << info.name << \"\\\"\" << endl;\n> -\t\t\tret = -1;\n> -\t\t}\n> -\n> -\t\tif (info.version != testInfo.version) {\n> -\t\t\tcerr << \"test IPA module has incorrect version\" << endl;\n> -\t\t\tcerr << \"expected \\\"\" << testInfo.version << \"\\\", got \\\"\"\n> -\t\t\t << info.version << \"\\\"\" << endl;\n> -\t\t\tret = -1;\n> +\t\tif (memcmp(&info, &testInfo, sizeof(info))) {\n> +\t\t\tcerr << \"IPA module information mismatch: expected:\" << endl\n> +\t\t\t << \"moduleAPIVersion = \" << testInfo.moduleAPIVersion << endl\n> +\t\t\t << \"pipelineVersion = \" << testInfo.pipelineVersion << endl\n> +\t\t\t << \"pipelineName = \" << testInfo.pipelineName << endl\n> +\t\t\t << \"name = \" << testInfo.name\n> +\t\t\t << \"got: \" << endl\n> +\t\t\t << \"moduleAPIVersion = \" << info.moduleAPIVersion << endl\n> +\t\t\t << \"pipelineVersion = \" << info.pipelineVersion << endl\n> +\t\t\t << \"pipelineName = \" << info.pipelineName << endl\n> +\t\t\t << \"name = \" << info.name << endl;\n> \t\t}\n> \n> \t\tdelete ll;\n> @@ -56,8 +55,10 @@ protected:\n> \t\tint count = 0;\n> \n> \t\tconst struct IPAModuleInfo testInfo = {\n> -\t\t\t\"It's over nine thousand!\",\n> +\t\t\tIPA_MODULE_API_VERSION,\n> \t\t\t9001,\n> +\t\t\t\"bleep\",\n> +\t\t\t\"It's over nine thousand!\",\n> \t\t};\n> \n> \t\tcount += runTest(\"test/ipa/ipa-dummy-cpp.so\", testInfo);\n> diff --git a/test/ipa/shared_test.cpp b/test/ipa/shared_test.cpp\n> index 4e5c976..8bac439 100644\n> --- a/test/ipa/shared_test.cpp\n> +++ b/test/ipa/shared_test.cpp\n> @@ -4,8 +4,10 @@ namespace libcamera {\n> \n> extern \"C\" {\n> const struct libcamera::IPAModuleInfo ipaModuleInfo = {\n> -\t\"It's over nine thousand!\",\n> +\tIPA_MODULE_API_VERSION,\n> \t9001,\n> +\t\"bleep\",\n> +\t\"It's over nine thousand!\",\n> };\n> };\n>","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 02FCA65F62\n\tfor ;\n\tWed, 5 Jun 2019 15:33:06 +0200 (CEST)","from pendragon.ideasonboard.com (85-76-19-33-nat.elisa-mobile.fi\n\t[85.76.19.33])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 411F584;\n\tWed, 5 Jun 2019 15:33:04 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1559741585;\n\tbh=4otb2FaTGvXx4MKGYkkCtHzTLT7fGKW1g1eMb82e0GM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ffEYI+dHbdcZ0oqUE7oN5+cxh/JtBoJHi4HMzLG+NfpImgxp5cE+12oiShmMlcofQ\n\tSujdaC1zdJpn4sbGtC6lmJdciJuWsR+y2TNunrwPyQJBk3vS3g4QgX2MzEpX5w5/ls\n\tzuTeaB6hNWHQaZXf+cQp1Y+k1hOAf1CENljvcFPQ=","Date":"Wed, 5 Jun 2019 16:32:46 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190605133246.GB4784@pendragon.ideasonboard.com>","References":"<20190605005316.4835-1-paul.elder@ideasonboard.com>\n\t<20190605005316.4835-4-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190605005316.4835-4-paul.elder@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 03/10] libcamera: ipa_module_info:\n\tupdate struct to allow IPA matching","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Wed, 05 Jun 2019 13:33:06 -0000"}}]