[libcamera-devel,1/2] Support float data type in options
diff mbox series

Message ID 20220421172549.138360-1-utkarsh02t@gmail.com
State Rejected
Headers show
Series
  • [libcamera-devel,1/2] Support float data type in options
Related show

Commit Message

Utkarsh Tiwari April 21, 2022, 5:25 p.m. UTC
---
 src/cam/options.cpp | 65 +++++++++++++++++++++++++++++++++++++++++++--
 src/cam/options.h   |  6 +++++
 2 files changed, 69 insertions(+), 2 deletions(-)

Comments

Kieran Bingham April 25, 2022, 4:07 p.m. UTC | #1
This needs a commit message, and a Signed-off-by: tag.

Quoting Utkarsh Tiwari via libcamera-devel (2022-04-21 18:25:48)
> ---
>  src/cam/options.cpp | 65 +++++++++++++++++++++++++++++++++++++++++++--
>  src/cam/options.h   |  6 +++++
>  2 files changed, 69 insertions(+), 2 deletions(-)
> 
> diff --git a/src/cam/options.cpp b/src/cam/options.cpp
> index 4f7e8691..759328fc 100644
> --- a/src/cam/options.cpp
> +++ b/src/cam/options.cpp
> @@ -6,9 +6,11 @@
>   */
>  
>  #include <assert.h>
> +#include <cmath>
>  #include <getopt.h>
>  #include <iomanip>
>  #include <iostream>
> +#include <limits>
>  #include <string.h>
>  
>  #include "options.h"
> @@ -43,6 +45,9 @@
>   *
>   * \var OptionType::OptionKeyValue
>   * \brief key=value list argument
> + *
> + * \var OptionType::OptionFloat
> + * \brief Float argument
>   */
>  
>  /* -----------------------------------------------------------------------------
> @@ -129,6 +134,9 @@ const char *Option::typeName() const
>  
>         case OptionKeyValue:
>                 return "key=value";
> +
> +       case OptionFloat:
> +               return "float";
>         }
>  
>         return "unknown";
> @@ -256,14 +264,14 @@ bool OptionsBase<T>::parseValue(const T &opt, const Option &option,
>                         integer = 0;
>                 }
>  
> -               value = OptionValue(integer);
> +               value = OptionValue(static_cast<int>(integer));
>                 break;
>  
>         case OptionString:
>                 value = OptionValue(arg ? arg : "");
>                 break;
>  
> -       case OptionKeyValue:
> +       case OptionKeyValue: {
>                 KeyValueParser *kvParser = option.keyValueParser;
>                 KeyValueParser::Options keyValues = kvParser->parse(arg);
>                 if (!keyValues.valid())
> @@ -273,6 +281,21 @@ bool OptionsBase<T>::parseValue(const T &opt, const Option &option,
>                 break;
>         }
>  
> +       case OptionFloat:
> +               float float_val;
> +
> +               if (arg) {
> +                       char *endptr;
> +                       float_val = strtof(arg, &endptr);
> +                       if (*endptr != '\0' || !std::isfinite(float_val))
> +                               return false;
> +               } else {
> +                       float_val = 0;
> +               }

I haven't looked at the other instances to identify if other types do
this - but I can't understand how it can make sense to have an
OptionFloat - without an argument value? Would that be a parser error
that needs to be propogated up?


> +               value = OptionValue(float_val);
> +               break;
> +       }
> +
>         if (option.isArray)
>                 values_[opt].addValue(value);
>         else
> @@ -283,6 +306,7 @@ bool OptionsBase<T>::parseValue(const T &opt, const Option &option,
>  
>  template class OptionsBase<int>;
>  template class OptionsBase<std::string>;
> +template class OptionsBase<float>;
>  
>  /* -----------------------------------------------------------------------------
>   * KeyValueParser
> @@ -505,6 +529,9 @@ void KeyValueParser::usage(int indent)
>   *
>   * \var OptionValue::ValueType::ValueArray
>   * \brief Array value
> + *
> + * \var OptionValue::ValueType::ValueFloat
> + * \brief Float value (float)
>   */
>  
>  /**
> @@ -561,6 +588,17 @@ OptionValue::OptionValue(const KeyValueParser::Options &value)
>  {
>  }
>  
> +/**
> + * \brief Construct an float OptionValue instance

s/an/a/

> + * \param[in] value The float value
> + *
> + * The value type is set to ValueType::ValueFloat.
> + */
> +OptionValue::OptionValue(const float value)
> +       : type_(ValueFloat), integer_(0), float_(value)
> +{
> +}
> +
>  /**
>   * \brief Add an entry to an array value
>   * \param[in] value The entry value
> @@ -600,6 +638,16 @@ OptionValue::operator int() const
>         return toInteger();
>  }
>  
> +/**
> + * \brief Cast the value to an int

to an int?

> + * \return The option value as an int, or 0 if the value type isn't

Same ?

> + * ValueType::ValueInteger

Same.

> + */
> +OptionValue::operator float() const
> +{
> +       return toFloat();
> +}
> +
>  /**
>   * \brief Cast the value to a std::string
>   * \return The option value as an std::string, or an empty string if the value
> @@ -662,6 +710,19 @@ const std::vector<OptionValue> &OptionValue::toArray() const
>         return array_;
>  }
>  
> +/**
> + * \brief Retrieve the value as an float

'as a float'

> + * \return The option value as an int, or signaling not-a-number if the value type isn't

as a float  ... not an int.

> + * ValueType::ValueFloat
> + */
> +float OptionValue::toFloat() const
> +{
> +       if (type_ != ValueFloat)
> +               return std::numeric_limits<float>::signaling_NaN();

What happens on Value types that /can/ convert to a float? For instance
an int could be turned into a float I presume ... (But I don't know if
this class  /should/ do that.)

Of course if we support Integer->float conversion, that could imply
supporting float->integer conversion (with precision loss) ...


> +
> +       return float_;
> +}
> +
>  /**
>   * \brief Retrieve the list of child values
>   * \return The list of child values
> diff --git a/src/cam/options.h b/src/cam/options.h
> index 4ddd4987..f0636f82 100644
> --- a/src/cam/options.h
> +++ b/src/cam/options.h
> @@ -28,6 +28,7 @@ enum OptionType {
>         OptionInteger,
>         OptionString,
>         OptionKeyValue,
> +       OptionFloat,
>  };
>  
>  template<typename T>
> @@ -124,6 +125,7 @@ public:
>                 ValueString,
>                 ValueKeyValue,
>                 ValueArray,
> +               ValueFloat,
>         };
>  
>         OptionValue();
> @@ -131,6 +133,7 @@ public:
>         OptionValue(const char *value);
>         OptionValue(const std::string &value);
>         OptionValue(const KeyValueParser::Options &value);
> +       OptionValue(const float value);
>  
>         void addValue(const OptionValue &value);
>  
> @@ -139,11 +142,13 @@ public:
>  
>         operator int() const;
>         operator std::string() const;
> +       operator float() const;
>  
>         int toInteger() const;
>         std::string toString() const;
>         const KeyValueParser::Options &toKeyValues() const;
>         const std::vector<OptionValue> &toArray() const;
> +       float toFloat() const;
>  
>         const OptionsParser::Options &children() const;
>  
> @@ -153,5 +158,6 @@ private:
>         std::string string_;
>         KeyValueParser::Options keyValues_;
>         std::vector<OptionValue> array_;
> +       float float_;
>         OptionsParser::Options children_;
>  };
> -- 
> 2.25.1
>

Patch
diff mbox series

diff --git a/src/cam/options.cpp b/src/cam/options.cpp
index 4f7e8691..759328fc 100644
--- a/src/cam/options.cpp
+++ b/src/cam/options.cpp
@@ -6,9 +6,11 @@ 
  */
 
 #include <assert.h>
