[{"id":37645,"web_url":"https://patchwork.libcamera.org/comment/37645/","msgid":"","date":"2026-01-14T10:56:38","subject":"Re: [PATCH 26/36] libcamera: value_node: Rework templates to prepare\n\tfor mutable views","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta:\n> ValueNode provides adapter classes to expose the object as an iteratable\n> list or dictionary. The Iterator and Adapter classes hardcode the\n> assumption that the ValueNode is const. To prepare for mutable views,\n> move the const specifier to the top-level DictAdapter and ListAdapter\n> class templates.\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> include/libcamera/internal/value_node.h | 54 ++++++++++++++++---------\n> 1 file changed, 35 insertions(+), 19 deletions(-)\n> \n> diff --git a/include/libcamera/internal/value_node.h b/include/libcamera/internal/value_node.h\n> index eb509d855810..dd45859f3501 100644\n> --- a/include/libcamera/internal/value_node.h\n> +++ b/include/libcamera/internal/value_node.h\n> @@ -38,14 +38,14 @@ private:\n> \n> public:\n> #ifndef __DOXYGEN__\n> -\ttemplate\n> +\ttemplate\n> \tclass Iterator\n> \t{\n> \tpublic:\n> \t\tusing difference_type = std::ptrdiff_t;\n> \t\tusing iterator_category = std::forward_iterator_tag;\n> \n> -\t\tIterator(typename ValueContainer::const_iterator it)\n> +\t\tIterator(ContainerIterator it)\n> \t\t\t: it_(it)\n> \t\t{\n> \t\t}\n> @@ -74,14 +74,14 @@ public:\n> \t\t}\n> \n> \tprotected:\n> -\t\tValueContainer::const_iterator it_;\n> +\t\tContainerIterator it_;\n> \t};\n> \n> -\ttemplate\n> +\ttemplate\n> \tclass Adapter\n> \t{\n> \tpublic:\n> -\t\tAdapter(const ValueContainer &container)\n> +\t\tAdapter(Container &container)\n> \t\t\t: container_(container)\n> \t\t{\n> \t\t}\n> @@ -97,47 +97,63 @@ public:\n> \t\t}\n> \n> \tprotected:\n> -\t\tconst ValueContainer &container_;\n> +\t\tContainer &container_;\n> \t};\n> \n> -\tclass ListIterator : public Iterator\n> +\ttemplate\n> +\tclass ListIterator : public Iterator,\n> +\t\t\t\t\t ContainerIterator>\n> \t{\n> -\tpublic:\n> -\t\tusing value_type = const ValueNode &;\n> -\t\tusing pointer = const ValueNode *;\n> -\t\tusing reference = value_type;\n> +\tprivate:\n> +\t\tusing Base = Iterator,\n> +\t\t\t\t ContainerIterator>;\n> \n> -\t\tvalue_type operator*() const\n> +\tpublic:\n> +\t\tusing value_type = Value;\n> +\t\tusing pointer = value_type *;\n> +\t\tusing reference = value_type &;\n> +\n> +\t\treference operator*() const\n> \t\t{\n> -\t\t\treturn *it_->value.get();\n> +\t\t\treturn *Base::it_->value.get();\n\n`Base` can be omitted here and removed altogether if one writes `this->it_`.\nSame applies everywhere else.\n\n\n> \t\t}\n> \n> \t\tpointer operator->() const\n> \t\t{\n> -\t\t\treturn it_->value.get();\n> +\t\t\treturn Base::it_->value.get();\n> \t\t}\n> \t};\n> \n> -\tclass DictIterator : public Iterator\n> +\ttemplate\n> +\tclass DictIterator : public Iterator,\n> +\t\t\t\t\t ContainerIterator>\n> \t{\n> +\tprivate:\n> +\t\tusing Base = Iterator,\n> +\t\t\t\t ContainerIterator>;\n> +\n> \tpublic:\n> -\t\tusing value_type = std::pair;\n> +\t\tusing value_type = std::pair;\n> \t\tusing pointer = value_type *;\n> \t\tusing reference = value_type &;\n> \n> \t\tvalue_type operator*() const\n\nI'm a bit concerned here because\n\n std::iterator_traits<...>::reference r = *it;\n\nwill/does not work with dictionary iterators. But given that it is an internal type,\nas long as we don't run into issues, it is probably fine.\n\nReviewed-by: Barnabás Pőcze \n\n\n> \t\t{\n> -\t\t\treturn { it_->key, *it_->value.get() };\n> +\t\t\treturn { Base::it_->key, *Base::it_->value.get() };\n> \t\t}\n> \t};\n> \n> -\tclass DictAdapter : public Adapter\n> +\tclass DictAdapter : public Adapter +\t\t\t\t\t\t\tValueContainer::const_iterator>,\n> +\t\t\t\t\t const ValueContainer>\n> \t{\n> \tpublic:\n> \t\tusing key_type = std::string;\n> \t};\n> \n> -\tclass ListAdapter : public Adapter\n> +\tclass ListAdapter : public Adapter +\t\t\t\t\t\t\tValueContainer::const_iterator>,\n> +\t\t\t\t\t const ValueContainer>\n> \t{\n> \t};\n> #endif /* __DOXYGEN__ */\n> --\n> Regards,\n> \n> Laurent Pinchart\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 0ED69BDCBF\n\tfor ;\n\tWed, 14 Jan 2026 10:56:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DE4C861FBC;\n\tWed, 14 Jan 2026 11:56:44 +0100 (CET)","from mail-4322.protonmail.ch (mail-4322.protonmail.ch\n\t[185.70.43.22])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6E303615B2\n\tfor ;\n\tWed, 14 Jan 2026 11:56:43 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"F/xxk1Cr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1768388202; x=1768647402;\n\tbh=I7v/2lj2VTAduEMONzdEhf/J0Eig2SXPBze/6+cX0xI=;\n\th=Date:To:From:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=F/xxk1CrqioNo73x5fTQVNDsIVlqg0yLP/aI18Chn3D1wJwYeEyj/dZ0r/rhwjxTa\n\tKrl9J6cSGusz3H58Srchx1L+7V/BnVyluwqSeE0vXLAeTbkeugiyEXXXFBbm7Ez1ze\n\tLFVeaugakDKgII1zA8/ZnIP9dfJih3JntTi3q1XLfp3M9bEGoR2l327qvZXCaECYvk\n\tfzKgLqec9q7FusPrFd18W5SItCYZG5yrfr5TBtI6hRSmNK0DeNLBJCI+wxtopW8y4s\n\tjDQq2xfJUBQ/WRfOzoNj6yWBZppEoXwPEON2vwGBtkUzQc7bjvW9YOTj7vecb8Y9/t\n\tpqkrCodktZg6w==","Date":"Wed, 14 Jan 2026 10:56:38 +0000","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","From":"=?utf-8?q?P=C5=91cze_Barnab=C3=A1s?= ","Subject":"Re: [PATCH 26/36] libcamera: value_node: Rework templates to prepare\n\tfor mutable views","Message-ID":"","In-Reply-To":"<20260113000808.15395-27-laurent.pinchart@ideasonboard.com>","References":"<20260113000808.15395-1-laurent.pinchart@ideasonboard.com>\n\t<20260113000808.15395-27-laurent.pinchart@ideasonboard.com>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"5c919faecaaea11d4be38aa8980304ba194bcf43","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","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":37714,"web_url":"https://patchwork.libcamera.org/comment/37714/","msgid":"<20260118231854.GC27500@pendragon.ideasonboard.com>","date":"2026-01-18T23:18:54","subject":"Re: [PATCH 26/36] libcamera: value_node: Rework templates to prepare\n\tfor mutable views","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Jan 14, 2026 at 10:56:38AM +0000, Barnabás Pőcze wrote:\n> 2026. 01. 13. 1:07 keltezéssel, Laurent Pinchart írta:\n> > ValueNode provides adapter classes to expose the object as an iteratable\n> > list or dictionary. The Iterator and Adapter classes hardcode the\n> > assumption that the ValueNode is const. To prepare for mutable views,\n> > move the const specifier to the top-level DictAdapter and ListAdapter\n> > class templates.\n> > \n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > include/libcamera/internal/value_node.h | 54 ++++++++++++++++---------\n> > 1 file changed, 35 insertions(+), 19 deletions(-)\n> > \n> > diff --git a/include/libcamera/internal/value_node.h b/include/libcamera/internal/value_node.h\n> > index eb509d855810..dd45859f3501 100644\n> > --- a/include/libcamera/internal/value_node.h\n> > +++ b/include/libcamera/internal/value_node.h\n> > @@ -38,14 +38,14 @@ private:\n> > \n> > public:\n> > #ifndef __DOXYGEN__\n> > -\ttemplate\n> > +\ttemplate\n> > \tclass Iterator\n> > \t{\n> > \tpublic:\n> > \t\tusing difference_type = std::ptrdiff_t;\n> > \t\tusing iterator_category = std::forward_iterator_tag;\n> > \n> > -\t\tIterator(typename ValueContainer::const_iterator it)\n> > +\t\tIterator(ContainerIterator it)\n> > \t\t\t: it_(it)\n> > \t\t{\n> > \t\t}\n> > @@ -74,14 +74,14 @@ public:\n> > \t\t}\n> > \n> > \tprotected:\n> > -\t\tValueContainer::const_iterator it_;\n> > +\t\tContainerIterator it_;\n> > \t};\n> > \n> > -\ttemplate\n> > +\ttemplate\n> > \tclass Adapter\n> > \t{\n> > \tpublic:\n> > -\t\tAdapter(const ValueContainer &container)\n> > +\t\tAdapter(Container &container)\n> > \t\t\t: container_(container)\n> > \t\t{\n> > \t\t}\n> > @@ -97,47 +97,63 @@ public:\n> > \t\t}\n> > \n> > \tprotected:\n> > -\t\tconst ValueContainer &container_;\n> > +\t\tContainer &container_;\n> > \t};\n> > \n> > -\tclass ListIterator : public Iterator\n> > +\ttemplate\n> > +\tclass ListIterator : public Iterator,\n> > +\t\t\t\t\t ContainerIterator>\n> > \t{\n> > -\tpublic:\n> > -\t\tusing value_type = const ValueNode &;\n> > -\t\tusing pointer = const ValueNode *;\n> > -\t\tusing reference = value_type;\n> > +\tprivate:\n> > +\t\tusing Base = Iterator,\n> > +\t\t\t\t ContainerIterator>;\n> > \n> > -\t\tvalue_type operator*() const\n> > +\tpublic:\n> > +\t\tusing value_type = Value;\n> > +\t\tusing pointer = value_type *;\n> > +\t\tusing reference = value_type &;\n> > +\n> > +\t\treference operator*() const\n> > \t\t{\n> > -\t\t\treturn *it_->value.get();\n> > +\t\t\treturn *Base::it_->value.get();\n> \n> `Base` can be omitted here and removed altogether if one writes `this->it_`.\n> Same applies everywhere else.\n\nIs there a reason to prefer one over the other ?\n\n> > \t\t}\n> > \n> > \t\tpointer operator->() const\n> > \t\t{\n> > -\t\t\treturn it_->value.get();\n> > +\t\t\treturn Base::it_->value.get();\n> > \t\t}\n> > \t};\n> > \n> > -\tclass DictIterator : public Iterator\n> > +\ttemplate\n> > +\tclass DictIterator : public Iterator,\n> > +\t\t\t\t\t ContainerIterator>\n> > \t{\n> > +\tprivate:\n> > +\t\tusing Base = Iterator,\n> > +\t\t\t\t ContainerIterator>;\n> > +\n> > \tpublic:\n> > -\t\tusing value_type = std::pair;\n> > +\t\tusing value_type = std::pair;\n> > \t\tusing pointer = value_type *;\n> > \t\tusing reference = value_type &;\n> > \n> > \t\tvalue_type operator*() const\n> \n> I'm a bit concerned here because\n> \n> std::iterator_traits<...>::reference r = *it;\n> \n> will/does not work with dictionary iterators. But given that it is an internal type,\n> as long as we don't run into issues, it is probably fine.\n\nMore effort is needed to make ValueNode a public class, if we ever\ndecide to do so. In that case I would be fixing this too.\n\n> Reviewed-by: Barnabás Pőcze \n> \n> > \t\t{\n> > -\t\t\treturn { it_->key, *it_->value.get() };\n> > +\t\t\treturn { Base::it_->key, *Base::it_->value.get() };\n> > \t\t}\n> > \t};\n> > \n> > -\tclass DictAdapter : public Adapter\n> > +\tclass DictAdapter : public Adapter > +\t\t\t\t\t\t\tValueContainer::const_iterator>,\n> > +\t\t\t\t\t const ValueContainer>\n> > \t{\n> > \tpublic:\n> > \t\tusing key_type = std::string;\n> > \t};\n> > \n> > -\tclass ListAdapter : public Adapter\n> > +\tclass ListAdapter : public Adapter > +\t\t\t\t\t\t\tValueContainer::const_iterator>,\n> > +\t\t\t\t\t const ValueContainer>\n> > \t{\n> > \t};\n> > #endif /* __DOXYGEN__ */","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 AD0BFBDCBF\n\tfor ;\n\tSun, 18 Jan 2026 23:19:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E6EDB61FBC;\n\tMon, 19 Jan 2026 00:19:17 +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 0219A61F61\n\tfor ;\n\tMon, 19 Jan 2026 00:19:16 +0100 (CET)","from pendragon.ideasonboard.com (81-175-209-152.bb.dnainternet.fi\n\t[81.175.209.152])\n\tby perceval.ideasonboard.com (Postfix) with UTF8SMTPSA id C0F8EC1;\n\tMon, 19 Jan 2026 00:18:46 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"kvvzYnk3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1768778326;\n\tbh=GSigkaDZaCkK3uswPJLgKaJUc8QunGo9XDz0v9lzFTk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kvvzYnk3Vya5Wot3L+UcSwuDaobeu/0iOGSYMyhHQrBpn4e3+5xigoG+7Y1i2qOii\n\tty9fkHzMc7SoeoSswjNFau5ao55Tk9/BGc4QrHlGfsc7tlyu+schLGhsy5qmuZlPUh\n\t8JdWgdZgXawtCy23tOH3lI4sVUgR3y7uvNWQ/2Nw=","Date":"Mon, 19 Jan 2026 01:18:54 +0200","From":"Laurent Pinchart ","To":"=?utf-8?q?P=C5=91cze_Barnab=C3=A1s?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 26/36] libcamera: value_node: Rework templates to prepare\n\tfor mutable views","Message-ID":"<20260118231854.GC27500@pendragon.ideasonboard.com>","References":"<20260113000808.15395-1-laurent.pinchart@ideasonboard.com>\n\t<20260113000808.15395-27-laurent.pinchart@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","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\" "}}]