[libcamera-devel,v2,03/13] libcamera: controls: Add supported values to ControlInfo
diff mbox series

Message ID 20201020180534.36855-4-jacopo@jmondi.org
State Superseded
Headers show
Series
  • libcamera: Introduce draft controls
Related show

Commit Message

Jacopo Mondi Oct. 20, 2020, 6:05 p.m. UTC
Add to the ControlInfo class a list of supported values that can be
provided at construction time and retrieved through an accessor method.

This is meant to support controls that have an enumerated list of
supported values.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 include/libcamera/controls.h |  5 ++++-
 src/libcamera/controls.cpp   | 22 +++++++++++++++++++---
 2 files changed, 23 insertions(+), 4 deletions(-)

Comments

Kieran Bingham Oct. 21, 2020, 12:47 p.m. UTC | #1
Hi Jacopo,

On 20/10/2020 19:05, Jacopo Mondi wrote:
> Add to the ControlInfo class a list of supported values that can be
> provided at construction time and retrieved through an accessor method.
> 
> This is meant to support controls that have an enumerated list of
> supported values.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  include/libcamera/controls.h |  5 ++++-
>  src/libcamera/controls.cpp   | 22 +++++++++++++++++++---
>  2 files changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 80944efc133a..d1f6d4490c35 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -267,11 +267,13 @@ class ControlInfo
>  public:
>  	explicit ControlInfo(const ControlValue &min = 0,
>  			     const ControlValue &max = 0,
> -			     const ControlValue &def = 0);
> +			     const ControlValue &def = 0,
> +			     const std::vector<ControlValue> &values = {});
>  
>  	const ControlValue &min() const { return min_; }
>  	const ControlValue &max() const { return max_; }
>  	const ControlValue &def() const { return def_; }
> +	const std::vector<ControlValue> &values() const { return values_; }
>  
>  	std::string toString() const;
>  
> @@ -289,6 +291,7 @@ private:
>  	ControlValue min_;
>  	ControlValue max_;
>  	ControlValue def_;
> +	std::vector<ControlValue> values_;
>  };
>  
>  using ControlIdMap = std::unordered_map<unsigned int, const ControlId *>;
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index dca782667d88..61feee37a1b8 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -479,15 +479,17 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
>   */
>  
>  /**
> - * \brief Construct a ControlInfo with minimum and maximum range parameters
> + * \brief Construct a ControlInfo with parameters
>   * \param[in] min The control minimum value
>   * \param[in] max The control maximum value
>   * \param[in] def The control default value
> + * \param[in] values The control supported values

How does this interact with min/max? I presume they should still get set
by the min/max of the values in the vector?


Perhaps there should be another Constructor for ControlInfo which takes
a set of values, and a default instead?

And then, the min/max would be set to the min/max of the values
provided...? or perhaps ignored?

I assume if there is a list of values given, then the min/max is a
little irrelevant?


>   */
>  ControlInfo::ControlInfo(const ControlValue &min,
>  			 const ControlValue &max,
> -			 const ControlValue &def)
> -	: min_(min), max_(max), def_(def)
> +			 const ControlValue &def,
> +			 const std::vector<ControlValue> &values)
> +	: min_(min), max_(max), def_(def), values_(values)
>  {
>  }
>  
> @@ -519,6 +521,20 @@ ControlInfo::ControlInfo(const ControlValue &min,
>   * \return A ControlValue with the default value for the control
>   */
>  
> +/**
> + * \fn ControlInfo::values()
> + * \brief Retrieve the values supported by the control
> + *
> + * For controls that support a pre-defined number of values, the enumeration of
> + * those is reported through a vector of ControlValue instances accessible with
> + * this method.
> + *
> + * If the control reports a list of supported values, setting values outside
> + * of the reported ones results in undefined behaviour.
> + *
> + * \return A vector of ControlValue instances with the supported values
> + */
> +
>  /**
>   * \brief Provide a string representation of the ControlInfo
>   */
>
Kieran Bingham Oct. 21, 2020, 12:51 p.m. UTC | #2
On 21/10/2020 13:47, Kieran Bingham wrote:
> Hi Jacopo,
> 
> On 20/10/2020 19:05, Jacopo Mondi wrote:
>> Add to the ControlInfo class a list of supported values that can be
>> provided at construction time and retrieved through an accessor method.
>>
>> This is meant to support controls that have an enumerated list of
>> supported values.
>>
>> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
>> ---
>>  include/libcamera/controls.h |  5 ++++-
>>  src/libcamera/controls.cpp   | 22 +++++++++++++++++++---
>>  2 files changed, 23 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
>> index 80944efc133a..d1f6d4490c35 100644
>> --- a/include/libcamera/controls.h
>> +++ b/include/libcamera/controls.h
>> @@ -267,11 +267,13 @@ class ControlInfo
>>  public:
>>  	explicit ControlInfo(const ControlValue &min = 0,
>>  			     const ControlValue &max = 0,
>> -			     const ControlValue &def = 0);
>> +			     const ControlValue &def = 0,
>> +			     const std::vector<ControlValue> &values = {});
>>  
>>  	const ControlValue &min() const { return min_; }
>>  	const ControlValue &max() const { return max_; }
>>  	const ControlValue &def() const { return def_; }
>> +	const std::vector<ControlValue> &values() const { return values_; }
>>  
>>  	std::string toString() const;
>>  
>> @@ -289,6 +291,7 @@ private:
>>  	ControlValue min_;
>>  	ControlValue max_;
>>  	ControlValue def_;
>> +	std::vector<ControlValue> values_;
>>  };
>>  
>>  using ControlIdMap = std::unordered_map<unsigned int, const ControlId *>;
>> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
>> index dca782667d88..61feee37a1b8 100644
>> --- a/src/libcamera/controls.cpp
>> +++ b/src/libcamera/controls.cpp
>> @@ -479,15 +479,17 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
>>   */
>>  
>>  /**
>> - * \brief Construct a ControlInfo with minimum and maximum range parameters
>> + * \brief Construct a ControlInfo with parameters
>>   * \param[in] min The control minimum value
>>   * \param[in] max The control maximum value
>>   * \param[in] def The control default value
>> + * \param[in] values The control supported values
> 
> How does this interact with min/max? I presume they should still get set
> by the min/max of the values in the vector?
> 
> 
> Perhaps there should be another Constructor for ControlInfo which takes
> a set of values, and a default instead?
> 
> And then, the min/max would be set to the min/max of the values
> provided...? or perhaps ignored?
> 
> I assume if there is a list of values given, then the min/max is a
> little irrelevant?

If only I could see the future, then old me here, would know that new me
now would find the suggestion already completed in the next patch.

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

> 
> 
>>   */
>>  ControlInfo::ControlInfo(const ControlValue &min,
>>  			 const ControlValue &max,
>> -			 const ControlValue &def)
>> -	: min_(min), max_(max), def_(def)
>> +			 const ControlValue &def,
>> +			 const std::vector<ControlValue> &values)
>> +	: min_(min), max_(max), def_(def), values_(values)
>>  {
>>  }
>>  
>> @@ -519,6 +521,20 @@ ControlInfo::ControlInfo(const ControlValue &min,
>>   * \return A ControlValue with the default value for the control
>>   */
>>  
>> +/**
>> + * \fn ControlInfo::values()
>> + * \brief Retrieve the values supported by the control
>> + *
>> + * For controls that support a pre-defined number of values, the enumeration of
>> + * those is reported through a vector of ControlValue instances accessible with
>> + * this method.
>> + *
>> + * If the control reports a list of supported values, setting values outside
>> + * of the reported ones results in undefined behaviour.
>> + *
>> + * \return A vector of ControlValue instances with the supported values
>> + */
>> +
>>  /**
>>   * \brief Provide a string representation of the ControlInfo
>>   */
>>
>

Patch
diff mbox series

diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
index 80944efc133a..d1f6d4490c35 100644
--- a/include/libcamera/controls.h
+++ b/include/libcamera/controls.h
@@ -267,11 +267,13 @@  class ControlInfo
 public:
 	explicit ControlInfo(const ControlValue &min = 0,
 			     const ControlValue &max = 0,
-			     const ControlValue &def = 0);
+			     const ControlValue &def = 0,
+			     const std::vector<ControlValue> &values = {});
 
 	const ControlValue &min() const { return min_; }
 	const ControlValue &max() const { return max_; }
 	const ControlValue &def() const { return def_; }
