[{"id":1716,"web_url":"https://patchwork.libcamera.org/comment/1716/","msgid":"<20190528152531.GF14336@pendragon.ideasonboard.com>","date":"2019-05-28T15:25:31","subject":"Re: [libcamera-devel] [PATCH 3/8] 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 patch.\n\nOn Mon, May 27, 2019 at 06:35:35PM -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 API version number, the pipeline name, and\n> the pipeline major and minor version.\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 <paul.elder@ideasonboard.com>\n> ---\n>  Documentation/Doxyfile.in               |  7 ++--\n>  include/libcamera/ipa/ipa_module_info.h | 11 ++++--\n>  src/libcamera/ipa_module.cpp            | 48 +++++++++++++++++++++----\n>  test/ipa/ipa_test.cpp                   | 45 ++++++++++++++++++-----\n>  test/ipa/shared_test.cpp                |  4 ++-\n>  5 files changed, 94 insertions(+), 21 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..84492ed 100644\n> --- a/include/libcamera/ipa/ipa_module_info.h\n> +++ b/include/libcamera/ipa/ipa_module_info.h\n> @@ -7,14 +7,21 @@\n>  #ifndef __LIBCAMERA_IPA_MODULE_INFO_H__\n>  #define __LIBCAMERA_IPA_MODULE_INFO_H__\n>  \n> +#include <stdint.h>\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> +\tint pipelineVersionMajor;\n> +\tint pipelineVersionMinor;\n\nPlease use integer types from stdint.h. These should be uint32_t.\n\nI think you can combine the minor and major in a single version field,\nwith a macro to create it from major and minor numbers.\n\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..6e68934 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 API version\n> + *\n> + * This version number specifies the version for the layout of\n> + * struct IPAModuleInfo, and the API between pipeline handlers and the IPA.\n\nThe module API version is just the former, not the latter, which are\nreported through the pipelineVersion.\n\n> + * The IPA module should use this macro to define its moduleAPIVersion field.\n\ns/should/shall/\ns/define/set/\n\n> + *\n> + * \\sa libcamera::IPAModuleInfo::moduleAPIVersion\n> + */\n> +\n>  /**\n>   * \\struct IPAModuleInfo\n>   * \\brief Information of an IPA module\n> @@ -176,14 +187,39 @@ 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 API version that the IPA module was made with\n\ns/was made with/implements/ ?\n\n> + *\n> + * This version number specifies the version for the layout of\n> + * struct IPAModuleInfo, and the API between pipeline handlers and the IPA.\n\ns/, .*/./\n\n> + * The IPA module shall report here the version that it was built for,\n> + * using the macro IPA_MODULE_API_VERSION.\n>   *\n> - * \\var IPAModuleInfo::version\n> - * \\brief The version of the IPA module\n> + * \\sa IPA_MODULE_API_VERSION\n>   *\n> - * \\todo abi compatability version\n> - * \\todo pipeline compatability matcher\n> + * \\var IPAModuleInfo::pipelineVersionMajor\n> + * \\brief The pipeline handler version (major) that the IPA module is for\n> + * \\todo actually match the pipeline handler against this\n> + *\n> + * The major version of the module and pipeline handler must be equal in\n> + * order for the pipeline handler and the module to be matched.\n> + *\n> + * \\var IPAModuleInfo::pipelineVersionMinor\n> + * \\brief The pipeline handler version (minor) that the IPA module is for\n> + *\n> + * The minor version of the module must be equal to or greater than\n> + * the pipeline handler minor version in order for the pipeline handler\n> + * and the module to be matched.\n> + *\n> + * \\var IPAModuleInfo::pipelineName\n> + * \\brief The name of the pipeline handler that the IPA module is for\n> + *\n> + * This name is used to match a pipeline handler with the module.\n> + * The name of the pipeline handler is declared in the pipeline handler\n> + * with the macro REGISTER_PIPELINE_HANDLER.\n\nI would drop the second sentence, as REGISTER_PIPELINE_HANDLER is an\ninternal API, and should thus not be referenced from external API files.\n\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..f50880e 100644\n> --- a/test/ipa/ipa_test.cpp\n> +++ b/test/ipa/ipa_test.cpp\n> @@ -33,20 +33,45 @@ 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\tstringstream errString;\n> +\n> +\t\tif (info.moduleAPIVersion != testInfo.moduleAPIVersion) {\n> +\t\t\terrString << \"incorrect name: expected \\\"\"\n\n\"name\" ? :-)\n\nYou can simplify all this with\n\n\t\tif (info.moduleAPIVersion != testInfo.moduleAPIVersion ||\n\t\t    info.version != testInfo.version ||\n\t\t    ...) {\n\t\t    \tcerr << \"IPA module information mismatch: expected \"\n\t\t\t     << \"moduleAPIVersion=\" << testInfo.moduleApiVersion\n\t\t\t     << ...\n\t\t\t     << \", got \"\n\t\t\t     << ...\n\t\t}\n\nWhat do you think ?\n\n\n> +\t\t\t\t  << testInfo.moduleAPIVersion << \"\\\", got \\\"\"\n> +\t\t\t\t  << info.moduleAPIVersion << \"\\\"\" << 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\tif (info.pipelineVersionMajor != testInfo.pipelineVersionMajor) {\n> +\t\t\terrString << \"incorrect name: expected \\\"\"\n> +\t\t\t\t  << testInfo.pipelineVersionMajor << \"\\\", got \\\"\"\n> +\t\t\t\t  << info.pipelineVersionMajor << \"\\\"\" << endl;\n>  \t\t\tret = -1;\n>  \t\t}\n>  \n> +\t\tif (info.pipelineVersionMinor != testInfo.pipelineVersionMinor) {\n> +\t\t\terrString << \"incorrect name: expected \\\"\"\n> +\t\t\t\t  << testInfo.pipelineVersionMinor << \"\\\", got \\\"\"\n> +\t\t\t\t  << info.pipelineVersionMinor << \"\\\"\" << endl;\n> +\t\t\tret = -1;\n> +\t\t}\n> +\n> +\t\tif (strcmp(info.pipelineName, testInfo.pipelineName)) {\n> +\t\t\terrString << \"incorrect name: expected \\\"\"\n> +\t\t\t\t  << testInfo.pipelineName << \"\\\", got \\\"\"\n> +\t\t\t\t  << info.pipelineName << \"\\\"\" << endl;\n> +\t\t\tret = -1;\n> +\t\t}\n> +\n> +\t\tif (strcmp(info.name, testInfo.name)) {\n> +\t\t\terrString << \"incorrect name: expected \\\"\"\n> +\t\t\t\t  << testInfo.name << \"\\\", got \\\"\"\n> +\t\t\t\t  << info.name << \"\\\"\" << endl;\n> +\t\t\tret = -1;\n> +\t\t}\n> +\n> +\t\tcerr << errString.str();\n> +\n>  \t\tdelete ll;\n>  \t\treturn ret;\n>  \t}\n> @@ -56,8 +81,10 @@ protected:\n>  \t\tint count = 0;\n>  \n>  \t\tconst struct IPAModuleInfo testInfo = {\n> +\t\t\t1,\n\nShould you use IPA_MODULE_API_VERSION as recommended by the\ndocumentation ? Same for the shared_test.cpp file below.\n\n> +\t\t\t0, 9001,\n> +\t\t\t\"bleep\",\n>  \t\t\t\"It's over nine thousand!\",\n> -\t\t\t9001,\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..d932a9b 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> +\t1,\n> +\t0, 9001,\n\nI would go for one field per line. It would be really great if we could\nuse named initialisers :-S\n\n> +\t\"bleep\",\n>  \t\"It's over nine thousand!\",\n\nLet's give those fields more plausible values :-)\n\n> -\t9001,\n>  };\n>  };\n>","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["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 7D553600EA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 28 May 2019 17:25:50 +0200 (CEST)","from pendragon.ideasonboard.com (85-76-96-53-nat.elisa-mobile.fi\n\t[85.76.96.53])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1A8A1D85;\n\tTue, 28 May 2019 17:25:48 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1559057150;\n\tbh=G3A86B68vgCjc35pu7GvYqDVijX1ICSiMbkuGhre/VE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=SCZZ9WHgsjSbTZBblJFezIrS8GEQ5Tf+pp+FEpDC1upVQddzrFGhTkkp2y8Xs1Mk4\n\thPKY3SKcPxXT4PzTnJ+5N7SfFvSqd6DfN8rWxq2ZuhosUaT913+6UmmQTtSUyCNmFf\n\txus2Hvd5kpq1mvWoETzs1yKaK+j+9/YYrc+NChhQ=","Date":"Tue, 28 May 2019 18:25:31 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190528152531.GF14336@pendragon.ideasonboard.com>","References":"<20190527223540.21855-1-paul.elder@ideasonboard.com>\n\t<20190527223540.21855-4-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190527223540.21855-4-paul.elder@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 3/8] 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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 28 May 2019 15:25:50 -0000"}},{"id":1745,"web_url":"https://patchwork.libcamera.org/comment/1745/","msgid":"<20190603230848.GF2960@bigcity.dyn.berto.se>","date":"2019-06-03T23:08:48","subject":"Re: [libcamera-devel] [PATCH 3/8] libcamera: ipa_module_info:\n\tupdate struct to allow IPA matching","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Paul,\n\nThanks for your work, and sorry for wall of text.\n\nOn 2019-05-27 18:35:35 -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 API version number, the pipeline name, and\n> the pipeline major and minor version.\n\nI'm not sure I like that the IPA API have a version number, I know now \nyou think I'm crazy but please hear me out ;-)\n\nIf we put API version numbers in both the PipelineHandler's and IPA's \nthat means that libcamera will need some logic to match the API version \nrequested by a pipeline handler to the version number reported by the \nIPA. On the surface this seems sane, but then we enter the fun world of \ncompatibility and 3rd party closed source IPA's.\n\n  - Case 1 - Open source IPA shipped with libcamera\n\n    Here we have no issue as both PipelineHanlder and IPA lives in the \n    same source tree and are build and distributed from the same \n    sources.\n\n    Recording API versions in both PipelineHandler and IPA make sens. In \n    this case the version could be equal to the libcamera version.\n\n  - Case 2 - Closed source IPA shipped as 3rd party component\n\n    If anything have changed in the API between pipeline handlers and \n    IPA's between libcamera version A and B they are no longer \n    compatible. This change can be a pipe <-> IPA specific API change or \n    a change in any of the libcamera common objects which are \n    sent/received between the pipe and IPA.\n\n    For example a new control is added which potentially could change \n    the size of libcamera::Request::Controls (theoretical object).\n\n    Now we the 3rd party vendor have a few options on how to handle \n    this. The ones I can think of on the top of my head is:\n\n    1. Rebuild and distribute a binary IPA for every new released \n       version of libcamera.\n    2. Try to upstream some compatibility conversion to the pipeline \n       handler which uses \"old\" libcamera objects.\n    3. Try to convince libcamera to provide a versioned LTS API between \n       pipeline and IPA (both for pipe<->IPA and objects).\n\nAll of the solutions for closed source IPAs have drawbacks, either from \na IPA provider point of view (1) or for libcamera (2, 3). I totally \nagree that we should not change any libcamera objects involved in the \nIPA without a good reason, but we should not be blocked form solving \nproblems in libcamera due to closed source components.\n\nThere fore I propose we think about a solution where all compatibility \nconversions and decisions are pushed to the IPA implementations. The IPA \nwould be initialized with two version numbers when the so is loaded.\n\n - A libcamera API version which control the version of all common \n   libcamera objects which can be transmitted/received to/from a IPA.\n - A pipeline API version which control the API of which the pipeline \n   expects to interact with the IPA.\n\nThen it's up to the IPA to accept if it can function with these two \nversions, or not. Then if a IPA implementation wish to function whit \nboth a newer and older version of one of these components it can \nimplement it's own compatibility layer. And we can still move forward \nwith libcamera core API and open source IPA in the libcamera tree \nwithout needing a huge verification program for 3rd party IPAs.\n\nOnce more; I'm not advocating an API which is changed lightly as we \nreally should do our most to make IPA designers life easy. But if we \nneed to do so to fix bugs in libcamera we should not be blocked by \nanything out of tree.\n\nI don't feel strongly about this topic but thought it worth bringing up \nwhen we design this. So feel free to ignore me ;-)\n\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 <paul.elder@ideasonboard.com>\n> ---\n>  Documentation/Doxyfile.in               |  7 ++--\n>  include/libcamera/ipa/ipa_module_info.h | 11 ++++--\n>  src/libcamera/ipa_module.cpp            | 48 +++++++++++++++++++++----\n>  test/ipa/ipa_test.cpp                   | 45 ++++++++++++++++++-----\n>  test/ipa/shared_test.cpp                |  4 ++-\n>  5 files changed, 94 insertions(+), 21 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..84492ed 100644\n> --- a/include/libcamera/ipa/ipa_module_info.h\n> +++ b/include/libcamera/ipa/ipa_module_info.h\n> @@ -7,14 +7,21 @@\n>  #ifndef __LIBCAMERA_IPA_MODULE_INFO_H__\n>  #define __LIBCAMERA_IPA_MODULE_INFO_H__\n>  \n> +#include <stdint.h>\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> +\tint pipelineVersionMajor;\n> +\tint pipelineVersionMinor;\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..6e68934 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 API version\n> + *\n> + * This version number specifies the version for the layout of\n> + * struct IPAModuleInfo, and the API between pipeline handlers and the IPA.\n> + * The IPA module should use this macro to define its moduleAPIVersion field.\n> + *\n> + * \\sa libcamera::IPAModuleInfo::moduleAPIVersion\n> + */\n> +\n>  /**\n>   * \\struct IPAModuleInfo\n>   * \\brief Information of an IPA module\n> @@ -176,14 +187,39 @@ 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 API version that the IPA module was made with\n> + *\n> + * This version number specifies the version for the layout of\n> + * struct IPAModuleInfo, and the API between pipeline handlers and the IPA.\n> + * The IPA module shall report here the version that it was built for,\n> + * using the macro IPA_MODULE_API_VERSION.\n>   *\n> - * \\var IPAModuleInfo::version\n> - * \\brief The version of the IPA module\n> + * \\sa IPA_MODULE_API_VERSION\n>   *\n> - * \\todo abi compatability version\n> - * \\todo pipeline compatability matcher\n> + * \\var IPAModuleInfo::pipelineVersionMajor\n> + * \\brief The pipeline handler version (major) that the IPA module is for\n> + * \\todo actually match the pipeline handler against this\n> + *\n> + * The major version of the module and pipeline handler must be equal in\n> + * order for the pipeline handler and the module to be matched.\n> + *\n> + * \\var IPAModuleInfo::pipelineVersionMinor\n> + * \\brief The pipeline handler version (minor) that the IPA module is for\n> + *\n> + * The minor version of the module must be equal to or greater than\n> + * the pipeline handler minor version in order for the pipeline handler\n> + * and the module to be matched.\n> + *\n> + * \\var IPAModuleInfo::pipelineName\n> + * \\brief The name of the pipeline handler that the IPA module is for\n> + *\n> + * This name is used to match a pipeline handler with the module.\n> + * The name of the pipeline handler is declared in the pipeline handler\n> + * with the macro REGISTER_PIPELINE_HANDLER.\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..f50880e 100644\n> --- a/test/ipa/ipa_test.cpp\n> +++ b/test/ipa/ipa_test.cpp\n> @@ -33,20 +33,45 @@ 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\tstringstream errString;\n> +\n> +\t\tif (info.moduleAPIVersion != testInfo.moduleAPIVersion) {\n> +\t\t\terrString << \"incorrect name: expected \\\"\"\n> +\t\t\t\t  << testInfo.moduleAPIVersion << \"\\\", got \\\"\"\n> +\t\t\t\t  << info.moduleAPIVersion << \"\\\"\" << 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\tif (info.pipelineVersionMajor != testInfo.pipelineVersionMajor) {\n> +\t\t\terrString << \"incorrect name: expected \\\"\"\n> +\t\t\t\t  << testInfo.pipelineVersionMajor << \"\\\", got \\\"\"\n> +\t\t\t\t  << info.pipelineVersionMajor << \"\\\"\" << endl;\n>  \t\t\tret = -1;\n>  \t\t}\n>  \n> +\t\tif (info.pipelineVersionMinor != testInfo.pipelineVersionMinor) {\n> +\t\t\terrString << \"incorrect name: expected \\\"\"\n> +\t\t\t\t  << testInfo.pipelineVersionMinor << \"\\\", got \\\"\"\n> +\t\t\t\t  << info.pipelineVersionMinor << \"\\\"\" << endl;\n> +\t\t\tret = -1;\n> +\t\t}\n> +\n> +\t\tif (strcmp(info.pipelineName, testInfo.pipelineName)) {\n> +\t\t\terrString << \"incorrect name: expected \\\"\"\n> +\t\t\t\t  << testInfo.pipelineName << \"\\\", got \\\"\"\n> +\t\t\t\t  << info.pipelineName << \"\\\"\" << endl;\n> +\t\t\tret = -1;\n> +\t\t}\n> +\n> +\t\tif (strcmp(info.name, testInfo.name)) {\n> +\t\t\terrString << \"incorrect name: expected \\\"\"\n> +\t\t\t\t  << testInfo.name << \"\\\", got \\\"\"\n> +\t\t\t\t  << info.name << \"\\\"\" << endl;\n> +\t\t\tret = -1;\n> +\t\t}\n> +\n> +\t\tcerr << errString.str();\n> +\n>  \t\tdelete ll;\n>  \t\treturn ret;\n>  \t}\n> @@ -56,8 +81,10 @@ protected:\n>  \t\tint count = 0;\n>  \n>  \t\tconst struct IPAModuleInfo testInfo = {\n> +\t\t\t1,\n> +\t\t\t0, 9001,\n> +\t\t\t\"bleep\",\n>  \t\t\t\"It's over nine thousand!\",\n> -\t\t\t9001,\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..d932a9b 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> +\t1,\n> +\t0, 9001,\n> +\t\"bleep\",\n>  \t\"It's over nine thousand!\",\n> -\t9001,\n>  };\n>  };\n>  \n> -- \n> 2.20.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lf1-x143.google.com (mail-lf1-x143.google.com\n\t[IPv6:2a00:1450:4864:20::143])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 14D0F60BD7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  4 Jun 2019 01:08:51 +0200 (CEST)","by mail-lf1-x143.google.com with SMTP id y13so14884802lfh.9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 03 Jun 2019 16:08:51 -0700 (PDT)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tq17sm3389781lfn.71.2019.06.03.16.08.49\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tMon, 03 Jun 2019 16:08:49 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=XJOhIMB6cuixyx+2K5vCGnFWMj0bAIc9AiJDtj7LVVw=;\n\tb=EO/f7btkmurwZWwZfaqcuOkqhBSCBVxcgmyh1CykCiWUpAGsC65yfJTRviJx0hIuWG\n\t9NVSghtcw27f2KCaoL0luFhNOy/yecAiLfUd9FpXRMQKRV8T8EBh84OPbGiOj0uXlnfU\n\tct1bGnpBqK5SUaShFpRdsSa8eeAcPlWquaAIWHJj82cQJE6XFXJjlPo7uU/KIFkEqyAY\n\tSLnEQksczSyx5mUfExOMUm8kuioRSJxc7oamKJvR4P30M+XIrqVQqrjYnNwTD6cUyyy4\n\tyDhdHdExQqv3HFBPGHcjZxWxUYHkksLKR+G95cknKJ7VoHwhmyH+3qv6s69FhRjEgBH6\n\t59kA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=XJOhIMB6cuixyx+2K5vCGnFWMj0bAIc9AiJDtj7LVVw=;\n\tb=nQXfHaBVaaHPJaqBzK18fdGmuzPqGp+DNfQAnSu/4aeeHSsj0vGqgaU9xwhhQL+AEg\n\tGwgku+IxuF2JoUXC770pC+VCrYm5NrVhRD3Zp7OKp+vWdgLLQ8QNDN6FjLM0fmaujwDr\n\tEOUP4WEnv4kMlqGSC0bUsJEfmlUlia5UXprIN233RDXLGnhef+ymGuPXUMjHAhgX0pHm\n\tUFMYte7+doBE9M+7AR4pLqo8XKpw+XhqmEKpXVqQerEoLac0a3SiQPJG0D5G7veTJCpb\n\tUAMtEUpcsddaB9hPGj1pl8DSrrkLA+My0O7KJsa+thRN4Hc/pJH+0ZUXKZs0tzjKWL+x\n\tp5Aw==","X-Gm-Message-State":"APjAAAURFboITGUVfX5Ex21InN+cD2opqzd2R4D+Wu/hwgW03Fwz4HiL\n\tblCMESQhmmYRKIwV4YPtqi6bqvfh8ek=","X-Google-Smtp-Source":"APXvYqwJRr5M+wHtO22h2mXWRYZ6i5iOhMW0SkK+LcDLicUOx23+700EivwNRnT73exFBARiYj4XaQ==","X-Received":"by 2002:ac2:5ecd:: with SMTP id\n\td13mr2941333lfq.132.1559603330440; \n\tMon, 03 Jun 2019 16:08:50 -0700 (PDT)","Date":"Tue, 4 Jun 2019 01:08:48 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190603230848.GF2960@bigcity.dyn.berto.se>","References":"<20190527223540.21855-1-paul.elder@ideasonboard.com>\n\t<20190527223540.21855-4-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190527223540.21855-4-paul.elder@ideasonboard.com>","User-Agent":"Mutt/1.11.4 (2019-03-13)","Subject":"Re: [libcamera-devel] [PATCH 3/8] 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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 03 Jun 2019 23:08:51 -0000"}},{"id":1751,"web_url":"https://patchwork.libcamera.org/comment/1751/","msgid":"<20190604095314.GD4771@pendragon.ideasonboard.com>","date":"2019-06-04T09:53:14","subject":"Re: [libcamera-devel] [PATCH 3/8] 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 Niklas,\n\nOn Tue, Jun 04, 2019 at 01:08:48AM +0200, Niklas Söderlund wrote:\n> On 2019-05-27 18:35:35 -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 API version number, the pipeline name, and\n> > the pipeline major and minor version.\n> \n> I'm not sure I like that the IPA API have a version number, I know now \n> you think I'm crazy but please hear me out ;-)\n> \n> If we put API version numbers in both the PipelineHandler's and IPA's \n> that means that libcamera will need some logic to match the API version \n> requested by a pipeline handler to the version number reported by the \n> IPA. On the surface this seems sane, but then we enter the fun world of \n> compatibility and 3rd party closed source IPA's.\n> \n>   - Case 1 - Open source IPA shipped with libcamera\n> \n>     Here we have no issue as both PipelineHanlder and IPA lives in the \n>     same source tree and are build and distributed from the same \n>     sources.\n> \n>     Recording API versions in both PipelineHandler and IPA make sens. In \n>     this case the version could be equal to the libcamera version.\n> \n>   - Case 2 - Closed source IPA shipped as 3rd party component\n> \n>     If anything have changed in the API between pipeline handlers and \n>     IPA's between libcamera version A and B they are no longer \n>     compatible. This change can be a pipe <-> IPA specific API change or \n>     a change in any of the libcamera common objects which are \n>     sent/received between the pipe and IPA.\n> \n>     For example a new control is added which potentially could change \n>     the size of libcamera::Request::Controls (theoretical object).\n> \n>     Now we the 3rd party vendor have a few options on how to handle \n>     this. The ones I can think of on the top of my head is:\n> \n>     1. Rebuild and distribute a binary IPA for every new released \n>        version of libcamera.\n>     2. Try to upstream some compatibility conversion to the pipeline \n>        handler which uses \"old\" libcamera objects.\n>     3. Try to convince libcamera to provide a versioned LTS API between \n>        pipeline and IPA (both for pipe<->IPA and objects).\n> \n> All of the solutions for closed source IPAs have drawbacks, either from \n> a IPA provider point of view (1) or for libcamera (2, 3). I totally \n> agree that we should not change any libcamera objects involved in the \n> IPA without a good reason, but we should not be blocked form solving \n> problems in libcamera due to closed source components.\n> \n> There fore I propose we think about a solution where all compatibility \n> conversions and decisions are pushed to the IPA implementations. The IPA \n> would be initialized with two version numbers when the so is loaded.\n> \n>  - A libcamera API version which control the version of all common \n>    libcamera objects which can be transmitted/received to/from a IPA.\n>  - A pipeline API version which control the API of which the pipeline \n>    expects to interact with the IPA.\n\nThe libcamera API version number stored in the info structure describes\nthe layout of that structure. It can't be passed by libcamera to the\nIPA, as the structure is read by libcamera before the IPA init()\nfunction is called. I thus think we need to keep that version number, in\norder to interpret the content of the structure.\n\n> Then it's up to the IPA to accept if it can function with these two \n> versions, or not. Then if a IPA implementation wish to function whit \n> both a newer and older version of one of these components it can \n> implement it's own compatibility layer. And we can still move forward \n> with libcamera core API and open source IPA in the libcamera tree \n> without needing a huge verification program for 3rd party IPAs.\n\nIt's not a bad idea, but it won't allow an old IPA to work with a newer\nversion of libcamera, which is the main issue we're trying to solve with\nversioning. I expect IPAs to be updated way less frequently than\nlibcamera.\n\n> Once more; I'm not advocating an API which is changed lightly as we \n> really should do our most to make IPA designers life easy. But if we \n> need to do so to fix bugs in libcamera we should not be blocked by \n> anything out of tree.\n> \n> I don't feel strongly about this topic but thought it worth bringing up \n> when we design this. So feel free to ignore me ;-)\n> \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 <paul.elder@ideasonboard.com>\n> > ---\n> >  Documentation/Doxyfile.in               |  7 ++--\n> >  include/libcamera/ipa/ipa_module_info.h | 11 ++++--\n> >  src/libcamera/ipa_module.cpp            | 48 +++++++++++++++++++++----\n> >  test/ipa/ipa_test.cpp                   | 45 ++++++++++++++++++-----\n> >  test/ipa/shared_test.cpp                |  4 ++-\n> >  5 files changed, 94 insertions(+), 21 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..84492ed 100644\n> > --- a/include/libcamera/ipa/ipa_module_info.h\n> > +++ b/include/libcamera/ipa/ipa_module_info.h\n> > @@ -7,14 +7,21 @@\n> >  #ifndef __LIBCAMERA_IPA_MODULE_INFO_H__\n> >  #define __LIBCAMERA_IPA_MODULE_INFO_H__\n> >  \n> > +#include <stdint.h>\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> > +\tint pipelineVersionMajor;\n> > +\tint pipelineVersionMinor;\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..6e68934 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 API version\n> > + *\n> > + * This version number specifies the version for the layout of\n> > + * struct IPAModuleInfo, and the API between pipeline handlers and the IPA.\n> > + * The IPA module should use this macro to define its moduleAPIVersion field.\n> > + *\n> > + * \\sa libcamera::IPAModuleInfo::moduleAPIVersion\n> > + */\n> > +\n> >  /**\n> >   * \\struct IPAModuleInfo\n> >   * \\brief Information of an IPA module\n> > @@ -176,14 +187,39 @@ 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 API version that the IPA module was made with\n> > + *\n> > + * This version number specifies the version for the layout of\n> > + * struct IPAModuleInfo, and the API between pipeline handlers and the IPA.\n> > + * The IPA module shall report here the version that it was built for,\n> > + * using the macro IPA_MODULE_API_VERSION.\n> >   *\n> > - * \\var IPAModuleInfo::version\n> > - * \\brief The version of the IPA module\n> > + * \\sa IPA_MODULE_API_VERSION\n> >   *\n> > - * \\todo abi compatability version\n> > - * \\todo pipeline compatability matcher\n> > + * \\var IPAModuleInfo::pipelineVersionMajor\n> > + * \\brief The pipeline handler version (major) that the IPA module is for\n> > + * \\todo actually match the pipeline handler against this\n> > + *\n> > + * The major version of the module and pipeline handler must be equal in\n> > + * order for the pipeline handler and the module to be matched.\n> > + *\n> > + * \\var IPAModuleInfo::pipelineVersionMinor\n> > + * \\brief The pipeline handler version (minor) that the IPA module is for\n> > + *\n> > + * The minor version of the module must be equal to or greater than\n> > + * the pipeline handler minor version in order for the pipeline handler\n> > + * and the module to be matched.\n> > + *\n> > + * \\var IPAModuleInfo::pipelineName\n> > + * \\brief The name of the pipeline handler that the IPA module is for\n> > + *\n> > + * This name is used to match a pipeline handler with the module.\n> > + * The name of the pipeline handler is declared in the pipeline handler\n> > + * with the macro REGISTER_PIPELINE_HANDLER.\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..f50880e 100644\n> > --- a/test/ipa/ipa_test.cpp\n> > +++ b/test/ipa/ipa_test.cpp\n> > @@ -33,20 +33,45 @@ 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\tstringstream errString;\n> > +\n> > +\t\tif (info.moduleAPIVersion != testInfo.moduleAPIVersion) {\n> > +\t\t\terrString << \"incorrect name: expected \\\"\"\n> > +\t\t\t\t  << testInfo.moduleAPIVersion << \"\\\", got \\\"\"\n> > +\t\t\t\t  << info.moduleAPIVersion << \"\\\"\" << 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\tif (info.pipelineVersionMajor != testInfo.pipelineVersionMajor) {\n> > +\t\t\terrString << \"incorrect name: expected \\\"\"\n> > +\t\t\t\t  << testInfo.pipelineVersionMajor << \"\\\", got \\\"\"\n> > +\t\t\t\t  << info.pipelineVersionMajor << \"\\\"\" << endl;\n> >  \t\t\tret = -1;\n> >  \t\t}\n> >  \n> > +\t\tif (info.pipelineVersionMinor != testInfo.pipelineVersionMinor) {\n> > +\t\t\terrString << \"incorrect name: expected \\\"\"\n> > +\t\t\t\t  << testInfo.pipelineVersionMinor << \"\\\", got \\\"\"\n> > +\t\t\t\t  << info.pipelineVersionMinor << \"\\\"\" << endl;\n> > +\t\t\tret = -1;\n> > +\t\t}\n> > +\n> > +\t\tif (strcmp(info.pipelineName, testInfo.pipelineName)) {\n> > +\t\t\terrString << \"incorrect name: expected \\\"\"\n> > +\t\t\t\t  << testInfo.pipelineName << \"\\\", got \\\"\"\n> > +\t\t\t\t  << info.pipelineName << \"\\\"\" << endl;\n> > +\t\t\tret = -1;\n> > +\t\t}\n> > +\n> > +\t\tif (strcmp(info.name, testInfo.name)) {\n> > +\t\t\terrString << \"incorrect name: expected \\\"\"\n> > +\t\t\t\t  << testInfo.name << \"\\\", got \\\"\"\n> > +\t\t\t\t  << info.name << \"\\\"\" << endl;\n> > +\t\t\tret = -1;\n> > +\t\t}\n> > +\n> > +\t\tcerr << errString.str();\n> > +\n> >  \t\tdelete ll;\n> >  \t\treturn ret;\n> >  \t}\n> > @@ -56,8 +81,10 @@ protected:\n> >  \t\tint count = 0;\n> >  \n> >  \t\tconst struct IPAModuleInfo testInfo = {\n> > +\t\t\t1,\n> > +\t\t\t0, 9001,\n> > +\t\t\t\"bleep\",\n> >  \t\t\t\"It's over nine thousand!\",\n> > -\t\t\t9001,\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..d932a9b 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> > +\t1,\n> > +\t0, 9001,\n> > +\t\"bleep\",\n> >  \t\"It's over nine thousand!\",\n> > -\t9001,\n> >  };\n> >  };\n> >  \n> > -- \n> > 2.20.1\n> > \n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\n> \n> -- \n> Regards,\n> Niklas Söderlund\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["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 43B0061CDA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  4 Jun 2019 11:53:27 +0200 (CEST)","from pendragon.ideasonboard.com (unknown\n\t[IPv6:2a02:2788:668:163:5bb7:9f6c:564c:d55e])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D263E51C;\n\tTue,  4 Jun 2019 11:53:26 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1559642006;\n\tbh=Nk9H4e58GFz3eMQ39q27k1hP7+nImMQCO23zlcnddz4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=W+DO1yk7EhCcGz2aLm1WerQj0eekpvM3OjC0VGMNdZTlmjEK7LHlZWPvALUKUymL/\n\tec9+QFv092sP0Psjec9YG2GUeyHitrjOFTS4+ZvIKffQ4+5RHuu4/BDpK6J/f62LBZ\n\tIVRFd4fclKZrciUrrseYJE3dAM/a2bKFZiKhM7RA=","Date":"Tue, 4 Jun 2019 12:53:14 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"Paul Elder <paul.elder@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20190604095314.GD4771@pendragon.ideasonboard.com>","References":"<20190527223540.21855-1-paul.elder@ideasonboard.com>\n\t<20190527223540.21855-4-paul.elder@ideasonboard.com>\n\t<20190603230848.GF2960@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190603230848.GF2960@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 3/8] 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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 04 Jun 2019 09:53:27 -0000"}}]