[{"id":26275,"web_url":"https://patchwork.libcamera.org/comment/26275/","msgid":"<167421154508.42371.13210409917658086413@Monstersaurus>","date":"2023-01-20T10:45:45","subject":"Re: [libcamera-devel] [PATCH v5 02/12] libcamera: pipeline: Add a\n\tplatform configuration file helper","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck via libcamera-devel (2023-01-18 08:59:43)\n> Add a new helper function PipelineHandler::configurationFile() that returns\n> the full path of a named configuration file. This configuration file may be read\n> by pipeline handlers for platform specific configuration parameters on\n> initialisation.\n> \n> The mechanism for searching for the configuration file is similar to the IPA\n> configuration file:\n> \n> - In the source tree if libcamera is not installed\n> - Otherwise in standard system locations (etc and share directories).\n> \n> When stored in the source tree, configuration files shall be located in a 'data'\n> subdirectory of their respective pipeline handler directory.\n> \n> Signed-off-by: Naushir Patuck \n> Reviewed-by: David Plowman \n\nThis patch itself looks fine to me. I do wonder when we should just have\na global libcamera configuration file though.\n\nBut each pipeline config will be distinct, and having each one\nseparately and directly documented, (as well as having different preset\noptions) seems reaonable and useful to me.\n\nReviewed-by: Kieran Bingham \n\n> ---\n> include/libcamera/internal/pipeline_handler.h | 3 +\n> src/libcamera/pipeline_handler.cpp | 60 +++++++++++++++++++\n> 2 files changed, 63 insertions(+)\n> \n> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> index ec4f662d7399..4c4dfe62a680 100644\n> --- a/include/libcamera/internal/pipeline_handler.h\n> +++ b/include/libcamera/internal/pipeline_handler.h\n> @@ -65,6 +65,9 @@ public:\n> bool completeBuffer(Request *request, FrameBuffer *buffer);\n> void completeRequest(Request *request);\n> \n> + std::string configurationFile(const std::string &subdir,\n> + const std::string &name) const;\n> +\n> const char *name() const { return name_; }\n> \n> protected:\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index cfade4908118..a515ad5ecffb 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -8,6 +8,7 @@\n> #include \"libcamera/internal/pipeline_handler.h\"\n> \n> #include \n> +#include \n> #include \n> \n> #include \n> @@ -534,6 +535,65 @@ void PipelineHandler::completeRequest(Request *request)\n> }\n> }\n> \n> +/**\n> + * \\brief Retrieve the absolute path to a platform configuration file\n> + * \\param[in] subdir The pipeline handler specific subdirectory name\n> + * \\param[in] name The configuration file name\n> + *\n> + * This function locates a named platform configuration file and returns\n> + * its absolute path to the pipeline handler. It searches the following\n> + * directories, in order:\n> + *\n> + * - If libcamera is not installed, the src/libcamera/pipeline//data/\n> + * directory within the source tree ; otherwise\n> + * - The system data (share/libcamera/pipeline/) directory.\n> + *\n> + * The system directories are not searched if libcamera is not installed.\n> + *\n> + * \\return The full path to the pipeline handler configuration file, or an empty\n> + * string if no configuration file can be found\n> + */\n> +std::string PipelineHandler::configurationFile(const std::string &subdir,\n> + const std::string &name) const\n> +{\n> + std::string confPath;\n> + struct stat statbuf;\n> + int ret;\n> +\n> + std::string root = utils::libcameraSourcePath();\n> + if (!root.empty()) {\n> + /*\n> + * When libcamera is used before it is installed, load\n> + * configuration files from the source directory. The\n> + * configuration files are then located in the 'data'\n> + * subdirectory of the corresponding pipeline handler.\n> + */\n> + std::string confDir = root + \"src/libcamera/pipeline/\";\n> + confPath = confDir + subdir + \"/data/\" + name;\n> +\n> + LOG(Pipeline, Info)\n> + << \"libcamera is not installed. Loading platform configuration file from '\"\n> + << confPath << \"'\";\n> +\n> + ret = stat(confPath.c_str(), &statbuf);\n> + if (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> + return confPath;\n> + } else {\n> + /* Else look in the system locations. */\n> + confPath = std::string(LIBCAMERA_DATA_DIR)\n> + + \"/pipeline/\" + subdir + '/' + name;\n> + ret = stat(confPath.c_str(), &statbuf);\n> + if (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> + return confPath;\n> + }\n> +\n> + LOG(Pipeline, Error)\n> + << \"Configuration file '\" << confPath\n> + << \"' not found for pipeline handler '\" << PipelineHandler::name() << \"'\";\n> +\n> + return std::string();\n> +}\n> +\n> /**\n> * \\brief Register a camera to the camera manager and pipeline handler\n> * \\param[in] camera The camera to be added\n> -- \n> 2.25.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id AB1A6BE175\n\tfor ;\n\tFri, 20 Jan 2023 10:45:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0A038625E4;\n\tFri, 20 Jan 2023 11:45:50 +0100 (CET)","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 544BB6045E\n\tfor ;\n\tFri, 20 Jan 2023 11:45:48 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D172A514;\n\tFri, 20 Jan 2023 11:45:47 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674211550;\n\tbh=ZEq0ZZHaB/1ZJaReSagSrqBn3fkuh4hLjGXb2+OqVys=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=mnP1wrUosyOiiFxQLBalvCk0yZmUrLDIUMW+xav2ccpRkzQYc304kYauKt6LTGuET\n\tkiV8UN8SzUfVWiHftvfecuZ7LpGYkC2g0Z61kauRIAF8UT/iYLDeD+Qu/xGTa3xUdN\n\tC3ZWUUE4xgwxvbxyttRobhNbGfojfkyiyacbUIvgoBq8mg0nMo3YUkspY9m7P+I1Zg\n\tI3Fhyb1yA/7UClr7eCkYU2OCw1EOz6z0KEzx1EQtPpTOkDUlF4k5K2xxCg5lLrjw55\n\tU6xSehGcqftOoZtVvPLvF88JQB1G05iWR48/inuyedqg7jZB6wkt/gh8x3FbA0F/u1\n\td0AKIlq1lFuuQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1674211547;\n\tbh=ZEq0ZZHaB/1ZJaReSagSrqBn3fkuh4hLjGXb2+OqVys=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=feYOErAcg/YxCxetT4otjfQdROZe7cHLF5AJgmk0uRuN0sTlrh8fqIPx/0a2NqlpO\n\txYIFdsLxMUU8JujeXI1B0hGh2ap7iMLqAtM2CNqgs/JbhtjzUEsUytJj+T7SiOZ4J7\n\tlxEKELdlETtD+daWTNlLglJwATdDbkZgDyLq4fHQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"feYOErAc\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230118085953.7027-3-naush@raspberrypi.com>","References":"<20230118085953.7027-1-naush@raspberrypi.com>\n\t<20230118085953.7027-3-naush@raspberrypi.com>","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 20 Jan 2023 10:45:45 +0000","Message-ID":"<167421154508.42371.13210409917658086413@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v5 02/12] libcamera: pipeline: Add a\n\tplatform configuration file helper","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26319,"web_url":"https://patchwork.libcamera.org/comment/26319/","msgid":"","date":"2023-01-22T23:24:03","subject":"Re: [libcamera-devel] [PATCH v5 02/12] libcamera: pipeline: Add a\n\tplatform configuration file helper","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Fri, Jan 20, 2023 at 10:45:45AM +0000, Kieran Bingham via libcamera-devel wrote:\n> Quoting Naushir Patuck via libcamera-devel (2023-01-18 08:59:43)\n> > Add a new helper function PipelineHandler::configurationFile() that returns\n> > the full path of a named configuration file. This configuration file may be read\n> > by pipeline handlers for platform specific configuration parameters on\n> > initialisation.\n> > \n> > The mechanism for searching for the configuration file is similar to the IPA\n> > configuration file:\n> > \n> > - In the source tree if libcamera is not installed\n> > - Otherwise in standard system locations (etc and share directories).\n> > \n> > When stored in the source tree, configuration files shall be located in a 'data'\n> > subdirectory of their respective pipeline handler directory.\n> > \n> > Signed-off-by: Naushir Patuck \n> > Reviewed-by: David Plowman \n> \n> This patch itself looks fine to me. I do wonder when we should just have\n> a global libcamera configuration file though.\n\nI agree. Short term this is fine, but I expect we'll revisit the\nconfiguration file topic.\n\n> But each pipeline config will be distinct, and having each one\n> separately and directly documented, (as well as having different preset\n> options) seems reaonable and useful to me.\n> \n> Reviewed-by: Kieran Bingham \n> \n> > ---\n> > include/libcamera/internal/pipeline_handler.h | 3 +\n> > src/libcamera/pipeline_handler.cpp | 60 +++++++++++++++++++\n> > 2 files changed, 63 insertions(+)\n> > \n> > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> > index ec4f662d7399..4c4dfe62a680 100644\n> > --- a/include/libcamera/internal/pipeline_handler.h\n> > +++ b/include/libcamera/internal/pipeline_handler.h\n> > @@ -65,6 +65,9 @@ public:\n> > bool completeBuffer(Request *request, FrameBuffer *buffer);\n> > void completeRequest(Request *request);\n> > \n> > + std::string configurationFile(const std::string &subdir,\n> > + const std::string &name) const;\n> > +\n> > const char *name() const { return name_; }\n> > \n> > protected:\n> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > index cfade4908118..a515ad5ecffb 100644\n> > --- a/src/libcamera/pipeline_handler.cpp\n> > +++ b/src/libcamera/pipeline_handler.cpp\n> > @@ -8,6 +8,7 @@\n> > #include \"libcamera/internal/pipeline_handler.h\"\n> > \n> > #include \n> > +#include \n> > #include \n> > \n> > #include \n> > @@ -534,6 +535,65 @@ void PipelineHandler::completeRequest(Request *request)\n> > }\n> > }\n> > \n> > +/**\n> > + * \\brief Retrieve the absolute path to a platform configuration file\n> > + * \\param[in] subdir The pipeline handler specific subdirectory name\n> > + * \\param[in] name The configuration file name\n> > + *\n> > + * This function locates a named platform configuration file and returns\n> > + * its absolute path to the pipeline handler. It searches the following\n> > + * directories, in order:\n> > + *\n> > + * - If libcamera is not installed, the src/libcamera/pipeline//data/\n> > + * directory within the source tree ; otherwise\n> > + * - The system data (share/libcamera/pipeline/) directory.\n> > + *\n> > + * The system directories are not searched if libcamera is not installed.\n> > + *\n> > + * \\return The full path to the pipeline handler configuration file, or an empty\n> > + * string if no configuration file can be found\n> > + */\n> > +std::string PipelineHandler::configurationFile(const std::string &subdir,\n> > + const std::string &name) const\n> > +{\n> > + std::string confPath;\n> > + struct stat statbuf;\n> > + int ret;\n> > +\n> > + std::string root = utils::libcameraSourcePath();\n> > + if (!root.empty()) {\n> > + /*\n> > + * When libcamera is used before it is installed, load\n> > + * configuration files from the source directory. The\n> > + * configuration files are then located in the 'data'\n> > + * subdirectory of the corresponding pipeline handler.\n> > + */\n> > + std::string confDir = root + \"src/libcamera/pipeline/\";\n> > + confPath = confDir + subdir + \"/data/\" + name;\n> > +\n> > + LOG(Pipeline, Info)\n> > + << \"libcamera is not installed. Loading platform configuration file from '\"\n> > + << confPath << \"'\";\n> > +\n> > + ret = stat(confPath.c_str(), &statbuf);\n> > + if (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> > + return confPath;\n\nThose three lines are the same in the other branch, they can be moved\nafter the else { ... }.\n\nReviewed-by: Laurent Pinchart \n\n> > + } else {\n> > + /* Else look in the system locations. */\n> > + confPath = std::string(LIBCAMERA_DATA_DIR)\n> > + + \"/pipeline/\" + subdir + '/' + name;\n> > + ret = stat(confPath.c_str(), &statbuf);\n> > + if (ret == 0 && (statbuf.st_mode & S_IFMT) == S_IFREG)\n> > + return confPath;\n> > + }\n> > +\n> > + LOG(Pipeline, Error)\n> > + << \"Configuration file '\" << confPath\n> > + << \"' not found for pipeline handler '\" << PipelineHandler::name() << \"'\";\n> > +\n> > + return std::string();\n> > +}\n> > +\n> > /**\n> > * \\brief Register a camera to the camera manager and pipeline handler\n> > * \\param[in] camera The camera to be added","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 493EABEFBE\n\tfor ;\n\tSun, 22 Jan 2023 23:24:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AE234625E4;\n\tMon, 23 Jan 2023 00:24:08 +0100 (CET)","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 F1A5A61EFC\n\tfor ;\n\tMon, 23 Jan 2023 00:24:07 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 050F974C;\n\tMon, 23 Jan 2023 00:24:06 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674429848;\n\tbh=KA1MRtCWmVRYWTDgJvRI8fgrH16OdzjEJ/6Y/II3/II=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=gzDaUaJBxe7V2XejSKmDiI1lXQbJiFSAWEM8VdIxNpwC03tkk2ZYIRrfcN+pZNEz+\n\tRx7F4eC8831TlXVS27FBdyQSOzEi55bZ4T27sLCfz8fQ250/nHu/kDQy2S7gfwEoHj\n\td8zKHqZuonVlIxbZ6bbD+UoMG41mErnGt4E4zJChG0pjyTXBhv1nIdlO3INrt3XilH\n\t5ZXRrPqiKB/nZCCJnUxGegIAoGdc1qIrumkAH9SBU1GBiXm+pcmZ9l3mi+Won1Jz5P\n\twArhfF9N1Nv38enmxp3QoOyldh0nmTEGWj47X17dUYB5giTZLO/qUzKEWHIjG6ScTu\n\tTLo4QWkLSiSVQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1674429847;\n\tbh=KA1MRtCWmVRYWTDgJvRI8fgrH16OdzjEJ/6Y/II3/II=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cT45eFn2t69E7tC92lo0gQHJ8NfJGiY7a+jMdBKF8LzQOKbAllflIClaUtSUCH9AF\n\tzlcxjqALgOp99cyP4MYuUh3FDVueGKqZhh5cnCt6FPy6/Q2HJbiPs2oplSmTnqWnp8\n\t2sHPSqYCT/OTGkVmXzprTh3mz99ir5MP1lCcBeJQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"cT45eFn2\"; dkim-atps=neutral","Date":"Mon, 23 Jan 2023 01:24:03 +0200","To":"Kieran Bingham ","Message-ID":"","References":"<20230118085953.7027-1-naush@raspberrypi.com>\n\t<20230118085953.7027-3-naush@raspberrypi.com>\n\t<167421154508.42371.13210409917658086413@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<167421154508.42371.13210409917658086413@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v5 02/12] libcamera: pipeline: Add a\n\tplatform configuration file helper","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]