[{"id":35938,"web_url":"https://patchwork.libcamera.org/comment/35938/","msgid":"<20250921044342.GF30137@pendragon.ideasonboard.com>","date":"2025-09-21T04:43:42","subject":"Re: [PATCH v18 03/12] config: Add configuration retrieval helpers","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Sep 12, 2025 at 04:29:04PM +0200, Milan Zamazal wrote:\n> Let's add some helpers to make accessing simple configuration values\n> simpler. The helpers are used in the followup patches.\n> \n> GlobalConfiguration::option ensures that no value is returned rather\n> than a value of YamlObject::empty.\n> \n> Reviewed-by: Paul Elder \n> Signed-off-by: Milan Zamazal \n> ---\n> .../libcamera/internal/global_configuration.h | 28 ++++++\n> src/libcamera/global_configuration.cpp | 99 +++++++++++++++++++\n> 2 files changed, 127 insertions(+)\n> \n> diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h\n> index f695498c4..8d09517ed 100644\n> --- a/include/libcamera/internal/global_configuration.h\n> +++ b/include/libcamera/internal/global_configuration.h\n> @@ -8,6 +8,11 @@\n> #pragma once\n> \n> #include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> \n> #include \"libcamera/internal/yaml_parser.h\"\n> \n> @@ -23,6 +28,29 @@ public:\n> \tunsigned int version() const;\n> \tConfiguration configuration() const;\n> \n> +\ttemplate\n> +\tstd::optional option(\n> +\t\tconst std::initializer_list confPath) const\n> +\t{\n> +\t\tconst YamlObject *c = &configuration();\n> +\t\tfor (auto part : confPath) {\n> +\t\t\tc = &(*c)[part];\n> +\t\t\tif (!*c)\n> +\t\t\t\treturn {};\n> +\t\t}\n> +\t\treturn c->get();\n> +\t}\n> +\n> +\tstd::optional> listOption(\n> +\t\tconst std::initializer_list confPath) const;\n> +\tstd::optional envOption(\n> +\t\tconst char *const envVariable,\n> +\t\tconst std::initializer_list confPath) const;\n> +\tstd::optional> envListOption(\n> +\t\tconst char *const envVariable,\n> +\t\tconst std::initializer_list confPath,\n> +\t\tconst std::string delimiter = \":\") const;\n> +\n> private:\n> \tbool loadFile(const std::filesystem::path &fileName);\n> \tvoid load();\n> diff --git a/src/libcamera/global_configuration.cpp b/src/libcamera/global_configuration.cpp\n> index d02668111..592edcf30 100644\n> --- a/src/libcamera/global_configuration.cpp\n> +++ b/src/libcamera/global_configuration.cpp\n> @@ -8,8 +8,12 @@\n> #include \"libcamera/internal/global_configuration.h\"\n> \n> #include \n> +#include \n> +#include \n> +#include \n> #include \n> #include \n> +#include \n> \n> #include \n> #include \n> @@ -43,6 +47,12 @@ LOG_DEFINE_CATEGORY(Configuration)\n> * If the first found configuration file cannot be opened or parsed, an error is\n> * reported and no configuration file is used. This is to prevent libcamera from\n> * using an unintended configuration file.\n> + *\n> + * The configuration can be accessed using the provided helpers. Namely\n> + * GlobalConfiguration::option(), GlobalConfiguration::envOption(),\n> + * GlobalConfiguration::listOption(), and GlobalConfiguration::envListOption()\n> + * to access individual options, or GlobalConfiguration::configuration() to\n> + * access the whole configuration.\n\nYou can drop the class name, doxygen should resolve function names as\nwe're in the class scope.\n\n> */\n> \n> bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)\n> @@ -110,6 +120,95 @@ GlobalConfiguration::GlobalConfiguration()\n> * the underlying type.\n> */\n> \n> +/**\n> + * \\fn std::optional GlobalConfiguration::option(const std::initializer_list &confPath) const\n> + * \\brief Return value of the configuration option identified by \\a confPath\n\ns/Return/Retrieve the/\n\nand s/identified by // to shorten the line.\n\nSame below.\n\n> + * \\param[in] confPath Sequence of the YAML section names (excluding\n> + * `configuration') leading to the requested option\n> + * \\return The value of the configuration item corresponding to \\a confPath if\n> + * it exists in the configuration file, or no value otherwise\n> + */\n> +\n> +/**\n> + * \\brief Return values of the configuration option identified by \\a confPath\n> + * \\tparam T The type of the retrieved configuration value\n> + * \\param[in] confPath Sequence of the YAML section names (excluding\n> + * `configuration') leading to the requested list option, separated by dots\n> + * \\return A vector of strings or no value if not found\n> + */\n> +std::optional> GlobalConfiguration::listOption(\n> +\tconst std::initializer_list confPath) const\n> +{\n> +\tconst YamlObject *c = &configuration();\n> +\tfor (auto part : confPath) {\n> +\t\tc = &(*c)[part];\n> +\t\tif (!*c)\n> +\t\t\treturn {};\n> +\t}\n> +\treturn c->getList();\n> +}\n> +\n> +/**\n> + * \\brief Return value of environment variable with a fallback on the configuration file\n> + * \\param[in] envVariable Environment variable to get the value from\n> + * \\param[in] confPath The sequence of YAML section names to fall back on when\n> + * \\a envVariable is unavailable\n> + *\n> + * This helper looks first at the given environment variable and if it is\n> + * defined then it returns its value (even if it is empty). Otherwise it looks\n> + * for \\a confPath the same way as in GlobalConfiguration::option. Only string\n> + * values are supported.\n> + *\n> + * \\note Support for using environment variables to configure libcamera behavior\n> + * is provided here mostly for backward compatibility reasons. Introducing new\n> + * configuration environment variables is discouraged.\n> + *\n> + * \\return The value retrieved from the given environment if it is set,\n> + * otherwise the value from the configuration file if it exists, or no value if\n> + * it does not\n> + */\n> +std::optional GlobalConfiguration::envOption(\n> +\tconst char *envVariable,\n> +\tconst std::initializer_list confPath) const\n> +{\n> +\tconst char *envValue = utils::secure_getenv(envVariable);\n> +\tif (envValue)\n> +\t\treturn std::optional{ std::string{ envValue } };\n> +\treturn option(confPath);\n> +}\n> +\n> +/**\n> + * \\brief Return values of the configuration option from a file or environment\n> + * \\param[in] envVariable Environment variable to get the value from\n> + * \\param[in] confPath The same as in GlobalConfiguration::option\n> + * \\param[in] delimiter Items separator in the environment variable\n> + *\n> + * This helper looks first at the given environment variable and if it is\n> + * defined (even if it is empty) then it splits its value by semicolons and\n> + * returns the resulting list of strings. Otherwise it looks for \\a confPath the\n> + * same way as in GlobalConfiguration::option, value of which must be a list of\n> + * strings.\n> + *\n> + * \\note Support for using environment variables to configure libcamera behavior\n> + * is provided here mostly for backward compatibility reasons. Introducing new\n> + * configuration environment variables is discouraged.\n> + *\n> + * \\return A vector of strings retrieved from the given environment option or\n> + * configuration file or no value if not found; the vector may be empty\n> + */\n> +std::optional> GlobalConfiguration::envListOption(\n> +\tconst char *const envVariable,\n> +\tconst std::initializer_list confPath,\n> +\tconst std::string delimiter) const\n\nI think it's more logical to keep the envVariable and delimiter\narguments grouped together, as the delimiter refers to the environment\nvariable. I'll swap them, and adjust the envOption() arguments for\nconsistency.\n\n> +{\n> +\tconst char *envValue = utils::secure_getenv(envVariable);\n> +\tif (envValue) {\n> +\t\tauto items = utils::split(envValue, delimiter);\n> +\t\treturn std::vector(items.begin(), items.end());\n> +\t}\n> +\treturn listOption(confPath);\n> +}\n\nI have a few ideas for improvements in these helpers, I'll send RFC\npatches.\n\nReviewed-by: Laurent Pinchart \n\n> +\n> /**\n> * \\brief Return configuration version\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 EBA17BE173\n\tfor ;\n\tSun, 21 Sep 2025 04:44:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C2B8C6B59C;\n\tSun, 21 Sep 2025 06:44:14 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DE02762C35\n\tfor ;\n\tSun, 21 Sep 2025 06:44:12 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id 0F5741838; \n\tSun, 21 Sep 2025 06:42:50 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"sE73bkbm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1758429771;\n\tbh=2lQ43afEtdkyyJBStqc5XKgMVFfrp5SfPl2ZyYVOae8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=sE73bkbmnOq7YKhRXTqemRNwnthcWCTL+WZf+Pa1GlTH4kn8+OZwzf82rbvfY6PYW\n\tutP/dvXvN5aq1CidF7hXyxQ53znQ57Kl2bSoCPt6ZSV34pu76en8CyKFXIl0VGKGOF\n\tPrtianAP5Qsg/Ko/8yY9jI/ulKQJkM1Frpo9s+Bc=","Date":"Sun, 21 Sep 2025 07:43:42 +0300","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org, Kieran Bingham\n\t, =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t, Paul Elder\n\t","Subject":"Re: [PATCH v18 03/12] config: Add configuration retrieval helpers","Message-ID":"<20250921044342.GF30137@pendragon.ideasonboard.com>","References":"<20250912142915.53949-1-mzamazal@redhat.com>\n\t<20250912142915.53949-4-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250912142915.53949-4-mzamazal@redhat.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":35941,"web_url":"https://patchwork.libcamera.org/comment/35941/","msgid":"<20250921055027.GJ10985@pendragon.ideasonboard.com>","date":"2025-09-21T05:50:27","subject":"Re: [PATCH v18 03/12] config: Add configuration retrieval helpers","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Sun, Sep 21, 2025 at 07:43:42AM +0300, Laurent Pinchart wrote:\n> On Fri, Sep 12, 2025 at 04:29:04PM +0200, Milan Zamazal wrote:\n> > Let's add some helpers to make accessing simple configuration values\n> > simpler. The helpers are used in the followup patches.\n> > \n> > GlobalConfiguration::option ensures that no value is returned rather\n> > than a value of YamlObject::empty.\n> > \n> > Reviewed-by: Paul Elder \n> > Signed-off-by: Milan Zamazal \n> > ---\n> > .../libcamera/internal/global_configuration.h | 28 ++++++\n> > src/libcamera/global_configuration.cpp | 99 +++++++++++++++++++\n> > 2 files changed, 127 insertions(+)\n> > \n> > diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h\n> > index f695498c4..8d09517ed 100644\n> > --- a/include/libcamera/internal/global_configuration.h\n> > +++ b/include/libcamera/internal/global_configuration.h\n> > @@ -8,6 +8,11 @@\n> > #pragma once\n> > \n> > #include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > \n> > #include \"libcamera/internal/yaml_parser.h\"\n> > \n> > @@ -23,6 +28,29 @@ public:\n> > \tunsigned int version() const;\n> > \tConfiguration configuration() const;\n> > \n> > +\ttemplate\n> > +\tstd::optional option(\n> > +\t\tconst std::initializer_list confPath) const\n> > +\t{\n> > +\t\tconst YamlObject *c = &configuration();\n> > +\t\tfor (auto part : confPath) {\n> > +\t\t\tc = &(*c)[part];\n> > +\t\t\tif (!*c)\n> > +\t\t\t\treturn {};\n> > +\t\t}\n> > +\t\treturn c->get();\n> > +\t}\n> > +\n> > +\tstd::optional> listOption(\n> > +\t\tconst std::initializer_list confPath) const;\n> > +\tstd::optional envOption(\n> > +\t\tconst char *const envVariable,\n> > +\t\tconst std::initializer_list confPath) const;\n> > +\tstd::optional> envListOption(\n> > +\t\tconst char *const envVariable,\n> > +\t\tconst std::initializer_list confPath,\n> > +\t\tconst std::string delimiter = \":\") const;\n> > +\n> > private:\n> > \tbool loadFile(const std::filesystem::path &fileName);\n> > \tvoid load();\n> > diff --git a/src/libcamera/global_configuration.cpp b/src/libcamera/global_configuration.cpp\n> > index d02668111..592edcf30 100644\n> > --- a/src/libcamera/global_configuration.cpp\n> > +++ b/src/libcamera/global_configuration.cpp\n> > @@ -8,8 +8,12 @@\n> > #include \"libcamera/internal/global_configuration.h\"\n> > \n> > #include \n> > +#include \n> > +#include \n> > +#include \n> > #include \n> > #include \n> > +#include \n> > \n> > #include \n> > #include \n> > @@ -43,6 +47,12 @@ LOG_DEFINE_CATEGORY(Configuration)\n> > * If the first found configuration file cannot be opened or parsed, an error is\n> > * reported and no configuration file is used. This is to prevent libcamera from\n> > * using an unintended configuration file.\n> > + *\n> > + * The configuration can be accessed using the provided helpers. Namely\n> > + * GlobalConfiguration::option(), GlobalConfiguration::envOption(),\n> > + * GlobalConfiguration::listOption(), and GlobalConfiguration::envListOption()\n> > + * to access individual options, or GlobalConfiguration::configuration() to\n> > + * access the whole configuration.\n> \n> You can drop the class name, doxygen should resolve function names as\n> we're in the class scope.\n> \n> > */\n> > \n> > bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)\n> > @@ -110,6 +120,95 @@ GlobalConfiguration::GlobalConfiguration()\n> > * the underlying type.\n> > */\n> > \n> > +/**\n> > + * \\fn std::optional GlobalConfiguration::option(const std::initializer_list &confPath) const\n> > + * \\brief Return value of the configuration option identified by \\a confPath\n> \n> s/Return/Retrieve the/\n> \n> and s/identified by // to shorten the line.\n> \n> Same below.\n> \n> > + * \\param[in] confPath Sequence of the YAML section names (excluding\n> > + * `configuration') leading to the requested option\n> > + * \\return The value of the configuration item corresponding to \\a confPath if\n> > + * it exists in the configuration file, or no value otherwise\n> > + */\n> > +\n> > +/**\n> > + * \\brief Return values of the configuration option identified by \\a confPath\n> > + * \\tparam T The type of the retrieved configuration value\n> > + * \\param[in] confPath Sequence of the YAML section names (excluding\n> > + * `configuration') leading to the requested list option, separated by dots\n> > + * \\return A vector of strings or no value if not found\n> > + */\n> > +std::optional> GlobalConfiguration::listOption(\n> > +\tconst std::initializer_list confPath) const\n> > +{\n> > +\tconst YamlObject *c = &configuration();\n> > +\tfor (auto part : confPath) {\n> > +\t\tc = &(*c)[part];\n> > +\t\tif (!*c)\n> > +\t\t\treturn {};\n> > +\t}\n> > +\treturn c->getList();\n> > +}\n> > +\n> > +/**\n> > + * \\brief Return value of environment variable with a fallback on the configuration file\n> > + * \\param[in] envVariable Environment variable to get the value from\n> > + * \\param[in] confPath The sequence of YAML section names to fall back on when\n> > + * \\a envVariable is unavailable\n> > + *\n> > + * This helper looks first at the given environment variable and if it is\n> > + * defined then it returns its value (even if it is empty). Otherwise it looks\n> > + * for \\a confPath the same way as in GlobalConfiguration::option. Only string\n> > + * values are supported.\n> > + *\n> > + * \\note Support for using environment variables to configure libcamera behavior\n> > + * is provided here mostly for backward compatibility reasons. Introducing new\n> > + * configuration environment variables is discouraged.\n> > + *\n> > + * \\return The value retrieved from the given environment if it is set,\n> > + * otherwise the value from the configuration file if it exists, or no value if\n> > + * it does not\n> > + */\n> > +std::optional GlobalConfiguration::envOption(\n> > +\tconst char *envVariable,\n> > +\tconst std::initializer_list confPath) const\n> > +{\n> > +\tconst char *envValue = utils::secure_getenv(envVariable);\n> > +\tif (envValue)\n> > +\t\treturn std::optional{ std::string{ envValue } };\n> > +\treturn option(confPath);\n> > +}\n> > +\n> > +/**\n> > + * \\brief Return values of the configuration option from a file or environment\n> > + * \\param[in] envVariable Environment variable to get the value from\n> > + * \\param[in] confPath The same as in GlobalConfiguration::option\n> > + * \\param[in] delimiter Items separator in the environment variable\n> > + *\n> > + * This helper looks first at the given environment variable and if it is\n> > + * defined (even if it is empty) then it splits its value by semicolons and\n> > + * returns the resulting list of strings. Otherwise it looks for \\a confPath the\n> > + * same way as in GlobalConfiguration::option, value of which must be a list of\n> > + * strings.\n> > + *\n> > + * \\note Support for using environment variables to configure libcamera behavior\n> > + * is provided here mostly for backward compatibility reasons. Introducing new\n> > + * configuration environment variables is discouraged.\n> > + *\n> > + * \\return A vector of strings retrieved from the given environment option or\n> > + * configuration file or no value if not found; the vector may be empty\n> > + */\n> > +std::optional> GlobalConfiguration::envListOption(\n> > +\tconst char *const envVariable,\n> > +\tconst std::initializer_list confPath,\n> > +\tconst std::string delimiter) const\n> \n> I think it's more logical to keep the envVariable and delimiter\n> arguments grouped together, as the delimiter refers to the environment\n> variable. I'll swap them, and adjust the envOption() arguments for\n> consistency.\n\nActually that would also require a change in the documentation. I'll\nsend a patch on top instead.\n\n> > +{\n> > +\tconst char *envValue = utils::secure_getenv(envVariable);\n> > +\tif (envValue) {\n> > +\t\tauto items = utils::split(envValue, delimiter);\n> > +\t\treturn std::vector(items.begin(), items.end());\n> > +\t}\n> > +\treturn listOption(confPath);\n> > +}\n> \n> I have a few ideas for improvements in these helpers, I'll send RFC\n> patches.\n> \n> Reviewed-by: Laurent Pinchart \n> \n> > +\n> > /**\n> > * \\brief Return configuration version\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 876B6C328C\n\tfor ;\n\tSun, 21 Sep 2025 05:51:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C7CA86B59C;\n\tSun, 21 Sep 2025 07:51:01 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E1B0062C35\n\tfor ;\n\tSun, 21 Sep 2025 07:50:59 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id D5F84EA0;\n\tSun, 21 Sep 2025 07:49:37 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"tyVOpfGd\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1758433778;\n\tbh=zs9vzDRie0psjH/BvbE6QC69veJlpRH9t2qAK0dRzhE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=tyVOpfGdnCACul1ISvx6TLfk8A0w/3OdSIXrIMv6xKZHZD8WnadR1T/D/9iXlvklB\n\twDSVINcMcXkEiHtmARSjngwBMFEdqrqb+y/tusA1+ZOONoIHiXl2F98ZgEtYB2F1ry\n\trO0eF1ZpZW28YikMY2yocLQ8whIZIcnL3QnC4XsA=","Date":"Sun, 21 Sep 2025 08:50:27 +0300","From":"Laurent Pinchart ","To":"Milan Zamazal ","Cc":"libcamera-devel@lists.libcamera.org, Kieran Bingham\n\t, =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t, Paul Elder\n\t","Subject":"Re: [PATCH v18 03/12] config: Add configuration retrieval helpers","Message-ID":"<20250921055027.GJ10985@pendragon.ideasonboard.com>","References":"<20250912142915.53949-1-mzamazal@redhat.com>\n\t<20250912142915.53949-4-mzamazal@redhat.com>\n\t<20250921044342.GF30137@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250921044342.GF30137@pendragon.ideasonboard.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]