[libcamera-devel,v5,02/12] libcamera: pipeline: Add a platform configuration file helper
diff mbox series

Message ID 20230118085953.7027-3-naush@raspberrypi.com
State Accepted
Headers show
Series
  • Raspberry Pi: Platform configuration and buffer allocation improvements
Related show

Commit Message

Naushir Patuck Jan. 18, 2023, 8:59 a.m. UTC
Add a new helper function PipelineHandler::configurationFile() that returns
the full path of a named configuration file. This configuration file may be read
by pipeline handlers for platform specific configuration parameters on
initialisation.

The mechanism for searching for the configuration file is similar to the IPA
configuration file:

- In the source tree if libcamera is not installed
- Otherwise in standard system locations (etc and share directories).

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

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
---
 include/libcamera/internal/pipeline_handler.h |  3 +
 src/libcamera/pipeline_handler.cpp            | 60 +++++++++++++++++++
 2 files changed, 63 insertions(+)

Comments

Kieran Bingham Jan. 20, 2023, 10:45 a.m. UTC | #1
Quoting Naushir Patuck via libcamera-devel (2023-01-18 08:59:43)
> Add a new helper function PipelineHandler::configurationFile() that returns
> the full path of a named configuration file. This configuration file may be read
> by pipeline handlers for platform specific configuration parameters on
> initialisation.
> 
> The mechanism for searching for the configuration file is similar to the IPA
> configuration file:
> 
> - In the source tree if libcamera is not installed
> - Otherwise in standard system locations (etc and share directories).
> 
> When stored in the source tree, configuration files shall be located in a 'data'
> subdirectory of their respective pipeline handler directory.
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

This patch itself looks fine to me. I do wonder when we should just have
a global libcamera configuration file though.

