[libcamera-devel,4/5] libcamera: ipa_manager: Search the runpath for IPA libraries

Message ID 20200205130420.8736-5-kieran.bingham@ideasonboard.com
State Superseded
Headers show
Series
  • Support loading IPAs from the build tree
Related show

Commit Message

Kieran Bingham Feb. 5, 2020, 1:04 p.m. UTC
Expose the IPA build path through the libcamera library runpath entry in
the elf string table. Utilise this path to search for compiled IPA
libraries when loading the IPAManager.

The build time path is removed during the library install phase.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/libcamera/ipa_manager.cpp | 9 +++++++++
 src/libcamera/meson.build     | 1 +
 src/meson.build               | 7 +++++++
 3 files changed, 17 insertions(+)

Comments

Laurent Pinchart Feb. 6, 2020, 8:34 p.m. UTC | #1
Hi Kieran,

Thank you for the patch.

On Wed, Feb 05, 2020 at 01:04:19PM +0000, Kieran Bingham wrote:
> Expose the IPA build path through the libcamera library runpath entry in

s/library runpath entry/library search path entry (DT_RUNPATH)/

> the elf string table. Utilise this path to search for compiled IPA

s/elf string table/ELF dynamic tags/ ?

> libraries when loading the IPAManager.
> 
> The build time path is removed during the library install phase.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/libcamera/ipa_manager.cpp | 9 +++++++++
>  src/libcamera/meson.build     | 1 +
>  src/meson.build               | 7 +++++++
>  3 files changed, 17 insertions(+)
> 
> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
> index 24fe709108fe..3dfe84fcf95e 100644
> --- a/src/libcamera/ipa_manager.cpp
> +++ b/src/libcamera/ipa_manager.cpp
> @@ -102,6 +102,15 @@ IPAManager::IPAManager()
>  	if (ret > 0)
>  		ipaCount += ret;
>  
> +	/*
> +	 * Utilise the Elf runpath to supply the IPA build directory, allowing

s/Elf/ELF/

> +	 * us to load IPA's before they are installed for testing and
> +	 * development.
> +	 */
> +	const char *runpath = utils::get_runpath();
> +	if (runpath)
> +		ipaCount += addPath(runpath, true);

Shouldn't this go after LIBCAMERA_IPA_MODULE_PATH ? If a user sets
LIBCAMERA_IPA_MODULE_PATH, it should be considered before DT_RUNPATH.

I think we also need to move IPA_MODULE_DIR after DT_RUNPATH, as the
build directory should have precedence over the system directory.

Maybe the above comment should be expanded to explain the load order ?


	/*
	 * Load IPA modules. The IPA manager uses the following search paths to
	 * locate the modules.
	 *
	 * - The directories in DT_RUNPATH. The build system sets DT_RUNPATH to
	 *   point to the src/ipa/ subdirectory of the build directory, allowing
	 *   finding modules in the build directory. DT_RUNPATH is stripped when
	 *   libcamera.so is installed.
	 *
	 * - The directories specified in the LIBCAMERA_IPA_MODULE_PATH
	 *   environment variable, if set.
	 *
	 * - The $prefix/ipa/ directory.
	 */

> +
>  	const char *modulePaths = utils::secure_getenv("LIBCAMERA_IPA_MODULE_PATH");
>  	if (!modulePaths) {
>  		if (!ipaCount)
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 68d89559b290..87c5bbbb17a4 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -107,6 +107,7 @@ libcamera = shared_library('camera',
>                             install : true,
>                             link_with : libcamera_link_with,
>                             include_directories : includes,
> +                           build_rpath : ipa_build_path,
>                             dependencies : libcamera_deps)
>  
>  libcamera_dep = declare_dependency(sources : [libcamera_api, libcamera_ipa_api, libcamera_h],
> diff --git a/src/meson.build b/src/meson.build
> index 5adcd61fd913..42e1eb984ccd 100644
> --- a/src/meson.build
> +++ b/src/meson.build
> @@ -2,6 +2,13 @@ if get_option('android')
>      subdir('android')
>  endif
>  
> +# Provide libcamera with a reference to the current build for IPA

s/current build/build directory path/ ?

> +# loading. This allows IPAs to be successfully loaded when tests and
> +# applications are run from the build tree, but is stripped during the
> +# install phase, ensuring we maintain reproducible builds.
> +
> +ipa_build_path = join_paths(meson.current_build_dir(), 'ipa')
> +
>  subdir('libcamera')
>  subdir('ipa')
>  subdir('cam')

Patch

diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
index 24fe709108fe..3dfe84fcf95e 100644
--- a/src/libcamera/ipa_manager.cpp
+++ b/src/libcamera/ipa_manager.cpp
@@ -102,6 +102,15 @@  IPAManager::IPAManager()
 	if (ret > 0)
 		ipaCount += ret;
 
+	/*
+	 * Utilise the Elf runpath to supply the IPA build directory, allowing
+	 * us to load IPA's before they are installed for testing and
+	 * development.
+	 */
+	const char *runpath = utils::get_runpath();
+	if (runpath)
+		ipaCount += addPath(runpath, true);
+
 	const char *modulePaths = utils::secure_getenv("LIBCAMERA_IPA_MODULE_PATH");
 	if (!modulePaths) {
 		if (!ipaCount)
diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
index 68d89559b290..87c5bbbb17a4 100644
--- a/src/libcamera/meson.build
+++ b/src/libcamera/meson.build
@@ -107,6 +107,7 @@  libcamera = shared_library('camera',
                            install : true,
                            link_with : libcamera_link_with,
                            include_directories : includes,
+                           build_rpath : ipa_build_path,
                            dependencies : libcamera_deps)
 
 libcamera_dep = declare_dependency(sources : [libcamera_api, libcamera_ipa_api, libcamera_h],
diff --git a/src/meson.build b/src/meson.build
index 5adcd61fd913..42e1eb984ccd 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -2,6 +2,13 @@  if get_option('android')
     subdir('android')
 endif
 
+# Provide libcamera with a reference to the current build for IPA
+# loading. This allows IPAs to be successfully loaded when tests and
+# applications are run from the build tree, but is stripped during the
+# install phase, ensuring we maintain reproducible builds.
+
+ipa_build_path = join_paths(meson.current_build_dir(), 'ipa')
+
 subdir('libcamera')
 subdir('ipa')
 subdir('cam')