[{"id":3824,"web_url":"https://patchwork.libcamera.org/comment/3824/","msgid":"<20200220212446.GO4998@pendragon.ideasonboard.com>","date":"2020-02-20T21:24:46","subject":"Re: [libcamera-devel] [PATCH v3 5/6] libcamera: ipa_manager: Search\n\tfor IPA libraries in build tree","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Thu, Feb 20, 2020 at 04:57:03PM +0000, Kieran Bingham wrote:\n> When libcamera is built and tested (or used at all) before installing to\n> the configured prefix path, it will be unable to locate the IPA\n> binaries, or IPA binaries previously installed in the system paths may\n> be incorrect to load.\n> \n> Utilise the build_rpath dynamic tag which is stripped out by meson at\n> install time to determine at runtime if the library currently executing\n> has been installed or not.\n> \n> When not installed and running from a build tree, identify the location\n> of that tree by finding the path of the active libcamera.so itself, and\n> from that point add a relative path to be able to load the most recently\n> built IPA modules.\n> \n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n> ---\n> v2:\n>  - Move get_runpath to ipa_manager.c : libcamera::elfRunPath()\n>  - Minor fixes\n>  - Squash elfRunPath() into this patch\n> \n> v3:\n>  - Full rework. It's just all different :-)\n>  - Maybe this one is going to cut it ...\n> ---\n>  src/libcamera/ipa_manager.cpp | 46 ++++++++++++++++++++++++++++++++++-\n>  src/libcamera/meson.build     |  6 +++++\n>  2 files changed, 51 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> index 3b1d4c0b295e..e60bf3dabebe 100644\n> --- a/src/libcamera/ipa_manager.cpp\n> +++ b/src/libcamera/ipa_manager.cpp\n> @@ -9,6 +9,9 @@\n>  \n>  #include <algorithm>\n>  #include <dirent.h>\n> +#include <dlfcn.h>\n> +#include <elf.h>\n> +#include <link.h>\n>  #include <string.h>\n>  #include <sys/types.h>\n>  \n> @@ -24,6 +27,30 @@\n>   * \\brief Image Processing Algorithm module manager\n>   */\n>  \n> +static bool isLibcameraInstalled()\n> +{\n\nI'd add a comment here to state\n\n\t/* musl doesn't declare _DYNAMIC in link.h, declare it manually. */\n\nto remember this is here for a reason.\n\n> +\textern ElfW(Dyn) _DYNAMIC[];\n\nMaybe add a blank line here ?\n\n> +\t/* DT_RUNPATH (DT_RPATH on musl) is removed on install. */\n\nDo we really need to check for DT_RPATH ? As far as I know the tag that\nis generated depends solely on the linker version, not on the C library.\nDoes musl really generate DT_RPATH ?\n\n> +\tfor (const ElfW(Dyn) *dyn = _DYNAMIC; dyn->d_tag != DT_NULL; ++dyn)\n> +\t\tif (dyn->d_tag == DT_RUNPATH || dyn->d_tag == DT_RPATH)\n> +\t\t\treturn false;\n\nBraces for the 'for' ?\n\n> +\n> +\treturn true;\n> +}\n> +\n> +static const char *libcameraPath()\n> +{\n> +\tDl_info info;\n> +\tint ret;\n> +\n> +\t/* look up our own symbol. */\n\ns/look/Look/\n\nThat's a neat way to do it, we could have used an unrelated static\nfunction (such as IPAManager::instance) and inlined this code in\nIPAManager::IPAManager, but create a separate static function makes the\ncode self-contained.\n\n> +\tret = dladdr(reinterpret_cast<void *>(libcameraPath), &info);\n> +\tif (ret == 0)\n> +\t\treturn nullptr;\n> +\n> +\treturn info.dli_fname;\n> +}\n\nI would move these two functions just above IPAManager::IPAManager to\nkeep them close to the location where they're used.\n\n> +\n>  namespace libcamera {\n>  \n>  LOG_DEFINE_CATEGORY(IPAManager)\n> @@ -108,7 +135,24 @@ IPAManager::IPAManager()\n>  \t\t\t\t<< \"No IPA found in '\" << modulePaths << \"'\";\n>  \t}\n>  \n> -\t/* Load IPAs from the installed system path. */\n> +\t/*\n> +\t * When libcamera is used before it is installed, load IPAs from the\n> +\t * same build directory as the libcamera library itself. This requires\n> +\t * identifying the path of the libcamera.so, and referencing a relative\n> +\t * path for the IPA from that point.\n\nMaybe add \"We need to recursive into one level of sub-directories to\nmatch the build tree.\" ?\n\n> +\t */\n> +\tif (!isLibcameraInstalled()) {\n> +\t\tstd::string ipaBuildPath = utils::dirname(libcameraPath()) + \"/../ipa\";\n> +\t\tconstexpr int maxDepth = 1;\n> +\n> +\t\tLOG(IPAManager, Info)\n> +\t\t\t<< \"libcamera is not installed. Adding '\"\n> +\t\t\t<< ipaBuildPath << \"' to the IPA search path\";\n> +\n> +\t\tipaCount += addDir(ipaBuildPath.c_str(), maxDepth);\n> +\t}\n> +\n> +\t/* Finally try to load IPAs from the installed system path. */\n>  \tret = addDir(IPA_MODULE_DIR);\n>  \tif (ret > 0)\n>  \t\tipaCount += ret;\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 1e5b54b34078..88658ac563f7 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -107,11 +107,17 @@ if get_option('android')\n>      libcamera_link_with += android_camera_metadata\n>  endif\n>  \n> +# We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.\n> +# The build_rpath is stripped at install time by meson, so we determine at\n> +# runtime if the library is running from an installed location by checking\n> +# for the presence or abscence of the dynamic tag.\n\ns/abscence/absence/\n\nDon't tell me this is a UK spelling :-)\n\n> +\n>  libcamera = shared_library('camera',\n>                             libcamera_sources,\n>                             install : true,\n>                             link_with : libcamera_link_with,\n>                             include_directories : includes,\n> +                           build_rpath : '/',\n>                             dependencies : libcamera_deps)\n\nI double-checked based on what criteria build_rpath would generate\nDT_RPATH or DT_RUNPATH. It turns out this is controlled by the\n--enable-new-dtags and --disable-new-dtags options to ld, with the\nformat generating DT_RUNPATH and the latter DT_RPATH. None of these\nare set by meson by default. GNU ld defaulted to --enable-new-dtags in\nbinutils v2.24, released on December 2013, before gcc 5.1 (April 2015)\nwhich is the earliest compiler version we support (I'm actually not even\nsure we support gcc 5.1, I have gcc 5.4 installed in my test setup,\nreleased on June 2016). Debian Jessie ships binutils v2.25 and Ubuntu\n16.04 ships binutils v2.26. We should thus be safe.\n\nAssuming the DT_RPATH comment for musl isn't right and the corresponding\ncondition can be removed,\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nOtherwise let's discuss it.\n\n>  \n>  libcamera_dep = declare_dependency(sources : [libcamera_api, libcamera_ipa_api, libcamera_h],","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 3E64460434\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 20 Feb 2020 22:25:06 +0100 (CET)","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 A7D03563;\n\tThu, 20 Feb 2020 22:25:05 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1582233905;\n\tbh=/a/z1vNdoFgCXsMsfpp5WJIYWswt5EEv2SFlCSaSy9s=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XyH+IjWQg8idGzsX4NYk5P3ShXaRXCRqxVQqS22Mmy35lR8lX+MwMNap78Bj9dfk2\n\trHzXL9ZpkP4blKB4O3HYpVkfTp0g9mCV2DYAMnHTkHl7+1HCCslHwAUxjmkOIpi+hx\n\tJyfbaxkwWZiOCwmHf05o3zOJQ6J4rZ0sUQe6i2lY=","Date":"Thu, 20 Feb 2020 23:24:46 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Message-ID":"<20200220212446.GO4998@pendragon.ideasonboard.com>","References":"<20200220165704.23600-1-kieran.bingham@ideasonboard.com>\n\t<20200220165704.23600-6-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200220165704.23600-6-kieran.bingham@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 5/6] libcamera: ipa_manager: Search\n\tfor IPA libraries in build tree","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":"Thu, 20 Feb 2020 21:25:06 -0000"}},{"id":3828,"web_url":"https://patchwork.libcamera.org/comment/3828/","msgid":"<f8307984-c227-90ff-c02b-95ffc5e86000@ideasonboard.com>","date":"2020-02-21T12:37:06","subject":"Re: [libcamera-devel] [PATCH v3 5/6] libcamera: ipa_manager: Search\n\tfor IPA libraries in build tree","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 20/02/2020 21:24, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> On Thu, Feb 20, 2020 at 04:57:03PM +0000, Kieran Bingham wrote:\n>> When libcamera is built and tested (or used at all) before installing to\n>> the configured prefix path, it will be unable to locate the IPA\n>> binaries, or IPA binaries previously installed in the system paths may\n>> be incorrect to load.\n>>\n>> Utilise the build_rpath dynamic tag which is stripped out by meson at\n>> install time to determine at runtime if the library currently executing\n>> has been installed or not.\n>>\n>> When not installed and running from a build tree, identify the location\n>> of that tree by finding the path of the active libcamera.so itself, and\n>> from that point add a relative path to be able to load the most recently\n>> built IPA modules.\n>>\n>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>>\n>> ---\n>> v2:\n>>  - Move get_runpath to ipa_manager.c : libcamera::elfRunPath()\n>>  - Minor fixes\n>>  - Squash elfRunPath() into this patch\n>>\n>> v3:\n>>  - Full rework. It's just all different :-)\n>>  - Maybe this one is going to cut it ...\n>> ---\n>>  src/libcamera/ipa_manager.cpp | 46 ++++++++++++++++++++++++++++++++++-\n>>  src/libcamera/meson.build     |  6 +++++\n>>  2 files changed, 51 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n>> index 3b1d4c0b295e..e60bf3dabebe 100644\n>> --- a/src/libcamera/ipa_manager.cpp\n>> +++ b/src/libcamera/ipa_manager.cpp\n>> @@ -9,6 +9,9 @@\n>>  \n>>  #include <algorithm>\n>>  #include <dirent.h>\n>> +#include <dlfcn.h>\n>> +#include <elf.h>\n>> +#include <link.h>\n>>  #include <string.h>\n>>  #include <sys/types.h>\n>>  \n>> @@ -24,6 +27,30 @@\n>>   * \\brief Image Processing Algorithm module manager\n>>   */\n>>  \n>> +static bool isLibcameraInstalled()\n>> +{\n> \n> I'd add a comment here to state\n> \n> \t/* musl doesn't declare _DYNAMIC in link.h, declare it manually. */\n> \n> to remember this is here for a reason.\n\nAdded.\n\n> \n>> +\textern ElfW(Dyn) _DYNAMIC[];\n> \n> Maybe add a blank line here ?\n\nHrm... I used to have one. Maybe I lost it during a rebase :(\nAdded\n\n> \n>> +\t/* DT_RUNPATH (DT_RPATH on musl) is removed on install. */\n> \n> Do we really need to check for DT_RPATH ? As far as I know the tag that\n> is generated depends solely on the linker version, not on the C library.\n> Does musl really generate DT_RPATH ?\n> \n>> +\tfor (const ElfW(Dyn) *dyn = _DYNAMIC; dyn->d_tag != DT_NULL; ++dyn)\n>> +\t\tif (dyn->d_tag == DT_RUNPATH || dyn->d_tag == DT_RPATH)\n>> +\t\t\treturn false;\n> \n> Braces for the 'for' ?\n\n(Re-)added\n\n> \n>> +\n>> +\treturn true;\n>> +}\n>> +\n>> +static const char *libcameraPath()\n>> +{\n>> +\tDl_info info;\n>> +\tint ret;\n>> +\n>> +\t/* look up our own symbol. */\n> \n> s/look/Look/\n> \n> That's a neat way to do it, we could have used an unrelated static\n> function (such as IPAManager::instance) and inlined this code in\n> IPAManager::IPAManager, but create a separate static function makes the\n> code self-contained.\n\nYes, and I envision this function being re-used (As you've noticed\nlater, we also have the proxy worker variable to deal with).\n\n> \n>> +\tret = dladdr(reinterpret_cast<void *>(libcameraPath), &info);\n>> +\tif (ret == 0)\n>> +\t\treturn nullptr;\n>> +\n>> +\treturn info.dli_fname;\n>> +}\n> \n> I would move these two functions just above IPAManager::IPAManager to\n> keep them close to the location where they're used.\n\nWithout them being member functions? Euewww ...\n\nI don't think they should be member functions, and in fact will have to\nbe moved to utils:: when we handle LIBCAMERA_IPA_PROXY_PATH\n\nI had been leaving that until I got this working.\n\nWould you prefer I put these functions in utils:: now?\n\n\n>> +\n>>  namespace libcamera {\n>>  \n>>  LOG_DEFINE_CATEGORY(IPAManager)\n>> @@ -108,7 +135,24 @@ IPAManager::IPAManager()\n>>  \t\t\t\t<< \"No IPA found in '\" << modulePaths << \"'\";\n>>  \t}\n>>  \n>> -\t/* Load IPAs from the installed system path. */\n>> +\t/*\n>> +\t * When libcamera is used before it is installed, load IPAs from the\n>> +\t * same build directory as the libcamera library itself. This requires\n>> +\t * identifying the path of the libcamera.so, and referencing a relative\n>> +\t * path for the IPA from that point.\n> \n> Maybe add \"We need to recursive into one level of sub-directories to\n> match the build tree.\" ?\n\nAdded\n\n> \n>> +\t */\n>> +\tif (!isLibcameraInstalled()) {\n>> +\t\tstd::string ipaBuildPath = utils::dirname(libcameraPath()) + \"/../ipa\";\n>> +\t\tconstexpr int maxDepth = 1;\n>> +\n>> +\t\tLOG(IPAManager, Info)\n>> +\t\t\t<< \"libcamera is not installed. Adding '\"\n>> +\t\t\t<< ipaBuildPath << \"' to the IPA search path\";\n>> +\n>> +\t\tipaCount += addDir(ipaBuildPath.c_str(), maxDepth);\n>> +\t}\n>> +\n>> +\t/* Finally try to load IPAs from the installed system path. */\n>>  \tret = addDir(IPA_MODULE_DIR);\n>>  \tif (ret > 0)\n>>  \t\tipaCount += ret;\n>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n>> index 1e5b54b34078..88658ac563f7 100644\n>> --- a/src/libcamera/meson.build\n>> +++ b/src/libcamera/meson.build\n>> @@ -107,11 +107,17 @@ if get_option('android')\n>>      libcamera_link_with += android_camera_metadata\n>>  endif\n>>  \n>> +# We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.\n>> +# The build_rpath is stripped at install time by meson, so we determine at\n>> +# runtime if the library is running from an installed location by checking\n>> +# for the presence or abscence of the dynamic tag.\n> \n> s/abscence/absence/\n> \n> Don't tell me this is a UK spelling :-)\n\nJust a typo ;-)\n\n> \n>> +\n>>  libcamera = shared_library('camera',\n>>                             libcamera_sources,\n>>                             install : true,\n>>                             link_with : libcamera_link_with,\n>>                             include_directories : includes,\n>> +                           build_rpath : '/',\n>>                             dependencies : libcamera_deps)\n> \n> I double-checked based on what criteria build_rpath would generate\n> DT_RPATH or DT_RUNPATH. It turns out this is controlled by the\n> --enable-new-dtags and --disable-new-dtags options to ld, with the\n> format generating DT_RUNPATH and the latter DT_RPATH. None of these\n> are set by meson by default. GNU ld defaulted to --enable-new-dtags in\n> binutils v2.24, released on December 2013, before gcc 5.1 (April 2015)\n> which is the earliest compiler version we support (I'm actually not even\n> sure we support gcc 5.1, I have gcc 5.4 installed in my test setup,\n> released on June 2016). Debian Jessie ships binutils v2.25 and Ubuntu\n> 16.04 ships binutils v2.26. We should thus be safe.\n> \n> Assuming the DT_RPATH comment for musl isn't right and the corresponding\n> condition can be removed,\n\nThe condition can't be removed. But having investigated it's possibly a\nlinker topic when using the Alpine distribution rather than an effect of\nthe musl c-library.\n\n\n> \n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> Otherwise let's discuss it.\n\nGoing to have to send a v4 of this series anyway now :-S\n\nCheck again then :-)\n\n\n>>  libcamera_dep = declare_dependency(sources : [libcamera_api, libcamera_ipa_api, libcamera_h],\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 D274560438\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 21 Feb 2020 13:37:09 +0100 (CET)","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 24538563;\n\tFri, 21 Feb 2020 13:37:09 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1582288629;\n\tbh=o8olI/rBqgBDKqzrlE3G/hlvGtliyCIRddXEwDS5Gdo=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=Gn64iArWPI7xHDJ1xak20ptN0YAoyZ3alx2QDiq1ZPsYdcCajiYBxnSkCPqa9YHr6\n\t2aLe4TSHsAE3iX59Ysf2UX+4kB4c4qto/IVnsf6+fvz3fHnvLTC6aHL18VRgX5643l\n\tRAvEIHGlD2Te7eJufsYYC9W/bsg+YMCkGVoakpz4=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","References":"<20200220165704.23600-1-kieran.bingham@ideasonboard.com>\n\t<20200220165704.23600-6-kieran.bingham@ideasonboard.com>\n\t<20200220212446.GO4998@pendragon.ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Openpgp":"preference=signencrypt","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":"<f8307984-c227-90ff-c02b-95ffc5e86000@ideasonboard.com>","Date":"Fri, 21 Feb 2020 12:37:06 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.9.1","MIME-Version":"1.0","In-Reply-To":"<20200220212446.GO4998@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v3 5/6] libcamera: ipa_manager: Search\n\tfor IPA libraries in build tree","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":"Fri, 21 Feb 2020 12:37:10 -0000"}},{"id":3831,"web_url":"https://patchwork.libcamera.org/comment/3831/","msgid":"<20200221130313.GB4955@pendragon.ideasonboard.com>","date":"2020-02-21T13:03:13","subject":"Re: [libcamera-devel] [PATCH v3 5/6] libcamera: ipa_manager: Search\n\tfor IPA libraries in build tree","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Fri, Feb 21, 2020 at 12:37:06PM +0000, Kieran Bingham wrote:\n> On 20/02/2020 21:24, Laurent Pinchart wrote:\n> > On Thu, Feb 20, 2020 at 04:57:03PM +0000, Kieran Bingham wrote:\n> >> When libcamera is built and tested (or used at all) before installing to\n> >> the configured prefix path, it will be unable to locate the IPA\n> >> binaries, or IPA binaries previously installed in the system paths may\n> >> be incorrect to load.\n> >>\n> >> Utilise the build_rpath dynamic tag which is stripped out by meson at\n> >> install time to determine at runtime if the library currently executing\n> >> has been installed or not.\n> >>\n> >> When not installed and running from a build tree, identify the location\n> >> of that tree by finding the path of the active libcamera.so itself, and\n> >> from that point add a relative path to be able to load the most recently\n> >> built IPA modules.\n> >>\n> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> >>\n> >> ---\n> >> v2:\n> >>  - Move get_runpath to ipa_manager.c : libcamera::elfRunPath()\n> >>  - Minor fixes\n> >>  - Squash elfRunPath() into this patch\n> >>\n> >> v3:\n> >>  - Full rework. It's just all different :-)\n> >>  - Maybe this one is going to cut it ...\n> >> ---\n> >>  src/libcamera/ipa_manager.cpp | 46 ++++++++++++++++++++++++++++++++++-\n> >>  src/libcamera/meson.build     |  6 +++++\n> >>  2 files changed, 51 insertions(+), 1 deletion(-)\n> >>\n> >> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n> >> index 3b1d4c0b295e..e60bf3dabebe 100644\n> >> --- a/src/libcamera/ipa_manager.cpp\n> >> +++ b/src/libcamera/ipa_manager.cpp\n> >> @@ -9,6 +9,9 @@\n> >>  \n> >>  #include <algorithm>\n> >>  #include <dirent.h>\n> >> +#include <dlfcn.h>\n> >> +#include <elf.h>\n> >> +#include <link.h>\n> >>  #include <string.h>\n> >>  #include <sys/types.h>\n> >>  \n> >> @@ -24,6 +27,30 @@\n> >>   * \\brief Image Processing Algorithm module manager\n> >>   */\n> >>  \n> >> +static bool isLibcameraInstalled()\n> >> +{\n> > \n> > I'd add a comment here to state\n> > \n> > \t/* musl doesn't declare _DYNAMIC in link.h, declare it manually. */\n> > \n> > to remember this is here for a reason.\n> \n> Added.\n> \n> > \n> >> +\textern ElfW(Dyn) _DYNAMIC[];\n> > \n> > Maybe add a blank line here ?\n> \n> Hrm... I used to have one. Maybe I lost it during a rebase :(\n> Added\n> \n> > \n> >> +\t/* DT_RUNPATH (DT_RPATH on musl) is removed on install. */\n> > \n> > Do we really need to check for DT_RPATH ? As far as I know the tag that\n> > is generated depends solely on the linker version, not on the C library.\n> > Does musl really generate DT_RPATH ?\n> > \n> >> +\tfor (const ElfW(Dyn) *dyn = _DYNAMIC; dyn->d_tag != DT_NULL; ++dyn)\n> >> +\t\tif (dyn->d_tag == DT_RUNPATH || dyn->d_tag == DT_RPATH)\n> >> +\t\t\treturn false;\n> > \n> > Braces for the 'for' ?\n> \n> (Re-)added\n> \n> > \n> >> +\n> >> +\treturn true;\n> >> +}\n> >> +\n> >> +static const char *libcameraPath()\n> >> +{\n> >> +\tDl_info info;\n> >> +\tint ret;\n> >> +\n> >> +\t/* look up our own symbol. */\n> > \n> > s/look/Look/\n> > \n> > That's a neat way to do it, we could have used an unrelated static\n> > function (such as IPAManager::instance) and inlined this code in\n> > IPAManager::IPAManager, but create a separate static function makes the\n> > code self-contained.\n> \n> Yes, and I envision this function being re-used (As you've noticed\n> later, we also have the proxy worker variable to deal with).\n> \n> >> +\tret = dladdr(reinterpret_cast<void *>(libcameraPath), &info);\n> >> +\tif (ret == 0)\n> >> +\t\treturn nullptr;\n> >> +\n> >> +\treturn info.dli_fname;\n> >> +}\n> > \n> > I would move these two functions just above IPAManager::IPAManager to\n> > keep them close to the location where they're used.\n> \n> Without them being member functions? Euewww ...\n> \n> I don't think they should be member functions, and in fact will have to\n> be moved to utils:: when we handle LIBCAMERA_IPA_PROXY_PATH\n> \n> I had been leaving that until I got this working.\n> \n> Would you prefer I put these functions in utils:: now?\n\nNo, it's fine leaving them here for now, we can rework that later when\ndealing with the proxy workers path.\n\n> >> +\n> >>  namespace libcamera {\n> >>  \n> >>  LOG_DEFINE_CATEGORY(IPAManager)\n> >> @@ -108,7 +135,24 @@ IPAManager::IPAManager()\n> >>  \t\t\t\t<< \"No IPA found in '\" << modulePaths << \"'\";\n> >>  \t}\n> >>  \n> >> -\t/* Load IPAs from the installed system path. */\n> >> +\t/*\n> >> +\t * When libcamera is used before it is installed, load IPAs from the\n> >> +\t * same build directory as the libcamera library itself. This requires\n> >> +\t * identifying the path of the libcamera.so, and referencing a relative\n> >> +\t * path for the IPA from that point.\n> > \n> > Maybe add \"We need to recursive into one level of sub-directories to\n\ns/recursive/recurse/ (or \"search recursively\")\n\n> > match the build tree.\" ?\n> \n> Added\n> \n> >> +\t */\n> >> +\tif (!isLibcameraInstalled()) {\n> >> +\t\tstd::string ipaBuildPath = utils::dirname(libcameraPath()) + \"/../ipa\";\n> >> +\t\tconstexpr int maxDepth = 1;\n> >> +\n> >> +\t\tLOG(IPAManager, Info)\n> >> +\t\t\t<< \"libcamera is not installed. Adding '\"\n> >> +\t\t\t<< ipaBuildPath << \"' to the IPA search path\";\n> >> +\n> >> +\t\tipaCount += addDir(ipaBuildPath.c_str(), maxDepth);\n> >> +\t}\n> >> +\n> >> +\t/* Finally try to load IPAs from the installed system path. */\n> >>  \tret = addDir(IPA_MODULE_DIR);\n> >>  \tif (ret > 0)\n> >>  \t\tipaCount += ret;\n> >> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> >> index 1e5b54b34078..88658ac563f7 100644\n> >> --- a/src/libcamera/meson.build\n> >> +++ b/src/libcamera/meson.build\n> >> @@ -107,11 +107,17 @@ if get_option('android')\n> >>      libcamera_link_with += android_camera_metadata\n> >>  endif\n> >>  \n> >> +# We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.\n> >> +# The build_rpath is stripped at install time by meson, so we determine at\n> >> +# runtime if the library is running from an installed location by checking\n> >> +# for the presence or abscence of the dynamic tag.\n> > \n> > s/abscence/absence/\n> > \n> > Don't tell me this is a UK spelling :-)\n> \n> Just a typo ;-)\n> \n> >> +\n> >>  libcamera = shared_library('camera',\n> >>                             libcamera_sources,\n> >>                             install : true,\n> >>                             link_with : libcamera_link_with,\n> >>                             include_directories : includes,\n> >> +                           build_rpath : '/',\n> >>                             dependencies : libcamera_deps)\n> > \n> > I double-checked based on what criteria build_rpath would generate\n> > DT_RPATH or DT_RUNPATH. It turns out this is controlled by the\n> > --enable-new-dtags and --disable-new-dtags options to ld, with the\n> > format generating DT_RUNPATH and the latter DT_RPATH. None of these\n> > are set by meson by default. GNU ld defaulted to --enable-new-dtags in\n> > binutils v2.24, released on December 2013, before gcc 5.1 (April 2015)\n> > which is the earliest compiler version we support (I'm actually not even\n> > sure we support gcc 5.1, I have gcc 5.4 installed in my test setup,\n> > released on June 2016). Debian Jessie ships binutils v2.25 and Ubuntu\n> > 16.04 ships binutils v2.26. We should thus be safe.\n> > \n> > Assuming the DT_RPATH comment for musl isn't right and the corresponding\n> > condition can be removed,\n> \n> The condition can't be removed. But having investigated it's possibly a\n> linker topic when using the Alpine distribution rather than an effect of\n> the musl c-library.\n\nThen updating the comment above should be all we need.\n\n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > \n> > Otherwise let's discuss it.\n> \n> Going to have to send a v4 of this series anyway now :-S\n> \n> Check again then :-)\n> \n> >>  libcamera_dep = declare_dependency(sources : [libcamera_api, libcamera_ipa_api, libcamera_h],","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 F161B60438\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 21 Feb 2020 14:03:33 +0100 (CET)","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 70104563;\n\tFri, 21 Feb 2020 14:03:33 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1582290213;\n\tbh=Rf9ekEc+AvX1DFglxbPE9Ym6f+AWZLY5RjzTToRsg30=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=TavxN+inWYF+8k48KN9LxFZjEh387UHDs6X+zJiAiLTwFJDtEDL2nOXxvOfieMs+F\n\tjy+XlAE6H/M0ASdDgReO/aODNcBQ+6unsw/y0uUmLLiGD0RaDb7qEYukB1/GdREJcy\n\tUsK+/PU+m14F+Ql0jkXLvSeqNp3/bhsWHhw/Snvc=","Date":"Fri, 21 Feb 2020 15:03:13 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Message-ID":"<20200221130313.GB4955@pendragon.ideasonboard.com>","References":"<20200220165704.23600-1-kieran.bingham@ideasonboard.com>\n\t<20200220165704.23600-6-kieran.bingham@ideasonboard.com>\n\t<20200220212446.GO4998@pendragon.ideasonboard.com>\n\t<f8307984-c227-90ff-c02b-95ffc5e86000@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<f8307984-c227-90ff-c02b-95ffc5e86000@ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 5/6] libcamera: ipa_manager: Search\n\tfor IPA libraries in build tree","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":"Fri, 21 Feb 2020 13:03:34 -0000"}},{"id":3833,"web_url":"https://patchwork.libcamera.org/comment/3833/","msgid":"<6c65e2d6-631a-27b0-eb55-cb4689fbe293@ideasonboard.com>","date":"2020-02-21T13:06:15","subject":"Re: [libcamera-devel] [PATCH v3 5/6] libcamera: ipa_manager: Search\n\tfor IPA libraries in build tree","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 21/02/2020 12:37, Kieran Bingham wrote:\n> Hi Laurent,\n> \n> On 20/02/2020 21:24, Laurent Pinchart wrote:\n>> Hi Kieran,\n>>\n>> Thank you for the patch.\n>>\n>> On Thu, Feb 20, 2020 at 04:57:03PM +0000, Kieran Bingham wrote:\n>>> When libcamera is built and tested (or used at all) before installing to\n>>> the configured prefix path, it will be unable to locate the IPA\n>>> binaries, or IPA binaries previously installed in the system paths may\n>>> be incorrect to load.\n>>>\n>>> Utilise the build_rpath dynamic tag which is stripped out by meson at\n>>> install time to determine at runtime if the library currently executing\n>>> has been installed or not.\n>>>\n>>> When not installed and running from a build tree, identify the location\n>>> of that tree by finding the path of the active libcamera.so itself, and\n>>> from that point add a relative path to be able to load the most recently\n>>> built IPA modules.\n>>>\n>>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>>>\n>>> ---\n>>> v2:\n>>>  - Move get_runpath to ipa_manager.c : libcamera::elfRunPath()\n>>>  - Minor fixes\n>>>  - Squash elfRunPath() into this patch\n>>>\n>>> v3:\n>>>  - Full rework. It's just all different :-)\n>>>  - Maybe this one is going to cut it ...\n>>> ---\n>>>  src/libcamera/ipa_manager.cpp | 46 ++++++++++++++++++++++++++++++++++-\n>>>  src/libcamera/meson.build     |  6 +++++\n>>>  2 files changed, 51 insertions(+), 1 deletion(-)\n>>>\n>>> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp\n>>> index 3b1d4c0b295e..e60bf3dabebe 100644\n>>> --- a/src/libcamera/ipa_manager.cpp\n>>> +++ b/src/libcamera/ipa_manager.cpp\n>>> @@ -9,6 +9,9 @@\n>>>  \n>>>  #include <algorithm>\n>>>  #include <dirent.h>\n>>> +#include <dlfcn.h>\n>>> +#include <elf.h>\n>>> +#include <link.h>\n>>>  #include <string.h>\n>>>  #include <sys/types.h>\n>>>  \n>>> @@ -24,6 +27,30 @@\n>>>   * \\brief Image Processing Algorithm module manager\n>>>   */\n>>>  \n>>> +static bool isLibcameraInstalled()\n>>> +{\n>>\n>> I'd add a comment here to state\n>>\n>> \t/* musl doesn't declare _DYNAMIC in link.h, declare it manually. */\n>>\n>> to remember this is here for a reason.\n> \n> Added.\n> \n>>\n>>> +\textern ElfW(Dyn) _DYNAMIC[];\n>>\n>> Maybe add a blank line here ?\n> \n> Hrm... I used to have one. Maybe I lost it during a rebase :(\n> Added\n> \n>>\n>>> +\t/* DT_RUNPATH (DT_RPATH on musl) is removed on install. */\n>>\n>> Do we really need to check for DT_RPATH ? As far as I know the tag that\n>> is generated depends solely on the linker version, not on the C library.\n>> Does musl really generate DT_RPATH ?\n>>\n>>> +\tfor (const ElfW(Dyn) *dyn = _DYNAMIC; dyn->d_tag != DT_NULL; ++dyn)\n>>> +\t\tif (dyn->d_tag == DT_RUNPATH || dyn->d_tag == DT_RPATH)\n>>> +\t\t\treturn false;\n>>\n>> Braces for the 'for' ?\n> \n> (Re-)added\n> \n>>\n>>> +\n>>> +\treturn true;\n>>> +}\n>>> +\n>>> +static const char *libcameraPath()\n>>> +{\n>>> +\tDl_info info;\n>>> +\tint ret;\n>>> +\n>>> +\t/* look up our own symbol. */\n>>\n>> s/look/Look/\n>>\n>> That's a neat way to do it, we could have used an unrelated static\n>> function (such as IPAManager::instance) and inlined this code in\n>> IPAManager::IPAManager, but create a separate static function makes the\n>> code self-contained.\n> \n> Yes, and I envision this function being re-used (As you've noticed\n> later, we also have the proxy worker variable to deal with).\n> \n>>\n>>> +\tret = dladdr(reinterpret_cast<void *>(libcameraPath), &info);\n>>> +\tif (ret == 0)\n>>> +\t\treturn nullptr;\n>>> +\n>>> +\treturn info.dli_fname;\n>>> +}\n>>\n>> I would move these two functions just above IPAManager::IPAManager to\n>> keep them close to the location where they're used.\n> \n> Without them being member functions? Euewww ...\n\nsorry - I just realised the position of this would /not/ put the\nfunctions between other IPAManager functions which is what I thought on\nfirst read.\n\nPutting just before should be fine.\n\n> I don't think they should be member functions, and in fact will have to\n> be moved to utils:: when we handle LIBCAMERA_IPA_PROXY_PATH\n> \n> I had been leaving that until I got this working.\n> \n> Would you prefer I put these functions in utils:: now?\n> \n> \n>>> +\n>>>  namespace libcamera {\n>>>  \n>>>  LOG_DEFINE_CATEGORY(IPAManager)\n>>> @@ -108,7 +135,24 @@ IPAManager::IPAManager()\n>>>  \t\t\t\t<< \"No IPA found in '\" << modulePaths << \"'\";\n>>>  \t}\n>>>  \n>>> -\t/* Load IPAs from the installed system path. */\n>>> +\t/*\n>>> +\t * When libcamera is used before it is installed, load IPAs from the\n>>> +\t * same build directory as the libcamera library itself. This requires\n>>> +\t * identifying the path of the libcamera.so, and referencing a relative\n>>> +\t * path for the IPA from that point.\n>>\n>> Maybe add \"We need to recursive into one level of sub-directories to\n>> match the build tree.\" ?\n> \n> Added\n> \n>>\n>>> +\t */\n>>> +\tif (!isLibcameraInstalled()) {\n>>> +\t\tstd::string ipaBuildPath = utils::dirname(libcameraPath()) + \"/../ipa\";\n>>> +\t\tconstexpr int maxDepth = 1;\n>>> +\n>>> +\t\tLOG(IPAManager, Info)\n>>> +\t\t\t<< \"libcamera is not installed. Adding '\"\n>>> +\t\t\t<< ipaBuildPath << \"' to the IPA search path\";\n>>> +\n>>> +\t\tipaCount += addDir(ipaBuildPath.c_str(), maxDepth);\n>>> +\t}\n>>> +\n>>> +\t/* Finally try to load IPAs from the installed system path. */\n>>>  \tret = addDir(IPA_MODULE_DIR);\n>>>  \tif (ret > 0)\n>>>  \t\tipaCount += ret;\n>>> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n>>> index 1e5b54b34078..88658ac563f7 100644\n>>> --- a/src/libcamera/meson.build\n>>> +++ b/src/libcamera/meson.build\n>>> @@ -107,11 +107,17 @@ if get_option('android')\n>>>      libcamera_link_with += android_camera_metadata\n>>>  endif\n>>>  \n>>> +# We add '/' to the build_rpath as a 'safe' path to act as a boolean flag.\n>>> +# The build_rpath is stripped at install time by meson, so we determine at\n>>> +# runtime if the library is running from an installed location by checking\n>>> +# for the presence or abscence of the dynamic tag.\n>>\n>> s/abscence/absence/\n>>\n>> Don't tell me this is a UK spelling :-)\n> \n> Just a typo ;-)\n> \n>>\n>>> +\n>>>  libcamera = shared_library('camera',\n>>>                             libcamera_sources,\n>>>                             install : true,\n>>>                             link_with : libcamera_link_with,\n>>>                             include_directories : includes,\n>>> +                           build_rpath : '/',\n>>>                             dependencies : libcamera_deps)\n>>\n>> I double-checked based on what criteria build_rpath would generate\n>> DT_RPATH or DT_RUNPATH. It turns out this is controlled by the\n>> --enable-new-dtags and --disable-new-dtags options to ld, with the\n>> format generating DT_RUNPATH and the latter DT_RPATH. None of these\n>> are set by meson by default. GNU ld defaulted to --enable-new-dtags in\n>> binutils v2.24, released on December 2013, before gcc 5.1 (April 2015)\n>> which is the earliest compiler version we support (I'm actually not even\n>> sure we support gcc 5.1, I have gcc 5.4 installed in my test setup,\n>> released on June 2016). Debian Jessie ships binutils v2.25 and Ubuntu\n>> 16.04 ships binutils v2.26. We should thus be safe.\n>>\n>> Assuming the DT_RPATH comment for musl isn't right and the corresponding\n>> condition can be removed,\n> \n> The condition can't be removed. But having investigated it's possibly a\n> linker topic when using the Alpine distribution rather than an effect of\n> the musl c-library.\n> \n> \n>>\n>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>>\n>> Otherwise let's discuss it.\n> \n> Going to have to send a v4 of this series anyway now :-S\n> \n> Check again then :-)\n> \n> \n>>>  libcamera_dep = declare_dependency(sources : [libcamera_api, libcamera_ipa_api, libcamera_h],\n>>\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 97D5760438\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 21 Feb 2020 14:06:19 +0100 (CET)","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 0C90D563;\n\tFri, 21 Feb 2020 14:06:18 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1582290379;\n\tbh=qfe/YwPyCbBFRYaA01VR0o3+DsOiYgF5XOq5Qb9eGL0=;\n\th=Reply-To:Subject:From:To:Cc:References:Date:In-Reply-To:From;\n\tb=f+nseqqOGXk1GyOH4aFe3dYP3ouLZyDv1qrYQFcYWDyLeRvCcwDOI6ge533eV90Lv\n\tucOqPYngLF4m2uuzIo8BQFglCQ6BjC0aGTYTQ/4XYMKiirfPVBeMBYlBZMc223qmcy\n\tc8P07J8hmDo1PUhTLv/vbnGqoCl847+D2t8du8SA=","Reply-To":"kieran.bingham@ideasonboard.com","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","References":"<20200220165704.23600-1-kieran.bingham@ideasonboard.com>\n\t<20200220165704.23600-6-kieran.bingham@ideasonboard.com>\n\t<20200220212446.GO4998@pendragon.ideasonboard.com>\n\t<f8307984-c227-90ff-c02b-95ffc5e86000@ideasonboard.com>","Openpgp":"preference=signencrypt","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":"<6c65e2d6-631a-27b0-eb55-cb4689fbe293@ideasonboard.com>","Date":"Fri, 21 Feb 2020 13:06:15 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.9.1","MIME-Version":"1.0","In-Reply-To":"<f8307984-c227-90ff-c02b-95ffc5e86000@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v3 5/6] libcamera: ipa_manager: Search\n\tfor IPA libraries in build tree","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":"Fri, 21 Feb 2020 13:06:19 -0000"}}]