[{"id":4547,"web_url":"https://patchwork.libcamera.org/comment/4547/","msgid":"<20200427083205.5ho7kss3w4hg34cs@uno.localdomain>","date":"2020-04-27T08:32:05","subject":"Re: [libcamera-devel] [PATCH 05/11] libcamera: ipa_proxy: Provide\n\tsuport for IPA configuration files","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Mon, Apr 27, 2020 at 06:17:07AM +0300, Laurent Pinchart wrote:\n> IPA modules may require configuration files, which may be stored in\n> different locations in the file system. To standardize file locations\n> between all IPAs and pipeline handlers, provide a helper function to\n> locate a configuration file by searching in the following directories:\n>\n> - All directories specified in the LIBCAMERA_IPA_CONFIG_PATH environment\n>   variable ; or\n> - In the build tree if libcamera is not installed ; otherwise\n> - In standard system locations (etc and share directories).\n>\n> More locations, or extensions to the mechanism, may be implemented\n> later.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  src/ipa/meson.build                      |  6 ++\n>  src/libcamera/include/ipa_proxy.h        | 11 ++-\n>  src/libcamera/ipa_proxy.cpp              | 91 +++++++++++++++++++++++-\n>  src/libcamera/proxy/ipa_proxy_linux.cpp  |  2 +-\n>  src/libcamera/proxy/ipa_proxy_thread.cpp |  2 +-\n>  5 files changed, 105 insertions(+), 7 deletions(-)\n>\n> diff --git a/src/ipa/meson.build b/src/ipa/meson.build\n> index cb4e3ab3388f..145bf8105af7 100644\n> --- a/src/ipa/meson.build\n> +++ b/src/ipa/meson.build\n> @@ -1,10 +1,16 @@\n>  ipa_install_dir = join_paths(get_option('libdir'), 'libcamera')\n> +ipa_data_dir = join_paths(get_option('datadir'), 'libcamera', 'ipa')\n> +ipa_sysconf_dir = join_paths(get_option('sysconfdir'), 'libcamera', 'ipa')\n\nAre these (datadir and sysconfdir) standard meson constructs ?\nOtherwise we should probably document them\n\n>\n>  ipa_includes = [\n>      libcamera_includes,\n>      libcamera_internal_includes,\n>  ]\n>\n> +config_h.set('IPA_CONFIG_DIR',\n> +             '\"' + join_paths(get_option('prefix'), ipa_sysconf_dir) +\n> +             ':' + join_paths(get_option('prefix'), ipa_data_dir) + '\"')\n> +\n\nAh yes, those are etc/ and share/ I suppose\n\n>  config_h.set('IPA_MODULE_DIR',\n>               '\"' + join_paths(get_option('prefix'), ipa_install_dir) + '\"')\n>\n> diff --git a/src/libcamera/include/ipa_proxy.h b/src/libcamera/include/ipa_proxy.h\n> index e696551af39f..1111065b36a7 100644\n> --- a/src/libcamera/include/ipa_proxy.h\n> +++ b/src/libcamera/include/ipa_proxy.h\n> @@ -13,22 +13,27 @@\n>\n>  #include <ipa/ipa_interface.h>\n>\n> -#include \"ipa_module.h\"\n> -\n>  namespace libcamera {\n>\n> +class IPAModule;\n> +\n>  class IPAProxy : public IPAInterface\n>  {\n>  public:\n> -\tIPAProxy();\n> +\tIPAProxy(IPAModule *ipam);\n>  \t~IPAProxy();\n>\n>  \tbool isValid() const { return valid_; }\n>\n> +\tstd::string configurationFile(const std::string &file) const;\n> +\n>  protected:\n>  \tstd::string resolvePath(const std::string &file) const;\n>\n>  \tbool valid_;\n> +\n> +private:\n> +\tIPAModule *ipam_;\n>  };\n>\n>  class IPAProxyFactory\n> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n> index 43ac9afc25cb..ea69e63d6bb8 100644\n> --- a/src/libcamera/ipa_proxy.cpp\n> +++ b/src/libcamera/ipa_proxy.cpp\n> @@ -8,8 +8,11 @@\n>  #include \"ipa_proxy.h\"\n>\n>  #include <string.h>\n> +#include <sys/stat.h>\n> +#include <sys/types.h>\n>  #include <unistd.h>\n>\n> +#include \"ipa_module.h\"\n>  #include \"log.h\"\n>  #include \"utils.h\"\n>\n> @@ -34,12 +37,13 @@ LOG_DEFINE_CATEGORY(IPAProxy)\n>\n>  /**\n>   * \\brief Construct an IPAProxy instance\n> + * \\param[in] ipam The IPA module\n>   *\n>   * IPAProxy instances shall be constructed through the IPAProxyFactory::create()\n>   * method implemented by the respective factories.\n>   */\n> -IPAProxy::IPAProxy()\n> -\t: valid_(false)\n> +IPAProxy::IPAProxy(IPAModule *ipam)\n> +\t: valid_(false), ipam_(ipam)\n>  {\n>  }\n>\n> @@ -57,6 +61,89 @@ IPAProxy::~IPAProxy()\n>   * \\return True if the IPAProxy is valid, false otherwise\n>   */\n>\n> +/**\n> + * \\brief Retrieve the absolute path to an IPA configuration file\n> + * \\param[in] name The configuration name\n\n\"configuration file\"\n\n> + *\n> + * This function locates the configuration file for an IPA and returns its\n> + * absolute path. It searches the following directories, in order:\n> + *\n> + * - All directories specified in the colon-separated LIBCAMERA_IPA_CONFIG_PATH\n> + *   environment variable ; or\n\nDo we have a place where we document the libcamera env variables ?\n\n> + * - If libcamera is not installed, the src/ipa/ directory within the source\n> + *   tree ; otherwise\n> + * - The system sysconf (etc/libcamera/ipa) and the data (share/libcamera/ipa/)\n> + *   directories.\n> + *\n> + * The system directories are not searched if libcamera is not installed.\n> + *\n> + * Within each of those directories, the function looks for a subdirectory\n> + * named after the IPA module name, as reported in IPAModuleInfo::name, and for\n> + * a file named \\a name within that directory. The \\a name is IPA-specific.\n> + *\n> + * \\return The full path to the IPA configuration file, or an empty string if\n> + * no configuration file can be found\n> + */\n> +std::string IPAProxy::configurationFile(const std::string &name) const\n> +{\n> +\tstruct stat statbuf;\n> +\tint ret;\n> +\n> +\t/*\n> +\t * The IPA module name can be used as-is to build directory names as it\n> +\t * has been validated when loading the module.\n> +\t */\n> +\tstd::string ipaName = ipam_->info().name;\n> +\n> +\t/* Check the environment variable first. */\n> +\tconst char *confPaths = utils::secure_getenv(\"LIBCAMERA_IPA_CONFIG_PATH\");\n> +\tif (confPaths) {\n> +\t\tfor (const auto &dir : utils::split(confPaths, \":\")) {\n> +\t\t\tif (dir.empty())\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n> +\t\t\tret = stat(confPath.c_str(), &statbuf);\n> +\t\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> +\t\t\t\treturn confPath;\n> +\t\t}\n> +\t}\n> +\n> +\t/*\n> +\t * When libcamera is used before it is installed, load configuration\n> +\t * files from the source directory. The configuration files are then\n> +\t * located in the 'data' subdirectory of the corresponding IPA module.\n> +\t */\n> +\tstd::string root = utils::libcameraSourcePath();\n\nThis is currently not stripped out, right ? So we always get a path\neven if libcamera is istalled. Should this be guarded by\nisLibcameraInstalled() ?\n\n> +\tif (!root.empty()) {\n> +\t\tstd::string ipaConfDir = root + \"src/ipa/\" + ipaName + \"/data\";\n> +\n> +\t\tLOG(IPAProxy, Info)\n> +\t\t\t<< \"libcamera is not installed. Loading IPA configuration from '\"\n> +\t\t\t<< ipaConfDir << \"'\";\n> +\n> +\t\tstd::string confPath = ipaConfDir + \"/\" + name;\n> +\t\tret = stat(confPath.c_str(), &statbuf);\n> +\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> +\t\t\treturn confPath;\n> +\n> +\t\treturn std::string();\n> +\t}\n> +\n> +\t/* Else look in the system locations. */\n> +\tfor (const auto &dir : utils::split(IPA_CONFIG_DIR, \":\")) {\n> +\t\tif (dir.empty())\n> +\t\t\tcontinue;\n\nCan this happen ?\n\n> +\n> +\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n> +\t\tret = stat(confPath.c_str(), &statbuf);\n> +\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> +\t\t\treturn confPath;\n> +\t}\n> +\nShould we be loud here and complain ?\n\n> +\treturn std::string();\n> +}\n> +\n>  /**\n>   * \\brief Find a valid full path for a proxy worker for a given executable name\n>   * \\param[in] file File name of proxy worker executable\n> diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> index 2aa80b946704..89f5cb54687b 100644\n> --- a/src/libcamera/proxy/ipa_proxy_linux.cpp\n> +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> @@ -44,7 +44,7 @@ private:\n>  };\n>\n>  IPAProxyLinux::IPAProxyLinux(IPAModule *ipam)\n> -\t: proc_(nullptr), socket_(nullptr)\n> +\t: IPAProxy(ipam), proc_(nullptr), socket_(nullptr)\n>  {\n>  \tLOG(IPAProxy, Debug)\n>  \t\t<< \"initializing dummy proxy: loading IPA from \"\n> diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> index 368ccca1cf60..1ebb9b6a6c9d 100644\n> --- a/src/libcamera/proxy/ipa_proxy_thread.cpp\n> +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> @@ -73,7 +73,7 @@ private:\n>  };\n>\n>  IPAProxyThread::IPAProxyThread(IPAModule *ipam)\n> -\t: running_(false)\n> +\t: IPAProxy(ipam), running_(false)\n>  {\n>  \tif (!ipam->load())\n>  \t\treturn;\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EB5E3603F8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Apr 2020 10:28:55 +0200 (CEST)","from uno.localdomain (a-ur1-85.tin.it [212.216.150.148])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay4-d.mail.gandi.net (Postfix) with ESMTPSA id BC7ACE000C;\n\tMon, 27 Apr 2020 08:28:54 +0000 (UTC)"],"X-Originating-IP":"212.216.150.148","Date":"Mon, 27 Apr 2020 10:32:05 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200427083205.5ho7kss3w4hg34cs@uno.localdomain>","References":"<20200427031713.14013-1-laurent.pinchart@ideasonboard.com>\n\t<20200427031713.14013-6-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200427031713.14013-6-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 05/11] libcamera: ipa_proxy: Provide\n\tsuport for IPA configuration files","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 27 Apr 2020 08:28:56 -0000"}},{"id":4555,"web_url":"https://patchwork.libcamera.org/comment/4555/","msgid":"<20200427091859.nalpkcvwlxrtgkxv@uno.localdomain>","date":"2020-04-27T09:18:59","subject":"Re: [libcamera-devel] [PATCH 05/11] libcamera: ipa_proxy: Provide\n\tsuport for IPA configuration files","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi, one more question\n\nOn Mon, Apr 27, 2020 at 06:17:07AM +0300, Laurent Pinchart wrote:\n> IPA modules may require configuration files, which may be stored in\n> different locations in the file system. To standardize file locations\n> between all IPAs and pipeline handlers, provide a helper function to\n> locate a configuration file by searching in the following directories:\n>\n> - All directories specified in the LIBCAMERA_IPA_CONFIG_PATH environment\n>   variable ; or\n> - In the build tree if libcamera is not installed ; otherwise\n> - In standard system locations (etc and share directories).\n\nHow do we deal with permissions here ? I think it's up to\ndistributions handle this though\n\n>\n> More locations, or extensions to the mechanism, may be implemented\n> later.\n>\n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  src/ipa/meson.build                      |  6 ++\n>  src/libcamera/include/ipa_proxy.h        | 11 ++-\n>  src/libcamera/ipa_proxy.cpp              | 91 +++++++++++++++++++++++-\n>  src/libcamera/proxy/ipa_proxy_linux.cpp  |  2 +-\n>  src/libcamera/proxy/ipa_proxy_thread.cpp |  2 +-\n>  5 files changed, 105 insertions(+), 7 deletions(-)\n>\n> diff --git a/src/ipa/meson.build b/src/ipa/meson.build\n> index cb4e3ab3388f..145bf8105af7 100644\n> --- a/src/ipa/meson.build\n> +++ b/src/ipa/meson.build\n> @@ -1,10 +1,16 @@\n>  ipa_install_dir = join_paths(get_option('libdir'), 'libcamera')\n> +ipa_data_dir = join_paths(get_option('datadir'), 'libcamera', 'ipa')\n> +ipa_sysconf_dir = join_paths(get_option('sysconfdir'), 'libcamera', 'ipa')\n>\n>  ipa_includes = [\n>      libcamera_includes,\n>      libcamera_internal_includes,\n>  ]\n>\n> +config_h.set('IPA_CONFIG_DIR',\n> +             '\"' + join_paths(get_option('prefix'), ipa_sysconf_dir) +\n> +             ':' + join_paths(get_option('prefix'), ipa_data_dir) + '\"')\n> +\n>  config_h.set('IPA_MODULE_DIR',\n>               '\"' + join_paths(get_option('prefix'), ipa_install_dir) + '\"')\n>\n> diff --git a/src/libcamera/include/ipa_proxy.h b/src/libcamera/include/ipa_proxy.h\n> index e696551af39f..1111065b36a7 100644\n> --- a/src/libcamera/include/ipa_proxy.h\n> +++ b/src/libcamera/include/ipa_proxy.h\n> @@ -13,22 +13,27 @@\n>\n>  #include <ipa/ipa_interface.h>\n>\n> -#include \"ipa_module.h\"\n> -\n>  namespace libcamera {\n>\n> +class IPAModule;\n> +\n>  class IPAProxy : public IPAInterface\n>  {\n>  public:\n> -\tIPAProxy();\n> +\tIPAProxy(IPAModule *ipam);\n>  \t~IPAProxy();\n>\n>  \tbool isValid() const { return valid_; }\n>\n> +\tstd::string configurationFile(const std::string &file) const;\n> +\n>  protected:\n>  \tstd::string resolvePath(const std::string &file) const;\n>\n>  \tbool valid_;\n> +\n> +private:\n> +\tIPAModule *ipam_;\n>  };\n>\n>  class IPAProxyFactory\n> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n> index 43ac9afc25cb..ea69e63d6bb8 100644\n> --- a/src/libcamera/ipa_proxy.cpp\n> +++ b/src/libcamera/ipa_proxy.cpp\n> @@ -8,8 +8,11 @@\n>  #include \"ipa_proxy.h\"\n>\n>  #include <string.h>\n> +#include <sys/stat.h>\n> +#include <sys/types.h>\n>  #include <unistd.h>\n>\n> +#include \"ipa_module.h\"\n>  #include \"log.h\"\n>  #include \"utils.h\"\n>\n> @@ -34,12 +37,13 @@ LOG_DEFINE_CATEGORY(IPAProxy)\n>\n>  /**\n>   * \\brief Construct an IPAProxy instance\n> + * \\param[in] ipam The IPA module\n>   *\n>   * IPAProxy instances shall be constructed through the IPAProxyFactory::create()\n>   * method implemented by the respective factories.\n>   */\n> -IPAProxy::IPAProxy()\n> -\t: valid_(false)\n> +IPAProxy::IPAProxy(IPAModule *ipam)\n> +\t: valid_(false), ipam_(ipam)\n>  {\n>  }\n>\n> @@ -57,6 +61,89 @@ IPAProxy::~IPAProxy()\n>   * \\return True if the IPAProxy is valid, false otherwise\n>   */\n>\n> +/**\n> + * \\brief Retrieve the absolute path to an IPA configuration file\n> + * \\param[in] name The configuration name\n> + *\n> + * This function locates the configuration file for an IPA and returns its\n> + * absolute path. It searches the following directories, in order:\n> + *\n> + * - All directories specified in the colon-separated LIBCAMERA_IPA_CONFIG_PATH\n> + *   environment variable ; or\n> + * - If libcamera is not installed, the src/ipa/ directory within the source\n> + *   tree ; otherwise\n> + * - The system sysconf (etc/libcamera/ipa) and the data (share/libcamera/ipa/)\n> + *   directories.\n> + *\n> + * The system directories are not searched if libcamera is not installed.\n> + *\n> + * Within each of those directories, the function looks for a subdirectory\n> + * named after the IPA module name, as reported in IPAModuleInfo::name, and for\n> + * a file named \\a name within that directory. The \\a name is IPA-specific.\n> + *\n> + * \\return The full path to the IPA configuration file, or an empty string if\n> + * no configuration file can be found\n> + */\n> +std::string IPAProxy::configurationFile(const std::string &name) const\n> +{\n> +\tstruct stat statbuf;\n> +\tint ret;\n> +\n> +\t/*\n> +\t * The IPA module name can be used as-is to build directory names as it\n> +\t * has been validated when loading the module.\n> +\t */\n> +\tstd::string ipaName = ipam_->info().name;\n> +\n> +\t/* Check the environment variable first. */\n> +\tconst char *confPaths = utils::secure_getenv(\"LIBCAMERA_IPA_CONFIG_PATH\");\n> +\tif (confPaths) {\n> +\t\tfor (const auto &dir : utils::split(confPaths, \":\")) {\n> +\t\t\tif (dir.empty())\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n> +\t\t\tret = stat(confPath.c_str(), &statbuf);\n> +\t\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> +\t\t\t\treturn confPath;\n> +\t\t}\n> +\t}\n> +\n> +\t/*\n> +\t * When libcamera is used before it is installed, load configuration\n> +\t * files from the source directory. The configuration files are then\n> +\t * located in the 'data' subdirectory of the corresponding IPA module.\n> +\t */\n> +\tstd::string root = utils::libcameraSourcePath();\n> +\tif (!root.empty()) {\n> +\t\tstd::string ipaConfDir = root + \"src/ipa/\" + ipaName + \"/data\";\n> +\n> +\t\tLOG(IPAProxy, Info)\n> +\t\t\t<< \"libcamera is not installed. Loading IPA configuration from '\"\n> +\t\t\t<< ipaConfDir << \"'\";\n> +\n> +\t\tstd::string confPath = ipaConfDir + \"/\" + name;\n> +\t\tret = stat(confPath.c_str(), &statbuf);\n> +\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> +\t\t\treturn confPath;\n> +\n> +\t\treturn std::string();\n> +\t}\n> +\n> +\t/* Else look in the system locations. */\n> +\tfor (const auto &dir : utils::split(IPA_CONFIG_DIR, \":\")) {\n> +\t\tif (dir.empty())\n> +\t\t\tcontinue;\n> +\n> +\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n> +\t\tret = stat(confPath.c_str(), &statbuf);\n> +\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> +\t\t\treturn confPath;\n> +\t}\n> +\n> +\treturn std::string();\n> +}\n> +\n>  /**\n>   * \\brief Find a valid full path for a proxy worker for a given executable name\n>   * \\param[in] file File name of proxy worker executable\n> diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> index 2aa80b946704..89f5cb54687b 100644\n> --- a/src/libcamera/proxy/ipa_proxy_linux.cpp\n> +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> @@ -44,7 +44,7 @@ private:\n>  };\n>\n>  IPAProxyLinux::IPAProxyLinux(IPAModule *ipam)\n> -\t: proc_(nullptr), socket_(nullptr)\n> +\t: IPAProxy(ipam), proc_(nullptr), socket_(nullptr)\n>  {\n>  \tLOG(IPAProxy, Debug)\n>  \t\t<< \"initializing dummy proxy: loading IPA from \"\n> diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> index 368ccca1cf60..1ebb9b6a6c9d 100644\n> --- a/src/libcamera/proxy/ipa_proxy_thread.cpp\n> +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> @@ -73,7 +73,7 @@ private:\n>  };\n>\n>  IPAProxyThread::IPAProxyThread(IPAModule *ipam)\n> -\t: running_(false)\n> +\t: IPAProxy(ipam), running_(false)\n>  {\n>  \tif (!ipam->load())\n>  \t\treturn;\n> --\n> Regards,\n>\n> Laurent Pinchart\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DA2E0603F8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Apr 2020 11:15:49 +0200 (CEST)","from uno.localdomain (a-ur1-85.tin.it [212.216.150.148])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id ED1CD1C0016;\n\tMon, 27 Apr 2020 09:15:48 +0000 (UTC)"],"X-Originating-IP":"212.216.150.148","Date":"Mon, 27 Apr 2020 11:18:59 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200427091859.nalpkcvwlxrtgkxv@uno.localdomain>","References":"<20200427031713.14013-1-laurent.pinchart@ideasonboard.com>\n\t<20200427031713.14013-6-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200427031713.14013-6-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 05/11] libcamera: ipa_proxy: Provide\n\tsuport for IPA configuration files","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 27 Apr 2020 09:15:50 -0000"}},{"id":4563,"web_url":"https://patchwork.libcamera.org/comment/4563/","msgid":"<20200427105500.GD5849@pendragon.ideasonboard.com>","date":"2020-04-27T10:55:00","subject":"Re: [libcamera-devel] [PATCH 05/11] libcamera: ipa_proxy: Provide\n\tsuport for IPA configuration files","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Apr 27, 2020 at 10:32:05AM +0200, Jacopo Mondi wrote:\n> On Mon, Apr 27, 2020 at 06:17:07AM +0300, Laurent Pinchart wrote:\n> > IPA modules may require configuration files, which may be stored in\n> > different locations in the file system. To standardize file locations\n> > between all IPAs and pipeline handlers, provide a helper function to\n> > locate a configuration file by searching in the following directories:\n> >\n> > - All directories specified in the LIBCAMERA_IPA_CONFIG_PATH environment\n> >   variable ; or\n> > - In the build tree if libcamera is not installed ; otherwise\n> > - In standard system locations (etc and share directories).\n> >\n> > More locations, or extensions to the mechanism, may be implemented\n> > later.\n> >\n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  src/ipa/meson.build                      |  6 ++\n> >  src/libcamera/include/ipa_proxy.h        | 11 ++-\n> >  src/libcamera/ipa_proxy.cpp              | 91 +++++++++++++++++++++++-\n> >  src/libcamera/proxy/ipa_proxy_linux.cpp  |  2 +-\n> >  src/libcamera/proxy/ipa_proxy_thread.cpp |  2 +-\n> >  5 files changed, 105 insertions(+), 7 deletions(-)\n> >\n> > diff --git a/src/ipa/meson.build b/src/ipa/meson.build\n> > index cb4e3ab3388f..145bf8105af7 100644\n> > --- a/src/ipa/meson.build\n> > +++ b/src/ipa/meson.build\n> > @@ -1,10 +1,16 @@\n> >  ipa_install_dir = join_paths(get_option('libdir'), 'libcamera')\n> > +ipa_data_dir = join_paths(get_option('datadir'), 'libcamera', 'ipa')\n> > +ipa_sysconf_dir = join_paths(get_option('sysconfdir'), 'libcamera', 'ipa')\n> \n> Are these (datadir and sysconfdir) standard meson constructs ?\n> Otherwise we should probably document them\n\nSee below ;-)\n\n> >\n> >  ipa_includes = [\n> >      libcamera_includes,\n> >      libcamera_internal_includes,\n> >  ]\n> >\n> > +config_h.set('IPA_CONFIG_DIR',\n> > +             '\"' + join_paths(get_option('prefix'), ipa_sysconf_dir) +\n> > +             ':' + join_paths(get_option('prefix'), ipa_data_dir) + '\"')\n> > +\n> \n> Ah yes, those are etc/ and share/ I suppose\n> \n> >  config_h.set('IPA_MODULE_DIR',\n> >               '\"' + join_paths(get_option('prefix'), ipa_install_dir) + '\"')\n> >\n> > diff --git a/src/libcamera/include/ipa_proxy.h b/src/libcamera/include/ipa_proxy.h\n> > index e696551af39f..1111065b36a7 100644\n> > --- a/src/libcamera/include/ipa_proxy.h\n> > +++ b/src/libcamera/include/ipa_proxy.h\n> > @@ -13,22 +13,27 @@\n> >\n> >  #include <ipa/ipa_interface.h>\n> >\n> > -#include \"ipa_module.h\"\n> > -\n> >  namespace libcamera {\n> >\n> > +class IPAModule;\n> > +\n> >  class IPAProxy : public IPAInterface\n> >  {\n> >  public:\n> > -\tIPAProxy();\n> > +\tIPAProxy(IPAModule *ipam);\n> >  \t~IPAProxy();\n> >\n> >  \tbool isValid() const { return valid_; }\n> >\n> > +\tstd::string configurationFile(const std::string &file) const;\n> > +\n> >  protected:\n> >  \tstd::string resolvePath(const std::string &file) const;\n> >\n> >  \tbool valid_;\n> > +\n> > +private:\n> > +\tIPAModule *ipam_;\n> >  };\n> >\n> >  class IPAProxyFactory\n> > diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n> > index 43ac9afc25cb..ea69e63d6bb8 100644\n> > --- a/src/libcamera/ipa_proxy.cpp\n> > +++ b/src/libcamera/ipa_proxy.cpp\n> > @@ -8,8 +8,11 @@\n> >  #include \"ipa_proxy.h\"\n> >\n> >  #include <string.h>\n> > +#include <sys/stat.h>\n> > +#include <sys/types.h>\n> >  #include <unistd.h>\n> >\n> > +#include \"ipa_module.h\"\n> >  #include \"log.h\"\n> >  #include \"utils.h\"\n> >\n> > @@ -34,12 +37,13 @@ LOG_DEFINE_CATEGORY(IPAProxy)\n> >\n> >  /**\n> >   * \\brief Construct an IPAProxy instance\n> > + * \\param[in] ipam The IPA module\n> >   *\n> >   * IPAProxy instances shall be constructed through the IPAProxyFactory::create()\n> >   * method implemented by the respective factories.\n> >   */\n> > -IPAProxy::IPAProxy()\n> > -\t: valid_(false)\n> > +IPAProxy::IPAProxy(IPAModule *ipam)\n> > +\t: valid_(false), ipam_(ipam)\n> >  {\n> >  }\n> >\n> > @@ -57,6 +61,89 @@ IPAProxy::~IPAProxy()\n> >   * \\return True if the IPAProxy is valid, false otherwise\n> >   */\n> >\n> > +/**\n> > + * \\brief Retrieve the absolute path to an IPA configuration file\n> > + * \\param[in] name The configuration name\n> \n> \"configuration file\"\n> \n> > + *\n> > + * This function locates the configuration file for an IPA and returns its\n> > + * absolute path. It searches the following directories, in order:\n> > + *\n> > + * - All directories specified in the colon-separated LIBCAMERA_IPA_CONFIG_PATH\n> > + *   environment variable ; or\n> \n> Do we have a place where we document the libcamera env variables ?\n\nNo. We should :-)\n\n> > + * - If libcamera is not installed, the src/ipa/ directory within the source\n> > + *   tree ; otherwise\n> > + * - The system sysconf (etc/libcamera/ipa) and the data (share/libcamera/ipa/)\n> > + *   directories.\n> > + *\n> > + * The system directories are not searched if libcamera is not installed.\n> > + *\n> > + * Within each of those directories, the function looks for a subdirectory\n> > + * named after the IPA module name, as reported in IPAModuleInfo::name, and for\n> > + * a file named \\a name within that directory. The \\a name is IPA-specific.\n> > + *\n> > + * \\return The full path to the IPA configuration file, or an empty string if\n> > + * no configuration file can be found\n> > + */\n> > +std::string IPAProxy::configurationFile(const std::string &name) const\n> > +{\n> > +\tstruct stat statbuf;\n> > +\tint ret;\n> > +\n> > +\t/*\n> > +\t * The IPA module name can be used as-is to build directory names as it\n> > +\t * has been validated when loading the module.\n> > +\t */\n> > +\tstd::string ipaName = ipam_->info().name;\n> > +\n> > +\t/* Check the environment variable first. */\n> > +\tconst char *confPaths = utils::secure_getenv(\"LIBCAMERA_IPA_CONFIG_PATH\");\n> > +\tif (confPaths) {\n> > +\t\tfor (const auto &dir : utils::split(confPaths, \":\")) {\n> > +\t\t\tif (dir.empty())\n> > +\t\t\t\tcontinue;\n> > +\n> > +\t\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n> > +\t\t\tret = stat(confPath.c_str(), &statbuf);\n> > +\t\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> > +\t\t\t\treturn confPath;\n> > +\t\t}\n> > +\t}\n> > +\n> > +\t/*\n> > +\t * When libcamera is used before it is installed, load configuration\n> > +\t * files from the source directory. The configuration files are then\n> > +\t * located in the 'data' subdirectory of the corresponding IPA module.\n> > +\t */\n> > +\tstd::string root = utils::libcameraSourcePath();\n> \n> This is currently not stripped out, right ? So we always get a path\n> even if libcamera is istalled. Should this be guarded by\n> isLibcameraInstalled() ?\n\nThe data isn't stripped from the libcamera.so binary, but there's such a\ncheck in utils::libcameraSourcePath() already, so we're safe.\n\n> > +\tif (!root.empty()) {\n> > +\t\tstd::string ipaConfDir = root + \"src/ipa/\" + ipaName + \"/data\";\n> > +\n> > +\t\tLOG(IPAProxy, Info)\n> > +\t\t\t<< \"libcamera is not installed. Loading IPA configuration from '\"\n> > +\t\t\t<< ipaConfDir << \"'\";\n> > +\n> > +\t\tstd::string confPath = ipaConfDir + \"/\" + name;\n> > +\t\tret = stat(confPath.c_str(), &statbuf);\n> > +\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> > +\t\t\treturn confPath;\n> > +\n> > +\t\treturn std::string();\n> > +\t}\n> > +\n> > +\t/* Else look in the system locations. */\n> > +\tfor (const auto &dir : utils::split(IPA_CONFIG_DIR, \":\")) {\n> > +\t\tif (dir.empty())\n> > +\t\t\tcontinue;\n> \n> Can this happen ?\n\nI suppose not, as we're in control of this, so I'll remove it.\n\n> > +\n> > +\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n> > +\t\tret = stat(confPath.c_str(), &statbuf);\n> > +\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> > +\t\t\treturn confPath;\n> > +\t}\n> > +\n>\n> Should we be loud here and complain ?\n\nGood idea.\n\n> > +\treturn std::string();\n> > +}\n> > +\n> >  /**\n> >   * \\brief Find a valid full path for a proxy worker for a given executable name\n> >   * \\param[in] file File name of proxy worker executable\n> > diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > index 2aa80b946704..89f5cb54687b 100644\n> > --- a/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > @@ -44,7 +44,7 @@ private:\n> >  };\n> >\n> >  IPAProxyLinux::IPAProxyLinux(IPAModule *ipam)\n> > -\t: proc_(nullptr), socket_(nullptr)\n> > +\t: IPAProxy(ipam), proc_(nullptr), socket_(nullptr)\n> >  {\n> >  \tLOG(IPAProxy, Debug)\n> >  \t\t<< \"initializing dummy proxy: loading IPA from \"\n> > diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> > index 368ccca1cf60..1ebb9b6a6c9d 100644\n> > --- a/src/libcamera/proxy/ipa_proxy_thread.cpp\n> > +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> > @@ -73,7 +73,7 @@ private:\n> >  };\n> >\n> >  IPAProxyThread::IPAProxyThread(IPAModule *ipam)\n> > -\t: running_(false)\n> > +\t: IPAProxy(ipam), running_(false)\n> >  {\n> >  \tif (!ipam->load())\n> >  \t\treturn;","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 30C46603F9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Apr 2020 12:55:16 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A7ECF72C;\n\tMon, 27 Apr 2020 12:55:15 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"iFgIy7Xc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1587984915;\n\tbh=AkJO/Te2108MzuTqrFXYwAo1fahzLeet7SD1EudVNJg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=iFgIy7Xc6yGF6vOu4FH4qOyXDWgye0Lf95w+fmq3u5G29OULEj7kqS/VMGB8zUoy5\n\tqeTjUBf54pLve86+kGkqwhCgMb73VqLwCj5y0HyWhHgDJ6UVIS7oIGO22OEeiGOeRA\n\tAhqY2eLOUynCOxDjV59Lh4Gd7C/xSpBJYfqTr4T8=","Date":"Mon, 27 Apr 2020 13:55:00 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200427105500.GD5849@pendragon.ideasonboard.com>","References":"<20200427031713.14013-1-laurent.pinchart@ideasonboard.com>\n\t<20200427031713.14013-6-laurent.pinchart@ideasonboard.com>\n\t<20200427083205.5ho7kss3w4hg34cs@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200427083205.5ho7kss3w4hg34cs@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 05/11] libcamera: ipa_proxy: Provide\n\tsuport for IPA configuration files","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 27 Apr 2020 10:55:16 -0000"}},{"id":4564,"web_url":"https://patchwork.libcamera.org/comment/4564/","msgid":"<20200427105640.GE5849@pendragon.ideasonboard.com>","date":"2020-04-27T10:56:40","subject":"Re: [libcamera-devel] [PATCH 05/11] libcamera: ipa_proxy: Provide\n\tsuport for IPA configuration files","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Apr 27, 2020 at 11:18:59AM +0200, Jacopo Mondi wrote:\n> Hi, one more question\n> \n> On Mon, Apr 27, 2020 at 06:17:07AM +0300, Laurent Pinchart wrote:\n> > IPA modules may require configuration files, which may be stored in\n> > different locations in the file system. To standardize file locations\n> > between all IPAs and pipeline handlers, provide a helper function to\n> > locate a configuration file by searching in the following directories:\n> >\n> > - All directories specified in the LIBCAMERA_IPA_CONFIG_PATH environment\n> >   variable ; or\n> > - In the build tree if libcamera is not installed ; otherwise\n> > - In standard system locations (etc and share directories).\n> \n> How do we deal with permissions here ? I think it's up to\n> distributions handle this though\n\nI considered it out of scope. If the file can't be open, the IPA will\ncomplain. Part of the reason is that the IPA could be isolated in a\nseparate process with different file access permissions, so even if an\naccess check passes here, it may not in the IPA. The other way around\nmay be true as well.\n\n> > More locations, or extensions to the mechanism, may be implemented\n> > later.\n> >\n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  src/ipa/meson.build                      |  6 ++\n> >  src/libcamera/include/ipa_proxy.h        | 11 ++-\n> >  src/libcamera/ipa_proxy.cpp              | 91 +++++++++++++++++++++++-\n> >  src/libcamera/proxy/ipa_proxy_linux.cpp  |  2 +-\n> >  src/libcamera/proxy/ipa_proxy_thread.cpp |  2 +-\n> >  5 files changed, 105 insertions(+), 7 deletions(-)\n> >\n> > diff --git a/src/ipa/meson.build b/src/ipa/meson.build\n> > index cb4e3ab3388f..145bf8105af7 100644\n> > --- a/src/ipa/meson.build\n> > +++ b/src/ipa/meson.build\n> > @@ -1,10 +1,16 @@\n> >  ipa_install_dir = join_paths(get_option('libdir'), 'libcamera')\n> > +ipa_data_dir = join_paths(get_option('datadir'), 'libcamera', 'ipa')\n> > +ipa_sysconf_dir = join_paths(get_option('sysconfdir'), 'libcamera', 'ipa')\n> >\n> >  ipa_includes = [\n> >      libcamera_includes,\n> >      libcamera_internal_includes,\n> >  ]\n> >\n> > +config_h.set('IPA_CONFIG_DIR',\n> > +             '\"' + join_paths(get_option('prefix'), ipa_sysconf_dir) +\n> > +             ':' + join_paths(get_option('prefix'), ipa_data_dir) + '\"')\n> > +\n> >  config_h.set('IPA_MODULE_DIR',\n> >               '\"' + join_paths(get_option('prefix'), ipa_install_dir) + '\"')\n> >\n> > diff --git a/src/libcamera/include/ipa_proxy.h b/src/libcamera/include/ipa_proxy.h\n> > index e696551af39f..1111065b36a7 100644\n> > --- a/src/libcamera/include/ipa_proxy.h\n> > +++ b/src/libcamera/include/ipa_proxy.h\n> > @@ -13,22 +13,27 @@\n> >\n> >  #include <ipa/ipa_interface.h>\n> >\n> > -#include \"ipa_module.h\"\n> > -\n> >  namespace libcamera {\n> >\n> > +class IPAModule;\n> > +\n> >  class IPAProxy : public IPAInterface\n> >  {\n> >  public:\n> > -\tIPAProxy();\n> > +\tIPAProxy(IPAModule *ipam);\n> >  \t~IPAProxy();\n> >\n> >  \tbool isValid() const { return valid_; }\n> >\n> > +\tstd::string configurationFile(const std::string &file) const;\n> > +\n> >  protected:\n> >  \tstd::string resolvePath(const std::string &file) const;\n> >\n> >  \tbool valid_;\n> > +\n> > +private:\n> > +\tIPAModule *ipam_;\n> >  };\n> >\n> >  class IPAProxyFactory\n> > diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n> > index 43ac9afc25cb..ea69e63d6bb8 100644\n> > --- a/src/libcamera/ipa_proxy.cpp\n> > +++ b/src/libcamera/ipa_proxy.cpp\n> > @@ -8,8 +8,11 @@\n> >  #include \"ipa_proxy.h\"\n> >\n> >  #include <string.h>\n> > +#include <sys/stat.h>\n> > +#include <sys/types.h>\n> >  #include <unistd.h>\n> >\n> > +#include \"ipa_module.h\"\n> >  #include \"log.h\"\n> >  #include \"utils.h\"\n> >\n> > @@ -34,12 +37,13 @@ LOG_DEFINE_CATEGORY(IPAProxy)\n> >\n> >  /**\n> >   * \\brief Construct an IPAProxy instance\n> > + * \\param[in] ipam The IPA module\n> >   *\n> >   * IPAProxy instances shall be constructed through the IPAProxyFactory::create()\n> >   * method implemented by the respective factories.\n> >   */\n> > -IPAProxy::IPAProxy()\n> > -\t: valid_(false)\n> > +IPAProxy::IPAProxy(IPAModule *ipam)\n> > +\t: valid_(false), ipam_(ipam)\n> >  {\n> >  }\n> >\n> > @@ -57,6 +61,89 @@ IPAProxy::~IPAProxy()\n> >   * \\return True if the IPAProxy is valid, false otherwise\n> >   */\n> >\n> > +/**\n> > + * \\brief Retrieve the absolute path to an IPA configuration file\n> > + * \\param[in] name The configuration name\n> > + *\n> > + * This function locates the configuration file for an IPA and returns its\n> > + * absolute path. It searches the following directories, in order:\n> > + *\n> > + * - All directories specified in the colon-separated LIBCAMERA_IPA_CONFIG_PATH\n> > + *   environment variable ; or\n> > + * - If libcamera is not installed, the src/ipa/ directory within the source\n> > + *   tree ; otherwise\n> > + * - The system sysconf (etc/libcamera/ipa) and the data (share/libcamera/ipa/)\n> > + *   directories.\n> > + *\n> > + * The system directories are not searched if libcamera is not installed.\n> > + *\n> > + * Within each of those directories, the function looks for a subdirectory\n> > + * named after the IPA module name, as reported in IPAModuleInfo::name, and for\n> > + * a file named \\a name within that directory. The \\a name is IPA-specific.\n> > + *\n> > + * \\return The full path to the IPA configuration file, or an empty string if\n> > + * no configuration file can be found\n> > + */\n> > +std::string IPAProxy::configurationFile(const std::string &name) const\n> > +{\n> > +\tstruct stat statbuf;\n> > +\tint ret;\n> > +\n> > +\t/*\n> > +\t * The IPA module name can be used as-is to build directory names as it\n> > +\t * has been validated when loading the module.\n> > +\t */\n> > +\tstd::string ipaName = ipam_->info().name;\n> > +\n> > +\t/* Check the environment variable first. */\n> > +\tconst char *confPaths = utils::secure_getenv(\"LIBCAMERA_IPA_CONFIG_PATH\");\n> > +\tif (confPaths) {\n> > +\t\tfor (const auto &dir : utils::split(confPaths, \":\")) {\n> > +\t\t\tif (dir.empty())\n> > +\t\t\t\tcontinue;\n> > +\n> > +\t\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n> > +\t\t\tret = stat(confPath.c_str(), &statbuf);\n> > +\t\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> > +\t\t\t\treturn confPath;\n> > +\t\t}\n> > +\t}\n> > +\n> > +\t/*\n> > +\t * When libcamera is used before it is installed, load configuration\n> > +\t * files from the source directory. The configuration files are then\n> > +\t * located in the 'data' subdirectory of the corresponding IPA module.\n> > +\t */\n> > +\tstd::string root = utils::libcameraSourcePath();\n> > +\tif (!root.empty()) {\n> > +\t\tstd::string ipaConfDir = root + \"src/ipa/\" + ipaName + \"/data\";\n> > +\n> > +\t\tLOG(IPAProxy, Info)\n> > +\t\t\t<< \"libcamera is not installed. Loading IPA configuration from '\"\n> > +\t\t\t<< ipaConfDir << \"'\";\n> > +\n> > +\t\tstd::string confPath = ipaConfDir + \"/\" + name;\n> > +\t\tret = stat(confPath.c_str(), &statbuf);\n> > +\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> > +\t\t\treturn confPath;\n> > +\n> > +\t\treturn std::string();\n> > +\t}\n> > +\n> > +\t/* Else look in the system locations. */\n> > +\tfor (const auto &dir : utils::split(IPA_CONFIG_DIR, \":\")) {\n> > +\t\tif (dir.empty())\n> > +\t\t\tcontinue;\n> > +\n> > +\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n> > +\t\tret = stat(confPath.c_str(), &statbuf);\n> > +\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> > +\t\t\treturn confPath;\n> > +\t}\n> > +\n> > +\treturn std::string();\n> > +}\n> > +\n> >  /**\n> >   * \\brief Find a valid full path for a proxy worker for a given executable name\n> >   * \\param[in] file File name of proxy worker executable\n> > diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > index 2aa80b946704..89f5cb54687b 100644\n> > --- a/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > @@ -44,7 +44,7 @@ private:\n> >  };\n> >\n> >  IPAProxyLinux::IPAProxyLinux(IPAModule *ipam)\n> > -\t: proc_(nullptr), socket_(nullptr)\n> > +\t: IPAProxy(ipam), proc_(nullptr), socket_(nullptr)\n> >  {\n> >  \tLOG(IPAProxy, Debug)\n> >  \t\t<< \"initializing dummy proxy: loading IPA from \"\n> > diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> > index 368ccca1cf60..1ebb9b6a6c9d 100644\n> > --- a/src/libcamera/proxy/ipa_proxy_thread.cpp\n> > +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> > @@ -73,7 +73,7 @@ private:\n> >  };\n> >\n> >  IPAProxyThread::IPAProxyThread(IPAModule *ipam)\n> > -\t: running_(false)\n> > +\t: IPAProxy(ipam), running_(false)\n> >  {\n> >  \tif (!ipam->load())\n> >  \t\treturn;","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3C2E2603F9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Apr 2020 12:56:56 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B63EF72C;\n\tMon, 27 Apr 2020 12:56:55 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"j0d7kwTI\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1587985015;\n\tbh=fRNkf7UzbnaaPpevzJt0cKa6fN8dgyDzGt+LBqJpiyk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=j0d7kwTIGBATLwvYYzwbvmp7JyAa6Ayt0vjjd14Kq2c8GfVWxwAt38lUi2CHUS8yE\n\ts0m/BzJcGWHlWsFHUhaKkQH+1eYpOgfoWYo23RRNlkMWtGwCJIpmCkTBTDmhuPaR3i\n\tWCfCyRpZ1gGqWRJy/EzDPC8PEUH4Bk/ZwOWPWxYk=","Date":"Mon, 27 Apr 2020 13:56:40 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200427105640.GE5849@pendragon.ideasonboard.com>","References":"<20200427031713.14013-1-laurent.pinchart@ideasonboard.com>\n\t<20200427031713.14013-6-laurent.pinchart@ideasonboard.com>\n\t<20200427091859.nalpkcvwlxrtgkxv@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200427091859.nalpkcvwlxrtgkxv@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 05/11] libcamera: ipa_proxy: Provide\n\tsuport for IPA configuration files","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 27 Apr 2020 10:56:56 -0000"}},{"id":4568,"web_url":"https://patchwork.libcamera.org/comment/4568/","msgid":"<02e5faae-92b8-8af9-ea7d-8b820dbb700c@ideasonboard.com>","date":"2020-04-27T12:08:31","subject":"Re: [libcamera-devel] [PATCH 05/11] libcamera: ipa_proxy: Provide\n\tsuport for IPA configuration files","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 27/04/2020 04:17, Laurent Pinchart wrote:\n> IPA modules may require configuration files, which may be stored in\n> different locations in the file system. To standardize file locations\n> between all IPAs and pipeline handlers, provide a helper function to\n> locate a configuration file by searching in the following directories:\n> \n> - All directories specified in the LIBCAMERA_IPA_CONFIG_PATH environment\n>   variable ; or\n> - In the build tree if libcamera is not installed ; otherwise\n> - In standard system locations (etc and share directories).\n> \n> More locations, or extensions to the mechanism, may be implemented\n> later.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  src/ipa/meson.build                      |  6 ++\n>  src/libcamera/include/ipa_proxy.h        | 11 ++-\n>  src/libcamera/ipa_proxy.cpp              | 91 +++++++++++++++++++++++-\n>  src/libcamera/proxy/ipa_proxy_linux.cpp  |  2 +-\n>  src/libcamera/proxy/ipa_proxy_thread.cpp |  2 +-\n>  5 files changed, 105 insertions(+), 7 deletions(-)\n> \n> diff --git a/src/ipa/meson.build b/src/ipa/meson.build\n> index cb4e3ab3388f..145bf8105af7 100644\n> --- a/src/ipa/meson.build\n> +++ b/src/ipa/meson.build\n> @@ -1,10 +1,16 @@\n>  ipa_install_dir = join_paths(get_option('libdir'), 'libcamera')\n> +ipa_data_dir = join_paths(get_option('datadir'), 'libcamera', 'ipa')\n> +ipa_sysconf_dir = join_paths(get_option('sysconfdir'), 'libcamera', 'ipa')\n>  \n>  ipa_includes = [\n>      libcamera_includes,\n>      libcamera_internal_includes,\n>  ]\n>  \n> +config_h.set('IPA_CONFIG_DIR',\n> +             '\"' + join_paths(get_option('prefix'), ipa_sysconf_dir) +\n> +             ':' + join_paths(get_option('prefix'), ipa_data_dir) + '\"')\n> +\n>  config_h.set('IPA_MODULE_DIR',\n>               '\"' + join_paths(get_option('prefix'), ipa_install_dir) + '\"')\n>  \n> diff --git a/src/libcamera/include/ipa_proxy.h b/src/libcamera/include/ipa_proxy.h\n> index e696551af39f..1111065b36a7 100644\n> --- a/src/libcamera/include/ipa_proxy.h\n> +++ b/src/libcamera/include/ipa_proxy.h\n> @@ -13,22 +13,27 @@\n>  \n>  #include <ipa/ipa_interface.h>\n>  \n> -#include \"ipa_module.h\"\n> -\n>  namespace libcamera {\n>  \n> +class IPAModule;\n> +\n>  class IPAProxy : public IPAInterface\n>  {\n>  public:\n> -\tIPAProxy();\n> +\tIPAProxy(IPAModule *ipam);\n>  \t~IPAProxy();\n>  \n>  \tbool isValid() const { return valid_; }\n>  \n> +\tstd::string configurationFile(const std::string &file) const;\n> +\n>  protected:\n>  \tstd::string resolvePath(const std::string &file) const;\n>  \n>  \tbool valid_;\n> +\n> +private:\n> +\tIPAModule *ipam_;\n>  };\n>  \n>  class IPAProxyFactory\n> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n> index 43ac9afc25cb..ea69e63d6bb8 100644\n> --- a/src/libcamera/ipa_proxy.cpp\n> +++ b/src/libcamera/ipa_proxy.cpp\n> @@ -8,8 +8,11 @@\n>  #include \"ipa_proxy.h\"\n>  \n>  #include <string.h>\n> +#include <sys/stat.h>\n> +#include <sys/types.h>\n>  #include <unistd.h>\n>  \n> +#include \"ipa_module.h\"\n>  #include \"log.h\"\n>  #include \"utils.h\"\n>  \n> @@ -34,12 +37,13 @@ LOG_DEFINE_CATEGORY(IPAProxy)\n>  \n>  /**\n>   * \\brief Construct an IPAProxy instance\n> + * \\param[in] ipam The IPA module\n>   *\n>   * IPAProxy instances shall be constructed through the IPAProxyFactory::create()\n>   * method implemented by the respective factories.\n>   */\n> -IPAProxy::IPAProxy()\n> -\t: valid_(false)\n> +IPAProxy::IPAProxy(IPAModule *ipam)\n> +\t: valid_(false), ipam_(ipam)\n>  {\n>  }\n>  \n> @@ -57,6 +61,89 @@ IPAProxy::~IPAProxy()\n>   * \\return True if the IPAProxy is valid, false otherwise\n>   */\n>  \n> +/**\n> + * \\brief Retrieve the absolute path to an IPA configuration file\n> + * \\param[in] name The configuration name\n> + *\n> + * This function locates the configuration file for an IPA and returns its\n> + * absolute path. It searches the following directories, in order:\n> + *\n> + * - All directories specified in the colon-separated LIBCAMERA_IPA_CONFIG_PATH\n> + *   environment variable ; or\n> + * - If libcamera is not installed, the src/ipa/ directory within the source\n> + *   tree ; otherwise\n> + * - The system sysconf (etc/libcamera/ipa) and the data (share/libcamera/ipa/)\n> + *   directories.\n> + *\n> + * The system directories are not searched if libcamera is not installed.\n> + *\n> + * Within each of those directories, the function looks for a subdirectory\n> + * named after the IPA module name, as reported in IPAModuleInfo::name, and for\n> + * a file named \\a name within that directory. The \\a name is IPA-specific.\n> + *\n> + * \\return The full path to the IPA configuration file, or an empty string if\n> + * no configuration file can be found\n> + */\n> +std::string IPAProxy::configurationFile(const std::string &name) const\n> +{\n> +\tstruct stat statbuf;\n> +\tint ret;\n> +\n> +\t/*\n> +\t * The IPA module name can be used as-is to build directory names as it\n> +\t * has been validated when loading the module.\n> +\t */\n> +\tstd::string ipaName = ipam_->info().name;\n> +\n> +\t/* Check the environment variable first. */\n> +\tconst char *confPaths = utils::secure_getenv(\"LIBCAMERA_IPA_CONFIG_PATH\");\n> +\tif (confPaths) {\n> +\t\tfor (const auto &dir : utils::split(confPaths, \":\")) {\n> +\t\t\tif (dir.empty())\n> +\t\t\t\tcontinue;\n> +\n> +\t\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n> +\t\t\tret = stat(confPath.c_str(), &statbuf);\n> +\t\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> +\t\t\t\treturn confPath;\n> +\t\t}\n> +\t}\n> +\n> +\t/*\n> +\t * When libcamera is used before it is installed, load configuration\n> +\t * files from the source directory. The configuration files are then\n> +\t * located in the 'data' subdirectory of the corresponding IPA module.\n\nHrm ... this seems to be hidden in the implementation.\n\nPerhaps worth describing that configuration files are stored in 'data'\nin the source tree as part of the commit log at least, and we'll need\nthis documenting in the 'ipa-writers-guide' at least, but as that\ndoesn't exist yet - I'll let you off for not adding it there. ;-)\n\nOther than that,\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n\n> +\t */\n> +\tstd::string root = utils::libcameraSourcePath();\n> +\tif (!root.empty()) {\n> +\t\tstd::string ipaConfDir = root + \"src/ipa/\" + ipaName + \"/data\";\n> +\n> +\t\tLOG(IPAProxy, Info)\n> +\t\t\t<< \"libcamera is not installed. Loading IPA configuration from '\"\n> +\t\t\t<< ipaConfDir << \"'\";\n> +\n> +\t\tstd::string confPath = ipaConfDir + \"/\" + name;\n> +\t\tret = stat(confPath.c_str(), &statbuf);\n> +\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> +\t\t\treturn confPath;\n> +\n> +\t\treturn std::string();\n> +\t}\n> +\n> +\t/* Else look in the system locations. */\n> +\tfor (const auto &dir : utils::split(IPA_CONFIG_DIR, \":\")) {\n> +\t\tif (dir.empty())\n> +\t\t\tcontinue;\n> +\n> +\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n> +\t\tret = stat(confPath.c_str(), &statbuf);\n> +\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> +\t\t\treturn confPath;\n> +\t}\n> +\n> +\treturn std::string();\n> +}\n> +\n>  /**\n>   * \\brief Find a valid full path for a proxy worker for a given executable name\n>   * \\param[in] file File name of proxy worker executable\n> diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> index 2aa80b946704..89f5cb54687b 100644\n> --- a/src/libcamera/proxy/ipa_proxy_linux.cpp\n> +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> @@ -44,7 +44,7 @@ private:\n>  };\n>  \n>  IPAProxyLinux::IPAProxyLinux(IPAModule *ipam)\n> -\t: proc_(nullptr), socket_(nullptr)\n> +\t: IPAProxy(ipam), proc_(nullptr), socket_(nullptr)\n>  {\n>  \tLOG(IPAProxy, Debug)\n>  \t\t<< \"initializing dummy proxy: loading IPA from \"\n> diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> index 368ccca1cf60..1ebb9b6a6c9d 100644\n> --- a/src/libcamera/proxy/ipa_proxy_thread.cpp\n> +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> @@ -73,7 +73,7 @@ private:\n>  };\n>  \n>  IPAProxyThread::IPAProxyThread(IPAModule *ipam)\n> -\t: running_(false)\n> +\t: IPAProxy(ipam), running_(false)\n>  {\n>  \tif (!ipam->load())\n>  \t\treturn;\n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4A8AD603F9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Apr 2020 14:08:35 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7427472C;\n\tMon, 27 Apr 2020 14:08:34 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"PqmJjmEb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1587989314;\n\tbh=9n/tBCUsUIHZ9EShCXz0HqeilomTMlm34ZzNdm+NPV0=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=PqmJjmEbu6Wl723exMHpuuQFsOZXvZXy9qvRn9UCAOmMJw+20MLTiKpaPPSUXw8YD\n\tqm/96XqwjnwLR/ODFJ/YGKDO///fan3qwyYAXzSZamoUeXcfrzEmI+1j8GeIxz2GOf\n\t92aC6fU6l5DBVb2DrcKbbu6v4huA+xKqIQxJs0Fk=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200427031713.14013-1-laurent.pinchart@ideasonboard.com>\n\t<20200427031713.14013-6-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<02e5faae-92b8-8af9-ea7d-8b820dbb700c@ideasonboard.com>","Date":"Mon, 27 Apr 2020 13:08:31 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.7.0","MIME-Version":"1.0","In-Reply-To":"<20200427031713.14013-6-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 05/11] libcamera: ipa_proxy: Provide\n\tsuport for IPA configuration files","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 27 Apr 2020 12:08:35 -0000"}},{"id":4569,"web_url":"https://patchwork.libcamera.org/comment/4569/","msgid":"<20200427122329.GD10040@pendragon.ideasonboard.com>","date":"2020-04-27T12:23:29","subject":"Re: [libcamera-devel] [PATCH 05/11] libcamera: ipa_proxy: Provide\n\tsuport for IPA configuration files","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Mon, Apr 27, 2020 at 01:08:31PM +0100, Kieran Bingham wrote:\n> On 27/04/2020 04:17, Laurent Pinchart wrote:\n> > IPA modules may require configuration files, which may be stored in\n> > different locations in the file system. To standardize file locations\n> > between all IPAs and pipeline handlers, provide a helper function to\n> > locate a configuration file by searching in the following directories:\n> > \n> > - All directories specified in the LIBCAMERA_IPA_CONFIG_PATH environment\n> >   variable ; or\n> > - In the build tree if libcamera is not installed ; otherwise\n> > - In standard system locations (etc and share directories).\n> > \n> > More locations, or extensions to the mechanism, may be implemented\n> > later.\n> > \n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  src/ipa/meson.build                      |  6 ++\n> >  src/libcamera/include/ipa_proxy.h        | 11 ++-\n> >  src/libcamera/ipa_proxy.cpp              | 91 +++++++++++++++++++++++-\n> >  src/libcamera/proxy/ipa_proxy_linux.cpp  |  2 +-\n> >  src/libcamera/proxy/ipa_proxy_thread.cpp |  2 +-\n> >  5 files changed, 105 insertions(+), 7 deletions(-)\n> > \n> > diff --git a/src/ipa/meson.build b/src/ipa/meson.build\n> > index cb4e3ab3388f..145bf8105af7 100644\n> > --- a/src/ipa/meson.build\n> > +++ b/src/ipa/meson.build\n> > @@ -1,10 +1,16 @@\n> >  ipa_install_dir = join_paths(get_option('libdir'), 'libcamera')\n> > +ipa_data_dir = join_paths(get_option('datadir'), 'libcamera', 'ipa')\n> > +ipa_sysconf_dir = join_paths(get_option('sysconfdir'), 'libcamera', 'ipa')\n> >  \n> >  ipa_includes = [\n> >      libcamera_includes,\n> >      libcamera_internal_includes,\n> >  ]\n> >  \n> > +config_h.set('IPA_CONFIG_DIR',\n> > +             '\"' + join_paths(get_option('prefix'), ipa_sysconf_dir) +\n> > +             ':' + join_paths(get_option('prefix'), ipa_data_dir) + '\"')\n> > +\n> >  config_h.set('IPA_MODULE_DIR',\n> >               '\"' + join_paths(get_option('prefix'), ipa_install_dir) + '\"')\n> >  \n> > diff --git a/src/libcamera/include/ipa_proxy.h b/src/libcamera/include/ipa_proxy.h\n> > index e696551af39f..1111065b36a7 100644\n> > --- a/src/libcamera/include/ipa_proxy.h\n> > +++ b/src/libcamera/include/ipa_proxy.h\n> > @@ -13,22 +13,27 @@\n> >  \n> >  #include <ipa/ipa_interface.h>\n> >  \n> > -#include \"ipa_module.h\"\n> > -\n> >  namespace libcamera {\n> >  \n> > +class IPAModule;\n> > +\n> >  class IPAProxy : public IPAInterface\n> >  {\n> >  public:\n> > -\tIPAProxy();\n> > +\tIPAProxy(IPAModule *ipam);\n> >  \t~IPAProxy();\n> >  \n> >  \tbool isValid() const { return valid_; }\n> >  \n> > +\tstd::string configurationFile(const std::string &file) const;\n> > +\n> >  protected:\n> >  \tstd::string resolvePath(const std::string &file) const;\n> >  \n> >  \tbool valid_;\n> > +\n> > +private:\n> > +\tIPAModule *ipam_;\n> >  };\n> >  \n> >  class IPAProxyFactory\n> > diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n> > index 43ac9afc25cb..ea69e63d6bb8 100644\n> > --- a/src/libcamera/ipa_proxy.cpp\n> > +++ b/src/libcamera/ipa_proxy.cpp\n> > @@ -8,8 +8,11 @@\n> >  #include \"ipa_proxy.h\"\n> >  \n> >  #include <string.h>\n> > +#include <sys/stat.h>\n> > +#include <sys/types.h>\n> >  #include <unistd.h>\n> >  \n> > +#include \"ipa_module.h\"\n> >  #include \"log.h\"\n> >  #include \"utils.h\"\n> >  \n> > @@ -34,12 +37,13 @@ LOG_DEFINE_CATEGORY(IPAProxy)\n> >  \n> >  /**\n> >   * \\brief Construct an IPAProxy instance\n> > + * \\param[in] ipam The IPA module\n> >   *\n> >   * IPAProxy instances shall be constructed through the IPAProxyFactory::create()\n> >   * method implemented by the respective factories.\n> >   */\n> > -IPAProxy::IPAProxy()\n> > -\t: valid_(false)\n> > +IPAProxy::IPAProxy(IPAModule *ipam)\n> > +\t: valid_(false), ipam_(ipam)\n> >  {\n> >  }\n> >  \n> > @@ -57,6 +61,89 @@ IPAProxy::~IPAProxy()\n> >   * \\return True if the IPAProxy is valid, false otherwise\n> >   */\n> >  \n> > +/**\n> > + * \\brief Retrieve the absolute path to an IPA configuration file\n> > + * \\param[in] name The configuration name\n> > + *\n> > + * This function locates the configuration file for an IPA and returns its\n> > + * absolute path. It searches the following directories, in order:\n> > + *\n> > + * - All directories specified in the colon-separated LIBCAMERA_IPA_CONFIG_PATH\n> > + *   environment variable ; or\n> > + * - If libcamera is not installed, the src/ipa/ directory within the source\n> > + *   tree ; otherwise\n> > + * - The system sysconf (etc/libcamera/ipa) and the data (share/libcamera/ipa/)\n> > + *   directories.\n> > + *\n> > + * The system directories are not searched if libcamera is not installed.\n> > + *\n> > + * Within each of those directories, the function looks for a subdirectory\n> > + * named after the IPA module name, as reported in IPAModuleInfo::name, and for\n> > + * a file named \\a name within that directory. The \\a name is IPA-specific.\n> > + *\n> > + * \\return The full path to the IPA configuration file, or an empty string if\n> > + * no configuration file can be found\n> > + */\n> > +std::string IPAProxy::configurationFile(const std::string &name) const\n> > +{\n> > +\tstruct stat statbuf;\n> > +\tint ret;\n> > +\n> > +\t/*\n> > +\t * The IPA module name can be used as-is to build directory names as it\n> > +\t * has been validated when loading the module.\n> > +\t */\n> > +\tstd::string ipaName = ipam_->info().name;\n> > +\n> > +\t/* Check the environment variable first. */\n> > +\tconst char *confPaths = utils::secure_getenv(\"LIBCAMERA_IPA_CONFIG_PATH\");\n> > +\tif (confPaths) {\n> > +\t\tfor (const auto &dir : utils::split(confPaths, \":\")) {\n> > +\t\t\tif (dir.empty())\n> > +\t\t\t\tcontinue;\n> > +\n> > +\t\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n> > +\t\t\tret = stat(confPath.c_str(), &statbuf);\n> > +\t\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> > +\t\t\t\treturn confPath;\n> > +\t\t}\n> > +\t}\n> > +\n> > +\t/*\n> > +\t * When libcamera is used before it is installed, load configuration\n> > +\t * files from the source directory. The configuration files are then\n> > +\t * located in the 'data' subdirectory of the corresponding IPA module.\n> \n> Hrm ... this seems to be hidden in the implementation.\n> \n> Perhaps worth describing that configuration files are stored in 'data'\n> in the source tree as part of the commit log at least, and we'll need\n> this documenting in the 'ipa-writers-guide' at least, but as that\n> doesn't exist yet - I'll let you off for not adding it there. ;-)\n\nI'll add this to the commit message.\n\nWhen stored in the source tree, configuration files shall be located in\na 'data' subdirectory of their respective IPA directory.\n\n> Other than that,\n> \n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n> > +\t */\n> > +\tstd::string root = utils::libcameraSourcePath();\n> > +\tif (!root.empty()) {\n> > +\t\tstd::string ipaConfDir = root + \"src/ipa/\" + ipaName + \"/data\";\n> > +\n> > +\t\tLOG(IPAProxy, Info)\n> > +\t\t\t<< \"libcamera is not installed. Loading IPA configuration from '\"\n> > +\t\t\t<< ipaConfDir << \"'\";\n> > +\n> > +\t\tstd::string confPath = ipaConfDir + \"/\" + name;\n> > +\t\tret = stat(confPath.c_str(), &statbuf);\n> > +\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> > +\t\t\treturn confPath;\n> > +\n> > +\t\treturn std::string();\n> > +\t}\n> > +\n> > +\t/* Else look in the system locations. */\n> > +\tfor (const auto &dir : utils::split(IPA_CONFIG_DIR, \":\")) {\n> > +\t\tif (dir.empty())\n> > +\t\t\tcontinue;\n> > +\n> > +\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n> > +\t\tret = stat(confPath.c_str(), &statbuf);\n> > +\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> > +\t\t\treturn confPath;\n> > +\t}\n> > +\n> > +\treturn std::string();\n> > +}\n> > +\n> >  /**\n> >   * \\brief Find a valid full path for a proxy worker for a given executable name\n> >   * \\param[in] file File name of proxy worker executable\n> > diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > index 2aa80b946704..89f5cb54687b 100644\n> > --- a/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> > @@ -44,7 +44,7 @@ private:\n> >  };\n> >  \n> >  IPAProxyLinux::IPAProxyLinux(IPAModule *ipam)\n> > -\t: proc_(nullptr), socket_(nullptr)\n> > +\t: IPAProxy(ipam), proc_(nullptr), socket_(nullptr)\n> >  {\n> >  \tLOG(IPAProxy, Debug)\n> >  \t\t<< \"initializing dummy proxy: loading IPA from \"\n> > diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> > index 368ccca1cf60..1ebb9b6a6c9d 100644\n> > --- a/src/libcamera/proxy/ipa_proxy_thread.cpp\n> > +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp\n> > @@ -73,7 +73,7 @@ private:\n> >  };\n> >  \n> >  IPAProxyThread::IPAProxyThread(IPAModule *ipam)\n> > -\t: running_(false)\n> > +\t: IPAProxy(ipam), running_(false)\n> >  {\n> >  \tif (!ipam->load())\n> >  \t\treturn;\n> >","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 62537603F9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Apr 2020 14:23:45 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C6A2772C;\n\tMon, 27 Apr 2020 14:23:44 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"eHDYUv2O\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1587990225;\n\tbh=a5kGrF8x3Cp9aNcgDN/mOxXvq7vYVBiK3RtqxB+/HvA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=eHDYUv2Ogr9ahuNghz/R7Oal52tKxy6xO8TaWEHQNlbNJS11RBrZEaRlg0f4nznPF\n\t55FTpdtbUkVIt50CUOuPef60V51FWTyw/gFkCqh5XvDEyA3oHUlO/YVfAvJajM0PkI\n\t5w1T/bJmy4xbpZrbNE5y90EVOw3yImtS9cWYsxH8=","Date":"Mon, 27 Apr 2020 15:23:29 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200427122329.GD10040@pendragon.ideasonboard.com>","References":"<20200427031713.14013-1-laurent.pinchart@ideasonboard.com>\n\t<20200427031713.14013-6-laurent.pinchart@ideasonboard.com>\n\t<02e5faae-92b8-8af9-ea7d-8b820dbb700c@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<02e5faae-92b8-8af9-ea7d-8b820dbb700c@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 05/11] libcamera: ipa_proxy: Provide\n\tsuport for IPA configuration files","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 27 Apr 2020 12:23:45 -0000"}},{"id":4571,"web_url":"https://patchwork.libcamera.org/comment/4571/","msgid":"<cbf1ff2f-b2d5-eb85-e230-83727a37ab60@ideasonboard.com>","date":"2020-04-27T12:27:42","subject":"Re: [libcamera-devel] [PATCH 05/11] libcamera: ipa_proxy: Provide\n\tsuport for IPA configuration files","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi,\n\nOn 27/04/2020 13:23, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> On Mon, Apr 27, 2020 at 01:08:31PM +0100, Kieran Bingham wrote:\n>> On 27/04/2020 04:17, Laurent Pinchart wrote:\n>>> IPA modules may require configuration files, which may be stored in\n>>> different locations in the file system. To standardize file locations\n>>> between all IPAs and pipeline handlers, provide a helper function to\n>>> locate a configuration file by searching in the following directories:\n>>>\n>>> - All directories specified in the LIBCAMERA_IPA_CONFIG_PATH environment\n>>>   variable ; or\n>>> - In the build tree if libcamera is not installed ; otherwise\n>>> - In standard system locations (etc and share directories).\n>>>\n>>> More locations, or extensions to the mechanism, may be implemented\n>>> later.\n>>>\n>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>>> ---\n>>>  src/ipa/meson.build                      |  6 ++\n>>>  src/libcamera/include/ipa_proxy.h        | 11 ++-\n>>>  src/libcamera/ipa_proxy.cpp              | 91 +++++++++++++++++++++++-\n>>>  src/libcamera/proxy/ipa_proxy_linux.cpp  |  2 +-\n>>>  src/libcamera/proxy/ipa_proxy_thread.cpp |  2 +-\n>>>  5 files changed, 105 insertions(+), 7 deletions(-)\n>>>\n>>> diff --git a/src/ipa/meson.build b/src/ipa/meson.build\n>>> index cb4e3ab3388f..145bf8105af7 100644\n>>> --- a/src/ipa/meson.build\n>>> +++ b/src/ipa/meson.build\n>>> @@ -1,10 +1,16 @@\n>>>  ipa_install_dir = join_paths(get_option('libdir'), 'libcamera')\n>>> +ipa_data_dir = join_paths(get_option('datadir'), 'libcamera', 'ipa')\n>>> +ipa_sysconf_dir = join_paths(get_option('sysconfdir'), 'libcamera', 'ipa')\n>>>  \n>>>  ipa_includes = [\n>>>      libcamera_includes,\n>>>      libcamera_internal_includes,\n>>>  ]\n>>>  \n>>> +config_h.set('IPA_CONFIG_DIR',\n>>> +             '\"' + join_paths(get_option('prefix'), ipa_sysconf_dir) +\n>>> +             ':' + join_paths(get_option('prefix'), ipa_data_dir) + '\"')\n>>> +\n>>>  config_h.set('IPA_MODULE_DIR',\n>>>               '\"' + join_paths(get_option('prefix'), ipa_install_dir) + '\"')\n>>>  \n>>> diff --git a/src/libcamera/include/ipa_proxy.h b/src/libcamera/include/ipa_proxy.h\n>>> index e696551af39f..1111065b36a7 100644\n>>> --- a/src/libcamera/include/ipa_proxy.h\n>>> +++ b/src/libcamera/include/ipa_proxy.h\n>>> @@ -13,22 +13,27 @@\n>>>  \n>>>  #include <ipa/ipa_interface.h>\n>>>  \n>>> -#include \"ipa_module.h\"\n>>> -\n>>>  namespace libcamera {\n>>>  \n>>> +class IPAModule;\n>>> +\n>>>  class IPAProxy : public IPAInterface\n>>>  {\n>>>  public:\n>>> -\tIPAProxy();\n>>> +\tIPAProxy(IPAModule *ipam);\n>>>  \t~IPAProxy();\n>>>  \n>>>  \tbool isValid() const { return valid_; }\n>>>  \n>>> +\tstd::string configurationFile(const std::string &file) const;\n>>> +\n>>>  protected:\n>>>  \tstd::string resolvePath(const std::string &file) const;\n>>>  \n>>>  \tbool valid_;\n>>> +\n>>> +private:\n>>> +\tIPAModule *ipam_;\n>>>  };\n>>>  \n>>>  class IPAProxyFactory\n>>> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\n>>> index 43ac9afc25cb..ea69e63d6bb8 100644\n>>> --- a/src/libcamera/ipa_proxy.cpp\n>>> +++ b/src/libcamera/ipa_proxy.cpp\n>>> @@ -8,8 +8,11 @@\n>>>  #include \"ipa_proxy.h\"\n>>>  \n>>>  #include <string.h>\n>>> +#include <sys/stat.h>\n>>> +#include <sys/types.h>\n>>>  #include <unistd.h>\n>>>  \n>>> +#include \"ipa_module.h\"\n>>>  #include \"log.h\"\n>>>  #include \"utils.h\"\n>>>  \n>>> @@ -34,12 +37,13 @@ LOG_DEFINE_CATEGORY(IPAProxy)\n>>>  \n>>>  /**\n>>>   * \\brief Construct an IPAProxy instance\n>>> + * \\param[in] ipam The IPA module\n>>>   *\n>>>   * IPAProxy instances shall be constructed through the IPAProxyFactory::create()\n>>>   * method implemented by the respective factories.\n>>>   */\n>>> -IPAProxy::IPAProxy()\n>>> -\t: valid_(false)\n>>> +IPAProxy::IPAProxy(IPAModule *ipam)\n>>> +\t: valid_(false), ipam_(ipam)\n>>>  {\n>>>  }\n>>>  \n>>> @@ -57,6 +61,89 @@ IPAProxy::~IPAProxy()\n>>>   * \\return True if the IPAProxy is valid, false otherwise\n>>>   */\n>>>  \n>>> +/**\n>>> + * \\brief Retrieve the absolute path to an IPA configuration file\n>>> + * \\param[in] name The configuration name\n>>> + *\n>>> + * This function locates the configuration file for an IPA and returns its\n>>> + * absolute path. It searches the following directories, in order:\n>>> + *\n>>> + * - All directories specified in the colon-separated LIBCAMERA_IPA_CONFIG_PATH\n>>> + *   environment variable ; or\n>>> + * - If libcamera is not installed, the src/ipa/ directory within the source\n>>> + *   tree ; otherwise\n>>> + * - The system sysconf (etc/libcamera/ipa) and the data (share/libcamera/ipa/)\n>>> + *   directories.\n>>> + *\n>>> + * The system directories are not searched if libcamera is not installed.\n>>> + *\n>>> + * Within each of those directories, the function looks for a subdirectory\n>>> + * named after the IPA module name, as reported in IPAModuleInfo::name, and for\n>>> + * a file named \\a name within that directory. The \\a name is IPA-specific.\n>>> + *\n>>> + * \\return The full path to the IPA configuration file, or an empty string if\n>>> + * no configuration file can be found\n>>> + */\n>>> +std::string IPAProxy::configurationFile(const std::string &name) const\n>>> +{\n>>> +\tstruct stat statbuf;\n>>> +\tint ret;\n>>> +\n>>> +\t/*\n>>> +\t * The IPA module name can be used as-is to build directory names as it\n>>> +\t * has been validated when loading the module.\n>>> +\t */\n>>> +\tstd::string ipaName = ipam_->info().name;\n>>> +\n>>> +\t/* Check the environment variable first. */\n>>> +\tconst char *confPaths = utils::secure_getenv(\"LIBCAMERA_IPA_CONFIG_PATH\");\n>>> +\tif (confPaths) {\n>>> +\t\tfor (const auto &dir : utils::split(confPaths, \":\")) {\n>>> +\t\t\tif (dir.empty())\n>>> +\t\t\t\tcontinue;\n>>> +\n>>> +\t\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n>>> +\t\t\tret = stat(confPath.c_str(), &statbuf);\n>>> +\t\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n>>> +\t\t\t\treturn confPath;\n>>> +\t\t}\n>>> +\t}\n>>> +\n>>> +\t/*\n>>> +\t * When libcamera is used before it is installed, load configuration\n>>> +\t * files from the source directory. The configuration files are then\n>>> +\t * located in the 'data' subdirectory of the corresponding IPA module.\n>>\n>> Hrm ... this seems to be hidden in the implementation.\n>>\n>> Perhaps worth describing that configuration files are stored in 'data'\n>> in the source tree as part of the commit log at least, and we'll need\n>> this documenting in the 'ipa-writers-guide' at least, but as that\n>> doesn't exist yet - I'll let you off for not adding it there. ;-)\n> \n> I'll add this to the commit message.\n> \n> When stored in the source tree, configuration files shall be located in\n> a 'data' subdirectory of their respective IPA directory.\n\nGreat, that will at least improve the git-documentation ;-)\n--\nKieran\n\n\n>> Other than that,\n>>\n>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>>\n>>> +\t */\n>>> +\tstd::string root = utils::libcameraSourcePath();\n>>> +\tif (!root.empty()) {\n>>> +\t\tstd::string ipaConfDir = root + \"src/ipa/\" + ipaName + \"/data\";\n>>> +\n>>> +\t\tLOG(IPAProxy, Info)\n>>> +\t\t\t<< \"libcamera is not installed. Loading IPA configuration from '\"\n>>> +\t\t\t<< ipaConfDir << \"'\";\n>>> +\n>>> +\t\tstd::string confPath = ipaConfDir + \"/\" + name;\n>>> +\t\tret = stat(confPath.c_str(), &statbuf);\n>>> +\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n>>> +\t\t\treturn confPath;\n>>> +\n>>> +\t\treturn std::string();\n>>> +\t}\n>>> +\n>>> +\t/* Else look in the system locations. */\n>>> +\tfor (const auto &dir : utils::split(IPA_CONFIG_DIR, \":\")) {\n>>> +\t\tif (dir.empty())\n>>> +\t\t\tcontinue;\n>>> +\n>>> +\t\tstd::string confPath = dir + \"/\" + ipaName + \"/\" + name;\n>>> +\t\tret = stat(confPath.c_str(), &statbuf);\n>>> +\t\tif (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n>>> +\t\t\treturn confPath;\n>>> +\t}\n>>> +\n>>> +\treturn std::string();\n>>> +}\n>>> +\n>>>  /**\n>>>   * \\brief Find a valid full path for a proxy worker for a given executable name\n>>>   * \\param[in] file File name of proxy worker executable\n>>> diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp\n>>> index 2aa80b946704..89f5cb54687b 100644\n>>> --- a/src/libcamera/proxy/ipa_proxy_linux.cpp\n>>> +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp\n>>> @@ -44,7 +44,7 @@ private:\n>>>  };\n>>>  \n>>>  IPAProxyLinux::IPAProxyLinux(IPAModule *ipam)\n>>> -\t: proc_(nullptr), socket_(nullptr)\n>>> +\t: IPAProxy(ipam), proc_(nullptr), socket_(nullptr)\n>>>  {\n>>>  \tLOG(IPAProxy, Debug)\n>>>  \t\t<< \"initializing dummy proxy: loading IPA from \"\n>>> diff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp\n>>> index 368ccca1cf60..1ebb9b6a6c9d 100644\n>>> --- a/src/libcamera/proxy/ipa_proxy_thread.cpp\n>>> +++ b/src/libcamera/proxy/ipa_proxy_thread.cpp\n>>> @@ -73,7 +73,7 @@ private:\n>>>  };\n>>>  \n>>>  IPAProxyThread::IPAProxyThread(IPAModule *ipam)\n>>> -\t: running_(false)\n>>> +\t: IPAProxy(ipam), running_(false)\n>>>  {\n>>>  \tif (!ipam->load())\n>>>  \t\treturn;\n>>>\n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DE7DF603F9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 27 Apr 2020 14:27:46 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2D39872C;\n\tMon, 27 Apr 2020 14:27:46 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"hrVoLNgY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1587990466;\n\tbh=y6ndvubSf5MK3H3/howFeBPs+TWQy1+WZ7lrTSyQl0k=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=hrVoLNgY9Cb43XN8wevHLRpJBwkizDxWL8cPMesRTXh3tiBZlR9zbo2DKXe4dC3bU\n\tAT4uPEdDOWu4W4mm3d7osdhjB3A7VyPlTseZ7B1B+yroyGRC2Mr9AoLanzr+AggS73\n\tpbt8NBsxgmfZW/arx3P5QEo1RHrlG85kSYg6wWHU=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","References":"<20200427031713.14013-1-laurent.pinchart@ideasonboard.com>\n\t<20200427031713.14013-6-laurent.pinchart@ideasonboard.com>\n\t<02e5faae-92b8-8af9-ea7d-8b820dbb700c@ideasonboard.com>\n\t<20200427122329.GD10040@pendragon.ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<cbf1ff2f-b2d5-eb85-e230-83727a37ab60@ideasonboard.com>","Date":"Mon, 27 Apr 2020 13:27:42 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.7.0","MIME-Version":"1.0","In-Reply-To":"<20200427122329.GD10040@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH 05/11] libcamera: ipa_proxy: Provide\n\tsuport for IPA configuration files","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 27 Apr 2020 12:27:47 -0000"}}]