[{"id":24291,"web_url":"https://patchwork.libcamera.org/comment/24291/","msgid":"<20220802090507.weinbpojvw2cm4ec@uno.localdomain>","date":"2022-08-02T09:05:07","subject":"Re: [libcamera-devel] [PATCH v12] 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 Fri, Jul 29, 2022 at 12:00:24AM +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\nI got this failure when running the series through our buildbot,\nspecifically with gcc8:\nhttps://buildbot.libcamera.org/#/builders/6/builds/430/steps/3/logs/stdio\n\nI'll have a look as well, but I can't reproduce locally to test\neasily.\n\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..7d314ed3 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({ blackLevelStatus->blackLevelR,\n> +\t\t\t\t\t\t\t\tblackLevelStatus->blackLevelG,\n> +\t\t\t\t\t\t\t\tblackLevelStatus->blackLevelG,\n> +\t\t\t\t\t\t\t\tblackLevelStatus->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 3756DC3275\n\tfor ;\n\tTue, 2 Aug 2022 09:05:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9AD5563312;\n\tTue, 2 Aug 2022 11:05:11 +0200 (CEST)","from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::228])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8BB736330D\n\tfor ;\n\tTue, 2 Aug 2022 11:05:10 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id A07DE1BF209;\n\tTue, 2 Aug 2022 09:05:09 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659431111;\n\tbh=LGFSFHHUzydctFyrNAiF3cnxWlIfaZgBMRJgNFdeGE4=;\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=mxgZHc3NF324WTTlZtFzflhlv+T8IRVQFa/0clEGZYOlFfSzF0mgBpzUpQ6jvL02i\n\tru+UA49dScZqExPdZNZNj13sq0zKe9ib5o8AvPNJxZoH2wM+1Q4fxFChI+8/vT/X7B\n\tICtKhoR9djZvQs+m1pTz6q+MHbJN0MBoUqpMpgg0fYWhW7uiJLFSBULJCjhtgRjhxy\n\tbzjBfJe4lgHoZbFWzUpwM0FEuCj+4txQLlLcQpvHtzj2ZKto9PWDvSK+LFQCHShizB\n\t0OssIhkZ8ILpcHEnFcDHAibnxZPKGkVKN/VuYkXJ1HSceC2wEz3+Hn2hq0CPufSx+/\n\tbUqevVC1NqYbQ==","Date":"Tue, 2 Aug 2022 11:05:07 +0200","To":"Christian Rauch ","Message-ID":"<20220802090507.weinbpojvw2cm4ec@uno.localdomain>","References":"<20220728220024.204239-1-Rauch.Christian@gmx.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220728220024.204239-1-Rauch.Christian@gmx.de>","Subject":"Re: [libcamera-devel] [PATCH v12] 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":24316,"web_url":"https://patchwork.libcamera.org/comment/24316/","msgid":"<8e6698be-f130-4aab-133d-73026157fd1b@gmx.de>","date":"2022-08-02T21:10:27","subject":"Re: [libcamera-devel] [PATCH v12] 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 Jacopo,\n\nIs there a way for others to push to that buildbot?\n\nI gave this a shot and sent a v13 with explicit type casts. Can you test\nthis? (I will keep the casts in any case.)\n\nBest,\nChristian\n\n\nAm 02.08.22 um 11:05 schrieb Jacopo Mondi:\n> Hi Christian\n>\n> On Fri, Jul 29, 2022 at 12:00:24AM +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> I got this failure when running the series through our buildbot,\n> specifically with gcc8:\n> https://buildbot.libcamera.org/#/builders/6/builds/430/steps/3/logs/stdio\n>\n> I'll have a look as well, but I can't reproduce locally to test\n> easily.\n>\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..7d314ed3 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({ blackLevelStatus->blackLevelR,\n>> +\t\t\t\t\t\t\t\tblackLevelStatus->blackLevelG,\n>> +\t\t\t\t\t\t\t\tblackLevelStatus->blackLevelG,\n>> +\t\t\t\t\t\t\t\tblackLevelStatus->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 E19D9C3275\n\tfor ;\n\tTue, 2 Aug 2022 21:10:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4CBC86330E;\n\tTue, 2 Aug 2022 23:10:30 +0200 (CEST)","from mout.gmx.net (mout.gmx.net [212.227.17.22])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6FDFE603E7\n\tfor ;\n\tTue, 2 Aug 2022 23:10:28 +0200 (CEST)","from [192.168.0.158] ([88.152.184.103]) by mail.gmx.net (mrgmx104\n\t[212.227.17.168]) with ESMTPSA (Nemesis) id\n\t1MMGNC-1o36hR47Lq-00JKcE for\n\t; Tue, 02 Aug 2022 23:10:28 +0200"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659474630;\n\tbh=HX+BWDXd8HwYFHY8Z/Ube9L8Ck9j6IsOd0ZtUz4x4E0=;\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=egSE95XEVPRqVa7Q7UJUcrw1ZsmK3Lt9ncd7JwqpCcGZJmGYu9tz4qc+p9gFMSzaD\n\tAz9mgwDQo1WUmNbQeMY+BMY8DG/JUvlSggMivamiPPekwTRx8yMNXqOiIQL+4Dim7+\n\txPGA9hYTs+1NQztWnWp46DJdcuoHJk7XRNMvjG8a4HZap1qGHdJaLDcF5TfqVwneDw\n\tAbRjp9vTvvB0DXKvX2vtoPv9LwbuWq2DP1BaTEiTAseZ1IoFgixB0ctBBYuSHaoKse\n\tx4vQAnIu1LXMeoxF10nDY7Ql5hdVEVUg/Bd0hZ23g1CQXlZzNeDGOMMpuW0J56SnjR\n\twlCp9t5bt8+Bg==","v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net;\n\ts=badeba3b8450; t=1659474628;\n\tbh=HX+BWDXd8HwYFHY8Z/Ube9L8Ck9j6IsOd0ZtUz4x4E0=;\n\th=X-UI-Sender-Class:Date:Subject:To:References:From:In-Reply-To;\n\tb=OF+wIZABsScrSZzG+OQ26n/X+M/nmnP7R0Lt85bbUWo7D83ZuqgPoasVFtiZV7RxS\n\t/IDfc/yHJw/2IBSUsAPK6OGbT9R5a1PTYZsUqY2PwaoiEnu0KqzCL3BjqebCOTAIe3\n\tTcGRmgUzmVhD0KnqTWnmi0Q/GKjdq4YNQizjV20g="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=gmx.net header.i=@gmx.net\n\theader.b=\"OF+wIZAB\"; dkim-atps=neutral","X-UI-Sender-Class":"01bb95c1-4bf8-414a-932a-4f6e2808ef9c","Message-ID":"<8e6698be-f130-4aab-133d-73026157fd1b@gmx.de>","Date":"Tue, 2 Aug 2022 23:10:27 +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":"libcamera-devel@lists.libcamera.org","References":"<20220728220024.204239-1-Rauch.Christian@gmx.de>\n\t<20220802090507.weinbpojvw2cm4ec@uno.localdomain>","In-Reply-To":"<20220802090507.weinbpojvw2cm4ec@uno.localdomain>","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"quoted-printable","X-Provags-ID":"V03:K1:LCU62hF5nhmB/vWU4kTY9vhpyFpHx86tqCPXs6WbLiSvgd5b2QM\n\tHjf9fvpNNj86kLvGZIqmBwtvu56LM5xRVhQ68hdSyv4QpdQ3CG72SSbA4rdhMZtYKXjXbaY\n\tDcoNZb3FeZ6NBk+5uR1zt2JcVFQBpSZDIedJcm8mRmUyNzE7YUWBw965RH0qQRclaP2PltF\n\t3zltG1jvpb1b3Ljl/tRYQ==","X-Spam-Flag":"NO","X-UI-Out-Filterresults":"notjunk:1; V03:K0:3qLXghpRms8=:skMazYVBdl/GYpYbRu8ZS0\n\tDelyEE8xuBJL22J2Rtb3x2jdZqcUY+iKuwubGHGDNjx9gwsF08lwLuPe7r9Ay5H0tvTH9B5J2\n\tE19EglHq76p9gMme1ga99tTB0qxKGUz6oNnlWuOK4uxqjpEKmZ8jVZrQ+hec9ll2cufjnrNgm\n\tS9kj7Pn6JfqzBvvwxiFBlVd+GL9TGe+/RnjRSoaOkqn75Xf3oFDqhL8wMwG5DorKvAPcllXlf\n\tdEENZC4fJHMyzL91Qh11FDNs16fWf4OIKIe25q8MzAqtFTNORbVLYt+i95gx6ehe1E2g4yb+H\n\tt9eHvix4xZfXOYoWgNgH2J72LChBizFU2WIYFRmhC8k9ySf+VOPdYI9fmnxBq9jRWAo4xQ338\n\tcAvfKA0mXwXs7q2YJrqSBqj+rHDsKjpBF7aM/ElPIriJhtplKVVN/2MCNRzBOc6xASl3kuWrI\n\tIWE93+0fMt62596MsOPs0E8SXioti9CMtqXUb4rcFuKvbb9LGqjZLuPm4Y+d9JYT75gPleSir\n\tWw00bcv2OJ3kh51bVJoRALK9XCdV1Nj7ci/khVyalXLBj26P8nn9MYSjDsE6UzDbGJplOJ9Sr\n\tB12q4IFYJ7SDIb9lSTYEWDVw1fKKwmm4hsDfDW8+9bzrQgxz5/ucWiS69S0H42wwq8hHbX2vg\n\t6Bk9/u9I8wXSLB7TzPnKPicDygm5MfIe7wQledbfJ10qJHrp9AxRsCzr1lJme5xjMc5iJlmqA\n\tL6YOtjUyFU63EbZU5WELDZZplzPQoso6eG9pXBn+YB2SoM/y74l/injo0yI6D5a8o6OWC6Uex\n\tKOv17bXxe1mrzmFw8syJrT4nMFhjhUwOiuGLOYtdouMb2+7iRmk8GdLTfmETDi/vwL7wCHVFk\n\tLUTUblMeGmHXimxIV9wDMBWWsfYdCo4TJzqGnYOoZ71wYKVcOY4n7V83hl4VyoDLgWetaWF+M\n\t/EsA0bUSogoGwqOr/xgV6HN+Ldc8XJ72CAa0TJTIrpTopcnvmsqmccL3PMM5NVgWpCygs9ZuG\n\t9IE4LVudNT2JR5OqQYx17bpMaKl24IVSJrWBN41B8nwGoFJBWlcR0jrg1MmwlvWDTVRg9scx/\n\tqZfycLzZrCAo+KuTYujGGr/pC+OEzGNwVYhM3scgkIzYq9KvAlGI+f+eg==","Subject":"Re: [libcamera-devel] [PATCH v12] 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":24323,"web_url":"https://patchwork.libcamera.org/comment/24323/","msgid":"<20220803091956.vrxs4shsh5vfup5o@uno.localdomain>","date":"2022-08-03T09:19:56","subject":"Re: [libcamera-devel] [PATCH v12] 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:10:27PM +0200, Christian Rauch via libcamera-devel wrote:\n> Hi Jacopo,\n>\n> Is there a way for others to push to that buildbot?\n>\n\nNot currently, I'm afraid\n\n> I gave this a shot and sent a v13 with explicit type casts. Can you test\n> this? (I will keep the casts in any case.)\n>\n\nThanks, that solves the issue below.\n\nI stomped upon an unrelated failure with YamlParser::getList() (you\nhave received a patch for this) which makes it impossible for me to\nsee a build with gcc8 to complete succesfully, but the control part\nseems now sane.\n\nI'll send a tag to v13 to mention I've tested it.\n\n> Best,\n> Christian\n>\n>\n> Am 02.08.22 um 11:05 schrieb Jacopo Mondi:\n> > Hi Christian\n> >\n> > On Fri, Jul 29, 2022 at 12:00:24AM +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> > I got this failure when running the series through our buildbot,\n> > specifically with gcc8:\n> > https://buildbot.libcamera.org/#/builders/6/builds/430/steps/3/logs/stdio\n> >\n> > I'll have a look as well, but I can't reproduce locally to test\n> > easily.\n> >\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..7d314ed3 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({ blackLevelStatus->blackLevelR,\n> >> +\t\t\t\t\t\t\t\tblackLevelStatus->blackLevelG,\n> >> +\t\t\t\t\t\t\t\tblackLevelStatus->blackLevelG,\n> >> +\t\t\t\t\t\t\t\tblackLevelStatus->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 58BFEC3272\n\tfor ;\n\tWed, 3 Aug 2022 09:20:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9A9C063315;\n\tWed, 3 Aug 2022 11:20:01 +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 B8CA9603EF\n\tfor ;\n\tWed, 3 Aug 2022 11:19:59 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id A3A5C6000D;\n\tWed, 3 Aug 2022 09:19:58 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659518401;\n\tbh=vYL9cJc0dvqRSPkvMufofdJle4dO2UilXCWdCs1CSJQ=;\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=mnRYGArHQV1zPr46toU8lfat9PYv64HB0SuDpR6n1iP5NDAzcVV/4lRYv6HavePdx\n\t7sNEHuYJxNAOLWUH+cNFVXyWbuus33VynU7tfL6hvPXyWOIOZo69M/2NIDxyGzzMiM\n\t+2igFj/TelVhqE0lMFIbdGrDIQT3pn8N8VrmdxjfRtuRageuBaLmjDTxFuwucja98J\n\tzAtWbLjl9I0Ulc4amCaZ7GHyIsTDGWfe4IEF7NPygKlt5t/4yjeo3MVz1TMK9Jwfds\n\to2IuOGCM7kVjN20hghRnrqfSOhWZKBI/W+xpvXyo0fAQcKNIsftbcM/50Hwiji+264\n\tWmQA0UeaNZKhQ==","Date":"Wed, 3 Aug 2022 11:19:56 +0200","To":"Christian Rauch ","Message-ID":"<20220803091956.vrxs4shsh5vfup5o@uno.localdomain>","References":"<20220728220024.204239-1-Rauch.Christian@gmx.de>\n\t<20220802090507.weinbpojvw2cm4ec@uno.localdomain>\n\t<8e6698be-f130-4aab-133d-73026157fd1b@gmx.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<8e6698be-f130-4aab-133d-73026157fd1b@gmx.de>","Subject":"Re: [libcamera-devel] [PATCH v12] 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\" "}}]