[libcamera-devel] [PATCH v3 1/7] libcamera: ipa_module_info: add license field

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Wed, Jul 10, 2019 at 03:44:44AM +0900, Paul Elder wrote:
> Add a field to IPAModuleInfo to contain the license of the module.
> 
> This license field will be used to determine whether the IPA module
> should be run in an isolated process or not. If the license is open
> source, then the IPA module will be allowed to run without process
> isolation, if the user enables it. If the license is not open source,
> then the IPA module will be run with process isolation.
> 
> Update the dummy IPA and IPA test to conform to the new struct layout.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
> Changes in v3:
> - make license field SPDX
> 
> New patch in v2
> - this replaces the isolate flag that was used in v1
> 
>  include/libcamera/ipa/ipa_module_info.h |  1 +
>  src/ipa/ipa_dummy.cpp                   |  1 +
>  src/libcamera/ipa_module.cpp            | 21 +++++++++++++++++++++
>  test/ipa/ipa_test.cpp                   |  1 +
>  4 files changed, 24 insertions(+)
> 
> diff --git a/include/libcamera/ipa/ipa_module_info.h b/include/libcamera/ipa/ipa_module_info.h
> index 585f753..d9e33c1 100644
> --- a/include/libcamera/ipa/ipa_module_info.h
> +++ b/include/libcamera/ipa/ipa_module_info.h
> @@ -18,6 +18,7 @@ struct IPAModuleInfo {
>  	uint32_t pipelineVersion;
>  	char pipelineName[256];
>  	char name[256];
> +	char license[64];
>  } __attribute__((packed));
>  
>  extern "C" {
> diff --git a/src/ipa/ipa_dummy.cpp b/src/ipa/ipa_dummy.cpp
> index ee7a3a8..4c8b665 100644
> --- a/src/ipa/ipa_dummy.cpp
> +++ b/src/ipa/ipa_dummy.cpp
> @@ -34,6 +34,7 @@ const struct IPAModuleInfo ipaModuleInfo = {
>  	0,
>  	"PipelineHandlerVimc",
>  	"Dummy IPA for Vimc",
> +	"LGPL-2.1-or-later",
>  };
>  
>  IPAInterface *ipaCreate()
> diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp
> index d2e3c36..06111c0 100644
> --- a/src/libcamera/ipa_module.cpp
> +++ b/src/libcamera/ipa_module.cpp
> @@ -214,6 +214,27 @@ elfLoadSymbol(void *map, size_t soSize, const char *symbol)
>   *
>   * \var IPAModuleInfo::name
>   * \brief The name of the IPA module
> + *
> + * \var IPAModuleInfo::license
> + * \brief License of the IPA module
> + *
> + * This license is used to determine whether to isolate the IPA in a

I would say "whether to force isolation of the IPA in a separate
process", otherwise it could imply that the license is the only decisive
factor.

> + * separate process. If the license is "Proprietary", then the IPA will
> + * be isolated.

And add here "If the license is open-source, then the IPA will be
allowed to run without isolation if the user enables it."

With those issues addressed,

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> The license should be an SPDX license string. The following
> + * licenses are currently available to allow the IPA to run unisolated:
> + *
> + * - GPL-2.0-only
> + * - GPL-2.0-or-later
> + * - GPL-3.0-only
> + * - GPL-3.0-or-later
> + * - LGPL-2.1-only
> + * - LGPL-2.1-or-later
> + * - LGPL-3.0-only
> + * - LGPL-3.0-or-later
> + *
> + * Any other license will cause the IPA to be run isolated.
> + *
> + * \todo Allow user to choose to isolated open source IPAs
>   */
>  
>  /**
> diff --git a/test/ipa/ipa_test.cpp b/test/ipa/ipa_test.cpp
> index bbef069..b9e1bd6 100644
> --- a/test/ipa/ipa_test.cpp
> +++ b/test/ipa/ipa_test.cpp
> @@ -59,6 +59,7 @@ protected:
>  			0,
>  			"PipelineHandlerVimc",
>  			"Dummy IPA for Vimc",
> +			"GPL-2.0-or-later",
>  		};
>  
>  		count += runTest("src/ipa/ipa_dummy.so", testInfo);

-- 
Regards,

Laurent Pinchart