[{"id":22788,"web_url":"https://patchwork.libcamera.org/comment/22788/","msgid":"<165090282112.3782398.16727100573934550858@Monstersaurus>","date":"2022-04-25T16:07:01","subject":"Re: [libcamera-devel] [PATCH 1/2] Support float data type in options","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"This needs a commit message, and a Signed-off-by: tag.\n\nQuoting Utkarsh Tiwari via libcamera-devel (2022-04-21 18:25:48)\n> ---\n> src/cam/options.cpp | 65 +++++++++++++++++++++++++++++++++++++++++++--\n> src/cam/options.h | 6 +++++\n> 2 files changed, 69 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/cam/options.cpp b/src/cam/options.cpp\n> index 4f7e8691..759328fc 100644\n> --- a/src/cam/options.cpp\n> +++ b/src/cam/options.cpp\n> @@ -6,9 +6,11 @@\n> */\n> \n> #include \n> +#include \n> #include \n> #include \n> #include \n> +#include \n> #include \n> \n> #include \"options.h\"\n> @@ -43,6 +45,9 @@\n> *\n> * \\var OptionType::OptionKeyValue\n> * \\brief key=value list argument\n> + *\n> + * \\var OptionType::OptionFloat\n> + * \\brief Float argument\n> */\n> \n> /* -----------------------------------------------------------------------------\n> @@ -129,6 +134,9 @@ const char *Option::typeName() const\n> \n> case OptionKeyValue:\n> return \"key=value\";\n> +\n> + case OptionFloat:\n> + return \"float\";\n> }\n> \n> return \"unknown\";\n> @@ -256,14 +264,14 @@ bool OptionsBase::parseValue(const T &opt, const Option &option,\n> integer = 0;\n> }\n> \n> - value = OptionValue(integer);\n> + value = OptionValue(static_cast(integer));\n> break;\n> \n> case OptionString:\n> value = OptionValue(arg ? arg : \"\");\n> break;\n> \n> - case OptionKeyValue:\n> + case OptionKeyValue: {\n> KeyValueParser *kvParser = option.keyValueParser;\n> KeyValueParser::Options keyValues = kvParser->parse(arg);\n> if (!keyValues.valid())\n> @@ -273,6 +281,21 @@ bool OptionsBase::parseValue(const T &opt, const Option &option,\n> break;\n> }\n> \n> + case OptionFloat:\n> + float float_val;\n> +\n> + if (arg) {\n> + char *endptr;\n> + float_val = strtof(arg, &endptr);\n> + if (*endptr != '\\0' || !std::isfinite(float_val))\n> + return false;\n> + } else {\n> + float_val = 0;\n> + }\n\nI haven't looked at the other instances to identify if other types do\nthis - but I can't understand how it can make sense to have an\nOptionFloat - without an argument value? Would that be a parser error\nthat needs to be propogated up?\n\n\n> + value = OptionValue(float_val);\n> + break;\n> + }\n> +\n> if (option.isArray)\n> values_[opt].addValue(value);\n> else\n> @@ -283,6 +306,7 @@ bool OptionsBase::parseValue(const T &opt, const Option &option,\n> \n> template class OptionsBase;\n> template class OptionsBase;\n> +template class OptionsBase;\n> \n> /* -----------------------------------------------------------------------------\n> * KeyValueParser\n> @@ -505,6 +529,9 @@ void KeyValueParser::usage(int indent)\n> *\n> * \\var OptionValue::ValueType::ValueArray\n> * \\brief Array value\n> + *\n> + * \\var OptionValue::ValueType::ValueFloat\n> + * \\brief Float value (float)\n> */\n> \n> /**\n> @@ -561,6 +588,17 @@ OptionValue::OptionValue(const KeyValueParser::Options &value)\n> {\n> }\n> \n> +/**\n> + * \\brief Construct an float OptionValue instance\n\ns/an/a/\n\n> + * \\param[in] value The float value\n> + *\n> + * The value type is set to ValueType::ValueFloat.\n> + */\n> +OptionValue::OptionValue(const float value)\n> + : type_(ValueFloat), integer_(0), float_(value)\n> +{\n> +}\n> +\n> /**\n> * \\brief Add an entry to an array value\n> * \\param[in] value The entry value\n> @@ -600,6 +638,16 @@ OptionValue::operator int() const\n> return toInteger();\n> }\n> \n> +/**\n> + * \\brief Cast the value to an int\n\nto an int?\n\n> + * \\return The option value as an int, or 0 if the value type isn't\n\nSame ?\n\n> + * ValueType::ValueInteger\n\nSame.\n\n> + */\n> +OptionValue::operator float() const\n> +{\n> + return toFloat();\n> +}\n> +\n> /**\n> * \\brief Cast the value to a std::string\n> * \\return The option value as an std::string, or an empty string if the value\n> @@ -662,6 +710,19 @@ const std::vector &OptionValue::toArray() const\n> return array_;\n> }\n> \n> +/**\n> + * \\brief Retrieve the value as an float\n\n'as a float'\n\n> + * \\return The option value as an int, or signaling not-a-number if the value type isn't\n\nas a float ... not an int.\n\n> + * ValueType::ValueFloat\n> + */\n> +float OptionValue::toFloat() const\n> +{\n> + if (type_ != ValueFloat)\n> + return std::numeric_limits::signaling_NaN();\n\nWhat happens on Value types that /can/ convert to a float? For instance\nan int could be turned into a float I presume ... (But I don't know if\nthis class /should/ do that.)\n\nOf course if we support Integer->float conversion, that could imply\nsupporting float->integer conversion (with precision loss) ...\n\n\n> +\n> + return float_;\n> +}\n> +\n> /**\n> * \\brief Retrieve the list of child values\n> * \\return The list of child values\n> diff --git a/src/cam/options.h b/src/cam/options.h\n> index 4ddd4987..f0636f82 100644\n> --- a/src/cam/options.h\n> +++ b/src/cam/options.h\n> @@ -28,6 +28,7 @@ enum OptionType {\n> OptionInteger,\n> OptionString,\n> OptionKeyValue,\n> + OptionFloat,\n> };\n> \n> template\n> @@ -124,6 +125,7 @@ public:\n> ValueString,\n> ValueKeyValue,\n> ValueArray,\n> + ValueFloat,\n> };\n> \n> OptionValue();\n> @@ -131,6 +133,7 @@ public:\n> OptionValue(const char *value);\n> OptionValue(const std::string &value);\n> OptionValue(const KeyValueParser::Options &value);\n> + OptionValue(const float value);\n> \n> void addValue(const OptionValue &value);\n> \n> @@ -139,11 +142,13 @@ public:\n> \n> operator int() const;\n> operator std::string() const;\n> + operator float() const;\n> \n> int toInteger() const;\n> std::string toString() const;\n> const KeyValueParser::Options &toKeyValues() const;\n> const std::vector &toArray() const;\n> + float toFloat() const;\n> \n> const OptionsParser::Options &children() const;\n> \n> @@ -153,5 +158,6 @@ private:\n> std::string string_;\n> KeyValueParser::Options keyValues_;\n> std::vector array_;\n> + float float_;\n> OptionsParser::Options children_;\n> };\n> -- \n> 2.25.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 517FFC3256\n\tfor ;\n\tMon, 25 Apr 2022 16:07:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7E9AC65649;\n\tMon, 25 Apr 2022 18:07:04 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B9B43604A9\n\tfor ;\n\tMon, 25 Apr 2022 18:07:03 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4524A496;\n\tMon, 25 Apr 2022 18:07:03 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1650902824;\n\tbh=X5f0ZD9AMJ9mb3H8tcISzfbWjGTAZ66sC9Ho8EsuV6Y=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=FYu9HFELZMUNxAemJ7vWFZz9qctDC8ha1+0UAULCAKVGhVSoMtH2nMM2gL12M14V2\n\tmpjKbTpgxYmnD4QqMynitCjymIH/BdkcglctWHjlD7Uqoonp3oZa83aJIBWsXgtXSQ\n\txRhAr9HG619v4E6hrxMQ6Ti87I5wGK4D0t4AKjSONV9eJ3UoOFiBuagJBvxDbGVdEU\n\tx67wb9Zy5Lyg8LlvqYltXryla3gYy+XuIIDiM/hLnisYZobIViomLFYQwKVtB4Y05c\n\t414bhCDumNiGrlieBHlLB5wp/S1mWeswDy7FBhoqJBJlKmvzsGhBPWnIh2+xlL+xb3\n\t2DseQ+57AiGuw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1650902823;\n\tbh=X5f0ZD9AMJ9mb3H8tcISzfbWjGTAZ66sC9Ho8EsuV6Y=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=qeBnm+tk/2vQeCA5IRBIoj/Uap7Ksn0IJhbkaSzTDAj7y5AunJt8HXlR0rMBEfXCy\n\tSVFZ4f+cNpJ8WxcDFtCvcxVxhOna9MyiBhS8Nf/LdRybDRQcbtY2+PT/nXh3Zd1C/N\n\tOVYkyfXpRkDxHKGWAaMxvEpMEfVda6PSucDgMBfE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"qeBnm+tk\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220421172549.138360-1-utkarsh02t@gmail.com>","References":"<20220421172549.138360-1-utkarsh02t@gmail.com>","To":"Utkarsh Tiwari ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 25 Apr 2022 17:07:01 +0100","Message-ID":"<165090282112.3782398.16727100573934550858@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH 1/2] Support float data type in options","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]