[{"id":22431,"web_url":"https://patchwork.libcamera.org/comment/22431/","msgid":"<20220324113630.GA3036343@pyrite.rasen.tech>","date":"2022-03-24T11:36:30","subject":"Re: [libcamera-devel] [PATCH] implement vector casts for Point,\n\tSize and Rectangle","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Hello Christian,\n\nThanks for the patch.\n\nEasy things first: missing Signed-off-by tag.\n\nOn Tue, Mar 22, 2022 at 08:57:08PM +0000, Christian Rauch via libcamera-devel wrote:\n> Cast operators from Point, Size and Rectangle to std::vector<int> allows to\n> serialise those types without dedicated conversion functions.\n\nIPADataSerializer converts to and from vectors of uint8_t, so this alone\nisn't sufficient for replacing the specialized serializers, because:\n- it's not plumbed into the IPADataSerializer\n- it only casts to a vector of int\n\n\nLet's look at the first issue first: it's not plumbed into the\nIPADataSerializer.\n\nHow would you use this std::vector<int> cast operator? If you really\nwant to avoid the specialized IPADataSerializers for these geometry\nstructs, then you'd have to use one of the existing IPADataSerializers.\n\nI presume you're imagining using IPADataSerializer<std::vector<V>>.\nIn this case, that means that when the pipeline handler and IPA send\ndata to each other, they'll be the ones that have to cast the geometry\nstructs (ie. *do the serialization*). This is not very nice. Imagine if\na pipeline handler had to do:\n\nipa_->configure(static_cast<std::vector<int>>(size));\n\ninstead of the simpler:\n\nipa_->configure(size);\n\nNot to mention the receiver then has to deserialize it.\n\nThat defeats the whole purpose of the IPA interface definitions and the\nIPADataSerializer. We might as well go back to the days where pipeline\nhandlers and IPAs had to manually serialize and deserialize everything.\n\nAnother problem with using IPADataSerializer<std::vector<V>> is that we\nwould be serializing/deserializing the vector twice, since it would\nfirst be Point -> std::vector<int> (via the cast operator), and then\nstd::vector<int> -> std::vector<uint8_t> (via\nIPADataSerializer<std::vector<V>>), which breaks every int into a vector\nof uint8_t.\n\nSo by avoiding a specialized IPADataSerializer you'd also be increasing\nthe serialization cost of the geometry types. The specialized\nIPADataSerializers are auto-generated anyway, not sure why you'd want to\nreplace them in this way.\n\n\nAnd now the second issue: it only casts to a vector of int.\n\nIf this patch defined a cast operator to std::vector<uint8_t>, then I\nwould say that that doesn't belong here. If Point/Size/Rectangle expose\na cast operator to std::vector<uint8_t>, that means it's a feature that\nwe support for applications to use, which is not something that we want\n(note that this is in the public API). A cast to std::vector<uint8_t>\nfor serialization purposes is clearly a job for the serializer.\n\nOn the other hand if the cast operator to std::vector<uint8_t> was just\n{ x, y }, then it's simply incorrect, as you're obviously losing 24 bits\nof precision.\n\nAlso, Size and Rectangle have unsigned ints, but you're casting them to\nints. If the purpose is for serialization, then as mentioned earlier, it\ndoesn't belong here. If the purpose is not for serialization, then...\nwhat is the purpose?\n\n\nThanks,\n\nPaul\n\n> ---\n>  include/libcamera/geometry.h | 15 +++++++++++++++\n>  1 file changed, 15 insertions(+)\n> \n> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> index 7838b679..7d1e403a 100644\n> --- a/include/libcamera/geometry.h\n> +++ b/include/libcamera/geometry.h\n> @@ -38,6 +38,11 @@ public:\n>  \t{\n>  \t\treturn { -x, -y };\n>  \t}\n> +\n> +\toperator std::vector<int>() const\n> +\t{\n> +\t\treturn { x, y };\n> +\t}\n>  };\n> \n>  bool operator==(const Point &lhs, const Point &rhs);\n> @@ -167,6 +172,11 @@ public:\n> \n>  \tSize &operator*=(float factor);\n>  \tSize &operator/=(float factor);\n> +\n> +\toperator std::vector<int>() const\n> +\t{\n> +\t\treturn { static_cast<int>(width), static_cast<int>(height) };\n> +\t}\n>  };\n> \n>  bool operator==(const Size &lhs, const Size &rhs);\n> @@ -283,6 +293,11 @@ public:\n>  \t__nodiscard Rectangle scaledBy(const Size &numerator,\n>  \t\t\t\t       const Size &denominator) const;\n>  \t__nodiscard Rectangle translatedBy(const Point &point) const;\n> +\n> +\toperator std::vector<int>() const\n> +\t{\n> +\t\treturn { x, y, static_cast<int>(width), static_cast<int>(height) };\n> +\t}\n>  };\n> \n>  bool operator==(const Rectangle &lhs, const Rectangle &rhs);\n> --\n> 2.25.1\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 E2B08C0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 24 Mar 2022 11:36:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 22106604D5;\n\tThu, 24 Mar 2022 12:36:40 +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 73DE060397\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 24 Mar 2022 12:36:38 +0100 (CET)","from pyrite.rasen.tech (h175-177-042-148.catv02.itscom.jp\n\t[175.177.42.148])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 148D01844;\n\tThu, 24 Mar 2022 12:36:36 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648121800;\n\tbh=uC1IwKF6hkEfOAnbfRWLP62ePxa0OXk6f1UAZRsvn4c=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=LXmcQuYJRMVo5i8EHlGJrTIGZYmgPQrQMWKIz5E5KdQvNIYqbaWEEcIqXyUEbuNR/\n\tmWgLTrJCaeSMy59fTXOxvVvgHfPBaDELIoQkKMT8n5SJ6t+48szj0zcWUmn8fVw21T\n\tgkxKAUvyPd/r/MvLC56S1A00K7xLKzjIfz/3lwsKQR/Yjg7EbyAQAdN0yby3wBpbGB\n\trEd1IfP+AujZhUQ9fWnucGcXOk85HffOXVbIIlQ3r1frdyWQUBwiEARtsfjhVofEDK\n\tjMZkQFFDL6prxDu+9wtADHtu/KUBjTCjg4DOwHg9dJ6VmuguGpHlPj5Dc4nnjrQr9b\n\tyf6KiOLpTyWQw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1648121798;\n\tbh=uC1IwKF6hkEfOAnbfRWLP62ePxa0OXk6f1UAZRsvn4c=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=QRKPU4HOHkDMSw5fo4sCfOmBsW3CWIKqYbyslCTVDUWWUzByaesClTuhJP6nLcFQY\n\tprp6IR0krE4PNLOEXrINvHqrjusevC/iESsKMjcxrm4wVSz4X5Cru8Fsd7akbdJC+s\n\tn41Gmz3Iqm5jBU7NoUMHoo/it6spqmqAwR+WVt+M="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"QRKPU4HO\"; dkim-atps=neutral","Date":"Thu, 24 Mar 2022 20:36:30 +0900","To":"Christian Rauch <Rauch.Christian@gmx.de>","Message-ID":"<20220324113630.GA3036343@pyrite.rasen.tech>","References":"<20220322205708.734021-1-Rauch.Christian@gmx.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20220322205708.734021-1-Rauch.Christian@gmx.de>","Subject":"Re: [libcamera-devel] [PATCH] implement vector casts for Point,\n\tSize and Rectangle","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Paul Elder via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"paul.elder@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22444,"web_url":"https://patchwork.libcamera.org/comment/22444/","msgid":"<16e3b9e5-8bc9-aef9-cc35-82397400a63f@gmx.de>","date":"2022-03-24T22:41:50","subject":"Re: [libcamera-devel] [PATCH] implement vector casts for Point,\n\tSize and Rectangle","submitter":{"id":111,"url":"https://patchwork.libcamera.org/api/people/111/","name":"Christian Rauch","email":"Rauch.Christian@gmx.de"},"content":"Hello Paul,\n\nTo clarify, with \"serialisation\" I did not mean to convert those types\ninto a byte stream, but rather \"flatten\" those structures, so they can\nbe represented as a list of values.\n\nI did not intend to use the IPADataSerializer and I actually was not\naware of this. The intention with the vector<int> cast was to represent\nthose libcamera specific types in the form of standard types for the\npublic API and not for internal use.\n\nBest,\nChristian\n\n\nAm 24.03.22 um 11:36 schrieb paul.elder@ideasonboard.com:\n> Hello Christian,\n>\n> Thanks for the patch.\n>\n> Easy things first: missing Signed-off-by tag.\n>\n> On Tue, Mar 22, 2022 at 08:57:08PM +0000, Christian Rauch via libcamera-devel wrote:\n>> Cast operators from Point, Size and Rectangle to std::vector<int> allows to\n>> serialise those types without dedicated conversion functions.\n>\n> IPADataSerializer converts to and from vectors of uint8_t, so this alone\n> isn't sufficient for replacing the specialized serializers, because:\n> - it's not plumbed into the IPADataSerializer\n> - it only casts to a vector of int\n>\n>\n> Let's look at the first issue first: it's not plumbed into the\n> IPADataSerializer.\n>\n> How would you use this std::vector<int> cast operator? If you really\n> want to avoid the specialized IPADataSerializers for these geometry\n> structs, then you'd have to use one of the existing IPADataSerializers.\n>\n> I presume you're imagining using IPADataSerializer<std::vector<V>>.\n> In this case, that means that when the pipeline handler and IPA send\n> data to each other, they'll be the ones that have to cast the geometry\n> structs (ie. *do the serialization*). This is not very nice. Imagine if\n> a pipeline handler had to do:\n>\n> ipa_->configure(static_cast<std::vector<int>>(size));\n>\n> instead of the simpler:\n>\n> ipa_->configure(size);\n>\n> Not to mention the receiver then has to deserialize it.\n>\n> That defeats the whole purpose of the IPA interface definitions and the\n> IPADataSerializer. We might as well go back to the days where pipeline\n> handlers and IPAs had to manually serialize and deserialize everything.\n>\n> Another problem with using IPADataSerializer<std::vector<V>> is that we\n> would be serializing/deserializing the vector twice, since it would\n> first be Point -> std::vector<int> (via the cast operator), and then\n> std::vector<int> -> std::vector<uint8_t> (via\n> IPADataSerializer<std::vector<V>>), which breaks every int into a vector\n> of uint8_t.\n>\n> So by avoiding a specialized IPADataSerializer you'd also be increasing\n> the serialization cost of the geometry types. The specialized\n> IPADataSerializers are auto-generated anyway, not sure why you'd want to\n> replace them in this way.\n>\n>\n> And now the second issue: it only casts to a vector of int.\n>\n> If this patch defined a cast operator to std::vector<uint8_t>, then I\n> would say that that doesn't belong here. If Point/Size/Rectangle expose\n> a cast operator to std::vector<uint8_t>, that means it's a feature that\n> we support for applications to use, which is not something that we want\n> (note that this is in the public API). A cast to std::vector<uint8_t>\n> for serialization purposes is clearly a job for the serializer.\n>\n> On the other hand if the cast operator to std::vector<uint8_t> was just\n> { x, y }, then it's simply incorrect, as you're obviously losing 24 bits\n> of precision.\n>\n> Also, Size and Rectangle have unsigned ints, but you're casting them to\n> ints. If the purpose is for serialization, then as mentioned earlier, it\n> doesn't belong here. If the purpose is not for serialization, then...\n> what is the purpose?\n>\n>\n> Thanks,\n>\n> Paul\n>\n>> ---\n>>  include/libcamera/geometry.h | 15 +++++++++++++++\n>>  1 file changed, 15 insertions(+)\n>>\n>> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n>> index 7838b679..7d1e403a 100644\n>> --- a/include/libcamera/geometry.h\n>> +++ b/include/libcamera/geometry.h\n>> @@ -38,6 +38,11 @@ public:\n>>  \t{\n>>  \t\treturn { -x, -y };\n>>  \t}\n>> +\n>> +\toperator std::vector<int>() const\n>> +\t{\n>> +\t\treturn { x, y };\n>> +\t}\n>>  };\n>>\n>>  bool operator==(const Point &lhs, const Point &rhs);\n>> @@ -167,6 +172,11 @@ public:\n>>\n>>  \tSize &operator*=(float factor);\n>>  \tSize &operator/=(float factor);\n>> +\n>> +\toperator std::vector<int>() const\n>> +\t{\n>> +\t\treturn { static_cast<int>(width), static_cast<int>(height) };\n>> +\t}\n>>  };\n>>\n>>  bool operator==(const Size &lhs, const Size &rhs);\n>> @@ -283,6 +293,11 @@ public:\n>>  \t__nodiscard Rectangle scaledBy(const Size &numerator,\n>>  \t\t\t\t       const Size &denominator) const;\n>>  \t__nodiscard Rectangle translatedBy(const Point &point) const;\n>> +\n>> +\toperator std::vector<int>() const\n>> +\t{\n>> +\t\treturn { x, y, static_cast<int>(width), static_cast<int>(height) };\n>> +\t}\n>>  };\n>>\n>>  bool operator==(const Rectangle &lhs, const Rectangle &rhs);\n>> --\n>> 2.25.1\n>>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 025ABC0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 24 Mar 2022 22:41:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 16C14604D5;\n\tThu, 24 Mar 2022 23:41:53 +0100 (CET)","from mout.gmx.net (mout.gmx.net [212.227.15.15])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2558460397\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 24 Mar 2022 23:41:52 +0100 (CET)","from [192.168.1.209] ([92.10.251.63]) by mail.gmx.net (mrgmx004\n\t[212.227.17.190]) with ESMTPSA (Nemesis) id 1MbirE-1o3q1h0sqU-00dGmM;\n\tThu, 24 Mar 2022 23:41:51 +0100"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648161713;\n\tbh=AIORWie/8YwdTWHtWTetGCmC3OnJ766pygL3+npy8SQ=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=yXFktVMb59BPVjJKPuDLjRv5LsnZafFBfPywLxoFK+7CEXPaDR0uHPJP/th+eTfHj\n\tn18r/TD5eWxsVxYOmlgAniasFAb/3lL684UfObXseaLARhEnvr0gJQdRFwkrm852Cv\n\tKEwaCGpnJMFVqnZQYfRMqLpsiaMyWrjwWiY0QTQbM3ijQV7QfIUte5nr7kAEallE2G\n\tPAGFCYwFEMJStk71WxcxjphBGMA9PpsOg76Iuo5sb+EgRLS+8BEK+R7N7uMJCfzy6R\n\tfO7OZ3GNH6alz/uyFix0A8jYTxsjRtvvxitu8Lwwe7bovaTFnPwqgsNjIaqAjmL4Vx\n\tNBGVZQHH0R7xA==","v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net;\n\ts=badeba3b8450; t=1648161711;\n\tbh=AIORWie/8YwdTWHtWTetGCmC3OnJ766pygL3+npy8SQ=;\n\th=X-UI-Sender-Class:Date:Subject:To:Cc:References:From:In-Reply-To;\n\tb=kkdCBQ5NLiVnPyQldRPGyaGF1bMTZyPhsrFfgRDZc/Sj5xSS17ALfDIPcmCvJnUBi\n\tRjpomjf2FIyqjvg6RfX7f0UKHDyP3C6APNBG4lp3ONSLRzdpPeIx7y7mq5kSd1nE4t\n\tEVTSKc2CUzXcs9P6i/P4mHfOV8mkk59SOgqI4L5U="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=gmx.net header.i=@gmx.net\n\theader.b=\"kkdCBQ5N\"; dkim-atps=neutral","X-UI-Sender-Class":"01bb95c1-4bf8-414a-932a-4f6e2808ef9c","Message-ID":"<16e3b9e5-8bc9-aef9-cc35-82397400a63f@gmx.de>","Date":"Thu, 24 Mar 2022 22:41:50 +0000","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.7.0","Content-Language":"en-GB","To":"paul.elder@ideasonboard.com","References":"<20220322205708.734021-1-Rauch.Christian@gmx.de>\n\t<20220324113630.GA3036343@pyrite.rasen.tech>","In-Reply-To":"<20220324113630.GA3036343@pyrite.rasen.tech>","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"quoted-printable","X-Provags-ID":"V03:K1:EMcMNKEBUZyjs7xfURLzeKgONREHFhODPlVzyDR1/iP0pnygDG3\n\t6Ku6d8n3lmVyyoRg0edXACL/QSdjaDcSWzXeqpHO/p+qzvNd6wi048RTwfmZE4zG5flLF3p\n\t/7Q7y9xSJbs1h5Jm5CabqvPFBqvADqtAOB9b9HYSVtudx07MZIVkJzqSLWT1RvC9RNG/G7c\n\tSoIMXUxMmoK9QcgZ2In+g==","X-Spam-Flag":"NO","X-UI-Out-Filterresults":"notjunk:1; V03:K0:Xmdm57actQI=:guIQ2VsM6y3VO1ileshpuy\n\tnL2U6PwXw5JMKvH/kaYOrSnLFcOzSb3cuF1tUXAVEbOGyBjWtTVQoBeO6swog51oy30c6G74r\n\t1YLdJazqyGZPrTUz9z8SXtMZhspRE49c0Tk1s6R+vJ5Y8YgT/O1vyhdEFLo+1AyF4w/OLkWg4\n\tHsas43PmNmPNEi5Vq0uk7qNrk3MIm7K7atRCEW9AJKqPlRovgZBM5NwK0S7hZ+/u72As4LV3y\n\ttFtXkJZDWxaEoLldRyLp8PpvzzWkhUqcJ7f/o9BxPkd1U9TJqJlI+ufHmK/clmXHPWWhp1rYW\n\tMVQp1a8ROSxyTco0Lpds6VYPu6J+isdAnWVYLLiArjk6QYr2VtJzNGr1FpCL+ctNlXuAUfFXf\n\tRPg+XzFsDd3C4wMaJSbJXgve5KnevAFD2+T7zGD3KPYBR4z9fXYBaPJtEVNZjsjTpxBa9jbra\n\tahcBuzKdRQMJNj6b+lLBbzhKDM6zXzKwsw4vNG+zMlAlC6PH1wEL3mp0NZIsJ/5p8o6u/EKKi\n\tlMa3IQqHhPFnfSUlOF+tkFjnfIO/mkcy6Jj+pi1GkDcUxRlzX9BuHsWf2cqSaH9I/MICC8Cuf\n\txQ0DItp/W44HbRyynXvktsR8625rZLZiyocW006Idl1TWK8jpSnUeln3lzmVrajGFGbY4P74J\n\tWilvg7gn/U8+H2LonkxpG6FSmpwARtll0FAXhh81S4YKizMIyrldpHkLy7mHjdNWbMTqG70ne\n\t3yCBZnQWiXgz8n+Ens2lgROkPJ3D5ykwtxLDqCIJnUypFWlFi2B8FXVyUb6mS2HB9rqnAWHU9\n\tgpdvj5T7ZBB1KBMItCPJzamvaczz5TRfhXHKZeOABEK5Lb7dqIdFTZ8jUeoXFzeupADzNH8Xw\n\t1D3024+8J4AIk79tF5YfDXJM7y0CTWC0tT0a+XiUkbuRAMy+LY9UkJqACiFWkSh2nLXHCYNKZ\n\t0hVwGUDriscuw48exq22kpXdQ/5qJrgP8+UY1Q/oYQRYUMYVxrqSNiG/OEpkgvXLMOjk2Ayyh\n\t1/1X57b30E6UDIsN+Bpc7Eau6QvwErd6R7GtOImROWkhCd66TJCGLLeWaydD8QlHiRqG3Kdgn\n\tWDpZkDrRkyg5Fk=","Subject":"Re: [libcamera-devel] [PATCH] implement vector casts for Point,\n\tSize and Rectangle","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Christian Rauch via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Christian Rauch <Rauch.Christian@gmx.de>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22449,"web_url":"https://patchwork.libcamera.org/comment/22449/","msgid":"<20220325102241.GD3036343@pyrite.rasen.tech>","date":"2022-03-25T10:22:41","subject":"Re: [libcamera-devel] [PATCH] implement vector casts for Point,\n\tSize and Rectangle","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Hi Christian,\n\nOn Thu, Mar 24, 2022 at 10:41:50PM +0000, Christian Rauch wrote:\n> Hello Paul,\n> \n> To clarify, with \"serialisation\" I did not mean to convert those types\n> into a byte stream, but rather \"flatten\" those structures, so they can\n> be represented as a list of values.\n> \n> I did not intend to use the IPADataSerializer and I actually was not\n> aware of this. The intention with the vector<int> cast was to represent\n\nAh, I see. I got confused since that's that only place that I know of\nwhere serialization actually takes place.\n\n> those libcamera specific types in the form of standard types for the\n> public API and not for internal use.\n\nI'm still not sure I see the use case of wanting to use a vector of ints\nover dedicated geometry types (which come with nice helpers btw),\nthough. Especially when unsigned ints are cast to signed ints.\n\n\nPaul\n\n> \n> \n> Am 24.03.22 um 11:36 schrieb paul.elder@ideasonboard.com:\n> > Hello Christian,\n> >\n> > Thanks for the patch.\n> >\n> > Easy things first: missing Signed-off-by tag.\n> >\n> > On Tue, Mar 22, 2022 at 08:57:08PM +0000, Christian Rauch via libcamera-devel wrote:\n> >> Cast operators from Point, Size and Rectangle to std::vector<int> allows to\n> >> serialise those types without dedicated conversion functions.\n> >\n> > IPADataSerializer converts to and from vectors of uint8_t, so this alone\n> > isn't sufficient for replacing the specialized serializers, because:\n> > - it's not plumbed into the IPADataSerializer\n> > - it only casts to a vector of int\n> >\n> >\n> > Let's look at the first issue first: it's not plumbed into the\n> > IPADataSerializer.\n> >\n> > How would you use this std::vector<int> cast operator? If you really\n> > want to avoid the specialized IPADataSerializers for these geometry\n> > structs, then you'd have to use one of the existing IPADataSerializers.\n> >\n> > I presume you're imagining using IPADataSerializer<std::vector<V>>.\n> > In this case, that means that when the pipeline handler and IPA send\n> > data to each other, they'll be the ones that have to cast the geometry\n> > structs (ie. *do the serialization*). This is not very nice. Imagine if\n> > a pipeline handler had to do:\n> >\n> > ipa_->configure(static_cast<std::vector<int>>(size));\n> >\n> > instead of the simpler:\n> >\n> > ipa_->configure(size);\n> >\n> > Not to mention the receiver then has to deserialize it.\n> >\n> > That defeats the whole purpose of the IPA interface definitions and the\n> > IPADataSerializer. We might as well go back to the days where pipeline\n> > handlers and IPAs had to manually serialize and deserialize everything.\n> >\n> > Another problem with using IPADataSerializer<std::vector<V>> is that we\n> > would be serializing/deserializing the vector twice, since it would\n> > first be Point -> std::vector<int> (via the cast operator), and then\n> > std::vector<int> -> std::vector<uint8_t> (via\n> > IPADataSerializer<std::vector<V>>), which breaks every int into a vector\n> > of uint8_t.\n> >\n> > So by avoiding a specialized IPADataSerializer you'd also be increasing\n> > the serialization cost of the geometry types. The specialized\n> > IPADataSerializers are auto-generated anyway, not sure why you'd want to\n> > replace them in this way.\n> >\n> >\n> > And now the second issue: it only casts to a vector of int.\n> >\n> > If this patch defined a cast operator to std::vector<uint8_t>, then I\n> > would say that that doesn't belong here. If Point/Size/Rectangle expose\n> > a cast operator to std::vector<uint8_t>, that means it's a feature that\n> > we support for applications to use, which is not something that we want\n> > (note that this is in the public API). A cast to std::vector<uint8_t>\n> > for serialization purposes is clearly a job for the serializer.\n> >\n> > On the other hand if the cast operator to std::vector<uint8_t> was just\n> > { x, y }, then it's simply incorrect, as you're obviously losing 24 bits\n> > of precision.\n> >\n> > Also, Size and Rectangle have unsigned ints, but you're casting them to\n> > ints. If the purpose is for serialization, then as mentioned earlier, it\n> > doesn't belong here. If the purpose is not for serialization, then...\n> > what is the purpose?\n> >\n> >\n> > Thanks,\n> >\n> > Paul\n> >\n> >> ---\n> >>  include/libcamera/geometry.h | 15 +++++++++++++++\n> >>  1 file changed, 15 insertions(+)\n> >>\n> >> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> >> index 7838b679..7d1e403a 100644\n> >> --- a/include/libcamera/geometry.h\n> >> +++ b/include/libcamera/geometry.h\n> >> @@ -38,6 +38,11 @@ public:\n> >>  \t{\n> >>  \t\treturn { -x, -y };\n> >>  \t}\n> >> +\n> >> +\toperator std::vector<int>() const\n> >> +\t{\n> >> +\t\treturn { x, y };\n> >> +\t}\n> >>  };\n> >>\n> >>  bool operator==(const Point &lhs, const Point &rhs);\n> >> @@ -167,6 +172,11 @@ public:\n> >>\n> >>  \tSize &operator*=(float factor);\n> >>  \tSize &operator/=(float factor);\n> >> +\n> >> +\toperator std::vector<int>() const\n> >> +\t{\n> >> +\t\treturn { static_cast<int>(width), static_cast<int>(height) };\n> >> +\t}\n> >>  };\n> >>\n> >>  bool operator==(const Size &lhs, const Size &rhs);\n> >> @@ -283,6 +293,11 @@ public:\n> >>  \t__nodiscard Rectangle scaledBy(const Size &numerator,\n> >>  \t\t\t\t       const Size &denominator) const;\n> >>  \t__nodiscard Rectangle translatedBy(const Point &point) const;\n> >> +\n> >> +\toperator std::vector<int>() const\n> >> +\t{\n> >> +\t\treturn { x, y, static_cast<int>(width), static_cast<int>(height) };\n> >> +\t}\n> >>  };\n> >>\n> >>  bool operator==(const Rectangle &lhs, const Rectangle &rhs);\n> >> --\n> >> 2.25.1\n> >>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 609B8BD80A\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 25 Mar 2022 10:22:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A4610604C5;\n\tFri, 25 Mar 2022 11:22:50 +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 67FBB60136\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 25 Mar 2022 11:22:48 +0100 (CET)","from pyrite.rasen.tech (h175-177-042-148.catv02.itscom.jp\n\t[175.177.42.148])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1F6946F3;\n\tFri, 25 Mar 2022 11:22:46 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648203770;\n\tbh=mnVzvs+iHJi2txY+ALla1JfCsLPLQPyzkttDeCMCj2g=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=JdeuVG0ZazkZu4czTI/0kgUthQl3uQQknIe9yagltpxZW4VvKtkcX4iAJW9NLuG3+\n\tDLm3wkFkAu6SCIE1gVkiPpr/HM3EFF3NE+ZO8Jq3xSmiEojBNvg1TSLmAlcyMw7QJf\n\t1iKDh7tYfhgv8vi7ppFbcysuEWFhUxoC9OVB239J+n69iNvaXlHlIC6mhdN2l4kAc4\n\tY6w98l/L1T5BQGXqfQX3Lovb7n+k3XQgvzR1bvTkbds43kO2Y8DdlTfJzFADc8A2R+\n\teZSuSLuoWmMmV3keoof6Md2fhApTkoKeJDPiv0xBW+RHv5qU3/uKJrwHJ32TGLWtAF\n\tdumKyGRIZyL1w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1648203768;\n\tbh=mnVzvs+iHJi2txY+ALla1JfCsLPLQPyzkttDeCMCj2g=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nL5d+GfdRHFsCTcWyMLshhe4HSCsOxwTl5VQTeQjx79nuJyd0FeGNaJUCScl39qPJ\n\t/ymJjMct7F75MlNbm/CeZu5Q5Od+xCP8E3uyKi1amE+CQtiOLddc4IEDmwRbqT1HAQ\n\tQUe5Awu5ZMgtqwAj4ZWS5T7UNjQ2jBztuqt8OzuU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"nL5d+Gfd\"; dkim-atps=neutral","Date":"Fri, 25 Mar 2022 19:22:41 +0900","To":"Christian Rauch <Rauch.Christian@gmx.de>","Message-ID":"<20220325102241.GD3036343@pyrite.rasen.tech>","References":"<20220322205708.734021-1-Rauch.Christian@gmx.de>\n\t<20220324113630.GA3036343@pyrite.rasen.tech>\n\t<16e3b9e5-8bc9-aef9-cc35-82397400a63f@gmx.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<16e3b9e5-8bc9-aef9-cc35-82397400a63f@gmx.de>","Subject":"Re: [libcamera-devel] [PATCH] implement vector casts for Point,\n\tSize and Rectangle","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Paul Elder via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"paul.elder@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22452,"web_url":"https://patchwork.libcamera.org/comment/22452/","msgid":"<6344c0a8-dd8c-19dc-c424-e0727d09675c@gmx.de>","date":"2022-03-26T02:15:17","subject":"Re: [libcamera-devel] [PATCH] implement vector casts for Point,\n\tSize and Rectangle","submitter":{"id":111,"url":"https://patchwork.libcamera.org/api/people/111/","name":"Christian Rauch","email":"Rauch.Christian@gmx.de"},"content":"Hi Paul,\n\nAm 25.03.22 um 10:22 schrieb paul.elder@ideasonboard.com:\n> Hi Christian,\n>\n> On Thu, Mar 24, 2022 at 10:41:50PM +0000, Christian Rauch wrote:\n>> Hello Paul,\n>>\n>> To clarify, with \"serialisation\" I did not mean to convert those types\n>> into a byte stream, but rather \"flatten\" those structures, so they can\n>> be represented as a list of values.\n>>\n>> I did not intend to use the IPADataSerializer and I actually was not\n>> aware of this. The intention with the vector<int> cast was to represent\n>\n> Ah, I see. I got confused since that's that only place that I know of\n> where serialization actually takes place.\n>\n>> those libcamera specific types in the form of standard types for the\n>> public API and not for internal use.\n>\n> I'm still not sure I see the use case of wanting to use a vector of ints\n> over dedicated geometry types (which come with nice helpers btw),\n> though. Especially when unsigned ints are cast to signed ints.\n\nThe use-case is that those data types can then be represented by\nprimitive data structures (like arrays). I want to use those geometry\ntypes in another framework as configurable parameter, but this only\nsupports plain old data types incl. strings, and arrays thereof.\n\nBest,\nChristian\n\n>\n>\n> Paul\n>\n>>\n>>\n>> Am 24.03.22 um 11:36 schrieb paul.elder@ideasonboard.com:\n>>> Hello Christian,\n>>>\n>>> Thanks for the patch.\n>>>\n>>> Easy things first: missing Signed-off-by tag.\n>>>\n>>> On Tue, Mar 22, 2022 at 08:57:08PM +0000, Christian Rauch via libcamera-devel wrote:\n>>>> Cast operators from Point, Size and Rectangle to std::vector<int> allows to\n>>>> serialise those types without dedicated conversion functions.\n>>>\n>>> IPADataSerializer converts to and from vectors of uint8_t, so this alone\n>>> isn't sufficient for replacing the specialized serializers, because:\n>>> - it's not plumbed into the IPADataSerializer\n>>> - it only casts to a vector of int\n>>>\n>>>\n>>> Let's look at the first issue first: it's not plumbed into the\n>>> IPADataSerializer.\n>>>\n>>> How would you use this std::vector<int> cast operator? If you really\n>>> want to avoid the specialized IPADataSerializers for these geometry\n>>> structs, then you'd have to use one of the existing IPADataSerializers.\n>>>\n>>> I presume you're imagining using IPADataSerializer<std::vector<V>>.\n>>> In this case, that means that when the pipeline handler and IPA send\n>>> data to each other, they'll be the ones that have to cast the geometry\n>>> structs (ie. *do the serialization*). This is not very nice. Imagine if\n>>> a pipeline handler had to do:\n>>>\n>>> ipa_->configure(static_cast<std::vector<int>>(size));\n>>>\n>>> instead of the simpler:\n>>>\n>>> ipa_->configure(size);\n>>>\n>>> Not to mention the receiver then has to deserialize it.\n>>>\n>>> That defeats the whole purpose of the IPA interface definitions and the\n>>> IPADataSerializer. We might as well go back to the days where pipeline\n>>> handlers and IPAs had to manually serialize and deserialize everything.\n>>>\n>>> Another problem with using IPADataSerializer<std::vector<V>> is that we\n>>> would be serializing/deserializing the vector twice, since it would\n>>> first be Point -> std::vector<int> (via the cast operator), and then\n>>> std::vector<int> -> std::vector<uint8_t> (via\n>>> IPADataSerializer<std::vector<V>>), which breaks every int into a vector\n>>> of uint8_t.\n>>>\n>>> So by avoiding a specialized IPADataSerializer you'd also be increasing\n>>> the serialization cost of the geometry types. The specialized\n>>> IPADataSerializers are auto-generated anyway, not sure why you'd want to\n>>> replace them in this way.\n>>>\n>>>\n>>> And now the second issue: it only casts to a vector of int.\n>>>\n>>> If this patch defined a cast operator to std::vector<uint8_t>, then I\n>>> would say that that doesn't belong here. If Point/Size/Rectangle expose\n>>> a cast operator to std::vector<uint8_t>, that means it's a feature that\n>>> we support for applications to use, which is not something that we want\n>>> (note that this is in the public API). A cast to std::vector<uint8_t>\n>>> for serialization purposes is clearly a job for the serializer.\n>>>\n>>> On the other hand if the cast operator to std::vector<uint8_t> was just\n>>> { x, y }, then it's simply incorrect, as you're obviously losing 24 bits\n>>> of precision.\n>>>\n>>> Also, Size and Rectangle have unsigned ints, but you're casting them to\n>>> ints. If the purpose is for serialization, then as mentioned earlier, it\n>>> doesn't belong here. If the purpose is not for serialization, then...\n>>> what is the purpose?\n>>>\n>>>\n>>> Thanks,\n>>>\n>>> Paul\n>>>\n>>>> ---\n>>>>  include/libcamera/geometry.h | 15 +++++++++++++++\n>>>>  1 file changed, 15 insertions(+)\n>>>>\n>>>> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n>>>> index 7838b679..7d1e403a 100644\n>>>> --- a/include/libcamera/geometry.h\n>>>> +++ b/include/libcamera/geometry.h\n>>>> @@ -38,6 +38,11 @@ public:\n>>>>  \t{\n>>>>  \t\treturn { -x, -y };\n>>>>  \t}\n>>>> +\n>>>> +\toperator std::vector<int>() const\n>>>> +\t{\n>>>> +\t\treturn { x, y };\n>>>> +\t}\n>>>>  };\n>>>>\n>>>>  bool operator==(const Point &lhs, const Point &rhs);\n>>>> @@ -167,6 +172,11 @@ public:\n>>>>\n>>>>  \tSize &operator*=(float factor);\n>>>>  \tSize &operator/=(float factor);\n>>>> +\n>>>> +\toperator std::vector<int>() const\n>>>> +\t{\n>>>> +\t\treturn { static_cast<int>(width), static_cast<int>(height) };\n>>>> +\t}\n>>>>  };\n>>>>\n>>>>  bool operator==(const Size &lhs, const Size &rhs);\n>>>> @@ -283,6 +293,11 @@ public:\n>>>>  \t__nodiscard Rectangle scaledBy(const Size &numerator,\n>>>>  \t\t\t\t       const Size &denominator) const;\n>>>>  \t__nodiscard Rectangle translatedBy(const Point &point) const;\n>>>> +\n>>>> +\toperator std::vector<int>() const\n>>>> +\t{\n>>>> +\t\treturn { x, y, static_cast<int>(width), static_cast<int>(height) };\n>>>> +\t}\n>>>>  };\n>>>>\n>>>>  bool operator==(const Rectangle &lhs, const Rectangle &rhs);\n>>>> --\n>>>> 2.25.1\n>>>>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 C72E1C0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 26 Mar 2022 02:15:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 02668604DA;\n\tSat, 26 Mar 2022 03:15:22 +0100 (CET)","from mout.gmx.net (mout.gmx.net [212.227.15.18])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D207060397\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 26 Mar 2022 03:15:19 +0100 (CET)","from [192.168.1.209] ([92.10.251.63]) by mail.gmx.net (mrgmx005\n\t[212.227.17.190]) with ESMTPSA (Nemesis) id 1M6lpM-1nPfYX0xTi-008GuY;\n\tSat, 26 Mar 2022 03:15:19 +0100"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648260922;\n\tbh=nCtZKQDpF1XoCE9Wmlcx8Pj+SqTQ8nrD0SIcmq3WOCw=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=fTUASnF8XuDueNADSCmRviWmI8BDGCU5U6OV2qiZo8q85HMnlb2iG+LPzQhLket+p\n\ti6AZDrD67Teo2jezbMmnS+kHCDRIZ1N/HnwPolmrmZ9OfvebVQOgdglLj4nSGodi/e\n\tnYvFJz3yqBjYTGTvvPrmJt1fzLkYMeBP2GyZsoWkOK79YY7jOrTV+0ZhgoNSiMjTID\n\tJHnRVOY+E/3oOCiDoz3EPVA53eT9VppVamh/iKs+iSOgdy33xMGgd2ht5s5HDLCVum\n\tm8/J/uVn+0ubZ0vjDp2zOI+SUN1+vbRb2RnOzdOpgWqZ5aD1ehJ3Jom9lzvVhJ75uN\n\tGa9hrd0+WvQCA==","v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net;\n\ts=badeba3b8450; t=1648260919;\n\tbh=nCtZKQDpF1XoCE9Wmlcx8Pj+SqTQ8nrD0SIcmq3WOCw=;\n\th=X-UI-Sender-Class:Date:Subject:To:Cc:References:From:In-Reply-To;\n\tb=HkNJk2hk6Ahzj2GthAdquscK1f8hyf7TNMXk823UWTXBmEKsKhuOLsxujdwXIjTQX\n\thYGrjJBbJL2uRWo5hOeokZKmkqxV7vvMJCpi3tg1DwiPy6Kuhn+X/GED8VLww0AEfk\n\tsN3HTFneN4/DO8KNi/UZsRL4hHhLDHZJ4sW7AE+E="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=gmx.net header.i=@gmx.net\n\theader.b=\"HkNJk2hk\"; dkim-atps=neutral","X-UI-Sender-Class":"01bb95c1-4bf8-414a-932a-4f6e2808ef9c","Message-ID":"<6344c0a8-dd8c-19dc-c424-e0727d09675c@gmx.de>","Date":"Sat, 26 Mar 2022 02:15:17 +0000","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.7.0","Content-Language":"en-GB","To":"paul.elder@ideasonboard.com","References":"<20220322205708.734021-1-Rauch.Christian@gmx.de>\n\t<20220324113630.GA3036343@pyrite.rasen.tech>\n\t<16e3b9e5-8bc9-aef9-cc35-82397400a63f@gmx.de>\n\t<20220325102241.GD3036343@pyrite.rasen.tech>","In-Reply-To":"<20220325102241.GD3036343@pyrite.rasen.tech>","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"quoted-printable","X-Provags-ID":"V03:K1:lxC9QnrXBkFPvMPpJnR6OI5vUVsaPD+9pdAaSJjCIitCCyWE7Li\n\ts4mgoGYaCNFydAqxBPSKkdzdpcQM1DcQLb4rS+OxCpfb3M4CHHrRn4rAmZODgKIa3aauC9P\n\t25KBKryd+LKEdO9wOKK6dBtyEyrs+9qlZlKjDuOshqKK7rp5pfhSMua0TfHIte02w2zqFgU\n\tZIBIBEu9udxERQYjE24cw==","X-Spam-Flag":"NO","X-UI-Out-Filterresults":"notjunk:1; V03:K0:V9SjJMp4DYc=:GoP8MS6dhpnL/oiXioBzRZ\n\thI3lPX+wccTVubNhNX2xFdM9jwaMF7jjkWEig0qdWHqNHHrW/d7Bo7GP2vY2pT8WUwD3ahN13\n\tLcmy3C9St65+PKQfOftkh3MPQyUB8rB6jCGPI8+OBgMxmlsOxPfyW08wbUYb8M95rmoCHQ+0m\n\tQnbSLJqvY2qXWQxMOyuU5bAgFxlqwkGEVe/zcfYnTDLhC+rIvCAQPdI0WR/IyjtZQtluyTUqM\n\tqTtCo9/CVu6dBCXAdnZJ8T55Zzm9Ym6CkBRDjmms/K88uCAuEu5zAsfKyto+EctRMFzNZVEmy\n\tGW5zdk9GGdr0wLDxfKnmG/wWhWUf9r/C9TghgivO8ej0nGbUQn74EStrQRtLgzWpwuf55Hxew\n\t+WmMPB48ad7hV+AH8sV35j7msN0f2ydhuI/0hvlC972UlPiF40pEvc1OEFgtaPPrFtuMoqKlv\n\tQAetJcA56+P7Zm6mrmWCtgGpmzBlo92OSOeX5eIfanv37/ijQzEW4NbwHYsZg2DY0KfxqIfC8\n\tPUHdAGQATqrj7R8wurNic85OPtIRBHPKJVki7ILjbBmGR8kJl7N2bNu6HqnzBu8L28+TTRc9M\n\td34kLThWjikuRWRVYKZh71tCi4mH2DzNPx76yR8oaobMicDdPqxfVh7napU+a4HidPOdEGbNJ\n\tUAC/e6b+4IC4nZpFzlJSKJTrAAKE1bVsoktOa0pB2pqqxcnxOc0RXEKkkQICgzNvgd3oibZXw\n\tv/AkhLqjGuX7qjbRsusxIAYbDxU2/FWkR84ojqWvNhAvxJqLpqr/LNuyXNn2rbgK0GtANLlap\n\tMy4jxYZgE5HhASnMWAr1eCsSj89rL8k9nXMaKFO/mstAnkBIdivniOK3IN1paOHslmgT9Y/2g\n\ti1s+i64vXaqCf7DO9cRXS1F7QUI0J83k6v5pIjqicJiUn70naiiBs5lq93FkT8nGiSbjfrPI6\n\tJTPRb8lFPyeyTet6TABquXX6H1KRZUr3XOLPWscwdTneXk1s4Y73SJ1QJYtLMWrW9lVglGKnw\n\tunsweDUXKKgTGfqlknh6uxJwX/+OC5498YFWkHbSLPRi9SxM7yYrwe7BI/4MDL+Ufw03AREON\n\t15j83yas26noRk=","Subject":"Re: [libcamera-devel] [PATCH] implement vector casts for Point,\n\tSize and Rectangle","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Christian Rauch via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Christian Rauch <Rauch.Christian@gmx.de>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22477,"web_url":"https://patchwork.libcamera.org/comment/22477/","msgid":"<20220328074458.m7bf5nuhj3la6xuy@uno.localdomain>","date":"2022-03-28T07:44:58","subject":"Re: [libcamera-devel] [PATCH] implement vector casts for Point,\n\tSize and Rectangle","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Chris\n\nOn Sat, Mar 26, 2022 at 02:15:17AM +0000, Christian Rauch via libcamera-devel wrote:\n> Hi Paul,\n>\n> Am 25.03.22 um 10:22 schrieb paul.elder@ideasonboard.com:\n> > Hi Christian,\n> >\n> > On Thu, Mar 24, 2022 at 10:41:50PM +0000, Christian Rauch wrote:\n> >> Hello Paul,\n> >>\n> >> To clarify, with \"serialisation\" I did not mean to convert those types\n> >> into a byte stream, but rather \"flatten\" those structures, so they can\n> >> be represented as a list of values.\n> >>\n> >> I did not intend to use the IPADataSerializer and I actually was not\n> >> aware of this. The intention with the vector<int> cast was to represent\n> >\n> > Ah, I see. I got confused since that's that only place that I know of\n> > where serialization actually takes place.\n> >\n> >> those libcamera specific types in the form of standard types for the\n> >> public API and not for internal use.\n> >\n> > I'm still not sure I see the use case of wanting to use a vector of ints\n> > over dedicated geometry types (which come with nice helpers btw),\n> > though. Especially when unsigned ints are cast to signed ints.\n>\n> The use-case is that those data types can then be represented by\n> primitive data structures (like arrays). I want to use those geometry\n> types in another framework as configurable parameter, but this only\n> supports plain old data types incl. strings, and arrays thereof.\n\nTo be honest I'm not opposed to this, even if it provides a tempting\nshortcut for applications to bypass usage of proper types.\n\nA few comments on the patch below\n\n>\n> Best,\n> Christian\n>\n> >\n> >\n> > Paul\n> >\n> >>\n> >>\n> >> Am 24.03.22 um 11:36 schrieb paul.elder@ideasonboard.com:\n> >>> Hello Christian,\n> >>>\n> >>> Thanks for the patch.\n> >>>\n> >>> Easy things first: missing Signed-off-by tag.\n> >>>\n> >>> On Tue, Mar 22, 2022 at 08:57:08PM +0000, Christian Rauch via libcamera-devel wrote:\n> >>>> Cast operators from Point, Size and Rectangle to std::vector<int> allows to\n> >>>> serialise those types without dedicated conversion functions.\n> >>>\n> >>> IPADataSerializer converts to and from vectors of uint8_t, so this alone\n> >>> isn't sufficient for replacing the specialized serializers, because:\n> >>> - it's not plumbed into the IPADataSerializer\n> >>> - it only casts to a vector of int\n> >>>\n> >>>\n> >>> Let's look at the first issue first: it's not plumbed into the\n> >>> IPADataSerializer.\n> >>>\n> >>> How would you use this std::vector<int> cast operator? If you really\n> >>> want to avoid the specialized IPADataSerializers for these geometry\n> >>> structs, then you'd have to use one of the existing IPADataSerializers.\n> >>>\n> >>> I presume you're imagining using IPADataSerializer<std::vector<V>>.\n> >>> In this case, that means that when the pipeline handler and IPA send\n> >>> data to each other, they'll be the ones that have to cast the geometry\n> >>> structs (ie. *do the serialization*). This is not very nice. Imagine if\n> >>> a pipeline handler had to do:\n> >>>\n> >>> ipa_->configure(static_cast<std::vector<int>>(size));\n> >>>\n> >>> instead of the simpler:\n> >>>\n> >>> ipa_->configure(size);\n> >>>\n> >>> Not to mention the receiver then has to deserialize it.\n> >>>\n> >>> That defeats the whole purpose of the IPA interface definitions and the\n> >>> IPADataSerializer. We might as well go back to the days where pipeline\n> >>> handlers and IPAs had to manually serialize and deserialize everything.\n> >>>\n> >>> Another problem with using IPADataSerializer<std::vector<V>> is that we\n> >>> would be serializing/deserializing the vector twice, since it would\n> >>> first be Point -> std::vector<int> (via the cast operator), and then\n> >>> std::vector<int> -> std::vector<uint8_t> (via\n> >>> IPADataSerializer<std::vector<V>>), which breaks every int into a vector\n> >>> of uint8_t.\n> >>>\n> >>> So by avoiding a specialized IPADataSerializer you'd also be increasing\n> >>> the serialization cost of the geometry types. The specialized\n> >>> IPADataSerializers are auto-generated anyway, not sure why you'd want to\n> >>> replace them in this way.\n> >>>\n> >>>\n> >>> And now the second issue: it only casts to a vector of int.\n> >>>\n> >>> If this patch defined a cast operator to std::vector<uint8_t>, then I\n> >>> would say that that doesn't belong here. If Point/Size/Rectangle expose\n> >>> a cast operator to std::vector<uint8_t>, that means it's a feature that\n> >>> we support for applications to use, which is not something that we want\n> >>> (note that this is in the public API). A cast to std::vector<uint8_t>\n> >>> for serialization purposes is clearly a job for the serializer.\n> >>>\n> >>> On the other hand if the cast operator to std::vector<uint8_t> was just\n> >>> { x, y }, then it's simply incorrect, as you're obviously losing 24 bits\n> >>> of precision.\n> >>>\n> >>> Also, Size and Rectangle have unsigned ints, but you're casting them to\n> >>> ints. If the purpose is for serialization, then as mentioned earlier, it\n> >>> doesn't belong here. If the purpose is not for serialization, then...\n> >>> what is the purpose?\n> >>>\n> >>>\n> >>> Thanks,\n> >>>\n> >>> Paul\n> >>>\n> >>>> ---\n> >>>>  include/libcamera/geometry.h | 15 +++++++++++++++\n> >>>>  1 file changed, 15 insertions(+)\n> >>>>\n> >>>> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> >>>> index 7838b679..7d1e403a 100644\n> >>>> --- a/include/libcamera/geometry.h\n> >>>> +++ b/include/libcamera/geometry.h\n> >>>> @@ -38,6 +38,11 @@ public:\n> >>>>  \t{\n> >>>>  \t\treturn { -x, -y };\n> >>>>  \t}\n\nThe first question I have is if we want to allow implicit conversion\n\n        Point p;\n        vector<int> a = p;\n\nI would prefer not and require users to go through explicit\nconversion.\n\n> >>>> +\n> >>>> +\toperator std::vector<int>() const\n> >>>> +\t{\n> >>>> +\t\treturn { x, y };\n> >>>> +\t}\n\nThere is also one question about the operator semantic: the returned\nvector will contain copies of x and y, hence:\n\n\n        Point p(5, 6);\n        std::vector<int> v = tmp;\n        v[0] = 7;\n        cout << p.x();\n\n        -> 5\n\nFrom a conversion operator I would expect to be able to get a\nreference to the object content, instead of copies. Maybe a dedicated\noperator is better suited ?\n\n\n> >>>>  };\n> >>>>\n> >>>>  bool operator==(const Point &lhs, const Point &rhs);\n> >>>> @@ -167,6 +172,11 @@ public:\n> >>>>\n> >>>>  \tSize &operator*=(float factor);\n> >>>>  \tSize &operator/=(float factor);\n> >>>> +\n> >>>> +\toperator std::vector<int>() const\n> >>>> +\t{\n> >>>> +\t\treturn { static_cast<int>(width), static_cast<int>(height) };\n> >>>> +\t}\n\nWhy cast to int ?\n\n> >>>>  };\n> >>>>\n> >>>>  bool operator==(const Size &lhs, const Size &rhs);\n> >>>> @@ -283,6 +293,11 @@ public:\n> >>>>  \t__nodiscard Rectangle scaledBy(const Size &numerator,\n> >>>>  \t\t\t\t       const Size &denominator) const;\n> >>>>  \t__nodiscard Rectangle translatedBy(const Point &point) const;\n> >>>> +\n> >>>> +\toperator std::vector<int>() const\n> >>>> +\t{\n> >>>> +\t\treturn { x, y, static_cast<int>(width), static_cast<int>(height) };\n> >>>> +\t}\n\nSame question about cast.\n\nAlso, you're missing documentation. Please have a look to geometry.cpp\nand provide documentations in complete form. As an example\n\n        /**\n         * \\fn Size::alignDownTo(unsigned int hAlignment, unsigned int vAlignment)\n         * \\brief Align the size down horizontally and vertically in place\n         * \\param[in] hAlignment Horizontal alignment\n         * \\param[in] vAlignment Vertical alignment\n         *\n         * This functions rounds the width and height down to the nearest multiple of\n         * \\a hAlignment and \\a vAlignment respectively.\n         *\n         * \\return A reference to this object\n         */\n\nWe can help refine the text if necessary\n\nThanks\n  j\n\n> >>>>  };\n> >>>>\n> >>>>  bool operator==(const Rectangle &lhs, const Rectangle &rhs);\n> >>>> --\n> >>>> 2.25.1\n> >>>>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 97BAFC3256\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 28 Mar 2022 07:45:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E7EFC65631;\n\tMon, 28 Mar 2022 09:45:02 +0200 (CEST)","from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::222])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CACD7604BF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 28 Mar 2022 09:45:00 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 2234A40012;\n\tMon, 28 Mar 2022 07:44:59 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648453502;\n\tbh=z/Ezwxv3GYC5kRe3icm0/9F06SaMWDvPF6phOo54K0I=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=bCByVji0gDZb60h40YUPWxtusT7lcbAATfHEwD4j9U+Qa+k9HunGnM9mmFvhbCl9P\n\tWnZRb2LqQSBKUWMESCAJ0Y+yb0ZWnv/Y4y6T2Fu7+9gkWC/MIVYA89RMBAeOgde4Sk\n\tIEWVdZUwJg1wd3wyS11qxKHi7NAgjgTWKo46f/duZp2kakeGW/rIJUBHU5tzQshQt+\n\tvXtR3tbNxMb96phNvjlvPhKtbLCBOlefUiTkh44L/UtGTp3VkSUYJ7Heu5gi+njo07\n\tKPSr8TEcvaRR/UBI44r3bVLkPkWzrHjgLA3BqD5aQRrtlQUbch/bb3Hc/hxoacyUH2\n\tIcWaPcHcM7Wwg==","Date":"Mon, 28 Mar 2022 09:44:58 +0200","To":"Christian Rauch <Rauch.Christian@gmx.de>","Message-ID":"<20220328074458.m7bf5nuhj3la6xuy@uno.localdomain>","References":"<20220322205708.734021-1-Rauch.Christian@gmx.de>\n\t<20220324113630.GA3036343@pyrite.rasen.tech>\n\t<16e3b9e5-8bc9-aef9-cc35-82397400a63f@gmx.de>\n\t<20220325102241.GD3036343@pyrite.rasen.tech>\n\t<6344c0a8-dd8c-19dc-c424-e0727d09675c@gmx.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<6344c0a8-dd8c-19dc-c424-e0727d09675c@gmx.de>","Subject":"Re: [libcamera-devel] [PATCH] implement vector casts for Point,\n\tSize and Rectangle","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Jacopo Mondi via libcamera-devel <libcamera-devel@lists.libcamera.org>","Reply-To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22519,"web_url":"https://patchwork.libcamera.org/comment/22519/","msgid":"<YkOQy1w8aAmNeyxN@pendragon.ideasonboard.com>","date":"2022-03-29T23:05:47","subject":"Re: [libcamera-devel] [PATCH] implement vector casts for Point,\n\tSize and Rectangle","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Christian,\n\nOn Sat, Mar 26, 2022 at 02:15:17AM +0000, Christian Rauch via libcamera-devel wrote:\n> Am 25.03.22 um 10:22 schrieb paul.elder@ideasonboard.com:\n> > On Thu, Mar 24, 2022 at 10:41:50PM +0000, Christian Rauch wrote:\n> >> Hello Paul,\n> >>\n> >> To clarify, with \"serialisation\" I did not mean to convert those types\n> >> into a byte stream, but rather \"flatten\" those structures, so they can\n> >> be represented as a list of values.\n> >>\n> >> I did not intend to use the IPADataSerializer and I actually was not\n> >> aware of this. The intention with the vector<int> cast was to represent\n> >\n> > Ah, I see. I got confused since that's that only place that I know of\n> > where serialization actually takes place.\n> >\n> >> those libcamera specific types in the form of standard types for the\n> >> public API and not for internal use.\n> >\n> > I'm still not sure I see the use case of wanting to use a vector of ints\n> > over dedicated geometry types (which come with nice helpers btw),\n> > though. Especially when unsigned ints are cast to signed ints.\n> \n> The use-case is that those data types can then be represented by\n> primitive data structures (like arrays). I want to use those geometry\n> types in another framework as configurable parameter, but this only\n> supports plain old data types incl. strings, and arrays thereof.\n\nI'm a bit reluctant to go this way, for two reasons.\n\nFirst, such an implicit cast will reduce type safety. It will be easy,\nfor instance, to mistakenly use a Point instead of a Size, without the\ncompiler warning you.\n\nThe second reason is that it encodes in the libcamera API assumptions\nmade by other frameworks. This is particularly true for the Rectangle\nclass. In libcamera we present it as { x, y, w, h }, but the alternative\n{ x1, y1, x2, y2 } representation is also commonly used. If the vector\ncast operator is used for compatibility with other APIs, I don't think\nwe should pick one representation over the other.\n\nFor those reasons, I'm tempted to say that conversion to other\nrepresentations for these data types don't belong to the libcamera\npublic API.\n\n> >> Am 24.03.22 um 11:36 schrieb paul.elder@ideasonboard.com:\n> >>> Hello Christian,\n> >>>\n> >>> Thanks for the patch.\n> >>>\n> >>> Easy things first: missing Signed-off-by tag.\n> >>>\n> >>> On Tue, Mar 22, 2022 at 08:57:08PM +0000, Christian Rauch via libcamera-devel wrote:\n> >>>> Cast operators from Point, Size and Rectangle to std::vector<int> allows to\n> >>>> serialise those types without dedicated conversion functions.\n> >>>\n> >>> IPADataSerializer converts to and from vectors of uint8_t, so this alone\n> >>> isn't sufficient for replacing the specialized serializers, because:\n> >>> - it's not plumbed into the IPADataSerializer\n> >>> - it only casts to a vector of int\n> >>>\n> >>>\n> >>> Let's look at the first issue first: it's not plumbed into the\n> >>> IPADataSerializer.\n> >>>\n> >>> How would you use this std::vector<int> cast operator? If you really\n> >>> want to avoid the specialized IPADataSerializers for these geometry\n> >>> structs, then you'd have to use one of the existing IPADataSerializers.\n> >>>\n> >>> I presume you're imagining using IPADataSerializer<std::vector<V>>.\n> >>> In this case, that means that when the pipeline handler and IPA send\n> >>> data to each other, they'll be the ones that have to cast the geometry\n> >>> structs (ie. *do the serialization*). This is not very nice. Imagine if\n> >>> a pipeline handler had to do:\n> >>>\n> >>> ipa_->configure(static_cast<std::vector<int>>(size));\n> >>>\n> >>> instead of the simpler:\n> >>>\n> >>> ipa_->configure(size);\n> >>>\n> >>> Not to mention the receiver then has to deserialize it.\n> >>>\n> >>> That defeats the whole purpose of the IPA interface definitions and the\n> >>> IPADataSerializer. We might as well go back to the days where pipeline\n> >>> handlers and IPAs had to manually serialize and deserialize everything.\n> >>>\n> >>> Another problem with using IPADataSerializer<std::vector<V>> is that we\n> >>> would be serializing/deserializing the vector twice, since it would\n> >>> first be Point -> std::vector<int> (via the cast operator), and then\n> >>> std::vector<int> -> std::vector<uint8_t> (via\n> >>> IPADataSerializer<std::vector<V>>), which breaks every int into a vector\n> >>> of uint8_t.\n> >>>\n> >>> So by avoiding a specialized IPADataSerializer you'd also be increasing\n> >>> the serialization cost of the geometry types. The specialized\n> >>> IPADataSerializers are auto-generated anyway, not sure why you'd want to\n> >>> replace them in this way.\n> >>>\n> >>>\n> >>> And now the second issue: it only casts to a vector of int.\n> >>>\n> >>> If this patch defined a cast operator to std::vector<uint8_t>, then I\n> >>> would say that that doesn't belong here. If Point/Size/Rectangle expose\n> >>> a cast operator to std::vector<uint8_t>, that means it's a feature that\n> >>> we support for applications to use, which is not something that we want\n> >>> (note that this is in the public API). A cast to std::vector<uint8_t>\n> >>> for serialization purposes is clearly a job for the serializer.\n> >>>\n> >>> On the other hand if the cast operator to std::vector<uint8_t> was just\n> >>> { x, y }, then it's simply incorrect, as you're obviously losing 24 bits\n> >>> of precision.\n> >>>\n> >>> Also, Size and Rectangle have unsigned ints, but you're casting them to\n> >>> ints. If the purpose is for serialization, then as mentioned earlier, it\n> >>> doesn't belong here. If the purpose is not for serialization, then...\n> >>> what is the purpose?\n> >>>\n> >>>\n> >>> Thanks,\n> >>>\n> >>> Paul\n> >>>\n> >>>> ---\n> >>>>  include/libcamera/geometry.h | 15 +++++++++++++++\n> >>>>  1 file changed, 15 insertions(+)\n> >>>>\n> >>>> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h\n> >>>> index 7838b679..7d1e403a 100644\n> >>>> --- a/include/libcamera/geometry.h\n> >>>> +++ b/include/libcamera/geometry.h\n> >>>> @@ -38,6 +38,11 @@ public:\n> >>>>  \t{\n> >>>>  \t\treturn { -x, -y };\n> >>>>  \t}\n> >>>> +\n> >>>> +\toperator std::vector<int>() const\n> >>>> +\t{\n> >>>> +\t\treturn { x, y };\n> >>>> +\t}\n> >>>>  };\n> >>>>\n> >>>>  bool operator==(const Point &lhs, const Point &rhs);\n> >>>> @@ -167,6 +172,11 @@ public:\n> >>>>\n> >>>>  \tSize &operator*=(float factor);\n> >>>>  \tSize &operator/=(float factor);\n> >>>> +\n> >>>> +\toperator std::vector<int>() const\n> >>>> +\t{\n> >>>> +\t\treturn { static_cast<int>(width), static_cast<int>(height) };\n> >>>> +\t}\n> >>>>  };\n> >>>>\n> >>>>  bool operator==(const Size &lhs, const Size &rhs);\n> >>>> @@ -283,6 +293,11 @@ public:\n> >>>>  \t__nodiscard Rectangle scaledBy(const Size &numerator,\n> >>>>  \t\t\t\t       const Size &denominator) const;\n> >>>>  \t__nodiscard Rectangle translatedBy(const Point &point) const;\n> >>>> +\n> >>>> +\toperator std::vector<int>() const\n> >>>> +\t{\n> >>>> +\t\treturn { x, y, static_cast<int>(width), static_cast<int>(height) };\n> >>>> +\t}\n> >>>>  };\n> >>>>\n> >>>>  bool operator==(const Rectangle &lhs, const Rectangle &rhs);","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 04EA9C0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 29 Mar 2022 23:05:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 23A1265636;\n\tWed, 30 Mar 2022 01:05:53 +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 7D2D4604BF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 30 Mar 2022 01:05:51 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C20812F7;\n\tWed, 30 Mar 2022 01:05:50 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648595153;\n\tbh=ZwGbdZND6I274XkjiY1HIXKmpbuE71JAkcW7PX1lXBQ=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=QBb5/1dHhRuZXbwTz5S1S2VrexwK6+HsZl98RZ4a7DQS4mtBosu/YvQqxMEd0FB4m\n\tAOIDb93i1KZvxchcRwtJEAqhxr+VrFtInpOFDabmxOgiByuVRSvD53l15S4WVRe+xq\n\tx1jumLegAORuvtowmX9mOR4XuvG2rdM5At6vy6DBVlmHYxAiFhyBbDmZdgrQFoQfkF\n\tFHbyAkkp2OiAEvPIFe/2n64WLibssuohXKKfh+mfrqiwdbHZPX5zPKF6WGG+j/XXKr\n\tUmMHOy47hqTb5s6tojlNTq0TrURVL956vx8/GKHm6jUQQ2r8w3cm4J5S66MwGomufj\n\t+Cj02uOtoMwvQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1648595151;\n\tbh=ZwGbdZND6I274XkjiY1HIXKmpbuE71JAkcW7PX1lXBQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=TAOgUsCNS3Me3FMGmgYm4VzyUGcPrsEo5QknLVDqYo7tKPptY4U789sNUE8b0t6XY\n\t4pszzFMW2UfqyrHqBnAQYq12I/ThUd9QR2uZIHEdUiMpjXYKlhGS6xgp+FeSzcJe46\n\tPEW7vhez9h/1Ny+AW26BdY+e8/aobsRkMz11fmT8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"TAOgUsCN\"; dkim-atps=neutral","Date":"Wed, 30 Mar 2022 02:05:47 +0300","To":"Christian Rauch <Rauch.Christian@gmx.de>","Message-ID":"<YkOQy1w8aAmNeyxN@pendragon.ideasonboard.com>","References":"<20220322205708.734021-1-Rauch.Christian@gmx.de>\n\t<20220324113630.GA3036343@pyrite.rasen.tech>\n\t<16e3b9e5-8bc9-aef9-cc35-82397400a63f@gmx.de>\n\t<20220325102241.GD3036343@pyrite.rasen.tech>\n\t<6344c0a8-dd8c-19dc-c424-e0727d09675c@gmx.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<6344c0a8-dd8c-19dc-c424-e0727d09675c@gmx.de>","Subject":"Re: [libcamera-devel] [PATCH] implement vector casts for Point,\n\tSize and Rectangle","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]