[{"id":36980,"web_url":"https://patchwork.libcamera.org/comment/36980/","msgid":"","date":"2025-11-21T11:47:57","subject":"Re: [PATCH v2 3/5] libcamera: controls: Implement move\n\tctor/assignment","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, Apr 21, 2025 at 05:35:54PM +0200, Barnabás Pőcze wrote:\n> Implement a move constructor and move assignment operator for `ControlValue`.\n> The `ControlValue` type already has an \"empty\" state that is used when\n> creating a default constructed `ControlValue`, so have the moved-from\n> instance return to that state after move construction/assignment.\n>\n> This is useful, for example, for `std::vector` as most implementations will\n> use the copy constructor when reallocating if no nothrow move constructor\n> is available. Having a nothrow move constructor avoids the extra copies.\n> It is also useful when using temporaries of `ControlValue` with other\n> containers such as `std::optional`, and it also makes `ControlInfo`\n> \"cheaply\" move constructible/assignable.\n>\n> Signed-off-by: Barnabás Pőcze \n> Reviewed-by: Kieran Bingham \n> Reviewed-by: Jacopo Mondi \n> ---\n> include/libcamera/controls.h | 26 ++++++++++++++++++++++++++\n> src/libcamera/controls.cpp | 18 ++++++++++++++++++\n> 2 files changed, 44 insertions(+)\n>\n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index 3f3580036..866d667c4 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -14,6 +14,7 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n>\n> #include \n> @@ -165,6 +166,31 @@ public:\n> \tControlValue(const ControlValue &other);\n> \tControlValue &operator=(const ControlValue &other);\n>\n> +\tControlValue(ControlValue &&other) noexcept\n> +\t\t: type_(other.type_),\n\nIs it necessary to reset the other's type to ControlTypeNone ?\n\n> +\t\t isArray_(std::exchange(other.isArray_, false)),\n> +\t\t numElements_(std::exchange(other.numElements_, 0)),\n> +\t\t storage_(std::exchange(other.storage_, {}))\n> +\t{\n> +\t\tother.type_ = ControlTypeNone;\n> +\t}\n> +\n> +\tControlValue &operator=(ControlValue &&other) noexcept\n> +\t{\n> +\t\tif (this != &other) {\n> +\t\t\trelease();\n> +\n> +\t\t\ttype_ = other.type_;\n> +\t\t\tisArray_ = std::exchange(other.isArray_, false);\n> +\t\t\tnumElements_ = std::exchange(other.numElements_, 0);\n> +\t\t\tstorage_ = std::exchange(other.storage_, {});\n> +\n> +\t\t\tother.type_ = ControlTypeNone;\n> +\t\t}\n> +\n> +\t\treturn *this;\n> +\t}\n> +\n> \tControlType type() const { return type_; }\n> \tbool isNone() const { return type_ == ControlTypeNone; }\n> \tbool isArray() const { return isArray_; }\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index d384e1ef7..135e87613 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -155,6 +155,24 @@ ControlValue &ControlValue::operator=(const ControlValue &other)\n> \treturn *this;\n> }\n>\n> +/**\n> + * \\fn ControlValue::ControlValue(ControlValue &&other) noexcept\n> + * \\brief Move constructor for ControlValue\n> + * \\param[in] other The ControlValue object to move from\n> + *\n> + * Move constructs a ControlValue instance from \\a other.\n> + * After this operation \\a other will be in the same state\n> + * as a default constructed ControlValue instance.\n> + */\n> +\n> +/**\n> + * \\fn ControlValue &ControlValue::operator=(ControlValue &&other) noexcept\n> + * \\brief Move assignment operator for ControlValue\n> + * \\param[in] other The ControlValue object to move from\n> + *\n> + * \\sa ControlValue::ControlValue(ControlValue &&other)\n> + */\n> +\n> /**\n> * \\fn ControlValue::type()\n> * \\brief Retrieve the data type of the value\n> --\n> 2.49.0\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 A6BD5C3335\n\tfor ;\n\tFri, 21 Nov 2025 11:48:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9D88960A9D;\n\tFri, 21 Nov 2025 12:48:01 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3F75B60805\n\tfor ;\n\tFri, 21 Nov 2025 12:48:00 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3A8AF66B;\n\tFri, 21 Nov 2025 12:45:54 +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=\"viLR7p3E\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763725554;\n\tbh=mNWnetmBOdKxtB1Ph+Yeizbd1aPV03a/VRJqqr/ADX0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=viLR7p3EVJYpBI1mwBogVyx6BQ56WqU3edIiQ8QkNgWOFrXhfX9ygwbSWEYi0Rn2H\n\tf8bhgO9xT4kjpA40S5eTcD+9leR+grbmP+b1bY59Twtv5Bg/X/KGH+R5NxmoLpUFcd\n\tScNTVG/YxtZKnh2wH76OZomMfDgDQ3aO/zbrqXuQ=","Date":"Fri, 21 Nov 2025 12:47:57 +0100","From":"Jacopo Mondi ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org, \n\tKieran Bingham ,\n\tJacopo Mondi ","Subject":"Re: [PATCH v2 3/5] libcamera: controls: Implement move\n\tctor/assignment","Message-ID":"","References":"<20250421153556.171192-1-barnabas.pocze@ideasonboard.com>\n\t<20250421153556.171192-4-barnabas.pocze@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20250421153556.171192-4-barnabas.pocze@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":36985,"web_url":"https://patchwork.libcamera.org/comment/36985/","msgid":"<70a8cb5f-68e2-4fac-a158-369f0ec1d503@ideasonboard.com>","date":"2025-11-21T12:49:45","subject":"Re: [PATCH v2 3/5] libcamera: controls: Implement move\n\tctor/assignment","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2025. 11. 21. 12:47 keltezéssel, Jacopo Mondi írta:\n> Hi Barnabás\n> \n> On Mon, Apr 21, 2025 at 05:35:54PM +0200, Barnabás Pőcze wrote:\n>> Implement a move constructor and move assignment operator for `ControlValue`.\n>> The `ControlValue` type already has an \"empty\" state that is used when\n>> creating a default constructed `ControlValue`, so have the moved-from\n>> instance return to that state after move construction/assignment.\n>>\n>> This is useful, for example, for `std::vector` as most implementations will\n>> use the copy constructor when reallocating if no nothrow move constructor\n>> is available. Having a nothrow move constructor avoids the extra copies.\n>> It is also useful when using temporaries of `ControlValue` with other\n>> containers such as `std::optional`, and it also makes `ControlInfo`\n>> \"cheaply\" move constructible/assignable.\n>>\n>> Signed-off-by: Barnabás Pőcze \n>> Reviewed-by: Kieran Bingham \n>> Reviewed-by: Jacopo Mondi \n>> ---\n>> include/libcamera/controls.h | 26 ++++++++++++++++++++++++++\n>> src/libcamera/controls.cpp | 18 ++++++++++++++++++\n>> 2 files changed, 44 insertions(+)\n>>\n>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n>> index 3f3580036..866d667c4 100644\n>> --- a/include/libcamera/controls.h\n>> +++ b/include/libcamera/controls.h\n>> @@ -14,6 +14,7 @@\n>> #include \n>> #include \n>> #include \n>> +#include \n>> #include \n>>\n>> #include \n>> @@ -165,6 +166,31 @@ public:\n>> \tControlValue(const ControlValue &other);\n>> \tControlValue &operator=(const ControlValue &other);\n>>\n>> +\tControlValue(ControlValue &&other) noexcept\n>> +\t\t: type_(other.type_),\n> \n> Is it necessary to reset the other's type to ControlTypeNone ?\n\nI think so. Since the underlying storage is moved, the previous type cannot always\nbe kept because there would no longer be storage (e.g. if it is a bigger array).\nSo setting it to none unconditionally seems the best decision to me.\n\n\n> \n>> +\t\t isArray_(std::exchange(other.isArray_, false)),\n>> +\t\t numElements_(std::exchange(other.numElements_, 0)),\n>> +\t\t storage_(std::exchange(other.storage_, {}))\n>> +\t{\n>> +\t\tother.type_ = ControlTypeNone;\n>> +\t}\n>> +\n>> +\tControlValue &operator=(ControlValue &&other) noexcept\n>> +\t{\n>> +\t\tif (this != &other) {\n>> +\t\t\trelease();\n>> +\n>> +\t\t\ttype_ = other.type_;\n>> +\t\t\tisArray_ = std::exchange(other.isArray_, false);\n>> +\t\t\tnumElements_ = std::exchange(other.numElements_, 0);\n>> +\t\t\tstorage_ = std::exchange(other.storage_, {});\n>> +\n>> +\t\t\tother.type_ = ControlTypeNone;\n>> +\t\t}\n>> +\n>> +\t\treturn *this;\n>> +\t}\n>> +\n>> \tControlType type() const { return type_; }\n>> \tbool isNone() const { return type_ == ControlTypeNone; }\n>> \tbool isArray() const { return isArray_; }\n>> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n>> index d384e1ef7..135e87613 100644\n>> --- a/src/libcamera/controls.cpp\n>> +++ b/src/libcamera/controls.cpp\n>> @@ -155,6 +155,24 @@ ControlValue &ControlValue::operator=(const ControlValue &other)\n>> \treturn *this;\n>> }\n>>\n>> +/**\n>> + * \\fn ControlValue::ControlValue(ControlValue &&other) noexcept\n>> + * \\brief Move constructor for ControlValue\n>> + * \\param[in] other The ControlValue object to move from\n>> + *\n>> + * Move constructs a ControlValue instance from \\a other.\n>> + * After this operation \\a other will be in the same state\n>> + * as a default constructed ControlValue instance.\n>> + */\n>> +\n>> +/**\n>> + * \\fn ControlValue &ControlValue::operator=(ControlValue &&other) noexcept\n>> + * \\brief Move assignment operator for ControlValue\n>> + * \\param[in] other The ControlValue object to move from\n>> + *\n>> + * \\sa ControlValue::ControlValue(ControlValue &&other)\n>> + */\n>> +\n>> /**\n>> * \\fn ControlValue::type()\n>> * \\brief Retrieve the data type of the value\n>> --\n>> 2.49.0\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 CB876C3330\n\tfor ;\n\tFri, 21 Nov 2025 12:49:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3333460A8B;\n\tFri, 21 Nov 2025 13:49: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 1B72660805\n\tfor ;\n\tFri, 21 Nov 2025 13:49:50 +0100 (CET)","from [192.168.33.39] (185.221.143.100.nat.pool.zt.hu\n\t[185.221.143.100])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A7D836A6;\n\tFri, 21 Nov 2025 13:47:43 +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=\"G7bBhXga\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1763729263;\n\tbh=jJSozehP1L7GMKCYH6/5yZtIVk5Ll46NZ+meQ6ZBuVU=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=G7bBhXgaucw75qngVJEpTGY8DEgFpYVlmHzTupvCTgKaouKxwxEUzKApSa6mmCpBC\n\tg0V9aff3arez+FsvbRwXEpr/586LwH1HJNTDjKhZPm8muDIt7r4V92SPxWZqsGcPrt\n\tsYVN7yS6vzyyrLskGgTRrnwOCuPg+hlZ96R3STYs=","Message-ID":"<70a8cb5f-68e2-4fac-a158-369f0ec1d503@ideasonboard.com>","Date":"Fri, 21 Nov 2025 13:49:45 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 3/5] libcamera: controls: Implement move\n\tctor/assignment","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org,\n\tKieran Bingham ","References":"<20250421153556.171192-1-barnabas.pocze@ideasonboard.com>\n\t<20250421153556.171192-4-barnabas.pocze@ideasonboard.com>\n\t<_skpKgArAn586p0Lx56EI7_fN2sohoiazqLDCZhPOm5pjQfTrktYes75qdHRzjg1GjqixF7UJluugxpjPNS9zw==@protonmail.internalid>\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\" "}}]