[libcamera-devel] [PATCH 05/11] libcamera: ipa_proxy: Provide suport for IPA configuration files

[Kieran Bingham]

Hi Laurent,

On 27/04/2020 04:17, Laurent Pinchart wrote:
> IPA modules may require configuration files, which may be stored in
> different locations in the file system. To standardize file locations
> between all IPAs and pipeline handlers, provide a helper function to
> locate a configuration file by searching in the following directories:
> 
> - All directories specified in the LIBCAMERA_IPA_CONFIG_PATH environment
>   variable ; or
> - In the build tree if libcamera is not installed ; otherwise
> - In standard system locations (etc and share directories).
> 
> More locations, or extensions to the mechanism, may be implemented
> later.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/ipa/meson.build                      |  6 ++
>  src/libcamera/include/ipa_proxy.h        | 11 ++-
>  src/libcamera/ipa_proxy.cpp              | 91 +++++++++++++++++++++++-
>  src/libcamera/proxy/ipa_proxy_linux.cpp  |  2 +-
>  src/libcamera/proxy/ipa_proxy_thread.cpp |  2 +-
>  5 files changed, 105 insertions(+), 7 deletions(-)
> 
> diff --git a/src/ipa/meson.build b/src/ipa/meson.build
> index cb4e3ab3388f..145bf8105af7 100644
> --- a/src/ipa/meson.build
> +++ b/src/ipa/meson.build
> @@ -1,10 +1,16 @@
>  ipa_install_dir = join_paths(get_option('libdir'), 'libcamera')
> +ipa_data_dir = join_paths(get_option('datadir'), 'libcamera', 'ipa')
> +ipa_sysconf_dir = join_paths(get_option('sysconfdir'), 'libcamera', 'ipa')
>  
>  ipa_includes = [
>      libcamera_includes,
>      libcamera_internal_includes,
>  ]
>  
> +config_h.set('IPA_CONFIG_DIR',
> +             '"' + join_paths(get_option('prefix'), ipa_sysconf_dir) +
> +             ':' + join_paths(get_option('prefix'), ipa_data_dir) + '"')
> +
>  config_h.set('IPA_MODULE_DIR',
>               '"' + join_paths(get_option('prefix'), ipa_install_dir) + '"')
>  
> diff --git a/src/libcamera/include/ipa_proxy.h b/src/libcamera/include/ipa_proxy.h
> index e696551af39f..1111065b36a7 100644
> --- a/src/libcamera/include/ipa_proxy.h
> +++ b/src/libcamera/include/ipa_proxy.h
> @@ -13,22 +13,27 @@
>  
>  #include <ipa/ipa_interface.h>
>  
> -#include "ipa_module.h"
> -
>  namespace libcamera {
>  
> +class IPAModule;
> +
>  class IPAProxy : public IPAInterface
>  {
>  public:
> -	IPAProxy();
> +	IPAProxy(IPAModule *ipam);
>  	~IPAProxy();
>  
>  	bool isValid() const { return valid_; }
>  
> +	std::string configurationFile(const std::string &file) const;
> +
>  protected:
>  	std::string resolvePath(const std::string &file) const;
>  
>  	bool valid_;
> +
> +private:
> +	IPAModule *ipam_;
>  };
>  
>  class IPAProxyFactory
> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp
> index 43ac9afc25cb..ea69e63d6bb8 100644
> --- a/src/libcamera/ipa_proxy.cpp
> +++ b/src/libcamera/ipa_proxy.cpp
> @@ -8,8 +8,11 @@
>  #include "ipa_proxy.h"
>  
>  #include <string.h>
> +#include <sys/stat.h>
> +#include <sys/types.h>
>  #include <unistd.h>
>  
> +#include "ipa_module.h"
>  #include "log.h"
>  #include "utils.h"
>  
> @@ -34,12 +37,13 @@ LOG_DEFINE_CATEGORY(IPAProxy)
>  
>  /**
>   * \brief Construct an IPAProxy instance
> + * \param[in] ipam The IPA module
>   *
>   * IPAProxy instances shall be constructed through the IPAProxyFactory::create()
>   * method implemented by the respective factories.
>   */
> -IPAProxy::IPAProxy()
> -	: valid_(false)
> +IPAProxy::IPAProxy(IPAModule *ipam)
> +	: valid_(false), ipam_(ipam)
>  {
>  }
>  
> @@ -57,6 +61,89 @@ IPAProxy::~IPAProxy()
>   * \return True if the IPAProxy is valid, false otherwise
>   */
>  
> +/**
> + * \brief Retrieve the absolute path to an IPA configuration file
> + * \param[in] name The configuration name
> + *
> + * This function locates the configuration file for an IPA and returns its
> + * absolute path. It searches the following directories, in order:
> + *
> + * - All directories specified in the colon-separated LIBCAMERA_IPA_CONFIG_PATH
> + *   environment variable ; or
> + * - If libcamera is not installed, the src/ipa/ directory within the source
> + *   tree ; otherwise
> + * - The system sysconf (etc/libcamera/ipa) and the data (share/libcamera/ipa/)
> + *   directories.
> + *
> + * The system directories are not searched if libcamera is not installed.
> + *
> + * Within each of those directories, the function looks for a subdirectory
> + * named after the IPA module name, as reported in IPAModuleInfo::name, and for
> + * a file named \a name within that directory. The \a name is IPA-specific.
> + *
> + * \return The full path to the IPA configuration file, or an empty string if
> + * no configuration file can be found
> + */
> +std::string IPAProxy::configurationFile(const std::string &name) const
> +{
> +	struct stat statbuf;
> +	int ret;
> +
> +	/*
> +	 * The IPA module name can be used as-is to build directory names as it
> +	 * has been validated when loading the module.
> +	 */
> +	std::string ipaName = ipam_->info().name;
> +
> +	/* Check the environment variable first. */
> +	const char *confPaths = utils::secure_getenv("LIBCAMERA_IPA_CONFIG_PATH");
> +	if (confPaths) {
> +		for (const auto &dir : utils::split(confPaths, ":")) {
> +			if (dir.empty())
> +				continue;
> +
> +			std::string confPath = dir + "/" + ipaName + "/" + name;
> +			ret = stat(confPath.c_str(), &statbuf);
> +			if (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)
> +				return confPath;
> +		}
> +	}
> +
> +	/*
> +	 * When libcamera is used before it is installed, load configuration
> +	 * files from the source directory. The configuration files are then
> +	 * located in the 'data' subdirectory of the corresponding IPA module.

Hrm ... this seems to be hidden in the implementation.

Perhaps worth describing that configuration files are stored in 'data'
in the source tree as part of the commit log at least, and we'll need
this documenting in the 'ipa-writers-guide' at least, but as that
doesn't exist yet - I'll let you off for not adding it there. ;-)

Other than that,

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>


> +	 */
> +	std::string root = utils::libcameraSourcePath();
> +	if (!root.empty()) {
> +		std::string ipaConfDir = root + "src/ipa/" + ipaName + "/data";
> +
> +		LOG(IPAProxy, Info)
> +			<< "libcamera is not installed. Loading IPA configuration from '"
> +			<< ipaConfDir << "'";
> +
> +		std::string confPath = ipaConfDir + "/" + name;
> +		ret = stat(confPath.c_str(), &statbuf);
> +		if (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)
> +			return confPath;
> +
> +		return std::string();
> +	}
> +
> +	/* Else look in the system locations. */
> +	for (const auto &dir : utils::split(IPA_CONFIG_DIR, ":")) {
> +		if (dir.empty())
> +			continue;
> +
> +		std::string confPath = dir + "/" + ipaName + "/" + name;
> +		ret = stat(confPath.c_str(), &statbuf);
> +		if (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)
> +			return confPath;
> +	}
> +
> +	return std::string();
> +}
> +
>  /**
>   * \brief Find a valid full path for a proxy worker for a given executable name
>   * \param[in] file File name of proxy worker executable
> diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp
> index 2aa80b946704..89f5cb54687b 100644
> --- a/src/libcamera/proxy/ipa_proxy_linux.cpp
> +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp
> @@ -44,7 +44,7 @@ private:
>  };
>  
>  IPAProxyLinux::IPAProxyLinux(IPAModule *ipam)
> -	: proc_(nullptr), socket_(nullptr)
> +	: IPAProxy(ipam), proc_(nullptr), socket_(nullptr)
>  {
>  	LOG(IPAProxy, Debug)
>  		<< "initializing dummy proxy: loading IPA from "
> diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp
> index 368ccca1cf60..1ebb9b6a6c9d 100644
> --- a/src/libcamera/proxy/ipa_proxy_thread.cpp
> +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp
> @@ -73,7 +73,7 @@ private:
>  };
>  
>  IPAProxyThread::IPAProxyThread(IPAModule *ipam)
> -	: running_(false)
> +	: IPAProxy(ipam), running_(false)
>  {
>  	if (!ipam->load())
>  		return;
> 

-- 
Regards
--
Kieran