[{"id":33957,"web_url":"https://patchwork.libcamera.org/comment/33957/","msgid":"<174490404205.2882969.14956490196356724694@ping.linuxembedded.co.uk>","date":"2025-04-17T15:34:02","subject":"Re: [PATCH v1 3/3] libcamera: controls: Implement `swap()`","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2025-04-17 12:35:39)\n> Implement both free and member function `swap()` for `ControlValue`.\n> The general `std::swap()` swaps two values by combining a move construction\n> and two move assignments, but for `ControlValue` a simpler implementation\n> can be provided by just swapping the members.\n> \n> Signed-off-by: Barnabás Pőcze \n> ---\n> include/libcamera/controls.h | 24 ++++++++++++++++++++++++\n> src/libcamera/controls.cpp | 15 +++++++++++++++\n> 2 files changed, 39 insertions(+)\n> \n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index 86245e7a9..27b314dbc 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -261,6 +261,30 @@ public:\n> void reserve(ControlType type, bool isArray = false,\n> std::size_t numElements = 1);\n> \n> + void swap(ControlValue &other) noexcept\n> + {\n> + {\n> + auto tmp = type_;\n> + type_ = other.type_;\n> + other.type_ = tmp;\n> + }\n\nI fear I'm missing something obvious ... why are type_ and numElements_\nnot items that can simply be swapped with std::swap() ?\n\nIs it because of the need to use auto on the type? or something else?\n\nShould this implementation go into controls.cpp ? or is it\nrequired/better inline in the header ?\n\n> +\n> + std::swap(isArray_, other.isArray_);\n> +\n> + {\n> + auto tmp = numElements_;\n> + numElements_ = other.numElements_;\n> + other.numElements_ = tmp;\n> + }\n> +\n> + std::swap(storage_, other.storage_);\n> + }\n> +\n> + friend void swap(ControlValue &a, ControlValue &b) noexcept\n> + {\n> + a.swap(b);\n> + }\n> +\n> private:\n> ControlType type_ : 8;\n> bool isArray_;\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 885287e71..c1ff60f39 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -411,6 +411,21 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> storage_.external = new uint8_t[newSize];\n> }\n> \n> +/**\n> + * \\fn ControlValue::swap(ControlValue &other) noexcept\n> + * \\brief Swap two control values\n> + *\n> + * This function swaps the contained value of \\a this with that of \\a other.\n> + */\n> +\n> +/**\n> + * \\fn ControlValue::swap(ControlValue &a, ControlValue &b) noexcept\n> + * \\brief Swap two control values\n> + *\n> + * This function swaps the contained value of \\a a with that of \\a b.\n> + * \\sa ControlValue::swap()\n> + */\n> +\n> /**\n> * \\class ControlId\n> * \\brief Control static metadata\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 08733C3213\n\tfor ;\n\tThu, 17 Apr 2025 15:34:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4902068AC7;\n\tThu, 17 Apr 2025 17:34:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6CA5768AC0\n\tfor ;\n\tThu, 17 Apr 2025 17:34:04 +0200 (CEST)","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 8D9B1415;\n\tThu, 17 Apr 2025 17:32:00 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"bzj74uAk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1744903920;\n\tbh=OYPzfboUb/eONZNjc5smEWMY+JMm4JQM+ziG/DgcA4Q=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=bzj74uAkRFeq25o8oUNdO4jKvB90JWceq31OELT1d390QF0/lGb5vzqTKzmgJz640\n\t6wuaaAlkpnbMSoQSqiVlhCthHsfqOX3mIfImaLZHs9jCD4u7nJgnWDMR2QIwrZZ7Ba\n\t1AqlTvXWaFSZ7QJCMaAwFoKlBvreKCgVMGwnRx1I=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250417113539.1137936-3-barnabas.pocze@ideasonboard.com>","References":"<20250417113539.1137936-1-barnabas.pocze@ideasonboard.com>\n\t<20250417113539.1137936-3-barnabas.pocze@ideasonboard.com>","Subject":"Re: [PATCH v1 3/3] libcamera: controls: Implement `swap()`","From":"Kieran Bingham ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 17 Apr 2025 16:34:02 +0100","Message-ID":"<174490404205.2882969.14956490196356724694@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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":33959,"web_url":"https://patchwork.libcamera.org/comment/33959/","msgid":"<41a87985-86fc-4166-b1b9-f9cd6905b161@ideasonboard.com>","date":"2025-04-17T16:03:07","subject":"Re: [PATCH v1 3/3] libcamera: controls: Implement `swap()`","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n\n2025. 04. 17. 17:34 keltezéssel, Kieran Bingham írta:\n> Quoting Barnabás Pőcze (2025-04-17 12:35:39)\n>> Implement both free and member function `swap()` for `ControlValue`.\n>> The general `std::swap()` swaps two values by combining a move construction\n>> and two move assignments, but for `ControlValue` a simpler implementation\n>> can be provided by just swapping the members.\n>>\n>> Signed-off-by: Barnabás Pőcze \n>> ---\n>> include/libcamera/controls.h | 24 ++++++++++++++++++++++++\n>> src/libcamera/controls.cpp | 15 +++++++++++++++\n>> 2 files changed, 39 insertions(+)\n>>\n>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n>> index 86245e7a9..27b314dbc 100644\n>> --- a/include/libcamera/controls.h\n>> +++ b/include/libcamera/controls.h\n>> @@ -261,6 +261,30 @@ public:\n>> void reserve(ControlType type, bool isArray = false,\n>> std::size_t numElements = 1);\n>> \n>> + void swap(ControlValue &other) noexcept\n>> + {\n>> + {\n>> + auto tmp = type_;\n>> + type_ = other.type_;\n>> + other.type_ = tmp;\n>> + }\n> \n> I fear I'm missing something obvious ... why are type_ and numElements_\n> not items that can simply be swapped with std::swap() ?\n> \n> Is it because of the need to use auto on the type? or something else?\n\nThey are bit fields, references to bit fields cannot be taken, so\n`std::swap()` cannot be used.\n\n> \n> Should this implementation go into controls.cpp ? or is it\n> required/better inline in the header ?\n\nI would leave it in the header file, it's not too many instructions.\n\nAlthough... it is not optimal in my opinion. I think the compilers\ndon't want to merge the loads/stores, possibly because of the padding.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n>> +\n>> + std::swap(isArray_, other.isArray_);\n>> +\n>> + {\n>> + auto tmp = numElements_;\n>> + numElements_ = other.numElements_;\n>> + other.numElements_ = tmp;\n>> + }\n>> +\n>> + std::swap(storage_, other.storage_);\n>> + }\n>> +\n>> + friend void swap(ControlValue &a, ControlValue &b) noexcept\n>> + {\n>> + a.swap(b);\n>> + }\n>> +\n>> private:\n>> ControlType type_ : 8;\n>> bool isArray_;\n>> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n>> index 885287e71..c1ff60f39 100644\n>> --- a/src/libcamera/controls.cpp\n>> +++ b/src/libcamera/controls.cpp\n>> @@ -411,6 +411,21 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n>> storage_.external = new uint8_t[newSize];\n>> }\n>> \n>> +/**\n>> + * \\fn ControlValue::swap(ControlValue &other) noexcept\n>> + * \\brief Swap two control values\n>> + *\n>> + * This function swaps the contained value of \\a this with that of \\a other.\n>> + */\n>> +\n>> +/**\n>> + * \\fn ControlValue::swap(ControlValue &a, ControlValue &b) noexcept\n>> + * \\brief Swap two control values\n>> + *\n>> + * This function swaps the contained value of \\a a with that of \\a b.\n>> + * \\sa ControlValue::swap()\n>> + */\n>> +\n>> /**\n>> * \\class ControlId\n>> * \\brief Control static metadata\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 6BE16C3213\n\tfor ;\n\tThu, 17 Apr 2025 16:03:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A339068AC7;\n\tThu, 17 Apr 2025 18:03:13 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2751668AC0\n\tfor ;\n\tThu, 17 Apr 2025 18:03:12 +0200 (CEST)","from [192.168.33.16] (185.221.143.16.nat.pool.zt.hu\n\t[185.221.143.16])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EC829415;\n\tThu, 17 Apr 2025 18:01:07 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"myFwishi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1744905668;\n\tbh=LWX474nyqO0PAZB3g0pedgSchu07whhm35s2CmXlnRA=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=myFwishi4Gghbagtmlydds6QPW6sFzV+w+jU3HzfKoM01avkwOgnMGUDIEB7Gryks\n\tzkJbz6EJxc23Hpmq5YcH5ZWeuc6k4Lt5VXEhmPfZ/uattXsOQ1zJjDU0AH0QOkRnWm\n\tvMcTCtYLakBzw2c/LuCuXwMWCIMBIhSqGTnKmqLY=","Message-ID":"<41a87985-86fc-4166-b1b9-f9cd6905b161@ideasonboard.com>","Date":"Thu, 17 Apr 2025 18:03:07 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v1 3/3] libcamera: controls: Implement `swap()`","To":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20250417113539.1137936-1-barnabas.pocze@ideasonboard.com>\n\t<20250417113539.1137936-3-barnabas.pocze@ideasonboard.com>\n\t<174490404205.2882969.14956490196356724694@ping.linuxembedded.co.uk>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<174490404205.2882969.14956490196356724694@ping.linuxembedded.co.uk>","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":33960,"web_url":"https://patchwork.libcamera.org/comment/33960/","msgid":"<174493500634.891349.16446808008134542980@ping.linuxembedded.co.uk>","date":"2025-04-18T00:10:06","subject":"Re: [PATCH v1 3/3] libcamera: controls: Implement `swap()`","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2025-04-17 17:03:07)\n> Hi\n> \n> \n> 2025. 04. 17. 17:34 keltezéssel, Kieran Bingham írta:\n> > Quoting Barnabás Pőcze (2025-04-17 12:35:39)\n> >> Implement both free and member function `swap()` for `ControlValue`.\n> >> The general `std::swap()` swaps two values by combining a move construction\n> >> and two move assignments, but for `ControlValue` a simpler implementation\n> >> can be provided by just swapping the members.\n> >>\n> >> Signed-off-by: Barnabás Pőcze \n> >> ---\n> >> include/libcamera/controls.h | 24 ++++++++++++++++++++++++\n> >> src/libcamera/controls.cpp | 15 +++++++++++++++\n> >> 2 files changed, 39 insertions(+)\n> >>\n> >> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> >> index 86245e7a9..27b314dbc 100644\n> >> --- a/include/libcamera/controls.h\n> >> +++ b/include/libcamera/controls.h\n> >> @@ -261,6 +261,30 @@ public:\n> >> void reserve(ControlType type, bool isArray = false,\n> >> std::size_t numElements = 1);\n> >> \n> >> + void swap(ControlValue &other) noexcept\n> >> + {\n> >> + {\n> >> + auto tmp = type_;\n> >> + type_ = other.type_;\n> >> + other.type_ = tmp;\n> >> + }\n> > \n> > I fear I'm missing something obvious ... why are type_ and numElements_\n> > not items that can simply be swapped with std::swap() ?\n> > \n> > Is it because of the need to use auto on the type? or something else?\n> \n> They are bit fields, references to bit fields cannot be taken, so\n> `std::swap()` cannot be used.\n> \n> > \n> > Should this implementation go into controls.cpp ? or is it\n> > required/better inline in the header ?\n> \n> I would leave it in the header file, it's not too many instructions.\n> \n> Although... it is not optimal in my opinion. I think the compilers\n> don't want to merge the loads/stores, possibly because of the padding.\n> \n\nThis won't be on a performance hotpath though will it ? So I don't think\na couple of extra operations here will be too upsetting.\n\nAnyway, if it's clear that std::swap can't be used directly - perhaps a\nshort oneline comment could describe that to stop someone coming in to\noptimise it into a swap call ... but otherwise:\n\nReviewed-by: Kieran Bingham \n\n> \n> Regards,\n> Barnabás Pőcze\n> \n> > \n> >> +\n> >> + std::swap(isArray_, other.isArray_);\n> >> +\n> >> + {\n> >> + auto tmp = numElements_;\n> >> + numElements_ = other.numElements_;\n> >> + other.numElements_ = tmp;\n> >> + }\n> >> +\n> >> + std::swap(storage_, other.storage_);\n> >> + }\n> >> +\n> >> + friend void swap(ControlValue &a, ControlValue &b) noexcept\n> >> + {\n> >> + a.swap(b);\n> >> + }\n> >> +\n> >> private:\n> >> ControlType type_ : 8;\n> >> bool isArray_;\n> >> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> >> index 885287e71..c1ff60f39 100644\n> >> --- a/src/libcamera/controls.cpp\n> >> +++ b/src/libcamera/controls.cpp\n> >> @@ -411,6 +411,21 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> >> storage_.external = new uint8_t[newSize];\n> >> }\n> >> \n> >> +/**\n> >> + * \\fn ControlValue::swap(ControlValue &other) noexcept\n> >> + * \\brief Swap two control values\n> >> + *\n> >> + * This function swaps the contained value of \\a this with that of \\a other.\n> >> + */\n> >> +\n> >> +/**\n> >> + * \\fn ControlValue::swap(ControlValue &a, ControlValue &b) noexcept\n> >> + * \\brief Swap two control values\n> >> + *\n> >> + * This function swaps the contained value of \\a a with that of \\a b.\n> >> + * \\sa ControlValue::swap()\n> >> + */\n> >> +\n> >> /**\n> >> * \\class ControlId\n> >> * \\brief Control static metadata\n> >> -- \n> >> 2.49.0\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 BEE2CC327D\n\tfor ;\n\tFri, 18 Apr 2025 00:10:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DBDCB68AC7;\n\tFri, 18 Apr 2025 02:10:11 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 56780617E6\n\tfor ;\n\tFri, 18 Apr 2025 02:10:09 +0200 (CEST)","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 E042F8D4;\n\tFri, 18 Apr 2025 02:08:04 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"NsW9Gkxx\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1744934885;\n\tbh=dujyD9auxWF3laF/BuWE8AkziujX5ficzrMKlCxJMZE=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=NsW9GkxxDHAqYcPu0CuXY8rNSL14CUAlFUPk+ObGt1i7x9ndiGG5VMF7+OL8WCQJy\n\t/p32k3tffTKQY8ZRJ+sBgzkY6IAGfql0Rp+wrTiDwTmzEIwQGAYWtclLLnUG4xg3/P\n\tWKNO+GP1CbSjxCp/QPgQCvLQCNptAjQDjPtDMDp8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<41a87985-86fc-4166-b1b9-f9cd6905b161@ideasonboard.com>","References":"<20250417113539.1137936-1-barnabas.pocze@ideasonboard.com>\n\t<20250417113539.1137936-3-barnabas.pocze@ideasonboard.com>\n\t<174490404205.2882969.14956490196356724694@ping.linuxembedded.co.uk>\n\t<41a87985-86fc-4166-b1b9-f9cd6905b161@ideasonboard.com>","Subject":"Re: [PATCH v1 3/3] libcamera: controls: Implement `swap()`","From":"Kieran Bingham ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 18 Apr 2025 01:10:06 +0100","Message-ID":"<174493500634.891349.16446808008134542980@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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":33967,"web_url":"https://patchwork.libcamera.org/comment/33967/","msgid":"<2vadpigrespy5d4jn3c47iewcj26ip4z5zbohdcd4ufzi7wmkm@7rxhdsp75aac>","date":"2025-04-18T10:35:28","subject":"Re: [PATCH v1 3/3] libcamera: controls: Implement `swap()`","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"On Fri, Apr 18, 2025 at 01:10:06AM +0100, Kieran Bingham wrote:\n> Quoting Barnabás Pőcze (2025-04-17 17:03:07)\n> > Hi\n> >\n> >\n> > 2025. 04. 17. 17:34 keltezéssel, Kieran Bingham írta:\n> > > Quoting Barnabás Pőcze (2025-04-17 12:35:39)\n> > >> Implement both free and member function `swap()` for `ControlValue`.\n> > >> The general `std::swap()` swaps two values by combining a move construction\n> > >> and two move assignments, but for `ControlValue` a simpler implementation\n> > >> can be provided by just swapping the members.\n> > >>\n> > >> Signed-off-by: Barnabás Pőcze \n> > >> ---\n> > >> include/libcamera/controls.h | 24 ++++++++++++++++++++++++\n> > >> src/libcamera/controls.cpp | 15 +++++++++++++++\n> > >> 2 files changed, 39 insertions(+)\n> > >>\n> > >> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > >> index 86245e7a9..27b314dbc 100644\n> > >> --- a/include/libcamera/controls.h\n> > >> +++ b/include/libcamera/controls.h\n> > >> @@ -261,6 +261,30 @@ public:\n> > >> void reserve(ControlType type, bool isArray = false,\n> > >> std::size_t numElements = 1);\n> > >>\n> > >> + void swap(ControlValue &other) noexcept\n> > >> + {\n> > >> + {\n> > >> + auto tmp = type_;\n> > >> + type_ = other.type_;\n> > >> + other.type_ = tmp;\n> > >> + }\n> > >\n> > > I fear I'm missing something obvious ... why are type_ and numElements_\n> > > not items that can simply be swapped with std::swap() ?\n> > >\n> > > Is it because of the need to use auto on the type? or something else?\n> >\n> > They are bit fields, references to bit fields cannot be taken, so\n> > `std::swap()` cannot be used.\n> >\n> > >\n> > > Should this implementation go into controls.cpp ? or is it\n> > > required/better inline in the header ?\n> >\n> > I would leave it in the header file, it's not too many instructions.\n> >\n> > Although... it is not optimal in my opinion. I think the compilers\n> > don't want to merge the loads/stores, possibly because of the padding.\n> >\n>\n> This won't be on a performance hotpath though will it ? So I don't think\n> a couple of extra operations here will be too upsetting.\n>\n> Anyway, if it's clear that std::swap can't be used directly - perhaps a\n> short oneline comment could describe that to stop someone coming in to\n> optimise it into a swap call ... but otherwise:\n>\n> Reviewed-by: Kieran Bingham \n>\n\nWith the above suggestions\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> >\n> > Regards,\n> > Barnabás Pőcze\n> >\n> > >\n> > >> +\n> > >> + std::swap(isArray_, other.isArray_);\n> > >> +\n> > >> + {\n> > >> + auto tmp = numElements_;\n> > >> + numElements_ = other.numElements_;\n> > >> + other.numElements_ = tmp;\n> > >> + }\n> > >> +\n> > >> + std::swap(storage_, other.storage_);\n> > >> + }\n> > >> +\n> > >> + friend void swap(ControlValue &a, ControlValue &b) noexcept\n> > >> + {\n> > >> + a.swap(b);\n> > >> + }\n> > >> +\n> > >> private:\n> > >> ControlType type_ : 8;\n> > >> bool isArray_;\n> > >> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > >> index 885287e71..c1ff60f39 100644\n> > >> --- a/src/libcamera/controls.cpp\n> > >> +++ b/src/libcamera/controls.cpp\n> > >> @@ -411,6 +411,21 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> > >> storage_.external = new uint8_t[newSize];\n> > >> }\n> > >>\n> > >> +/**\n> > >> + * \\fn ControlValue::swap(ControlValue &other) noexcept\n> > >> + * \\brief Swap two control values\n> > >> + *\n> > >> + * This function swaps the contained value of \\a this with that of \\a other.\n> > >> + */\n> > >> +\n> > >> +/**\n> > >> + * \\fn ControlValue::swap(ControlValue &a, ControlValue &b) noexcept\n> > >> + * \\brief Swap two control values\n> > >> + *\n> > >> + * This function swaps the contained value of \\a a with that of \\a b.\n> > >> + * \\sa ControlValue::swap()\n> > >> + */\n> > >> +\n> > >> /**\n> > >> * \\class ControlId\n> > >> * \\brief Control static metadata\n> > >> --\n> > >> 2.49.0\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 35F51C327D\n\tfor ;\n\tFri, 18 Apr 2025 10:35:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D18E768ABB;\n\tFri, 18 Apr 2025 12:35:32 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E8B0D617E8\n\tfor ;\n\tFri, 18 Apr 2025 12:35:30 +0200 (CEST)","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 740FAF6;\n\tFri, 18 Apr 2025 12:33:26 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"mGLTqkM6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1744972406;\n\tbh=gRnEV0Y3rr1/CzFHUNi7NRjRiICCgn35pUHK/WUf5o8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mGLTqkM6l1cLpyG0BEe1VpTh17kjkyqKZgmnxDG17f0NKvmSezSksck/CHvR9+qLt\n\t55UxtMuSL3rjQzOugVp2c/MlYbiTs2lQ3yc9dx625km5YmwF9u0+AXOgccxLKn41iu\n\toga/4qB37VEv0Dv+TT4kkEbZhC14evAwcDOxpLUI=","Date":"Fri, 18 Apr 2025 12:35:28 +0200","From":"Jacopo Mondi ","To":"Kieran Bingham ","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1 3/3] libcamera: controls: Implement `swap()`","Message-ID":"<2vadpigrespy5d4jn3c47iewcj26ip4z5zbohdcd4ufzi7wmkm@7rxhdsp75aac>","References":"<20250417113539.1137936-1-barnabas.pocze@ideasonboard.com>\n\t<20250417113539.1137936-3-barnabas.pocze@ideasonboard.com>\n\t<174490404205.2882969.14956490196356724694@ping.linuxembedded.co.uk>\n\t<41a87985-86fc-4166-b1b9-f9cd6905b161@ideasonboard.com>\n\t<174493500634.891349.16446808008134542980@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<174493500634.891349.16446808008134542980@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\" "}}]