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

[Laurent Pinchart]

Hi Kieran,

On Mon, Apr 27, 2020 at 01:08:31PM +0100, Kieran Bingham wrote:
> 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. ;-)

I'll add this to the commit message.

When stored in the source tree, configuration files shall be located in
a 'data' subdirectory of their respective IPA directory.

> 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,

Laurent Pinchart