[{"id":38629,"web_url":"https://patchwork.libcamera.org/comment/38629/","msgid":"<20260423194825.GC3086691@killaraus.ideasonboard.com>","date":"2026-04-23T19:48:25","subject":"Re: [PATCH v2 12/42] libcamera: yaml_parser: Add function to set a\n\tYamlObject value","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Apr 23, 2026 at 03:59:25PM +0200, Barnabás Pőcze wrote:\n> 2026. 04. 07. 17:33 keltezéssel, Laurent Pinchart írta:\n> > Add a YamlObject::set() function to set the value of an object, with\n> > specializations for scalar types.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > Changes since v1:\n> > \n> > - Remove cv in addition to reference in YamlObject::set()\n> > ---\n> > include/libcamera/internal/yaml_parser.h | 8 ++++\n> > src/libcamera/yaml_parser.cpp | 59 ++++++++++++++++++++++++\n> > 2 files changed, 67 insertions(+)\n> > \n> > diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n> > index 7953befe11e2..0666762308ac 100644\n> > --- a/include/libcamera/internal/yaml_parser.h\n> > +++ b/include/libcamera/internal/yaml_parser.h\n> > @@ -182,6 +182,13 @@ public:\n> > \t\treturn get().value_or(std::forward(defaultValue));\n> > \t}\n> > \n> > +\ttemplate\n> > +\tvoid set(T &&value)\n> > +\t{\n> > +\t\treturn Accessor>>{}\n> > +\t\t\t.set(*this, std::forward(value));\n> > +\t}\n> > +\n> > \tDictAdapter asDict() const { return DictAdapter{ list_ }; }\n> > \tListAdapter asList() const { return ListAdapter{ list_ }; }\n> > \n> > @@ -207,6 +214,7 @@ private:\n> > \ttemplate\n> > \tstruct Accessor {\n> > \t\tstd::optional get(const YamlObject &obj) const;\n> > +\t\tvoid set(YamlObject &obj, T value);\n> > \t};\n> > \n> > \tType type_;\n> > diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp\n> > index 399e7529385f..3119ab41d89e 100644\n> > --- a/src/libcamera/yaml_parser.cpp\n> > +++ b/src/libcamera/yaml_parser.cpp\n> > @@ -137,6 +137,20 @@ std::size_t YamlObject::size() const\n> > * \\return The YamlObject value, or \\a defaultValue if parsing failed\n> > */\n> > \n> > +/**\n> > + * \\fn template YamlObject::set(T &&value)\n> > + * \\brief Set the value of a YamlObject\n> > + * \\param[in] value The value\n> > + *\n> > + * This function sets the value stored in a YamlObject to \\a value. The value is\n> > + * converted to a string in an implementation-specific way that guarantees that\n> > + * subsequent calls to get() will return the same value.\n> \n> Is this true for floating pointer types as well?\n\n... no ... :-/\n\nI would like it to be, and we could use std::to_chars() and\nstd::from_chars() to implement that. Despite being specified in C++17,\nthey're only available since gcc 11 and clang 20 for floating-point\ntypes. On the gcc side, once we drop Debian Bullseye from CI this\nsummer, we'll be fine. For clang, however, even Trixie ships a too old\nversion, so we won't be able to offer a guarantee.\n\nDo you have any recommendation on how to handle this ? Should I just\ndocument that the promise doesn't hold true for floating point types ?\n\n> > + *\n> > + * Values can only be set on YamlObject of Type::Value type or empty YamlObject.\n> > + * Attempting to set a value on an object of type Type::Dict or Type::List does\n> > + * not modify the YamlObject.\n> > + */\n> > +\n> > #ifndef __DOXYGEN__\n> > \n> > template<>\n> > @@ -154,6 +168,16 @@ YamlObject::Accessor::get(const YamlObject &obj) const\n> > \treturn std::nullopt;\n> > }\n> > \n> > +template<>\n> > +void YamlObject::Accessor::set(YamlObject &obj, bool value)\n> > +{\n> > +\tif (obj.type_ != Type::Empty && obj.type_ != Type::Value)\n> > +\t\treturn;\n> > +\n> > +\tobj.type_ = Type::Value;\n> > +\tobj.value_ = value ? \"true\" : \"false\";\n> > +}\n> > +\n> > template\n> > struct YamlObject::Accessor > \tstd::is_same_v ||\n> > @@ -178,6 +202,15 @@ struct YamlObject::Accessor > \n> > \t\treturn value;\n> > \t}\n> > +\n> > +\tvoid set(YamlObject &obj, T value)\n> > +\t{\n> > +\t\tif (obj.type_ != Type::Empty && obj.type_ != Type::Value)\n> > +\t\t\treturn;\n> > +\n> > +\t\tobj.type_ = Type::Value;\n> > +\t\tobj.value_ = std::to_string(value);\n> > +\t}\n> > };\n> > \n> > template struct YamlObject::Accessor;\n> > @@ -194,6 +227,12 @@ YamlObject::Accessor::get(const YamlObject &obj) const\n> > \treturn obj.get();\n> > }\n> > \n> > +template<>\n> > +void YamlObject::Accessor::set(YamlObject &obj, float value)\n> > +{\n> > +\tobj.set(std::forward(value));\n> \n> This forwarding looks a bit unnecessary. In any case\n\nI'll drop it.\n\n> Reviewed-by: Barnabás Pőcze \n> \n> > +}\n> > +\n> > template<>\n> > std::optional\n> > YamlObject::Accessor::get(const YamlObject &obj) const\n> > @@ -215,6 +254,16 @@ YamlObject::Accessor::get(const YamlObject &obj) const\n> > \treturn value;\n> > }\n> > \n> > +template<>\n> > +void YamlObject::Accessor::set(YamlObject &obj, double value)\n> > +{\n> > +\tif (obj.type_ != Type::Empty && obj.type_ != Type::Value)\n> > +\t\treturn;\n> > +\n> > +\tobj.type_ = Type::Value;\n> > +\tobj.value_ = std::to_string(value);\n> > +}\n> > +\n> > template<>\n> > std::optional\n> > YamlObject::Accessor::get(const YamlObject &obj) const\n> > @@ -225,6 +274,16 @@ YamlObject::Accessor::get(const YamlObject &obj) const\n> > \treturn obj.value_;\n> > }\n> > \n> > +template<>\n> > +void YamlObject::Accessor::set(YamlObject &obj, std::string value)\n> > +{\n> > +\tif (obj.type_ != Type::Empty && obj.type_ != Type::Value)\n> > +\t\treturn;\n> > +\n> > +\tobj.type_ = Type::Value;\n> > +\tobj.value_ = std::move(value);\n> > +}\n> > +\n> > template<>\n> > std::optional\n> > YamlObject::Accessor::get(const YamlObject &obj) const","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 1D95ABDCB5\n\tfor ;\n\tThu, 23 Apr 2026 19:48:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2F51762F57;\n\tThu, 23 Apr 2026 21:48:28 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 208C362010\n\tfor ;\n\tThu, 23 Apr 2026 21:48:27 +0200 (CEST)","from killaraus.ideasonboard.com\n\t(2001-14ba-703d-e500--2a1.rev.dnainternet.fi\n\t[IPv6:2001:14ba:703d:e500::2a1])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 77DEE802;\n\tThu, 23 Apr 2026 21:46:47 +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=\"ethzJAMl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1776973607;\n\tbh=34irfdlOPZ7HdcFxrrlCIREDiof1tB6za8QSew70AY8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ethzJAMlcuxRRw5ckIwgxlQ8ONSplxdcsyz2+lUw4kYh6O3W4TgO1XbUTP5q4lHYm\n\tu1WE7rM2Y4eCnZqTpkb3uiKpjgfxbeOGWudQT/8YPmJ6t+ilj9UFDtiirlmSwPF/VG\n\teDQMtQkRpAR2RsoEqHdCeT248A0chNvR4FpsSxVs=","Date":"Thu, 23 Apr 2026 22:48:25 +0300","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2 12/42] libcamera: yaml_parser: Add function to set a\n\tYamlObject value","Message-ID":"<20260423194825.GC3086691@killaraus.ideasonboard.com>","References":"<20260407153427.1825999-1-laurent.pinchart@ideasonboard.com>\n\t<20260407153427.1825999-13-laurent.pinchart@ideasonboard.com>\n\t<614efd5f-c46a-48fc-8d56-7549519ecb61@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<614efd5f-c46a-48fc-8d56-7549519ecb61@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\" "}},{"id":38644,"web_url":"https://patchwork.libcamera.org/comment/38644/","msgid":"<0dc9cf17-36e7-4dd9-88c1-ec1c29c923db@ideasonboard.com>","date":"2026-04-24T08:02:08","subject":"Re: [PATCH v2 12/42] libcamera: yaml_parser: Add function to set a\n\tYamlObject value","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 04. 23. 21:48 keltezéssel, Laurent Pinchart írta:\n> On Thu, Apr 23, 2026 at 03:59:25PM +0200, Barnabás Pőcze wrote:\n>> 2026. 04. 07. 17:33 keltezéssel, Laurent Pinchart írta:\n>>> Add a YamlObject::set() function to set the value of an object, with\n>>> specializations for scalar types.\n>>>\n>>> Signed-off-by: Laurent Pinchart \n>>> ---\n>>> Changes since v1:\n>>>\n>>> - Remove cv in addition to reference in YamlObject::set()\n>>> ---\n>>> include/libcamera/internal/yaml_parser.h | 8 ++++\n>>> src/libcamera/yaml_parser.cpp | 59 ++++++++++++++++++++++++\n>>> 2 files changed, 67 insertions(+)\n>>>\n>>> diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h\n>>> index 7953befe11e2..0666762308ac 100644\n>>> --- a/include/libcamera/internal/yaml_parser.h\n>>> +++ b/include/libcamera/internal/yaml_parser.h\n>>> @@ -182,6 +182,13 @@ public:\n>>> \t\treturn get().value_or(std::forward(defaultValue));\n>>> \t}\n>>> \n>>> +\ttemplate\n>>> +\tvoid set(T &&value)\n>>> +\t{\n>>> +\t\treturn Accessor>>{}\n>>> +\t\t\t.set(*this, std::forward(value));\n>>> +\t}\n>>> +\n>>> \tDictAdapter asDict() const { return DictAdapter{ list_ }; }\n>>> \tListAdapter asList() const { return ListAdapter{ list_ }; }\n>>> \n>>> @@ -207,6 +214,7 @@ private:\n>>> \ttemplate\n>>> \tstruct Accessor {\n>>> \t\tstd::optional get(const YamlObject &obj) const;\n>>> +\t\tvoid set(YamlObject &obj, T value);\n>>> \t};\n>>> \n>>> \tType type_;\n>>> diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp\n>>> index 399e7529385f..3119ab41d89e 100644\n>>> --- a/src/libcamera/yaml_parser.cpp\n>>> +++ b/src/libcamera/yaml_parser.cpp\n>>> @@ -137,6 +137,20 @@ std::size_t YamlObject::size() const\n>>> * \\return The YamlObject value, or \\a defaultValue if parsing failed\n>>> */\n>>> \n>>> +/**\n>>> + * \\fn template YamlObject::set(T &&value)\n>>> + * \\brief Set the value of a YamlObject\n>>> + * \\param[in] value The value\n>>> + *\n>>> + * This function sets the value stored in a YamlObject to \\a value. The value is\n>>> + * converted to a string in an implementation-specific way that guarantees that\n>>> + * subsequent calls to get() will return the same value.\n>>\n>> Is this true for floating pointer types as well?\n> \n> ... no ... :-/\n> \n> I would like it to be, and we could use std::to_chars() and\n> std::from_chars() to implement that. Despite being specified in C++17,\n> they're only available since gcc 11 and clang 20 for floating-point\n> types. On the gcc side, once we drop Debian Bullseye from CI this\n> summer, we'll be fine. For clang, however, even Trixie ships a too old\n> version, so we won't be able to offer a guarantee.\n> \n> Do you have any recommendation on how to handle this ? Should I just\n> document that the promise doesn't hold true for floating point types ?\n\nI think it is fine as is for now. I'd maybe drop this whole statement\nor mention that \"\\todo Guarantee for floating point types\".\n\n\n> \n>>> + *\n>>> + * Values can only be set on YamlObject of Type::Value type or empty YamlObject.\n>>> + * Attempting to set a value on an object of type Type::Dict or Type::List does\n>>> + * not modify the YamlObject.\n>>> + */\n>>> +\n>>> #ifndef __DOXYGEN__\n>>> \n>>> template<>\n>>> @@ -154,6 +168,16 @@ YamlObject::Accessor::get(const YamlObject &obj) const\n>>> \treturn std::nullopt;\n>>> }\n>>> \n>>> +template<>\n>>> +void YamlObject::Accessor::set(YamlObject &obj, bool value)\n>>> +{\n>>> +\tif (obj.type_ != Type::Empty && obj.type_ != Type::Value)\n>>> +\t\treturn;\n>>> +\n>>> +\tobj.type_ = Type::Value;\n>>> +\tobj.value_ = value ? \"true\" : \"false\";\n>>> +}\n>>> +\n>>> template\n>>> struct YamlObject::Accessor>> \tstd::is_same_v ||\n>>> @@ -178,6 +202,15 @@ struct YamlObject::Accessor>> \n>>> \t\treturn value;\n>>> \t}\n>>> +\n>>> +\tvoid set(YamlObject &obj, T value)\n>>> +\t{\n>>> +\t\tif (obj.type_ != Type::Empty && obj.type_ != Type::Value)\n>>> +\t\t\treturn;\n>>> +\n>>> +\t\tobj.type_ = Type::Value;\n>>> +\t\tobj.value_ = std::to_string(value);\n>>> +\t}\n>>> };\n>>> \n>>> template struct YamlObject::Accessor;\n>>> @@ -194,6 +227,12 @@ YamlObject::Accessor::get(const YamlObject &obj) const\n>>> \treturn obj.get();\n>>> }\n>>> \n>>> +template<>\n>>> +void YamlObject::Accessor::set(YamlObject &obj, float value)\n>>> +{\n>>> +\tobj.set(std::forward(value));\n>>\n>> This forwarding looks a bit unnecessary. In any case\n> \n> I'll drop it.\n> \n>> Reviewed-by: Barnabás Pőcze \n>>\n>>> +}\n>>> +\n>>> template<>\n>>> std::optional\n>>> YamlObject::Accessor::get(const YamlObject &obj) const\n>>> @@ -215,6 +254,16 @@ YamlObject::Accessor::get(const YamlObject &obj) const\n>>> \treturn value;\n>>> }\n>>> \n>>> +template<>\n>>> +void YamlObject::Accessor::set(YamlObject &obj, double value)\n>>> +{\n>>> +\tif (obj.type_ != Type::Empty && obj.type_ != Type::Value)\n>>> +\t\treturn;\n>>> +\n>>> +\tobj.type_ = Type::Value;\n>>> +\tobj.value_ = std::to_string(value);\n>>> +}\n>>> +\n>>> template<>\n>>> std::optional\n>>> YamlObject::Accessor::get(const YamlObject &obj) const\n>>> @@ -225,6 +274,16 @@ YamlObject::Accessor::get(const YamlObject &obj) const\n>>> \treturn obj.value_;\n>>> }\n>>> \n>>> +template<>\n>>> +void YamlObject::Accessor::set(YamlObject &obj, std::string value)\n>>> +{\n>>> +\tif (obj.type_ != Type::Empty && obj.type_ != Type::Value)\n>>> +\t\treturn;\n>>> +\n>>> +\tobj.type_ = Type::Value;\n>>> +\tobj.value_ = std::move(value);\n>>> +}\n>>> +\n>>> template<>\n>>> std::optional\n>>> YamlObject::Accessor::get(const YamlObject &obj) const\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 C4B45BDCB5\n\tfor ;\n\tFri, 24 Apr 2026 08:02:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C676862F75;\n\tFri, 24 Apr 2026 10:02: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 ACC2962CE6\n\tfor ;\n\tFri, 24 Apr 2026 10:02:12 +0200 (CEST)","from [192.168.33.62] (185.221.140.120.nat.pool.zt.hu\n\t[185.221.140.120])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C35F9802;\n\tFri, 24 Apr 2026 10:00:32 +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=\"lqZvqMca\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1777017632;\n\tbh=7zlXNHECViWRcebwTNkXGhx34LCYElVyIAXFXYh6wS8=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=lqZvqMcapkBWM6RjwKi/+rkZAeDz0zMgzj7SGq2N5l6yX2hBHIUlaNkO99qceWfMB\n\tg+QAKc1JE4Zptay+ecnw+9psK/rHgUUkPNyDq42NtQIXOAjYmMoeGWTbwTcDjPrVDr\n\tR1dME8i9O+YHRmq3ltlN59AXLddjid9oHO5xFkvQ=","Message-ID":"<0dc9cf17-36e7-4dd9-88c1-ec1c29c923db@ideasonboard.com>","Date":"Fri, 24 Apr 2026 10:02:08 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 12/42] libcamera: yaml_parser: Add function to set a\n\tYamlObject value","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20260407153427.1825999-1-laurent.pinchart@ideasonboard.com>\n\t<20260407153427.1825999-13-laurent.pinchart@ideasonboard.com>\n\t<614efd5f-c46a-48fc-8d56-7549519ecb61@ideasonboard.com>\n\t<20260423194825.GC3086691@killaraus.ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20260423194825.GC3086691@killaraus.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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\" "}}]