[libcamera-devel,v4] libcamera: utils: adapt libcameraPath to match use cases

Message ID 20200319151919.GA4693@kaaira-HP-Pavilion-Notebook
State Superseded
Headers show
Series
  • [libcamera-devel,v4] libcamera: utils: adapt libcameraPath to match use cases
Related show

Commit Message

Kaaira Gupta March 19, 2020, 3:19 p.m. UTC
The two callers of functions libcameraPath() and isLibcameraInstalled()
end up using the same process and finally use the path with
libcamera.so. Hence write a function libcameraBuildPath() which
combines their functions and returns the root of the build sources
when the library has not been installed, but is running, thereby making
call sites simpler.

When the library is installed, libcameraBuildPath() will return an empty
string.

Make changes in the call sites accordingly.

Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in>
---
Changes since v3:
	- removed isLibcameraInstalled() from utils.h
	- reformatted the code to reduce indentation of the code that
	  implements actual logic, also, to catch the errors
	  immediately.
	- changes in spellings and whitespaces in commit messages.

Changes since v2:
        - rename subject line and add necessary details in commit
          message.
        - Solve whitespace issues.
        - specify the final 'slash' for directory path in return
          statement itself.

Changes since v1:
        - rename function to libcameraBuildPath() instead, and return
          the root of libcamera source, and not the source directory
          root.
        - Return empty string instead of nullptr when ret==0
 
 src/libcamera/include/utils.h |  2 +-
 src/libcamera/ipa_manager.cpp |  5 +++--
 src/libcamera/ipa_proxy.cpp   |  6 +++---
 src/libcamera/utils.cpp       | 16 +++++++++-------
 4 files changed, 16 insertions(+), 13 deletions(-)

Comments

Kaaira Gupta March 19, 2020, 3:22 p.m. UTC | #1
Please ignore.
Sent wrong version.

On Thu, 19 Mar, 2020, 20:49 Kaaira Gupta, <kgupta@es.iitr.ac.in> wrote:

