[libcamera-devel] [PATCH v2 1/2] libcamera: controls: Add read-only flag to ControlInfo

[Jacopo Mondi]

Hi Naush

On Fri, Dec 02, 2022 at 12:40:04PM +0000, Naushir Patuck via libcamera-devel wrote:
> Add a read-only flag to the ControlInfo flag to indicate whether a V4L2 control
> is read-only or not. This flag only makes sense for V2L2 controls at preset, so
> only update the ControlInfo constructor signature used by the V4L2Device class
> to set the value of the flag.
>

It feels a bit the three constructors are mis-aligned now :/

I would have gone for a default parameter for all three of them to be
honest... What do others think ?

> Add a ControlInfo::readOnly() member function to retrive this flag.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
>  include/libcamera/controls.h  |  5 ++++-
>  src/libcamera/controls.cpp    | 17 +++++++++++++----
>  src/libcamera/v4l2_device.cpp | 12 ++++++++----
>  3 files changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index cf94205577a5..488663a7ba04 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -270,7 +270,8 @@ class ControlInfo
>  public:
>  	explicit ControlInfo(const ControlValue &min = {},
>  			     const ControlValue &max = {},
> -			     const ControlValue &def = {});
> +			     const ControlValue &def = {},
> +			     bool readOnly = false);
>  	explicit ControlInfo(Span<const ControlValue> values,
>  			     const ControlValue &def = {});
>  	explicit ControlInfo(std::set<bool> values, bool def);
> @@ -279,6 +280,7 @@ public:
>  	const ControlValue &min() const { return min_; }
>  	const ControlValue &max() const { return max_; }
>  	const ControlValue &def() const { return def_; }
> +	bool readOnly() const { return readOnly_; }
>  	const std::vector<ControlValue> &values() const { return values_; }
>
>  	std::string toString() const;
> @@ -297,6 +299,7 @@ private:
>  	ControlValue min_;
>  	ControlValue max_;
>  	ControlValue def_;
> +	bool readOnly_;
>  	std::vector<ControlValue> values_;
>  };
>
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 6dbf9b348709..fc66abad600d 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -484,11 +484,13 @@ void ControlValue::reserve(ControlType type, bool isArray, std::size_t numElemen
>   * \param[in] min The control minimum value
>   * \param[in] max The control maximum value
>   * \param[in] def The control default value
> + * \param[in] readOnly Read-only status of a V4L2 control
>   */
>  ControlInfo::ControlInfo(const ControlValue &min,
>  			 const ControlValue &max,
> -			 const ControlValue &def)
> -	: min_(min), max_(max), def_(def)
> +			 const ControlValue &def,
> +			 bool readOnly)
> +	: min_(min), max_(max), def_(def), readOnly_(readOnly)
>  {
>  }
>
> @@ -525,7 +527,8 @@ ControlInfo::ControlInfo(Span<const ControlValue> values,
>   * default value is \a def.
>   */
>  ControlInfo::ControlInfo(std::set<bool> values, bool def)
> -	: min_(false), max_(true), def_(def), values_({ false, true })
> +	: min_(false), max_(true), def_(def), readOnly_(false),
> +	  values_({ false, true })
>  {
>  	ASSERT(values.count(def) && values.size() == 2);
>  }
> @@ -538,7 +541,7 @@ ControlInfo::ControlInfo(std::set<bool> values, bool def)
>   * value. The minimum, maximum, and default values will all be \a value.
>   */
>  ControlInfo::ControlInfo(bool value)
> -	: min_(value), max_(value), def_(value)
> +	: min_(value), max_(value), def_(value), readOnly_(false)
>  {
>  	values_ = { value };
>  }
> @@ -571,6 +574,12 @@ ControlInfo::ControlInfo(bool value)
>   * \return A ControlValue with the default value for the control
>   */
>
> +/**
> + * \fn ControlInfo::readOnly()
> + * \brief Identifies if a V4L2 control is flagged as read-only
> + * \return True if the V4L2 control is read-only, false otherwise

I would not mention V4L2 here and keep the documentation generic

> + */
> +
>  /**
>   * \fn ControlInfo::values()
>   * \brief Retrieve the list of valid values
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 57a88d96b12c..9018f1b0b9e1 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -535,17 +535,20 @@ std::optional<ControlInfo> V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl
>  	case V4L2_CTRL_TYPE_U8:
>  		return ControlInfo(static_cast<uint8_t>(ctrl.minimum),
>  				   static_cast<uint8_t>(ctrl.maximum),
> -				   static_cast<uint8_t>(ctrl.default_value));
> +				   static_cast<uint8_t>(ctrl.default_value),
> +				   !!(ctrl.flags & V4L2_CTRL_FLAG_READ_ONLY));
>
>  	case V4L2_CTRL_TYPE_BOOLEAN:
>  		return ControlInfo(static_cast<bool>(ctrl.minimum),
>  				   static_cast<bool>(ctrl.maximum),
> -				   static_cast<bool>(ctrl.default_value));
> +				   static_cast<bool>(ctrl.default_value),
> +				   !!(ctrl.flags & V4L2_CTRL_FLAG_READ_ONLY));
>
>  	case V4L2_CTRL_TYPE_INTEGER64:
>  		return ControlInfo(static_cast<int64_t>(ctrl.minimum),
>  				   static_cast<int64_t>(ctrl.maximum),
> -				   static_cast<int64_t>(ctrl.default_value));
> +				   static_cast<int64_t>(ctrl.default_value),
> +				   !!(ctrl.flags & V4L2_CTRL_FLAG_READ_ONLY));
>
>  	case V4L2_CTRL_TYPE_INTEGER_MENU:
>  	case V4L2_CTRL_TYPE_MENU:
> @@ -554,7 +557,8 @@ std::optional<ControlInfo> V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl
>  	default:
>  		return ControlInfo(static_cast<int32_t>(ctrl.minimum),
>  				   static_cast<int32_t>(ctrl.maximum),
> -				   static_cast<int32_t>(ctrl.default_value));
> +				   static_cast<int32_t>(ctrl.default_value),
> +				   !!(ctrl.flags & V4L2_CTRL_FLAG_READ_ONLY));
>  	}
>  }
>
> --
> 2.25.1
>