[libcamera-devel,10/30] cam: options: Drop some OptionValue cast operators
diff mbox series

Message ID 20210707021941.20804-11-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • Multi-camera support in the cam application
Related show

Commit Message

Laurent Pinchart July 7, 2021, 2:19 a.m. UTC
While OptionValue cast operators to int and std::string allow useful
shortcut syntaxes, the cast operators to KeyValueParser::Options and
std::vector<OptionValue> are less useful. A an explicit static_cast call
would be more cumbersome to write than an explicit .toKeyValues() or
toArray(), and implicit cast hide too much of what's going on.

Drop those two cast operators, and replace the only implicit cast
occurrence with .toKeyValues(). While at it, drop the local opts
variable in StreamKeyValueParser::roles() as it isn't used.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/cam/options.cpp        | 20 --------------------
 src/cam/options.h          |  2 --
 src/cam/stream_options.cpp |  3 +--
 3 files changed, 1 insertion(+), 24 deletions(-)

Comments

Kieran Bingham July 12, 2021, 3:08 p.m. UTC | #1
On 07/07/2021 03:19, Laurent Pinchart wrote:
> While OptionValue cast operators to int and std::string allow useful
> shortcut syntaxes, the cast operators to KeyValueParser::Options and
> std::vector<OptionValue> are less useful. A an explicit static_cast call
> would be more cumbersome to write than an explicit .toKeyValues() or
> toArray(), and implicit cast hide too much of what's going on.
> 
> Drop those two cast operators, and replace the only implicit cast
> occurrence with .toKeyValues(). While at it, drop the local opts
> variable in StreamKeyValueParser::roles() as it isn't used.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  src/cam/options.cpp        | 20 --------------------
>  src/cam/options.h          |  2 --
>  src/cam/stream_options.cpp |  3 +--
>  3 files changed, 1 insertion(+), 24 deletions(-)
> 
> diff --git a/src/cam/options.cpp b/src/cam/options.cpp
> index d7f9f741c731..319fab6143c5 100644
> --- a/src/cam/options.cpp
> +++ b/src/cam/options.cpp
> @@ -604,26 +604,6 @@ OptionValue::operator std::string() const
>  	return toString();
>  }
>  
> -/**
> - * \brief Cast the value to a key-value list
> - * \return The option value as a KeyValueParser::Options, or an empty list if
> - * the value type isn't ValueType::ValueKeyValue
> - */
> -OptionValue::operator KeyValueParser::Options() const
> -{
> -	return toKeyValues();
> -}
> -
> -/**
> - * \brief Cast the value to an array
> - * \return The option value as a std::vector of OptionValue, or an empty vector
> - * if the value type isn't ValueType::ValueArray
> - */
> -OptionValue::operator std::vector<OptionValue>() const
> -{
> -	return toArray();
> -}
> -
>  /**
>   * \brief Retrieve the value as an int
>   * \return The option value as an int, or 0 if the value type isn't
> diff --git a/src/cam/options.h b/src/cam/options.h
> index e894822c0061..210e502a24e1 100644
> --- a/src/cam/options.h
> +++ b/src/cam/options.h
> @@ -138,8 +138,6 @@ public:
>  
>  	operator int() const;
>  	operator std::string() const;
> -	operator KeyValueParser::Options() const;
> -	operator std::vector<OptionValue>() const;
>  
>  	int toInteger() const;
>  	std::string toString() const;
> diff --git a/src/cam/stream_options.cpp b/src/cam/stream_options.cpp
> index c58272c23d25..b90dbb97f087 100644
> --- a/src/cam/stream_options.cpp
> +++ b/src/cam/stream_options.cpp
> @@ -48,11 +48,10 @@ StreamRoles StreamKeyValueParser::roles(const OptionValue &values)
>  
>  	StreamRoles roles;
>  	for (auto const &value : streamParameters) {
> -		KeyValueParser::Options opts = value.toKeyValues();
>  		StreamRole role;
>  
>  		/* If role is invalid or not set default to viewfinder. */
> -		if (!parseRole(&role, value))
> +		if (!parseRole(&role, value.toKeyValues()))
>  			role = StreamRole::Viewfinder;
>  
>  		roles.push_back(role);
>

Patch
diff mbox series

diff --git a/src/cam/options.cpp b/src/cam/options.cpp
index d7f9f741c731..319fab6143c5 100644
--- a/src/cam/options.cpp
+++ b/src/cam/options.cpp
@@ -604,26 +604,6 @@  OptionValue::operator std::string() const
 	return toString();
 }
 
-/**
- * \brief Cast the value to a key-value list
- * \return The option value as a KeyValueParser::Options, or an empty list if
- * the value type isn't ValueType::ValueKeyValue
- */
-OptionValue::operator KeyValueParser::Options() const
-{
-	return toKeyValues();
-}
-
-/**
- * \brief Cast the value to an array
- * \return The option value as a std::vector of OptionValue, or an empty vector
- * if the value type isn't ValueType::ValueArray
- */
-OptionValue::operator std::vector<OptionValue>() const
-{
-	return toArray();
-}
-
 /**
  * \brief Retrieve the value as an int
  * \return The option value as an int, or 0 if the value type isn't
diff --git a/src/cam/options.h b/src/cam/options.h
index e894822c0061..210e502a24e1 100644
--- a/src/cam/options.h
+++ b/src/cam/options.h
@@ -138,8 +138,6 @@  public:
 
 	operator int() const;
 	operator std::string() const;
-	operator KeyValueParser::Options() const;
-	operator std::vector<OptionValue>() const;
 
 	int toInteger() const;
 	std::string toString() const;
diff --git a/src/cam/stream_options.cpp b/src/cam/stream_options.cpp
index c58272c23d25..b90dbb97f087 100644
--- a/src/cam/stream_options.cpp
+++ b/src/cam/stream_options.cpp
@@ -48,11 +48,10 @@  StreamRoles StreamKeyValueParser::roles(const OptionValue &values)
 
 	StreamRoles roles;
 	for (auto const &value : streamParameters) {
-		KeyValueParser::Options opts = value.toKeyValues();
 		StreamRole role;
 
 		/* If role is invalid or not set default to viewfinder. */
-		if (!parseRole(&role, value))
+		if (!parseRole(&role, value.toKeyValues()))
 			role = StreamRole::Viewfinder;
 
 		roles.push_back(role);