[libcamera-devel] [PATCH 10/31] libcamera: controls: Return control by value in ControlList::get()

[Kieran Bingham]

Heya,

It's both the ControlList::get() and ControlValue::get() isn't it? so
maybe just
  s/in ControlList::get()//

on $SUBJECT

On 29/02/2020 16:42, Laurent Pinchart wrote:
> The ControlList::get() and ControlValue::get() methods return the
> control value by reference. This requires the ControlValue class to
> store the control value in the same form as the one returned by those
> functions. For compound controls that are soon to be added, the

/For compound/For the compound/

> ControlValue class would need to store a span<> instance in addition to
> the control value itself, which would increase the required storage
> space.
> 
> Prepare for support of compound controls by returning from get() by
> value. As all control values are 8 bytes at most, this doesn't affect
> efficiency negatively.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>


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

> ---
>  include/libcamera/controls.h | 10 ++++------
>  src/libcamera/controls.cpp   | 10 +++++-----
>  2 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 9d93064c11b2..3b6b231c7c64 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -42,7 +42,7 @@ public:
>  	}
>  
>  	template<typename T>
> -	const T &get() const;
> +	T get() const;
>  	template<typename T>
>  	void set(const T &value);
>  
> @@ -212,13 +212,11 @@ public:
>  	bool contains(unsigned int id) const;
>  
>  	template<typename T>
> -	const T &get(const Control<T> &ctrl) const
> +	T get(const Control<T> &ctrl) const
>  	{
>  		const ControlValue *val = find(ctrl.id());
> -		if (!val) {
> -			static T t(0);
> -			return t;
> -		}
> +		if (!val)
> +			return T{};
>  
>  		return val->get<T>();
>  	}
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index a136ebd2653b..6a0d66fbea8d 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -160,7 +160,7 @@ bool ControlValue::operator==(const ControlValue &other) const
>   */
>  
>  /**
> - * \fn template<typename T> const T &ControlValue::get() const
> + * \fn template<typename T> T ControlValue::get() const
>   * \brief Get the control value
>   *
>   * The control value type shall match the type T, otherwise the behaviour is
> @@ -177,7 +177,7 @@ bool ControlValue::operator==(const ControlValue &other) const
>  
>  #ifndef __DOXYGEN__
>  template<>
> -const bool &ControlValue::get<bool>() const
> +bool ControlValue::get<bool>() const
>  {
>  	ASSERT(type_ == ControlTypeBool);
>  
> @@ -185,7 +185,7 @@ const bool &ControlValue::get<bool>() const
>  }
>  
>  template<>
> -const int32_t &ControlValue::get<int32_t>() const
> +int32_t ControlValue::get<int32_t>() const
>  {
>  	ASSERT(type_ == ControlTypeInteger32);
>  
> @@ -193,7 +193,7 @@ const int32_t &ControlValue::get<int32_t>() const
>  }
>  
>  template<>
> -const int64_t &ControlValue::get<int64_t>() const
> +int64_t ControlValue::get<int64_t>() const
>  {
>  	ASSERT(type_ == ControlTypeInteger64);
>  
> @@ -720,7 +720,7 @@ bool ControlList::contains(unsigned int id) const
>  }
>  
>  /**
> - * \fn template<typename T> const T &ControlList::get(const Control<T> &ctrl) const
> + * \fn template<typename T> T ControlList::get(const Control<T> &ctrl) const
>   * \brief Get the value of control \a ctrl
>   * \param[in] ctrl The control
>   *
> 

-- 
Regards
--
Kieran