+#include <cmath>
 #include <getopt.h>
 #include <iomanip>
 #include <iostream>
+#include <limits>
 #include <string.h>
 
 #include "options.h"
@@ -43,6 +45,9 @@ 
  *
  * \var OptionType::OptionKeyValue
  * \brief key=value list argument
+ *
+ * \var OptionType::OptionFloat
+ * \brief Float argument
  */
 
 /* -----------------------------------------------------------------------------
@@ -129,6 +134,9 @@  const char *Option::typeName() const
 
 	case OptionKeyValue:
 		return "key=value";
+
+	case OptionFloat:
+		return "float";
 	}
 
 	return "unknown";
@@ -256,14 +264,14 @@  bool OptionsBase<T>::parseValue(const T &opt, const Option &option,
 			integer = 0;
 		}
 
-		value = OptionValue(integer);
+		value = OptionValue(static_cast<int>(integer));
 		break;
 
 	case OptionString:
 		value = OptionValue(arg ? arg : "");
 		break;
 
-	case OptionKeyValue:
+	case OptionKeyValue: {
 		KeyValueParser *kvParser = option.keyValueParser;
 		KeyValueParser::Options keyValues = kvParser->parse(arg);
 		if (!keyValues.valid())
@@ -273,6 +281,21 @@  bool OptionsBase<T>::parseValue(const T &opt, const Option &option,
 		break;
 	}
 
+	case OptionFloat:
+		float float_val;
+
+		if (arg) {
+			char *endptr;
+			float_val = strtof(arg, &endptr);
+			if (*endptr != '\0' || !std::isfinite(float_val))
+				return false;
+		} else {
+			float_val = 0;
+		}
+		value = OptionValue(float_val);
+		break;
+	}
+
 	if (option.isArray)
 		values_[opt].addValue(value);
 	else
@@ -283,6 +306,7 @@  bool OptionsBase<T>::parseValue(const T &opt, const Option &option,
 
 template class OptionsBase<int>;
 template class OptionsBase<std::string>;
+template class OptionsBase<float>;
 
 /* -----------------------------------------------------------------------------
  * KeyValueParser
@@ -505,6 +529,9 @@  void KeyValueParser::usage(int indent)
  *
  * \var OptionValue::ValueType::ValueArray
  * \brief Array value
+ *
+ * \var OptionValue::ValueType::ValueFloat
+ * \brief Float value (float)
  */
 
 /**
@@ -561,6 +588,17 @@  OptionValue::OptionValue(const KeyValueParser::Options &value)
 {
 }
 
+/**
+ * \brief Construct an float OptionValue instance
+ * \param[in] value The float value
+ *
+ * The value type is set to ValueType::ValueFloat.
+ */
+OptionValue::OptionValue(const float value)
+	: type_(ValueFloat), integer_(0), float_(value)
+{
+}
+
 /**
  * \brief Add an entry to an array value
  * \param[in] value The entry value
@@ -600,6 +638,16 @@  OptionValue::operator int() const
 	return toInteger();
 }
 
+/**
+ * \brief Cast the value to an int
+ * \return The option value as an int, or 0 if the value type isn't
+ * ValueType::ValueInteger
+ */
+OptionValue::operator float() const
+{
+	return toFloat();
+}
+
 /**
  * \brief Cast the value to a std::string
  * \return The option value as an std::string, or an empty string if the value
@@ -662,6 +710,19 @@  const std::vector<OptionValue> &OptionValue::toArray() const
 	return array_;
 }
 
+/**
+ * \brief Retrieve the value as an float
+ * \return The option value as an int, or signaling not-a-number if the value type isn't
+ * ValueType::ValueFloat
+ */
+float OptionValue::toFloat() const
+{
+	if (type_ != ValueFloat)
+		return std::numeric_limits<float>::signaling_NaN();
+
+	return float_;
+}
+
 /**
  * \brief Retrieve the list of child values
  * \return The list of child values
diff --git a/src/cam/options.h b/src/cam/options.h
index 4ddd4987..f0636f82 100644
--- a/src/cam/options.h
+++ b/src/cam/options.h
@@ -28,6 +28,7 @@  enum OptionType {
 	OptionInteger,
 	OptionString,
 	OptionKeyValue,
+	OptionFloat,
 };
 
 template<typename T>
@@ -124,6 +125,7 @@  public:
 		ValueString,
 		ValueKeyValue,
 		ValueArray,
+		ValueFloat,
 	};
 
 	OptionValue();
@@ -131,6 +133,7 @@  public:
 	OptionValue(const char *value);
 	OptionValue(const std::string &value);
 	OptionValue(const KeyValueParser::Options &value);
+	OptionValue(const float value);
 
 	void addValue(const OptionValue &value);
 
@@ -139,11 +142,13 @@  public:
 
 	operator int() const;
 	operator std::string() const;
+	operator float() const;
 
 	int toInteger() const;
 	std::string toString() const;
 	const KeyValueParser::Options &toKeyValues() const;
 	const std::vector<OptionValue> &toArray() const;
+	float toFloat() const;
 
 	const OptionsParser::Options &children() const;
 
@@ -153,5 +158,6 @@  private:
 	std::string string_;
 	KeyValueParser::Options keyValues_;
 	std::vector<OptionValue> array_;
+	float float_;
 	OptionsParser::Options children_;
 };