| Message ID | 20221209090050.19441-3-naush@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting Naushir Patuck via libcamera-devel (2022-12-09 09:00:40) > 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(+) > > 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) Will this stop configuration files being a symlink to an alternative file? (S_IFREG vs S_IFLINK) ? Does that even matter? We could do: if (File::exists(confPath)) return confPath; That only checks that the 'name' is not a directory... So ... it's different again, though I would expect the > + 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() << "'"; This sounds like it suddenly makes configuration files mandatory for all pipelines ? (or perhaps only if a pipeline looks for one ?) > + > + 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 >
Hi Kieran, Thank you for the feedback! On Mon, 16 Jan 2023 at 17:18, Kieran Bingham < kieran.bingham@ideasonboard.com> wrote: > Quoting Naushir Patuck via libcamera-devel (2022-12-09 09:00:40) > > 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(+) > > > > 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) > > Will this stop configuration files being a symlink to an alternative > file? (S_IFREG vs S_IFLINK) ? Does that even matter? > > We could do: > > if (File::exists(confPath)) > return confPath; > > That only checks that the 'name' is not a directory... So ... it's > different again, though I would expect the I was copying the implementation of IPAProxy::configurationFile() for this function to retain consistency, but could change if other folks agree? > > > > > + 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() << "'"; > > This sounds like it suddenly makes configuration files mandatory for all > pipelines ? > > (or perhaps only if a pipeline looks for one ?) > Correct, the config file is only mandatory if a pipeline handler explicitly looks for one. Regards, Naush > > > + > > + 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 > > >
Quoting Naushir Patuck (2023-01-17 08:50:46) > Hi Kieran, > > Thank you for the feedback! > > On Mon, 16 Jan 2023 at 17:18, Kieran Bingham < > kieran.bingham@ideasonboard.com> wrote: > > > Quoting Naushir Patuck via libcamera-devel (2022-12-09 09:00:40) > > > 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(+) > > > > > > 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) > > > > Will this stop configuration files being a symlink to an alternative > > file? (S_IFREG vs S_IFLINK) ? Does that even matter? > > > > We could do: > > > > if (File::exists(confPath)) > > return confPath; > > > > That only checks that the 'name' is not a directory... So ... it's > > different again, though I would expect the "I would expect the user of the confPath to use the File class, so it should be appropriate" ... was supposed to be written here ;) > > > I was copying the implementation of IPAProxy::configurationFile() > for this function to retain consistency, but could change if other folks > agree? If it's better we can fix the configurationFile location too. But both seem functional anyway. -- Kieran > > > > > > > > > > > + 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() << "'"; > > > > This sounds like it suddenly makes configuration files mandatory for all > > pipelines ? > > > > (or perhaps only if a pipeline looks for one ?) > > > > Correct, the config file is only mandatory if a pipeline handler explicitly > looks for one. > Would you expect pipelines to have a default configuration for all options? Anyway, while it reports an error, it's up to the pipeline handler to decide how to respond if there's no file. -- Kieran > Regards, > Naush > > > > > > > > + > > > + 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 > > > > >
On Tue, 17 Jan 2023 at 09:40, Kieran Bingham < kieran.bingham@ideasonboard.com> wrote: > Quoting Naushir Patuck (2023-01-17 08:50:46) > > Hi Kieran, > > > > Thank you for the feedback! > > > > On Mon, 16 Jan 2023 at 17:18, Kieran Bingham < > > kieran.bingham@ideasonboard.com> wrote: > > > > > Quoting Naushir Patuck via libcamera-devel (2022-12-09 09:00:40) > > > > 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(+) > > > > > > > > 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) > > > > > > Will this stop configuration files being a symlink to an alternative > > > file? (S_IFREG vs S_IFLINK) ? Does that even matter? > > > > > > We could do: > > > > > > if (File::exists(confPath)) > > > return confPath; > > > > > > That only checks that the 'name' is not a directory... So ... it's > > > different again, though I would expect the > > "I would expect the user of the confPath to use the File class, so it > should be appropriate" ... was supposed to be written here ;) > > > > > > > I was copying the implementation of IPAProxy::configurationFile() > > for this function to retain consistency, but could change if other folks > > agree? > > If it's better we can fix the configurationFile location too. But both > seem functional anyway. > -- > Kieran > > > > > > > > > > > > > > > > > > > + 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() << "'"; > > > > > > This sounds like it suddenly makes configuration files mandatory for > all > > > pipelines ? > > > > > > (or perhaps only if a pipeline looks for one ?) > > > > > > > Correct, the config file is only mandatory if a pipeline handler > explicitly > > looks for one. > > > > Would you expect pipelines to have a default configuration for all > options? > > Anyway, while it reports an error, it's up to the pipeline handler to > decide how to respond if there's no file. > In an earlier version of the patch, we did have a default.yaml file that loaded a default configuration if no other file was provided. However, Laurent suggested (and I agree) that we remove that and hard-code the default configuration in the source itself. As you said, it's up to the individual pipeline handlers to decide the behavior. Naush > > -- > Kieran > > > > Regards, > > Naush > > > > > > > > > > > > > + > > > > + 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 > > > > > > > >
Quoting Naushir Patuck (2023-01-17 09:44:12) > On Tue, 17 Jan 2023 at 09:40, Kieran Bingham < > kieran.bingham@ideasonboard.com> wrote: > > > Quoting Naushir Patuck (2023-01-17 08:50:46) > > > Hi Kieran, > > > > > > Thank you for the feedback! > > > > > > On Mon, 16 Jan 2023 at 17:18, Kieran Bingham < > > > kieran.bingham@ideasonboard.com> wrote: > > > > > > > Quoting Naushir Patuck via libcamera-devel (2022-12-09 09:00:40) > > > > > 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(+) > > > > > > > > > > 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) > > > > > > > > Will this stop configuration files being a symlink to an alternative > > > > file? (S_IFREG vs S_IFLINK) ? Does that even matter? > > > > > > > > We could do: > > > > > > > > if (File::exists(confPath)) > > > > return confPath; > > > > > > > > That only checks that the 'name' is not a directory... So ... it's > > > > different again, though I would expect the > > > > "I would expect the user of the confPath to use the File class, so it > > should be appropriate" ... was supposed to be written here ;) > > > > > > > > > > > I was copying the implementation of IPAProxy::configurationFile() > > > for this function to retain consistency, but could change if other folks > > > agree? > > > > If it's better we can fix the configurationFile location too. But both > > seem functional anyway. > > -- > > Kieran > > > > > > > > > > > > > > > > > > > > > > > > > > > + 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() << "'"; > > > > > > > > This sounds like it suddenly makes configuration files mandatory for > > all > > > > pipelines ? > > > > > > > > (or perhaps only if a pipeline looks for one ?) > > > > > > > > > > Correct, the config file is only mandatory if a pipeline handler > > explicitly > > > looks for one. > > > > > > > Would you expect pipelines to have a default configuration for all > > options? > > > > Anyway, while it reports an error, it's up to the pipeline handler to > > decide how to respond if there's no file. > > > > In an earlier version of the patch, we did have a default.yaml file that > loaded > a default configuration if no other file was provided. However, Laurent > suggested > (and I agree) that we remove that and hard-code the default configuration > in the > source itself. Absolutely, yes I think the defaults should be handled by the pipeline handler itself. -- Kieran > As you said, it's up to the individual pipeline handlers to decide the > behavior. > > Naush > > > > > > > -- > > Kieran > > > > > > > Regards, > > > Naush > > > > > > > > > > > > > > > > > > + > > > > > + 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 > > > > > > > > > > >
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