> The two callers of functions libcameraPath() and isLibcameraInstalled()
> end up using the same process and finally use the path with
> libcamera.so. Hence write a function libcameraBuildPath() which
> combines their functions and returns the root of the build sources
> when the library has not been installed, but is running, thereby making
> call sites simpler.
>
> When the library is installed, libcameraBuildPath() will return an empty
> string.
>
> Make changes in the call sites accordingly.
>
> Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in>
> ---
> Changes since v3:
>         - removed isLibcameraInstalled() from utils.h
>         - reformatted the code to reduce indentation of the code that
>           implements actual logic, also, to catch the errors
>           immediately.
>         - changes in spellings and whitespaces in commit messages.
>
> Changes since v2:
>         - rename subject line and add necessary details in commit
>           message.
>         - Solve whitespace issues.
>         - specify the final 'slash' for directory path in return
>           statement itself.
>
> Changes since v1:
>         - rename function to libcameraBuildPath() instead, and return
>           the root of libcamera source, and not the source directory
>           root.
>         - Return empty string instead of nullptr when ret==0
>
>  src/libcamera/include/utils.h |  2 +-
>  src/libcamera/ipa_manager.cpp |  5 +++--
>  src/libcamera/ipa_proxy.cpp   |  6 +++---
>  src/libcamera/utils.cpp       | 16 +++++++++-------
>  4 files changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h
> index bc96e79..5dea8d2 100644
> --- a/src/libcamera/include/utils.h
> +++ b/src/libcamera/include/utils.h
> @@ -145,7 +145,7 @@ details::StringSplitter split(const std::string &str,
> const std::string &delim);
>
>  bool isLibcameraInstalled();
>
> -std::string libcameraPath();
> +std::string libcameraBuildPath();
>
>  } /* namespace utils */
>
> diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
> index 0bd280c..bcaae35 100644
> --- a/src/libcamera/ipa_manager.cpp
> +++ b/src/libcamera/ipa_manager.cpp
> @@ -119,8 +119,9 @@ IPAManager::IPAManager()
>          * path for the IPA from that point. We need to recurse one level
> of
>          * sub-directories to match the build tree.
>          */
> -       if (!utils::isLibcameraInstalled()) {
> -               std::string ipaBuildPath =
> utils::dirname(utils::libcameraPath()) + "/../ipa";
> +       std::string root = utils::libcameraBuildPath();
> +       if (!root.empty()) {
> +               std::string ipaBuildPath = root + "src/ipa";
>                 constexpr int maxDepth = 1;
>
>                 LOG(IPAManager, Info)
> diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp
> index 2f866cc..5fd88a4 100644
> --- a/src/libcamera/ipa_proxy.cpp
> +++ b/src/libcamera/ipa_proxy.cpp
> @@ -97,9 +97,9 @@ std::string IPAProxy::resolvePath(const std::string
> &file) const
>          * This requires identifying the path of the libcamera.so, and
>          * referencing a relative path for the proxy workers from that
> point.
>          */
> -       if (!utils::isLibcameraInstalled()) {
> -               std::string ipaProxyDir =
> utils::dirname(utils::libcameraPath())
> -                                         + "/proxy/worker";
> +       std::string root = utils::libcameraBuildPath();
> +       if (!root.empty()) {
> +               std::string ipaProxyDir = root +
> "src/libcamera/proxy/worker";
>
>                 LOG(IPAProxy, Info)
>                         << "libcamera is not installed. Loading proxy
> workers from'"
> diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
> index 7e118fa..42c5c76 100644
> --- a/src/libcamera/utils.cpp
> +++ b/src/libcamera/utils.cpp
> @@ -347,16 +347,18 @@ bool isLibcameraInstalled()
>   *
>   * \return A string stating the path
>   */
> -std::string libcameraPath()
> +std::string libcameraBuildPath()
>  {
> -       Dl_info info;
> +       if (!isLibcameraInstalled()) {
> +               Dl_info info;
>
> -       /* Look up our own symbol. */
> -       int ret = dladdr(reinterpret_cast<void *>(libcameraPath), &info);
> -       if (ret == 0)
> -               return nullptr;
> +               /* Look up our own symbol. */
> +               int ret = dladdr(reinterpret_cast<void
> *>(libcameraBuildPath), &info);
> +               if (ret)
> +                       return dirname(info.dli_fname) + "/../../";
> +       }
>
> -       return info.dli_fname;
> +       return std::string();
>  }
>
>  } /* namespace utils */
> --
> 2.17.1
>
>

Patch

diff --git a/src/libcamera/include/utils.h b/src/libcamera/include/utils.h
index bc96e79..5dea8d2 100644
--- a/src/libcamera/include/utils.h
+++ b/src/libcamera/include/utils.h
@@ -145,7 +145,7 @@  details::StringSplitter split(const std::string &str, const std::string &delim);
 
 bool isLibcameraInstalled();
 
-std::string libcameraPath();
+std::string libcameraBuildPath();
 
 } /* namespace utils */
 
diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp
index 0bd280c..bcaae35 100644
--- a/src/libcamera/ipa_manager.cpp
+++ b/src/libcamera/ipa_manager.cpp
@@ -119,8 +119,9 @@  IPAManager::IPAManager()
 	 * path for the IPA from that point. We need to recurse one level of
 	 * sub-directories to match the build tree.
 	 */
-	if (!utils::isLibcameraInstalled()) {
-		std::string ipaBuildPath = utils::dirname(utils::libcameraPath()) + "/../ipa";
+	std::string root = utils::libcameraBuildPath();
+	if (!root.empty()) {
+		std::string ipaBuildPath = root + "src/ipa";
 		constexpr int maxDepth = 1;
 
 		LOG(IPAManager, Info)
diff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp
index 2f866cc..5fd88a4 100644
--- a/src/libcamera/ipa_proxy.cpp
+++ b/src/libcamera/ipa_proxy.cpp
@@ -97,9 +97,9 @@  std::string IPAProxy::resolvePath(const std::string &file) const
 	 * This requires identifying the path of the libcamera.so, and
 	 * referencing a relative path for the proxy workers from that point.
 	 */
-	if (!utils::isLibcameraInstalled()) {
-		std::string ipaProxyDir = utils::dirname(utils::libcameraPath())
-					  + "/proxy/worker";
+	std::string root = utils::libcameraBuildPath();
+	if (!root.empty()) {
+		std::string ipaProxyDir = root + "src/libcamera/proxy/worker";
 
 		LOG(IPAProxy, Info)
 			<< "libcamera is not installed. Loading proxy workers from'"
diff --git a/src/libcamera/utils.cpp b/src/libcamera/utils.cpp
index 7e118fa..42c5c76 100644
--- a/src/libcamera/utils.cpp
+++ b/src/libcamera/utils.cpp
@@ -347,16 +347,18 @@  bool isLibcameraInstalled()
  *
  * \return A string stating the path
  */
-std::string libcameraPath()
+std::string libcameraBuildPath()
 {
-	Dl_info info;
+	if (!isLibcameraInstalled()) {
+		Dl_info info;
 
-	/* Look up our own symbol. */
-	int ret = dladdr(reinterpret_cast<void *>(libcameraPath), &info);
-	if (ret == 0)
-		return nullptr;
+		/* Look up our own symbol. */
+		int ret = dladdr(reinterpret_cast<void *>(libcameraBuildPath), &info);
+		if (ret)
+			return dirname(info.dli_fname) + "/../../";
+	}
 
-	return info.dli_fname;
+	return std::string();
 }
 
 } /* namespace utils */