[libcamera-devel] [PATCH v3 03/10] libcamera: ipa_module_info: update struct to allow IPA matching
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Jun 5 15:32:46 CEST 2019
Hi Paul,
Thank you for the match.
On Tue, Jun 04, 2019 at 08:53:09PM -0400, Paul Elder wrote:
> We need a way to match pipelines with IPA modules, so add fields in
> IPAModuleInfo to hold the IPA module API version number, the pipeline
> name, and the pipeline version.
>
> The module API version is used to determine the layout of struct
> IPAModuleInfo.
>
> Also update IPA module tests and Doxygen accordingly. Doxygen needs to
> be updated to accomodate __attribute__((packed)).
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> Changes in v3:
> - remove PIPELINE_VERSION documentation (since the macro was removed,
> since pipeline versions are now a single number)
>
> Changes in v2:
> - combine pipeline major and minor versions into one using the
> Documentation/Doxyfile.in | 7 +++---
>
> include/libcamera/ipa/ipa_module_info.h | 10 ++++++--
> src/libcamera/ipa_module.cpp | 32 ++++++++++++++++++++-----
> test/ipa/ipa_test.cpp | 27 +++++++++++----------
> test/ipa/shared_test.cpp | 4 +++-
> 5 files changed, 55 insertions(+), 25 deletions(-)
>
> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
> index 67eaded..ac70efb 100644
> --- a/Documentation/Doxyfile.in
> +++ b/Documentation/Doxyfile.in
> @@ -2016,7 +2016,7 @@ ENABLE_PREPROCESSING = YES
> # The default value is: NO.
> # This tag requires that the tag ENABLE_PREPROCESSING is set to YES.
>
> -MACRO_EXPANSION = NO
> +MACRO_EXPANSION = YES
>
> # If the EXPAND_ONLY_PREDEF and MACRO_EXPANSION tags are both set to YES then
> # the macro expansion is limited to the macros specified with the PREDEFINED and
> @@ -2024,7 +2024,7 @@ MACRO_EXPANSION = NO
> # The default value is: NO.
> # This tag requires that the tag ENABLE_PREPROCESSING is set to YES.
>
> -EXPAND_ONLY_PREDEF = NO
> +EXPAND_ONLY_PREDEF = YES
>
> # If the SEARCH_INCLUDES tag is set to YES, the include files in the
> # INCLUDE_PATH will be searched if a #include is found.
> @@ -2057,7 +2057,8 @@ INCLUDE_FILE_PATTERNS = *.h
> # This tag requires that the tag ENABLE_PREPROCESSING is set to YES.
>
> PREDEFINED = __DOXYGEN__ \
> - __cplusplus
> + __cplusplus \
> + __attribute__(x)=
>
> # If the MACRO_EXPANSION and EXPAND_ONLY_PREDEF tags are set to YES then this
> # tag can be used to specify a list of macro names that should be expanded. The
> diff --git a/include/libcamera/ipa/ipa_module_info.h b/include/libcamera/ipa/ipa_module_info.h
> index 4e0d668..803b5d3 100644
> --- a/include/libcamera/ipa/ipa_module_info.h
> +++ b/include/libcamera/ipa/ipa_module_info.h
> @@ -7,14 +7,20 @@
> #ifndef __LIBCAMERA_IPA_MODULE_INFO_H__
> #define __LIBCAMERA_IPA_MODULE_INFO_H__
>
> +#include <stdint.h>
> +
> +#define IPA_MODULE_API_VERSION 1
> +
> #ifdef __cplusplus
> namespace libcamera {
> #endif
>
> struct IPAModuleInfo {
> + int moduleAPIVersion;
> + uint32_t pipelineVersion;
> + char pipelineName[256];
> char name[256];
> - unsigned int version;
> -};
> +} __attribute__((packed));
>
> #ifdef __cplusplus
> extern "C" {
> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp
> index 86cbe71..f79a44e 100644
> --- a/src/libcamera/ipa_module.cpp
> +++ b/src/libcamera/ipa_module.cpp
> @@ -169,6 +169,17 @@ int elfLoadSymbol(void *dst, size_t size, void *map, size_t soSize,
>
> } /* namespace */
>
> +/**
> + * \def IPA_MODULE_API_VERSION
> + * \brief The IPA module API version
> + *
> + * This version number specifies the version for the layout of
> + * struct IPAModuleInfo. The IPA module shall use this macro to
> + * set its moduleAPIVersion field.
> + *
> + * \sa IPAModuleInfo::moduleAPIVersion
> + */
> +
> /**
> * \struct IPAModuleInfo
> * \brief Information of an IPA module
> @@ -176,14 +187,23 @@ int elfLoadSymbol(void *dst, size_t size, void *map, size_t soSize,
> * This structure contains the information of an IPA module. It is loaded,
> * read, and validated before anything else is loaded from the shared object.
> *
> - * \var IPAModuleInfo::name
> - * \brief The name of the IPA module
> + * \var IPAModuleInfo::moduleAPIVersion
> + * \brief The IPA module API version that the IPA module implements
> + *
> + * This version number specifies the version for the layout of
> + * struct IPAModuleInfo. The IPA module shall report here the version that
> + * it was built for, using the macro IPA_MODULE_API_VERSION.
> + *
> + * \var IPAModuleInfo::pipelineVersion
> + * \brief The pipeline handler version that the IPA module is for
> *
> - * \var IPAModuleInfo::version
> - * \brief The version of the IPA module
> + * \var IPAModuleInfo::pipelineName
> + * \brief The name of the pipeline handler that the IPA module is for
> *
> - * \todo abi compatability version
> - * \todo pipeline compatability matcher
> + * This name is used to match a pipeline handler with the module.
> + *
> + * \var IPAModuleInfo::name
> + * \brief The name of the IPA module
> */
>
> /**
> diff --git a/test/ipa/ipa_test.cpp b/test/ipa/ipa_test.cpp
> index 2dbc702..63e4243 100644
> --- a/test/ipa/ipa_test.cpp
> +++ b/test/ipa/ipa_test.cpp
> @@ -33,18 +33,17 @@ protected:
>
> const struct IPAModuleInfo &info = ll->info();
>
> - if (strcmp(info.name, testInfo.name)) {
> - cerr << "test IPA module has incorrect name" << endl;
> - cerr << "expected \"" << testInfo.name << "\", got \""
> - << info.name << "\"" << endl;
> - ret = -1;
> - }
> -
> - if (info.version != testInfo.version) {
> - cerr << "test IPA module has incorrect version" << endl;
> - cerr << "expected \"" << testInfo.version << "\", got \""
> - << info.version << "\"" << endl;
> - ret = -1;
> + if (memcmp(&info, &testInfo, sizeof(info))) {
> + cerr << "IPA module information mismatch: expected:" << endl
> + << "moduleAPIVersion = " << testInfo.moduleAPIVersion << endl
> + << "pipelineVersion = " << testInfo.pipelineVersion << endl
> + << "pipelineName = " << testInfo.pipelineName << endl
> + << "name = " << testInfo.name
> + << "got: " << endl
> + << "moduleAPIVersion = " << info.moduleAPIVersion << endl
> + << "pipelineVersion = " << info.pipelineVersion << endl
> + << "pipelineName = " << info.pipelineName << endl
> + << "name = " << info.name << endl;
> }
>
> delete ll;
> @@ -56,8 +55,10 @@ protected:
> int count = 0;
>
> const struct IPAModuleInfo testInfo = {
> - "It's over nine thousand!",
> + IPA_MODULE_API_VERSION,
> 9001,
> + "bleep",
> + "It's over nine thousand!",
> };
>
> count += runTest("test/ipa/ipa-dummy-cpp.so", testInfo);
> diff --git a/test/ipa/shared_test.cpp b/test/ipa/shared_test.cpp
> index 4e5c976..8bac439 100644
> --- a/test/ipa/shared_test.cpp
> +++ b/test/ipa/shared_test.cpp
> @@ -4,8 +4,10 @@ namespace libcamera {
>
> extern "C" {
> const struct libcamera::IPAModuleInfo ipaModuleInfo = {
> - "It's over nine thousand!",
> + IPA_MODULE_API_VERSION,
> 9001,
> + "bleep",
> + "It's over nine thousand!",
> };
> };
>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list