[{"id":37036,"web_url":"https://patchwork.libcamera.org/comment/37036/","msgid":"<176400449093.567526.9671839480214993818@ping.linuxembedded.co.uk>","date":"2025-11-24T17:14:50","subject":"Re: [PATCH 1/2] apps: cam: Do not override Request::controls()","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jacopo Mondi (2025-11-24 16:43:44)\n> The ControlList associated with a Request are created with\n> the libcamera::controls::controls id map.\n> \n> The cam application, when parsing controls from a script\n> assigns to the Request's control list a newly created one, which is\n> initialized without a valid idmap or info map.\n> \n> This causes IPA that run in isolated mode to fail, as the IPC\n> serialization/deserialization code requires all ControlList to have\n> a valid idmap or info map associated.\n> \n> Instead of overwriting the Request::controls() list, merge it with\n> the one created by cam when parsing the capture script.\n> \n> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/295\n> Signed-off-by: Jacopo Mondi \n> ---\n> src/apps/cam/camera_session.cpp | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp\n> index 1596a25a3abed9c2d93e6657b92e35fdfd3d1a26..0a24b7fafc5333f88053cb52a500cdafd7bb603f 100644\n> --- a/src/apps/cam/camera_session.cpp\n> +++ b/src/apps/cam/camera_session.cpp\n> @@ -447,7 +447,7 @@ int CameraSession::queueRequest(Request *request)\n> return 0;\n> \n> if (script_)\n> - request->controls() = script_->frameControls(queueCount_);\n> + request->controls().merge(script_->frameControls(queueCount_));\n\nThat looks accurate even after we find a way to fix the permissions to\nprevent the underlying list itself being replaced.\n\nIs that possible by making it const somehow? a const pointer, to\nnon-const data ?\n\nAnyway,\n\nReviewed-by: Kieran Bingham \n\n> \n> queueCount_++;\n> \n> \n> -- \n> 2.51.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 09880C32E0\n\tfor ;\n\tMon, 24 Nov 2025 17:14:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 86634609D8;\n\tMon, 24 Nov 2025 18:14:55 +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 2163F6096B\n\tfor ;\n\tMon, 24 Nov 2025 18:14:54 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A353D2C5;\n\tMon, 24 Nov 2025 18:12:45 +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=\"Lmqygx//\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764004365;\n\tbh=EP4wRZPkCgfT3lK6ZKdHjvNsnRVvgY36huy3qeOgQKI=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=Lmqygx//+IpBBLO6WGhhTTzZrncU6izlbtMk1YwSXhTdvTt31vbJNtBs4ujD2wRO6\n\tlNsu68F9+FO7rysJiWyYNi+PfES/BSFngXRBk5jtwjaW8fiEWfjnQBbQH+eop/fwSX\n\tP0CQsXT6Lw5XXA/pyNLU7ig/jwSCaxXMLONHMIrM=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20251124-cam-control-override-v1-1-dfc3a2f3feee@ideasonboard.com>","References":"<20251124-cam-control-override-v1-0-dfc3a2f3feee@ideasonboard.com>\n\t<20251124-cam-control-override-v1-1-dfc3a2f3feee@ideasonboard.com>","Subject":"Re: [PATCH 1/2] apps: cam: Do not override Request::controls()","From":"Kieran Bingham ","Cc":"Jacopo Mondi ","To":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 24 Nov 2025 17:14:50 +0000","Message-ID":"<176400449093.567526.9671839480214993818@ping.linuxembedded.co.uk>","User-Agent":"alot/0.9.1","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":37043,"web_url":"https://patchwork.libcamera.org/comment/37043/","msgid":"","date":"2025-11-25T07:54:09","subject":"Re: [PATCH 1/2] apps: cam: Do not override Request::controls()","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Kieran\n\nOn Mon, Nov 24, 2025 at 05:14:50PM +0000, Kieran Bingham wrote:\n> Quoting Jacopo Mondi (2025-11-24 16:43:44)\n> > The ControlList associated with a Request are created with\n> > the libcamera::controls::controls id map.\n> >\n> > The cam application, when parsing controls from a script\n> > assigns to the Request's control list a newly created one, which is\n> > initialized without a valid idmap or info map.\n> >\n> > This causes IPA that run in isolated mode to fail, as the IPC\n> > serialization/deserialization code requires all ControlList to have\n> > a valid idmap or info map associated.\n> >\n> > Instead of overwriting the Request::controls() list, merge it with\n> > the one created by cam when parsing the capture script.\n> >\n> > Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/295\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/apps/cam/camera_session.cpp | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> >\n> > diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp\n> > index 1596a25a3abed9c2d93e6657b92e35fdfd3d1a26..0a24b7fafc5333f88053cb52a500cdafd7bb603f 100644\n> > --- a/src/apps/cam/camera_session.cpp\n> > +++ b/src/apps/cam/camera_session.cpp\n> > @@ -447,7 +447,7 @@ int CameraSession::queueRequest(Request *request)\n> > return 0;\n> >\n> > if (script_)\n> > - request->controls() = script_->frameControls(queueCount_);\n> > + request->controls().merge(script_->frameControls(queueCount_));\n>\n> That looks accurate even after we find a way to fix the permissions to\n> prevent the underlying list itself being replaced.\n>\n> Is that possible by making it const somehow? a const pointer, to\n> non-const data ?\n\nI'm looking into that\n\nSimply making \"ControlList &Request::controls()\" a \"const ControlList\n&Request::controls() const\" doesn't work\n\n1) The Camera class modifies the control list of a request, and we can\nuse the usual PIMPL pattern to expose to the library and interface\nwhich is different than the one exposed to applications\n2) However a ControlList::merge()/set() can't be called on a const\ninstance of ControlList, so application won't be able to add controls\nto a Request if we simply return \"const ControlList\" and we should add\nmethods to the Request class to set/merge controls like\n\"Request::addControls()\" I'm not sure I like it though\n\n>\n> Anyway,\n>\n> Reviewed-by: Kieran Bingham \n>\n> >\n> > queueCount_++;\n> >\n> >\n> > --\n> > 2.51.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 33B91C32EF\n\tfor ;\n\tTue, 25 Nov 2025 07:54:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 49AF060A9D;\n\tTue, 25 Nov 2025 08:54:16 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CDA55608CF\n\tfor ;\n\tTue, 25 Nov 2025 08:54:14 +0100 (CET)","from ideasonboard.com (mob-5-90-51-179.net.vodafone.it\n\t[5.90.51.179])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 32769524;\n\tTue, 25 Nov 2025 08:52:04 +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=\"lVjPykzf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764057125;\n\tbh=13qFKQpICQWEOyA2Tu1oXOKx6QqumKy22UblVXcpyw8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=lVjPykzftt/0YILJDTB2jUbIKzKpbK2Cd1reo4FRn03mVctmSB6k7YHd/22XrCxnj\n\t2d7Of368YWsDoBy06vexbw8bpx61CORIr/72My7hn6hne25FFQpHCzzGqsz3sotmDo\n\thWCfdNQ+wwE/D3j2iFOFmrGaD9mkxZhpo7BKrbLM=","Date":"Tue, 25 Nov 2025 08:54:09 +0100","From":"Jacopo Mondi ","To":"Kieran Bingham ","Cc":"Jacopo Mondi , \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/2] apps: cam: Do not override Request::controls()","Message-ID":"","References":"<20251124-cam-control-override-v1-0-dfc3a2f3feee@ideasonboard.com>\n\t<20251124-cam-control-override-v1-1-dfc3a2f3feee@ideasonboard.com>\n\t<176400449093.567526.9671839480214993818@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<176400449093.567526.9671839480214993818@ping.linuxembedded.co.uk>","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":37113,"web_url":"https://patchwork.libcamera.org/comment/37113/","msgid":"<3aebd778-33cc-441d-9a34-dc92a5249420@ideasonboard.com>","date":"2025-12-01T08:52:58","subject":"Re: [PATCH 1/2] apps: cam: Do not override Request::controls()","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 11. 24. 17:43 keltezéssel, Jacopo Mondi írta:\n> The ControlList associated with a Request are created with\n> the libcamera::controls::controls id map.\n> \n> The cam application, when parsing controls from a script\n> assigns to the Request's control list a newly created one, which is\n> initialized without a valid idmap or info map.\n> \n> This causes IPA that run in isolated mode to fail, as the IPC\n> serialization/deserialization code requires all ControlList to have\n> a valid idmap or info map associated.\n> \n> Instead of overwriting the Request::controls() list, merge it with\n> the one created by cam when parsing the capture script.\n> \n> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/295\n\nFixes: 568865a6c14355 (\"cam: Use script parser to set controls\")\n\n\n> Signed-off-by: Jacopo Mondi \n> ---\n> src/apps/cam/camera_session.cpp | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp\n> index 1596a25a3abed9c2d93e6657b92e35fdfd3d1a26..0a24b7fafc5333f88053cb52a500cdafd7bb603f 100644\n> --- a/src/apps/cam/camera_session.cpp\n> +++ b/src/apps/cam/camera_session.cpp\n> @@ -447,7 +447,7 @@ int CameraSession::queueRequest(Request *request)\n> \t\treturn 0;\n> \n> \tif (script_)\n> -\t\trequest->controls() = script_->frameControls(queueCount_);\n> +\t\trequest->controls().merge(script_->frameControls(queueCount_));\n\nI feel like this should specify `MergePolicy::OverwriteExisting`. Not that it makes\nany difference at the moment.\n\nReviewed-by: Barnabás Pőcze \n\n\n> \n> \tqueueCount_++;\n> \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 3021BC0F1B\n\tfor ;\n\tMon, 1 Dec 2025 08:53:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1B8A160AAA;\n\tMon, 1 Dec 2025 09:53:03 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 647BF609D8\n\tfor ;\n\tMon, 1 Dec 2025 09:53:01 +0100 (CET)","from [192.168.33.24] (185.182.214.104.nat.pool.zt.hu\n\t[185.182.214.104])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 196DC6AC;\n\tMon, 1 Dec 2025 09:50:48 +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=\"LDjDum5+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764579048;\n\tbh=1f2Yq8JmfiwnwXnrfZ632Jjf938Sstkwj0ATmRnBhzc=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=LDjDum5+gDLDCv/CcNXJx+Indt4Y2i95/4JNvSfaLqtfLmKizOIu5GdmcPqcrGst3\n\t0WRNM/mXM/ep4IggsT6QgYXqpbwDEL4aTBrI4euh2ScMOHbR+RJvHUTfkS1fCNLgDJ\n\t4nHHp5YDx9VNABcUQX83hmWsJMHX9PSTDiqP+aRI=","Message-ID":"<3aebd778-33cc-441d-9a34-dc92a5249420@ideasonboard.com>","Date":"Mon, 1 Dec 2025 09:52:58 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 1/2] apps: cam: Do not override Request::controls()","To":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20251124-cam-control-override-v1-0-dfc3a2f3feee@ideasonboard.com>\n\t<20251124-cam-control-override-v1-1-dfc3a2f3feee@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20251124-cam-control-override-v1-1-dfc3a2f3feee@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\" "}},{"id":37115,"web_url":"https://patchwork.libcamera.org/comment/37115/","msgid":"<215dd0be-4ad4-4756-9321-97227e7bbfba@ideasonboard.com>","date":"2025-12-01T09:19:20","subject":"Re: [PATCH 1/2] apps: cam: Do not override Request::controls()","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 11. 25. 8:54 keltezéssel, Jacopo Mondi írta:\n> Hi Kieran\n> \n> On Mon, Nov 24, 2025 at 05:14:50PM +0000, Kieran Bingham wrote:\n>> Quoting Jacopo Mondi (2025-11-24 16:43:44)\n>>> The ControlList associated with a Request are created with\n>>> the libcamera::controls::controls id map.\n>>>\n>>> The cam application, when parsing controls from a script\n>>> assigns to the Request's control list a newly created one, which is\n>>> initialized without a valid idmap or info map.\n>>>\n>>> This causes IPA that run in isolated mode to fail, as the IPC\n>>> serialization/deserialization code requires all ControlList to have\n>>> a valid idmap or info map associated.\n>>>\n>>> Instead of overwriting the Request::controls() list, merge it with\n>>> the one created by cam when parsing the capture script.\n>>>\n>>> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/295\n>>> Signed-off-by: Jacopo Mondi \n>>> ---\n>>> src/apps/cam/camera_session.cpp | 2 +-\n>>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>>\n>>> diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp\n>>> index 1596a25a3abed9c2d93e6657b92e35fdfd3d1a26..0a24b7fafc5333f88053cb52a500cdafd7bb603f 100644\n>>> --- a/src/apps/cam/camera_session.cpp\n>>> +++ b/src/apps/cam/camera_session.cpp\n>>> @@ -447,7 +447,7 @@ int CameraSession::queueRequest(Request *request)\n>>> return 0;\n>>>\n>>> if (script_)\n>>> - request->controls() = script_->frameControls(queueCount_);\n>>> + request->controls().merge(script_->frameControls(queueCount_));\n>>\n>> That looks accurate even after we find a way to fix the permissions to\n>> prevent the underlying list itself being replaced.\n>>\n>> Is that possible by making it const somehow? a const pointer, to\n>> non-const data ?\n> \n> I'm looking into that\n> \n> Simply making \"ControlList &Request::controls()\" a \"const ControlList\n> &Request::controls() const\" doesn't work\n> \n> 1) The Camera class modifies the control list of a request, and we can\n> use the usual PIMPL pattern to expose to the library and interface\n> which is different than the one exposed to applications\n> 2) However a ControlList::merge()/set() can't be called on a const\n> instance of ControlList, so application won't be able to add controls\n> to a Request if we simply return \"const ControlList\" and we should add\n> methods to the Request class to set/merge controls like\n> \"Request::addControls()\" I'm not sure I like it though\n\nI'm not entirely sure that pimpl is sufficient here. I think pimpl is a bit\nof an orthogonal question here. Because what is important here is where the\nassignment operators are deleted. I feel like if we were to delete them on\nthe \"main\" `ControlList` type, that could cause non-negligible inconvenience.\n\nOne thing that could be done is to add a wrapper around the reference, this\nway no assignment can be done, but every other member function can be preserved.\nIt unfortunately has quite a bit of duplication, but it only needs two changes\nin the current code base (in addition to the `.merge()` change).\n\nOr alternatively, maybe we could change the design of idmap/infomap/serialization.\n\n\ndiff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\nindex b24a297400..595752d4b6 100644\n--- a/include/libcamera/camera.h\n+++ b/include/libcamera/camera.h\n@@ -166,7 +166,7 @@ private:\n \tint exportFrameBuffers(Stream *stream,\n \t\t\t std::vector> *buffers);\n \n-\tvoid patchControlList(ControlList &controls);\n+\tvoid patchControlList(Request::ControlListWrapper controls);\n };\n \n } /* namespace libcamera */\ndiff --git a/include/libcamera/request.h b/include/libcamera/request.h\nindex 0c5939f7b3..1e85dcff1a 100644\n--- a/include/libcamera/request.h\n+++ b/include/libcamera/request.h\n@@ -49,7 +49,65 @@ public:\n \n \tvoid reuse(ReuseFlag flags = Default);\n \n-\tControlList &controls() { return *controls_; }\n+#ifndef __DOXYGEN__\n+\tclass ControlListWrapper {\n+\tpublic:\n+\t\tusing iterator = ControlList::iterator;\n+\t\tusing const_iterator = ControlList::const_iterator;\n+\n+\t\tControlListWrapper(ControlList& list)\n+\t\t\t: list_(list)\n+\t\t{\n+\t\t}\n+\n+\t\titerator begin() { return list_.begin(); }\n+\t\titerator end() { return list_.end(); }\n+\t\tconst_iterator begin() const { return list_.begin(); }\n+\t\tconst_iterator end() const { return list_.end(); }\n+\n+\t\tbool empty() const { return list_.empty(); }\n+\t\tstd::size_t size() const { return list_.size(); }\n+\n+\t\tvoid clear() { list_.clear(); }\n+\t\tvoid merge(const ControlList &source, ControlList::MergePolicy policy = ControlList::MergePolicy::KeepExisting)\n+\t\t{\n+\t\t\tlist_.merge(source, policy);\n+\t\t}\n+\n+\t\tbool contains(unsigned int id) const { return list_.contains(id); }\n+\n+\t\ttemplate\n+\t\tstd::optional get(const Control &ctrl) const\n+\t\t{\n+\t\t\treturn list_.get(ctrl);\n+\t\t}\n+\n+\t\ttemplate\n+\t\tvoid set(const Control &ctrl, const V &value)\n+\t\t{\n+\t\t\tlist_.set(ctrl, value);\n+\t\t}\n+\n+\t\ttemplate\n+\t\tvoid set(const Control> &ctrl, const std::initializer_list &value)\n+\t\t{\n+\t\t\tlist_.set(ctrl, value);\n+\t\t}\n+\n+\t\tconst ControlValue &get(unsigned int id) const { return list_.get(id); }\n+\t\tvoid set(unsigned int id, const ControlValue &value) { return list_.set(id, value); }\n+\n+\t\tconst ControlInfoMap *infoMap() const { return list_.infoMap(); }\n+\t\tconst ControlIdMap *idMap() const { return list_.idMap(); }\n+\n+\t\toperator const ControlList&() const { return list_; }\n+\n+\tprivate:\n+\t\tControlList &list_;\n+\t};\n+#endif\n+\n+\tControlListWrapper controls() { return *controls_; }\n \tControlList &metadata() { return *metadata_; }\n \tconst BufferMap &buffers() const { return bufferMap_; }\n \tint addBuffer(const Stream *stream, FrameBuffer *buffer,\ndiff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\nindex 80ff248c2a..266064f209 100644\n--- a/src/android/camera_device.cpp\n+++ b/src/android/camera_device.cpp\n@@ -804,7 +804,7 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n \t\treturn 0;\n \n \t/* Translate the Android request settings to libcamera controls. */\n-\tControlList &controls = descriptor->request_->controls();\n+\tauto controls = descriptor->request_->controls();\n \tcamera_metadata_ro_entry_t entry;\n \tif (settings.getEntry(ANDROID_SCALER_CROP_REGION, &entry)) {\n \t\tconst int32_t *data = entry.data.i32;\ndiff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\nindex 2e1e146a25..23d8323963 100644\n--- a/src/libcamera/camera.cpp\n+++ b/src/libcamera/camera.cpp\n@@ -1283,7 +1283,7 @@ std::unique_ptr Camera::createRequest(uint64_t cookie)\n * The control list is patched in place, turning the AeEnable control into\n * the equivalent ExposureTimeMode/AnalogueGainMode controls.\n */\n-void Camera::patchControlList(ControlList &controls)\n+void Camera::patchControlList(Request::ControlListWrapper controls)\n {\n \tconst auto &aeEnable = controls.get(controls::AeEnable);\n \tif (aeEnable) {\n\n\n\n> \n>>\n>> Anyway,\n>>\n>> Reviewed-by: Kieran Bingham \n>>\n>>>\n>>> queueCount_++;\n>>>\n>>>\n>>> --\n>>> 2.51.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 EE052BD80A\n\tfor ;\n\tMon, 1 Dec 2025 09:19:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3C72460AAC;\n\tMon, 1 Dec 2025 10:19:28 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D76DC609D8\n\tfor ;\n\tMon, 1 Dec 2025 10:19:25 +0100 (CET)","from [192.168.33.24] (185.182.214.104.nat.pool.zt.hu\n\t[185.182.214.104])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7975EF13;\n\tMon, 1 Dec 2025 10:17:12 +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=\"cBWM/i3z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764580632;\n\tbh=JkiBIEO6CrM/nLUZwbJFAyWd2OFUCnZj/trrIURxMs8=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=cBWM/i3zC49MlXnI38BsbNgLxMcBgu+qyOYiNc6lhIZXeUDP10jZAksDFVFrHZa6P\n\tf/ynGSd0bR+mijs5tAjWIpY1gldVsT3aj6Q/Q1gJCynT4TTITbBSY7UdqWTDC+u8Vz\n\tm4yoQVfIK6AyZ5fYzYPmOrzghkP5eZDdCnH7/rok=","Message-ID":"<215dd0be-4ad4-4756-9321-97227e7bbfba@ideasonboard.com>","Date":"Mon, 1 Dec 2025 10:19:20 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 1/2] apps: cam: Do not override Request::controls()","To":"Jacopo Mondi ,\n\tKieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20251124-cam-control-override-v1-0-dfc3a2f3feee@ideasonboard.com>\n\t<20251124-cam-control-override-v1-1-dfc3a2f3feee@ideasonboard.com>\n\t<176400449093.567526.9671839480214993818@ping.linuxembedded.co.uk>\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","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\" "}},{"id":37122,"web_url":"https://patchwork.libcamera.org/comment/37122/","msgid":"","date":"2025-12-01T09:59:22","subject":"Re: [PATCH 1/2] apps: cam: Do not override Request::controls()","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Mon, Dec 01, 2025 at 10:19:20AM +0100, Barnabás Pőcze wrote:\n> Hi\n>\n> 2025. 11. 25. 8:54 keltezéssel, Jacopo Mondi írta:\n> > Hi Kieran\n> >\n> > On Mon, Nov 24, 2025 at 05:14:50PM +0000, Kieran Bingham wrote:\n> > > Quoting Jacopo Mondi (2025-11-24 16:43:44)\n> > > > The ControlList associated with a Request are created with\n> > > > the libcamera::controls::controls id map.\n> > > >\n> > > > The cam application, when parsing controls from a script\n> > > > assigns to the Request's control list a newly created one, which is\n> > > > initialized without a valid idmap or info map.\n> > > >\n> > > > This causes IPA that run in isolated mode to fail, as the IPC\n> > > > serialization/deserialization code requires all ControlList to have\n> > > > a valid idmap or info map associated.\n> > > >\n> > > > Instead of overwriting the Request::controls() list, merge it with\n> > > > the one created by cam when parsing the capture script.\n> > > >\n> > > > Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/295\n> > > > Signed-off-by: Jacopo Mondi \n> > > > ---\n> > > > src/apps/cam/camera_session.cpp | 2 +-\n> > > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > > >\n> > > > diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp\n> > > > index 1596a25a3abed9c2d93e6657b92e35fdfd3d1a26..0a24b7fafc5333f88053cb52a500cdafd7bb603f 100644\n> > > > --- a/src/apps/cam/camera_session.cpp\n> > > > +++ b/src/apps/cam/camera_session.cpp\n> > > > @@ -447,7 +447,7 @@ int CameraSession::queueRequest(Request *request)\n> > > > return 0;\n> > > >\n> > > > if (script_)\n> > > > - request->controls() = script_->frameControls(queueCount_);\n> > > > + request->controls().merge(script_->frameControls(queueCount_));\n> > >\n> > > That looks accurate even after we find a way to fix the permissions to\n> > > prevent the underlying list itself being replaced.\n> > >\n> > > Is that possible by making it const somehow? a const pointer, to\n> > > non-const data ?\n> >\n> > I'm looking into that\n> >\n> > Simply making \"ControlList &Request::controls()\" a \"const ControlList\n> > &Request::controls() const\" doesn't work\n> >\n> > 1) The Camera class modifies the control list of a request, and we can\n> > use the usual PIMPL pattern to expose to the library and interface\n> > which is different than the one exposed to applications\n> > 2) However a ControlList::merge()/set() can't be called on a const\n> > instance of ControlList, so application won't be able to add controls\n> > to a Request if we simply return \"const ControlList\" and we should add\n> > methods to the Request class to set/merge controls like\n> > \"Request::addControls()\" I'm not sure I like it though\n>\n> I'm not entirely sure that pimpl is sufficient here. I think pimpl is a bit\n> of an orthogonal question here. Because what is important here is where the\n\nCould you explain a bit more what you mean here ?\n\nThe problem at end, in my view, is that we allow to override the\nControlLis which is part of a Request, and the Request class API\nshould be changed to disallow that.\n\n> assignment operators are deleted. I feel like if we were to delete them on\n> the \"main\" `ControlList` type, that could cause non-negligible inconvenience.\n\nI don't this we should disallow assigning ControlList completely, no.\n\n>\n> One thing that could be done is to add a wrapper around the reference, this\n> way no assignment can be done, but every other member function can be preserved.\n> It unfortunately has quite a bit of duplication, but it only needs two changes\n> in the current code base (in addition to the `.merge()` change).\n>\n> Or alternatively, maybe we could change the design of idmap/infomap/serialization.\n>\n>\n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index b24a297400..595752d4b6 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -166,7 +166,7 @@ private:\n> \tint exportFrameBuffers(Stream *stream,\n> \t\t\t std::vector> *buffers);\n> -\tvoid patchControlList(ControlList &controls);\n> +\tvoid patchControlList(Request::ControlListWrapper controls);\n> };\n> } /* namespace libcamera */\n> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> index 0c5939f7b3..1e85dcff1a 100644\n> --- a/include/libcamera/request.h\n> +++ b/include/libcamera/request.h\n> @@ -49,7 +49,65 @@ public:\n> \tvoid reuse(ReuseFlag flags = Default);\n> -\tControlList &controls() { return *controls_; }\n> +#ifndef __DOXYGEN__\n> +\tclass ControlListWrapper {\n> +\tpublic:\n> +\t\tusing iterator = ControlList::iterator;\n> +\t\tusing const_iterator = ControlList::const_iterator;\n> +\n> +\t\tControlListWrapper(ControlList& list)\n> +\t\t\t: list_(list)\n> +\t\t{\n> +\t\t}\n> +\n> +\t\titerator begin() { return list_.begin(); }\n> +\t\titerator end() { return list_.end(); }\n> +\t\tconst_iterator begin() const { return list_.begin(); }\n> +\t\tconst_iterator end() const { return list_.end(); }\n> +\n> +\t\tbool empty() const { return list_.empty(); }\n> +\t\tstd::size_t size() const { return list_.size(); }\n> +\n> +\t\tvoid clear() { list_.clear(); }\n> +\t\tvoid merge(const ControlList &source, ControlList::MergePolicy policy = ControlList::MergePolicy::KeepExisting)\n> +\t\t{\n> +\t\t\tlist_.merge(source, policy);\n> +\t\t}\n> +\n> +\t\tbool contains(unsigned int id) const { return list_.contains(id); }\n> +\n> +\t\ttemplate\n> +\t\tstd::optional get(const Control &ctrl) const\n> +\t\t{\n> +\t\t\treturn list_.get(ctrl);\n> +\t\t}\n> +\n> +\t\ttemplate\n> +\t\tvoid set(const Control &ctrl, const V &value)\n> +\t\t{\n> +\t\t\tlist_.set(ctrl, value);\n> +\t\t}\n> +\n> +\t\ttemplate\n> +\t\tvoid set(const Control> &ctrl, const std::initializer_list &value)\n> +\t\t{\n> +\t\t\tlist_.set(ctrl, value);\n> +\t\t}\n> +\n> +\t\tconst ControlValue &get(unsigned int id) const { return list_.get(id); }\n> +\t\tvoid set(unsigned int id, const ControlValue &value) { return list_.set(id, value); }\n> +\n> +\t\tconst ControlInfoMap *infoMap() const { return list_.infoMap(); }\n> +\t\tconst ControlIdMap *idMap() const { return list_.idMap(); }\n> +\n> +\t\toperator const ControlList&() const { return list_; }\n> +\n> +\tprivate:\n> +\t\tControlList &list_;\n> +\t};\n> +#endif\n> +\n> +\tControlListWrapper controls() { return *controls_; }\n> \tControlList &metadata() { return *metadata_; }\n> \tconst BufferMap &buffers() const { return bufferMap_; }\n> \tint addBuffer(const Stream *stream, FrameBuffer *buffer,\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 80ff248c2a..266064f209 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -804,7 +804,7 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n> \t\treturn 0;\n> \t/* Translate the Android request settings to libcamera controls. */\n> -\tControlList &controls = descriptor->request_->controls();\n> +\tauto controls = descriptor->request_->controls();\n> \tcamera_metadata_ro_entry_t entry;\n> \tif (settings.getEntry(ANDROID_SCALER_CROP_REGION, &entry)) {\n> \t\tconst int32_t *data = entry.data.i32;\n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 2e1e146a25..23d8323963 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -1283,7 +1283,7 @@ std::unique_ptr Camera::createRequest(uint64_t cookie)\n> * The control list is patched in place, turning the AeEnable control into\n> * the equivalent ExposureTimeMode/AnalogueGainMode controls.\n> */\n> -void Camera::patchControlList(ControlList &controls)\n> +void Camera::patchControlList(Request::ControlListWrapper controls)\n> {\n> \tconst auto &aeEnable = controls.get(controls::AeEnable);\n> \tif (aeEnable) {\n>\n>\n>\n> >\n> > >\n> > > Anyway,\n> > >\n> > > Reviewed-by: Kieran Bingham \n> > >\n> > > >\n> > > > queueCount_++;\n> > > >\n> > > >\n> > > > --\n> > > > 2.51.1\n> > > >\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 BB260C0F1B\n\tfor ;\n\tMon, 1 Dec 2025 09:59:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EFF7460AB3;\n\tMon, 1 Dec 2025 10:59:26 +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 97FA9609D8\n\tfor ;\n\tMon, 1 Dec 2025 10:59:25 +0100 (CET)","from ideasonboard.com (net-93-65-100-155.cust.vodafonedsl.it\n\t[93.65.100.155])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 64ECD4F1;\n\tMon, 1 Dec 2025 10:57:12 +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=\"QS26Fqdv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764583032;\n\tbh=e0mlvck2VLa4Mu/c07nhZPsts9k2Rof4e6vLWPro4wA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=QS26FqdvhVgyzbNnjumP2Qre2atWNWH2IynU8/xGywhQ5rkoiX5UjaZrF6uEvkl8z\n\trLYl6ftqenF2iEmvZjUe6qZNcB5j0ldS7y77XynPXFU52Qa3pCPXVTVBNBjiWFKIq5\n\tfP/MXrA2ArGSZxAkE26cf0rRD4X12A0g8w5ZBf7c=","Date":"Mon, 1 Dec 2025 10:59:22 +0100","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Jacopo Mondi , \n\tKieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/2] apps: cam: Do not override Request::controls()","Message-ID":"","References":"<20251124-cam-control-override-v1-0-dfc3a2f3feee@ideasonboard.com>\n\t<20251124-cam-control-override-v1-1-dfc3a2f3feee@ideasonboard.com>\n\t<176400449093.567526.9671839480214993818@ping.linuxembedded.co.uk>\n\t\n\t<215dd0be-4ad4-4756-9321-97227e7bbfba@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<215dd0be-4ad4-4756-9321-97227e7bbfba@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":37123,"web_url":"https://patchwork.libcamera.org/comment/37123/","msgid":"<2bfaf8f6-2a37-41ad-ba74-d197f447991f@ideasonboard.com>","date":"2025-12-01T10:05:46","subject":"Re: [PATCH 1/2] apps: cam: Do not override Request::controls()","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 12. 01. 10:59 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Mon, Dec 01, 2025 at 10:19:20AM +0100, Barnabás Pőcze wrote:\n>> Hi\n>>\n>> 2025. 11. 25. 8:54 keltezéssel, Jacopo Mondi írta:\n>>> Hi Kieran\n>>>\n>>> On Mon, Nov 24, 2025 at 05:14:50PM +0000, Kieran Bingham wrote:\n>>>> Quoting Jacopo Mondi (2025-11-24 16:43:44)\n>>>>> The ControlList associated with a Request are created with\n>>>>> the libcamera::controls::controls id map.\n>>>>>\n>>>>> The cam application, when parsing controls from a script\n>>>>> assigns to the Request's control list a newly created one, which is\n>>>>> initialized without a valid idmap or info map.\n>>>>>\n>>>>> This causes IPA that run in isolated mode to fail, as the IPC\n>>>>> serialization/deserialization code requires all ControlList to have\n>>>>> a valid idmap or info map associated.\n>>>>>\n>>>>> Instead of overwriting the Request::controls() list, merge it with\n>>>>> the one created by cam when parsing the capture script.\n>>>>>\n>>>>> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/295\n>>>>> Signed-off-by: Jacopo Mondi \n>>>>> ---\n>>>>> src/apps/cam/camera_session.cpp | 2 +-\n>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>>>>\n>>>>> diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp\n>>>>> index 1596a25a3abed9c2d93e6657b92e35fdfd3d1a26..0a24b7fafc5333f88053cb52a500cdafd7bb603f 100644\n>>>>> --- a/src/apps/cam/camera_session.cpp\n>>>>> +++ b/src/apps/cam/camera_session.cpp\n>>>>> @@ -447,7 +447,7 @@ int CameraSession::queueRequest(Request *request)\n>>>>> return 0;\n>>>>>\n>>>>> if (script_)\n>>>>> - request->controls() = script_->frameControls(queueCount_);\n>>>>> + request->controls().merge(script_->frameControls(queueCount_));\n>>>>\n>>>> That looks accurate even after we find a way to fix the permissions to\n>>>> prevent the underlying list itself being replaced.\n>>>>\n>>>> Is that possible by making it const somehow? a const pointer, to\n>>>> non-const data ?\n>>>\n>>> I'm looking into that\n>>>\n>>> Simply making \"ControlList &Request::controls()\" a \"const ControlList\n>>> &Request::controls() const\" doesn't work\n>>>\n>>> 1) The Camera class modifies the control list of a request, and we can\n>>> use the usual PIMPL pattern to expose to the library and interface\n>>> which is different than the one exposed to applications\n>>> 2) However a ControlList::merge()/set() can't be called on a const\n>>> instance of ControlList, so application won't be able to add controls\n>>> to a Request if we simply return \"const ControlList\" and we should add\n>>> methods to the Request class to set/merge controls like\n>>> \"Request::addControls()\" I'm not sure I like it though\n>>\n>> I'm not entirely sure that pimpl is sufficient here. I think pimpl is a bit\n>> of an orthogonal question here. Because what is important here is where the\n> \n> Could you explain a bit more what you mean here ?\n\nIf you have a method return `ControlList&`, then that can be assigned to\nunless the assignment operators have been deleted in the `ControlList` type.\nSo whether `ControlList` is a \"direct\" implementation (like it is today),\nor a \"pimpl\" one does not have any direct effect on whether or not it can\nbe assigned to. But disabling the assignment on the \"main\" `ControlList`\ntype might not be desirable, hence my proposal below with the wrapper type.\nI hope I'm making sense.\n\n\n> \n> The problem at end, in my view, is that we allow to override the\n> ControlLis which is part of a Request, and the Request class API\n> should be changed to disallow that.\n> \n>> assignment operators are deleted. I feel like if we were to delete them on\n>> the \"main\" `ControlList` type, that could cause non-negligible inconvenience.\n> \n> I don't this we should disallow assigning ControlList completely, no.\n> \n>>\n>> One thing that could be done is to add a wrapper around the reference, this\n>> way no assignment can be done, but every other member function can be preserved.\n>> It unfortunately has quite a bit of duplication, but it only needs two changes\n>> in the current code base (in addition to the `.merge()` change).\n>>\n>> Or alternatively, maybe we could change the design of idmap/infomap/serialization.\n>>\n>>\n>> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n>> index b24a297400..595752d4b6 100644\n>> --- a/include/libcamera/camera.h\n>> +++ b/include/libcamera/camera.h\n>> @@ -166,7 +166,7 @@ private:\n>> \tint exportFrameBuffers(Stream *stream,\n>> \t\t\t std::vector> *buffers);\n>> -\tvoid patchControlList(ControlList &controls);\n>> +\tvoid patchControlList(Request::ControlListWrapper controls);\n>> };\n>> } /* namespace libcamera */\n>> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n>> index 0c5939f7b3..1e85dcff1a 100644\n>> --- a/include/libcamera/request.h\n>> +++ b/include/libcamera/request.h\n>> @@ -49,7 +49,65 @@ public:\n>> \tvoid reuse(ReuseFlag flags = Default);\n>> -\tControlList &controls() { return *controls_; }\n>> +#ifndef __DOXYGEN__\n>> +\tclass ControlListWrapper {\n>> +\tpublic:\n>> +\t\tusing iterator = ControlList::iterator;\n>> +\t\tusing const_iterator = ControlList::const_iterator;\n>> +\n>> +\t\tControlListWrapper(ControlList& list)\n>> +\t\t\t: list_(list)\n>> +\t\t{\n>> +\t\t}\n>> +\n>> +\t\titerator begin() { return list_.begin(); }\n>> +\t\titerator end() { return list_.end(); }\n>> +\t\tconst_iterator begin() const { return list_.begin(); }\n>> +\t\tconst_iterator end() const { return list_.end(); }\n>> +\n>> +\t\tbool empty() const { return list_.empty(); }\n>> +\t\tstd::size_t size() const { return list_.size(); }\n>> +\n>> +\t\tvoid clear() { list_.clear(); }\n>> +\t\tvoid merge(const ControlList &source, ControlList::MergePolicy policy = ControlList::MergePolicy::KeepExisting)\n>> +\t\t{\n>> +\t\t\tlist_.merge(source, policy);\n>> +\t\t}\n>> +\n>> +\t\tbool contains(unsigned int id) const { return list_.contains(id); }\n>> +\n>> +\t\ttemplate\n>> +\t\tstd::optional get(const Control &ctrl) const\n>> +\t\t{\n>> +\t\t\treturn list_.get(ctrl);\n>> +\t\t}\n>> +\n>> +\t\ttemplate\n>> +\t\tvoid set(const Control &ctrl, const V &value)\n>> +\t\t{\n>> +\t\t\tlist_.set(ctrl, value);\n>> +\t\t}\n>> +\n>> +\t\ttemplate\n>> +\t\tvoid set(const Control> &ctrl, const std::initializer_list &value)\n>> +\t\t{\n>> +\t\t\tlist_.set(ctrl, value);\n>> +\t\t}\n>> +\n>> +\t\tconst ControlValue &get(unsigned int id) const { return list_.get(id); }\n>> +\t\tvoid set(unsigned int id, const ControlValue &value) { return list_.set(id, value); }\n>> +\n>> +\t\tconst ControlInfoMap *infoMap() const { return list_.infoMap(); }\n>> +\t\tconst ControlIdMap *idMap() const { return list_.idMap(); }\n>> +\n>> +\t\toperator const ControlList&() const { return list_; }\n>> +\n>> +\tprivate:\n>> +\t\tControlList &list_;\n>> +\t};\n>> +#endif\n>> +\n>> +\tControlListWrapper controls() { return *controls_; }\n>> \tControlList &metadata() { return *metadata_; }\n>> \tconst BufferMap &buffers() const { return bufferMap_; }\n>> \tint addBuffer(const Stream *stream, FrameBuffer *buffer,\n>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n>> index 80ff248c2a..266064f209 100644\n>> --- a/src/android/camera_device.cpp\n>> +++ b/src/android/camera_device.cpp\n>> @@ -804,7 +804,7 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n>> \t\treturn 0;\n>> \t/* Translate the Android request settings to libcamera controls. */\n>> -\tControlList &controls = descriptor->request_->controls();\n>> +\tauto controls = descriptor->request_->controls();\n>> \tcamera_metadata_ro_entry_t entry;\n>> \tif (settings.getEntry(ANDROID_SCALER_CROP_REGION, &entry)) {\n>> \t\tconst int32_t *data = entry.data.i32;\n>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n>> index 2e1e146a25..23d8323963 100644\n>> --- a/src/libcamera/camera.cpp\n>> +++ b/src/libcamera/camera.cpp\n>> @@ -1283,7 +1283,7 @@ std::unique_ptr Camera::createRequest(uint64_t cookie)\n>> * The control list is patched in place, turning the AeEnable control into\n>> * the equivalent ExposureTimeMode/AnalogueGainMode controls.\n>> */\n>> -void Camera::patchControlList(ControlList &controls)\n>> +void Camera::patchControlList(Request::ControlListWrapper controls)\n>> {\n>> \tconst auto &aeEnable = controls.get(controls::AeEnable);\n>> \tif (aeEnable) {\n>>\n>>\n>>\n>>>\n>>>>\n>>>> Anyway,\n>>>>\n>>>> Reviewed-by: Kieran Bingham \n>>>>\n>>>>>\n>>>>> queueCount_++;\n>>>>>\n>>>>>\n>>>>> --\n>>>>> 2.51.1\n>>>>>\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 5ED13BD80A\n\tfor ;\n\tMon, 1 Dec 2025 10:05:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5926C60AB6;\n\tMon, 1 Dec 2025 11:05:52 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 09FBC609D8\n\tfor ;\n\tMon, 1 Dec 2025 11:05:51 +0100 (CET)","from [192.168.33.24] (185.182.214.104.nat.pool.zt.hu\n\t[185.182.214.104])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 76B1D4F1;\n\tMon, 1 Dec 2025 11:03:37 +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=\"iaaOsfTd\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764583417;\n\tbh=7NHiCyr9hFkTTWLFcnqaUkUbkS8XSBuoXLnZOJzEGzs=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=iaaOsfTdX/qd4/L5Sb6UsYSnLIgb9oninVzvdw3Y0z/iqjefk3GwmMAonV1cuC3dz\n\tiU7QkPia3MMrggdDW7W+rexIPhgFUZ02zeKOceM9Fp24DL049fa9DMvYHbH1wNMvSH\n\tc6VIU4eVQwjhqDEFv1Wg+gvg7MDGxYzBL/gGQN8c=","Message-ID":"<2bfaf8f6-2a37-41ad-ba74-d197f447991f@ideasonboard.com>","Date":"Mon, 1 Dec 2025 11:05:46 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 1/2] apps: cam: Do not override Request::controls()","To":"Jacopo Mondi ","Cc":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20251124-cam-control-override-v1-0-dfc3a2f3feee@ideasonboard.com>\n\t<20251124-cam-control-override-v1-1-dfc3a2f3feee@ideasonboard.com>\n\t<176400449093.567526.9671839480214993818@ping.linuxembedded.co.uk>\n\t\n\t<215dd0be-4ad4-4756-9321-97227e7bbfba@ideasonboard.com>\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","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\" "}},{"id":37124,"web_url":"https://patchwork.libcamera.org/comment/37124/","msgid":"","date":"2025-12-01T10:10:17","subject":"Re: [PATCH 1/2] apps: cam: Do not override Request::controls()","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Mon, Dec 01, 2025 at 11:05:46AM +0100, Barnabás Pőcze wrote:\n> 2025. 12. 01. 10:59 keltezéssel, Jacopo Mondi írta:\n> > Hi Barnabás\n> >\n> > On Mon, Dec 01, 2025 at 10:19:20AM +0100, Barnabás Pőcze wrote:\n> > > Hi\n> > >\n> > > 2025. 11. 25. 8:54 keltezéssel, Jacopo Mondi írta:\n> > > > Hi Kieran\n> > > >\n> > > > On Mon, Nov 24, 2025 at 05:14:50PM +0000, Kieran Bingham wrote:\n> > > > > Quoting Jacopo Mondi (2025-11-24 16:43:44)\n> > > > > > The ControlList associated with a Request are created with\n> > > > > > the libcamera::controls::controls id map.\n> > > > > >\n> > > > > > The cam application, when parsing controls from a script\n> > > > > > assigns to the Request's control list a newly created one, which is\n> > > > > > initialized without a valid idmap or info map.\n> > > > > >\n> > > > > > This causes IPA that run in isolated mode to fail, as the IPC\n> > > > > > serialization/deserialization code requires all ControlList to have\n> > > > > > a valid idmap or info map associated.\n> > > > > >\n> > > > > > Instead of overwriting the Request::controls() list, merge it with\n> > > > > > the one created by cam when parsing the capture script.\n> > > > > >\n> > > > > > Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/295\n> > > > > > Signed-off-by: Jacopo Mondi \n> > > > > > ---\n> > > > > > src/apps/cam/camera_session.cpp | 2 +-\n> > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > > > > >\n> > > > > > diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp\n> > > > > > index 1596a25a3abed9c2d93e6657b92e35fdfd3d1a26..0a24b7fafc5333f88053cb52a500cdafd7bb603f 100644\n> > > > > > --- a/src/apps/cam/camera_session.cpp\n> > > > > > +++ b/src/apps/cam/camera_session.cpp\n> > > > > > @@ -447,7 +447,7 @@ int CameraSession::queueRequest(Request *request)\n> > > > > > return 0;\n> > > > > >\n> > > > > > if (script_)\n> > > > > > - request->controls() = script_->frameControls(queueCount_);\n> > > > > > + request->controls().merge(script_->frameControls(queueCount_));\n> > > > >\n> > > > > That looks accurate even after we find a way to fix the permissions to\n> > > > > prevent the underlying list itself being replaced.\n> > > > >\n> > > > > Is that possible by making it const somehow? a const pointer, to\n> > > > > non-const data ?\n> > > >\n> > > > I'm looking into that\n> > > >\n> > > > Simply making \"ControlList &Request::controls()\" a \"const ControlList\n> > > > &Request::controls() const\" doesn't work\n> > > >\n> > > > 1) The Camera class modifies the control list of a request, and we can\n> > > > use the usual PIMPL pattern to expose to the library and interface\n> > > > which is different than the one exposed to applications\n> > > > 2) However a ControlList::merge()/set() can't be called on a const\n> > > > instance of ControlList, so application won't be able to add controls\n> > > > to a Request if we simply return \"const ControlList\" and we should add\n> > > > methods to the Request class to set/merge controls like\n> > > > \"Request::addControls()\" I'm not sure I like it though\n> > >\n> > > I'm not entirely sure that pimpl is sufficient here. I think pimpl is a bit\n> > > of an orthogonal question here. Because what is important here is where the\n> >\n> > Could you explain a bit more what you mean here ?\n>\n> If you have a method return `ControlList&`, then that can be assigned to\n> unless the assignment operators have been deleted in the `ControlList` type.\n\nOr you can return a 'const Control&' and provide an interface in the\nRequest class to set controls \"on the request\" (which make also sense\nsemantically imho)\n\nhttps://patchwork.libcamera.org/patch/25212/\n\n> So whether `ControlList` is a \"direct\" implementation (like it is today),\n> or a \"pimpl\" one does not have any direct effect on whether or not it can\n> be assigned to. But disabling the assignment on the \"main\" `ControlList`\n> type might not be desirable, hence my proposal below with the wrapper type.\n> I hope I'm making sense.\n\nMaybe I was clear in my idea of pimpl-ing the Request class and not\nthe ControlList class ?\n>\n>\n> >\n> > The problem at end, in my view, is that we allow to override the\n> > ControlLis which is part of a Request, and the Request class API\n> > should be changed to disallow that.\n> >\n> > > assignment operators are deleted. I feel like if we were to delete them on\n> > > the \"main\" `ControlList` type, that could cause non-negligible inconvenience.\n> >\n> > I don't this we should disallow assigning ControlList completely, no.\n> >\n> > >\n> > > One thing that could be done is to add a wrapper around the reference, this\n> > > way no assignment can be done, but every other member function can be preserved.\n> > > It unfortunately has quite a bit of duplication, but it only needs two changes\n> > > in the current code base (in addition to the `.merge()` change).\n> > >\n> > > Or alternatively, maybe we could change the design of idmap/infomap/serialization.\n> > >\n> > >\n> > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > > index b24a297400..595752d4b6 100644\n> > > --- a/include/libcamera/camera.h\n> > > +++ b/include/libcamera/camera.h\n> > > @@ -166,7 +166,7 @@ private:\n> > > \tint exportFrameBuffers(Stream *stream,\n> > > \t\t\t std::vector> *buffers);\n> > > -\tvoid patchControlList(ControlList &controls);\n> > > +\tvoid patchControlList(Request::ControlListWrapper controls);\n> > > };\n> > > } /* namespace libcamera */\n> > > diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> > > index 0c5939f7b3..1e85dcff1a 100644\n> > > --- a/include/libcamera/request.h\n> > > +++ b/include/libcamera/request.h\n> > > @@ -49,7 +49,65 @@ public:\n> > > \tvoid reuse(ReuseFlag flags = Default);\n> > > -\tControlList &controls() { return *controls_; }\n> > > +#ifndef __DOXYGEN__\n> > > +\tclass ControlListWrapper {\n> > > +\tpublic:\n> > > +\t\tusing iterator = ControlList::iterator;\n> > > +\t\tusing const_iterator = ControlList::const_iterator;\n> > > +\n> > > +\t\tControlListWrapper(ControlList& list)\n> > > +\t\t\t: list_(list)\n> > > +\t\t{\n> > > +\t\t}\n> > > +\n> > > +\t\titerator begin() { return list_.begin(); }\n> > > +\t\titerator end() { return list_.end(); }\n> > > +\t\tconst_iterator begin() const { return list_.begin(); }\n> > > +\t\tconst_iterator end() const { return list_.end(); }\n> > > +\n> > > +\t\tbool empty() const { return list_.empty(); }\n> > > +\t\tstd::size_t size() const { return list_.size(); }\n> > > +\n> > > +\t\tvoid clear() { list_.clear(); }\n> > > +\t\tvoid merge(const ControlList &source, ControlList::MergePolicy policy = ControlList::MergePolicy::KeepExisting)\n> > > +\t\t{\n> > > +\t\t\tlist_.merge(source, policy);\n> > > +\t\t}\n> > > +\n> > > +\t\tbool contains(unsigned int id) const { return list_.contains(id); }\n> > > +\n> > > +\t\ttemplate\n> > > +\t\tstd::optional get(const Control &ctrl) const\n> > > +\t\t{\n> > > +\t\t\treturn list_.get(ctrl);\n> > > +\t\t}\n> > > +\n> > > +\t\ttemplate\n> > > +\t\tvoid set(const Control &ctrl, const V &value)\n> > > +\t\t{\n> > > +\t\t\tlist_.set(ctrl, value);\n> > > +\t\t}\n> > > +\n> > > +\t\ttemplate\n> > > +\t\tvoid set(const Control> &ctrl, const std::initializer_list &value)\n> > > +\t\t{\n> > > +\t\t\tlist_.set(ctrl, value);\n> > > +\t\t}\n> > > +\n> > > +\t\tconst ControlValue &get(unsigned int id) const { return list_.get(id); }\n> > > +\t\tvoid set(unsigned int id, const ControlValue &value) { return list_.set(id, value); }\n> > > +\n> > > +\t\tconst ControlInfoMap *infoMap() const { return list_.infoMap(); }\n> > > +\t\tconst ControlIdMap *idMap() const { return list_.idMap(); }\n> > > +\n> > > +\t\toperator const ControlList&() const { return list_; }\n> > > +\n> > > +\tprivate:\n> > > +\t\tControlList &list_;\n> > > +\t};\n> > > +#endif\n> > > +\n> > > +\tControlListWrapper controls() { return *controls_; }\n> > > \tControlList &metadata() { return *metadata_; }\n> > > \tconst BufferMap &buffers() const { return bufferMap_; }\n> > > \tint addBuffer(const Stream *stream, FrameBuffer *buffer,\n> > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > index 80ff248c2a..266064f209 100644\n> > > --- a/src/android/camera_device.cpp\n> > > +++ b/src/android/camera_device.cpp\n> > > @@ -804,7 +804,7 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n> > > \t\treturn 0;\n> > > \t/* Translate the Android request settings to libcamera controls. */\n> > > -\tControlList &controls = descriptor->request_->controls();\n> > > +\tauto controls = descriptor->request_->controls();\n> > > \tcamera_metadata_ro_entry_t entry;\n> > > \tif (settings.getEntry(ANDROID_SCALER_CROP_REGION, &entry)) {\n> > > \t\tconst int32_t *data = entry.data.i32;\n> > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > index 2e1e146a25..23d8323963 100644\n> > > --- a/src/libcamera/camera.cpp\n> > > +++ b/src/libcamera/camera.cpp\n> > > @@ -1283,7 +1283,7 @@ std::unique_ptr Camera::createRequest(uint64_t cookie)\n> > > * The control list is patched in place, turning the AeEnable control into\n> > > * the equivalent ExposureTimeMode/AnalogueGainMode controls.\n> > > */\n> > > -void Camera::patchControlList(ControlList &controls)\n> > > +void Camera::patchControlList(Request::ControlListWrapper controls)\n> > > {\n> > > \tconst auto &aeEnable = controls.get(controls::AeEnable);\n> > > \tif (aeEnable) {\n> > >\n> > >\n> > >\n> > > >\n> > > > >\n> > > > > Anyway,\n> > > > >\n> > > > > Reviewed-by: Kieran Bingham \n> > > > >\n> > > > > >\n> > > > > > queueCount_++;\n> > > > > >\n> > > > > >\n> > > > > > --\n> > > > > > 2.51.1\n> > > > > >\n> > >\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 5F905C0F1B\n\tfor ;\n\tMon, 1 Dec 2025 10:10:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5D23B60AB6;\n\tMon, 1 Dec 2025 11:10:23 +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 E1E89609D8\n\tfor ;\n\tMon, 1 Dec 2025 11:10:21 +0100 (CET)","from ideasonboard.com (net-93-65-100-155.cust.vodafonedsl.it\n\t[93.65.100.155])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1FE0DB7D;\n\tMon, 1 Dec 2025 11:08:07 +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=\"hWDzixT4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764583688;\n\tbh=8Cx+H9cWxbLvyFlbU5s80ACmSdF4EbgYRoXZyVypjjI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=hWDzixT4IVvkVnm2WFp/dPFI03M8LHNOFwD62JjGkSC75jgYDbMsMWKvAwGdeifeA\n\t3MBZ3FLab7YMzm60WGFXpEDWuqnHCb2Hn79PTIuvoDkre8JnoujLyW7hGdIbLX89nw\n\tkadf8rYjxbGFwHvkgPv+/KED2taTeOecudeMcj1k=","Date":"Mon, 1 Dec 2025 11:10:17 +0100","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Jacopo Mondi , \n\tKieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/2] apps: cam: Do not override Request::controls()","Message-ID":"","References":"<20251124-cam-control-override-v1-0-dfc3a2f3feee@ideasonboard.com>\n\t<20251124-cam-control-override-v1-1-dfc3a2f3feee@ideasonboard.com>\n\t<176400449093.567526.9671839480214993818@ping.linuxembedded.co.uk>\n\t\n\t<215dd0be-4ad4-4756-9321-97227e7bbfba@ideasonboard.com>\n\t\n\t<2bfaf8f6-2a37-41ad-ba74-d197f447991f@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<2bfaf8f6-2a37-41ad-ba74-d197f447991f@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":37126,"web_url":"https://patchwork.libcamera.org/comment/37126/","msgid":"<40bb1752-a23f-49a2-872f-220116ca9763@ideasonboard.com>","date":"2025-12-01T10:19:48","subject":"Re: [PATCH 1/2] apps: cam: Do not override Request::controls()","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 12. 01. 11:10 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Mon, Dec 01, 2025 at 11:05:46AM +0100, Barnabás Pőcze wrote:\n>> 2025. 12. 01. 10:59 keltezéssel, Jacopo Mondi írta:\n>>> Hi Barnabás\n>>>\n>>> On Mon, Dec 01, 2025 at 10:19:20AM +0100, Barnabás Pőcze wrote:\n>>>> Hi\n>>>>\n>>>> 2025. 11. 25. 8:54 keltezéssel, Jacopo Mondi írta:\n>>>>> Hi Kieran\n>>>>>\n>>>>> On Mon, Nov 24, 2025 at 05:14:50PM +0000, Kieran Bingham wrote:\n>>>>>> Quoting Jacopo Mondi (2025-11-24 16:43:44)\n>>>>>>> The ControlList associated with a Request are created with\n>>>>>>> the libcamera::controls::controls id map.\n>>>>>>>\n>>>>>>> The cam application, when parsing controls from a script\n>>>>>>> assigns to the Request's control list a newly created one, which is\n>>>>>>> initialized without a valid idmap or info map.\n>>>>>>>\n>>>>>>> This causes IPA that run in isolated mode to fail, as the IPC\n>>>>>>> serialization/deserialization code requires all ControlList to have\n>>>>>>> a valid idmap or info map associated.\n>>>>>>>\n>>>>>>> Instead of overwriting the Request::controls() list, merge it with\n>>>>>>> the one created by cam when parsing the capture script.\n>>>>>>>\n>>>>>>> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/295\n>>>>>>> Signed-off-by: Jacopo Mondi \n>>>>>>> ---\n>>>>>>> src/apps/cam/camera_session.cpp | 2 +-\n>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>>>>>>\n>>>>>>> diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp\n>>>>>>> index 1596a25a3abed9c2d93e6657b92e35fdfd3d1a26..0a24b7fafc5333f88053cb52a500cdafd7bb603f 100644\n>>>>>>> --- a/src/apps/cam/camera_session.cpp\n>>>>>>> +++ b/src/apps/cam/camera_session.cpp\n>>>>>>> @@ -447,7 +447,7 @@ int CameraSession::queueRequest(Request *request)\n>>>>>>> return 0;\n>>>>>>>\n>>>>>>> if (script_)\n>>>>>>> - request->controls() = script_->frameControls(queueCount_);\n>>>>>>> + request->controls().merge(script_->frameControls(queueCount_));\n>>>>>>\n>>>>>> That looks accurate even after we find a way to fix the permissions to\n>>>>>> prevent the underlying list itself being replaced.\n>>>>>>\n>>>>>> Is that possible by making it const somehow? a const pointer, to\n>>>>>> non-const data ?\n>>>>>\n>>>>> I'm looking into that\n>>>>>\n>>>>> Simply making \"ControlList &Request::controls()\" a \"const ControlList\n>>>>> &Request::controls() const\" doesn't work\n>>>>>\n>>>>> 1) The Camera class modifies the control list of a request, and we can\n>>>>> use the usual PIMPL pattern to expose to the library and interface\n>>>>> which is different than the one exposed to applications\n>>>>> 2) However a ControlList::merge()/set() can't be called on a const\n>>>>> instance of ControlList, so application won't be able to add controls\n>>>>> to a Request if we simply return \"const ControlList\" and we should add\n>>>>> methods to the Request class to set/merge controls like\n>>>>> \"Request::addControls()\" I'm not sure I like it though\n>>>>\n>>>> I'm not entirely sure that pimpl is sufficient here. I think pimpl is a bit\n>>>> of an orthogonal question here. Because what is important here is where the\n>>>\n>>> Could you explain a bit more what you mean here ?\n>>\n>> If you have a method return `ControlList&`, then that can be assigned to\n>> unless the assignment operators have been deleted in the `ControlList` type.\n> \n> Or you can return a 'const Control&' and provide an interface in the\n> Request class to set controls \"on the request\" (which make also sense\n> semantically imho)\n> \n> https://patchwork.libcamera.org/patch/25212/\n\nThat's right, but it would be ideal if the interface could be kept the same.\n\n\n> \n>> So whether `ControlList` is a \"direct\" implementation (like it is today),\n>> or a \"pimpl\" one does not have any direct effect on whether or not it can\n>> be assigned to. But disabling the assignment on the \"main\" `ControlList`\n>> type might not be desirable, hence my proposal below with the wrapper type.\n>> I hope I'm making sense.\n> \n> Maybe I was clear in my idea of pimpl-ing the Request class and not\n> the ControlList class ?\n\nAhh, I must have misunderstood.\n\n\n>>\n>>\n>>>\n>>> The problem at end, in my view, is that we allow to override the\n>>> ControlLis which is part of a Request, and the Request class API\n>>> should be changed to disallow that.\n>>>\n>>>> assignment operators are deleted. I feel like if we were to delete them on\n>>>> the \"main\" `ControlList` type, that could cause non-negligible inconvenience.\n>>>\n>>> I don't this we should disallow assigning ControlList completely, no.\n>>>\n>>>>\n>>>> One thing that could be done is to add a wrapper around the reference, this\n>>>> way no assignment can be done, but every other member function can be preserved.\n>>>> It unfortunately has quite a bit of duplication, but it only needs two changes\n>>>> in the current code base (in addition to the `.merge()` change).\n>>>>\n>>>> Or alternatively, maybe we could change the design of idmap/infomap/serialization.\n>>>>\n>>>>\n>>>> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n>>>> index b24a297400..595752d4b6 100644\n>>>> --- a/include/libcamera/camera.h\n>>>> +++ b/include/libcamera/camera.h\n>>>> @@ -166,7 +166,7 @@ private:\n>>>> \tint exportFrameBuffers(Stream *stream,\n>>>> \t\t\t std::vector> *buffers);\n>>>> -\tvoid patchControlList(ControlList &controls);\n>>>> +\tvoid patchControlList(Request::ControlListWrapper controls);\n>>>> };\n>>>> } /* namespace libcamera */\n>>>> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n>>>> index 0c5939f7b3..1e85dcff1a 100644\n>>>> --- a/include/libcamera/request.h\n>>>> +++ b/include/libcamera/request.h\n>>>> @@ -49,7 +49,65 @@ public:\n>>>> \tvoid reuse(ReuseFlag flags = Default);\n>>>> -\tControlList &controls() { return *controls_; }\n>>>> +#ifndef __DOXYGEN__\n>>>> +\tclass ControlListWrapper {\n>>>> +\tpublic:\n>>>> +\t\tusing iterator = ControlList::iterator;\n>>>> +\t\tusing const_iterator = ControlList::const_iterator;\n>>>> +\n>>>> +\t\tControlListWrapper(ControlList& list)\n>>>> +\t\t\t: list_(list)\n>>>> +\t\t{\n>>>> +\t\t}\n>>>> +\n>>>> +\t\titerator begin() { return list_.begin(); }\n>>>> +\t\titerator end() { return list_.end(); }\n>>>> +\t\tconst_iterator begin() const { return list_.begin(); }\n>>>> +\t\tconst_iterator end() const { return list_.end(); }\n>>>> +\n>>>> +\t\tbool empty() const { return list_.empty(); }\n>>>> +\t\tstd::size_t size() const { return list_.size(); }\n>>>> +\n>>>> +\t\tvoid clear() { list_.clear(); }\n>>>> +\t\tvoid merge(const ControlList &source, ControlList::MergePolicy policy = ControlList::MergePolicy::KeepExisting)\n>>>> +\t\t{\n>>>> +\t\t\tlist_.merge(source, policy);\n>>>> +\t\t}\n>>>> +\n>>>> +\t\tbool contains(unsigned int id) const { return list_.contains(id); }\n>>>> +\n>>>> +\t\ttemplate\n>>>> +\t\tstd::optional get(const Control &ctrl) const\n>>>> +\t\t{\n>>>> +\t\t\treturn list_.get(ctrl);\n>>>> +\t\t}\n>>>> +\n>>>> +\t\ttemplate\n>>>> +\t\tvoid set(const Control &ctrl, const V &value)\n>>>> +\t\t{\n>>>> +\t\t\tlist_.set(ctrl, value);\n>>>> +\t\t}\n>>>> +\n>>>> +\t\ttemplate\n>>>> +\t\tvoid set(const Control> &ctrl, const std::initializer_list &value)\n>>>> +\t\t{\n>>>> +\t\t\tlist_.set(ctrl, value);\n>>>> +\t\t}\n>>>> +\n>>>> +\t\tconst ControlValue &get(unsigned int id) const { return list_.get(id); }\n>>>> +\t\tvoid set(unsigned int id, const ControlValue &value) { return list_.set(id, value); }\n>>>> +\n>>>> +\t\tconst ControlInfoMap *infoMap() const { return list_.infoMap(); }\n>>>> +\t\tconst ControlIdMap *idMap() const { return list_.idMap(); }\n>>>> +\n>>>> +\t\toperator const ControlList&() const { return list_; }\n>>>> +\n>>>> +\tprivate:\n>>>> +\t\tControlList &list_;\n>>>> +\t};\n>>>> +#endif\n>>>> +\n>>>> +\tControlListWrapper controls() { return *controls_; }\n>>>> \tControlList &metadata() { return *metadata_; }\n>>>> \tconst BufferMap &buffers() const { return bufferMap_; }\n>>>> \tint addBuffer(const Stream *stream, FrameBuffer *buffer,\n>>>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n>>>> index 80ff248c2a..266064f209 100644\n>>>> --- a/src/android/camera_device.cpp\n>>>> +++ b/src/android/camera_device.cpp\n>>>> @@ -804,7 +804,7 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n>>>> \t\treturn 0;\n>>>> \t/* Translate the Android request settings to libcamera controls. */\n>>>> -\tControlList &controls = descriptor->request_->controls();\n>>>> +\tauto controls = descriptor->request_->controls();\n>>>> \tcamera_metadata_ro_entry_t entry;\n>>>> \tif (settings.getEntry(ANDROID_SCALER_CROP_REGION, &entry)) {\n>>>> \t\tconst int32_t *data = entry.data.i32;\n>>>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n>>>> index 2e1e146a25..23d8323963 100644\n>>>> --- a/src/libcamera/camera.cpp\n>>>> +++ b/src/libcamera/camera.cpp\n>>>> @@ -1283,7 +1283,7 @@ std::unique_ptr Camera::createRequest(uint64_t cookie)\n>>>> * The control list is patched in place, turning the AeEnable control into\n>>>> * the equivalent ExposureTimeMode/AnalogueGainMode controls.\n>>>> */\n>>>> -void Camera::patchControlList(ControlList &controls)\n>>>> +void Camera::patchControlList(Request::ControlListWrapper controls)\n>>>> {\n>>>> \tconst auto &aeEnable = controls.get(controls::AeEnable);\n>>>> \tif (aeEnable) {\n>>>>\n>>>>\n>>>>\n>>>>>\n>>>>>>\n>>>>>> Anyway,\n>>>>>>\n>>>>>> Reviewed-by: Kieran Bingham \n>>>>>>\n>>>>>>>\n>>>>>>> queueCount_++;\n>>>>>>>\n>>>>>>>\n>>>>>>> --\n>>>>>>> 2.51.1\n>>>>>>>\n>>>>\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 B1683BD80A\n\tfor ;\n\tMon, 1 Dec 2025 10:19:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 76C9960AB8;\n\tMon, 1 Dec 2025 11:19:54 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6FDD8609D8\n\tfor ;\n\tMon, 1 Dec 2025 11:19:53 +0100 (CET)","from [192.168.33.24] (185.182.214.104.nat.pool.zt.hu\n\t[185.182.214.104])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 946C66AC;\n\tMon, 1 Dec 2025 11:17:39 +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=\"fBenTjQc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764584259;\n\tbh=lpKMH2JIjnQbqXS1+9ai3+DoE+M/o2OcfLV51p/K8PU=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=fBenTjQcVPZwVvSlpl8RDv3l2+7HE/je0tTytjPSknZHbGGLqWtH0LYeCeI5VGrrf\n\tKRQHFFcbQzB3GaU8KnD6TTVZ9s3HbiFAhuSx25mnS5zYGu/0FqVHAIO4xHPq7tdJ2e\n\t/6oaMNzjyZSu3JKf0bYPPXYp6Av4B35C6MF77Wy8=","Message-ID":"<40bb1752-a23f-49a2-872f-220116ca9763@ideasonboard.com>","Date":"Mon, 1 Dec 2025 11:19:48 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 1/2] apps: cam: Do not override Request::controls()","To":"Jacopo Mondi ","Cc":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20251124-cam-control-override-v1-0-dfc3a2f3feee@ideasonboard.com>\n\t<20251124-cam-control-override-v1-1-dfc3a2f3feee@ideasonboard.com>\n\t<176400449093.567526.9671839480214993818@ping.linuxembedded.co.uk>\n\t\n\t<215dd0be-4ad4-4756-9321-97227e7bbfba@ideasonboard.com>\n\t\n\t<2bfaf8f6-2a37-41ad-ba74-d197f447991f@ideasonboard.com>\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","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\" "}},{"id":37131,"web_url":"https://patchwork.libcamera.org/comment/37131/","msgid":"","date":"2025-12-01T10:43:26","subject":"Re: [PATCH 1/2] apps: cam: Do not override Request::controls()","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Barnabás\n\nOn Mon, Dec 01, 2025 at 11:19:48AM +0100, Barnabás Pőcze wrote:\n> 2025. 12. 01. 11:10 keltezéssel, Jacopo Mondi írta:\n> > Hi Barnabás\n> >\n> > On Mon, Dec 01, 2025 at 11:05:46AM +0100, Barnabás Pőcze wrote:\n> > > 2025. 12. 01. 10:59 keltezéssel, Jacopo Mondi írta:\n> > > > Hi Barnabás\n> > > >\n> > > > On Mon, Dec 01, 2025 at 10:19:20AM +0100, Barnabás Pőcze wrote:\n> > > > > Hi\n> > > > >\n> > > > > 2025. 11. 25. 8:54 keltezéssel, Jacopo Mondi írta:\n> > > > > > Hi Kieran\n> > > > > >\n> > > > > > On Mon, Nov 24, 2025 at 05:14:50PM +0000, Kieran Bingham wrote:\n> > > > > > > Quoting Jacopo Mondi (2025-11-24 16:43:44)\n> > > > > > > > The ControlList associated with a Request are created with\n> > > > > > > > the libcamera::controls::controls id map.\n> > > > > > > >\n> > > > > > > > The cam application, when parsing controls from a script\n> > > > > > > > assigns to the Request's control list a newly created one, which is\n> > > > > > > > initialized without a valid idmap or info map.\n> > > > > > > >\n> > > > > > > > This causes IPA that run in isolated mode to fail, as the IPC\n> > > > > > > > serialization/deserialization code requires all ControlList to have\n> > > > > > > > a valid idmap or info map associated.\n> > > > > > > >\n> > > > > > > > Instead of overwriting the Request::controls() list, merge it with\n> > > > > > > > the one created by cam when parsing the capture script.\n> > > > > > > >\n> > > > > > > > Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/295\n> > > > > > > > Signed-off-by: Jacopo Mondi \n> > > > > > > > ---\n> > > > > > > > src/apps/cam/camera_session.cpp | 2 +-\n> > > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > > > > > > >\n> > > > > > > > diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp\n> > > > > > > > index 1596a25a3abed9c2d93e6657b92e35fdfd3d1a26..0a24b7fafc5333f88053cb52a500cdafd7bb603f 100644\n> > > > > > > > --- a/src/apps/cam/camera_session.cpp\n> > > > > > > > +++ b/src/apps/cam/camera_session.cpp\n> > > > > > > > @@ -447,7 +447,7 @@ int CameraSession::queueRequest(Request *request)\n> > > > > > > > return 0;\n> > > > > > > >\n> > > > > > > > if (script_)\n> > > > > > > > - request->controls() = script_->frameControls(queueCount_);\n> > > > > > > > + request->controls().merge(script_->frameControls(queueCount_));\n> > > > > > >\n> > > > > > > That looks accurate even after we find a way to fix the permissions to\n> > > > > > > prevent the underlying list itself being replaced.\n> > > > > > >\n> > > > > > > Is that possible by making it const somehow? a const pointer, to\n> > > > > > > non-const data ?\n> > > > > >\n> > > > > > I'm looking into that\n> > > > > >\n> > > > > > Simply making \"ControlList &Request::controls()\" a \"const ControlList\n> > > > > > &Request::controls() const\" doesn't work\n> > > > > >\n> > > > > > 1) The Camera class modifies the control list of a request, and we can\n> > > > > > use the usual PIMPL pattern to expose to the library and interface\n> > > > > > which is different than the one exposed to applications\n> > > > > > 2) However a ControlList::merge()/set() can't be called on a const\n> > > > > > instance of ControlList, so application won't be able to add controls\n> > > > > > to a Request if we simply return \"const ControlList\" and we should add\n> > > > > > methods to the Request class to set/merge controls like\n> > > > > > \"Request::addControls()\" I'm not sure I like it though\n> > > > >\n> > > > > I'm not entirely sure that pimpl is sufficient here. I think pimpl is a bit\n> > > > > of an orthogonal question here. Because what is important here is where the\n> > > >\n> > > > Could you explain a bit more what you mean here ?\n> > >\n> > > If you have a method return `ControlList&`, then that can be assigned to\n> > > unless the assignment operators have been deleted in the `ControlList` type.\n> >\n> > Or you can return a 'const Control&' and provide an interface in the\n> > Request class to set controls \"on the request\" (which make also sense\n> > semantically imho)\n> >\n> > https://patchwork.libcamera.org/patch/25212/\n>\n> That's right, but it would be ideal if the interface could be kept the same.\n\nWhich \"interface\" ?\n\nTo be honest, the thing I don't like about my above patch is that\nwe're replicating the \"set\" ControlList interface on the Request.\n\nIt's not a huge deal in my opinion, but it feels like we're now bound\nto keep the two in sync.\n\n>\n>\n> >\n> > > So whether `ControlList` is a \"direct\" implementation (like it is today),\n> > > or a \"pimpl\" one does not have any direct effect on whether or not it can\n> > > be assigned to. But disabling the assignment on the \"main\" `ControlList`\n> > > type might not be desirable, hence my proposal below with the wrapper type.\n> > > I hope I'm making sense.\n> >\n> > Maybe I was clear in my idea of pimpl-ing the Request class and not\n> > the ControlList class ?\n>\n> Ahh, I must have misunderstood.\n>\n>\n> > >\n> > >\n> > > >\n> > > > The problem at end, in my view, is that we allow to override the\n> > > > ControlLis which is part of a Request, and the Request class API\n> > > > should be changed to disallow that.\n> > > >\n> > > > > assignment operators are deleted. I feel like if we were to delete them on\n> > > > > the \"main\" `ControlList` type, that could cause non-negligible inconvenience.\n> > > >\n> > > > I don't this we should disallow assigning ControlList completely, no.\n> > > >\n> > > > >\n> > > > > One thing that could be done is to add a wrapper around the reference, this\n> > > > > way no assignment can be done, but every other member function can be preserved.\n> > > > > It unfortunately has quite a bit of duplication, but it only needs two changes\n> > > > > in the current code base (in addition to the `.merge()` change).\n> > > > >\n> > > > > Or alternatively, maybe we could change the design of idmap/infomap/serialization.\n> > > > >\n> > > > >\n> > > > > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> > > > > index b24a297400..595752d4b6 100644\n> > > > > --- a/include/libcamera/camera.h\n> > > > > +++ b/include/libcamera/camera.h\n> > > > > @@ -166,7 +166,7 @@ private:\n> > > > > \tint exportFrameBuffers(Stream *stream,\n> > > > > \t\t\t std::vector> *buffers);\n> > > > > -\tvoid patchControlList(ControlList &controls);\n> > > > > +\tvoid patchControlList(Request::ControlListWrapper controls);\n> > > > > };\n> > > > > } /* namespace libcamera */\n> > > > > diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> > > > > index 0c5939f7b3..1e85dcff1a 100644\n> > > > > --- a/include/libcamera/request.h\n> > > > > +++ b/include/libcamera/request.h\n> > > > > @@ -49,7 +49,65 @@ public:\n> > > > > \tvoid reuse(ReuseFlag flags = Default);\n> > > > > -\tControlList &controls() { return *controls_; }\n> > > > > +#ifndef __DOXYGEN__\n> > > > > +\tclass ControlListWrapper {\n> > > > > +\tpublic:\n> > > > > +\t\tusing iterator = ControlList::iterator;\n> > > > > +\t\tusing const_iterator = ControlList::const_iterator;\n> > > > > +\n> > > > > +\t\tControlListWrapper(ControlList& list)\n> > > > > +\t\t\t: list_(list)\n> > > > > +\t\t{\n> > > > > +\t\t}\n> > > > > +\n> > > > > +\t\titerator begin() { return list_.begin(); }\n> > > > > +\t\titerator end() { return list_.end(); }\n> > > > > +\t\tconst_iterator begin() const { return list_.begin(); }\n> > > > > +\t\tconst_iterator end() const { return list_.end(); }\n> > > > > +\n> > > > > +\t\tbool empty() const { return list_.empty(); }\n> > > > > +\t\tstd::size_t size() const { return list_.size(); }\n> > > > > +\n> > > > > +\t\tvoid clear() { list_.clear(); }\n> > > > > +\t\tvoid merge(const ControlList &source, ControlList::MergePolicy policy = ControlList::MergePolicy::KeepExisting)\n> > > > > +\t\t{\n> > > > > +\t\t\tlist_.merge(source, policy);\n> > > > > +\t\t}\n> > > > > +\n> > > > > +\t\tbool contains(unsigned int id) const { return list_.contains(id); }\n> > > > > +\n> > > > > +\t\ttemplate\n> > > > > +\t\tstd::optional get(const Control &ctrl) const\n> > > > > +\t\t{\n> > > > > +\t\t\treturn list_.get(ctrl);\n> > > > > +\t\t}\n> > > > > +\n> > > > > +\t\ttemplate\n> > > > > +\t\tvoid set(const Control &ctrl, const V &value)\n> > > > > +\t\t{\n> > > > > +\t\t\tlist_.set(ctrl, value);\n> > > > > +\t\t}\n> > > > > +\n> > > > > +\t\ttemplate\n> > > > > +\t\tvoid set(const Control> &ctrl, const std::initializer_list &value)\n> > > > > +\t\t{\n> > > > > +\t\t\tlist_.set(ctrl, value);\n> > > > > +\t\t}\n> > > > > +\n> > > > > +\t\tconst ControlValue &get(unsigned int id) const { return list_.get(id); }\n> > > > > +\t\tvoid set(unsigned int id, const ControlValue &value) { return list_.set(id, value); }\n> > > > > +\n> > > > > +\t\tconst ControlInfoMap *infoMap() const { return list_.infoMap(); }\n> > > > > +\t\tconst ControlIdMap *idMap() const { return list_.idMap(); }\n> > > > > +\n> > > > > +\t\toperator const ControlList&() const { return list_; }\n> > > > > +\n> > > > > +\tprivate:\n> > > > > +\t\tControlList &list_;\n> > > > > +\t};\n> > > > > +#endif\n> > > > > +\n> > > > > +\tControlListWrapper controls() { return *controls_; }\n> > > > > \tControlList &metadata() { return *metadata_; }\n> > > > > \tconst BufferMap &buffers() const { return bufferMap_; }\n> > > > > \tint addBuffer(const Stream *stream, FrameBuffer *buffer,\n> > > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > > > index 80ff248c2a..266064f209 100644\n> > > > > --- a/src/android/camera_device.cpp\n> > > > > +++ b/src/android/camera_device.cpp\n> > > > > @@ -804,7 +804,7 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n> > > > > \t\treturn 0;\n> > > > > \t/* Translate the Android request settings to libcamera controls. */\n> > > > > -\tControlList &controls = descriptor->request_->controls();\n> > > > > +\tauto controls = descriptor->request_->controls();\n> > > > > \tcamera_metadata_ro_entry_t entry;\n> > > > > \tif (settings.getEntry(ANDROID_SCALER_CROP_REGION, &entry)) {\n> > > > > \t\tconst int32_t *data = entry.data.i32;\n> > > > > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> > > > > index 2e1e146a25..23d8323963 100644\n> > > > > --- a/src/libcamera/camera.cpp\n> > > > > +++ b/src/libcamera/camera.cpp\n> > > > > @@ -1283,7 +1283,7 @@ std::unique_ptr Camera::createRequest(uint64_t cookie)\n> > > > > * The control list is patched in place, turning the AeEnable control into\n> > > > > * the equivalent ExposureTimeMode/AnalogueGainMode controls.\n> > > > > */\n> > > > > -void Camera::patchControlList(ControlList &controls)\n> > > > > +void Camera::patchControlList(Request::ControlListWrapper controls)\n> > > > > {\n> > > > > \tconst auto &aeEnable = controls.get(controls::AeEnable);\n> > > > > \tif (aeEnable) {\n> > > > >\n> > > > >\n> > > > >\n> > > > > >\n> > > > > > >\n> > > > > > > Anyway,\n> > > > > > >\n> > > > > > > Reviewed-by: Kieran Bingham \n> > > > > > >\n> > > > > > > >\n> > > > > > > > queueCount_++;\n> > > > > > > >\n> > > > > > > >\n> > > > > > > > --\n> > > > > > > > 2.51.1\n> > > > > > > >\n> > > > >\n> > >\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 032ABC0F1B\n\tfor ;\n\tMon, 1 Dec 2025 10:43:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F3DEB60AC4;\n\tMon, 1 Dec 2025 11:43:30 +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 31637609D8\n\tfor ;\n\tMon, 1 Dec 2025 11:43:29 +0100 (CET)","from ideasonboard.com (net-93-65-100-155.cust.vodafonedsl.it\n\t[93.65.100.155])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A600F6AC;\n\tMon, 1 Dec 2025 11:41:15 +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=\"s3TNxEN/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764585675;\n\tbh=+PJqNSLBOhFMrBcXvrKRpMhfZsCm5hiQzAcZOzfjkBc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=s3TNxEN/e+RqumGChJD4Y210/yZuCbBe0b4SInZSFF3RXtkHuZWxoK0U2MZzx+Cg7\n\try+J9gI/25mxxXZiwD2YGAGhgRPXPHEK1hP+6amnipsImb5cXha3vA0y6QVcdR4rMj\n\t9SNV40MQEMJMhfLrOA3yxSkw5eE6UYMsEFaJ8HfM=","Date":"Mon, 1 Dec 2025 11:43:26 +0100","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Jacopo Mondi , \n\tKieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH 1/2] apps: cam: Do not override Request::controls()","Message-ID":"","References":"<20251124-cam-control-override-v1-0-dfc3a2f3feee@ideasonboard.com>\n\t<20251124-cam-control-override-v1-1-dfc3a2f3feee@ideasonboard.com>\n\t<176400449093.567526.9671839480214993818@ping.linuxembedded.co.uk>\n\t\n\t<215dd0be-4ad4-4756-9321-97227e7bbfba@ideasonboard.com>\n\t\n\t<2bfaf8f6-2a37-41ad-ba74-d197f447991f@ideasonboard.com>\n\t\n\t<40bb1752-a23f-49a2-872f-220116ca9763@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<40bb1752-a23f-49a2-872f-220116ca9763@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":37139,"web_url":"https://patchwork.libcamera.org/comment/37139/","msgid":"","date":"2025-12-01T12:27:46","subject":"Re: [PATCH 1/2] apps: cam: Do not override Request::controls()","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 12. 01. 11:43 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Mon, Dec 01, 2025 at 11:19:48AM +0100, Barnabás Pőcze wrote:\n>> 2025. 12. 01. 11:10 keltezéssel, Jacopo Mondi írta:\n>>> Hi Barnabás\n>>>\n>>> On Mon, Dec 01, 2025 at 11:05:46AM +0100, Barnabás Pőcze wrote:\n>>>> 2025. 12. 01. 10:59 keltezéssel, Jacopo Mondi írta:\n>>>>> Hi Barnabás\n>>>>>\n>>>>> On Mon, Dec 01, 2025 at 10:19:20AM +0100, Barnabás Pőcze wrote:\n>>>>>> Hi\n>>>>>>\n>>>>>> 2025. 11. 25. 8:54 keltezéssel, Jacopo Mondi írta:\n>>>>>>> Hi Kieran\n>>>>>>>\n>>>>>>> On Mon, Nov 24, 2025 at 05:14:50PM +0000, Kieran Bingham wrote:\n>>>>>>>> Quoting Jacopo Mondi (2025-11-24 16:43:44)\n>>>>>>>>> The ControlList associated with a Request are created with\n>>>>>>>>> the libcamera::controls::controls id map.\n>>>>>>>>>\n>>>>>>>>> The cam application, when parsing controls from a script\n>>>>>>>>> assigns to the Request's control list a newly created one, which is\n>>>>>>>>> initialized without a valid idmap or info map.\n>>>>>>>>>\n>>>>>>>>> This causes IPA that run in isolated mode to fail, as the IPC\n>>>>>>>>> serialization/deserialization code requires all ControlList to have\n>>>>>>>>> a valid idmap or info map associated.\n>>>>>>>>>\n>>>>>>>>> Instead of overwriting the Request::controls() list, merge it with\n>>>>>>>>> the one created by cam when parsing the capture script.\n>>>>>>>>>\n>>>>>>>>> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/295\n>>>>>>>>> Signed-off-by: Jacopo Mondi \n>>>>>>>>> ---\n>>>>>>>>> src/apps/cam/camera_session.cpp | 2 +-\n>>>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>>>>>>>>\n>>>>>>>>> diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp\n>>>>>>>>> index 1596a25a3abed9c2d93e6657b92e35fdfd3d1a26..0a24b7fafc5333f88053cb52a500cdafd7bb603f 100644\n>>>>>>>>> --- a/src/apps/cam/camera_session.cpp\n>>>>>>>>> +++ b/src/apps/cam/camera_session.cpp\n>>>>>>>>> @@ -447,7 +447,7 @@ int CameraSession::queueRequest(Request *request)\n>>>>>>>>> return 0;\n>>>>>>>>>\n>>>>>>>>> if (script_)\n>>>>>>>>> - request->controls() = script_->frameControls(queueCount_);\n>>>>>>>>> + request->controls().merge(script_->frameControls(queueCount_));\n>>>>>>>>\n>>>>>>>> That looks accurate even after we find a way to fix the permissions to\n>>>>>>>> prevent the underlying list itself being replaced.\n>>>>>>>>\n>>>>>>>> Is that possible by making it const somehow? a const pointer, to\n>>>>>>>> non-const data ?\n>>>>>>>\n>>>>>>> I'm looking into that\n>>>>>>>\n>>>>>>> Simply making \"ControlList &Request::controls()\" a \"const ControlList\n>>>>>>> &Request::controls() const\" doesn't work\n>>>>>>>\n>>>>>>> 1) The Camera class modifies the control list of a request, and we can\n>>>>>>> use the usual PIMPL pattern to expose to the library and interface\n>>>>>>> which is different than the one exposed to applications\n>>>>>>> 2) However a ControlList::merge()/set() can't be called on a const\n>>>>>>> instance of ControlList, so application won't be able to add controls\n>>>>>>> to a Request if we simply return \"const ControlList\" and we should add\n>>>>>>> methods to the Request class to set/merge controls like\n>>>>>>> \"Request::addControls()\" I'm not sure I like it though\n>>>>>>\n>>>>>> I'm not entirely sure that pimpl is sufficient here. I think pimpl is a bit\n>>>>>> of an orthogonal question here. Because what is important here is where the\n>>>>>\n>>>>> Could you explain a bit more what you mean here ?\n>>>>\n>>>> If you have a method return `ControlList&`, then that can be assigned to\n>>>> unless the assignment operators have been deleted in the `ControlList` type.\n>>>\n>>> Or you can return a 'const Control&' and provide an interface in the\n>>> Request class to set controls \"on the request\" (which make also sense\n>>> semantically imho)\n>>>\n>>> https://patchwork.libcamera.org/patch/25212/\n>>\n>> That's right, but it would be ideal if the interface could be kept the same.\n> \n> Which \"interface\" ?\n\nBy \"interface\" I meant that it might be nice to make `request->controls()`\nbehave like a proper `ControlList`, i.e. have (mostly) the same methods,\neven for setting controls.\n\n\n> \n> To be honest, the thing I don't like about my above patch is that\n> we're replicating the \"set\" ControlList interface on the Request.\n> \n> It's not a huge deal in my opinion, but it feels like we're now bound\n> to keep the two in sync.\n> \n>>\n>>\n>>>\n>>>> So whether `ControlList` is a \"direct\" implementation (like it is today),\n>>>> or a \"pimpl\" one does not have any direct effect on whether or not it can\n>>>> be assigned to. But disabling the assignment on the \"main\" `ControlList`\n>>>> type might not be desirable, hence my proposal below with the wrapper type.\n>>>> I hope I'm making sense.\n>>>\n>>> Maybe I was clear in my idea of pimpl-ing the Request class and not\n>>> the ControlList class ?\n>>\n>> Ahh, I must have misunderstood.\n>>\n>>\n>>>>\n>>>>\n>>>>>\n>>>>> The problem at end, in my view, is that we allow to override the\n>>>>> ControlLis which is part of a Request, and the Request class API\n>>>>> should be changed to disallow that.\n>>>>>\n>>>>>> assignment operators are deleted. I feel like if we were to delete them on\n>>>>>> the \"main\" `ControlList` type, that could cause non-negligible inconvenience.\n>>>>>\n>>>>> I don't this we should disallow assigning ControlList completely, no.\n>>>>>\n>>>>>>\n>>>>>> One thing that could be done is to add a wrapper around the reference, this\n>>>>>> way no assignment can be done, but every other member function can be preserved.\n>>>>>> It unfortunately has quite a bit of duplication, but it only needs two changes\n>>>>>> in the current code base (in addition to the `.merge()` change).\n>>>>>>\n>>>>>> Or alternatively, maybe we could change the design of idmap/infomap/serialization.\n>>>>>>\n>>>>>>\n>>>>>> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n>>>>>> index b24a297400..595752d4b6 100644\n>>>>>> --- a/include/libcamera/camera.h\n>>>>>> +++ b/include/libcamera/camera.h\n>>>>>> @@ -166,7 +166,7 @@ private:\n>>>>>> \tint exportFrameBuffers(Stream *stream,\n>>>>>> \t\t\t std::vector> *buffers);\n>>>>>> -\tvoid patchControlList(ControlList &controls);\n>>>>>> +\tvoid patchControlList(Request::ControlListWrapper controls);\n>>>>>> };\n>>>>>> } /* namespace libcamera */\n>>>>>> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n>>>>>> index 0c5939f7b3..1e85dcff1a 100644\n>>>>>> --- a/include/libcamera/request.h\n>>>>>> +++ b/include/libcamera/request.h\n>>>>>> @@ -49,7 +49,65 @@ public:\n>>>>>> \tvoid reuse(ReuseFlag flags = Default);\n>>>>>> -\tControlList &controls() { return *controls_; }\n>>>>>> +#ifndef __DOXYGEN__\n>>>>>> +\tclass ControlListWrapper {\n>>>>>> +\tpublic:\n>>>>>> +\t\tusing iterator = ControlList::iterator;\n>>>>>> +\t\tusing const_iterator = ControlList::const_iterator;\n>>>>>> +\n>>>>>> +\t\tControlListWrapper(ControlList& list)\n>>>>>> +\t\t\t: list_(list)\n>>>>>> +\t\t{\n>>>>>> +\t\t}\n>>>>>> +\n>>>>>> +\t\titerator begin() { return list_.begin(); }\n>>>>>> +\t\titerator end() { return list_.end(); }\n>>>>>> +\t\tconst_iterator begin() const { return list_.begin(); }\n>>>>>> +\t\tconst_iterator end() const { return list_.end(); }\n>>>>>> +\n>>>>>> +\t\tbool empty() const { return list_.empty(); }\n>>>>>> +\t\tstd::size_t size() const { return list_.size(); }\n>>>>>> +\n>>>>>> +\t\tvoid clear() { list_.clear(); }\n>>>>>> +\t\tvoid merge(const ControlList &source, ControlList::MergePolicy policy = ControlList::MergePolicy::KeepExisting)\n>>>>>> +\t\t{\n>>>>>> +\t\t\tlist_.merge(source, policy);\n>>>>>> +\t\t}\n>>>>>> +\n>>>>>> +\t\tbool contains(unsigned int id) const { return list_.contains(id); }\n>>>>>> +\n>>>>>> +\t\ttemplate\n>>>>>> +\t\tstd::optional get(const Control &ctrl) const\n>>>>>> +\t\t{\n>>>>>> +\t\t\treturn list_.get(ctrl);\n>>>>>> +\t\t}\n>>>>>> +\n>>>>>> +\t\ttemplate\n>>>>>> +\t\tvoid set(const Control &ctrl, const V &value)\n>>>>>> +\t\t{\n>>>>>> +\t\t\tlist_.set(ctrl, value);\n>>>>>> +\t\t}\n>>>>>> +\n>>>>>> +\t\ttemplate\n>>>>>> +\t\tvoid set(const Control> &ctrl, const std::initializer_list &value)\n>>>>>> +\t\t{\n>>>>>> +\t\t\tlist_.set(ctrl, value);\n>>>>>> +\t\t}\n>>>>>> +\n>>>>>> +\t\tconst ControlValue &get(unsigned int id) const { return list_.get(id); }\n>>>>>> +\t\tvoid set(unsigned int id, const ControlValue &value) { return list_.set(id, value); }\n>>>>>> +\n>>>>>> +\t\tconst ControlInfoMap *infoMap() const { return list_.infoMap(); }\n>>>>>> +\t\tconst ControlIdMap *idMap() const { return list_.idMap(); }\n>>>>>> +\n>>>>>> +\t\toperator const ControlList&() const { return list_; }\n>>>>>> +\n>>>>>> +\tprivate:\n>>>>>> +\t\tControlList &list_;\n>>>>>> +\t};\n>>>>>> +#endif\n>>>>>> +\n>>>>>> +\tControlListWrapper controls() { return *controls_; }\n>>>>>> \tControlList &metadata() { return *metadata_; }\n>>>>>> \tconst BufferMap &buffers() const { return bufferMap_; }\n>>>>>> \tint addBuffer(const Stream *stream, FrameBuffer *buffer,\n>>>>>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n>>>>>> index 80ff248c2a..266064f209 100644\n>>>>>> --- a/src/android/camera_device.cpp\n>>>>>> +++ b/src/android/camera_device.cpp\n>>>>>> @@ -804,7 +804,7 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n>>>>>> \t\treturn 0;\n>>>>>> \t/* Translate the Android request settings to libcamera controls. */\n>>>>>> -\tControlList &controls = descriptor->request_->controls();\n>>>>>> +\tauto controls = descriptor->request_->controls();\n>>>>>> \tcamera_metadata_ro_entry_t entry;\n>>>>>> \tif (settings.getEntry(ANDROID_SCALER_CROP_REGION, &entry)) {\n>>>>>> \t\tconst int32_t *data = entry.data.i32;\n>>>>>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n>>>>>> index 2e1e146a25..23d8323963 100644\n>>>>>> --- a/src/libcamera/camera.cpp\n>>>>>> +++ b/src/libcamera/camera.cpp\n>>>>>> @@ -1283,7 +1283,7 @@ std::unique_ptr Camera::createRequest(uint64_t cookie)\n>>>>>> * The control list is patched in place, turning the AeEnable control into\n>>>>>> * the equivalent ExposureTimeMode/AnalogueGainMode controls.\n>>>>>> */\n>>>>>> -void Camera::patchControlList(ControlList &controls)\n>>>>>> +void Camera::patchControlList(Request::ControlListWrapper controls)\n>>>>>> {\n>>>>>> \tconst auto &aeEnable = controls.get(controls::AeEnable);\n>>>>>> \tif (aeEnable) {\n>>>>>>\n>>>>>>\n>>>>>>\n>>>>>>>\n>>>>>>>>\n>>>>>>>> Anyway,\n>>>>>>>>\n>>>>>>>> Reviewed-by: Kieran Bingham \n>>>>>>>>\n>>>>>>>>>\n>>>>>>>>> queueCount_++;\n>>>>>>>>>\n>>>>>>>>>\n>>>>>>>>> --\n>>>>>>>>> 2.51.1\n>>>>>>>>>\n>>>>>>\n>>>>\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 93034BD80A\n\tfor ;\n\tMon, 1 Dec 2025 12:27:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AB5B160A7B;\n\tMon, 1 Dec 2025 13:27:51 +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 CEC7060A7B\n\tfor ;\n\tMon, 1 Dec 2025 13:27:49 +0100 (CET)","from [192.168.33.24] (185.182.214.104.nat.pool.zt.hu\n\t[185.182.214.104])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 92B6FB3;\n\tMon, 1 Dec 2025 13:25:36 +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=\"Vodjn8Xp\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1764591936;\n\tbh=M6o2V99PcLGxUG78l1tyEWWAcFkchIMKLcTJB6Z2sqU=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=Vodjn8XpXfFeJMGU+r0APegCF/Mg4WbaRaOrp0n2Ws84fBiSnsJIHv4YNoq9eRKR0\n\tj9zLUaLAWgMLhONgKIUU09v0NG+Ar66S8cV41GVDYPZhAZgEASKEIoPTiXqFFcihMw\n\tKbhJisadCTrapolMGOHWQc9Q9s1xSdd8Tx5nERiE=","Message-ID":"","Date":"Mon, 1 Dec 2025 13:27:46 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 1/2] apps: cam: Do not override Request::controls()","To":"Jacopo Mondi ","Cc":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20251124-cam-control-override-v1-0-dfc3a2f3feee@ideasonboard.com>\n\t<20251124-cam-control-override-v1-1-dfc3a2f3feee@ideasonboard.com>\n\t<176400449093.567526.9671839480214993818@ping.linuxembedded.co.uk>\n\t\n\t<215dd0be-4ad4-4756-9321-97227e7bbfba@ideasonboard.com>\n\t\n\t<2bfaf8f6-2a37-41ad-ba74-d197f447991f@ideasonboard.com>\n\t\n\t<40bb1752-a23f-49a2-872f-220116ca9763@ideasonboard.com>\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","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\" "}}]