+	const std::vector<ControlValue> &values() const { return values_; }
 
 	std::string toString() const;
 
@@ -289,6 +291,7 @@  private:
 	ControlValue min_;
 	ControlValue max_;
 	ControlValue def_;
+	std::vector<ControlValue> values_;
 };
 
 using ControlIdMap = std::unordered_map<unsigned int, const ControlId *>;
diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
index dca782667d88..61feee37a1b8 100644
--- a/src/libcamera/controls.cpp
+++ b/src/libcamera/controls.cpp
@@ -479,15 +479,17 @@  void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
  */
 
 /**
- * \brief Construct a ControlInfo with minimum and maximum range parameters
+ * \brief Construct a ControlInfo with parameters
  * \param[in] min The control minimum value
  * \param[in] max The control maximum value
  * \param[in] def The control default value
+ * \param[in] values The control supported values
  */
 ControlInfo::ControlInfo(const ControlValue &min,
 			 const ControlValue &max,
-			 const ControlValue &def)
-	: min_(min), max_(max), def_(def)
+			 const ControlValue &def,
+			 const std::vector<ControlValue> &values)
+	: min_(min), max_(max), def_(def), values_(values)
 {
 }
 
@@ -519,6 +521,20 @@  ControlInfo::ControlInfo(const ControlValue &min,
  * \return A ControlValue with the default value for the control
  */
 
+/**
+ * \fn ControlInfo::values()
+ * \brief Retrieve the values supported by the control
+ *
+ * For controls that support a pre-defined number of values, the enumeration of
+ * those is reported through a vector of ControlValue instances accessible with
+ * this method.
+ *
+ * If the control reports a list of supported values, setting values outside
+ * of the reported ones results in undefined behaviour.
+ *
+ * \return A vector of ControlValue instances with the supported values
+ */
+
 /**
  * \brief Provide a string representation of the ControlInfo
  */