[{"id":24326,"web_url":"https://patchwork.libcamera.org/comment/24326/","msgid":"<20220803094401.453dofsh22yjpw5z@uno.localdomain>","date":"2022-08-03T09:44:01","subject":"Re: [libcamera-devel] [PATCH v13] libcamera: controls: Generate and\n\tuse fixed-sized Span types","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Christian,\n\nOn Tue, Aug 02, 2022 at 11:03:24PM +0200, Christian Rauch wrote:\n> Define Span types explicitly as either variable- or fixed-sized. This\n> introduces a new convention for defining Span dimensions in the property\n> and control value definitions and generates Span types as variable-sized\n> Span or as fixed-sized Span.\n>\n> Signed-off-by: Christian Rauch \n> Reviewed-by: Jacopo Mondi \n\nThe issue with gcc8 seems now solved.\n\nOne more tag and I hope we can collect the patch ?\n\nThanks\n j\n\n> ---\n> include/libcamera/controls.h | 2 +-\n> src/ipa/raspberrypi/raspberrypi.cpp | 19 +++++++++--------\n> src/libcamera/control_ids.yaml | 4 ++--\n> src/libcamera/property_ids.yaml | 4 ++--\n> src/qcam/dng_writer.cpp | 2 +-\n> utils/gen-controls.py | 32 ++++++++++++++++++++---------\n> 6 files changed, 38 insertions(+), 25 deletions(-)\n>\n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index b1b52acb..ebc168fc 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -168,7 +168,7 @@ public:\n>\n> \t\tusing V = typename T::value_type;\n> \t\tconst V *value = reinterpret_cast(data().data());\n> -\t\treturn { value, numElements_ };\n> +\t\treturn T{ value, numElements_ };\n> \t}\n>\n> #ifndef __DOXYGEN__\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 6befdd71..69c73f8c 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -567,18 +567,19 @@ void IPARPi::reportMetadata()\n>\n> \tAwbStatus *awbStatus = rpiMetadata_.getLocked(\"awb.status\");\n> \tif (awbStatus) {\n> -\t\tlibcameraMetadata_.set(controls::ColourGains, { static_cast(awbStatus->gainR),\n> -\t\t\t\t\t\t\t\tstatic_cast(awbStatus->gainB) });\n> +\t\tlibcameraMetadata_.set(controls::ColourGains,\n> +\t\t\t\t Span({ static_cast(awbStatus->gainR),\n> +\t\t\t\t\t\t\t static_cast(awbStatus->gainB) }));\n> \t\tlibcameraMetadata_.set(controls::ColourTemperature, awbStatus->temperatureK);\n> \t}\n>\n> \tBlackLevelStatus *blackLevelStatus = rpiMetadata_.getLocked(\"black_level.status\");\n> \tif (blackLevelStatus)\n> \t\tlibcameraMetadata_.set(controls::SensorBlackLevels,\n> -\t\t\t\t { static_cast(blackLevelStatus->blackLevelR),\n> -\t\t\t\t\t static_cast(blackLevelStatus->blackLevelG),\n> -\t\t\t\t\t static_cast(blackLevelStatus->blackLevelG),\n> -\t\t\t\t\t static_cast(blackLevelStatus->blackLevelB) });\n> +\t\t\t\t Span({ static_cast(blackLevelStatus->blackLevelR),\n> +\t\t\t\t\t\t\t\tstatic_cast(blackLevelStatus->blackLevelG),\n> +\t\t\t\t\t\t\t\tstatic_cast(blackLevelStatus->blackLevelG),\n> +\t\t\t\t\t\t\t\tstatic_cast(blackLevelStatus->blackLevelB) }));\n>\n> \tFocusStatus *focusStatus = rpiMetadata_.getLocked(\"focus.status\");\n> \tif (focusStatus && focusStatus->num == 12) {\n> @@ -883,7 +884,7 @@ void IPARPi::queueRequest(const ControlList &controls)\n> \t\t\tif (gains[0] != 0.0f && gains[1] != 0.0f)\n> \t\t\t\t/* A gain of 0.0f will switch back to auto mode. */\n> \t\t\t\tlibcameraMetadata_.set(controls::ColourGains,\n> -\t\t\t\t\t\t { gains[0], gains[1] });\n> +\t\t\t\t\t\t Span({ gains[0], gains[1] }));\n> \t\t\tbreak;\n> \t\t}\n>\n> @@ -1167,8 +1168,8 @@ void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDur\n>\n> \t/* Return the validated limits via metadata. */\n> \tlibcameraMetadata_.set(controls::FrameDurationLimits,\n> -\t\t\t { static_cast(minFrameDuration_.get()),\n> -\t\t\t\t static_cast(maxFrameDuration_.get()) });\n> +\t\t\t Span({ static_cast(minFrameDuration_.get()),\n> +\t\t\t\t\t\t\tstatic_cast(maxFrameDuration_.get()) }));\n>\n> \t/*\n> \t * Calculate the maximum exposure time possible for the AGC to use.\n> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> index ecab3ae9..5fa168c6 100644\n> --- a/src/libcamera/control_ids.yaml\n> +++ b/src/libcamera/control_ids.yaml\n> @@ -291,7 +291,7 @@ controls:\n> transformation. The 3x3 matrix is stored in conventional reading\n> order in an array of 9 floating point values.\n>\n> - size: [3x3]\n> + size: [3,3]\n>\n> - ScalerCrop:\n> type: Rectangle\n> @@ -525,7 +525,7 @@ controls:\n> the window where the focal distance for the objects shown in that part\n> of the image are closest to the camera.\n>\n> - size: [n]\n> + size: []\n>\n> - AfTrigger:\n> type: int32_t\n> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml\n> index 11b7ebdc..a87485d7 100644\n> --- a/src/libcamera/property_ids.yaml\n> +++ b/src/libcamera/property_ids.yaml\n> @@ -497,7 +497,7 @@ controls:\n>\n> - PixelArrayOpticalBlackRectangles:\n> type: Rectangle\n> - size: [n]\n> + size: []\n> description: |\n> The pixel array region(s) which contain optical black pixels\n> considered valid for calibration purposes.\n> @@ -592,7 +592,7 @@ controls:\n>\n> - PixelArrayActiveAreas:\n> type: Rectangle\n> - size: [n]\n> + size: []\n> description: |\n> The PixelArrayActiveAreas property defines the (possibly multiple and\n> overlapping) portions of the camera sensor readable pixel matrix\n> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n> index 5b94b3f2..b4362537 100644\n> --- a/src/qcam/dng_writer.cpp\n> +++ b/src/qcam/dng_writer.cpp\n> @@ -517,7 +517,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n>\n> \tconst auto &blackLevels = metadata.get(controls::SensorBlackLevels);\n> \tif (blackLevels) {\n> -\t\tSpan levels = *blackLevels;\n> +\t\tSpan levels = *blackLevels;\n>\n> \t\t/*\n> \t\t * The black levels control is specified in R, Gr, Gb, B order.\n> diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> index 3f99b5e2..46ba4394 100755\n> --- a/utils/gen-controls.py\n> +++ b/utils/gen-controls.py\n> @@ -7,6 +7,8 @@\n> # gen-controls.py - Generate control definitions from YAML\n>\n> import argparse\n> +from functools import reduce\n> +import operator\n> import string\n> import sys\n> import yaml\n> @@ -22,6 +24,24 @@ def format_description(description):\n> return '\\n'.join([(line and ' * ' or ' *') + line for line in description])\n>\n>\n> +def get_ctrl_type(ctrl):\n> + ctrl_type = ctrl['type']\n> + ctrl_size_arr = ctrl.get('size')\n> +\n> + if ctrl_type == 'string':\n> + return 'std::string'\n> + elif ctrl_size_arr is not None:\n> + if len(ctrl_size_arr) > 0:\n> + # fixed-sized Span\n> + ctrl_span_size = reduce(operator.mul, ctrl_size_arr)\n> + return f\"Span\"\n> + else:\n> + # variable-sized Span\n> + return f\"Span\"\n> + else:\n> + return ctrl_type\n> +\n> +\n> def generate_cpp(controls):\n> enum_doc_start_template = string.Template('''/**\n> * \\\\enum ${name}Enum\n> @@ -50,11 +70,7 @@ ${description}\n> name, ctrl = ctrl.popitem()\n> id_name = snake_case(name).upper()\n>\n> - ctrl_type = ctrl['type']\n> - if ctrl_type == 'string':\n> - ctrl_type = 'std::string'\n> - elif ctrl.get('size'):\n> - ctrl_type = 'Span' % ctrl_type\n> + ctrl_type = get_ctrl_type(ctrl)\n>\n> info = {\n> 'name': name,\n> @@ -135,11 +151,7 @@ def generate_h(controls):\n>\n> ids.append('\\t' + id_name + ' = ' + str(id_value) + ',')\n>\n> - ctrl_type = ctrl['type']\n> - if ctrl_type == 'string':\n> - ctrl_type = 'std::string'\n> - elif ctrl.get('size'):\n> - ctrl_type = 'Span' % ctrl_type\n> + ctrl_type = get_ctrl_type(ctrl)\n>\n> info = {\n> 'name': name,\n> --\n> 2.34.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2D138C3272\n\tfor ;\n\tWed, 3 Aug 2022 09:44:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D78F063315;\n\tWed, 3 Aug 2022 11:44:05 +0200 (CEST)","from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[217.70.183.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AF90A603EF\n\tfor ;\n\tWed, 3 Aug 2022 11:44:04 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id ADBCA60006;\n\tWed, 3 Aug 2022 09:44:03 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659519845;\n\tbh=iml9KX8y/tN45dP4+zhVxrU6SbhOMus+1QZdzxucBHI=;\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=MVHjyoZvxm8fGDOTx7T7HopxmOKZhBFxAcxk66uRwLcz3zkB2uWefuUHq8IZA3Bua\n\toU0ytL0tkTw4lJrZwo3Wt7JNNaiugCRuqYvddfQC+s2ZJ3rTl2zbt8K5a4DgOwn5E+\n\tioBk2A95xef1aPEAIbiq7xis0eLB1QvzIl+k12FjLKKDLwaYOoMagBr51pjvj33Jis\n\tQJRPCAH9tijeFznqWmxlMRlHYvUFMb+M9J3C/4j1ALL9EVXX6nilnPKy3V8lyNH/0h\n\tKcvVYkm2O3KeBSbJ8I1g4GMrEyqY3SqMi62H6gD6naZhLbpROAd93q3QU8EoSXueV/\n\toa8EnfIf2qGfQ==","Date":"Wed, 3 Aug 2022 11:44:01 +0200","To":"Christian Rauch ","Message-ID":"<20220803094401.453dofsh22yjpw5z@uno.localdomain>","References":"<20220802210324.115893-1-Rauch.Christian@gmx.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220802210324.115893-1-Rauch.Christian@gmx.de>","Subject":"Re: [libcamera-devel] [PATCH v13] libcamera: controls: Generate and\n\tuse fixed-sized Span types","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24345,"web_url":"https://patchwork.libcamera.org/comment/24345/","msgid":"<165952907378.3981176.3748316334722316806@Monstersaurus>","date":"2022-08-03T12:17:53","subject":"Re: [libcamera-devel] [PATCH v13] libcamera: controls: Generate and\n\tuse fixed-sized Span types","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jacopo Mondi via libcamera-devel (2022-08-03 10:44:01)\n> Hi Christian,\n> \n> On Tue, Aug 02, 2022 at 11:03:24PM +0200, Christian Rauch wrote:\n> > Define Span types explicitly as either variable- or fixed-sized. This\n> > introduces a new convention for defining Span dimensions in the property\n> > and control value definitions and generates Span types as variable-sized\n> > Span or as fixed-sized Span.\n> >\n> > Signed-off-by: Christian Rauch \n> > Reviewed-by: Jacopo Mondi \n> \n> The issue with gcc8 seems now solved.\n> \n> One more tag and I hope we can collect the patch ?\n\nWhat are the user facing API changes here? (i.e. how much does this\nbreak applications?)\n\nI'm surprised there are no updates to the python bindings in this patch\n?\n\nIs that all handled gracefully? (Is it tested?)\n\n--\nKieran\n\n\n> \n> Thanks\n> j\n> \n> > ---\n> > include/libcamera/controls.h | 2 +-\n> > src/ipa/raspberrypi/raspberrypi.cpp | 19 +++++++++--------\n> > src/libcamera/control_ids.yaml | 4 ++--\n> > src/libcamera/property_ids.yaml | 4 ++--\n> > src/qcam/dng_writer.cpp | 2 +-\n> > utils/gen-controls.py | 32 ++++++++++++++++++++---------\n> > 6 files changed, 38 insertions(+), 25 deletions(-)\n> >\n> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > index b1b52acb..ebc168fc 100644\n> > --- a/include/libcamera/controls.h\n> > +++ b/include/libcamera/controls.h\n> > @@ -168,7 +168,7 @@ public:\n> >\n> > using V = typename T::value_type;\n> > const V *value = reinterpret_cast(data().data());\n> > - return { value, numElements_ };\n> > + return T{ value, numElements_ };\n> > }\n> >\n> > #ifndef __DOXYGEN__\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index 6befdd71..69c73f8c 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -567,18 +567,19 @@ void IPARPi::reportMetadata()\n> >\n> > AwbStatus *awbStatus = rpiMetadata_.getLocked(\"awb.status\");\n> > if (awbStatus) {\n> > - libcameraMetadata_.set(controls::ColourGains, { static_cast(awbStatus->gainR),\n> > - static_cast(awbStatus->gainB) });\n> > + libcameraMetadata_.set(controls::ColourGains,\n> > + Span({ static_cast(awbStatus->gainR),\n> > + static_cast(awbStatus->gainB) }));\n> > libcameraMetadata_.set(controls::ColourTemperature, awbStatus->temperatureK);\n> > }\n> >\n> > BlackLevelStatus *blackLevelStatus = rpiMetadata_.getLocked(\"black_level.status\");\n> > if (blackLevelStatus)\n> > libcameraMetadata_.set(controls::SensorBlackLevels,\n> > - { static_cast(blackLevelStatus->blackLevelR),\n> > - static_cast(blackLevelStatus->blackLevelG),\n> > - static_cast(blackLevelStatus->blackLevelG),\n> > - static_cast(blackLevelStatus->blackLevelB) });\n> > + Span({ static_cast(blackLevelStatus->blackLevelR),\n> > + static_cast(blackLevelStatus->blackLevelG),\n> > + static_cast(blackLevelStatus->blackLevelG),\n> > + static_cast(blackLevelStatus->blackLevelB) }));\n> >\n> > FocusStatus *focusStatus = rpiMetadata_.getLocked(\"focus.status\");\n> > if (focusStatus && focusStatus->num == 12) {\n> > @@ -883,7 +884,7 @@ void IPARPi::queueRequest(const ControlList &controls)\n> > if (gains[0] != 0.0f && gains[1] != 0.0f)\n> > /* A gain of 0.0f will switch back to auto mode. */\n> > libcameraMetadata_.set(controls::ColourGains,\n> > - { gains[0], gains[1] });\n> > + Span({ gains[0], gains[1] }));\n> > break;\n> > }\n> >\n> > @@ -1167,8 +1168,8 @@ void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDur\n> >\n> > /* Return the validated limits via metadata. */\n> > libcameraMetadata_.set(controls::FrameDurationLimits,\n> > - { static_cast(minFrameDuration_.get()),\n> > - static_cast(maxFrameDuration_.get()) });\n> > + Span({ static_cast(minFrameDuration_.get()),\n> > + static_cast(maxFrameDuration_.get()) }));\n> >\n> > /*\n> > * Calculate the maximum exposure time possible for the AGC to use.\n> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > index ecab3ae9..5fa168c6 100644\n> > --- a/src/libcamera/control_ids.yaml\n> > +++ b/src/libcamera/control_ids.yaml\n> > @@ -291,7 +291,7 @@ controls:\n> > transformation. The 3x3 matrix is stored in conventional reading\n> > order in an array of 9 floating point values.\n> >\n> > - size: [3x3]\n> > + size: [3,3]\n> >\n> > - ScalerCrop:\n> > type: Rectangle\n> > @@ -525,7 +525,7 @@ controls:\n> > the window where the focal distance for the objects shown in that part\n> > of the image are closest to the camera.\n> >\n> > - size: [n]\n> > + size: []\n> >\n> > - AfTrigger:\n> > type: int32_t\n> > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml\n> > index 11b7ebdc..a87485d7 100644\n> > --- a/src/libcamera/property_ids.yaml\n> > +++ b/src/libcamera/property_ids.yaml\n> > @@ -497,7 +497,7 @@ controls:\n> >\n> > - PixelArrayOpticalBlackRectangles:\n> > type: Rectangle\n> > - size: [n]\n> > + size: []\n> > description: |\n> > The pixel array region(s) which contain optical black pixels\n> > considered valid for calibration purposes.\n> > @@ -592,7 +592,7 @@ controls:\n> >\n> > - PixelArrayActiveAreas:\n> > type: Rectangle\n> > - size: [n]\n> > + size: []\n> > description: |\n> > The PixelArrayActiveAreas property defines the (possibly multiple and\n> > overlapping) portions of the camera sensor readable pixel matrix\n> > diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n> > index 5b94b3f2..b4362537 100644\n> > --- a/src/qcam/dng_writer.cpp\n> > +++ b/src/qcam/dng_writer.cpp\n> > @@ -517,7 +517,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> >\n> > const auto &blackLevels = metadata.get(controls::SensorBlackLevels);\n> > if (blackLevels) {\n> > - Span levels = *blackLevels;\n> > + Span levels = *blackLevels;\n> >\n> > /*\n> > * The black levels control is specified in R, Gr, Gb, B order.\n> > diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> > index 3f99b5e2..46ba4394 100755\n> > --- a/utils/gen-controls.py\n> > +++ b/utils/gen-controls.py\n> > @@ -7,6 +7,8 @@\n> > # gen-controls.py - Generate control definitions from YAML\n> >\n> > import argparse\n> > +from functools import reduce\n> > +import operator\n> > import string\n> > import sys\n> > import yaml\n> > @@ -22,6 +24,24 @@ def format_description(description):\n> > return '\\n'.join([(line and ' * ' or ' *') + line for line in description])\n> >\n> >\n> > +def get_ctrl_type(ctrl):\n> > + ctrl_type = ctrl['type']\n> > + ctrl_size_arr = ctrl.get('size')\n> > +\n> > + if ctrl_type == 'string':\n> > + return 'std::string'\n> > + elif ctrl_size_arr is not None:\n> > + if len(ctrl_size_arr) > 0:\n> > + # fixed-sized Span\n> > + ctrl_span_size = reduce(operator.mul, ctrl_size_arr)\n> > + return f\"Span\"\n> > + else:\n> > + # variable-sized Span\n> > + return f\"Span\"\n> > + else:\n> > + return ctrl_type\n> > +\n> > +\n> > def generate_cpp(controls):\n> > enum_doc_start_template = string.Template('''/**\n> > * \\\\enum ${name}Enum\n> > @@ -50,11 +70,7 @@ ${description}\n> > name, ctrl = ctrl.popitem()\n> > id_name = snake_case(name).upper()\n> >\n> > - ctrl_type = ctrl['type']\n> > - if ctrl_type == 'string':\n> > - ctrl_type = 'std::string'\n> > - elif ctrl.get('size'):\n> > - ctrl_type = 'Span' % ctrl_type\n> > + ctrl_type = get_ctrl_type(ctrl)\n> >\n> > info = {\n> > 'name': name,\n> > @@ -135,11 +151,7 @@ def generate_h(controls):\n> >\n> > ids.append('\\t' + id_name + ' = ' + str(id_value) + ',')\n> >\n> > - ctrl_type = ctrl['type']\n> > - if ctrl_type == 'string':\n> > - ctrl_type = 'std::string'\n> > - elif ctrl.get('size'):\n> > - ctrl_type = 'Span' % ctrl_type\n> > + ctrl_type = get_ctrl_type(ctrl)\n> >\n> > info = {\n> > 'name': name,\n> > --\n> > 2.34.1\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 56419BE173\n\tfor ;\n\tWed, 3 Aug 2022 12:17:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C8581603E6;\n\tWed, 3 Aug 2022 14:17:58 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B8D75603E6\n\tfor ;\n\tWed, 3 Aug 2022 14:17:56 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 399A930B;\n\tWed, 3 Aug 2022 14:17:56 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659529078;\n\tbh=+7BNZYIdceoPZivvZfpqie9jjfiuDYYXMX3JWph3OdQ=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=JIw22/SkKHIOXjnxFcJgvG4FYFw/JTdqfNzne7AFbhU4wFOOy6WfnK9/FKDKyvS6h\n\tt4Ph+XlVVzrmRU5gft6Seg8oIQJIsgp36xviqfhrMSzAo270eOVpJML3wXAeZwoyxQ\n\tBVGyHCCmY5462A0RhJY5t4cgQomKfGUIwfjD75INy41asI6R06Z/yQdC36wOhk+kHa\n\tyemWTgU7jSSNJrNHEUZFcpOmtqBcXWCTBOzU2F8QsUDIoXL9iJneLZ9eeIOFeRKvdh\n\tR0y39uqWYAeDsSb2d7CGhevdngZ62eai4QLy2G55uF7OaWTTNS8leQO9GWgNOwZVeS\n\tdNkmanBuu1K3A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1659529076;\n\tbh=+7BNZYIdceoPZivvZfpqie9jjfiuDYYXMX3JWph3OdQ=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=JRrnNxd7ERBcRQchZ1T1T0NRQvv62KWyabskjJgGV+P7ZTGANQ955ez6oVNl+1/n1\n\tfZnx55Zawx8s852N8576aKGKwDRFRL2YZwsJN3bYT5BPggG/G29wu9dws+7MA16Osj\n\tNjhrIsMWwJUIRH0o3LUkfXqYcJjjt0LDziGX1XYk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"JRrnNxd7\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220803094401.453dofsh22yjpw5z@uno.localdomain>","References":"<20220802210324.115893-1-Rauch.Christian@gmx.de>\n\t<20220803094401.453dofsh22yjpw5z@uno.localdomain>","To":"Christian Rauch ,\n\tJacopo Mondi , Jacopo Mondi via libcamera-devel\n\t","Date":"Wed, 03 Aug 2022 13:17:53 +0100","Message-ID":"<165952907378.3981176.3748316334722316806@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v13] libcamera: controls: Generate and\n\tuse fixed-sized Span types","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24360,"web_url":"https://patchwork.libcamera.org/comment/24360/","msgid":"","date":"2022-08-03T20:13:23","subject":"Re: [libcamera-devel] [PATCH v13] libcamera: controls: Generate and\n\tuse fixed-sized Span types","submitter":{"id":111,"url":"https://patchwork.libcamera.org/api/people/111/","name":"Christian Rauch","email":"Rauch.Christian@gmx.de"},"content":"Hi Kieran,\n\nAm 03.08.22 um 14:17 schrieb Kieran Bingham:\n> Quoting Jacopo Mondi via libcamera-devel (2022-08-03 10:44:01)\n>> Hi Christian,\n>>\n>> On Tue, Aug 02, 2022 at 11:03:24PM +0200, Christian Rauch wrote:\n>>> Define Span types explicitly as either variable- or fixed-sized. This\n>>> introduces a new convention for defining Span dimensions in the property\n>>> and control value definitions and generates Span types as variable-sized\n>>> Span or as fixed-sized Span.\n>>>\n>>> Signed-off-by: Christian Rauch \n>>> Reviewed-by: Jacopo Mondi \n>>\n>> The issue with gcc8 seems now solved.\n>>\n>> One more tag and I hope we can collect the patch ?\n>\n> What are the user facing API changes here? (i.e. how much does this\n> break applications?)\n\nEverything that touches fixed-sized Spans with explicit types (i.e. no\nauto) need a change that defines the size. variable-sized spans should\nbehave as before. E.g. in the diff you can see that something like\n\"Span levels = *blackLevels;\"\nneeds to be set the Span size as template paremter:\n\"Span levels = *blackLevels;\"\n\nBut overall, I only had to change this in 5 places in the entire\ncodebase to make this compile. So I don't think the impact will be that big.\n\n>\n> I'm surprised there are no updates to the python bindings in this patch\n> ?\n>\n> Is that all handled gracefully? (Is it tested?)\n\nI compiled (\"meson build_py -Dpycamera=enabled\") and ran the tests\n(\"ninja -C build_py/ test\") without failures. Is there another way to\ntest that the python bindings work?\n\nA quick glimpse into \"py_controls_generated.cpp\" tells me that Spans are\nnot used there.\n\n>\n> --\n> Kieran\n>\n>\n>>\n>> Thanks\n>> j\n>>\n>>> ---\n>>> include/libcamera/controls.h | 2 +-\n>>> src/ipa/raspberrypi/raspberrypi.cpp | 19 +++++++++--------\n>>> src/libcamera/control_ids.yaml | 4 ++--\n>>> src/libcamera/property_ids.yaml | 4 ++--\n>>> src/qcam/dng_writer.cpp | 2 +-\n>>> utils/gen-controls.py | 32 ++++++++++++++++++++---------\n>>> 6 files changed, 38 insertions(+), 25 deletions(-)\n>>>\n>>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n>>> index b1b52acb..ebc168fc 100644\n>>> --- a/include/libcamera/controls.h\n>>> +++ b/include/libcamera/controls.h\n>>> @@ -168,7 +168,7 @@ public:\n>>>\n>>> using V = typename T::value_type;\n>>> const V *value = reinterpret_cast(data().data());\n>>> - return { value, numElements_ };\n>>> + return T{ value, numElements_ };\n>>> }\n>>>\n>>> #ifndef __DOXYGEN__\n>>> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n>>> index 6befdd71..69c73f8c 100644\n>>> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n>>> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n>>> @@ -567,18 +567,19 @@ void IPARPi::reportMetadata()\n>>>\n>>> AwbStatus *awbStatus = rpiMetadata_.getLocked(\"awb.status\");\n>>> if (awbStatus) {\n>>> - libcameraMetadata_.set(controls::ColourGains, { static_cast(awbStatus->gainR),\n>>> - static_cast(awbStatus->gainB) });\n>>> + libcameraMetadata_.set(controls::ColourGains,\n>>> + Span({ static_cast(awbStatus->gainR),\n>>> + static_cast(awbStatus->gainB) }));\n>>> libcameraMetadata_.set(controls::ColourTemperature, awbStatus->temperatureK);\n>>> }\n>>>\n>>> BlackLevelStatus *blackLevelStatus = rpiMetadata_.getLocked(\"black_level.status\");\n>>> if (blackLevelStatus)\n>>> libcameraMetadata_.set(controls::SensorBlackLevels,\n>>> - { static_cast(blackLevelStatus->blackLevelR),\n>>> - static_cast(blackLevelStatus->blackLevelG),\n>>> - static_cast(blackLevelStatus->blackLevelG),\n>>> - static_cast(blackLevelStatus->blackLevelB) });\n>>> + Span({ static_cast(blackLevelStatus->blackLevelR),\n>>> + static_cast(blackLevelStatus->blackLevelG),\n>>> + static_cast(blackLevelStatus->blackLevelG),\n>>> + static_cast(blackLevelStatus->blackLevelB) }));\n>>>\n>>> FocusStatus *focusStatus = rpiMetadata_.getLocked(\"focus.status\");\n>>> if (focusStatus && focusStatus->num == 12) {\n>>> @@ -883,7 +884,7 @@ void IPARPi::queueRequest(const ControlList &controls)\n>>> if (gains[0] != 0.0f && gains[1] != 0.0f)\n>>> /* A gain of 0.0f will switch back to auto mode. */\n>>> libcameraMetadata_.set(controls::ColourGains,\n>>> - { gains[0], gains[1] });\n>>> + Span({ gains[0], gains[1] }));\n>>> break;\n>>> }\n>>>\n>>> @@ -1167,8 +1168,8 @@ void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDur\n>>>\n>>> /* Return the validated limits via metadata. */\n>>> libcameraMetadata_.set(controls::FrameDurationLimits,\n>>> - { static_cast(minFrameDuration_.get()),\n>>> - static_cast(maxFrameDuration_.get()) });\n>>> + Span({ static_cast(minFrameDuration_.get()),\n>>> + static_cast(maxFrameDuration_.get()) }));\n>>>\n>>> /*\n>>> * Calculate the maximum exposure time possible for the AGC to use.\n>>> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n>>> index ecab3ae9..5fa168c6 100644\n>>> --- a/src/libcamera/control_ids.yaml\n>>> +++ b/src/libcamera/control_ids.yaml\n>>> @@ -291,7 +291,7 @@ controls:\n>>> transformation. The 3x3 matrix is stored in conventional reading\n>>> order in an array of 9 floating point values.\n>>>\n>>> - size: [3x3]\n>>> + size: [3,3]\n>>>\n>>> - ScalerCrop:\n>>> type: Rectangle\n>>> @@ -525,7 +525,7 @@ controls:\n>>> the window where the focal distance for the objects shown in that part\n>>> of the image are closest to the camera.\n>>>\n>>> - size: [n]\n>>> + size: []\n>>>\n>>> - AfTrigger:\n>>> type: int32_t\n>>> diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml\n>>> index 11b7ebdc..a87485d7 100644\n>>> --- a/src/libcamera/property_ids.yaml\n>>> +++ b/src/libcamera/property_ids.yaml\n>>> @@ -497,7 +497,7 @@ controls:\n>>>\n>>> - PixelArrayOpticalBlackRectangles:\n>>> type: Rectangle\n>>> - size: [n]\n>>> + size: []\n>>> description: |\n>>> The pixel array region(s) which contain optical black pixels\n>>> considered valid for calibration purposes.\n>>> @@ -592,7 +592,7 @@ controls:\n>>>\n>>> - PixelArrayActiveAreas:\n>>> type: Rectangle\n>>> - size: [n]\n>>> + size: []\n>>> description: |\n>>> The PixelArrayActiveAreas property defines the (possibly multiple and\n>>> overlapping) portions of the camera sensor readable pixel matrix\n>>> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n>>> index 5b94b3f2..b4362537 100644\n>>> --- a/src/qcam/dng_writer.cpp\n>>> +++ b/src/qcam/dng_writer.cpp\n>>> @@ -517,7 +517,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n>>>\n>>> const auto &blackLevels = metadata.get(controls::SensorBlackLevels);\n>>> if (blackLevels) {\n>>> - Span levels = *blackLevels;\n>>> + Span levels = *blackLevels;\n>>>\n>>> /*\n>>> * The black levels control is specified in R, Gr, Gb, B order.\n>>> diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n>>> index 3f99b5e2..46ba4394 100755\n>>> --- a/utils/gen-controls.py\n>>> +++ b/utils/gen-controls.py\n>>> @@ -7,6 +7,8 @@\n>>> # gen-controls.py - Generate control definitions from YAML\n>>>\n>>> import argparse\n>>> +from functools import reduce\n>>> +import operator\n>>> import string\n>>> import sys\n>>> import yaml\n>>> @@ -22,6 +24,24 @@ def format_description(description):\n>>> return '\\n'.join([(line and ' * ' or ' *') + line for line in description])\n>>>\n>>>\n>>> +def get_ctrl_type(ctrl):\n>>> + ctrl_type = ctrl['type']\n>>> + ctrl_size_arr = ctrl.get('size')\n>>> +\n>>> + if ctrl_type == 'string':\n>>> + return 'std::string'\n>>> + elif ctrl_size_arr is not None:\n>>> + if len(ctrl_size_arr) > 0:\n>>> + # fixed-sized Span\n>>> + ctrl_span_size = reduce(operator.mul, ctrl_size_arr)\n>>> + return f\"Span\"\n>>> + else:\n>>> + # variable-sized Span\n>>> + return f\"Span\"\n>>> + else:\n>>> + return ctrl_type\n>>> +\n>>> +\n>>> def generate_cpp(controls):\n>>> enum_doc_start_template = string.Template('''/**\n>>> * \\\\enum ${name}Enum\n>>> @@ -50,11 +70,7 @@ ${description}\n>>> name, ctrl = ctrl.popitem()\n>>> id_name = snake_case(name).upper()\n>>>\n>>> - ctrl_type = ctrl['type']\n>>> - if ctrl_type == 'string':\n>>> - ctrl_type = 'std::string'\n>>> - elif ctrl.get('size'):\n>>> - ctrl_type = 'Span' % ctrl_type\n>>> + ctrl_type = get_ctrl_type(ctrl)\n>>>\n>>> info = {\n>>> 'name': name,\n>>> @@ -135,11 +151,7 @@ def generate_h(controls):\n>>>\n>>> ids.append('\\t' + id_name + ' = ' + str(id_value) + ',')\n>>>\n>>> - ctrl_type = ctrl['type']\n>>> - if ctrl_type == 'string':\n>>> - ctrl_type = 'std::string'\n>>> - elif ctrl.get('size'):\n>>> - ctrl_type = 'Span' % ctrl_type\n>>> + ctrl_type = get_ctrl_type(ctrl)\n>>>\n>>> info = {\n>>> 'name': name,\n>>> --\n>>> 2.34.1\n>>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id BE28CC3272\n\tfor ;\n\tWed, 3 Aug 2022 20:13:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F3EA861FAE;\n\tWed, 3 Aug 2022 22:13:25 +0200 (CEST)","from mout.gmx.net (mout.gmx.net [212.227.15.18])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5739C603E6\n\tfor ;\n\tWed, 3 Aug 2022 22:13:24 +0200 (CEST)","from [192.168.0.158] ([88.152.184.103]) by mail.gmx.net (mrgmx004\n\t[212.227.17.190]) with ESMTPSA (Nemesis) id\n\t1Mt79P-1nQPm63bon-00tTNS for\n\t; Wed, 03 Aug 2022 22:13:23 +0200"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659557606;\n\tbh=JyoyJCcThCb5ADZUUG/8Nrr8OlxvZ7WKSL7THXAaaVA=;\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:\n\tFrom;\n\tb=S4aklLa7bsxsTwKJAtMTzwKgPCq3FLR6MjxNfzyTlphmcT5G88Wz+489evW7duN4e\n\tbIS8QVDUZ/TdsUtnaUOI1fCeC4fKqzI7aGccPnIx4CrueawXEocoDny7PyhNDD3SVW\n\tCluRb67RF67yt3ocZHpMNqmGQxJxO2dXzasKakub6kzZhShuw6MieuPd6fgdhcOCFp\n\twhsB3qKCmreRJ0ajlDNPwUjsaI7HCo9pU33CFeHWTC8S28V1E/UszeaSsv0YTZoOKk\n\tccoUFGnqu3BjIY+i/tidt06s2GbVk0dWZUN2nnEB04xhBPhgIuhO8ex4mHicD0axyJ\n\t5sCQesjJkdHLA==","v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net;\n\ts=badeba3b8450; t=1659557604;\n\tbh=JyoyJCcThCb5ADZUUG/8Nrr8OlxvZ7WKSL7THXAaaVA=;\n\th=X-UI-Sender-Class:Date:Subject:To:References:From:In-Reply-To;\n\tb=eDMx747j+NQlvfOXPNFl4jSSqYE6AkTSnnbVFYN5HxfJdeT3BJL28Fr6RKrTbol2D\n\t8OuaBkACAK7yTLav9IOZTv4mxZU9xC2ypP/eju7T95r/wFy6FpQr9y9NyC/2Z/ZVAD\n\t1St3gvi43HABqx9vppenB+ayKECzGGX7H+fkrvfE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=gmx.net header.i=@gmx.net\n\theader.b=\"eDMx747j\"; dkim-atps=neutral","X-UI-Sender-Class":"01bb95c1-4bf8-414a-932a-4f6e2808ef9c","Message-ID":"","Date":"Wed, 3 Aug 2022 22:13:23 +0200","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.11.0","Content-Language":"en-GB","To":"Jacopo Mondi via libcamera-devel ","References":"<20220802210324.115893-1-Rauch.Christian@gmx.de>\n\t<20220803094401.453dofsh22yjpw5z@uno.localdomain>\n\t<165952907378.3981176.3748316334722316806@Monstersaurus>","In-Reply-To":"<165952907378.3981176.3748316334722316806@Monstersaurus>","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"quoted-printable","X-Provags-ID":"V03:K1:oausVQgNSc9EFlJL69F9/HVhnwu3AnX1LA8NQLbpkWoRZbnqnPa\n\tl+q9EaNmVTaomE2eLVgRTt8hx5XwWRmdjDnqC3vTQJdi8/ury753xiipoWjA5QhdbnD1v/s\n\tmgPcYf00Pggb6Ihz2U6W6NM7ZA7OtTHz+6QGi/ysOoEFygv7pXKuwwM73fksI9+W/YhVl21\n\twbjmAR2E2RBriykxnmO4w==","X-Spam-Flag":"NO","X-UI-Out-Filterresults":"notjunk:1; V03:K0:ckTiQupGUVg=:d9c+F9u55cEZG0bSWt/q7U\n\teDb+ElMurKPL3W8nYOPZIYuoHHd0nSyDjpUytlVNDRXPYdidK0d71TsKJLhOYwaz+OBXkhyaK\n\t/SB81mFk9yjNNpisBLIPI1H2HcRHWcygFtANEm3PCeDGLqZ6UGU3ClxtnUQ/9b79RbaeflccW\n\tsZma5L5tp0bgrc4hFT6pskKTuVkNN+tiGrwr9t1FTJIelblHrfee074Vg/XzGlM/233nNLGnG\n\tcVrQpAx0q72OhFyT2t3HbYGdkL3OKPDMlTVXZ00JUOADsvyTq7idPsjC8EOf5ym1Hz8o3HFq6\n\t5pacWIHbX/4Q8mxy+m2TMA6lPTncIUG+SX9G/XFlB/GPW9braF7VDZRyVYlfhhvmdiBmY3VxB\n\thhPe8OljGWrwC9urd1l8xEqlGwU9Wk8V0RTbZeEwhVrSLmfvSkiTSbwddAjwYnENn/epeLp9l\n\tsSZaU0htTzfnD5aNwLleaULy54mhSOixkccfD7XVAilm62VOzGdEDEUK+O3jXAlDSOkPbLe4O\n\te0/AGx1p6U979UTNi/GbdpCRtgO+AMHgUGrhBc6b0ps+rrxXAR5aKYIZmEzpivay9KYsdLQE/\n\tBmWafmCiba6EAp5PSRiRaA99bHEsCPGD0o2cFT2zsP6Veg02BAM4R4HWaS37lbnC4QGAQ0kPe\n\tHSNYK6lV4QAl0TzQ39cA0Uj+Tozcxeem4xvUuKBvA/5dKAPKKfG81bfHr4ZTRwCy2ytI+F457\n\tL1Hm4MKc6cG8vmGoZUfq+NARyahB9k4c3lJQFUgpz5/uey1swL+UJbCzjAyUeHKYRrN6BCN2o\n\tWdIudUqXbfkr7fcTNLve1LkR5KVtd/3NPPmumMqKnerlvxhIGWj2H4nVv45me1UP3OPw9a7Ll\n\tnw919bdLrSxD2IcwikWLIHEDLPGtohT8rbUf2PM80MC8+N1o9RwOF46qfucnSzlTWzG3ruE6R\n\teeGNZUmAqJjfHH74WY3FtTOvqQ4Z4EvB/LngSxCeyhj8dP3kQNfzUpRK3XCb2xJixQr5SB4f4\n\toxx688/EjMlr38on0O4GxPetvONeVXd0fyGTFhYhOkMuxCV2U6nQIqDQzAYjtGlO2FCJkuor1\n\toDcdAs6J9Oothje7gHuL+MM7XWvbl05PBU9wbHdt+E5w5BJsBMX0aPnog==","Subject":"Re: [libcamera-devel] [PATCH v13] libcamera: controls: Generate and\n\tuse fixed-sized Span types","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","From":"Christian Rauch via libcamera-devel\n\t","Reply-To":"Christian Rauch ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24467,"web_url":"https://patchwork.libcamera.org/comment/24467/","msgid":"","date":"2022-08-09T10:53:20","subject":"Re: [libcamera-devel] [PATCH v13] libcamera: controls: Generate and\n\tuse fixed-sized Span types","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Aug 03, 2022 at 11:44:01AM +0200, Jacopo Mondi via libcamera-devel wrote:\n> Hi Christian,\n> \n> On Tue, Aug 02, 2022 at 11:03:24PM +0200, Christian Rauch wrote:\n> > Define Span types explicitly as either variable- or fixed-sized. This\n> > introduces a new convention for defining Span dimensions in the property\n> > and control value definitions and generates Span types as variable-sized\n> > Span or as fixed-sized Span.\n> >\n> > Signed-off-by: Christian Rauch \n> > Reviewed-by: Jacopo Mondi \n> \n> The issue with gcc8 seems now solved.\n> \n> One more tag and I hope we can collect the patch ?\n\nThere are still a few things I'd like to change, but I'll do so by\nsubmitting patches on top, it will be more efficient.\n\nReviewed-by: Laurent Pinchart \n\n> > ---\n> > include/libcamera/controls.h | 2 +-\n> > src/ipa/raspberrypi/raspberrypi.cpp | 19 +++++++++--------\n> > src/libcamera/control_ids.yaml | 4 ++--\n> > src/libcamera/property_ids.yaml | 4 ++--\n> > src/qcam/dng_writer.cpp | 2 +-\n> > utils/gen-controls.py | 32 ++++++++++++++++++++---------\n> > 6 files changed, 38 insertions(+), 25 deletions(-)\n> >\n> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > index b1b52acb..ebc168fc 100644\n> > --- a/include/libcamera/controls.h\n> > +++ b/include/libcamera/controls.h\n> > @@ -168,7 +168,7 @@ public:\n> >\n> > \t\tusing V = typename T::value_type;\n> > \t\tconst V *value = reinterpret_cast(data().data());\n> > -\t\treturn { value, numElements_ };\n> > +\t\treturn T{ value, numElements_ };\n> > \t}\n> >\n> > #ifndef __DOXYGEN__\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index 6befdd71..69c73f8c 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -567,18 +567,19 @@ void IPARPi::reportMetadata()\n> >\n> > \tAwbStatus *awbStatus = rpiMetadata_.getLocked(\"awb.status\");\n> > \tif (awbStatus) {\n> > -\t\tlibcameraMetadata_.set(controls::ColourGains, { static_cast(awbStatus->gainR),\n> > -\t\t\t\t\t\t\t\tstatic_cast(awbStatus->gainB) });\n> > +\t\tlibcameraMetadata_.set(controls::ColourGains,\n> > +\t\t\t\t Span({ static_cast(awbStatus->gainR),\n> > +\t\t\t\t\t\t\t static_cast(awbStatus->gainB) }));\n> > \t\tlibcameraMetadata_.set(controls::ColourTemperature, awbStatus->temperatureK);\n> > \t}\n> >\n> > \tBlackLevelStatus *blackLevelStatus = rpiMetadata_.getLocked(\"black_level.status\");\n> > \tif (blackLevelStatus)\n> > \t\tlibcameraMetadata_.set(controls::SensorBlackLevels,\n> > -\t\t\t\t { static_cast(blackLevelStatus->blackLevelR),\n> > -\t\t\t\t\t static_cast(blackLevelStatus->blackLevelG),\n> > -\t\t\t\t\t static_cast(blackLevelStatus->blackLevelG),\n> > -\t\t\t\t\t static_cast(blackLevelStatus->blackLevelB) });\n> > +\t\t\t\t Span({ static_cast(blackLevelStatus->blackLevelR),\n> > +\t\t\t\t\t\t\t\tstatic_cast(blackLevelStatus->blackLevelG),\n> > +\t\t\t\t\t\t\t\tstatic_cast(blackLevelStatus->blackLevelG),\n> > +\t\t\t\t\t\t\t\tstatic_cast(blackLevelStatus->blackLevelB) }));\n> >\n> > \tFocusStatus *focusStatus = rpiMetadata_.getLocked(\"focus.status\");\n> > \tif (focusStatus && focusStatus->num == 12) {\n> > @@ -883,7 +884,7 @@ void IPARPi::queueRequest(const ControlList &controls)\n> > \t\t\tif (gains[0] != 0.0f && gains[1] != 0.0f)\n> > \t\t\t\t/* A gain of 0.0f will switch back to auto mode. */\n> > \t\t\t\tlibcameraMetadata_.set(controls::ColourGains,\n> > -\t\t\t\t\t\t { gains[0], gains[1] });\n> > +\t\t\t\t\t\t Span({ gains[0], gains[1] }));\n> > \t\t\tbreak;\n> > \t\t}\n> >\n> > @@ -1167,8 +1168,8 @@ void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDur\n> >\n> > \t/* Return the validated limits via metadata. */\n> > \tlibcameraMetadata_.set(controls::FrameDurationLimits,\n> > -\t\t\t { static_cast(minFrameDuration_.get()),\n> > -\t\t\t\t static_cast(maxFrameDuration_.get()) });\n> > +\t\t\t Span({ static_cast(minFrameDuration_.get()),\n> > +\t\t\t\t\t\t\tstatic_cast(maxFrameDuration_.get()) }));\n> >\n> > \t/*\n> > \t * Calculate the maximum exposure time possible for the AGC to use.\n> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > index ecab3ae9..5fa168c6 100644\n> > --- a/src/libcamera/control_ids.yaml\n> > +++ b/src/libcamera/control_ids.yaml\n> > @@ -291,7 +291,7 @@ controls:\n> > transformation. The 3x3 matrix is stored in conventional reading\n> > order in an array of 9 floating point values.\n> >\n> > - size: [3x3]\n> > + size: [3,3]\n> >\n> > - ScalerCrop:\n> > type: Rectangle\n> > @@ -525,7 +525,7 @@ controls:\n> > the window where the focal distance for the objects shown in that part\n> > of the image are closest to the camera.\n> >\n> > - size: [n]\n> > + size: []\n> >\n> > - AfTrigger:\n> > type: int32_t\n> > diff --git a/src/libcamera/property_ids.yaml b/src/libcamera/property_ids.yaml\n> > index 11b7ebdc..a87485d7 100644\n> > --- a/src/libcamera/property_ids.yaml\n> > +++ b/src/libcamera/property_ids.yaml\n> > @@ -497,7 +497,7 @@ controls:\n> >\n> > - PixelArrayOpticalBlackRectangles:\n> > type: Rectangle\n> > - size: [n]\n> > + size: []\n> > description: |\n> > The pixel array region(s) which contain optical black pixels\n> > considered valid for calibration purposes.\n> > @@ -592,7 +592,7 @@ controls:\n> >\n> > - PixelArrayActiveAreas:\n> > type: Rectangle\n> > - size: [n]\n> > + size: []\n> > description: |\n> > The PixelArrayActiveAreas property defines the (possibly multiple and\n> > overlapping) portions of the camera sensor readable pixel matrix\n> > diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n> > index 5b94b3f2..b4362537 100644\n> > --- a/src/qcam/dng_writer.cpp\n> > +++ b/src/qcam/dng_writer.cpp\n> > @@ -517,7 +517,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> >\n> > \tconst auto &blackLevels = metadata.get(controls::SensorBlackLevels);\n> > \tif (blackLevels) {\n> > -\t\tSpan levels = *blackLevels;\n> > +\t\tSpan levels = *blackLevels;\n> >\n> > \t\t/*\n> > \t\t * The black levels control is specified in R, Gr, Gb, B order.\n> > diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> > index 3f99b5e2..46ba4394 100755\n> > --- a/utils/gen-controls.py\n> > +++ b/utils/gen-controls.py\n> > @@ -7,6 +7,8 @@\n> > # gen-controls.py - Generate control definitions from YAML\n> >\n> > import argparse\n> > +from functools import reduce\n> > +import operator\n> > import string\n> > import sys\n> > import yaml\n> > @@ -22,6 +24,24 @@ def format_description(description):\n> > return '\\n'.join([(line and ' * ' or ' *') + line for line in description])\n> >\n> >\n> > +def get_ctrl_type(ctrl):\n> > + ctrl_type = ctrl['type']\n> > + ctrl_size_arr = ctrl.get('size')\n> > +\n> > + if ctrl_type == 'string':\n> > + return 'std::string'\n> > + elif ctrl_size_arr is not None:\n> > + if len(ctrl_size_arr) > 0:\n> > + # fixed-sized Span\n> > + ctrl_span_size = reduce(operator.mul, ctrl_size_arr)\n> > + return f\"Span\"\n> > + else:\n> > + # variable-sized Span\n> > + return f\"Span\"\n> > + else:\n> > + return ctrl_type\n> > +\n> > +\n> > def generate_cpp(controls):\n> > enum_doc_start_template = string.Template('''/**\n> > * \\\\enum ${name}Enum\n> > @@ -50,11 +70,7 @@ ${description}\n> > name, ctrl = ctrl.popitem()\n> > id_name = snake_case(name).upper()\n> >\n> > - ctrl_type = ctrl['type']\n> > - if ctrl_type == 'string':\n> > - ctrl_type = 'std::string'\n> > - elif ctrl.get('size'):\n> > - ctrl_type = 'Span' % ctrl_type\n> > + ctrl_type = get_ctrl_type(ctrl)\n> >\n> > info = {\n> > 'name': name,\n> > @@ -135,11 +151,7 @@ def generate_h(controls):\n> >\n> > ids.append('\\t' + id_name + ' = ' + str(id_value) + ',')\n> >\n> > - ctrl_type = ctrl['type']\n> > - if ctrl_type == 'string':\n> > - ctrl_type = 'std::string'\n> > - elif ctrl.get('size'):\n> > - ctrl_type = 'Span' % ctrl_type\n> > + ctrl_type = get_ctrl_type(ctrl)\n> >\n> > info = {\n> > 'name': name,","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 94199BE173\n\tfor ;\n\tTue, 9 Aug 2022 10:53:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0C7C363328;\n\tTue, 9 Aug 2022 12:53:34 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C8D38600EA\n\tfor ;\n\tTue, 9 Aug 2022 12:53:32 +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 25475481;\n\tTue, 9 Aug 2022 12:53:32 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660042414;\n\tbh=2X36tUBC/teg31p+4Y0d/+Gg5eJJqcIngzQ/8oUlXJ4=;\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=beZFOJ0fwo7LlPTGZx9EmxmtAbTSZOilQtCLij8j5UMdpFyiCA9cWKEZuGeF2dFqS\n\t+TJP6ScZzsRzyArdO/9DEOJ12PSdaMAeYHYzpdZeuq4VH7nnjPMF7NrO2MIYfpATBA\n\tfpfpjt+X13jlaTnm7mMZ9Ye7FN5y3ooci5htHkvdpE+zacMaVaKx5BB0mxx/dPQysx\n\tTqgx1BKSZJsmt/5p7V0ZgC836aoCFmra73CHaEKPL+kivbaMMted3/gqOSQhD6OULx\n\tiUnHQCJlpuSI0G3bDEWcH3+hRcbVuOx0HHmq/U7zo7JzVGm2BunblzI9+PnZhUTH8t\n\t/TllDSqBnrQVQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1660042412;\n\tbh=2X36tUBC/teg31p+4Y0d/+Gg5eJJqcIngzQ/8oUlXJ4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=aEc3c3IJk612rHCEjmBjwSkly5VG31KbTgYq75QDJOkrH0r2RXK+cSlGAbhW6YJFi\n\tC+BIzxRLynP+HDwAtRlSgZsCdRhyxR4+PTUkeFdDCQOl+E3pVJEk3yIZ/scc4F0WK8\n\tn8RC+XAVGQfy9PEY47xMZK2HiONJE9tgPh0a+lQM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"aEc3c3IJ\"; dkim-atps=neutral","Date":"Tue, 9 Aug 2022 13:53:20 +0300","To":"Jacopo Mondi ","Message-ID":"","References":"<20220802210324.115893-1-Rauch.Christian@gmx.de>\n\t<20220803094401.453dofsh22yjpw5z@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220803094401.453dofsh22yjpw5z@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v13] libcamera: controls: Generate and\n\tuse fixed-sized Span types","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]