[{"id":34502,"web_url":"https://patchwork.libcamera.org/comment/34502/","msgid":"<20250616233026.GG28664@pendragon.ideasonboard.com>","date":"2025-06-16T23:30:26","subject":"Re: [PATCH v10 04/13] 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":"Hi Milan,\n\nThank you for the patch.\n\nOn Mon, Jun 16, 2025 at 10:47:22AM +0200, Milan Zamazal wrote:\n> Let's add some helpers to make accessing simple configuration values\n> simpler.\n> \n> GlobalConfiguration::option must check for the key presence before\n> retrieving the value using operator[] because if the key is not present\n> then YamlObject::empty may be returned as a regular value.\n> \n> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n> ---\n>  .../libcamera/internal/global_configuration.h |  5 ++\n>  src/libcamera/base/global_configuration.cpp   | 52 +++++++++++++++++++\n>  2 files changed, 57 insertions(+)\n> \n> diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h\n> index 8d0410ac2..5d9531158 100644\n> --- a/include/libcamera/internal/global_configuration.h\n> +++ b/include/libcamera/internal/global_configuration.h\n> @@ -8,6 +8,8 @@\n>  #pragma once\n>  \n>  #include <filesystem>\n> +#include <optional>\n> +#include <string>\n>  \n>  #include \"libcamera/internal/yaml_parser.h\"\n>  \n> @@ -22,6 +24,9 @@ public:\n>  \n>  \tunsigned int version() const;\n>  \tConfiguration configuration() const;\n> +\tstd::optional<std::string> option(const std::string &confPath) const;\n> +\tstd::optional<std::string> envOption(const char *const envVariable,\n> +\t\t\t\t\t     const std::string &confPath) const;\n>  \n>  private:\n>  \tbool loadFile(const std::filesystem::path &fileName);\n> diff --git a/src/libcamera/base/global_configuration.cpp b/src/libcamera/base/global_configuration.cpp\n> index 1ddf5bc33..1ef63750c 100644\n> --- a/src/libcamera/base/global_configuration.cpp\n> +++ b/src/libcamera/base/global_configuration.cpp\n> @@ -8,6 +8,8 @@\n>  #include \"libcamera/internal/global_configuration.h\"\n>  \n>  #include <filesystem>\n> +#include <memory>\n> +#include <string>\n>  #include <string_view>\n>  #include <sys/types.h>\n>  \n> @@ -40,6 +42,11 @@ LOG_DEFINE_CATEGORY(Configuration)\n>   * is not found then in system-wide configuration directories. If multiple\n>   * configuration files exist then only the first one found is used and no\n>   * configuration merging is performed.\n> + *\n> + * The configuration can be accessed using the provided helpers. Namely\n> + * GlobalConfiguration::option() or GlobalConfiguration::envOption() to access\n> + * individual options or GlobalConfiguration::configuration() to access the\n> + * whole configuration.\n>   */\n>  \n>  bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)\n> @@ -112,6 +119,51 @@ GlobalConfiguration::GlobalConfiguration()\n>   * the underlying type.\n>   */\n>  \n> +/**\n> + * \\brief Return value of the configuration option identified by \\a confPath\n> + * \\param[in] confPath Sequence of the YAML section names (excluding\n> + * `configuration') leading to the requested option separated by dots\n\nPeriods are allowed in key names, but I suppose we can forbid them in\nlibcamera. Is there a specific reason you picked the period as a\nseparator over, for instance, '/' ? I suppose somehow matching the\njquery syntax is a good idea.\n\n> + * \\return A value if an item corresponding to \\a confPath exists in the\n> + * configuration file, no value otherwise\n> + */\n> +std::optional<std::string> GlobalConfiguration::option(const std::string &confPath) const\n\nThis seems possibly useful for YamlObject, not just GlobalConfiguration.\nWould it make sense to implement it there ?\n\nI'll comment more on this patch after reviewing the callers.\n\n> +{\n> +\tconst YamlObject *c = &configuration();\n> +\tfor (auto part : utils::split(confPath, \".\")) {\n> +\t\tc = &(*c)[part];\n> +\t\tif (!*c)\n> +\t\t\treturn {};\n> +\t}\n> +\treturn c->get<std::string>();\n> +}\n> +\n> +/**\n> + * \\brief Return value 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> + *\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 A value retrieved from the given environment option or configuration\n> + * file or no value if not found\n> + */\n> +std::optional<std::string> GlobalConfiguration::envOption(\n> +\tconst char *const envVariable,\n> +\tconst std::string &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 configuration version\n>   *","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 947FFBDE6B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 16 Jun 2025 23:30:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8B67668DCB;\n\tTue, 17 Jun 2025 01:30:44 +0200 (CEST)","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 EBC1E614E2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 17 Jun 2025 01:30:42 +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 ESMTPSA id A8E1DA57;\n\tTue, 17 Jun 2025 01:30:30 +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=\"JQd5/Ys7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1750116630;\n\tbh=GNAeL25quqNXZ5V7mmR2yfaIpq4P1PV0T9R1/L+2UgI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=JQd5/Ys7R5AxC/D8l7scG3wuKQ4fqmRLNvdNNa4sTtBzHhizxT0KhRxrqg4mmbq3P\n\t+BT8gam6t6dgygr7VJVaNPNCAAYxkyzvHqLOlbdxw8DuolNj3ca003LJ0OWWeMH1NS\n\t4uZ8/NTQ1vagXhBJ/MWYcDCkFhfHXekMNmdLveCY=","Date":"Tue, 17 Jun 2025 02:30:26 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Milan Zamazal <mzamazal@redhat.com>","Cc":"libcamera-devel@lists.libcamera.org, Kieran Bingham\n\t<kieran.bingham@ideasonboard.com>, =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?=\n\t<pobrn@protonmail.com>","Subject":"Re: [PATCH v10 04/13] config: Add configuration retrieval helpers","Message-ID":"<20250616233026.GG28664@pendragon.ideasonboard.com>","References":"<20250616084733.18707-1-mzamazal@redhat.com>\n\t<20250616084733.18707-5-mzamazal@redhat.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250616084733.18707-5-mzamazal@redhat.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":34590,"web_url":"https://patchwork.libcamera.org/comment/34590/","msgid":"<85wm96drvj.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-06-20T21:29:04","subject":"Re: [PATCH v10 04/13] config: Add configuration retrieval helpers","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Hi Laurent,\n\nLaurent Pinchart <laurent.pinchart@ideasonboard.com> writes:\n\n> Hi Milan,\n>\n> Thank you for the patch.\n>\n> On Mon, Jun 16, 2025 at 10:47:22AM +0200, Milan Zamazal wrote:\n>> Let's add some helpers to make accessing simple configuration values\n>> simpler.\n>> \n>> GlobalConfiguration::option must check for the key presence before\n>> retrieving the value using operator[] because if the key is not present\n>> then YamlObject::empty may be returned as a regular value.\n>> \n>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>> ---\n>>  .../libcamera/internal/global_configuration.h |  5 ++\n>>  src/libcamera/base/global_configuration.cpp   | 52 +++++++++++++++++++\n>>  2 files changed, 57 insertions(+)\n>> \n>> diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h\n>> index 8d0410ac2..5d9531158 100644\n>> --- a/include/libcamera/internal/global_configuration.h\n>> +++ b/include/libcamera/internal/global_configuration.h\n>> @@ -8,6 +8,8 @@\n>>  #pragma once\n>>  \n>>  #include <filesystem>\n>> +#include <optional>\n>> +#include <string>\n>>  \n>>  #include \"libcamera/internal/yaml_parser.h\"\n>>  \n>> @@ -22,6 +24,9 @@ public:\n>>  \n>>  \tunsigned int version() const;\n>>  \tConfiguration configuration() const;\n>> +\tstd::optional<std::string> option(const std::string &confPath) const;\n>> +\tstd::optional<std::string> envOption(const char *const envVariable,\n>> +\t\t\t\t\t     const std::string &confPath) const;\n>>  \n>>  private:\n>>  \tbool loadFile(const std::filesystem::path &fileName);\n>> diff --git a/src/libcamera/base/global_configuration.cpp b/src/libcamera/base/global_configuration.cpp\n>> index 1ddf5bc33..1ef63750c 100644\n>> --- a/src/libcamera/base/global_configuration.cpp\n>> +++ b/src/libcamera/base/global_configuration.cpp\n>> @@ -8,6 +8,8 @@\n>>  #include \"libcamera/internal/global_configuration.h\"\n>>  \n>>  #include <filesystem>\n>> +#include <memory>\n>> +#include <string>\n>>  #include <string_view>\n>>  #include <sys/types.h>\n>>  \n>> @@ -40,6 +42,11 @@ LOG_DEFINE_CATEGORY(Configuration)\n>>   * is not found then in system-wide configuration directories. If multiple\n>>   * configuration files exist then only the first one found is used and no\n>>   * configuration merging is performed.\n>> + *\n>> + * The configuration can be accessed using the provided helpers. Namely\n>> + * GlobalConfiguration::option() or GlobalConfiguration::envOption() to access\n>> + * individual options or GlobalConfiguration::configuration() to access the\n>> + * whole configuration.\n>>   */\n>>  \n>>  bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)\n>> @@ -112,6 +119,51 @@ GlobalConfiguration::GlobalConfiguration()\n>>   * the underlying type.\n>>   */\n>>  \n>> +/**\n>> + * \\brief Return value of the configuration option identified by \\a confPath\n>> + * \\param[in] confPath Sequence of the YAML section names (excluding\n>> + * `configuration') leading to the requested option separated by dots\n>\n> Periods are allowed in key names, but I suppose we can forbid them in\n> libcamera. Is there a specific reason you picked the period as a\n> separator over, for instance, '/' ? \n\nNo, I can change it.\n\n> I suppose somehow matching the jquery syntax is a good idea.\n>\n>> + * \\return A value if an item corresponding to \\a confPath exists in the\n>> + * configuration file, no value otherwise\n>> + */\n>> +std::optional<std::string> GlobalConfiguration::option(const std::string &confPath) const\n>\n> This seems possibly useful for YamlObject, not just GlobalConfiguration.\n> Would it make sense to implement it there ?\n\nMaybe, I can try.\n\n> I'll comment more on this patch after reviewing the callers.\n>\n>> +{\n>> +\tconst YamlObject *c = &configuration();\n>> +\tfor (auto part : utils::split(confPath, \".\")) {\n>> +\t\tc = &(*c)[part];\n>> +\t\tif (!*c)\n>> +\t\t\treturn {};\n>> +\t}\n>> +\treturn c->get<std::string>();\n>> +}\n>> +\n>> +/**\n>> + * \\brief Return value 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>> + *\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 A value retrieved from the given environment option or configuration\n>> + * file or no value if not found\n>> + */\n>> +std::optional<std::string> GlobalConfiguration::envOption(\n>> +\tconst char *const envVariable,\n>> +\tconst std::string &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 configuration version\n>>   *","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 1A57DC3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 20 Jun 2025 21:29:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C402868DE2;\n\tFri, 20 Jun 2025 23:29:13 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EC04061538\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 20 Jun 2025 23:29:11 +0200 (CEST)","from mail-wm1-f71.google.com (mail-wm1-f71.google.com\n\t[209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-100-RCek4DKZOYW9uoT2asujiQ-1; Fri, 20 Jun 2025 17:29:07 -0400","by mail-wm1-f71.google.com with SMTP id\n\t5b1f17b1804b1-45359bfe631so12685205e9.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 20 Jun 2025 14:29:07 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-2.net.vodafone.cz. [77.48.47.2])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-4535e98b48asm70887165e9.16.2025.06.20.14.29.04\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 20 Jun 2025 14:29:05 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"TPqcw/R2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1750454951;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=IGyzS57+DVuuqbJVj7YuqIKLBe+ejfijKlDmDaTkGeM=;\n\tb=TPqcw/R2wEr9KWWTym7Gk1x/sodHhwom0TwT6YpFk0c4nkZf5pfoeemF35xT9/PgJadOIJ\n\tmnhOGDUFoAFiKLNFg3nyJkc73T06N5fo+I0sWHZDmkWFVqw1UlFbYBENFO1E0lyZbnA6Xq\n\tdh9Ij1sYRLLHaDhdqGLAZYehN0c+KUw=","X-MC-Unique":"RCek4DKZOYW9uoT2asujiQ-1","X-Mimecast-MFC-AGG-ID":"RCek4DKZOYW9uoT2asujiQ_1750454946","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1750454946; x=1751059746;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=IGyzS57+DVuuqbJVj7YuqIKLBe+ejfijKlDmDaTkGeM=;\n\tb=X2VnPl9/DAvcp8gtXiNq8qQEWChDuuVzkID/pu1qLbTdrqlpE9fIdYx6dYObn/1lTx\n\tcyPIX7ZbmuM+4RBe4WJknH0O7jhmGHPI9PhKvUoR3M6lR3aqRsbfg3l+HaxuSpexIaA7\n\tiaqY/pjmHp5nvCqYZ2OzFTOs/GsoHpExVzUVx6wOqsVH6TrWwZQTl0Q2lXotoyA9MLE2\n\t1QTUdp4cF8U2xK4D7yme852IYjjtE7DHqkWKN69TWLeEFK7ayMDimRD5ZJDbEiu9q6De\n\tdAIfkgX4UFiaAOoPa5sAoL8coz39p3eg4YH1XJwc2VECxuZkrR0NgoOpjjrLQycXropO\n\tK6PQ==","X-Gm-Message-State":"AOJu0YwNVIxGsVcqk6oSPBgOeli2du5cWyTah/TFFh5B35WM+zLRDkcZ\n\tC3+08DG61j8b4ZzC44wOb08LF72AwvW2iBD3C6rsNCWsVgukAe3GQVWJZsulE/0EdgnxqK5QVE1\n\tF05IgIA/T5CXM24tYu9I82xA1Soogv2dJ6uXIxehDLfOvfgcqglMvg4M/xGtKi9JSXUWsgccfdM\n\tPNCksKNEM=","X-Gm-Gg":"ASbGncsSs5d9KbBFySFw7B1zARcHYQLXp3wfVLtFTo4QUroLs4y0UDhiPFxpZ3zs8i3\n\tAbDJEJMRhJK9ECXfWZOrkxN1PBqXsKtAG4aSZUeUsS77Wdpt8L/trkm/SPZNf/v77eLim/NPxO3\n\tr4j/h7roA/m2Koi91msqFM0d7iDmc401t19y2+tNzHwsiaX/umvfrdvK9pTtvD5f0b5KIQDAIKA\n\tI3S9D+46CT2IgSL+/NmJWxgx89EUtKJSlRhkGN+FITPE0kykqLW2ayejNsYYX8dmFGbQRdBfsjL\n\tExpktFMQFs/sxm40XRZBfpvMyuVSx946+24jLcy/RL9EzXJvbX+/2VCudxsldv0vCPLuXhxQwtc\n\t=","X-Received":["by 2002:a05:600c:83c4:b0:442:e9eb:cb9e with SMTP id\n\t5b1f17b1804b1-453659ede90mr35934465e9.26.1750454946135; \n\tFri, 20 Jun 2025 14:29:06 -0700 (PDT)","by 2002:a05:600c:83c4:b0:442:e9eb:cb9e with SMTP id\n\t5b1f17b1804b1-453659ede90mr35934385e9.26.1750454945719; \n\tFri, 20 Jun 2025 14:29:05 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IG1GcESp20zQHIOpcGGXBXaYrfPhgqhKAXb89hW7mucRuzXYu2/JAm7oWKUAJ6Yt6CQfeZxpg==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>, =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?=  <pobrn@protonmail.com>","Subject":"Re: [PATCH v10 04/13] config: Add configuration retrieval helpers","In-Reply-To":"<20250616233026.GG28664@pendragon.ideasonboard.com> (Laurent\n\tPinchart's message of \"Tue, 17 Jun 2025 02:30:26 +0300\")","References":"<20250616084733.18707-1-mzamazal@redhat.com>\n\t<20250616084733.18707-5-mzamazal@redhat.com>\n\t<20250616233026.GG28664@pendragon.ideasonboard.com>","Date":"Fri, 20 Jun 2025 23:29:04 +0200","Message-ID":"<85wm96drvj.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"1tfkMa2egqtGVrNQ0HEjKpspuMNa4DvT717jQzgyREg_1750454946","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":34622,"web_url":"https://patchwork.libcamera.org/comment/34622/","msgid":"<85v7olcywp.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2025-06-24T08:43:50","subject":"Re: [PATCH v10 04/13] config: Add configuration retrieval helpers","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Milan Zamazal <mzamazal@redhat.com> writes:\n\n> Hi Laurent,\n>\n> Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:\n>\n>> Hi Milan,\n>>\n>> Thank you for the patch.\n>>\n>> On Mon, Jun 16, 2025 at 10:47:22AM +0200, Milan Zamazal wrote:\n>>> Let's add some helpers to make accessing simple configuration values\n>>> simpler.\n>>> \n>>> GlobalConfiguration::option must check for the key presence before\n>>> retrieving the value using operator[] because if the key is not present\n>>> then YamlObject::empty may be returned as a regular value.\n>>> \n>>> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>\n>>> ---\n>>>  .../libcamera/internal/global_configuration.h |  5 ++\n>>>  src/libcamera/base/global_configuration.cpp   | 52 +++++++++++++++++++\n>>>  2 files changed, 57 insertions(+)\n>>> \n>>> diff --git a/include/libcamera/internal/global_configuration.h b/include/libcamera/internal/global_configuration.h\n>>> index 8d0410ac2..5d9531158 100644\n>>> --- a/include/libcamera/internal/global_configuration.h\n>>> +++ b/include/libcamera/internal/global_configuration.h\n>>> @@ -8,6 +8,8 @@\n>>>  #pragma once\n>>>  \n>>>  #include <filesystem>\n>>> +#include <optional>\n>>> +#include <string>\n>>>  \n>>>  #include \"libcamera/internal/yaml_parser.h\"\n>>>  \n>>> @@ -22,6 +24,9 @@ public:\n>>>  \n>>>  \tunsigned int version() const;\n>>>  \tConfiguration configuration() const;\n>>> +\tstd::optional<std::string> option(const std::string &confPath) const;\n>>> +\tstd::optional<std::string> envOption(const char *const envVariable,\n>>> +\t\t\t\t\t     const std::string &confPath) const;\n>>>  \n>>>  private:\n>>>  \tbool loadFile(const std::filesystem::path &fileName);\n>>> diff --git a/src/libcamera/base/global_configuration.cpp b/src/libcamera/base/global_configuration.cpp\n>>> index 1ddf5bc33..1ef63750c 100644\n>>> --- a/src/libcamera/base/global_configuration.cpp\n>>> +++ b/src/libcamera/base/global_configuration.cpp\n>>> @@ -8,6 +8,8 @@\n>>>  #include \"libcamera/internal/global_configuration.h\"\n>>>  \n>>>  #include <filesystem>\n>>> +#include <memory>\n>>> +#include <string>\n>>>  #include <string_view>\n>>>  #include <sys/types.h>\n>>>  \n>>> @@ -40,6 +42,11 @@ LOG_DEFINE_CATEGORY(Configuration)\n>>>   * is not found then in system-wide configuration directories. If multiple\n>>>   * configuration files exist then only the first one found is used and no\n>>>   * configuration merging is performed.\n>>> + *\n>>> + * The configuration can be accessed using the provided helpers. Namely\n>>> + * GlobalConfiguration::option() or GlobalConfiguration::envOption() to access\n>>> + * individual options or GlobalConfiguration::configuration() to access the\n>>> + * whole configuration.\n>>>   */\n>>>  \n>>>  bool GlobalConfiguration::loadFile(const std::filesystem::path &fileName)\n>>> @@ -112,6 +119,51 @@ GlobalConfiguration::GlobalConfiguration()\n>>>   * the underlying type.\n>>>   */\n>>>  \n>>> +/**\n>>> + * \\brief Return value of the configuration option identified by \\a confPath\n>>> + * \\param[in] confPath Sequence of the YAML section names (excluding\n>>> + * `configuration') leading to the requested option separated by dots\n>>\n>> Periods are allowed in key names, but I suppose we can forbid them in\n>> libcamera. Is there a specific reason you picked the period as a\n>> separator over, for instance, '/' ? \n>\n> No, I can change it.\n>\n>> I suppose somehow matching the jquery syntax is a good idea.\n>>\n>>> + * \\return A value if an item corresponding to \\a confPath exists in the\n>>> + * configuration file, no value otherwise\n>>> + */\n>>> +std::optional<std::string> GlobalConfiguration::option(const std::string &confPath) const\n>>\n>> This seems possibly useful for YamlObject, not just GlobalConfiguration.\n>> Would it make sense to implement it there ?\n>\n> Maybe, I can try.\n\nTried but it doesn't look that useful there, so I decided not to put it\nthere.\n\n>> I'll comment more on this patch after reviewing the callers.\n>>\n>>> +{\n>>> +\tconst YamlObject *c = &configuration();\n>>> +\tfor (auto part : utils::split(confPath, \".\")) {\n>>> +\t\tc = &(*c)[part];\n>>> +\t\tif (!*c)\n>>> +\t\t\treturn {};\n>>> +\t}\n>>> +\treturn c->get<std::string>();\n>>> +}\n>>> +\n>>> +/**\n>>> + * \\brief Return value 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>>> + *\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 A value retrieved from the given environment option or configuration\n>>> + * file or no value if not found\n>>> + */\n>>> +std::optional<std::string> GlobalConfiguration::envOption(\n>>> +\tconst char *const envVariable,\n>>> +\tconst std::string &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 configuration version\n>>>   *","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 7F968C3237\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 24 Jun 2025 08:43:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 53EFD68DFC;\n\tTue, 24 Jun 2025 10:43:58 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2E24968DDD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 24 Jun 2025 10:43:56 +0200 (CEST)","from mail-wm1-f71.google.com (mail-wm1-f71.google.com\n\t[209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-88-LeRxPkxQPbKbMI-gVAp2hg-1; Tue, 24 Jun 2025 04:43:53 -0400","by mail-wm1-f71.google.com with SMTP id\n\t5b1f17b1804b1-451ac1b43c4so1239815e9.0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 24 Jun 2025 01:43:53 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb ([85.93.96.130])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-4535e98b48asm169626785e9.16.2025.06.24.01.43.51\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 24 Jun 2025 01:43:51 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"dA2w7rNs\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1750754635;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=7q5NKcmDCXiR+nZWY8Vt7MRkRfiIv4mTl5nibW4eXJ4=;\n\tb=dA2w7rNsaqp0S8JgxUButY+WKWh+VdwnryldiSDf0X9K3WZ+U7ojNUx11QvwNYIEzgU6rE\n\tTzZk3OMM8k49mMtTtDkLaI2jEuIl3hh7q+A8y+XlazPcUM9C3fl1JRW9xUaI0JmW4enGjt\n\t5wVQvImY/gaorD1XwQbhz+PEZUSLVV4=","X-MC-Unique":"LeRxPkxQPbKbMI-gVAp2hg-1","X-Mimecast-MFC-AGG-ID":"LeRxPkxQPbKbMI-gVAp2hg_1750754632","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1750754632; x=1751359432;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=7q5NKcmDCXiR+nZWY8Vt7MRkRfiIv4mTl5nibW4eXJ4=;\n\tb=fondGYwr7NEcJ3GfRgf+ZX0P4Ytw6sWPdE728cSQofVmKp6aWA5lCRCwAFcJsOHiUS\n\tORixHRqx5aQlIIRxYkNX2hx8DMfWgjGmy1qm8Cef0FJMNI/gzoFFlHNS0Q2239mQCtht\n\tV5Ok89PPQirZN+RjX1gV1tZiF7kBP1gcKHSSbvJLQgBYsbFQOKH8hYBvqRaopiNELHyR\n\thdrjkZ0NwkOBPVToZvtCw4s7a/o6Fvhw0LDLrUuap76m6fJWS82PM+pURm8LNTaUB37j\n\txb7sWdNhLDCir+jxIn2hdiqZV9Qd/f+nEFMaLmAVffqCZNY1sbtje1KBOH37GUT+yodS\n\tdz8g==","X-Gm-Message-State":"AOJu0Yxtr/lL65ORsT+S5sIPfpkgJNun1ps5r4DYjo1Ae75XCDub8QXU\n\tNBDU0WfpM3584OgzPC09eIfGMlihx/jh9XATfYWWFF8Xq0Df+NJl5SvPAqP2PiNZQTSGezA5pTK\n\tYvKd6uLW3g0OcOtvoYxlkfsk276jhEdjuHk/sC5hsLYZRJPKDQERHLR+ioIx75kVTImGKCbO1wB\n\tI=","X-Gm-Gg":"ASbGncvqYpkCyekWIp3VsQ7TDncfz2RXjNCXa122sOy98/7tUbLUP0cn5KkFR0+aYDF\n\t06BgBTo3w/Z1W6BSnD5rmhzusuI3SapGV+Mw6EU6T8hh2BW8Bn5Zytq8Eqy3GhXgr2r5K/j/WyU\n\tmVToTxOxjtx3sDjCTo3kxlPcpJKssgJuhFAhrcFGbvQaSrNh75kjt162hVBEN5sy3Mvwkab43E0\n\tf+W400CrxNjQOypdz0sF0fTVOhOPrWC6+vsfnUbaV1NeGoJd9r2cJw8Cq/Q5JguoTmhdgNmLsRE\n\tLIHosW3Azd7xiCV+i41LxFy8rrQ0CvVR5wyKbzgJz+Mr9JU=","X-Received":["by 2002:a05:600c:c4aa:b0:450:d00d:588b with SMTP id\n\t5b1f17b1804b1-453654cbfd0mr170838525e9.9.1750754632137; \n\tTue, 24 Jun 2025 01:43:52 -0700 (PDT)","by 2002:a05:600c:c4aa:b0:450:d00d:588b with SMTP id\n\t5b1f17b1804b1-453654cbfd0mr170838165e9.9.1750754631706; \n\tTue, 24 Jun 2025 01:43:51 -0700 (PDT)"],"X-Google-Smtp-Source":"AGHT+IHpBEbb4NDWELWW23Va06+9wxvuFzLHMicczvXSrynSZhfrOmpTEhpOs3loBpYKe3PJ6zo9pA==","From":"Milan Zamazal <mzamazal@redhat.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>, =?utf-8?q?Barnab?=\n\t=?utf-8?b?w6FzIFDFkWN6ZQ==?=  <pobrn@protonmail.com>","Subject":"Re: [PATCH v10 04/13] config: Add configuration retrieval helpers","In-Reply-To":"<85wm96drvj.fsf@mzamazal-thinkpadp1gen7.tpbc.csb> (Milan\n\tZamazal's message of \"Fri, 20 Jun 2025 23:29:04 +0200\")","References":"<20250616084733.18707-1-mzamazal@redhat.com>\n\t<20250616084733.18707-5-mzamazal@redhat.com>\n\t<20250616233026.GG28664@pendragon.ideasonboard.com>\n\t<85wm96drvj.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","Date":"Tue, 24 Jun 2025 10:43:50 +0200","Message-ID":"<85v7olcywp.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"P7xLzCyu_sjtWSibE9J5hLmFKtthXkH0Jy-9GvQ53Iw_1750754632","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]