<div dir="ltr"><div dir="ltr">Hi David,<div><br></div><div>Thank you for your feedback!</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, 1 Nov 2022 at 11:43, David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Naush<br>
<br>
Thanks for this patch!<br>
<br>
On Fri, 14 Oct 2022 at 14:18, Naushir Patuck via libcamera-devel<br>
<<a href="mailto:libcamera-devel@lists.libcamera.org" target="_blank">libcamera-devel@lists.libcamera.org</a>> wrote:<br>
><br>
> Add a new helper function PipelineHandler::configurationFile() that returns<br>
> the full path of a named configuration file. This configuration file may be read<br>
> by pipeline handlers for platform specific configuration parameters on<br>
> initialisation.<br>
><br>
> The mechanism for searching for the configuration file is similar to the IPA<br>
> configuration file:<br>
><br>
> - In the source tree if libcamera is not installed<br>
> - Otherwise in standard system locations (etc and share directories).<br>
><br>
> When stored in the source tree, configuration files shall be located in a 'data'<br>
> subdirectory of their respective pipeline handler directory.<br>
><br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> ---<br>
> include/libcamera/internal/pipeline_handler.h | 2 +<br>
> src/libcamera/pipeline_handler.cpp | 57 +++++++++++++++++++<br>
> 2 files changed, 59 insertions(+)<br>
><br>
> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h<br>
> index b6139a88d421..6648975f15de 100644<br>
> --- a/include/libcamera/internal/pipeline_handler.h<br>
> +++ b/include/libcamera/internal/pipeline_handler.h<br>
> @@ -74,6 +74,8 @@ protected:<br>
> virtual int queueRequestDevice(Camera *camera, Request *request) = 0;<br>
> virtual void stopDevice(Camera *camera) = 0;<br>
><br>
> + std::string configurationFile(const std::string &name) const;<br>
> +<br>
> CameraManager *manager_;<br>
><br>
> private:<br>
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp<br>
> index 588a3db30e82..998dcece05f2 100644<br>
> --- a/src/libcamera/pipeline_handler.cpp<br>
> +++ b/src/libcamera/pipeline_handler.cpp<br>
> @@ -8,6 +8,7 @@<br>
> #include "libcamera/internal/pipeline_handler.h"<br>
><br>
> #include <chrono><br>
> +#include <sys/stat.h><br>
> #include <sys/sysmacros.h><br>
><br>
> #include <libcamera/base/log.h><br>
> @@ -625,6 +626,62 @@ void PipelineHandler::disconnect()<br>
> }<br>
> }<br>
><br>
> +/**<br>
> + * \brief Retrieve the absolute path to a platform configuration file<br>
> + * \param[in] name The configuration file name<br>
> + *<br>
> + * This function locates a named platform configuration file and returns<br>
> + * its absolute path to the pipeline handler. It searches the following<br>
> + * directories, in order:<br>
> + *<br>
> + * - If libcamera is not installed, the src/libcamera/pipeline/ directory within<br>
> + * the source tree ; otherwise<br>
> + * - The system data (share/libcamera/pipeline/) directory.<br>
> + *<br>
> + * The system directories are not searched if libcamera is not installed.<br>
> + *<br>
> + * \return The full path to the pipeline handler configuration file, or an empty<br>
> + * string if no configuration file can be found<br>
> + */<br>
> +std::string PipelineHandler::configurationFile(const std::string &name) const<br>
> +{<br>
> + struct stat statbuf;<br>
> + int ret;<br>
> +<br>
> + std::string root = utils::libcameraSourcePath();<br>
> + if (!root.empty()) {<br>
> + /*<br>
> + * When libcamera is used before it is installed, load<br>
> + * configuration files from the source directory. The<br>
> + * configuration files are then located in the 'data'<br>
> + * subdirectory of the corresponding IPA module.<br>
> + */<br>
> + std::string confDir = root + "src/libcamera/pipeline/data";<br>
> +<br>
> + LOG(Pipeline, Info)<br>
> + << "libcamera is not installed. Loading platform configuration file from '"<br>
> + << confDir << "'";<br>
> +<br>
> + std::string confPath = confDir + "/" + name;<br>
> + ret = stat(confPath.c_str(), &statbuf);<br>
> + if (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)<br>
<br>
I guess I was just wondering slightly whether this is worth checking<br>
given that we fail with a helpful message anyway if we can't open the<br>
file, but it's fine like this too, so probably just ignore me!<br></blockquote><div><br></div><div>This function is (mostly) a duplicate of what is used for the tuning file,</div><div>so I'll probably leave this check in to be consistent.</div><div><br></div><div>Regards,</div><div>Naush</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Reviewed-by: David Plowman <<a href="mailto:david.plowman@raspberrypi.com" target="_blank">david.plowman@raspberrypi.com</a>><br>
<br>
Thanks!<br>
David<br>
<br>
> + return confPath;<br>
> +<br>
> + } else {<br>
> + /* Else look in the system locations. */<br>
> + std::string confPath = std::string(LIBCAMERA_DATA_DIR) + "/pipeline/" + name;<br>
> + ret = stat(confPath.c_str(), &statbuf);<br>
> + if (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)<br>
> + return confPath;<br>
> + }<br>
> +<br>
> + LOG(Pipeline, Error)<br>
> + << "Configuration file '" << name<br>
> + << "' not found for pipeline handler '" << PipelineHandler::name() << "'";<br>
> +<br>
> + return std::string();<br>
> +}<br>
> +<br>
> /**<br>
> * \var PipelineHandler::manager_<br>
> * \brief The Camera manager associated with the pipeline handler<br>
> --<br>
> 2.25.1<br>
><br>
</blockquote></div>
</div>