[{"id":4509,"web_url":"https://patchwork.libcamera.org/comment/4509/","msgid":"<20200425125402.GB10975@pendragon.ideasonboard.com>","date":"2020-04-25T12:54:02","subject":"Re: [libcamera-devel] [PATCH v3 06/13] libcamera: camera_sensor:\n\tCollect pixel array properties","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Fri, Apr 24, 2020 at 11:52:57PM +0200, Jacopo Mondi wrote:\n> Collect the sensor pixel array properties by retrieving the subdevice\n> native size and active pixel array size.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/camera_sensor.cpp | 23 +++++++++++++++++++++++\n> 1 file changed, 23 insertions(+)\n> \n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 8d7abc7147a7..a54751fecf5a 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -169,6 +169,29 @@ int CameraSensor::initProperties()\n> \t\tpropertyValue = rotationControl->second.def().get();\n> \tproperties_.set(properties::Rotation, propertyValue);\n> \n> +\t/*\n> +\t * Sensor pixel array properties. Conditionally register them if the\n> +\t * sub-device provides support for the selection API.\n> +\t */\n> +\tSize size{};\n> +\tint ret = subdev_->getNativeSize(0, &size);\n> +\tif (ret && ret != -ENOTTY)\n> +\t\treturn ret;\n\nThis answers my previous question :-)\n\nShould failures (other than -ENOTTY) be considered fatal, or should we\ncontinue with other properties ?\n\n> +\tif (!ret)\n> +\t\tproperties_.set(properties::PixelArray, { static_cast(size.width),\n> +\t\t\t\t\t\t\t static_cast(size.height) });\n> +\n> +\t/*\n> +\t * \\todo The sub-device API only support a single active area rectangle\n\nI don't think it will ever support more, I think you can drop this\ncomment.\n\n> +\t */\n> +\tRectangle rect{};\n> +\tret = subdev_->getActiveArea(0, &rect);\n> +\tif (ret && ret != -ENOTTY)\n> +\t\treturn ret;\n> +\tif (!ret)\n> +\t\tproperties_.set(properties::ActiveAreas, { rect.x, rect.y,\n> +\t\t\t\t\t\t\t static_cast(rect.width),\n> +\t\t\t\t\t\t\t static_cast(rect.height) });\n\nHow about adding two control types for Size and Rectangle, and using\nthem for these properties ? I wrote this some time ago as a test patch:\n\ncommit 08f1cb4e8477cbf1062dcc1d6bf99a7c72347e9b\nAuthor: Laurent Pinchart \nDate: Sat Feb 29 03:39:46 2020 +0200\n\n [DNI] How easy is it to add a Size control type ?\n\n Signed-off-by: Laurent Pinchart \n\ndiff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\nindex 4b2e7e9cdd6c..89d5a6a72820 100644\n--- a/include/libcamera/controls.h\n+++ b/include/libcamera/controls.h\n@@ -13,6 +13,7 @@\n #include \n #include \n\n+#include \n #include \n\n namespace libcamera {\n@@ -27,6 +28,7 @@ enum ControlType {\n \tControlTypeInteger64,\n \tControlTypeFloat,\n \tControlTypeString,\n+\tControlTypeSize,\n };\n\n namespace details {\n@@ -70,6 +72,11 @@ struct control_type {\n \tstatic constexpr ControlType value = ControlTypeString;\n };\n\n+template<>\n+struct control_type {\n+\tstatic constexpr ControlType value = ControlTypeSize;\n+};\n+\n template\n struct control_type> : public control_type> {\n };\ndiff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\nindex 83555c021b6c..97ac0c7b3942 100644\n--- a/src/libcamera/control_ids.yaml\n+++ b/src/libcamera/control_ids.yaml\n@@ -58,4 +58,14 @@ controls:\n - SensorModel:\n type: string\n description: The sensor model name\n+\n+ - TheSize:\n+ type: Size\n+ description: A Size property\n+\n+ - TheSizes:\n+ type: Size\n+ description: A Size array property\n+ size: [n]\n+\n ...\ndiff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\nindex 540cc026417a..a1ec994900a5 100644\n--- a/src/libcamera/controls.cpp\n+++ b/src/libcamera/controls.cpp\n@@ -58,6 +58,7 @@ static constexpr size_t ControlValueSize[] = {\n \t[ControlTypeInteger64]\t\t= sizeof(int64_t),\n \t[ControlTypeFloat]\t\t= sizeof(float),\n \t[ControlTypeString]\t\t= sizeof(char),\n+\t[ControlTypeSize]\t\t= sizeof(Size),\n };\n\n } /* namespace */\n@@ -242,6 +243,12 @@ std::string ControlValue::toString() const\n \t\t\tstr += std::to_string(*value);\n \t\t\tbreak;\n \t\t}\n+\t\tcase ControlTypeSize: {\n+\t\t\tconst Size *value = reinterpret_cast(data);\n+\t\t\tstr += std::to_string(value->width) + \"x\"\n+\t\t\t + std::to_string(value->height);\n+\t\t\tbreak;\n+\t\t}\n \t\tcase ControlTypeNone:\n \t\tcase ControlTypeString:\n \t\t\tbreak;\ndiff --git a/test/serialization/control_serialization.cpp b/test/serialization/control_serialization.cpp\nindex e1d0055d139c..4885501bdcf3 100644\n--- a/test/serialization/control_serialization.cpp\n+++ b/test/serialization/control_serialization.cpp\n@@ -47,6 +47,8 @@ protected:\n \t\tlist.set(controls::Saturation, 50);\n \t\tlist.set(controls::BayerGains, { 1.0f });\n \t\tlist.set(controls::SensorModel, \"VIMC Sensor B\");\n+\t\tlist.set(controls::TheSize, Size{ 640, 480 });\n+\t\tlist.set(controls::TheSizes, { Size{ 640, 480 }, Size{ 1280, 720 } });\n\n \t\t/*\n \t\t * Serialize the control list, this should fail as the control\n\nI think it would lead to cleaner code than storing rectangles and sizes\nin integer arrays.\n\n> \treturn 0;\n> }\n>","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F3EDD603FB\n\tfor ;\n\tSat, 25 Apr 2020 14:54:17 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 574EE4F7;\n\tSat, 25 Apr 2020 14:54:17 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"jWal5HPI\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1587819257;\n\tbh=26ykDicVNszl5d3MCtA+eYFpV+U5+AYsE5HgKpWX3rI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jWal5HPIhDZBfAsNbLPE5RDTLCWbygNLnVXFwbO6qnsALU29oYzGRT+/fFoxrkfeF\n\tXqzeznVtEgpcyOPOFuXRg2gOiB8oZ0VsmQqeA/jiZCd45YuPAfg4C85iLx7WsgXLIC\n\tHCYnGZsmWePLvyXSgH62qhQt9yJZggW8u7NQXmRE=","Date":"Sat, 25 Apr 2020 15:54:02 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200425125402.GB10975@pendragon.ideasonboard.com>","References":"<20200424215304.558317-1-jacopo@jmondi.org>\n\t<20200424215304.558317-7-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200424215304.558317-7-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v3 06/13] libcamera: camera_sensor:\n\tCollect pixel array properties","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","X-List-Received-Date":"Sat, 25 Apr 2020 12:54:18 -0000"}},{"id":4512,"web_url":"https://patchwork.libcamera.org/comment/4512/","msgid":"<20200425134710.ficlfkpl4ovkgdp4@uno.localdomain>","date":"2020-04-25T13:47:10","subject":"Re: [libcamera-devel] [PATCH v3 06/13] libcamera: camera_sensor:\n\tCollect pixel array properties","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"On Sat, Apr 25, 2020 at 03:54:02PM +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Fri, Apr 24, 2020 at 11:52:57PM +0200, Jacopo Mondi wrote:\n> > Collect the sensor pixel array properties by retrieving the subdevice\n> > native size and active pixel array size.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/camera_sensor.cpp | 23 +++++++++++++++++++++++\n> > 1 file changed, 23 insertions(+)\n> >\n> > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > index 8d7abc7147a7..a54751fecf5a 100644\n> > --- a/src/libcamera/camera_sensor.cpp\n> > +++ b/src/libcamera/camera_sensor.cpp\n> > @@ -169,6 +169,29 @@ int CameraSensor::initProperties()\n> > \t\tpropertyValue = rotationControl->second.def().get();\n> > \tproperties_.set(properties::Rotation, propertyValue);\n> >\n> > +\t/*\n> > +\t * Sensor pixel array properties. Conditionally register them if the\n> > +\t * sub-device provides support for the selection API.\n> > +\t */\n> > +\tSize size{};\n> > +\tint ret = subdev_->getNativeSize(0, &size);\n> > +\tif (ret && ret != -ENOTTY)\n> > +\t\treturn ret;\n>\n> This answers my previous question :-)\n>\n> Should failures (other than -ENOTTY) be considered fatal, or should we\n> continue with other properties ?\n\nAn failure != -ENOTTY mean something went wrong, possibly on the\nkernel side, so I would rather fail here. The failure is loud in the\nV4L2Subdevice class already. Do you think we should continue instead ?\n\n>\n> > +\tif (!ret)\n> > +\t\tproperties_.set(properties::PixelArray, { static_cast(size.width),\n> > +\t\t\t\t\t\t\t static_cast(size.height) });\n> > +\n> > +\t/*\n> > +\t * \\todo The sub-device API only support a single active area rectangle\n>\n> I don't think it will ever support more, I think you can drop this\n> comment.\n>\n\nack, although the property defines more rectangles, and one could\nwonder why we ignore that\n\n> > +\t */\n> > +\tRectangle rect{};\n> > +\tret = subdev_->getActiveArea(0, &rect);\n> > +\tif (ret && ret != -ENOTTY)\n> > +\t\treturn ret;\n> > +\tif (!ret)\n> > +\t\tproperties_.set(properties::ActiveAreas, { rect.x, rect.y,\n> > +\t\t\t\t\t\t\t static_cast(rect.width),\n> > +\t\t\t\t\t\t\t static_cast(rect.height) });\n>\n> How about adding two control types for Size and Rectangle, and using\n> them for these properties ? I wrote this some time ago as a test patch:\n\nI would be glad to do this after the series went in, the gain is\nminimal imho, the main advantage I see is that it won't be possible to\nget the order of fields wrong.\n\n\n>\n> commit 08f1cb4e8477cbf1062dcc1d6bf99a7c72347e9b\n> Author: Laurent Pinchart \n> Date: Sat Feb 29 03:39:46 2020 +0200\n>\n> [DNI] How easy is it to add a Size control type ?\n>\n> Signed-off-by: Laurent Pinchart \n>\n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index 4b2e7e9cdd6c..89d5a6a72820 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -13,6 +13,7 @@\n> #include \n> #include \n>\n> +#include \n> #include \n>\n> namespace libcamera {\n> @@ -27,6 +28,7 @@ enum ControlType {\n> \tControlTypeInteger64,\n> \tControlTypeFloat,\n> \tControlTypeString,\n> +\tControlTypeSize,\n> };\n>\n> namespace details {\n> @@ -70,6 +72,11 @@ struct control_type {\n> \tstatic constexpr ControlType value = ControlTypeString;\n> };\n>\n> +template<>\n> +struct control_type {\n> +\tstatic constexpr ControlType value = ControlTypeSize;\n> +};\n> +\n> template\n> struct control_type> : public control_type> {\n> };\n> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> index 83555c021b6c..97ac0c7b3942 100644\n> --- a/src/libcamera/control_ids.yaml\n> +++ b/src/libcamera/control_ids.yaml\n> @@ -58,4 +58,14 @@ controls:\n> - SensorModel:\n> type: string\n> description: The sensor model name\n> +\n> + - TheSize:\n> + type: Size\n> + description: A Size property\n> +\n> + - TheSizes:\n> + type: Size\n> + description: A Size array property\n> + size: [n]\n> +\n> ...\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 540cc026417a..a1ec994900a5 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -58,6 +58,7 @@ static constexpr size_t ControlValueSize[] = {\n> \t[ControlTypeInteger64]\t\t= sizeof(int64_t),\n> \t[ControlTypeFloat]\t\t= sizeof(float),\n> \t[ControlTypeString]\t\t= sizeof(char),\n> +\t[ControlTypeSize]\t\t= sizeof(Size),\n> };\n>\n> } /* namespace */\n> @@ -242,6 +243,12 @@ std::string ControlValue::toString() const\n> \t\t\tstr += std::to_string(*value);\n> \t\t\tbreak;\n> \t\t}\n> +\t\tcase ControlTypeSize: {\n> +\t\t\tconst Size *value = reinterpret_cast(data);\n> +\t\t\tstr += std::to_string(value->width) + \"x\"\n> +\t\t\t + std::to_string(value->height);\n> +\t\t\tbreak;\n> +\t\t}\n> \t\tcase ControlTypeNone:\n> \t\tcase ControlTypeString:\n> \t\t\tbreak;\n> diff --git a/test/serialization/control_serialization.cpp b/test/serialization/control_serialization.cpp\n> index e1d0055d139c..4885501bdcf3 100644\n> --- a/test/serialization/control_serialization.cpp\n> +++ b/test/serialization/control_serialization.cpp\n> @@ -47,6 +47,8 @@ protected:\n> \t\tlist.set(controls::Saturation, 50);\n> \t\tlist.set(controls::BayerGains, { 1.0f });\n> \t\tlist.set(controls::SensorModel, \"VIMC Sensor B\");\n> +\t\tlist.set(controls::TheSize, Size{ 640, 480 });\n> +\t\tlist.set(controls::TheSizes, { Size{ 640, 480 }, Size{ 1280, 720 } });\n>\n> \t\t/*\n> \t\t * Serialize the control list, this should fail as the control\n\nWhat about control serialization ?\n\n>\n> I think it would lead to cleaner code than storing rectangles and sizes\n> in integer arrays.\n>\n> > \treturn 0;\n> > }\n> >\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E99E7603FB\n\tfor ;\n\tSat, 25 Apr 2020 15:44:01 +0200 (CEST)","from uno.localdomain\n\t(host240-55-dynamic.3-87-r.retail.telecomitalia.it [87.3.55.240])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 1717B1C0005;\n\tSat, 25 Apr 2020 13:44:00 +0000 (UTC)"],"X-Originating-IP":"87.3.55.240","Date":"Sat, 25 Apr 2020 15:47:10 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200425134710.ficlfkpl4ovkgdp4@uno.localdomain>","References":"<20200424215304.558317-1-jacopo@jmondi.org>\n\t<20200424215304.558317-7-jacopo@jmondi.org>\n\t<20200425125402.GB10975@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200425125402.GB10975@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 06/13] libcamera: camera_sensor:\n\tCollect pixel array properties","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","X-List-Received-Date":"Sat, 25 Apr 2020 13:44:02 -0000"}},{"id":4517,"web_url":"https://patchwork.libcamera.org/comment/4517/","msgid":"<20200425165646.GD11157@pendragon.ideasonboard.com>","date":"2020-04-25T16:56:46","subject":"Re: [libcamera-devel] [PATCH v3 06/13] libcamera: camera_sensor:\n\tCollect pixel array properties","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Sat, Apr 25, 2020 at 03:47:10PM +0200, Jacopo Mondi wrote:\n> On Sat, Apr 25, 2020 at 03:54:02PM +0300, Laurent Pinchart wrote:\n> > On Fri, Apr 24, 2020 at 11:52:57PM +0200, Jacopo Mondi wrote:\n> > > Collect the sensor pixel array properties by retrieving the subdevice\n> > > native size and active pixel array size.\n> > >\n> > > Signed-off-by: Jacopo Mondi \n> > > ---\n> > > src/libcamera/camera_sensor.cpp | 23 +++++++++++++++++++++++\n> > > 1 file changed, 23 insertions(+)\n> > >\n> > > diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> > > index 8d7abc7147a7..a54751fecf5a 100644\n> > > --- a/src/libcamera/camera_sensor.cpp\n> > > +++ b/src/libcamera/camera_sensor.cpp\n> > > @@ -169,6 +169,29 @@ int CameraSensor::initProperties()\n> > > \t\tpropertyValue = rotationControl->second.def().get();\n> > > \tproperties_.set(properties::Rotation, propertyValue);\n> > >\n> > > +\t/*\n> > > +\t * Sensor pixel array properties. Conditionally register them if the\n> > > +\t * sub-device provides support for the selection API.\n> > > +\t */\n> > > +\tSize size{};\n> > > +\tint ret = subdev_->getNativeSize(0, &size);\n> > > +\tif (ret && ret != -ENOTTY)\n> > > +\t\treturn ret;\n> >\n> > This answers my previous question :-)\n> >\n> > Should failures (other than -ENOTTY) be considered fatal, or should we\n> > continue with other properties ?\n> \n> An failure != -ENOTTY mean something went wrong, possibly on the\n> kernel side, so I would rather fail here. The failure is loud in the\n> V4L2Subdevice class already. Do you think we should continue instead ?\n\nI think you're right, it's probably best to fail to ensure drivers are\ncorrectly implemented.\n\n> > > +\tif (!ret)\n> > > +\t\tproperties_.set(properties::PixelArray, { static_cast(size.width),\n> > > +\t\t\t\t\t\t\t static_cast(size.height) });\n> > > +\n> > > +\t/*\n> > > +\t * \\todo The sub-device API only support a single active area rectangle\n> >\n> > I don't think it will ever support more, I think you can drop this\n> > comment.\n> \n> ack, although the property defines more rectangles, and one could\n> wonder why we ignore that\n> \n> > > +\t */\n> > > +\tRectangle rect{};\n> > > +\tret = subdev_->getActiveArea(0, &rect);\n> > > +\tif (ret && ret != -ENOTTY)\n> > > +\t\treturn ret;\n> > > +\tif (!ret)\n> > > +\t\tproperties_.set(properties::ActiveAreas, { rect.x, rect.y,\n> > > +\t\t\t\t\t\t\t static_cast(rect.width),\n> > > +\t\t\t\t\t\t\t static_cast(rect.height) });\n> >\n> > How about adding two control types for Size and Rectangle, and using\n> > them for these properties ? I wrote this some time ago as a test patch:\n> \n> I would be glad to do this after the series went in, the gain is\n> minimal imho, the main advantage I see is that it won't be possible to\n> get the order of fields wrong.\n\nAnd the fields will also be easier to access, you won't have to convert\nback and forth between Size/Rectangle and integer arrays.\n\nI'll post the patch below, as well as another one for Rectangle\ncontrols.\n\n> > commit 08f1cb4e8477cbf1062dcc1d6bf99a7c72347e9b\n> > Author: Laurent Pinchart \n> > Date: Sat Feb 29 03:39:46 2020 +0200\n> >\n> > [DNI] How easy is it to add a Size control type ?\n> >\n> > Signed-off-by: Laurent Pinchart \n> >\n> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > index 4b2e7e9cdd6c..89d5a6a72820 100644\n> > --- a/include/libcamera/controls.h\n> > +++ b/include/libcamera/controls.h\n> > @@ -13,6 +13,7 @@\n> > #include \n> > #include \n> >\n> > +#include \n> > #include \n> >\n> > namespace libcamera {\n> > @@ -27,6 +28,7 @@ enum ControlType {\n> > \tControlTypeInteger64,\n> > \tControlTypeFloat,\n> > \tControlTypeString,\n> > +\tControlTypeSize,\n> > };\n> >\n> > namespace details {\n> > @@ -70,6 +72,11 @@ struct control_type {\n> > \tstatic constexpr ControlType value = ControlTypeString;\n> > };\n> >\n> > +template<>\n> > +struct control_type {\n> > +\tstatic constexpr ControlType value = ControlTypeSize;\n> > +};\n> > +\n> > template\n> > struct control_type> : public control_type> {\n> > };\n> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > index 83555c021b6c..97ac0c7b3942 100644\n> > --- a/src/libcamera/control_ids.yaml\n> > +++ b/src/libcamera/control_ids.yaml\n> > @@ -58,4 +58,14 @@ controls:\n> > - SensorModel:\n> > type: string\n> > description: The sensor model name\n> > +\n> > + - TheSize:\n> > + type: Size\n> > + description: A Size property\n> > +\n> > + - TheSizes:\n> > + type: Size\n> > + description: A Size array property\n> > + size: [n]\n> > +\n> > ...\n> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > index 540cc026417a..a1ec994900a5 100644\n> > --- a/src/libcamera/controls.cpp\n> > +++ b/src/libcamera/controls.cpp\n> > @@ -58,6 +58,7 @@ static constexpr size_t ControlValueSize[] = {\n> > \t[ControlTypeInteger64]\t\t= sizeof(int64_t),\n> > \t[ControlTypeFloat]\t\t= sizeof(float),\n> > \t[ControlTypeString]\t\t= sizeof(char),\n> > +\t[ControlTypeSize]\t\t= sizeof(Size),\n> > };\n> >\n> > } /* namespace */\n> > @@ -242,6 +243,12 @@ std::string ControlValue::toString() const\n> > \t\t\tstr += std::to_string(*value);\n> > \t\t\tbreak;\n> > \t\t}\n> > +\t\tcase ControlTypeSize: {\n> > +\t\t\tconst Size *value = reinterpret_cast(data);\n> > +\t\t\tstr += std::to_string(value->width) + \"x\"\n> > +\t\t\t + std::to_string(value->height);\n> > +\t\t\tbreak;\n> > +\t\t}\n> > \t\tcase ControlTypeNone:\n> > \t\tcase ControlTypeString:\n> > \t\t\tbreak;\n> > diff --git a/test/serialization/control_serialization.cpp b/test/serialization/control_serialization.cpp\n> > index e1d0055d139c..4885501bdcf3 100644\n> > --- a/test/serialization/control_serialization.cpp\n> > +++ b/test/serialization/control_serialization.cpp\n> > @@ -47,6 +47,8 @@ protected:\n> > \t\tlist.set(controls::Saturation, 50);\n> > \t\tlist.set(controls::BayerGains, { 1.0f });\n> > \t\tlist.set(controls::SensorModel, \"VIMC Sensor B\");\n> > +\t\tlist.set(controls::TheSize, Size{ 640, 480 });\n> > +\t\tlist.set(controls::TheSizes, { Size{ 640, 480 }, Size{ 1280, 720 } });\n> >\n> > \t\t/*\n> > \t\t * Serialize the control list, this should fail as the control\n> \n> What about control serialization ?\n\nWhat about it ? :-)\n\n> > I think it would lead to cleaner code than storing rectangles and sizes\n> > in integer arrays.\n> >\n> > > \treturn 0;\n> > > }","headers":{"Return-Path":"","Received":["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 5B899603FC\n\tfor ;\n\tSat, 25 Apr 2020 18:57:01 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C7FD44F7;\n\tSat, 25 Apr 2020 18:57:00 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"b/BDdl4E\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1587833821;\n\tbh=YyEkEGLywd1qq8bo6sqjciCnzaEYy0Mo8X8bznjkHMY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=b/BDdl4E39xxom2xllOmXEJ8p4Jdiz9vkp0bJVR5S/E5IjoPHbuceSp2l7aMjRAhX\n\tOmIZwpVwxs7nO3rE9i2jV7BePMKrtY/sRpgkkiyg/a8QWwZLRw1N83b8AE+SRivORB\n\tOM6hFVm8+YGlqprJKmwoF82R1WrgFtEB4ql1blEk=","Date":"Sat, 25 Apr 2020 19:56:46 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200425165646.GD11157@pendragon.ideasonboard.com>","References":"<20200424215304.558317-1-jacopo@jmondi.org>\n\t<20200424215304.558317-7-jacopo@jmondi.org>\n\t<20200425125402.GB10975@pendragon.ideasonboard.com>\n\t<20200425134710.ficlfkpl4ovkgdp4@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200425134710.ficlfkpl4ovkgdp4@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v3 06/13] libcamera: camera_sensor:\n\tCollect pixel array properties","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","X-List-Received-Date":"Sat, 25 Apr 2020 16:57:01 -0000"}}]