[libcamera-devel] [PATCH 1/2] Support float data type in options

[Kieran Bingham]

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
>