[{"id":4125,"web_url":"https://patchwork.libcamera.org/comment/4125/","msgid":"<20200320120844.2zch3cu2gajvjit2@uno.localdomain>","date":"2020-03-20T12:08:44","subject":"Re: [libcamera-devel] [PATCH 03/11] libcamera: controls: Add\n\tsupport for string controls","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n\nOn Mon, Mar 09, 2020 at 05:24:06PM +0100, Jacopo Mondi wrote:\n> From: Laurent Pinchart \n>\n> String controls are stored internally as an array of char, but the\n> ControlValue constructor, get() and set() functions operate on an\n> std::string for convenience. Array of strings are thus not supported.\n>\n> Unlike for other control types, the ControlInfo range reports the\n> minimum and maximum allowed lengths of the string (the minimum will\n> usually be 0), not the minimum and maximum value of each element.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> include/libcamera/controls.h | 30 +++++++++++++++++++-----\n> src/libcamera/control_serializer.cpp | 22 +++++++++++++++++\n> src/libcamera/controls.cpp | 35 ++++++++++++++++++++++++----\n> src/libcamera/gen-controls.py | 18 +++++++-------\n> 4 files changed, 87 insertions(+), 18 deletions(-)\n>\n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index 9c6cbffb88b5..5cf6280e612b 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -26,6 +26,7 @@ enum ControlType {\n> \tControlTypeInteger32,\n> \tControlTypeInteger64,\n> \tControlTypeFloat,\n> +\tControlTypeString,\n> };\n>\n> namespace details {\n> @@ -64,6 +65,11 @@ struct control_type {\n> \tstatic constexpr ControlType value = ControlTypeFloat;\n> };\n>\n> +template<>\n> +struct control_type {\n> +\tstatic constexpr ControlType value = ControlTypeString;\n> +};\n> +\n> template\n> struct control_type> : public control_type> {\n> };\n> @@ -76,7 +82,9 @@ public:\n> \tControlValue();\n>\n> #ifndef __DOXYGEN__\n> -\ttemplate::value, std::nullptr_t> = nullptr>\n> +\ttemplate::value &&\n> +\t\t\t\t\t\t !std::is_same>::value,\n> +\t\t\t\t\t\t std::nullptr_t> = nullptr>\n> \tControlValue(const T &value)\n> \t\t: type_(ControlTypeNone), numElements_(0)\n> \t{\n> @@ -84,7 +92,9 @@ public:\n> \t\t &value, 1, sizeof(T));\n> \t}\n>\n> -\ttemplate::value, std::nullptr_t> = nullptr>\n> +\ttemplate::value ||\n> +\t\t\t\t\t\t std::is_same>::value,\n> +\t\t\t\t\t\t std::nullptr_t> = nullptr>\n> #else\n> \ttemplate\n> #endif\n> @@ -115,7 +125,9 @@ public:\n> \t}\n>\n> #ifndef __DOXYGEN__\n> -\ttemplate::value, std::nullptr_t> = nullptr>\n> +\ttemplate::value &&\n> +\t\t\t\t\t\t !std::is_same>::value,\n> +\t\t\t\t\t\t std::nullptr_t> = nullptr>\n> \tT get() const\n> \t{\n> \t\tassert(type_ == details::control_type>::value);\n> @@ -124,7 +136,9 @@ public:\n> \t\treturn *reinterpret_cast(data().data());\n> \t}\n>\n> -\ttemplate::value, std::nullptr_t> = nullptr>\n> +\ttemplate::value ||\n> +\t\t\t\t\t\t std::is_same>::value,\n> +\t\t\t\t\t\t std::nullptr_t> = nullptr>\n> #else\n> \ttemplate\n> #endif\n> @@ -139,14 +153,18 @@ public:\n> \t}\n>\n> #ifndef __DOXYGEN__\n> -\ttemplate::value, std::nullptr_t> = nullptr>\n> +\ttemplate::value &&\n> +\t\t\t\t\t\t !std::is_same>::value,\n> +\t\t\t\t\t\t std::nullptr_t> = nullptr>\n> \tvoid set(const T &value)\n> \t{\n> \t\tset(details::control_type>::value, false,\n> \t\t reinterpret_cast(&value), 1, sizeof(T));\n> \t}\n>\n> -\ttemplate::value, std::nullptr_t> = nullptr>\n> +\ttemplate::value ||\n> +\t\t\t\t\t\t std::is_same>::value,\n> +\t\t\t\t\t\t std::nullptr_t> = nullptr>\n> #else\n> \ttemplate\n> #endif\n> diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp\n> index eef875f4d96c..808419f246c0 100644\n> --- a/src/libcamera/control_serializer.cpp\n> +++ b/src/libcamera/control_serializer.cpp\n> @@ -314,6 +314,22 @@ ControlValue ControlSerializer::loadControlValue(ByteStreamBuffer &buffer,\n> \treturn value;\n> }\n>\n> +template<>\n> +ControlValue ControlSerializer::loadControlValue(ByteStreamBuffer &buffer,\n> +\t\t\t\t\t\t bool isArray,\n> +\t\t\t\t\t\t unsigned int count)\n\nNot may way around this specialization I gueess..\nI see std::string being defined as std::basic_string and I was\nwondering if we could reuse the template argument, but I don't\nsee a clear way to do so\n\nAnyway, ControlTypeFloat documentation removal apart\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> +{\n> +\tControlValue value;\n> +\n> +\tconst char *data = buffer.read(count);\n> +\tif (!data)\n> +\t\treturn value;\n> +\n> +\tvalue.set(std::string{ data, count });\n> +\n> +\treturn value;\n> +}\n> +\n> ControlValue ControlSerializer::loadControlValue(ControlType type,\n> \t\t\t\t\t\t ByteStreamBuffer &buffer,\n> \t\t\t\t\t\t bool isArray,\n> @@ -335,6 +351,9 @@ ControlValue ControlSerializer::loadControlValue(ControlType type,\n> \tcase ControlTypeFloat:\n> \t\treturn loadControlValue(buffer, isArray, count);\n>\n> +\tcase ControlTypeString:\n> +\t\treturn loadControlValue(buffer, isArray, count);\n> +\n> \tcase ControlTypeNone:\n> \t\treturn ControlValue();\n> \t}\n> @@ -345,6 +364,9 @@ ControlValue ControlSerializer::loadControlValue(ControlType type,\n> ControlInfo ControlSerializer::loadControlInfo(ControlType type,\n> \t\t\t\t\t ByteStreamBuffer &b)\n> {\n> +\tif (type == ControlTypeString)\n> +\t\ttype = ControlTypeInteger32;\n> +\n> \tControlValue min = loadControlValue(type, b);\n> \tControlValue max = loadControlValue(type, b);\n>\n> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> index 53649fe897db..68d040db4daa 100644\n> --- a/src/libcamera/controls.cpp\n> +++ b/src/libcamera/controls.cpp\n> @@ -57,6 +57,7 @@ static constexpr size_t ControlValueSize[] = {\n> \t[ControlTypeInteger32]\t\t= sizeof(int32_t),\n> \t[ControlTypeInteger64]\t\t= sizeof(int64_t),\n> \t[ControlTypeFloat]\t\t= sizeof(float),\n> +\t[ControlTypeString]\t\t= sizeof(char),\n> };\n>\n> } /* namespace */\n> @@ -74,8 +75,8 @@ static constexpr size_t ControlValueSize[] = {\n> * The control stores a 32-bit integer value\n> * \\var ControlTypeInteger64\n> * The control stores a 64-bit integer value\n> - * \\var ControlTypeFloat\n> - * The control stores a 32-bit floating point value\n\nWhat did the poor Float type do to you ? :(\n\n> + * \\var ControlTypeString\n> + * The control stores a string value as an array of char\n> */\n>\n> /**\n> @@ -164,7 +165,8 @@ ControlValue &ControlValue::operator=(const ControlValue &other)\n> * \\brief Retrieve the number of elements stored in the ControlValue\n> *\n> * For instances storing an array, this function returns the number of elements\n> - * in the array. Otherwise, it returns 1.\n> + * in the array. For instances storing a string, it returns the length of the\n> + * string, not counting the terminating '\\0'. Otherwise, it returns 1.\n> *\n> * \\return The number of elements stored in the ControlValue\n> */\n> @@ -192,6 +194,11 @@ std::string ControlValue::toString() const\n> \t\treturn \"\";\n>\n> \tconst uint8_t *data = ControlValue::data().data();\n> +\n> +\tif (type_ == ControlTypeString)\n> +\t\treturn std::string(reinterpret_cast(data),\n> +\t\t\t\t numElements_);\n> +\n> \tstd::string str(isArray_ ? \"[ \" : \"\");\n>\n> \tfor (unsigned int i = 0; i < numElements_; ++i) {\n> @@ -222,6 +229,7 @@ std::string ControlValue::toString() const\n> \t\t\tbreak;\n> \t\t}\n> \t\tcase ControlTypeNone:\n> +\t\tcase ControlTypeString:\n> \t\t\tbreak;\n> \t\t}\n>\n> @@ -439,12 +447,22 @@ ControlInfo::ControlInfo(const ControlValue &min,\n> /**\n> * \\fn ControlInfo::min()\n> * \\brief Retrieve the minimum value of the control\n> + *\n> + * For string controls, this is the minimum length of the string, not counting\n> + * the terminating '\\0'. For all other control types, this is the minimum value\n> + * of each element.\n> + *\n> * \\return A ControlValue with the minimum value for the control\n> */\n>\n> /**\n> * \\fn ControlInfo::max()\n> * \\brief Retrieve the maximum value of the control\n> + *\n> + * For string controls, this is the maximum length of the string, not counting\n> + * the terminating '\\0'. For all other control types, this is the maximum value\n> + * of each element.\n> + *\n> * \\return A ControlValue with the maximum value for the control\n> */\n>\n> @@ -653,7 +671,16 @@ void ControlInfoMap::generateIdmap()\n> \tidmap_.clear();\n>\n> \tfor (const auto &ctrl : *this) {\n> -\t\tif (ctrl.first->type() != ctrl.second.min().type()) {\n> +\t\t/*\n> +\t\t * For string controls, min and max define the valid\n> +\t\t * range for the string size, not for the individual\n> +\t\t * values.\n> +\t\t */\n> +\t\tControlType rangeType = ctrl.first->type() == ControlTypeString\n> +\t\t\t\t ? ControlTypeInteger32 : ctrl.first->type();\n> +\t\tconst ControlInfo &info = ctrl.second;\n> +\n> +\t\tif (info.min().type() != rangeType) {\n> \t\t\tLOG(Controls, Error)\n> \t\t\t\t<< \"Control \" << utils::hex(ctrl.first->id())\n> \t\t\t\t<< \" type and info type mismatch\";\n> diff --git a/src/libcamera/gen-controls.py b/src/libcamera/gen-controls.py\n> index ff8bda6b16c1..87c3d52ada6d 100755\n> --- a/src/libcamera/gen-controls.py\n> +++ b/src/libcamera/gen-controls.py\n> @@ -42,10 +42,11 @@ ${description}\n> name, ctrl = ctrl.popitem()\n> id_name = snake_case(name).upper()\n>\n> - if ctrl.get('size'):\n> - ctrl_type = 'Span' % ctrl['type']\n> - else:\n> - ctrl_type = ctrl['type']\n> + ctrl_type = ctrl['type']\n> + if ctrl_type == 'string':\n> + ctrl_type = 'std::string'\n> + elif ctrl.get('size'):\n> + ctrl_type = 'Span' % ctrl_type\n>\n> info = {\n> 'name': name,\n> @@ -97,10 +98,11 @@ def generate_h(controls):\n>\n> ids.append('\\t' + id_name + ' = ' + str(id_value) + ',')\n>\n> - if ctrl.get('size'):\n> - ctrl_type = 'Span' % ctrl['type']\n> - else:\n> - ctrl_type = ctrl['type']\n> + ctrl_type = ctrl['type']\n> + if ctrl_type == 'string':\n> + ctrl_type = 'std::string'\n> + elif ctrl.get('size'):\n> + ctrl_type = 'Span' % ctrl_type\n>\n> info = {\n> 'name': name,\n> --\n> 2.25.0\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[217.70.183.200])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2FB8E60416\n\tfor ;\n\tFri, 20 Mar 2020 13:05:48 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay7-d.mail.gandi.net (Postfix) with ESMTPSA id D0FDB2000D\n\tfor ;\n\tFri, 20 Mar 2020 12:05:47 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Fri, 20 Mar 2020 13:08:44 +0100","From":"Jacopo Mondi ","To":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200320120844.2zch3cu2gajvjit2@uno.localdomain>","References":"<20200309162414.720306-1-jacopo@jmondi.org>\n\t<20200309162414.720306-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200309162414.720306-4-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 03/11] libcamera: controls: Add\n\tsupport for string controls","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Fri, 20 Mar 2020 12:05:48 -0000"}},{"id":4127,"web_url":"https://patchwork.libcamera.org/comment/4127/","msgid":"<20200320121214.GG5193@pendragon.ideasonboard.com>","date":"2020-03-20T12:12:14","subject":"Re: [libcamera-devel] [PATCH 03/11] libcamera: controls: Add\n\tsupport for string controls","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Fri, Mar 20, 2020 at 01:08:44PM +0100, Jacopo Mondi wrote:\n> On Mon, Mar 09, 2020 at 05:24:06PM +0100, Jacopo Mondi wrote:\n> > From: Laurent Pinchart \n> >\n> > String controls are stored internally as an array of char, but the\n> > ControlValue constructor, get() and set() functions operate on an\n> > std::string for convenience. Array of strings are thus not supported.\n> >\n> > Unlike for other control types, the ControlInfo range reports the\n> > minimum and maximum allowed lengths of the string (the minimum will\n> > usually be 0), not the minimum and maximum value of each element.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > include/libcamera/controls.h | 30 +++++++++++++++++++-----\n> > src/libcamera/control_serializer.cpp | 22 +++++++++++++++++\n> > src/libcamera/controls.cpp | 35 ++++++++++++++++++++++++----\n> > src/libcamera/gen-controls.py | 18 +++++++-------\n> > 4 files changed, 87 insertions(+), 18 deletions(-)\n> >\n> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > index 9c6cbffb88b5..5cf6280e612b 100644\n> > --- a/include/libcamera/controls.h\n> > +++ b/include/libcamera/controls.h\n> > @@ -26,6 +26,7 @@ enum ControlType {\n> > \tControlTypeInteger32,\n> > \tControlTypeInteger64,\n> > \tControlTypeFloat,\n> > +\tControlTypeString,\n> > };\n> >\n> > namespace details {\n> > @@ -64,6 +65,11 @@ struct control_type {\n> > \tstatic constexpr ControlType value = ControlTypeFloat;\n> > };\n> >\n> > +template<>\n> > +struct control_type {\n> > +\tstatic constexpr ControlType value = ControlTypeString;\n> > +};\n> > +\n> > template\n> > struct control_type> : public control_type> {\n> > };\n> > @@ -76,7 +82,9 @@ public:\n> > \tControlValue();\n> >\n> > #ifndef __DOXYGEN__\n> > -\ttemplate::value, std::nullptr_t> = nullptr>\n> > +\ttemplate::value &&\n> > +\t\t\t\t\t\t !std::is_same>::value,\n> > +\t\t\t\t\t\t std::nullptr_t> = nullptr>\n> > \tControlValue(const T &value)\n> > \t\t: type_(ControlTypeNone), numElements_(0)\n> > \t{\n> > @@ -84,7 +92,9 @@ public:\n> > \t\t &value, 1, sizeof(T));\n> > \t}\n> >\n> > -\ttemplate::value, std::nullptr_t> = nullptr>\n> > +\ttemplate::value ||\n> > +\t\t\t\t\t\t std::is_same>::value,\n> > +\t\t\t\t\t\t std::nullptr_t> = nullptr>\n> > #else\n> > \ttemplate\n> > #endif\n> > @@ -115,7 +125,9 @@ public:\n> > \t}\n> >\n> > #ifndef __DOXYGEN__\n> > -\ttemplate::value, std::nullptr_t> = nullptr>\n> > +\ttemplate::value &&\n> > +\t\t\t\t\t\t !std::is_same>::value,\n> > +\t\t\t\t\t\t std::nullptr_t> = nullptr>\n> > \tT get() const\n> > \t{\n> > \t\tassert(type_ == details::control_type>::value);\n> > @@ -124,7 +136,9 @@ public:\n> > \t\treturn *reinterpret_cast(data().data());\n> > \t}\n> >\n> > -\ttemplate::value, std::nullptr_t> = nullptr>\n> > +\ttemplate::value ||\n> > +\t\t\t\t\t\t std::is_same>::value,\n> > +\t\t\t\t\t\t std::nullptr_t> = nullptr>\n> > #else\n> > \ttemplate\n> > #endif\n> > @@ -139,14 +153,18 @@ public:\n> > \t}\n> >\n> > #ifndef __DOXYGEN__\n> > -\ttemplate::value, std::nullptr_t> = nullptr>\n> > +\ttemplate::value &&\n> > +\t\t\t\t\t\t !std::is_same>::value,\n> > +\t\t\t\t\t\t std::nullptr_t> = nullptr>\n> > \tvoid set(const T &value)\n> > \t{\n> > \t\tset(details::control_type>::value, false,\n> > \t\t reinterpret_cast(&value), 1, sizeof(T));\n> > \t}\n> >\n> > -\ttemplate::value, std::nullptr_t> = nullptr>\n> > +\ttemplate::value ||\n> > +\t\t\t\t\t\t std::is_same>::value,\n> > +\t\t\t\t\t\t std::nullptr_t> = nullptr>\n> > #else\n> > \ttemplate\n> > #endif\n> > diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp\n> > index eef875f4d96c..808419f246c0 100644\n> > --- a/src/libcamera/control_serializer.cpp\n> > +++ b/src/libcamera/control_serializer.cpp\n> > @@ -314,6 +314,22 @@ ControlValue ControlSerializer::loadControlValue(ByteStreamBuffer &buffer,\n> > \treturn value;\n> > }\n> >\n> > +template<>\n> > +ControlValue ControlSerializer::loadControlValue(ByteStreamBuffer &buffer,\n> > +\t\t\t\t\t\t bool isArray,\n> > +\t\t\t\t\t\t unsigned int count)\n> \n> Not may way around this specialization I gueess..\n> I see std::string being defined as std::basic_string and I was\n> wondering if we could reuse the template argument, but I don't\n> see a clear way to do so\n\nThis is also the part of this patch that I dislike, but for now I think\nit's the best option.\n\n> Anyway, ControlTypeFloat documentation removal apart\n> Reviewed-by: Jacopo Mondi \n> \n> > +{\n> > +\tControlValue value;\n> > +\n> > +\tconst char *data = buffer.read(count);\n> > +\tif (!data)\n> > +\t\treturn value;\n> > +\n> > +\tvalue.set(std::string{ data, count });\n> > +\n> > +\treturn value;\n> > +}\n> > +\n> > ControlValue ControlSerializer::loadControlValue(ControlType type,\n> > \t\t\t\t\t\t ByteStreamBuffer &buffer,\n> > \t\t\t\t\t\t bool isArray,\n> > @@ -335,6 +351,9 @@ ControlValue ControlSerializer::loadControlValue(ControlType type,\n> > \tcase ControlTypeFloat:\n> > \t\treturn loadControlValue(buffer, isArray, count);\n> >\n> > +\tcase ControlTypeString:\n> > +\t\treturn loadControlValue(buffer, isArray, count);\n> > +\n> > \tcase ControlTypeNone:\n> > \t\treturn ControlValue();\n> > \t}\n> > @@ -345,6 +364,9 @@ ControlValue ControlSerializer::loadControlValue(ControlType type,\n> > ControlInfo ControlSerializer::loadControlInfo(ControlType type,\n> > \t\t\t\t\t ByteStreamBuffer &b)\n> > {\n> > +\tif (type == ControlTypeString)\n> > +\t\ttype = ControlTypeInteger32;\n> > +\n> > \tControlValue min = loadControlValue(type, b);\n> > \tControlValue max = loadControlValue(type, b);\n> >\n> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\n> > index 53649fe897db..68d040db4daa 100644\n> > --- a/src/libcamera/controls.cpp\n> > +++ b/src/libcamera/controls.cpp\n> > @@ -57,6 +57,7 @@ static constexpr size_t ControlValueSize[] = {\n> > \t[ControlTypeInteger32]\t\t= sizeof(int32_t),\n> > \t[ControlTypeInteger64]\t\t= sizeof(int64_t),\n> > \t[ControlTypeFloat]\t\t= sizeof(float),\n> > +\t[ControlTypeString]\t\t= sizeof(char),\n> > };\n> >\n> > } /* namespace */\n> > @@ -74,8 +75,8 @@ static constexpr size_t ControlValueSize[] = {\n> > * The control stores a 32-bit integer value\n> > * \\var ControlTypeInteger64\n> > * The control stores a 64-bit integer value\n> > - * \\var ControlTypeFloat\n> > - * The control stores a 32-bit floating point value\n> \n> What did the poor Float type do to you ? :(\n\nOops. My sincere apologies to all the Floats in this world, I never\nmeant to wish for your disappearance, even if you can't express the\nspeed of light with enough accuracy\n(https://git.linuxtv.org/libcamera.git/tree/test/controls/control_value.cpp#n228).\n\n> > + * \\var ControlTypeString\n> > + * The control stores a string value as an array of char\n> > */\n> >\n> > /**\n> > @@ -164,7 +165,8 @@ ControlValue &ControlValue::operator=(const ControlValue &other)\n> > * \\brief Retrieve the number of elements stored in the ControlValue\n> > *\n> > * For instances storing an array, this function returns the number of elements\n> > - * in the array. Otherwise, it returns 1.\n> > + * in the array. For instances storing a string, it returns the length of the\n> > + * string, not counting the terminating '\\0'. Otherwise, it returns 1.\n> > *\n> > * \\return The number of elements stored in the ControlValue\n> > */\n> > @@ -192,6 +194,11 @@ std::string ControlValue::toString() const\n> > \t\treturn \"\";\n> >\n> > \tconst uint8_t *data = ControlValue::data().data();\n> > +\n> > +\tif (type_ == ControlTypeString)\n> > +\t\treturn std::string(reinterpret_cast(data),\n> > +\t\t\t\t numElements_);\n> > +\n> > \tstd::string str(isArray_ ? \"[ \" : \"\");\n> >\n> > \tfor (unsigned int i = 0; i < numElements_; ++i) {\n> > @@ -222,6 +229,7 @@ std::string ControlValue::toString() const\n> > \t\t\tbreak;\n> > \t\t}\n> > \t\tcase ControlTypeNone:\n> > +\t\tcase ControlTypeString:\n> > \t\t\tbreak;\n> > \t\t}\n> >\n> > @@ -439,12 +447,22 @@ ControlInfo::ControlInfo(const ControlValue &min,\n> > /**\n> > * \\fn ControlInfo::min()\n> > * \\brief Retrieve the minimum value of the control\n> > + *\n> > + * For string controls, this is the minimum length of the string, not counting\n> > + * the terminating '\\0'. For all other control types, this is the minimum value\n> > + * of each element.\n> > + *\n> > * \\return A ControlValue with the minimum value for the control\n> > */\n> >\n> > /**\n> > * \\fn ControlInfo::max()\n> > * \\brief Retrieve the maximum value of the control\n> > + *\n> > + * For string controls, this is the maximum length of the string, not counting\n> > + * the terminating '\\0'. For all other control types, this is the maximum value\n> > + * of each element.\n> > + *\n> > * \\return A ControlValue with the maximum value for the control\n> > */\n> >\n> > @@ -653,7 +671,16 @@ void ControlInfoMap::generateIdmap()\n> > \tidmap_.clear();\n> >\n> > \tfor (const auto &ctrl : *this) {\n> > -\t\tif (ctrl.first->type() != ctrl.second.min().type()) {\n> > +\t\t/*\n> > +\t\t * For string controls, min and max define the valid\n> > +\t\t * range for the string size, not for the individual\n> > +\t\t * values.\n> > +\t\t */\n> > +\t\tControlType rangeType = ctrl.first->type() == ControlTypeString\n> > +\t\t\t\t ? ControlTypeInteger32 : ctrl.first->type();\n> > +\t\tconst ControlInfo &info = ctrl.second;\n> > +\n> > +\t\tif (info.min().type() != rangeType) {\n> > \t\t\tLOG(Controls, Error)\n> > \t\t\t\t<< \"Control \" << utils::hex(ctrl.first->id())\n> > \t\t\t\t<< \" type and info type mismatch\";\n> > diff --git a/src/libcamera/gen-controls.py b/src/libcamera/gen-controls.py\n> > index ff8bda6b16c1..87c3d52ada6d 100755\n> > --- a/src/libcamera/gen-controls.py\n> > +++ b/src/libcamera/gen-controls.py\n> > @@ -42,10 +42,11 @@ ${description}\n> > name, ctrl = ctrl.popitem()\n> > id_name = snake_case(name).upper()\n> >\n> > - if ctrl.get('size'):\n> > - ctrl_type = 'Span' % ctrl['type']\n> > - else:\n> > - ctrl_type = ctrl['type']\n> > + ctrl_type = ctrl['type']\n> > + if ctrl_type == 'string':\n> > + ctrl_type = 'std::string'\n> > + elif ctrl.get('size'):\n> > + ctrl_type = 'Span' % ctrl_type\n> >\n> > info = {\n> > 'name': name,\n> > @@ -97,10 +98,11 @@ def generate_h(controls):\n> >\n> > ids.append('\\t' + id_name + ' = ' + str(id_value) + ',')\n> >\n> > - if ctrl.get('size'):\n> > - ctrl_type = 'Span' % ctrl['type']\n> > - else:\n> > - ctrl_type = ctrl['type']\n> > + ctrl_type = ctrl['type']\n> > + if ctrl_type == 'string':\n> > + ctrl_type = 'std::string'\n> > + elif ctrl.get('size'):\n> > + ctrl_type = 'Span' % ctrl_type\n> >\n> > info = {\n> > 'name': name,","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7C41F60416\n\tfor ;\n\tFri, 20 Mar 2020 13:12:20 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D9479504;\n\tFri, 20 Mar 2020 13:12:19 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1584706340;\n\tbh=DqVgtjBcbaS6kmLesHAQk7zrzpOAH/PsrqFIGQKpc4A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VJQQjN1Vtl5QQSEVoD035/6RHoo4PYeqUyfrMAs8SRlnMAVn13pytOdJAen3wPlWt\n\t6WOEv+SKuV3odK/vrAbvDxYEd8Ers8INyNbOL1PthE7JGXvq0zExcz+aMqVxZmLja3\n\tct6bLK1DGvpO3RiMF62zgQRUKye8UayKdYT04pXM=","Date":"Fri, 20 Mar 2020 14:12:14 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200320121214.GG5193@pendragon.ideasonboard.com>","References":"<20200309162414.720306-1-jacopo@jmondi.org>\n\t<20200309162414.720306-4-jacopo@jmondi.org>\n\t<20200320120844.2zch3cu2gajvjit2@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200320120844.2zch3cu2gajvjit2@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 03/11] libcamera: controls: Add\n\tsupport for string controls","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Fri, 20 Mar 2020 12:12:20 -0000"}}]