But each pipeline config will be distinct, and having each one
separately and directly documented, (as well as having different preset
options) seems reaonable and useful to me.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  include/libcamera/internal/pipeline_handler.h |  3 +
>  src/libcamera/pipeline_handler.cpp            | 60 +++++++++++++++++++
>  2 files changed, 63 insertions(+)
> 
> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> index ec4f662d7399..4c4dfe62a680 100644
> --- a/include/libcamera/internal/pipeline_handler.h
> +++ b/include/libcamera/internal/pipeline_handler.h
> @@ -65,6 +65,9 @@ public:
>         bool completeBuffer(Request *request, FrameBuffer *buffer);
>         void completeRequest(Request *request);
>  
> +       std::string configurationFile(const std::string &subdir,
> +                                     const std::string &name) const;
> +
>         const char *name() const { return name_; }
>  
>  protected:
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index cfade4908118..a515ad5ecffb 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -8,6 +8,7 @@
>  #include "libcamera/internal/pipeline_handler.h"
>  
>  #include <chrono>
> +#include <sys/stat.h>
>  #include <sys/sysmacros.h>
>  
>  #include <libcamera/base/log.h>
> @@ -534,6 +535,65 @@ void PipelineHandler::completeRequest(Request *request)
>         }
>  }
>  
> +/**
> + * \brief Retrieve the absolute path to a platform configuration file
> + * \param[in] subdir The pipeline handler specific subdirectory name
> + * \param[in] name The configuration file name
> + *
> + * This function locates a named platform configuration file and returns
> + * its absolute path to the pipeline handler. It searches the following
> + * directories, in order:
> + *
> + * - If libcamera is not installed, the src/libcamera/pipeline/<subdir>/data/
> + *   directory within the source tree ; otherwise
> + * - The system data (share/libcamera/pipeline/<subdir>) directory.
> + *
> + * The system directories are not searched if libcamera is not installed.
> + *
> + * \return The full path to the pipeline handler configuration file, or an empty
> + * string if no configuration file can be found
> + */
> +std::string PipelineHandler::configurationFile(const std::string &subdir,
> +                                              const std::string &name) const
> +{
> +       std::string confPath;
> +       struct stat statbuf;
> +       int ret;
> +
> +       std::string root = utils::libcameraSourcePath();
> +       if (!root.empty()) {
> +               /*
> +                * When libcamera is used before it is installed, load
> +                * configuration files from the source directory. The
> +                * configuration files are then located in the 'data'
> +                * subdirectory of the corresponding pipeline handler.
> +                */
> +               std::string confDir = root + "src/libcamera/pipeline/";
> +               confPath = confDir + subdir + "/data/" + name;
> +
> +               LOG(Pipeline, Info)
> +                       << "libcamera is not installed. Loading platform configuration file from '"
> +                       << confPath << "'";
> +
> +               ret = stat(confPath.c_str(), &statbuf);
> +               if (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)
> +                       return confPath;
> +       } else {
> +               /* Else look in the system locations. */
> +               confPath = std::string(LIBCAMERA_DATA_DIR)
> +                               + "/pipeline/" + subdir + '/' + name;
> +               ret = stat(confPath.c_str(), &statbuf);
> +               if (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)
> +                       return confPath;
> +       }
> +
> +       LOG(Pipeline, Error)
> +               << "Configuration file '" << confPath
> +               << "' not found for pipeline handler '" << PipelineHandler::name() << "'";
> +
> +       return std::string();
> +}
> +
>  /**
>   * \brief Register a camera to the camera manager and pipeline handler
>   * \param[in] camera The camera to be added
> -- 
> 2.25.1
>
Laurent Pinchart Jan. 22, 2023, 11:24 p.m. UTC | #2
Hello,

On Fri, Jan 20, 2023 at 10:45:45AM +0000, Kieran Bingham via libcamera-devel wrote:
> Quoting Naushir Patuck via libcamera-devel (2023-01-18 08:59:43)
> > Add a new helper function PipelineHandler::configurationFile() that returns
> > the full path of a named configuration file. This configuration file may be read
> > by pipeline handlers for platform specific configuration parameters on
> > initialisation.
> > 
> > The mechanism for searching for the configuration file is similar to the IPA
> > configuration file:
> > 
> > - In the source tree if libcamera is not installed
> > - Otherwise in standard system locations (etc and share directories).
> > 
> > When stored in the source tree, configuration files shall be located in a 'data'
> > subdirectory of their respective pipeline handler directory.
> > 
> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
> 
> This patch itself looks fine to me. I do wonder when we should just have
> a global libcamera configuration file though.

I agree. Short term this is fine, but I expect we'll revisit the
configuration file topic.

> But each pipeline config will be distinct, and having each one
> separately and directly documented, (as well as having different preset
> options) seems reaonable and useful to me.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > ---
> >  include/libcamera/internal/pipeline_handler.h |  3 +
> >  src/libcamera/pipeline_handler.cpp            | 60 +++++++++++++++++++
> >  2 files changed, 63 insertions(+)
> > 
> > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
> > index ec4f662d7399..4c4dfe62a680 100644
> > --- a/include/libcamera/internal/pipeline_handler.h
> > +++ b/include/libcamera/internal/pipeline_handler.h
> > @@ -65,6 +65,9 @@ public:
> >         bool completeBuffer(Request *request, FrameBuffer *buffer);
> >         void completeRequest(Request *request);
> >  
> > +       std::string configurationFile(const std::string &subdir,
> > +                                     const std::string &name) const;
> > +
> >         const char *name() const { return name_; }
> >  
> >  protected:
> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> > index cfade4908118..a515ad5ecffb 100644
> > --- a/src/libcamera/pipeline_handler.cpp
> > +++ b/src/libcamera/pipeline_handler.cpp
> > @@ -8,6 +8,7 @@
> >  #include "libcamera/internal/pipeline_handler.h"
> >  
> >  #include <chrono>
> > +#include <sys/stat.h>
> >  #include <sys/sysmacros.h>
> >  
> >  #include <libcamera/base/log.h>
> > @@ -534,6 +535,65 @@ void PipelineHandler::completeRequest(Request *request)
> >         }
> >  }
> >  
> > +/**
> > + * \brief Retrieve the absolute path to a platform configuration file
> > + * \param[in] subdir The pipeline handler specific subdirectory name
> > + * \param[in] name The configuration file name
> > + *
> > + * This function locates a named platform configuration file and returns
> > + * its absolute path to the pipeline handler. It searches the following
> > + * directories, in order:
> > + *
> > + * - If libcamera is not installed, the src/libcamera/pipeline/<subdir>/data/
> > + *   directory within the source tree ; otherwise
> > + * - The system data (share/libcamera/pipeline/<subdir>) directory.
> > + *
> > + * The system directories are not searched if libcamera is not installed.
> > + *
> > + * \return The full path to the pipeline handler configuration file, or an empty
> > + * string if no configuration file can be found
> > + */
> > +std::string PipelineHandler::configurationFile(const std::string &subdir,
> > +                                              const std::string &name) const
> > +{
> > +       std::string confPath;
> > +       struct stat statbuf;
> > +       int ret;
> > +
> > +       std::string root = utils::libcameraSourcePath();
> > +       if (!root.empty()) {
> > +               /*
> > +                * When libcamera is used before it is installed, load
> > +                * configuration files from the source directory. The
> > +                * configuration files are then located in the 'data'
> > +                * subdirectory of the corresponding pipeline handler.
> > +                */
> > +               std::string confDir = root + "src/libcamera/pipeline/";
> > +               confPath = confDir + subdir + "/data/" + name;
> > +
> > +               LOG(Pipeline, Info)
> > +                       << "libcamera is not installed. Loading platform configuration file from '"
> > +                       << confPath << "'";
> > +
> > +               ret = stat(confPath.c_str(), &statbuf);
> > +               if (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)
> > +                       return confPath;

Those three lines are the same in the other branch, they can be moved
after the else { ... }.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > +       } else {
> > +               /* Else look in the system locations. */
> > +               confPath = std::string(LIBCAMERA_DATA_DIR)
> > +                               + "/pipeline/" + subdir + '/' + name;
> > +               ret = stat(confPath.c_str(), &statbuf);
> > +               if (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)
> > +                       return confPath;
> > +       }
> > +
> > +       LOG(Pipeline, Error)
> > +               << "Configuration file '" << confPath
> > +               << "' not found for pipeline handler '" << PipelineHandler::name() << "'";
> > +
> > +       return std::string();
> > +}
> > +
> >  /**
> >   * \brief Register a camera to the camera manager and pipeline handler
> >   * \param[in] camera The camera to be added

Patch
diff mbox series

diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
index ec4f662d7399..4c4dfe62a680 100644
--- a/include/libcamera/internal/pipeline_handler.h
+++ b/include/libcamera/internal/pipeline_handler.h
@@ -65,6 +65,9 @@  public:
 	bool completeBuffer(Request *request, FrameBuffer *buffer);
 	void completeRequest(Request *request);
 
+	std::string configurationFile(const std::string &subdir,
+				      const std::string &name) const;
+
 	const char *name() const { return name_; }
 
 protected:
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index cfade4908118..a515ad5ecffb 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -8,6 +8,7 @@ 
 #include "libcamera/internal/pipeline_handler.h"
 
 #include <chrono>
+#include <sys/stat.h>
 #include <sys/sysmacros.h>
 
 #include <libcamera/base/log.h>
@@ -534,6 +535,65 @@  void PipelineHandler::completeRequest(Request *request)
 	}
 }
 
+/**
+ * \brief Retrieve the absolute path to a platform configuration file
+ * \param[in] subdir The pipeline handler specific subdirectory name
+ * \param[in] name The configuration file name
+ *
+ * This function locates a named platform configuration file and returns
+ * its absolute path to the pipeline handler. It searches the following
+ * directories, in order:
+ *
+ * - If libcamera is not installed, the src/libcamera/pipeline/<subdir>/data/
+ *   directory within the source tree ; otherwise
+ * - The system data (share/libcamera/pipeline/<subdir>) directory.
+ *
+ * The system directories are not searched if libcamera is not installed.
+ *
+ * \return The full path to the pipeline handler configuration file, or an empty
+ * string if no configuration file can be found
+ */
+std::string PipelineHandler::configurationFile(const std::string &subdir,
+					       const std::string &name) const
+{
+	std::string confPath;
+	struct stat statbuf;
+	int ret;
+
+	std::string root = utils::libcameraSourcePath();
+	if (!root.empty()) {
+		/*
+		 * When libcamera is used before it is installed, load
+		 * configuration files from the source directory. The
+		 * configuration files are then located in the 'data'
+		 * subdirectory of the corresponding pipeline handler.
+		 */
+		std::string confDir = root + "src/libcamera/pipeline/";
+		confPath = confDir + subdir + "/data/" + name;
+
+		LOG(Pipeline, Info)
+			<< "libcamera is not installed. Loading platform configuration file from '"
+			<< confPath << "'";
+
+		ret = stat(confPath.c_str(), &statbuf);
+		if (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)
+			return confPath;
+	} else {
+		/* Else look in the system locations. */
+		confPath = std::string(LIBCAMERA_DATA_DIR)
+				+ "/pipeline/" + subdir + '/' + name;
+		ret = stat(confPath.c_str(), &statbuf);
+		if (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)
+			return confPath;
+	}
+
+	LOG(Pipeline, Error)
+		<< "Configuration file '" << confPath
+		<< "' not found for pipeline handler '" << PipelineHandler::name() << "'";
+
+	return std::string();
+}
+
 /**
  * \brief Register a camera to the camera manager and pipeline handler
  * \param[in] camera The camera to be added