[libcamera-devel] [PATCH LIBCAMERA 3/4] libcamera: ipa_proxy: search for proxy in build tree

[Kieran Bingham]

Hi Kaaira

On 17/03/2020 17:31, Kaaira Gupta wrote:
> When libcamera is build and tested before installing, it will

s/build/built/

> be unable to locate the path to proxy files,or previously

s/files,or/files, or/

> installed files in the system path may be incorrect to load.
> 
> Hence, when libcamera is not installed and running from a build
> tree, identify the location of that tree by finding the path of
> active libcamera.so, and from that point add the relative path to
> the proxy file.
> 
> Signed-off-by: Kaaira Gupta <kgupta at es.iitr.ac.in>
> ---
>  src/libcamera/ipa_proxy.cpp | 54 +++++++++++++++++++++++++++++++++----
>  1 file changed, 49 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp
> index 363d034..d7c32a1 100644
> --- a/src/libcamera/ipa_proxy.cpp
> +++ b/src/libcamera/ipa_proxy.cpp
> @@ -9,7 +9,9 @@
>  
>  #include <string.h>
>  #include <unistd.h>
> -
> +#include <dlfcn.h>
> +#include <elf.h>
> +#include <link.h>

Aha, we shouldn't need to bring this in here ... I'm sorry - I should
have spotted this in advance and we'll need to move the
isLibcameraInstalled() and libcameraPath() functions into
  src/libcamera/utils.cpp

For that extra patch (which would then be patch 3 in the series), we
need to move the two fucntions below to utils.cpp, and add appropriate
doxygen briefs as they are then 'global' functions, and put the function
prototypes into src/libcamera/include/utils.h

At that point, the functions will be in the libcamera utils:: namespace
so we'll have to do a fixup in IPAManager::IPAManager() to call
  utils::isLibcameraInstalled()
instead...

And of course as part of the code move, the headers that you had to
bring in here, can be removed from the ipa_manager.cpp file too.


>  #include "log.h"
>  #include "utils.h"
>  
> @@ -20,6 +22,35 @@
>   * \brief IPA Proxy
>   */
>  
> +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 std::string 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;
> +}
> +

As described above, isLibcameraInstalled() and libcameraPath() should
become utils::isLibcameraInstalled() and utils::libcameraPath() and then
these do not need to be added here.


>  namespace libcamera {
>  
>  LOG_DEFINE_CATEGORY(IPAProxy)
> @@ -89,10 +120,23 @@ std::string IPAProxy::resolvePath(const std::string &file) const
>  		}
>  	}
>  
> -	/*Try finding the exec target from the install directory.*/
> -        std::string proxyPath = std::string(IPA_PROXY_DIR) + proxyFile;
> -        if (!access(proxyPath.c_str(), X_OK))
> -                return proxyPath;
> +	/*
> +	 *When libcamera is used before it is installed, load proxies from the
> +	 *same build directory as the libcamera directory itself. This requires
> +	 *identifying the path of the libcamera.so, and referencing a relative
> +	 *path for the proxies from that point.
> +	 */

There should always be a space after the * in a comment to separate the
words from the comment markers.


> +	if (!isLibcameraInstalled()){
> +		std::string ipaProxyDir = utils::dirname(libcameraPath()) + "/../../proxy";
> +
> +		LOG(IPAProxy, Info)
> +			<< "libcamera is not installed. Adding '"
> +			<< ipaProxyDir << "' to the Proxy search path";
> +
> +		std::string proxyPath = ipaProxyDir + proxyFile;
> +		if (!access(proxyPath.c_str(), X_OK))
> +			return proxyPath;
> +	}
>  

The actual additions here are good, and as I expected, so its' just some
whitespace fixes, and to remove the static helpers here so they aren't
duplicated.

>  	return std::string();
>  }
> 

-- 
Regards
--
Kieran