| Message ID | 20200427031713.14013-6-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, On Mon, Apr 27, 2020 at 06:17:07AM +0300, 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@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') Are these (datadir and sysconfdir) standard meson constructs ? Otherwise we should probably document them > > 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) + '"') > + Ah yes, those are etc/ and share/ I suppose > 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 "configuration file" > + * > + * 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 Do we have a place where we document the libcamera env variables ? > + * - 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. > + */ > + std::string root = utils::libcameraSourcePath(); This is currently not stripped out, right ? So we always get a path even if libcamera is istalled. Should this be guarded by isLibcameraInstalled() ? > + 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; Can this happen ? > + > + std::string confPath = dir + "/" + ipaName + "/" + name; > + ret = stat(confPath.c_str(), &statbuf); > + if (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG) > + return confPath; > + } > + Should we be loud here and complain ? > + 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 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi, one more question On Mon, Apr 27, 2020 at 06:17:07AM +0300, 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). How do we deal with permissions here ? I think it's up to distributions handle this though > > More locations, or extensions to the mechanism, may be implemented > later. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@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. > + */ > + 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 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, On Mon, Apr 27, 2020 at 10:32:05AM +0200, Jacopo Mondi wrote: > On Mon, Apr 27, 2020 at 06:17:07AM +0300, 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@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') > > Are these (datadir and sysconfdir) standard meson constructs ? > Otherwise we should probably document them See below ;-) > > > > 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) + '"') > > + > > Ah yes, those are etc/ and share/ I suppose > > > 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 > > "configuration file" > > > + * > > + * 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 > > Do we have a place where we document the libcamera env variables ? No. We should :-) > > + * - 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. > > + */ > > + std::string root = utils::libcameraSourcePath(); > > This is currently not stripped out, right ? So we always get a path > even if libcamera is istalled. Should this be guarded by > isLibcameraInstalled() ? The data isn't stripped from the libcamera.so binary, but there's such a check in utils::libcameraSourcePath() already, so we're safe. > > + 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; > > Can this happen ? I suppose not, as we're in control of this, so I'll remove it. > > + > > + std::string confPath = dir + "/" + ipaName + "/" + name; > > + ret = stat(confPath.c_str(), &statbuf); > > + if (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG) > > + return confPath; > > + } > > + > > Should we be loud here and complain ? Good idea. > > + 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;
Hi Jacopo, On Mon, Apr 27, 2020 at 11:18:59AM +0200, Jacopo Mondi wrote: > Hi, one more question > > On Mon, Apr 27, 2020 at 06:17:07AM +0300, 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). > > How do we deal with permissions here ? I think it's up to > distributions handle this though I considered it out of scope. If the file can't be open, the IPA will complain. Part of the reason is that the IPA could be isolated in a separate process with different file access permissions, so even if an access check passes here, it may not in the IPA. The other way around may be true as well. > > More locations, or extensions to the mechanism, may be implemented > > later. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@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. > > + */ > > + 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;
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@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@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; >
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@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@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; > >
Hi, On 27/04/2020 13:23, Laurent Pinchart wrote: > 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@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. Great, that will at least improve the git-documentation ;-) -- Kieran >> Other than that, >> >> Reviewed-by: Kieran Bingham <kieran.bingham@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; >>> >
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. + */ + 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;
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@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(-)