[{"id":24515,"web_url":"https://patchwork.libcamera.org/comment/24515/","msgid":"<20220810085032.ngehm4wszyk7ljyy@uno.localdomain>","date":"2022-08-10T08:50:32","subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: controls: Store array\n\tcontrol size in Control class","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n\nOn Wed, Aug 10, 2022 at 03:29:05AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> Baside just identifying a control, the Control class enables usage of\n> the control type for metaprogramming. This is mainly used by the\n> ControlList get() and set() functions to type-check and cast the control\n> value automatically at compilation time.\n>\n> Extend this with a new Size template argument for the Control class that\n> specifies the size of array controls. Use it already in the\n> ControlList::set() overload that takes an std::initializer list to\n> construct the Span with an explicit size, enabling usage of the function\n> for fixed-size array controls.\n>\n> A possible future extension would be to pass the size to the ControlId\n> constructor and store it internally, enabling access to the size at\n> runtime, for instance to perform validity checks.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> include/libcamera/controls.h | 20 +++++++++++--------\n> src/libcamera/controls.cpp | 37 +++++++++++++++++++++++++++---------\n> utils/gen-controls.py | 24 +++++++++++++++++++++--\n> 3 files changed, 62 insertions(+), 19 deletions(-)\n>\n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index ebc168fc28b7..dd474a807d68 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -8,6 +8,7 @@\n> #pragma once\n>\n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -213,6 +214,8 @@ private:\n> class ControlId\n> {\n> public:\n> +\tstatic constexpr size_t dynamic_size = std::numeric_limits::max();\n\nnit: should this be kDynamicSize (the notion we use for constants) ?\n\n> +\n> \tControlId(unsigned int id, const std::string &name, ControlType type)\n> \t\t: id_(id), name_(name), type_(type)\n> \t{\n> @@ -250,11 +253,12 @@ static inline bool operator!=(const ControlId &lhs, unsigned int rhs)\n> \treturn !(lhs == rhs);\n> }\n>\n> -template\n> +template\n\nthis bothers me a bit. Non-array controls are of size 0 for real ? or\nshould they be of size 1 ?\n\nI don't see implications in this patch in having fixed size controls\nwith size set to 1, have I missed them ?\n\n\n> class Control : public ControlId\n> {\n> public:\n> \tusing type = T;\n> +\tstatic constexpr size_t size = Size;\n>\n> \tControl(unsigned int id, const char *name)\n> \t\t: ControlId(id, name, details::control_type>::value)\n> @@ -372,8 +376,8 @@ public:\n>\n> \tbool contains(unsigned int id) const;\n>\n> -\ttemplate\n> -\tstd::optional get(const Control &ctrl) const\n> +\ttemplate\n> +\tstd::optional get(const Control &ctrl) const\n> \t{\n> \t\tconst auto entry = controls_.find(ctrl.id());\n> \t\tif (entry == controls_.end())\n> @@ -383,8 +387,8 @@ public:\n> \t\treturn val.get();\n> \t}\n>\n> -\ttemplate\n> -\tvoid set(const Control &ctrl, const V &value)\n> +\ttemplate\n> +\tvoid set(const Control &ctrl, const V &value)\n> \t{\n> \t\tControlValue *val = find(ctrl.id());\n> \t\tif (!val)\n> @@ -393,14 +397,14 @@ public:\n> \t\tval->set(value);\n> \t}\n>\n> -\ttemplate\n> -\tvoid set(const Control &ctrl, const std::initializer_list &value)\n> +\ttemplate\n> +\tvoid set(const Control &ctrl, const std::initializer_list &value)\n> \t{\n> \t\tControlValue *val = find(ctrl.id());\n> \t\tif (!val)\n> \t\t\treturn;\n>\n> -\t\tval->set(Span>{ value.begin(), value.size() });\n> +\t\tval->set(Span, Size>{ value.begin(), value.size() });\n> \t}\n>\n> \tconst ControlValue &get(unsigned int id) const;\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index bc3db4f69388..24c836382f10 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -384,6 +384,11 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> * Control class for more information.\n> */\n>\n> +/**\n> + * \\var ControlId::dynamic_size\n> + * \\brief Size value for dynamic array controls\n> + */\n> +\n> /**\n> * \\fn ControlId::ControlId(unsigned int id, const std::string &name, ControlType type)\n> * \\brief Construct a ControlId instance\n> @@ -431,12 +436,17 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> /**\n> * \\class Control\n> * \\brief Describe a control and its intrinsic properties\n> + * \\tparam T The control data type\n> + * \\tparam Size The number of elements (for array controls only)\n> *\n> - * The Control class models a control exposed by an object. Its template type\n> - * name T refers to the control data type, and allows functions that operate on\n> - * control values to be defined as template functions using the same type T for\n> - * the control value. See for instance how the ControlList::get() function\n> - * returns a value corresponding to the type of the requested control.\n> + * The Control class models a control exposed by an object. Its template\n> + * paramter \\a T refers to the control data type, and allows functions that\n> + * operate on control values to be defined as template functions using the same\n> + * type T for the control value. See for instance how the ControlList::get()\n> + * function returns a value corresponding to the type of the requested control.\n> + *\n> + * Similarly, for array controls the template parameter \\a Size indicates the\n> + * number of elements in the array.\n> *\n> * While this class is the main means to refer to a control, the control\n> * identifying information is stored in the non-template base ControlId class.\n> @@ -469,6 +479,15 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> * \\brief The Control template type T\n> */\n>\n> +/**\n> + * \\var Control::size\n> + * \\brief The number of elements for array controls\n> + *\n> + * The \\a size reports the number of elements stored by an array Control. It is\n> + * equal to 0 for non-array controls, and to ControlId::dynamic_size for\n> + * variable-size controls.\n> + */\n> +\n> /**\n> * \\class ControlInfo\n> * \\brief Describe the limits of valid values for a Control\n> @@ -943,7 +962,7 @@ bool ControlList::contains(unsigned int id) const\n> }\n>\n> /**\n> - * \\fn ControlList::get(const Control &ctrl) const\n> + * \\fn ControlList::get(const Control &ctrl) const\n> * \\brief Get the value of control \\a ctrl\n> * \\param[in] ctrl The control\n> *\n> @@ -956,7 +975,7 @@ bool ControlList::contains(unsigned int id) const\n> */\n>\n> /**\n> - * \\fn ControlList::set(const Control &ctrl, const V &value)\n> + * \\fn ControlList::set(const Control &ctrl, const V &value)\n> * \\brief Set the control \\a ctrl value to \\a value\n> * \\param[in] ctrl The control\n> * \\param[in] value The control value\n> @@ -970,8 +989,8 @@ bool ControlList::contains(unsigned int id) const\n> */\n>\n> /**\n> - * \\fn ControlList::set(const Control &ctrl, const std::initializer_list &value)\n> - * \\copydoc ControlList::set(const Control &ctrl, const V &value)\n> + * \\fn ControlList::set(const Control &ctrl, const std::initializer_list &value)\n> + * \\copydoc ControlList::set(const Control &ctrl, const V &value)\n> */\n>\n> /**\n> diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> index bcfbeb7f9f17..a3b9c58beec6 100755\n> --- a/utils/gen-controls.py\n> +++ b/utils/gen-controls.py\n> @@ -100,6 +100,10 @@ class Control(object):\n> ns = 'draft::' if self.is_draft else ''\n> return ns + self.__name\n>\n> + @property\n> + def size(self):\n> + return self.__size\n> +\n> @property\n> def type(self):\n> typ = self.__data.get('type')\n> @@ -137,7 +141,7 @@ ${description}''')\n> * \\\\var ${name}\n> ${description}\n> */''')\n> - def_template = string.Template('extern const Control<${type}> ${name}(${id_name}, \"${name}\");')\n> + def_template = string.Template('extern const Control<${type}${size}> ${name}(${id_name}, \"${name}\");')\n> enum_values_doc = string.Template('''/**\n> * \\\\var ${name}Values\n> * \\\\brief List of all $name supported values\n> @@ -154,9 +158,17 @@ ${description}\n> for ctrl in controls:\n> id_name = snake_case(ctrl.name).upper()\n>\n> + if ctrl.size is None:\n> + size = ''\n> + elif ctrl.size == 0:\n> + size = ', ControlId::dynamic_size'\n> + else:\n> + size = f', {ctrl.size}'\n> +\n> info = {\n> 'name': ctrl.name,\n> 'type': ctrl.type,\n> + 'size': size,\n> 'description': format_description(ctrl.description),\n> 'id_name': id_name,\n> }\n> @@ -216,7 +228,7 @@ def generate_h(controls):\n> enum_template_start = string.Template('''enum ${name}Enum {''')\n> enum_value_template = string.Template('''\\t${name} = ${value},''')\n> enum_values_template = string.Template('''extern const std::array ${name}Values;''')\n> - template = string.Template('''extern const Control<${type}> ${name};''')\n> + template = string.Template('''extern const Control<${type}${size}> ${name};''')\n>\n> ctrls = []\n> draft_ctrls = []\n> @@ -228,9 +240,17 @@ def generate_h(controls):\n>\n> ids.append('\\t' + id_name + ' = ' + str(id_value) + ',')\n>\n> + if ctrl.size is None:\n> + size = ''\n> + elif ctrl.size == 0:\n> + size = ', ControlId::dynamic_size'\n> + else:\n> + size = f', {ctrl.size}'\n> +\n> info = {\n> 'name': ctrl.name,\n> 'type': ctrl.type,\n> + 'size': size,\n> }\n>\n> target_ctrls = ctrls\n> --\n> Regards,\n>\n> Laurent Pinchart\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 2467EC3272\n\tfor ;\n\tWed, 10 Aug 2022 08:50:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9AB116332B;\n\tWed, 10 Aug 2022 10:50:36 +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 6986D61FAB\n\tfor ;\n\tWed, 10 Aug 2022 10:50:35 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id D2A6A60006;\n\tWed, 10 Aug 2022 08:50:34 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660121436;\n\tbh=3mEULP/cWiDMrbiGCDHIT9n/uvEZiUtTynyAvtA/M1g=;\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=lqc8xoSe1UwZ0iNTIZIdXAmMJxPbrauf3L5F5dGk8Vh04Avsk9fDhGBYeGR9OuB2N\n\twi8nMf/Q0PUWIHMXfLDM2msDTMfTrlSMTuq7y1SoxrewSDJcAwhD9Mis/P9fehs4dm\n\tY7L4iu4spyih8agIKM1mW70VKtDcXOgB2aiwQYRzdNpGFON6KDbYQ1b7hJ0Nz4fS5f\n\tgUMKZeF6Q2XBbpbLrxfv+UvAG8wDLx3E9ggLmMxsy1KkH3i+H2HGoMmzlI5fFAgWis\n\todkSKVfK8KPbakFv0QciHHMVRXYG2M3IcOVNBi7Mu116QBwYmaTEiDfpjFiZXj98ac\n\t9XKgww6J5uuyA==","Date":"Wed, 10 Aug 2022 10:50:32 +0200","To":"Laurent Pinchart ","Message-ID":"<20220810085032.ngehm4wszyk7ljyy@uno.localdomain>","References":"<20220810002906.5406-1-laurent.pinchart@ideasonboard.com>\n\t<20220810002906.5406-4-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220810002906.5406-4-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: controls: Store array\n\tcontrol size in Control class","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":24526,"web_url":"https://patchwork.libcamera.org/comment/24526/","msgid":"<25a997c7-67f8-0179-d861-65d0b310d4a1@gmx.de>","date":"2022-08-10T21:44:06","subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: controls: Store array\n\tcontrol size in Control class","submitter":{"id":111,"url":"https://patchwork.libcamera.org/api/people/111/","name":"Christian Rauch","email":"Rauch.Christian@gmx.de"},"content":"Dear Laurent,\n\nI don't see yet, how the \"ControlId::dynamic_size\" would be used\nadditionally to the \"Span::extent\". Since the size of the Span is\nalready encoded in the type, what is the benefit of duplicating this\ninformation in the ControlId? Will this information be identical? And\nwhat would be the meaning of \"Control::size\" when its T is a scalar and\nnot a Span?\n\nBest,\nChristian\n\n\nAm 10.08.22 um 02:29 schrieb Laurent Pinchart:\n> Baside just identifying a control, the Control class enables usage of\n> the control type for metaprogramming. This is mainly used by the\n> ControlList get() and set() functions to type-check and cast the control\n> value automatically at compilation time.\n>\n> Extend this with a new Size template argument for the Control class that\n> specifies the size of array controls. Use it already in the\n> ControlList::set() overload that takes an std::initializer list to\n> construct the Span with an explicit size, enabling usage of the function\n> for fixed-size array controls.\n>\n> A possible future extension would be to pass the size to the ControlId\n> constructor and store it internally, enabling access to the size at\n> runtime, for instance to perform validity checks.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> include/libcamera/controls.h | 20 +++++++++++--------\n> src/libcamera/controls.cpp | 37 +++++++++++++++++++++++++++---------\n> utils/gen-controls.py | 24 +++++++++++++++++++++--\n> 3 files changed, 62 insertions(+), 19 deletions(-)\n>\n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index ebc168fc28b7..dd474a807d68 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -8,6 +8,7 @@\n> #pragma once\n>\n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -213,6 +214,8 @@ private:\n> class ControlId\n> {\n> public:\n> +\tstatic constexpr size_t dynamic_size = std::numeric_limits::max();\n> +\n> \tControlId(unsigned int id, const std::string &name, ControlType type)\n> \t\t: id_(id), name_(name), type_(type)\n> \t{\n> @@ -250,11 +253,12 @@ static inline bool operator!=(const ControlId &lhs, unsigned int rhs)\n> \treturn !(lhs == rhs);\n> }\n>\n> -template\n> +template\n> class Control : public ControlId\n> {\n> public:\n> \tusing type = T;\n> +\tstatic constexpr size_t size = Size;\n>\n> \tControl(unsigned int id, const char *name)\n> \t\t: ControlId(id, name, details::control_type>::value)\n> @@ -372,8 +376,8 @@ public:\n>\n> \tbool contains(unsigned int id) const;\n>\n> -\ttemplate\n> -\tstd::optional get(const Control &ctrl) const\n> +\ttemplate\n> +\tstd::optional get(const Control &ctrl) const\n> \t{\n> \t\tconst auto entry = controls_.find(ctrl.id());\n> \t\tif (entry == controls_.end())\n> @@ -383,8 +387,8 @@ public:\n> \t\treturn val.get();\n> \t}\n>\n> -\ttemplate\n> -\tvoid set(const Control &ctrl, const V &value)\n> +\ttemplate\n> +\tvoid set(const Control &ctrl, const V &value)\n> \t{\n> \t\tControlValue *val = find(ctrl.id());\n> \t\tif (!val)\n> @@ -393,14 +397,14 @@ public:\n> \t\tval->set(value);\n> \t}\n>\n> -\ttemplate\n> -\tvoid set(const Control &ctrl, const std::initializer_list &value)\n> +\ttemplate\n> +\tvoid set(const Control &ctrl, const std::initializer_list &value)\n> \t{\n> \t\tControlValue *val = find(ctrl.id());\n> \t\tif (!val)\n> \t\t\treturn;\n>\n> -\t\tval->set(Span>{ value.begin(), value.size() });\n> +\t\tval->set(Span, Size>{ value.begin(), value.size() });\n> \t}\n>\n> \tconst ControlValue &get(unsigned int id) const;\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index bc3db4f69388..24c836382f10 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -384,6 +384,11 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> * Control class for more information.\n> */\n>\n> +/**\n> + * \\var ControlId::dynamic_size\n> + * \\brief Size value for dynamic array controls\n> + */\n> +\n> /**\n> * \\fn ControlId::ControlId(unsigned int id, const std::string &name, ControlType type)\n> * \\brief Construct a ControlId instance\n> @@ -431,12 +436,17 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> /**\n> * \\class Control\n> * \\brief Describe a control and its intrinsic properties\n> + * \\tparam T The control data type\n> + * \\tparam Size The number of elements (for array controls only)\n> *\n> - * The Control class models a control exposed by an object. Its template type\n> - * name T refers to the control data type, and allows functions that operate on\n> - * control values to be defined as template functions using the same type T for\n> - * the control value. See for instance how the ControlList::get() function\n> - * returns a value corresponding to the type of the requested control.\n> + * The Control class models a control exposed by an object. Its template\n> + * paramter \\a T refers to the control data type, and allows functions that\n> + * operate on control values to be defined as template functions using the same\n> + * type T for the control value. See for instance how the ControlList::get()\n> + * function returns a value corresponding to the type of the requested control.\n> + *\n> + * Similarly, for array controls the template parameter \\a Size indicates the\n> + * number of elements in the array.\n> *\n> * While this class is the main means to refer to a control, the control\n> * identifying information is stored in the non-template base ControlId class.\n> @@ -469,6 +479,15 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> * \\brief The Control template type T\n> */\n>\n> +/**\n> + * \\var Control::size\n> + * \\brief The number of elements for array controls\n> + *\n> + * The \\a size reports the number of elements stored by an array Control. It is\n> + * equal to 0 for non-array controls, and to ControlId::dynamic_size for\n> + * variable-size controls.\n> + */\n> +\n> /**\n> * \\class ControlInfo\n> * \\brief Describe the limits of valid values for a Control\n> @@ -943,7 +962,7 @@ bool ControlList::contains(unsigned int id) const\n> }\n>\n> /**\n> - * \\fn ControlList::get(const Control &ctrl) const\n> + * \\fn ControlList::get(const Control &ctrl) const\n> * \\brief Get the value of control \\a ctrl\n> * \\param[in] ctrl The control\n> *\n> @@ -956,7 +975,7 @@ bool ControlList::contains(unsigned int id) const\n> */\n>\n> /**\n> - * \\fn ControlList::set(const Control &ctrl, const V &value)\n> + * \\fn ControlList::set(const Control &ctrl, const V &value)\n> * \\brief Set the control \\a ctrl value to \\a value\n> * \\param[in] ctrl The control\n> * \\param[in] value The control value\n> @@ -970,8 +989,8 @@ bool ControlList::contains(unsigned int id) const\n> */\n>\n> /**\n> - * \\fn ControlList::set(const Control &ctrl, const std::initializer_list &value)\n> - * \\copydoc ControlList::set(const Control &ctrl, const V &value)\n> + * \\fn ControlList::set(const Control &ctrl, const std::initializer_list &value)\n> + * \\copydoc ControlList::set(const Control &ctrl, const V &value)\n> */\n>\n> /**\n> diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> index bcfbeb7f9f17..a3b9c58beec6 100755\n> --- a/utils/gen-controls.py\n> +++ b/utils/gen-controls.py\n> @@ -100,6 +100,10 @@ class Control(object):\n> ns = 'draft::' if self.is_draft else ''\n> return ns + self.__name\n>\n> + @property\n> + def size(self):\n> + return self.__size\n> +\n> @property\n> def type(self):\n> typ = self.__data.get('type')\n> @@ -137,7 +141,7 @@ ${description}''')\n> * \\\\var ${name}\n> ${description}\n> */''')\n> - def_template = string.Template('extern const Control<${type}> ${name}(${id_name}, \"${name}\");')\n> + def_template = string.Template('extern const Control<${type}${size}> ${name}(${id_name}, \"${name}\");')\n> enum_values_doc = string.Template('''/**\n> * \\\\var ${name}Values\n> * \\\\brief List of all $name supported values\n> @@ -154,9 +158,17 @@ ${description}\n> for ctrl in controls:\n> id_name = snake_case(ctrl.name).upper()\n>\n> + if ctrl.size is None:\n> + size = ''\n> + elif ctrl.size == 0:\n> + size = ', ControlId::dynamic_size'\n> + else:\n> + size = f', {ctrl.size}'\n> +\n> info = {\n> 'name': ctrl.name,\n> 'type': ctrl.type,\n> + 'size': size,\n> 'description': format_description(ctrl.description),\n> 'id_name': id_name,\n> }\n> @@ -216,7 +228,7 @@ def generate_h(controls):\n> enum_template_start = string.Template('''enum ${name}Enum {''')\n> enum_value_template = string.Template('''\\t${name} = ${value},''')\n> enum_values_template = string.Template('''extern const std::array ${name}Values;''')\n> - template = string.Template('''extern const Control<${type}> ${name};''')\n> + template = string.Template('''extern const Control<${type}${size}> ${name};''')\n>\n> ctrls = []\n> draft_ctrls = []\n> @@ -228,9 +240,17 @@ def generate_h(controls):\n>\n> ids.append('\\t' + id_name + ' = ' + str(id_value) + ',')\n>\n> + if ctrl.size is None:\n> + size = ''\n> + elif ctrl.size == 0:\n> + size = ', ControlId::dynamic_size'\n> + else:\n> + size = f', {ctrl.size}'\n> +\n> info = {\n> 'name': ctrl.name,\n> 'type': ctrl.type,\n> + 'size': size,\n> }\n>\n> target_ctrls = ctrls","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 8F3E2BE173\n\tfor ;\n\tWed, 10 Aug 2022 21:44:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E06CF63328;\n\tWed, 10 Aug 2022 23:44:09 +0200 (CEST)","from mout.gmx.net (mout.gmx.net [212.227.17.22])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D4F58600EA\n\tfor ;\n\tWed, 10 Aug 2022 23:44:07 +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\t1MVvPJ-1nuhak1IPN-00RtCO for\n\t; Wed, 10 Aug 2022 23:44:07 +0200"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660167849;\n\tbh=xib7U4QyVHxh11L5CK2viPiLk6vJT0qZdzjarjDCwvc=;\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=K+DUPw/dlfTT+hXdivaSdvWy2AyzTa19RnMvw6+nvd63uM3GxZX5uD1SDNYzCJ2Tg\n\tAcQji5AI7C2JVSfowNUcYBn4OlkAMrg+eO7BhHzXYESrqxPGlCajXaOidNA9tTZq5W\n\tl1CwIay7jgW3GDQFXwwI35el6ugLA1Bli13dh9DNN5S1rgwb1uUvW8kihxt4cWe+XU\n\t7EKauBpkSjEGZK8n8XKcM/w4s4HJ2Z9kOzWlfJwxc4cfJJPFBLpQKwmwhp9WuQLnGo\n\ttl8V4c1Vs0Fu1F8tCKGPomaN6Blg9S3ErcTfJPGj/9bJXhvu7wAsdfC8nifN48BFF2\n\t5vfLBp9kIv2Dg==","v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net;\n\ts=badeba3b8450; t=1660167847;\n\tbh=xib7U4QyVHxh11L5CK2viPiLk6vJT0qZdzjarjDCwvc=;\n\th=X-UI-Sender-Class:Date:Subject:To:References:From:In-Reply-To;\n\tb=SzgFRyFhcmRV0KERFgCaUDJVagEqsomRYtLf8mvd6JQP5jgB/WtcdJIYrc/leb0DX\n\thrjC/ITCZtQLfvJLl9eUWsQ4h7rRWepZ39hOTad3PhqKfzPhN+f8nGerJgWvbHHTr2\n\txOTLhSqQ8bnZP0jv2lzFb779POrBGFJ5FzBa0X/Q="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=gmx.net header.i=@gmx.net\n\theader.b=\"SzgFRyFh\"; dkim-atps=neutral","X-UI-Sender-Class":"01bb95c1-4bf8-414a-932a-4f6e2808ef9c","Message-ID":"<25a997c7-67f8-0179-d861-65d0b310d4a1@gmx.de>","Date":"Wed, 10 Aug 2022 23:44:06 +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":"<20220810002906.5406-1-laurent.pinchart@ideasonboard.com>\n\t<20220810002906.5406-4-laurent.pinchart@ideasonboard.com>","In-Reply-To":"<20220810002906.5406-4-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"quoted-printable","X-Provags-ID":"V03:K1:qEpYz+1qiFhtJ5FP1aY+Bq0Ui4rUJFuVwIdTUg5YmwNpwh6RVUk\n\thTf3/yKDQmyU4eXHb1BaOKkk5ys1b87rAHxIoXZV4tI4DVKsgeiJYu3AU2kAo9aPW0mz1fZ\n\tGWxNvMCRkexoNhKTyTxCwhOaS5yyroih59zGTYJFmx84qo0/KvOXc5skiInUYvnnqmP3MKo\n\tm7GbrLkrRpMKPX0gRt6AQ==","X-Spam-Flag":"NO","X-UI-Out-Filterresults":"notjunk:1; V03:K0:5AsCQfA0038=:EDQXa/RZmJNybP4cI6WtdY\n\tVACoZeACtuhW4VPZ1LUHJILHI0KDzCQT/j18UWN+SJbb1F/sUSgMAcAGdhn6nCA5aeT6oTpjD\n\tqvVD5xqeptraExuOhzEw7l0sr+opol0KsqOBSAKZw3eOlRlPB3up3aXQfkjPpgCPXKKsqoCSB\n\t51I7B5VPoXOOWq8FYtL8XbB2EVXviBTko9qj+1rc7oemsKinbfjozHlTpnNIzxVDyWXr7pusr\n\tHIlgvkV4EumxDQUn3JVln5zs+2e4dX+neKPz+Ujyxa9m7w6Rwe6/syhgtRShQvgaUAuaT4Yf5\n\tel4h+S7XYZ7+APnhQralSUyVsdX5K/gMkbLyjmFjoOoIerc+GjYovNjlez2/CjLUX97Hpx78K\n\tROn5DsmUKvT+P0yLD49Wg8GvqyM/cdO6Dw01CrpnGn0kQuSe48PoGzds4TJ5xTjDlcWUUJF0E\n\tzJJQyc3istuZSDdJWDequYdKkBv0QgUj/wIaqnw8G5zmIP9/9oNXYVDHgJ6r0Rhje2sMrx0Si\n\t3DTSzQtHsXb3rN7xCd135jO06FFB5DP2jVkZ1T/zqKP3PaN/W5EGkbSOiaEdOpYifTZrpfdKC\n\tayXHD6eM16P6/QeqFow5M2H1Cc/QclpCFQ85oRH1sSxIcDB59EnLaQCMr47hk/eTQTj10zS7T\n\tS62ZKFsCCTeg32Ve/1gUxnyfJgCW7RfkbLhNMLWKTD4ANzH7x7/cDjWSf7N8CvfeN8A/qXM8V\n\tWCnoOK6d7HmfIFfvWQl9hTP4B7Cas5z5MNeEWEq/2D+WaR8t14UlAZe4jiq4rRGFdhVNjq5yK\n\tw0M7ogtg9Vj1wzARal3yIiZnfQBArj5j5B4/GayZRef3PqoxpVUUcD6EOVsMKTmMEy0sfbfyz\n\tV6UszbmkOhBstDc4qcF8TegVcjs0vkH38N20IFe+Yyr6l2CYQnFrU3eqy3F43OJT4sdbTy3/W\n\tI/bIeBC+4QpCuKLeiaJ+bIw5j6B8HXvHoSaysCPmqzxmTo53uCmZvH8ItrUVmtJgWdVJG0Msi\n\tKNkoTGnCx4r+BU8QAsmkhrgTC1hvY4QNV+CUA/RG0Tcbl4NHeIk1bCd88KZ4Kx0tMmAsAsK2E\n\tLVBxnT8+z8H+6eUlB6P6146wtIybS1b416eqGOsH1MwRiyFrrJLEl6djw==","Subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: controls: Store array\n\tcontrol size in Control class","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":24527,"web_url":"https://patchwork.libcamera.org/comment/24527/","msgid":"","date":"2022-08-10T22:23:18","subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: controls: Store array\n\tcontrol size in Control class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Christian,\n\nOn Wed, Aug 10, 2022 at 11:44:06PM +0200, Christian Rauch via libcamera-devel wrote:\n> Dear Laurent,\n> \n> I don't see yet, how the \"ControlId::dynamic_size\" would be used\n> additionally to the \"Span::extent\".\n\nThe two have the same nuumerical value, but they differ in their intent.\nControlId::dynamic_size is meant to indicate that the array control has\na dynamic size, while utils::dynamic_extent is meant to indicate that a\nspan has a dynamic number of elements. \n\n> Since the size of the Span is\n> already encoded in the type, what is the benefit of duplicating this\n> information in the ControlId? Will this information be identical?\n\nIt allows the ControlList::set() function to access the size, and\nconstruct the Span with the right size, removing the need for explicit\nSpan construction in the caller as shown by patch 4/4.\n\nNow that I'm writing this, I wonder if the following could work\n\n\ttemplate\n\tvoid set(const Control> &ctrl, const std::initializer_list &value)\n\t{\n\t\tControlValue *val = find(ctrl.id());\n\t\tif (!val)\n\t\t\treturn;\n\n\t\tval->set(Span, Size>{ value.begin(), value.size() });\n\t}\n\nwithout adding the Size template argument to the Control class. I'll\ngive it a try.\n\nI'm also tempted to figure out if there would be a way to define a\ncontrol as Control with T being the type of each element, not a\nSpan. Size would be 1 for non-array controls, >1 for array controls with\na fixed size, and std::numeric_limits::max() for array\ncontrols with a dynamic size. I'm not sure of the implications yet, but\nhaving the size available as a template argument of the Control class\nwould allow passing it easily to the ControlId constructor, to make it\navailable at runtime. I think there are use cases for validation for\ninstance.\n\n> And\n> what would be the meaning of \"Control::size\" when its T is a scalar and\n> not a Span?\n\nIt has no meaning in that case, which is indicated by setting it to 0.\nJacopo mentioned that 0 may be a confusing value, I agree with that.\n\n> Am 10.08.22 um 02:29 schrieb Laurent Pinchart:\n> > Baside just identifying a control, the Control class enables usage of\n> > the control type for metaprogramming. This is mainly used by the\n> > ControlList get() and set() functions to type-check and cast the control\n> > value automatically at compilation time.\n> >\n> > Extend this with a new Size template argument for the Control class that\n> > specifies the size of array controls. Use it already in the\n> > ControlList::set() overload that takes an std::initializer list to\n> > construct the Span with an explicit size, enabling usage of the function\n> > for fixed-size array controls.\n> >\n> > A possible future extension would be to pass the size to the ControlId\n> > constructor and store it internally, enabling access to the size at\n> > runtime, for instance to perform validity checks.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > include/libcamera/controls.h | 20 +++++++++++--------\n> > src/libcamera/controls.cpp | 37 +++++++++++++++++++++++++++---------\n> > utils/gen-controls.py | 24 +++++++++++++++++++++--\n> > 3 files changed, 62 insertions(+), 19 deletions(-)\n> >\n> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > index ebc168fc28b7..dd474a807d68 100644\n> > --- a/include/libcamera/controls.h\n> > +++ b/include/libcamera/controls.h\n> > @@ -8,6 +8,7 @@\n> > #pragma once\n> >\n> > #include \n> > +#include \n> > #include \n> > #include \n> > #include \n> > @@ -213,6 +214,8 @@ private:\n> > class ControlId\n> > {\n> > public:\n> > +\tstatic constexpr size_t dynamic_size = std::numeric_limits::max();\n> > +\n> > \tControlId(unsigned int id, const std::string &name, ControlType type)\n> > \t\t: id_(id), name_(name), type_(type)\n> > \t{\n> > @@ -250,11 +253,12 @@ static inline bool operator!=(const ControlId &lhs, unsigned int rhs)\n> > \treturn !(lhs == rhs);\n> > }\n> >\n> > -template\n> > +template\n> > class Control : public ControlId\n> > {\n> > public:\n> > \tusing type = T;\n> > +\tstatic constexpr size_t size = Size;\n> >\n> > \tControl(unsigned int id, const char *name)\n> > \t\t: ControlId(id, name, details::control_type>::value)\n> > @@ -372,8 +376,8 @@ public:\n> >\n> > \tbool contains(unsigned int id) const;\n> >\n> > -\ttemplate\n> > -\tstd::optional get(const Control &ctrl) const\n> > +\ttemplate\n> > +\tstd::optional get(const Control &ctrl) const\n> > \t{\n> > \t\tconst auto entry = controls_.find(ctrl.id());\n> > \t\tif (entry == controls_.end())\n> > @@ -383,8 +387,8 @@ public:\n> > \t\treturn val.get();\n> > \t}\n> >\n> > -\ttemplate\n> > -\tvoid set(const Control &ctrl, const V &value)\n> > +\ttemplate\n> > +\tvoid set(const Control &ctrl, const V &value)\n> > \t{\n> > \t\tControlValue *val = find(ctrl.id());\n> > \t\tif (!val)\n> > @@ -393,14 +397,14 @@ public:\n> > \t\tval->set(value);\n> > \t}\n> >\n> > -\ttemplate\n> > -\tvoid set(const Control &ctrl, const std::initializer_list &value)\n> > +\ttemplate\n> > +\tvoid set(const Control &ctrl, const std::initializer_list &value)\n> > \t{\n> > \t\tControlValue *val = find(ctrl.id());\n> > \t\tif (!val)\n> > \t\t\treturn;\n> >\n> > -\t\tval->set(Span>{ value.begin(), value.size() });\n> > +\t\tval->set(Span, Size>{ value.begin(), value.size() });\n> > \t}\n> >\n> > \tconst ControlValue &get(unsigned int id) const;\n> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > index bc3db4f69388..24c836382f10 100644\n> > --- a/src/libcamera/controls.cpp\n> > +++ b/src/libcamera/controls.cpp\n> > @@ -384,6 +384,11 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> > * Control class for more information.\n> > */\n> >\n> > +/**\n> > + * \\var ControlId::dynamic_size\n> > + * \\brief Size value for dynamic array controls\n> > + */\n> > +\n> > /**\n> > * \\fn ControlId::ControlId(unsigned int id, const std::string &name, ControlType type)\n> > * \\brief Construct a ControlId instance\n> > @@ -431,12 +436,17 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> > /**\n> > * \\class Control\n> > * \\brief Describe a control and its intrinsic properties\n> > + * \\tparam T The control data type\n> > + * \\tparam Size The number of elements (for array controls only)\n> > *\n> > - * The Control class models a control exposed by an object. Its template type\n> > - * name T refers to the control data type, and allows functions that operate on\n> > - * control values to be defined as template functions using the same type T for\n> > - * the control value. See for instance how the ControlList::get() function\n> > - * returns a value corresponding to the type of the requested control.\n> > + * The Control class models a control exposed by an object. Its template\n> > + * paramter \\a T refers to the control data type, and allows functions that\n> > + * operate on control values to be defined as template functions using the same\n> > + * type T for the control value. See for instance how the ControlList::get()\n> > + * function returns a value corresponding to the type of the requested control.\n> > + *\n> > + * Similarly, for array controls the template parameter \\a Size indicates the\n> > + * number of elements in the array.\n> > *\n> > * While this class is the main means to refer to a control, the control\n> > * identifying information is stored in the non-template base ControlId class.\n> > @@ -469,6 +479,15 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> > * \\brief The Control template type T\n> > */\n> >\n> > +/**\n> > + * \\var Control::size\n> > + * \\brief The number of elements for array controls\n> > + *\n> > + * The \\a size reports the number of elements stored by an array Control. It is\n> > + * equal to 0 for non-array controls, and to ControlId::dynamic_size for\n> > + * variable-size controls.\n> > + */\n> > +\n> > /**\n> > * \\class ControlInfo\n> > * \\brief Describe the limits of valid values for a Control\n> > @@ -943,7 +962,7 @@ bool ControlList::contains(unsigned int id) const\n> > }\n> >\n> > /**\n> > - * \\fn ControlList::get(const Control &ctrl) const\n> > + * \\fn ControlList::get(const Control &ctrl) const\n> > * \\brief Get the value of control \\a ctrl\n> > * \\param[in] ctrl The control\n> > *\n> > @@ -956,7 +975,7 @@ bool ControlList::contains(unsigned int id) const\n> > */\n> >\n> > /**\n> > - * \\fn ControlList::set(const Control &ctrl, const V &value)\n> > + * \\fn ControlList::set(const Control &ctrl, const V &value)\n> > * \\brief Set the control \\a ctrl value to \\a value\n> > * \\param[in] ctrl The control\n> > * \\param[in] value The control value\n> > @@ -970,8 +989,8 @@ bool ControlList::contains(unsigned int id) const\n> > */\n> >\n> > /**\n> > - * \\fn ControlList::set(const Control &ctrl, const std::initializer_list &value)\n> > - * \\copydoc ControlList::set(const Control &ctrl, const V &value)\n> > + * \\fn ControlList::set(const Control &ctrl, const std::initializer_list &value)\n> > + * \\copydoc ControlList::set(const Control &ctrl, const V &value)\n> > */\n> >\n> > /**\n> > diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> > index bcfbeb7f9f17..a3b9c58beec6 100755\n> > --- a/utils/gen-controls.py\n> > +++ b/utils/gen-controls.py\n> > @@ -100,6 +100,10 @@ class Control(object):\n> > ns = 'draft::' if self.is_draft else ''\n> > return ns + self.__name\n> >\n> > + @property\n> > + def size(self):\n> > + return self.__size\n> > +\n> > @property\n> > def type(self):\n> > typ = self.__data.get('type')\n> > @@ -137,7 +141,7 @@ ${description}''')\n> > * \\\\var ${name}\n> > ${description}\n> > */''')\n> > - def_template = string.Template('extern const Control<${type}> ${name}(${id_name}, \"${name}\");')\n> > + def_template = string.Template('extern const Control<${type}${size}> ${name}(${id_name}, \"${name}\");')\n> > enum_values_doc = string.Template('''/**\n> > * \\\\var ${name}Values\n> > * \\\\brief List of all $name supported values\n> > @@ -154,9 +158,17 @@ ${description}\n> > for ctrl in controls:\n> > id_name = snake_case(ctrl.name).upper()\n> >\n> > + if ctrl.size is None:\n> > + size = ''\n> > + elif ctrl.size == 0:\n> > + size = ', ControlId::dynamic_size'\n> > + else:\n> > + size = f', {ctrl.size}'\n> > +\n> > info = {\n> > 'name': ctrl.name,\n> > 'type': ctrl.type,\n> > + 'size': size,\n> > 'description': format_description(ctrl.description),\n> > 'id_name': id_name,\n> > }\n> > @@ -216,7 +228,7 @@ def generate_h(controls):\n> > enum_template_start = string.Template('''enum ${name}Enum {''')\n> > enum_value_template = string.Template('''\\t${name} = ${value},''')\n> > enum_values_template = string.Template('''extern const std::array ${name}Values;''')\n> > - template = string.Template('''extern const Control<${type}> ${name};''')\n> > + template = string.Template('''extern const Control<${type}${size}> ${name};''')\n> >\n> > ctrls = []\n> > draft_ctrls = []\n> > @@ -228,9 +240,17 @@ def generate_h(controls):\n> >\n> > ids.append('\\t' + id_name + ' = ' + str(id_value) + ',')\n> >\n> > + if ctrl.size is None:\n> > + size = ''\n> > + elif ctrl.size == 0:\n> > + size = ', ControlId::dynamic_size'\n> > + else:\n> > + size = f', {ctrl.size}'\n> > +\n> > info = {\n> > 'name': ctrl.name,\n> > 'type': ctrl.type,\n> > + 'size': size,\n> > }\n> >\n> > target_ctrls = ctrls","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 D068FC3272\n\tfor ;\n\tWed, 10 Aug 2022 22:23:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 320B76332B;\n\tThu, 11 Aug 2022 00:23:33 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 491EF61FAA\n\tfor ;\n\tThu, 11 Aug 2022 00:23:31 +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 BF9083F1;\n\tThu, 11 Aug 2022 00:23:30 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660170213;\n\tbh=s2C4a9dx1DUqjI5YbGJ2ghvu4xokL7BMopsFxXr9AcA=;\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=T8FNFSGjsggCHhsjY0cPZ3L9W9G4ZeTj/oX50SIjOJqZXh/jWDwtBVzI/f0awEVmT\n\tnNpQSdHtSwiWIlKzL/s7Z4t21GwHXujK2EzK43xjn1/PEP3mT3ETcpHdqImPRy9jlp\n\tVgK88UK8WNulaZ6qpir/DR3hyoIRUJ+XSHs68SnEWAw0zIEaBJm3ZOe79b62YTmZ8v\n\tITt3BXtFMh6JEiKLDswVMaP3HY7Rt+ZcNP7r9ZB1VVJBpJLN7cIXR143nsKhk2bnZ8\n\tSMILLLoZtupf57os6l0zbM+wjuc5FVoTpTtzaF5Sts7X4zodmQac3D7f3GZMlscGo6\n\tmjyYVR0Z1T4aA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1660170210;\n\tbh=s2C4a9dx1DUqjI5YbGJ2ghvu4xokL7BMopsFxXr9AcA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=buVKadwiHHAExZm7SEqjerUgRjdg1gle9uViK40qBokN7QgjQCDYMWadKnDaJg5W1\n\tIeDN3VjPU78Cgf/Ru7xuUZnzVvqzq9pxz4Jxah9IVdYtUbUeb+p2fy4Ps54fYMxbb8\n\tgGvOXi8uIjMbH8kCztW8wU0S40gyGD4EsKVVds7k="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"buVKadwi\"; dkim-atps=neutral","Date":"Thu, 11 Aug 2022 01:23:18 +0300","To":"Christian Rauch ","Message-ID":"","References":"<20220810002906.5406-1-laurent.pinchart@ideasonboard.com>\n\t<20220810002906.5406-4-laurent.pinchart@ideasonboard.com>\n\t<25a997c7-67f8-0179-d861-65d0b310d4a1@gmx.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<25a997c7-67f8-0179-d861-65d0b310d4a1@gmx.de>","Subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: controls: Store array\n\tcontrol size in Control class","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\" "}},{"id":24530,"web_url":"https://patchwork.libcamera.org/comment/24530/","msgid":"","date":"2022-08-10T22:33:36","subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: controls: Store array\n\tcontrol size in Control class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Wed, Aug 10, 2022 at 10:50:32AM +0200, Jacopo Mondi wrote:\n> On Wed, Aug 10, 2022 at 03:29:05AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> > Baside just identifying a control, the Control class enables usage of\n> > the control type for metaprogramming. This is mainly used by the\n> > ControlList get() and set() functions to type-check and cast the control\n> > value automatically at compilation time.\n> >\n> > Extend this with a new Size template argument for the Control class that\n> > specifies the size of array controls. Use it already in the\n> > ControlList::set() overload that takes an std::initializer list to\n> > construct the Span with an explicit size, enabling usage of the function\n> > for fixed-size array controls.\n> >\n> > A possible future extension would be to pass the size to the ControlId\n> > constructor and store it internally, enabling access to the size at\n> > runtime, for instance to perform validity checks.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > include/libcamera/controls.h | 20 +++++++++++--------\n> > src/libcamera/controls.cpp | 37 +++++++++++++++++++++++++++---------\n> > utils/gen-controls.py | 24 +++++++++++++++++++++--\n> > 3 files changed, 62 insertions(+), 19 deletions(-)\n> >\n> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > index ebc168fc28b7..dd474a807d68 100644\n> > --- a/include/libcamera/controls.h\n> > +++ b/include/libcamera/controls.h\n> > @@ -8,6 +8,7 @@\n> > #pragma once\n> >\n> > #include \n> > +#include \n> > #include \n> > #include \n> > #include \n> > @@ -213,6 +214,8 @@ private:\n> > class ControlId\n> > {\n> > public:\n> > +\tstatic constexpr size_t dynamic_size = std::numeric_limits::max();\n> \n> nit: should this be kDynamicSize (the notion we use for constants) ?\n\nYes, I modelled it after utils::dynamic_extent, which comes from the\nstd::dynamic_extent of C++20, but here there are less justifications\nfor departing from the libcamera coding style.\n\n> > +\n> > \tControlId(unsigned int id, const std::string &name, ControlType type)\n> > \t\t: id_(id), name_(name), type_(type)\n> > \t{\n> > @@ -250,11 +253,12 @@ static inline bool operator!=(const ControlId &lhs, unsigned int rhs)\n> > \treturn !(lhs == rhs);\n> > }\n> >\n> > -template\n> > +template\n> \n> this bothers me a bit. Non-array controls are of size 0 for real ? or\n> should they be of size 1 ?\n> \n> I don't see implications in this patch in having fixed size controls\n> with size set to 1, have I missed them ?\n\nThat would probably work, I can give it a try. I'm wondering, though, if\nthere's a need to distinguish arrays that have a single element from\nnon-array controls. I don't really see a reason why we would define an\narray control with a fixed size set to 1 though.\n\n> > class Control : public ControlId\n> > {\n> > public:\n> > \tusing type = T;\n> > +\tstatic constexpr size_t size = Size;\n> >\n> > \tControl(unsigned int id, const char *name)\n> > \t\t: ControlId(id, name, details::control_type>::value)\n> > @@ -372,8 +376,8 @@ public:\n> >\n> > \tbool contains(unsigned int id) const;\n> >\n> > -\ttemplate\n> > -\tstd::optional get(const Control &ctrl) const\n> > +\ttemplate\n> > +\tstd::optional get(const Control &ctrl) const\n> > \t{\n> > \t\tconst auto entry = controls_.find(ctrl.id());\n> > \t\tif (entry == controls_.end())\n> > @@ -383,8 +387,8 @@ public:\n> > \t\treturn val.get();\n> > \t}\n> >\n> > -\ttemplate\n> > -\tvoid set(const Control &ctrl, const V &value)\n> > +\ttemplate\n> > +\tvoid set(const Control &ctrl, const V &value)\n> > \t{\n> > \t\tControlValue *val = find(ctrl.id());\n> > \t\tif (!val)\n> > @@ -393,14 +397,14 @@ public:\n> > \t\tval->set(value);\n> > \t}\n> >\n> > -\ttemplate\n> > -\tvoid set(const Control &ctrl, const std::initializer_list &value)\n> > +\ttemplate\n> > +\tvoid set(const Control &ctrl, const std::initializer_list &value)\n> > \t{\n> > \t\tControlValue *val = find(ctrl.id());\n> > \t\tif (!val)\n> > \t\t\treturn;\n> >\n> > -\t\tval->set(Span>{ value.begin(), value.size() });\n> > +\t\tval->set(Span, Size>{ value.begin(), value.size() });\n> > \t}\n> >\n> > \tconst ControlValue &get(unsigned int id) const;\n> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > index bc3db4f69388..24c836382f10 100644\n> > --- a/src/libcamera/controls.cpp\n> > +++ b/src/libcamera/controls.cpp\n> > @@ -384,6 +384,11 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> > * Control class for more information.\n> > */\n> >\n> > +/**\n> > + * \\var ControlId::dynamic_size\n> > + * \\brief Size value for dynamic array controls\n> > + */\n> > +\n> > /**\n> > * \\fn ControlId::ControlId(unsigned int id, const std::string &name, ControlType type)\n> > * \\brief Construct a ControlId instance\n> > @@ -431,12 +436,17 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> > /**\n> > * \\class Control\n> > * \\brief Describe a control and its intrinsic properties\n> > + * \\tparam T The control data type\n> > + * \\tparam Size The number of elements (for array controls only)\n> > *\n> > - * The Control class models a control exposed by an object. Its template type\n> > - * name T refers to the control data type, and allows functions that operate on\n> > - * control values to be defined as template functions using the same type T for\n> > - * the control value. See for instance how the ControlList::get() function\n> > - * returns a value corresponding to the type of the requested control.\n> > + * The Control class models a control exposed by an object. Its template\n> > + * paramter \\a T refers to the control data type, and allows functions that\n> > + * operate on control values to be defined as template functions using the same\n> > + * type T for the control value. See for instance how the ControlList::get()\n> > + * function returns a value corresponding to the type of the requested control.\n> > + *\n> > + * Similarly, for array controls the template parameter \\a Size indicates the\n> > + * number of elements in the array.\n> > *\n> > * While this class is the main means to refer to a control, the control\n> > * identifying information is stored in the non-template base ControlId class.\n> > @@ -469,6 +479,15 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> > * \\brief The Control template type T\n> > */\n> >\n> > +/**\n> > + * \\var Control::size\n> > + * \\brief The number of elements for array controls\n> > + *\n> > + * The \\a size reports the number of elements stored by an array Control. It is\n> > + * equal to 0 for non-array controls, and to ControlId::dynamic_size for\n> > + * variable-size controls.\n> > + */\n> > +\n> > /**\n> > * \\class ControlInfo\n> > * \\brief Describe the limits of valid values for a Control\n> > @@ -943,7 +962,7 @@ bool ControlList::contains(unsigned int id) const\n> > }\n> >\n> > /**\n> > - * \\fn ControlList::get(const Control &ctrl) const\n> > + * \\fn ControlList::get(const Control &ctrl) const\n> > * \\brief Get the value of control \\a ctrl\n> > * \\param[in] ctrl The control\n> > *\n> > @@ -956,7 +975,7 @@ bool ControlList::contains(unsigned int id) const\n> > */\n> >\n> > /**\n> > - * \\fn ControlList::set(const Control &ctrl, const V &value)\n> > + * \\fn ControlList::set(const Control &ctrl, const V &value)\n> > * \\brief Set the control \\a ctrl value to \\a value\n> > * \\param[in] ctrl The control\n> > * \\param[in] value The control value\n> > @@ -970,8 +989,8 @@ bool ControlList::contains(unsigned int id) const\n> > */\n> >\n> > /**\n> > - * \\fn ControlList::set(const Control &ctrl, const std::initializer_list &value)\n> > - * \\copydoc ControlList::set(const Control &ctrl, const V &value)\n> > + * \\fn ControlList::set(const Control &ctrl, const std::initializer_list &value)\n> > + * \\copydoc ControlList::set(const Control &ctrl, const V &value)\n> > */\n> >\n> > /**\n> > diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> > index bcfbeb7f9f17..a3b9c58beec6 100755\n> > --- a/utils/gen-controls.py\n> > +++ b/utils/gen-controls.py\n> > @@ -100,6 +100,10 @@ class Control(object):\n> > ns = 'draft::' if self.is_draft else ''\n> > return ns + self.__name\n> >\n> > + @property\n> > + def size(self):\n> > + return self.__size\n> > +\n> > @property\n> > def type(self):\n> > typ = self.__data.get('type')\n> > @@ -137,7 +141,7 @@ ${description}''')\n> > * \\\\var ${name}\n> > ${description}\n> > */''')\n> > - def_template = string.Template('extern const Control<${type}> ${name}(${id_name}, \"${name}\");')\n> > + def_template = string.Template('extern const Control<${type}${size}> ${name}(${id_name}, \"${name}\");')\n> > enum_values_doc = string.Template('''/**\n> > * \\\\var ${name}Values\n> > * \\\\brief List of all $name supported values\n> > @@ -154,9 +158,17 @@ ${description}\n> > for ctrl in controls:\n> > id_name = snake_case(ctrl.name).upper()\n> >\n> > + if ctrl.size is None:\n> > + size = ''\n> > + elif ctrl.size == 0:\n> > + size = ', ControlId::dynamic_size'\n> > + else:\n> > + size = f', {ctrl.size}'\n> > +\n> > info = {\n> > 'name': ctrl.name,\n> > 'type': ctrl.type,\n> > + 'size': size,\n> > 'description': format_description(ctrl.description),\n> > 'id_name': id_name,\n> > }\n> > @@ -216,7 +228,7 @@ def generate_h(controls):\n> > enum_template_start = string.Template('''enum ${name}Enum {''')\n> > enum_value_template = string.Template('''\\t${name} = ${value},''')\n> > enum_values_template = string.Template('''extern const std::array ${name}Values;''')\n> > - template = string.Template('''extern const Control<${type}> ${name};''')\n> > + template = string.Template('''extern const Control<${type}${size}> ${name};''')\n> >\n> > ctrls = []\n> > draft_ctrls = []\n> > @@ -228,9 +240,17 @@ def generate_h(controls):\n> >\n> > ids.append('\\t' + id_name + ' = ' + str(id_value) + ',')\n> >\n> > + if ctrl.size is None:\n> > + size = ''\n> > + elif ctrl.size == 0:\n> > + size = ', ControlId::dynamic_size'\n> > + else:\n> > + size = f', {ctrl.size}'\n> > +\n> > info = {\n> > 'name': ctrl.name,\n> > 'type': ctrl.type,\n> > + 'size': size,\n> > }\n> >\n> > target_ctrls = ctrls","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 6001AC3272\n\tfor ;\n\tWed, 10 Aug 2022 22:33:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BE2176332B;\n\tThu, 11 Aug 2022 00:33:50 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 305A061FAA\n\tfor ;\n\tThu, 11 Aug 2022 00:33:49 +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 964993F1;\n\tThu, 11 Aug 2022 00:33:48 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660170830;\n\tbh=VfK/lwFZvAZ6npAknFONrHFnEd1q5Pc8TugiAsx2Fws=;\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=hSBhk0KRJwBWCJI71Tai0K8ieRhGisndjLXoY9JwJ1AnIGH8491EnKZorV3iUC6dw\n\tTdYInkb0GehUjMR1bdT0J5RWFk5tVrBOdgUVbBdWORx7+S5navWgVp6FaLPXwdTj05\n\tOZBUiSd9VcOwaUxF1bsRX/RdeVmaMZuJOvFSg/IsHbJBRqxCUVxrLKr2+zUDq9fkwu\n\tOmndzSqJn/FcqN3vSHUDfeBf9LR6xwfksqkpCPdVwmNRPDQwTmyW7fB+JrEHTVZx97\n\tGLo3BKKm5xjXSCnHchAxDjeGHyhf0L1dGxXLHOyQYkRVs6uywJcCsRB1zCpr4LBCIB\n\tYwTW4prvch84A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1660170828;\n\tbh=VfK/lwFZvAZ6npAknFONrHFnEd1q5Pc8TugiAsx2Fws=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=InO/U8hcaeScIMZUfPZLOJSg2bhEe/E3b/up6wkqA5LpLExnK5NFQ0otFIVdrQkNY\n\tAMhmUuRB6NZybIsXkFqqwDj7tXnPaMxa/9Pl3uWeulUYYCW/DFxaJlqojubftz1h50\n\tRL5NSE6XMi91npriVFt5AUe311rjcOF6WGGTTlkQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"InO/U8hc\"; dkim-atps=neutral","Date":"Thu, 11 Aug 2022 01:33:36 +0300","To":"Jacopo Mondi ","Message-ID":"","References":"<20220810002906.5406-1-laurent.pinchart@ideasonboard.com>\n\t<20220810002906.5406-4-laurent.pinchart@ideasonboard.com>\n\t<20220810085032.ngehm4wszyk7ljyy@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220810085032.ngehm4wszyk7ljyy@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: controls: Store array\n\tcontrol size in Control class","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\" "}},{"id":24531,"web_url":"https://patchwork.libcamera.org/comment/24531/","msgid":"","date":"2022-08-10T22:52:58","subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: controls: Store array\n\tcontrol size in Control class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Christian,\n\nOn Thu, Aug 11, 2022 at 01:23:18AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> On Wed, Aug 10, 2022 at 11:44:06PM +0200, Christian Rauch via libcamera-devel wrote:\n> > Dear Laurent,\n> > \n> > I don't see yet, how the \"ControlId::dynamic_size\" would be used\n> > additionally to the \"Span::extent\".\n> \n> The two have the same nuumerical value, but they differ in their intent.\n> ControlId::dynamic_size is meant to indicate that the array control has\n> a dynamic size, while utils::dynamic_extent is meant to indicate that a\n> span has a dynamic number of elements. \n> \n> > Since the size of the Span is\n> > already encoded in the type, what is the benefit of duplicating this\n> > information in the ControlId? Will this information be identical?\n> \n> It allows the ControlList::set() function to access the size, and\n> construct the Span with the right size, removing the need for explicit\n> Span construction in the caller as shown by patch 4/4.\n> \n> Now that I'm writing this, I wonder if the following could work\n> \n> \ttemplate\n> \tvoid set(const Control> &ctrl, const std::initializer_list &value)\n> \t{\n> \t\tControlValue *val = find(ctrl.id());\n> \t\tif (!val)\n> \t\t\treturn;\n> \n> \t\tval->set(Span, Size>{ value.begin(), value.size() });\n> \t}\n> \n> without adding the Size template argument to the Control class. I'll\n> give it a try.\n\nI can replace this patch with\n\ndiff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\nindex ebc168fc28b7..2305202c4570 100644\n--- a/include/libcamera/controls.h\n+++ b/include/libcamera/controls.h\n@@ -393,14 +393,14 @@ public:\n \t\tval->set(value);\n \t}\n\n-\ttemplate\n-\tvoid set(const Control &ctrl, const std::initializer_list &value)\n+\ttemplate\n+\tvoid set(const Control> &ctrl, const std::initializer_list &value)\n \t{\n \t\tControlValue *val = find(ctrl.id());\n \t\tif (!val)\n \t\t\treturn;\n\n-\t\tval->set(Span>{ value.begin(), value.size() });\n+\t\tval->set(Span, Size>{ value.begin(), value.size() });\n \t}\n\n \tconst ControlValue &get(unsigned int id) const;\ndiff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\nindex bc3db4f69388..6dbf9b348709 100644\n--- a/src/libcamera/controls.cpp\n+++ b/src/libcamera/controls.cpp\n@@ -970,7 +970,7 @@ bool ControlList::contains(unsigned int id) const\n */\n\n /**\n- * \\fn ControlList::set(const Control &ctrl, const std::initializer_list &value)\n+ * \\fn ControlList::set(const Control> &ctrl, const std::initializer_list &value)\n * \\copydoc ControlList::set(const Control &ctrl, const V &value)\n */\n\nand 4/4 still compiles. I think I'll go with this for the time being. I\nmay revive the idea of adding a size directly to the Control class in\nthe future when a use case will arise though, but that's for later.\n\n> I'm also tempted to figure out if there would be a way to define a\n> control as Control with T being the type of each element, not a\n> Span. Size would be 1 for non-array controls, >1 for array controls with\n> a fixed size, and std::numeric_limits::max() for array\n> controls with a dynamic size. I'm not sure of the implications yet, but\n> having the size available as a template argument of the Control class\n> would allow passing it easily to the ControlId constructor, to make it\n> available at runtime. I think there are use cases for validation for\n> instance.\n> \n> > And\n> > what would be the meaning of \"Control::size\" when its T is a scalar and\n> > not a Span?\n> \n> It has no meaning in that case, which is indicated by setting it to 0.\n> Jacopo mentioned that 0 may be a confusing value, I agree with that.\n> \n> > Am 10.08.22 um 02:29 schrieb Laurent Pinchart:\n> > > Baside just identifying a control, the Control class enables usage of\n> > > the control type for metaprogramming. This is mainly used by the\n> > > ControlList get() and set() functions to type-check and cast the control\n> > > value automatically at compilation time.\n> > >\n> > > Extend this with a new Size template argument for the Control class that\n> > > specifies the size of array controls. Use it already in the\n> > > ControlList::set() overload that takes an std::initializer list to\n> > > construct the Span with an explicit size, enabling usage of the function\n> > > for fixed-size array controls.\n> > >\n> > > A possible future extension would be to pass the size to the ControlId\n> > > constructor and store it internally, enabling access to the size at\n> > > runtime, for instance to perform validity checks.\n> > >\n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > include/libcamera/controls.h | 20 +++++++++++--------\n> > > src/libcamera/controls.cpp | 37 +++++++++++++++++++++++++++---------\n> > > utils/gen-controls.py | 24 +++++++++++++++++++++--\n> > > 3 files changed, 62 insertions(+), 19 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > > index ebc168fc28b7..dd474a807d68 100644\n> > > --- a/include/libcamera/controls.h\n> > > +++ b/include/libcamera/controls.h\n> > > @@ -8,6 +8,7 @@\n> > > #pragma once\n> > >\n> > > #include \n> > > +#include \n> > > #include \n> > > #include \n> > > #include \n> > > @@ -213,6 +214,8 @@ private:\n> > > class ControlId\n> > > {\n> > > public:\n> > > +\tstatic constexpr size_t dynamic_size = std::numeric_limits::max();\n> > > +\n> > > \tControlId(unsigned int id, const std::string &name, ControlType type)\n> > > \t\t: id_(id), name_(name), type_(type)\n> > > \t{\n> > > @@ -250,11 +253,12 @@ static inline bool operator!=(const ControlId &lhs, unsigned int rhs)\n> > > \treturn !(lhs == rhs);\n> > > }\n> > >\n> > > -template\n> > > +template\n> > > class Control : public ControlId\n> > > {\n> > > public:\n> > > \tusing type = T;\n> > > +\tstatic constexpr size_t size = Size;\n> > >\n> > > \tControl(unsigned int id, const char *name)\n> > > \t\t: ControlId(id, name, details::control_type>::value)\n> > > @@ -372,8 +376,8 @@ public:\n> > >\n> > > \tbool contains(unsigned int id) const;\n> > >\n> > > -\ttemplate\n> > > -\tstd::optional get(const Control &ctrl) const\n> > > +\ttemplate\n> > > +\tstd::optional get(const Control &ctrl) const\n> > > \t{\n> > > \t\tconst auto entry = controls_.find(ctrl.id());\n> > > \t\tif (entry == controls_.end())\n> > > @@ -383,8 +387,8 @@ public:\n> > > \t\treturn val.get();\n> > > \t}\n> > >\n> > > -\ttemplate\n> > > -\tvoid set(const Control &ctrl, const V &value)\n> > > +\ttemplate\n> > > +\tvoid set(const Control &ctrl, const V &value)\n> > > \t{\n> > > \t\tControlValue *val = find(ctrl.id());\n> > > \t\tif (!val)\n> > > @@ -393,14 +397,14 @@ public:\n> > > \t\tval->set(value);\n> > > \t}\n> > >\n> > > -\ttemplate\n> > > -\tvoid set(const Control &ctrl, const std::initializer_list &value)\n> > > +\ttemplate\n> > > +\tvoid set(const Control &ctrl, const std::initializer_list &value)\n> > > \t{\n> > > \t\tControlValue *val = find(ctrl.id());\n> > > \t\tif (!val)\n> > > \t\t\treturn;\n> > >\n> > > -\t\tval->set(Span>{ value.begin(), value.size() });\n> > > +\t\tval->set(Span, Size>{ value.begin(), value.size() });\n> > > \t}\n> > >\n> > > \tconst ControlValue &get(unsigned int id) const;\n> > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > > index bc3db4f69388..24c836382f10 100644\n> > > --- a/src/libcamera/controls.cpp\n> > > +++ b/src/libcamera/controls.cpp\n> > > @@ -384,6 +384,11 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> > > * Control class for more information.\n> > > */\n> > >\n> > > +/**\n> > > + * \\var ControlId::dynamic_size\n> > > + * \\brief Size value for dynamic array controls\n> > > + */\n> > > +\n> > > /**\n> > > * \\fn ControlId::ControlId(unsigned int id, const std::string &name, ControlType type)\n> > > * \\brief Construct a ControlId instance\n> > > @@ -431,12 +436,17 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> > > /**\n> > > * \\class Control\n> > > * \\brief Describe a control and its intrinsic properties\n> > > + * \\tparam T The control data type\n> > > + * \\tparam Size The number of elements (for array controls only)\n> > > *\n> > > - * The Control class models a control exposed by an object. Its template type\n> > > - * name T refers to the control data type, and allows functions that operate on\n> > > - * control values to be defined as template functions using the same type T for\n> > > - * the control value. See for instance how the ControlList::get() function\n> > > - * returns a value corresponding to the type of the requested control.\n> > > + * The Control class models a control exposed by an object. Its template\n> > > + * paramter \\a T refers to the control data type, and allows functions that\n> > > + * operate on control values to be defined as template functions using the same\n> > > + * type T for the control value. See for instance how the ControlList::get()\n> > > + * function returns a value corresponding to the type of the requested control.\n> > > + *\n> > > + * Similarly, for array controls the template parameter \\a Size indicates the\n> > > + * number of elements in the array.\n> > > *\n> > > * While this class is the main means to refer to a control, the control\n> > > * identifying information is stored in the non-template base ControlId class.\n> > > @@ -469,6 +479,15 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> > > * \\brief The Control template type T\n> > > */\n> > >\n> > > +/**\n> > > + * \\var Control::size\n> > > + * \\brief The number of elements for array controls\n> > > + *\n> > > + * The \\a size reports the number of elements stored by an array Control. It is\n> > > + * equal to 0 for non-array controls, and to ControlId::dynamic_size for\n> > > + * variable-size controls.\n> > > + */\n> > > +\n> > > /**\n> > > * \\class ControlInfo\n> > > * \\brief Describe the limits of valid values for a Control\n> > > @@ -943,7 +962,7 @@ bool ControlList::contains(unsigned int id) const\n> > > }\n> > >\n> > > /**\n> > > - * \\fn ControlList::get(const Control &ctrl) const\n> > > + * \\fn ControlList::get(const Control &ctrl) const\n> > > * \\brief Get the value of control \\a ctrl\n> > > * \\param[in] ctrl The control\n> > > *\n> > > @@ -956,7 +975,7 @@ bool ControlList::contains(unsigned int id) const\n> > > */\n> > >\n> > > /**\n> > > - * \\fn ControlList::set(const Control &ctrl, const V &value)\n> > > + * \\fn ControlList::set(const Control &ctrl, const V &value)\n> > > * \\brief Set the control \\a ctrl value to \\a value\n> > > * \\param[in] ctrl The control\n> > > * \\param[in] value The control value\n> > > @@ -970,8 +989,8 @@ bool ControlList::contains(unsigned int id) const\n> > > */\n> > >\n> > > /**\n> > > - * \\fn ControlList::set(const Control &ctrl, const std::initializer_list &value)\n> > > - * \\copydoc ControlList::set(const Control &ctrl, const V &value)\n> > > + * \\fn ControlList::set(const Control &ctrl, const std::initializer_list &value)\n> > > + * \\copydoc ControlList::set(const Control &ctrl, const V &value)\n> > > */\n> > >\n> > > /**\n> > > diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> > > index bcfbeb7f9f17..a3b9c58beec6 100755\n> > > --- a/utils/gen-controls.py\n> > > +++ b/utils/gen-controls.py\n> > > @@ -100,6 +100,10 @@ class Control(object):\n> > > ns = 'draft::' if self.is_draft else ''\n> > > return ns + self.__name\n> > >\n> > > + @property\n> > > + def size(self):\n> > > + return self.__size\n> > > +\n> > > @property\n> > > def type(self):\n> > > typ = self.__data.get('type')\n> > > @@ -137,7 +141,7 @@ ${description}''')\n> > > * \\\\var ${name}\n> > > ${description}\n> > > */''')\n> > > - def_template = string.Template('extern const Control<${type}> ${name}(${id_name}, \"${name}\");')\n> > > + def_template = string.Template('extern const Control<${type}${size}> ${name}(${id_name}, \"${name}\");')\n> > > enum_values_doc = string.Template('''/**\n> > > * \\\\var ${name}Values\n> > > * \\\\brief List of all $name supported values\n> > > @@ -154,9 +158,17 @@ ${description}\n> > > for ctrl in controls:\n> > > id_name = snake_case(ctrl.name).upper()\n> > >\n> > > + if ctrl.size is None:\n> > > + size = ''\n> > > + elif ctrl.size == 0:\n> > > + size = ', ControlId::dynamic_size'\n> > > + else:\n> > > + size = f', {ctrl.size}'\n> > > +\n> > > info = {\n> > > 'name': ctrl.name,\n> > > 'type': ctrl.type,\n> > > + 'size': size,\n> > > 'description': format_description(ctrl.description),\n> > > 'id_name': id_name,\n> > > }\n> > > @@ -216,7 +228,7 @@ def generate_h(controls):\n> > > enum_template_start = string.Template('''enum ${name}Enum {''')\n> > > enum_value_template = string.Template('''\\t${name} = ${value},''')\n> > > enum_values_template = string.Template('''extern const std::array ${name}Values;''')\n> > > - template = string.Template('''extern const Control<${type}> ${name};''')\n> > > + template = string.Template('''extern const Control<${type}${size}> ${name};''')\n> > >\n> > > ctrls = []\n> > > draft_ctrls = []\n> > > @@ -228,9 +240,17 @@ def generate_h(controls):\n> > >\n> > > ids.append('\\t' + id_name + ' = ' + str(id_value) + ',')\n> > >\n> > > + if ctrl.size is None:\n> > > + size = ''\n> > > + elif ctrl.size == 0:\n> > > + size = ', ControlId::dynamic_size'\n> > > + else:\n> > > + size = f', {ctrl.size}'\n> > > +\n> > > info = {\n> > > 'name': ctrl.name,\n> > > 'type': ctrl.type,\n> > > + 'size': size,\n> > > }\n> > >\n> > > target_ctrls = ctrls","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 47D07BE173\n\tfor ;\n\tWed, 10 Aug 2022 22:53:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AFEDB6332B;\n\tThu, 11 Aug 2022 00:53:12 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 474DF61FA8\n\tfor ;\n\tThu, 11 Aug 2022 00:53:11 +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 ACEE53F1;\n\tThu, 11 Aug 2022 00:53:10 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660171992;\n\tbh=qj/rD0vH6jUkrceu8uWXFLp9ib0UyEU8EqNoCX9quHU=;\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=EVwyWPz/Mg8lulaUw+booyZoeGuGsjAoGWa+cHhUc/e02xbERoErbUB1n2JaDz81B\n\tbI5cVrDBb7IgRuo1Lb73eBWg2T2YyKSVEQkvH7NLarh9kgLPex2OYFdpnYKsEsClVp\n\tkGKKi/76sHXoOEFJFMzBZ0bbYti8rpOVieOFYsY+bUJqGfNNpJi3TO2a3gImmTnnUu\n\teROd8WIvLq5TRvN+GpbC71RkK+kbcmecf6nmP0fTqmCbA15Fyfhlxcho32MUL5HnXM\n\tU8bl+k3cVJgikywRBU7YDv6DyqU+z3+cx+KdxA2Tt3/nYmMPKd7aAVWKKHci+CqHHp\n\tYClkd2WbZtVxg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1660171990;\n\tbh=qj/rD0vH6jUkrceu8uWXFLp9ib0UyEU8EqNoCX9quHU=;\n\th=Date:From:To:Subject:References:In-Reply-To:From;\n\tb=PROG/QwuoKp35Irit4H0786+f/ZlF7139640Gm4LvV6UIr7Xvc3CMCNzd/rKtTkoM\n\tlG5digHtG+Q1eW+YEwasDSSONRes3Vx5FWPSG6ylwL1VMlrHwumzHUtq5FumK8mM7O\n\tbUNv4kNt05JQ9gzWv8ETpdvdEmJl8psxi4FsS6xs="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"PROG/Qwu\"; dkim-atps=neutral","Date":"Thu, 11 Aug 2022 01:52:58 +0300","To":"Christian Rauch ,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"","References":"<20220810002906.5406-1-laurent.pinchart@ideasonboard.com>\n\t<20220810002906.5406-4-laurent.pinchart@ideasonboard.com>\n\t<25a997c7-67f8-0179-d861-65d0b310d4a1@gmx.de>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: controls: Store array\n\tcontrol size in Control class","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":24552,"web_url":"https://patchwork.libcamera.org/comment/24552/","msgid":"","date":"2022-08-11T22:11:21","subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: controls: Store array\n\tcontrol size in Control class","submitter":{"id":111,"url":"https://patchwork.libcamera.org/api/people/111/","name":"Christian Rauch","email":"Rauch.Christian@gmx.de"},"content":"Hi Laurent,\n\nAm 11.08.22 um 00:52 schrieb Laurent Pinchart:\n> Hi Christian,\n>\n> On Thu, Aug 11, 2022 at 01:23:18AM +0300, Laurent Pinchart via libcamera-devel wrote:\n>> On Wed, Aug 10, 2022 at 11:44:06PM +0200, Christian Rauch via libcamera-devel wrote:\n>>> Dear Laurent,\n>>>\n>>> I don't see yet, how the \"ControlId::dynamic_size\" would be used\n>>> additionally to the \"Span::extent\".\n>>\n>> The two have the same nuumerical value, but they differ in their intent.\n>> ControlId::dynamic_size is meant to indicate that the array control has\n>> a dynamic size, while utils::dynamic_extent is meant to indicate that a\n>> span has a dynamic number of elements.\n>>\n>>> Since the size of the Span is\n>>> already encoded in the type, what is the benefit of duplicating this\n>>> information in the ControlId? Will this information be identical?\n>>\n>> It allows the ControlList::set() function to access the size, and\n>> construct the Span with the right size, removing the need for explicit\n>> Span construction in the caller as shown by patch 4/4.\n>>\n>> Now that I'm writing this, I wonder if the following could work\n>>\n>> \ttemplate\n>> \tvoid set(const Control> &ctrl, const std::initializer_list &value)\n>> \t{\n>> \t\tControlValue *val = find(ctrl.id());\n>> \t\tif (!val)\n>> \t\t\treturn;\n>>\n>> \t\tval->set(Span, Size>{ value.begin(), value.size() });\n>> \t}\n>>\n>> without adding the Size template argument to the Control class. I'll\n>> give it a try.\n>\n> I can replace this patch with\n>\n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index ebc168fc28b7..2305202c4570 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -393,14 +393,14 @@ public:\n> \t\tval->set(value);\n> \t}\n>\n> -\ttemplate\n> -\tvoid set(const Control &ctrl, const std::initializer_list &value)\n> +\ttemplate\n> +\tvoid set(const Control> &ctrl, const std::initializer_list &value)\n> \t{\n> \t\tControlValue *val = find(ctrl.id());\n> \t\tif (!val)\n> \t\t\treturn;\n>\n> -\t\tval->set(Span>{ value.begin(), value.size() });\n> +\t\tval->set(Span, Size>{ value.begin(), value.size() });\n> \t}\n>\n> \tconst ControlValue &get(unsigned int id) const;\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index bc3db4f69388..6dbf9b348709 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -970,7 +970,7 @@ bool ControlList::contains(unsigned int id) const\n> */\n>\n> /**\n> - * \\fn ControlList::set(const Control &ctrl, const std::initializer_list &value)\n> + * \\fn ControlList::set(const Control> &ctrl, const std::initializer_list &value)\n> * \\copydoc ControlList::set(const Control &ctrl, const V &value)\n> */\n>\n> and 4/4 still compiles. I think I'll go with this for the time being. I\n> may revive the idea of adding a size directly to the Control class in\n> the future when a use case will arise though, but that's for later.\n\nIf all you want to know is the size of the stored Span (or scalar),\nwouldn't something like\n\ntemplate\nclass Control : public ControlId\n{\n std::size_t extent() const\n {\n return T::extent;\n }\n}\n\nbe sufficient? You could use \"is_span::value\" to implement an \"array\"\n(returning 'extent') and \"scalar\" (returning 0) version of this.\n\n>\n>> I'm also tempted to figure out if there would be a way to define a\n>> control as Control with T being the type of each element, not a\n>> Span. Size would be 1 for non-array controls, >1 for array controls with\n>> a fixed size, and std::numeric_limits::max() for array\n>> controls with a dynamic size. I'm not sure of the implications yet, but\n>> having the size available as a template argument of the Control class\n>> would allow passing it easily to the ControlId constructor, to make it\n>> available at runtime. I think there are use cases for validation for\n>> instance.\n>>\n>>> And\n>>> what would be the meaning of \"Control::size\" when its T is a scalar and\n>>> not a Span?\n>>\n>> It has no meaning in that case, which is indicated by setting it to 0.\n>> Jacopo mentioned that 0 may be a confusing value, I agree with that.\n>>\n>>> Am 10.08.22 um 02:29 schrieb Laurent Pinchart:\n>>>> Baside just identifying a control, the Control class enables usage of\n>>>> the control type for metaprogramming. This is mainly used by the\n>>>> ControlList get() and set() functions to type-check and cast the control\n>>>> value automatically at compilation time.\n>>>>\n>>>> Extend this with a new Size template argument for the Control class that\n>>>> specifies the size of array controls. Use it already in the\n>>>> ControlList::set() overload that takes an std::initializer list to\n>>>> construct the Span with an explicit size, enabling usage of the function\n>>>> for fixed-size array controls.\n>>>>\n>>>> A possible future extension would be to pass the size to the ControlId\n>>>> constructor and store it internally, enabling access to the size at\n>>>> runtime, for instance to perform validity checks.\n>>>>\n>>>> Signed-off-by: Laurent Pinchart \n>>>> ---\n>>>> include/libcamera/controls.h | 20 +++++++++++--------\n>>>> src/libcamera/controls.cpp | 37 +++++++++++++++++++++++++++---------\n>>>> utils/gen-controls.py | 24 +++++++++++++++++++++--\n>>>> 3 files changed, 62 insertions(+), 19 deletions(-)\n>>>>\n>>>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n>>>> index ebc168fc28b7..dd474a807d68 100644\n>>>> --- a/include/libcamera/controls.h\n>>>> +++ b/include/libcamera/controls.h\n>>>> @@ -8,6 +8,7 @@\n>>>> #pragma once\n>>>>\n>>>> #include \n>>>> +#include \n>>>> #include \n>>>> #include \n>>>> #include \n>>>> @@ -213,6 +214,8 @@ private:\n>>>> class ControlId\n>>>> {\n>>>> public:\n>>>> +\tstatic constexpr size_t dynamic_size = std::numeric_limits::max();\n>>>> +\n>>>> \tControlId(unsigned int id, const std::string &name, ControlType type)\n>>>> \t\t: id_(id), name_(name), type_(type)\n>>>> \t{\n>>>> @@ -250,11 +253,12 @@ static inline bool operator!=(const ControlId &lhs, unsigned int rhs)\n>>>> \treturn !(lhs == rhs);\n>>>> }\n>>>>\n>>>> -template\n>>>> +template\n>>>> class Control : public ControlId\n>>>> {\n>>>> public:\n>>>> \tusing type = T;\n>>>> +\tstatic constexpr size_t size = Size;\n>>>>\n>>>> \tControl(unsigned int id, const char *name)\n>>>> \t\t: ControlId(id, name, details::control_type>::value)\n>>>> @@ -372,8 +376,8 @@ public:\n>>>>\n>>>> \tbool contains(unsigned int id) const;\n>>>>\n>>>> -\ttemplate\n>>>> -\tstd::optional get(const Control &ctrl) const\n>>>> +\ttemplate\n>>>> +\tstd::optional get(const Control &ctrl) const\n>>>> \t{\n>>>> \t\tconst auto entry = controls_.find(ctrl.id());\n>>>> \t\tif (entry == controls_.end())\n>>>> @@ -383,8 +387,8 @@ public:\n>>>> \t\treturn val.get();\n>>>> \t}\n>>>>\n>>>> -\ttemplate\n>>>> -\tvoid set(const Control &ctrl, const V &value)\n>>>> +\ttemplate\n>>>> +\tvoid set(const Control &ctrl, const V &value)\n>>>> \t{\n>>>> \t\tControlValue *val = find(ctrl.id());\n>>>> \t\tif (!val)\n>>>> @@ -393,14 +397,14 @@ public:\n>>>> \t\tval->set(value);\n>>>> \t}\n>>>>\n>>>> -\ttemplate\n>>>> -\tvoid set(const Control &ctrl, const std::initializer_list &value)\n>>>> +\ttemplate\n>>>> +\tvoid set(const Control &ctrl, const std::initializer_list &value)\n>>>> \t{\n>>>> \t\tControlValue *val = find(ctrl.id());\n>>>> \t\tif (!val)\n>>>> \t\t\treturn;\n>>>>\n>>>> -\t\tval->set(Span>{ value.begin(), value.size() });\n>>>> +\t\tval->set(Span, Size>{ value.begin(), value.size() });\n>>>> \t}\n>>>>\n>>>> \tconst ControlValue &get(unsigned int id) const;\n>>>> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n>>>> index bc3db4f69388..24c836382f10 100644\n>>>> --- a/src/libcamera/controls.cpp\n>>>> +++ b/src/libcamera/controls.cpp\n>>>> @@ -384,6 +384,11 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n>>>> * Control class for more information.\n>>>> */\n>>>>\n>>>> +/**\n>>>> + * \\var ControlId::dynamic_size\n>>>> + * \\brief Size value for dynamic array controls\n>>>> + */\n>>>> +\n>>>> /**\n>>>> * \\fn ControlId::ControlId(unsigned int id, const std::string &name, ControlType type)\n>>>> * \\brief Construct a ControlId instance\n>>>> @@ -431,12 +436,17 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n>>>> /**\n>>>> * \\class Control\n>>>> * \\brief Describe a control and its intrinsic properties\n>>>> + * \\tparam T The control data type\n>>>> + * \\tparam Size The number of elements (for array controls only)\n>>>> *\n>>>> - * The Control class models a control exposed by an object. Its template type\n>>>> - * name T refers to the control data type, and allows functions that operate on\n>>>> - * control values to be defined as template functions using the same type T for\n>>>> - * the control value. See for instance how the ControlList::get() function\n>>>> - * returns a value corresponding to the type of the requested control.\n>>>> + * The Control class models a control exposed by an object. Its template\n>>>> + * paramter \\a T refers to the control data type, and allows functions that\n>>>> + * operate on control values to be defined as template functions using the same\n>>>> + * type T for the control value. See for instance how the ControlList::get()\n>>>> + * function returns a value corresponding to the type of the requested control.\n>>>> + *\n>>>> + * Similarly, for array controls the template parameter \\a Size indicates the\n>>>> + * number of elements in the array.\n>>>> *\n>>>> * While this class is the main means to refer to a control, the control\n>>>> * identifying information is stored in the non-template base ControlId class.\n>>>> @@ -469,6 +479,15 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n>>>> * \\brief The Control template type T\n>>>> */\n>>>>\n>>>> +/**\n>>>> + * \\var Control::size\n>>>> + * \\brief The number of elements for array controls\n>>>> + *\n>>>> + * The \\a size reports the number of elements stored by an array Control. It is\n>>>> + * equal to 0 for non-array controls, and to ControlId::dynamic_size for\n>>>> + * variable-size controls.\n>>>> + */\n>>>> +\n>>>> /**\n>>>> * \\class ControlInfo\n>>>> * \\brief Describe the limits of valid values for a Control\n>>>> @@ -943,7 +962,7 @@ bool ControlList::contains(unsigned int id) const\n>>>> }\n>>>>\n>>>> /**\n>>>> - * \\fn ControlList::get(const Control &ctrl) const\n>>>> + * \\fn ControlList::get(const Control &ctrl) const\n>>>> * \\brief Get the value of control \\a ctrl\n>>>> * \\param[in] ctrl The control\n>>>> *\n>>>> @@ -956,7 +975,7 @@ bool ControlList::contains(unsigned int id) const\n>>>> */\n>>>>\n>>>> /**\n>>>> - * \\fn ControlList::set(const Control &ctrl, const V &value)\n>>>> + * \\fn ControlList::set(const Control &ctrl, const V &value)\n>>>> * \\brief Set the control \\a ctrl value to \\a value\n>>>> * \\param[in] ctrl The control\n>>>> * \\param[in] value The control value\n>>>> @@ -970,8 +989,8 @@ bool ControlList::contains(unsigned int id) const\n>>>> */\n>>>>\n>>>> /**\n>>>> - * \\fn ControlList::set(const Control &ctrl, const std::initializer_list &value)\n>>>> - * \\copydoc ControlList::set(const Control &ctrl, const V &value)\n>>>> + * \\fn ControlList::set(const Control &ctrl, const std::initializer_list &value)\n>>>> + * \\copydoc ControlList::set(const Control &ctrl, const V &value)\n>>>> */\n>>>>\n>>>> /**\n>>>> diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n>>>> index bcfbeb7f9f17..a3b9c58beec6 100755\n>>>> --- a/utils/gen-controls.py\n>>>> +++ b/utils/gen-controls.py\n>>>> @@ -100,6 +100,10 @@ class Control(object):\n>>>> ns = 'draft::' if self.is_draft else ''\n>>>> return ns + self.__name\n>>>>\n>>>> + @property\n>>>> + def size(self):\n>>>> + return self.__size\n>>>> +\n>>>> @property\n>>>> def type(self):\n>>>> typ = self.__data.get('type')\n>>>> @@ -137,7 +141,7 @@ ${description}''')\n>>>> * \\\\var ${name}\n>>>> ${description}\n>>>> */''')\n>>>> - def_template = string.Template('extern const Control<${type}> ${name}(${id_name}, \"${name}\");')\n>>>> + def_template = string.Template('extern const Control<${type}${size}> ${name}(${id_name}, \"${name}\");')\n>>>> enum_values_doc = string.Template('''/**\n>>>> * \\\\var ${name}Values\n>>>> * \\\\brief List of all $name supported values\n>>>> @@ -154,9 +158,17 @@ ${description}\n>>>> for ctrl in controls:\n>>>> id_name = snake_case(ctrl.name).upper()\n>>>>\n>>>> + if ctrl.size is None:\n>>>> + size = ''\n>>>> + elif ctrl.size == 0:\n>>>> + size = ', ControlId::dynamic_size'\n>>>> + else:\n>>>> + size = f', {ctrl.size}'\n>>>> +\n>>>> info = {\n>>>> 'name': ctrl.name,\n>>>> 'type': ctrl.type,\n>>>> + 'size': size,\n>>>> 'description': format_description(ctrl.description),\n>>>> 'id_name': id_name,\n>>>> }\n>>>> @@ -216,7 +228,7 @@ def generate_h(controls):\n>>>> enum_template_start = string.Template('''enum ${name}Enum {''')\n>>>> enum_value_template = string.Template('''\\t${name} = ${value},''')\n>>>> enum_values_template = string.Template('''extern const std::array ${name}Values;''')\n>>>> - template = string.Template('''extern const Control<${type}> ${name};''')\n>>>> + template = string.Template('''extern const Control<${type}${size}> ${name};''')\n>>>>\n>>>> ctrls = []\n>>>> draft_ctrls = []\n>>>> @@ -228,9 +240,17 @@ def generate_h(controls):\n>>>>\n>>>> ids.append('\\t' + id_name + ' = ' + str(id_value) + ',')\n>>>>\n>>>> + if ctrl.size is None:\n>>>> + size = ''\n>>>> + elif ctrl.size == 0:\n>>>> + size = ', ControlId::dynamic_size'\n>>>> + else:\n>>>> + size = f', {ctrl.size}'\n>>>> +\n>>>> info = {\n>>>> 'name': ctrl.name,\n>>>> 'type': ctrl.type,\n>>>> + 'size': size,\n>>>> }\n>>>>\n>>>> target_ctrls = ctrls\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 DBC2BBE173\n\tfor ;\n\tThu, 11 Aug 2022 22:11:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 071626332A;\n\tFri, 12 Aug 2022 00:11:25 +0200 (CEST)","from mout.gmx.net (mout.gmx.net [212.227.15.15])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CEF91600EA\n\tfor ;\n\tFri, 12 Aug 2022 00:11:22 +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\t1MPXd2-1o0j8q0Q7u-00MaZb for\n\t; Fri, 12 Aug 2022 00:11:22 +0200"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660255885;\n\tbh=yd7e4H9wOxaBhJE8TszBhiN6vrGcGuHj06BwGAqQX1c=;\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=WUViVyAXHD9Kfe/kT/KmLxBoFiCboVdwPyObUT+PGYtzmImF6ghad05LOnbNnIbE8\n\tRC2kIuQLB6GRRSZZV06mzi5Yh1TaId6OCR+3vdQth4DxAiGqjQvPJjVsPXc+nE1tbn\n\tsXiaV5kKLvT+KsOg/4iY13JUTWQBwYnM0+Oh4qMbQTL94Jn4mYRSCYj+HuzEjFtnSI\n\tqyR25GcxbQAyPR3inQyZpXF2p/fIHMz3n+du0szN0LEeBmFmqakw5cmxbfVHb8hmm8\n\tmZ4wfcKw5d/NCMSAaRuX+0IG/yT4+xcKBazFZNegEraVh+jhlQPGScqVyGsqrzWIsd\n\tcDVNQt4m1TkIQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net;\n\ts=badeba3b8450; t=1660255882;\n\tbh=yd7e4H9wOxaBhJE8TszBhiN6vrGcGuHj06BwGAqQX1c=;\n\th=X-UI-Sender-Class:Date:Subject:To:References:From:In-Reply-To;\n\tb=le1vHwlxtOcZiuixjMf56PXJ2QVUOwF7Ga6ZtvfApzV4MFm33vWHwjMvzRpaFkqs/\n\t1L/H5WwCpChowW4OtE07X4+JXzv+ie8AKF7AqGQZVgvK9aSyKeq561zR1q6ztzVPe8\n\t0VGfl8Nk1X/Q5SylHNYJoNJId937CV4gPfQWGNrw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=gmx.net header.i=@gmx.net\n\theader.b=\"le1vHwlx\"; dkim-atps=neutral","X-UI-Sender-Class":"01bb95c1-4bf8-414a-932a-4f6e2808ef9c","Message-ID":"","Date":"Fri, 12 Aug 2022 00:11:21 +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":"<20220810002906.5406-1-laurent.pinchart@ideasonboard.com>\n\t<20220810002906.5406-4-laurent.pinchart@ideasonboard.com>\n\t<25a997c7-67f8-0179-d861-65d0b310d4a1@gmx.de>\n\t\n\t","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"quoted-printable","X-Provags-ID":"V03:K1:LsEDdNQVeCZYJt5zlzkzkvlHSbLTSeW1Y8yH/Fk6GywCDmQJaFi\n\tOjp08PM08EUl0QcD/1PrWfyheNX94BiGLvH9kUgnrYjyQzqJSe1WBJtghzzuNzQk3CpcaMS\n\tn5oSvtjpIP4CGM/hNHKJxdzz2wRiGny2FH/AtZ5HKXznlWyojBMUIKWREFyuNJSO5AyRPJf\n\tJpdlYxOy6pRz8PWWfqZJw==","X-Spam-Flag":"NO","X-UI-Out-Filterresults":"notjunk:1; V03:K0:o3146sR5A6Y=:uKByCb7/oJibMHMvMF98Mv\n\tdnZwh4+ZdCe7S8ZPmiTq16HU0Hgi2IDgTbQcc8TTcgCMlwJxGKQHKlUj9Qo/FOBUUhWv0EzbO\n\t3f/73NIu733cRMFe/H2unDUo/iEs2QNIdzzyeTHXwxwsXhgMULtMJLh/OSgre7/mcIhTSQ4Rk\n\tW97a/5+XUCc0M45XluWDxCayKKnOqaJPLSLgNZ3cY2MtAQb1dxfGUJ5evFJ7arlOyk3b6xJrC\n\tcazZCvr55ho5v4tU0Kl22qcHDJ98Rs1uiqVAZIOPRlo7DXsycQP7OTs/Juf7RupGpJC4s/aWq\n\t2UC9eUHmp83udvnrH37zVdJ3ou752VYE9tFs5P+LR2LmH/GpHJ7zz83Q9rChv5VSzfYSV1urq\n\tscpstA/WnC7N2j9UIfCWfzRzJ8Zv2+1paPc5Z+CnbM5IWT6hfU8xQnngV5G5pVCwCVBwqE3X8\n\tDA9cl/MxUk6cVogMQXFLcuxo2TLvj8GC040eqFBW8ZDPIYSUjK6PfD9E1/R5ecnYwfFCbzpiU\n\tzXyLNxG2jrhfGERed4BPXSRXX7jST/U/2pDZeLfsFEl+cX3nLt3Pzlq9tjpCtDszlL1egyTLT\n\tOjRY19n4AHDwnv9puufzSV8lkBvgZEgj49sbIGBbD4CjHb3y9oVypy/Fq5TJYEkMCPmI+p7W3\n\t88G4gkeOZLtyaV7DNkyJonox8T1Ykd/IaRvmtPXSle9XeYu4RWsag44ZTZzZeJ7T5NAUqBl5D\n\tZAZwp3JTfi1hrqRxfEnd8xqRlwTshfhHnNzDN0mCs6+Mcpfxb3LnF4nH/42YmGyJI3lfdWQcS\n\toinoslJkI19AsAFJJTvLGX+IjaNCXt5hcKtXbIiIM/69WdwgBya28QD5fnTuB355uWWMWjv3b\n\t5s1pLAVSBg+CEENxd6mmIhYFpyDE9yVe1/lkB69DzERJmSMYjD+46tTEHyIp8/UmH9566ZY02\n\t1BYLOIyQ8ctq3+lWrjHsamSzwdOBhLzveB2EGSVaBZSjvGgpYz+zysUT5nbm2fLI2/+8Wdggq\n\tqNSaV81c+1k3pIL+h/x0S3VqDPs88VYP6txt2aqDuGJL9zTn5cPR/tK76Yo5XZQ2PbYK4xwoy\n\tsYk54fm+pTDKAUSR9wp999O6xiQcEnNKDqqccW3uopmadhrONDgyDwpqw==","Subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: controls: Store array\n\tcontrol size in Control class","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":24553,"web_url":"https://patchwork.libcamera.org/comment/24553/","msgid":"","date":"2022-08-11T22:24:34","subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: controls: Store array\n\tcontrol size in Control class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Christian,\n\nOn Fri, Aug 12, 2022 at 12:11:21AM +0200, Christian Rauch via libcamera-devel wrote:\n> Am 11.08.22 um 00:52 schrieb Laurent Pinchart:\n> > On Thu, Aug 11, 2022 at 01:23:18AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> >> On Wed, Aug 10, 2022 at 11:44:06PM +0200, Christian Rauch via libcamera-devel wrote:\n> >>> Dear Laurent,\n> >>>\n> >>> I don't see yet, how the \"ControlId::dynamic_size\" would be used\n> >>> additionally to the \"Span::extent\".\n> >>\n> >> The two have the same nuumerical value, but they differ in their intent.\n> >> ControlId::dynamic_size is meant to indicate that the array control has\n> >> a dynamic size, while utils::dynamic_extent is meant to indicate that a\n> >> span has a dynamic number of elements.\n> >>\n> >>> Since the size of the Span is\n> >>> already encoded in the type, what is the benefit of duplicating this\n> >>> information in the ControlId? Will this information be identical?\n> >>\n> >> It allows the ControlList::set() function to access the size, and\n> >> construct the Span with the right size, removing the need for explicit\n> >> Span construction in the caller as shown by patch 4/4.\n> >>\n> >> Now that I'm writing this, I wonder if the following could work\n> >>\n> >> \ttemplate\n> >> \tvoid set(const Control> &ctrl, const std::initializer_list &value)\n> >> \t{\n> >> \t\tControlValue *val = find(ctrl.id());\n> >> \t\tif (!val)\n> >> \t\t\treturn;\n> >>\n> >> \t\tval->set(Span, Size>{ value.begin(), value.size() });\n> >> \t}\n> >>\n> >> without adding the Size template argument to the Control class. I'll\n> >> give it a try.\n> >\n> > I can replace this patch with\n> >\n> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > index ebc168fc28b7..2305202c4570 100644\n> > --- a/include/libcamera/controls.h\n> > +++ b/include/libcamera/controls.h\n> > @@ -393,14 +393,14 @@ public:\n> > \t\tval->set(value);\n> > \t}\n> >\n> > -\ttemplate\n> > -\tvoid set(const Control &ctrl, const std::initializer_list &value)\n> > +\ttemplate\n> > +\tvoid set(const Control> &ctrl, const std::initializer_list &value)\n> > \t{\n> > \t\tControlValue *val = find(ctrl.id());\n> > \t\tif (!val)\n> > \t\t\treturn;\n> >\n> > -\t\tval->set(Span>{ value.begin(), value.size() });\n> > +\t\tval->set(Span, Size>{ value.begin(), value.size() });\n> > \t}\n> >\n> > \tconst ControlValue &get(unsigned int id) const;\n> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > index bc3db4f69388..6dbf9b348709 100644\n> > --- a/src/libcamera/controls.cpp\n> > +++ b/src/libcamera/controls.cpp\n> > @@ -970,7 +970,7 @@ bool ControlList::contains(unsigned int id) const\n> > */\n> >\n> > /**\n> > - * \\fn ControlList::set(const Control &ctrl, const std::initializer_list &value)\n> > + * \\fn ControlList::set(const Control> &ctrl, const std::initializer_list &value)\n> > * \\copydoc ControlList::set(const Control &ctrl, const V &value)\n> > */\n> >\n> > and 4/4 still compiles. I think I'll go with this for the time being. I\n> > may revive the idea of adding a size directly to the Control class in\n> > the future when a use case will arise though, but that's for later.\n> \n> If all you want to know is the size of the stored Span (or scalar),\n> wouldn't something like\n> \n> template\n> class Control : public ControlId\n> {\n> std::size_t extent() const\n> {\n> return T::extent;\n> }\n> }\n> \n> be sufficient? You could use \"is_span::value\" to implement an \"array\"\n> (returning 'extent') and \"scalar\" (returning 0) version of this.\n\nWe'd have to guard that with a Span check on T indeed. It would\notherwise work, and should be a constexpr. The issue is that it won't\ngive access to the size from ControlId, which is something that could be\nfixed by passing it to the ControlId constructor.\n\nRegardless, adding the size as a template parameter to the Control class\nis indeed redundant, so I would likely do so only if coupled with\nturning T to the item type for array controls:\n\n\tControl\n\tControl>\n\tControl>\n\nwould become respectively\n\n\tControl\n\tControl\n\tControl\n\nI think it looks cleaner, but that's not something I'll spend time on\nfor now. I have in the back of my head a plan to rework ControlList to\nmake it possible to avoid most of the dynamic memory allocations,\nturning it into a convenient accessor API on top of a large buffer of\nserialized data. I have no idea yet how that would look like exactly,\nand I don't want to think about it now, there are more urgent topics to\nhandle.\n\n> >> I'm also tempted to figure out if there would be a way to define a\n> >> control as Control with T being the type of each element, not a\n> >> Span. Size would be 1 for non-array controls, >1 for array controls with\n> >> a fixed size, and std::numeric_limits::max() for array\n> >> controls with a dynamic size. I'm not sure of the implications yet, but\n> >> having the size available as a template argument of the Control class\n> >> would allow passing it easily to the ControlId constructor, to make it\n> >> available at runtime. I think there are use cases for validation for\n> >> instance.\n> >>\n> >>> And\n> >>> what would be the meaning of \"Control::size\" when its T is a scalar and\n> >>> not a Span?\n> >>\n> >> It has no meaning in that case, which is indicated by setting it to 0.\n> >> Jacopo mentioned that 0 may be a confusing value, I agree with that.\n> >>\n> >>> Am 10.08.22 um 02:29 schrieb Laurent Pinchart:\n> >>>> Baside just identifying a control, the Control class enables usage of\n> >>>> the control type for metaprogramming. This is mainly used by the\n> >>>> ControlList get() and set() functions to type-check and cast the control\n> >>>> value automatically at compilation time.\n> >>>>\n> >>>> Extend this with a new Size template argument for the Control class that\n> >>>> specifies the size of array controls. Use it already in the\n> >>>> ControlList::set() overload that takes an std::initializer list to\n> >>>> construct the Span with an explicit size, enabling usage of the function\n> >>>> for fixed-size array controls.\n> >>>>\n> >>>> A possible future extension would be to pass the size to the ControlId\n> >>>> constructor and store it internally, enabling access to the size at\n> >>>> runtime, for instance to perform validity checks.\n> >>>>\n> >>>> Signed-off-by: Laurent Pinchart \n> >>>> ---\n> >>>> include/libcamera/controls.h | 20 +++++++++++--------\n> >>>> src/libcamera/controls.cpp | 37 +++++++++++++++++++++++++++---------\n> >>>> utils/gen-controls.py | 24 +++++++++++++++++++++--\n> >>>> 3 files changed, 62 insertions(+), 19 deletions(-)\n> >>>>\n> >>>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> >>>> index ebc168fc28b7..dd474a807d68 100644\n> >>>> --- a/include/libcamera/controls.h\n> >>>> +++ b/include/libcamera/controls.h\n> >>>> @@ -8,6 +8,7 @@\n> >>>> #pragma once\n> >>>>\n> >>>> #include \n> >>>> +#include \n> >>>> #include \n> >>>> #include \n> >>>> #include \n> >>>> @@ -213,6 +214,8 @@ private:\n> >>>> class ControlId\n> >>>> {\n> >>>> public:\n> >>>> +\tstatic constexpr size_t dynamic_size = std::numeric_limits::max();\n> >>>> +\n> >>>> \tControlId(unsigned int id, const std::string &name, ControlType type)\n> >>>> \t\t: id_(id), name_(name), type_(type)\n> >>>> \t{\n> >>>> @@ -250,11 +253,12 @@ static inline bool operator!=(const ControlId &lhs, unsigned int rhs)\n> >>>> \treturn !(lhs == rhs);\n> >>>> }\n> >>>>\n> >>>> -template\n> >>>> +template\n> >>>> class Control : public ControlId\n> >>>> {\n> >>>> public:\n> >>>> \tusing type = T;\n> >>>> +\tstatic constexpr size_t size = Size;\n> >>>>\n> >>>> \tControl(unsigned int id, const char *name)\n> >>>> \t\t: ControlId(id, name, details::control_type>::value)\n> >>>> @@ -372,8 +376,8 @@ public:\n> >>>>\n> >>>> \tbool contains(unsigned int id) const;\n> >>>>\n> >>>> -\ttemplate\n> >>>> -\tstd::optional get(const Control &ctrl) const\n> >>>> +\ttemplate\n> >>>> +\tstd::optional get(const Control &ctrl) const\n> >>>> \t{\n> >>>> \t\tconst auto entry = controls_.find(ctrl.id());\n> >>>> \t\tif (entry == controls_.end())\n> >>>> @@ -383,8 +387,8 @@ public:\n> >>>> \t\treturn val.get();\n> >>>> \t}\n> >>>>\n> >>>> -\ttemplate\n> >>>> -\tvoid set(const Control &ctrl, const V &value)\n> >>>> +\ttemplate\n> >>>> +\tvoid set(const Control &ctrl, const V &value)\n> >>>> \t{\n> >>>> \t\tControlValue *val = find(ctrl.id());\n> >>>> \t\tif (!val)\n> >>>> @@ -393,14 +397,14 @@ public:\n> >>>> \t\tval->set(value);\n> >>>> \t}\n> >>>>\n> >>>> -\ttemplate\n> >>>> -\tvoid set(const Control &ctrl, const std::initializer_list &value)\n> >>>> +\ttemplate\n> >>>> +\tvoid set(const Control &ctrl, const std::initializer_list &value)\n> >>>> \t{\n> >>>> \t\tControlValue *val = find(ctrl.id());\n> >>>> \t\tif (!val)\n> >>>> \t\t\treturn;\n> >>>>\n> >>>> -\t\tval->set(Span>{ value.begin(), value.size() });\n> >>>> +\t\tval->set(Span, Size>{ value.begin(), value.size() });\n> >>>> \t}\n> >>>>\n> >>>> \tconst ControlValue &get(unsigned int id) const;\n> >>>> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> >>>> index bc3db4f69388..24c836382f10 100644\n> >>>> --- a/src/libcamera/controls.cpp\n> >>>> +++ b/src/libcamera/controls.cpp\n> >>>> @@ -384,6 +384,11 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> >>>> * Control class for more information.\n> >>>> */\n> >>>>\n> >>>> +/**\n> >>>> + * \\var ControlId::dynamic_size\n> >>>> + * \\brief Size value for dynamic array controls\n> >>>> + */\n> >>>> +\n> >>>> /**\n> >>>> * \\fn ControlId::ControlId(unsigned int id, const std::string &name, ControlType type)\n> >>>> * \\brief Construct a ControlId instance\n> >>>> @@ -431,12 +436,17 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> >>>> /**\n> >>>> * \\class Control\n> >>>> * \\brief Describe a control and its intrinsic properties\n> >>>> + * \\tparam T The control data type\n> >>>> + * \\tparam Size The number of elements (for array controls only)\n> >>>> *\n> >>>> - * The Control class models a control exposed by an object. Its template type\n> >>>> - * name T refers to the control data type, and allows functions that operate on\n> >>>> - * control values to be defined as template functions using the same type T for\n> >>>> - * the control value. See for instance how the ControlList::get() function\n> >>>> - * returns a value corresponding to the type of the requested control.\n> >>>> + * The Control class models a control exposed by an object. Its template\n> >>>> + * paramter \\a T refers to the control data type, and allows functions that\n> >>>> + * operate on control values to be defined as template functions using the same\n> >>>> + * type T for the control value. See for instance how the ControlList::get()\n> >>>> + * function returns a value corresponding to the type of the requested control.\n> >>>> + *\n> >>>> + * Similarly, for array controls the template parameter \\a Size indicates the\n> >>>> + * number of elements in the array.\n> >>>> *\n> >>>> * While this class is the main means to refer to a control, the control\n> >>>> * identifying information is stored in the non-template base ControlId class.\n> >>>> @@ -469,6 +479,15 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen\n> >>>> * \\brief The Control template type T\n> >>>> */\n> >>>>\n> >>>> +/**\n> >>>> + * \\var Control::size\n> >>>> + * \\brief The number of elements for array controls\n> >>>> + *\n> >>>> + * The \\a size reports the number of elements stored by an array Control. It is\n> >>>> + * equal to 0 for non-array controls, and to ControlId::dynamic_size for\n> >>>> + * variable-size controls.\n> >>>> + */\n> >>>> +\n> >>>> /**\n> >>>> * \\class ControlInfo\n> >>>> * \\brief Describe the limits of valid values for a Control\n> >>>> @@ -943,7 +962,7 @@ bool ControlList::contains(unsigned int id) const\n> >>>> }\n> >>>>\n> >>>> /**\n> >>>> - * \\fn ControlList::get(const Control &ctrl) const\n> >>>> + * \\fn ControlList::get(const Control &ctrl) const\n> >>>> * \\brief Get the value of control \\a ctrl\n> >>>> * \\param[in] ctrl The control\n> >>>> *\n> >>>> @@ -956,7 +975,7 @@ bool ControlList::contains(unsigned int id) const\n> >>>> */\n> >>>>\n> >>>> /**\n> >>>> - * \\fn ControlList::set(const Control &ctrl, const V &value)\n> >>>> + * \\fn ControlList::set(const Control &ctrl, const V &value)\n> >>>> * \\brief Set the control \\a ctrl value to \\a value\n> >>>> * \\param[in] ctrl The control\n> >>>> * \\param[in] value The control value\n> >>>> @@ -970,8 +989,8 @@ bool ControlList::contains(unsigned int id) const\n> >>>> */\n> >>>>\n> >>>> /**\n> >>>> - * \\fn ControlList::set(const Control &ctrl, const std::initializer_list &value)\n> >>>> - * \\copydoc ControlList::set(const Control &ctrl, const V &value)\n> >>>> + * \\fn ControlList::set(const Control &ctrl, const std::initializer_list &value)\n> >>>> + * \\copydoc ControlList::set(const Control &ctrl, const V &value)\n> >>>> */\n> >>>>\n> >>>> /**\n> >>>> diff --git a/utils/gen-controls.py b/utils/gen-controls.py\n> >>>> index bcfbeb7f9f17..a3b9c58beec6 100755\n> >>>> --- a/utils/gen-controls.py\n> >>>> +++ b/utils/gen-controls.py\n> >>>> @@ -100,6 +100,10 @@ class Control(object):\n> >>>> ns = 'draft::' if self.is_draft else ''\n> >>>> return ns + self.__name\n> >>>>\n> >>>> + @property\n> >>>> + def size(self):\n> >>>> + return self.__size\n> >>>> +\n> >>>> @property\n> >>>> def type(self):\n> >>>> typ = self.__data.get('type')\n> >>>> @@ -137,7 +141,7 @@ ${description}''')\n> >>>> * \\\\var ${name}\n> >>>> ${description}\n> >>>> */''')\n> >>>> - def_template = string.Template('extern const Control<${type}> ${name}(${id_name}, \"${name}\");')\n> >>>> + def_template = string.Template('extern const Control<${type}${size}> ${name}(${id_name}, \"${name}\");')\n> >>>> enum_values_doc = string.Template('''/**\n> >>>> * \\\\var ${name}Values\n> >>>> * \\\\brief List of all $name supported values\n> >>>> @@ -154,9 +158,17 @@ ${description}\n> >>>> for ctrl in controls:\n> >>>> id_name = snake_case(ctrl.name).upper()\n> >>>>\n> >>>> + if ctrl.size is None:\n> >>>> + size = ''\n> >>>> + elif ctrl.size == 0:\n> >>>> + size = ', ControlId::dynamic_size'\n> >>>> + else:\n> >>>> + size = f', {ctrl.size}'\n> >>>> +\n> >>>> info = {\n> >>>> 'name': ctrl.name,\n> >>>> 'type': ctrl.type,\n> >>>> + 'size': size,\n> >>>> 'description': format_description(ctrl.description),\n> >>>> 'id_name': id_name,\n> >>>> }\n> >>>> @@ -216,7 +228,7 @@ def generate_h(controls):\n> >>>> enum_template_start = string.Template('''enum ${name}Enum {''')\n> >>>> enum_value_template = string.Template('''\\t${name} = ${value},''')\n> >>>> enum_values_template = string.Template('''extern const std::array ${name}Values;''')\n> >>>> - template = string.Template('''extern const Control<${type}> ${name};''')\n> >>>> + template = string.Template('''extern const Control<${type}${size}> ${name};''')\n> >>>>\n> >>>> ctrls = []\n> >>>> draft_ctrls = []\n> >>>> @@ -228,9 +240,17 @@ def generate_h(controls):\n> >>>>\n> >>>> ids.append('\\t' + id_name + ' = ' + str(id_value) + ',')\n> >>>>\n> >>>> + if ctrl.size is None:\n> >>>> + size = ''\n> >>>> + elif ctrl.size == 0:\n> >>>> + size = ', ControlId::dynamic_size'\n> >>>> + else:\n> >>>> + size = f', {ctrl.size}'\n> >>>> +\n> >>>> info = {\n> >>>> 'name': ctrl.name,\n> >>>> 'type': ctrl.type,\n> >>>> + 'size': size,\n> >>>> }\n> >>>>\n> >>>> target_ctrls = ctrls","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 B0EE3C3272\n\tfor ;\n\tThu, 11 Aug 2022 22:24:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 21BF46332A;\n\tFri, 12 Aug 2022 00:24:49 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 58799600EA\n\tfor ;\n\tFri, 12 Aug 2022 00:24:48 +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 BF0353F1;\n\tFri, 12 Aug 2022 00:24:47 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1660256689;\n\tbh=kUrNm0LO2++epSTYDtLbQGzcB0AjnkfP9L16aYx5FLA=;\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=ABLO0V9ZY6l3YqA49McrR9Qpu4UO+TP2vHNbO4FnfrOnIYzaUBnzcmob/9gokAAH6\n\t8KRC2+d78RS5btsLXY9p29zKmOtSn3N9AA2LkL/p3vjVDCPobaWv0hiBzIF3/3tqX1\n\t6jXH8T9cm9nbsa9iptVtTwet2fT5M1CzIYHQ/ZSIQzdjMpTXR2/wNd+awwujhm2aaH\n\tLujxM0HBdbnFPpoovJFbm3xMVaePots/HNrRiTWmIz9YfXK/5YgAL/9RlB2dTTyAPt\n\ts5KOg74uZFQHfMWomLUHvWCyfGUcErAV51OTPet60wDMk4jX8xSPNDPyuK5XUf9ZhI\n\tCo7Wd2kPzm/8A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1660256688;\n\tbh=kUrNm0LO2++epSTYDtLbQGzcB0AjnkfP9L16aYx5FLA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=BB5Zvt4l0w3j3ZvXpX0kz9QN16wc44Dt+e1AJOSUcZNfJyJ71JTjz+c6i3/GGfkY2\n\tARSSodsIT6Ji6MTeVFm5ZcuB7yn+sFuqVg+6l5JzuGPHzi+i98TzUp4dbN8fqIej9d\n\t4hLnS2pRabvE6Ky1eQ0uaje/GzweBUHxmGRorWdc="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"BB5Zvt4l\"; dkim-atps=neutral","Date":"Fri, 12 Aug 2022 01:24:34 +0300","To":"Christian Rauch ","Message-ID":"","References":"<20220810002906.5406-1-laurent.pinchart@ideasonboard.com>\n\t<20220810002906.5406-4-laurent.pinchart@ideasonboard.com>\n\t<25a997c7-67f8-0179-d861-65d0b310d4a1@gmx.de>\n\t\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH 3/4] libcamera: controls: Store array\n\tcontrol size in Control class","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\" "}}]