[libcamera-devel] [PATCH v4 5/6] libcamera: ipa_manager: Search for IPA libraries in build tree

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Fri, Feb 21, 2020 at 04:31:29PM +0000, Kieran Bingham wrote:
> When libcamera is built and tested (or used at all) before installing to
> the configured prefix path, it will be unable to locate the IPA
> binaries, or IPA binaries previously installed in the system paths may
> be incorrect to load.
> 
> Utilise the build_rpath dynamic tag which is stripped out by meson at
> install time to determine at runtime if the library currently executing
> has been installed or not.
> 
> When not installed and running from a build tree, identify the location
> of that tree by finding the path of the active libcamera.so itself, and
> from that point add a relative path to be able to load the most recently
> built IPA modules.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> ---
> v2:
>  - Move get_runpath to ipa_manager.c : libcamera::elfRunPath()
>  - Minor fixes
>  - Squash elfRunPath() into this patch
> 
> v3:
>  - Full rework. It's just all different :-)
>  - Maybe this one is going to cut it ...
> 
> v4:
>  - rebase and cleanup
> ---
>  src/libcamera/ipa_manager.cpp | 52 ++++++++++++++++++++++++++++++++++-
>  src/libcamera/meson.build     |  6 ++++
>  2 files changed, 57 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
> index de99a0bc3ce1..a5f2707f6788 100644
> --- a/src/libcamera/ipa_manager.cpp
> +++ b/src/libcamera/ipa_manager.cpp
> @@ -9,6 +9,9 @@
>  
>  #include <algorithm>
>  #include <dirent.h>
> +#include <dlfcn.h>
> +#include <elf.h>
> +#include <link.h>
>  #include <string.h>
>  #include <sys/types.h>
>  
> @@ -24,6 +27,35 @@
>   * \brief Image Processing Algorithm module manager
>   */
>  
> +static bool isLibcameraInstalled()
> +{
> +	/* musl doesn't declare _DYNAMIC in link.h, declare it manually. */
> +	extern ElfW(Dyn) _DYNAMIC[];
> +
> +	/*
> +	 * DT_RUNPATH (DT_RPATH when the linker uses old dtags) is removed on
> +	 * install.
> +	 */
> +	for (const ElfW(Dyn) *dyn = _DYNAMIC; dyn->d_tag != DT_NULL; ++dyn) {
> +		if (dyn->d_tag == DT_RUNPATH || dyn->d_tag == DT_RPATH)
> +			return false;
> +	}
> +
> +	return true;
> +}
> +
> +static const char *libcameraPath()
> +{
> +	Dl_info info;
> +
> +	/* Look up our own symbol. */
> +	int ret = dladdr(reinterpret_cast<void *>(libcameraPath), &info);
> +	if (ret == 0)
> +		return nullptr;
> +
> +	return info.dli_fname;

You're returning a pointer to a local stack variable. I'm surprised that
compilers don't warn us. The easiest fix is to turn the function into

static std::string libcameraPath()

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

> +}
> +
>  namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(IPAManager)
> @@ -112,7 +144,25 @@ IPAManager::IPAManager()
>  				<< "No IPA found in '" << modulePaths << "'";
>  	}
>  
> -	/* Load IPAs from the installed system path. */
> +	/*
> +	 * When libcamera is used before it is installed, load IPAs from the
> +	 * same build directory as the libcamera library itself. This requires
> +	 * identifying the path of the libcamera.so, and referencing a relative
> +	 * path for the IPA from that point. We need to recurse one level of
> +	 * sub-directories to match the build tree.
> +	 */
> +	if (!isLibcameraInstalled()) {
> +		std::string ipaBuildPath = utils::dirname(libcameraPath()) + "/../ipa";
> +		constexpr int maxDepth = 1;
> +
> +		LOG(IPAManager, Info)
> +			<< "libcamera is not installed. Adding '"
> +			<< ipaBuildPath << "' to the IPA search path";
> +
> +		ipaCount += addDir(ipaBuildPath.c_str(), maxDepth);
> +	}
> +
> +	/* Finally try to load IPAs from the installed system path. */
>  	ipaCount += addDir(IPA_MODULE_DIR);
>  
>  	if (!ipaCount)
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 1e5b54b34078..88658ac563f7 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -107,11 +107,17 @@ if get_option('android')
>      libcamera_link_with += android_camera_metadata
>  endif
>  
> +# We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.
> +# The build_rpath is stripped at install time by meson, so we determine at
> +# runtime if the library is running from an installed location by checking
> +# for the presence or abscence of the dynamic tag.
> +
>  libcamera = shared_library('camera',
>                             libcamera_sources,
>                             install : true,
>                             link_with : libcamera_link_with,
>                             include_directories : includes,
> +                           build_rpath : '/',
>                             dependencies : libcamera_deps)
>  
>  libcamera_dep = declare_dependency(sources : [libcamera_api, libcamera_ipa_api, libcamera_h],

-- 
Regards,

Laurent Pinchart