[libcamera-devel,03/12] libcamera: controls: Use explicit 32-bit integer types
diff mbox series

Message ID 20190928152238.23752-4-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • Improve the application-facing controls API
Related show

Commit Message

Laurent Pinchart Sept. 28, 2019, 3:22 p.m. UTC 
Make the control API more explicit when dealing with integer controls by
specifying the size. We already do so for 64-bit integers, using int64_t
and ControlTypeInteger64, do the same for 32-bit integers.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/controls.h        |  6 ++---
 src/libcamera/controls.cpp          | 36 ++++++++++++++---------------
 src/libcamera/pipeline/uvcvideo.cpp | 10 ++++----
 src/libcamera/pipeline/vimc.cpp     |  6 ++---
 test/controls/control_info.cpp      | 10 ++++----
 test/controls/control_list.cpp      | 16 ++++++-------
 test/controls/control_value.cpp     |  6 ++---
 7 files changed, 46 insertions(+), 44 deletions(-)

Comments

Jacopo Mondi Sept. 29, 2019, 9:08 a.m. UTC | #1 
Hi Laurent,

On Sat, Sep 28, 2019 at 06:22:29PM +0300, Laurent Pinchart wrote:
> Make the control API more explicit when dealing with integer controls by
> specifying the size. We already do so for 64-bit integers, using int64_t
> and ControlTypeInteger64, do the same for 32-bit integers.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  include/libcamera/controls.h        |  6 ++---
>  src/libcamera/controls.cpp          | 36 ++++++++++++++---------------
>  src/libcamera/pipeline/uvcvideo.cpp | 10 ++++----
>  src/libcamera/pipeline/vimc.cpp     |  6 ++---
>  test/controls/control_info.cpp      | 10 ++++----
>  test/controls/control_list.cpp      | 16 ++++++-------
>  test/controls/control_value.cpp     |  6 ++---
>  7 files changed, 46 insertions(+), 44 deletions(-)
>
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 0886370f0901..a3089064c4b5 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -21,7 +21,7 @@ class Camera;
>  enum ControlType {
>  	ControlTypeNone,
>  	ControlTypeBool,
> -	ControlTypeInteger,
> +	ControlTypeInteger32,
>  	ControlTypeInteger64,
>  };
>
> @@ -30,7 +30,7 @@ class ControlValue
>  public:
>  	ControlValue();
>  	ControlValue(bool value);
> -	ControlValue(int value);
> +	ControlValue(int32_t value);
>  	ControlValue(int64_t value);
>
>  	ControlType type() const { return type_; };
> @@ -48,7 +48,7 @@ private:
>
>  	union {
>  		bool bool_;
> -		int integer_;
> +		int32_t integer32_;
>  		int64_t integer64_;
>  	};
>  };
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 88aab88db327..295ccd55a622 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -31,8 +31,8 @@ LOG_DEFINE_CATEGORY(Controls)
>   * Invalid type, for empty values
>   * \var ControlTypeBool
>   * The control stores a boolean value
> - * \var ControlTypeInteger
> - * The control stores an integer value
> + * \var ControlTypeInteger32
> + * The control stores a 32-bit integer value
>   * \var ControlTypeInteger64
>   * The control stores a 64-bit integer value
>   */
> @@ -63,8 +63,8 @@ ControlValue::ControlValue(bool value)
>   * \brief Construct an integer ControlValue
>   * \param[in] value Integer value to store
>   */
> -ControlValue::ControlValue(int value)
> -	: type_(ControlTypeInteger), integer_(value)
> +ControlValue::ControlValue(int32_t value)
> +	: type_(ControlTypeInteger32), integer32_(value)
>  {
>  }
>
> @@ -115,17 +115,17 @@ const bool &ControlValue::get<bool>() const
>  }
>
>  template<>
> -const int &ControlValue::get<int>() const
> +const int32_t &ControlValue::get<int32_t>() const
>  {
> -	ASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64);
> +	ASSERT(type_ == ControlTypeInteger32 || type_ == ControlTypeInteger64);
>
> -	return integer_;
> +	return integer32_;
>  }
>
>  template<>
>  const int64_t &ControlValue::get<int64_t>() const
>  {
> -	ASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64);
> +	ASSERT(type_ == ControlTypeInteger32 || type_ == ControlTypeInteger64);
>
>  	return integer64_;
>  }
> @@ -138,10 +138,10 @@ void ControlValue::set<bool>(const bool &value)
>  }
>
>  template<>
> -void ControlValue::set<int>(const int &value)
> +void ControlValue::set<int32_t>(const int32_t &value)
>  {
> -	type_ = ControlTypeInteger;
> -	integer_ = value;
> +	type_ = ControlTypeInteger32;
> +	integer32_ = value;
>  }
>
>  template<>
> @@ -163,8 +163,8 @@ std::string ControlValue::toString() const
>  		return "<None>";
>  	case ControlTypeBool:
>  		return bool_ ? "True" : "False";
> -	case ControlTypeInteger:
> -		return std::to_string(integer_);
> +	case ControlTypeInteger32:
> +		return std::to_string(integer32_);
>  	case ControlTypeInteger64:
>  		return std::to_string(integer64_);
>  	}
> @@ -186,35 +186,35 @@ std::string ControlValue::toString() const
>
>  /**
>   * \var Brightness
> - * ControlType: Integer
> + * ControlType: Integer32
>   *
>   * Specify a fixed brightness parameter.
>   */
>
>  /**
>   * \var Contrast
> - * ControlType: Integer
> + * ControlType: Integer32
>   *
>   * Specify a fixed contrast parameter.
>   */
>
>  /**
>   * \var Saturation
> - * ControlType: Integer
> + * ControlType: Integer32
>   *
>   * Specify a fixed saturation parameter.
>   */
>
>  /**
>   * \var ManualExposure
> - * ControlType: Integer
> + * ControlType: Integer32
>   *
>   * Specify a fixed exposure time in milli-seconds
>   */
>
>  /**
>   * \var ManualGain
> - * ControlType: Integer
> + * ControlType: Integer32
>   *
>   * Specify a fixed gain parameter
>   */
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 81c548af2c64..0d56758e3e1d 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -235,24 +235,24 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)
>
>  		switch (ci->id()) {
>  		case Brightness:
> -			controls.add(V4L2_CID_BRIGHTNESS, value.get<int>());
> +			controls.add(V4L2_CID_BRIGHTNESS, value.get<int32_t>());

Due to C++ internal type casting rules (I presume) the previous syntax
value.get<int>() is still legal. I assume the template argument
substitution process matches this with the <int32_t> specialization.
Shouldn't we keep using the shorter one ?

Anyway
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j

>  			break;
>
>  		case Contrast:
> -			controls.add(V4L2_CID_CONTRAST, value.get<int>());
> +			controls.add(V4L2_CID_CONTRAST, value.get<int32_t>());
>  			break;
>
>  		case Saturation:
> -			controls.add(V4L2_CID_SATURATION, value.get<int>());
> +			controls.add(V4L2_CID_SATURATION, value.get<int32_t>());
>  			break;
>
>  		case ManualExposure:
>  			controls.add(V4L2_CID_EXPOSURE_AUTO, 1);
> -			controls.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get<int>());
> +			controls.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get<int32_t>());
>  			break;
>
>  		case ManualGain:
> -			controls.add(V4L2_CID_GAIN, value.get<int>());
> +			controls.add(V4L2_CID_GAIN, value.get<int32_t>());
>  			break;
>
>  		default:
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index 3e34e7a0edbf..e549dc64b996 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -288,15 +288,15 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
>
>  		switch (ci->id()) {
>  		case Brightness:
> -			controls.add(V4L2_CID_BRIGHTNESS, value.get<int>());
> +			controls.add(V4L2_CID_BRIGHTNESS, value.get<int32_t>());
>  			break;
>
>  		case Contrast:
> -			controls.add(V4L2_CID_CONTRAST, value.get<int>());
> +			controls.add(V4L2_CID_CONTRAST, value.get<int32_t>());
>  			break;
>
>  		case Saturation:
> -			controls.add(V4L2_CID_SATURATION, value.get<int>());
> +			controls.add(V4L2_CID_SATURATION, value.get<int32_t>());
>  			break;
>
>  		default:
> diff --git a/test/controls/control_info.cpp b/test/controls/control_info.cpp
> index faefaaa444d9..2aba568a302e 100644
> --- a/test/controls/control_info.cpp
> +++ b/test/controls/control_info.cpp
> @@ -26,13 +26,14 @@ protected:
>  		ControlInfo info(Brightness);
>
>  		if (info.id() != Brightness ||
> -		    info.type() != ControlTypeInteger ||
> +		    info.type() != ControlTypeInteger32 ||
>  		    info.name() != std::string("Brightness")) {
>  			cout << "Invalid control identification for Brightness" << endl;
>  			return TestFail;
>  		}
>
> -		if (info.min().get<int>() != 0 || info.max().get<int>() != 0) {
> +		if (info.min().get<int32_t>() != 0 ||
> +		    info.max().get<int32_t>() != 0) {
>  			cout << "Invalid control range for Brightness" << endl;
>  			return TestFail;
>  		}
> @@ -44,13 +45,14 @@ protected:
>  		info = ControlInfo(Contrast, 10, 200);
>
>  		if (info.id() != Contrast ||
> -		    info.type() != ControlTypeInteger ||
> +		    info.type() != ControlTypeInteger32 ||
>  		    info.name() != std::string("Contrast")) {
>  			cout << "Invalid control identification for Contrast" << endl;
>  			return TestFail;
>  		}
>
> -		if (info.min().get<int>() != 10 || info.max().get<int>() != 200) {
> +		if (info.min().get<int32_t>() != 10 ||
> +		    info.max().get<int32_t>() != 200) {
>  			cout << "Invalid control range for Contrast" << endl;
>  			return TestFail;
>  		}
> diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp
> index 0402e7c23dba..5215840b1c4e 100644
> --- a/test/controls/control_list.cpp
> +++ b/test/controls/control_list.cpp
> @@ -96,7 +96,7 @@ protected:
>  			return TestFail;
>  		}
>
> -		if (list[Brightness].get<int>() != 255) {
> +		if (list[Brightness].get<int32_t>() != 255) {
>  			cout << "Incorrest Brightness control value" << endl;
>  			return TestFail;
>  		}
> @@ -125,8 +125,8 @@ protected:
>  		/*
>  		 * Test control value retrieval and update through ControlInfo.
>  		 */
> -		if (list[brightness].get<int>() != 64 ||
> -		    list[contrast].get<int>() != 128) {
> +		if (list[brightness].get<int32_t>() != 64 ||
> +		    list[contrast].get<int32_t>() != 128) {
>  			cout << "Failed to retrieve control value" << endl;
>  			return TestFail;
>  		}
> @@ -134,8 +134,8 @@ protected:
>  		list[brightness] = 10;
>  		list[contrast] = 20;
>
> -		if (list[brightness].get<int>() != 10 ||
> -		    list[contrast].get<int>() != 20) {
> +		if (list[brightness].get<int32_t>() != 10 ||
> +		    list[contrast].get<int32_t>() != 20) {
>  			cout << "Failed to update control value" << endl;
>  			return TestFail;
>  		}
> @@ -185,9 +185,9 @@ protected:
>  			return TestFail;
>  		}
>
> -		if (newList[Brightness].get<int>() != 10 ||
> -		    newList[Contrast].get<int>() != 20 ||
> -		    newList[Saturation].get<int>() != 255) {
> +		if (newList[Brightness].get<int32_t>() != 10 ||
> +		    newList[Contrast].get<int32_t>() != 20 ||
> +		    newList[Saturation].get<int32_t>() != 255) {
>  			cout << "New list contains incorrect values" << endl;
>  			return TestFail;
>  		}
> diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp
> index 397c43f799ad..a1ffa842f29e 100644
> --- a/test/controls/control_value.cpp
> +++ b/test/controls/control_value.cpp
> @@ -27,7 +27,7 @@ protected:
>  		     << " Bool: " << boolean.toString()
>  		     << endl;
>
> -		if (integer.get<int>() != 1234) {
> +		if (integer.get<int32_t>() != 1234) {
>  			cerr << "Failed to get Integer" << endl;
>  			return TestFail;
>  		}
> @@ -56,8 +56,8 @@ protected:
>  			return TestFail;
>  		}
>
> -		value.set<int>(10);
> -		if (value.get<int>() != 10) {
> +		value.set<int32_t>(10);
> +		if (value.get<int32_t>() != 10) {
>  			cerr << "Failed to get Integer" << endl;
>  			return TestFail;
>  		}
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Sept. 29, 2019, 11:39 a.m. UTC | #2 
Hi Jacopo,

On Sun, Sep 29, 2019 at 11:08:48AM +0200, Jacopo Mondi wrote:
> On Sat, Sep 28, 2019 at 06:22:29PM +0300, Laurent Pinchart wrote:
> > Make the control API more explicit when dealing with integer controls by
> > specifying the size. We already do so for 64-bit integers, using int64_t
> > and ControlTypeInteger64, do the same for 32-bit integers.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  include/libcamera/controls.h        |  6 ++---
> >  src/libcamera/controls.cpp          | 36 ++++++++++++++---------------
> >  src/libcamera/pipeline/uvcvideo.cpp | 10 ++++----
> >  src/libcamera/pipeline/vimc.cpp     |  6 ++---
> >  test/controls/control_info.cpp      | 10 ++++----
> >  test/controls/control_list.cpp      | 16 ++++++-------
> >  test/controls/control_value.cpp     |  6 ++---
> >  7 files changed, 46 insertions(+), 44 deletions(-)
> >
> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> > index 0886370f0901..a3089064c4b5 100644
> > --- a/include/libcamera/controls.h
> > +++ b/include/libcamera/controls.h
> > @@ -21,7 +21,7 @@ class Camera;
> >  enum ControlType {
> >  	ControlTypeNone,
> >  	ControlTypeBool,
> > -	ControlTypeInteger,
> > +	ControlTypeInteger32,
> >  	ControlTypeInteger64,
> >  };
> >
> > @@ -30,7 +30,7 @@ class ControlValue
> >  public:
> >  	ControlValue();
> >  	ControlValue(bool value);
> > -	ControlValue(int value);
> > +	ControlValue(int32_t value);
> >  	ControlValue(int64_t value);
> >
> >  	ControlType type() const { return type_; };
> > @@ -48,7 +48,7 @@ private:
> >
> >  	union {
> >  		bool bool_;
> > -		int integer_;
> > +		int32_t integer32_;
> >  		int64_t integer64_;
> >  	};
> >  };
> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> > index 88aab88db327..295ccd55a622 100644
> > --- a/src/libcamera/controls.cpp
> > +++ b/src/libcamera/controls.cpp
> > @@ -31,8 +31,8 @@ LOG_DEFINE_CATEGORY(Controls)
> >   * Invalid type, for empty values
> >   * \var ControlTypeBool
> >   * The control stores a boolean value
> > - * \var ControlTypeInteger
> > - * The control stores an integer value
> > + * \var ControlTypeInteger32
> > + * The control stores a 32-bit integer value
> >   * \var ControlTypeInteger64
> >   * The control stores a 64-bit integer value
> >   */
> > @@ -63,8 +63,8 @@ ControlValue::ControlValue(bool value)
> >   * \brief Construct an integer ControlValue
> >   * \param[in] value Integer value to store
> >   */
> > -ControlValue::ControlValue(int value)
> > -	: type_(ControlTypeInteger), integer_(value)
> > +ControlValue::ControlValue(int32_t value)
> > +	: type_(ControlTypeInteger32), integer32_(value)
> >  {
> >  }
> >
> > @@ -115,17 +115,17 @@ const bool &ControlValue::get<bool>() const
> >  }
> >
> >  template<>
> > -const int &ControlValue::get<int>() const
> > +const int32_t &ControlValue::get<int32_t>() const
> >  {
> > -	ASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64);
> > +	ASSERT(type_ == ControlTypeInteger32 || type_ == ControlTypeInteger64);
> >
> > -	return integer_;
> > +	return integer32_;
> >  }
> >
> >  template<>
> >  const int64_t &ControlValue::get<int64_t>() const
> >  {
> > -	ASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64);
> > +	ASSERT(type_ == ControlTypeInteger32 || type_ == ControlTypeInteger64);
> >
> >  	return integer64_;
> >  }
> > @@ -138,10 +138,10 @@ void ControlValue::set<bool>(const bool &value)
> >  }
> >
> >  template<>
> > -void ControlValue::set<int>(const int &value)
> > +void ControlValue::set<int32_t>(const int32_t &value)
> >  {
> > -	type_ = ControlTypeInteger;
> > -	integer_ = value;
> > +	type_ = ControlTypeInteger32;
> > +	integer32_ = value;
> >  }
> >
> >  template<>
> > @@ -163,8 +163,8 @@ std::string ControlValue::toString() const
> >  		return "<None>";
> >  	case ControlTypeBool:
> >  		return bool_ ? "True" : "False";
> > -	case ControlTypeInteger:
> > -		return std::to_string(integer_);
> > +	case ControlTypeInteger32:
> > +		return std::to_string(integer32_);
> >  	case ControlTypeInteger64:
> >  		return std::to_string(integer64_);
> >  	}
> > @@ -186,35 +186,35 @@ std::string ControlValue::toString() const
> >
> >  /**
> >   * \var Brightness
> > - * ControlType: Integer
> > + * ControlType: Integer32
> >   *
> >   * Specify a fixed brightness parameter.
> >   */
> >
> >  /**
> >   * \var Contrast
> > - * ControlType: Integer
> > + * ControlType: Integer32
> >   *
> >   * Specify a fixed contrast parameter.
> >   */
> >
> >  /**
> >   * \var Saturation
> > - * ControlType: Integer
> > + * ControlType: Integer32
> >   *
> >   * Specify a fixed saturation parameter.
> >   */
> >
> >  /**
> >   * \var ManualExposure
> > - * ControlType: Integer
> > + * ControlType: Integer32
> >   *
> >   * Specify a fixed exposure time in milli-seconds
> >   */
> >
> >  /**
> >   * \var ManualGain
> > - * ControlType: Integer
> > + * ControlType: Integer32
> >   *
> >   * Specify a fixed gain parameter
> >   */
> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> > index 81c548af2c64..0d56758e3e1d 100644
> > --- a/src/libcamera/pipeline/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo.cpp
> > @@ -235,24 +235,24 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)
> >
> >  		switch (ci->id()) {
> >  		case Brightness:
> > -			controls.add(V4L2_CID_BRIGHTNESS, value.get<int>());
> > +			controls.add(V4L2_CID_BRIGHTNESS, value.get<int32_t>());
> 
> Due to C++ internal type casting rules (I presume) the previous syntax
> value.get<int>() is still legal. I assume the template argument
> substitution process matches this with the <int32_t> specialization.
> Shouldn't we keep using the shorter one ?

It's still legal, and will still compile, the same way that get<long>
will match get<int64_t> on 64-bit platforms. However, get<long long>
won't, as long long and long are not the same type, even if they end up
having the same size and behaving exactly the same. On 32-bit platforms,
it's the other way around, get<long long> will match get<int64_t>, but
get<long> won't match get<int32_t>.

The reason that pushed me to go for int32_t is to push authors to
consider the data type when writing code, instead of blindly relying on
"an integer being good enough". Down the line I think it would be useful
to replace V4L2_CID_* with Control instances to get the same type safety
for V4L2 controls that we have for libcamera controls, but that will
require manually defining Control instances for all the V4L2 controls,
which isn't very nice. I still need to think about it.

> Anyway
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> >  			break;
> >
> >  		case Contrast:
> > -			controls.add(V4L2_CID_CONTRAST, value.get<int>());
> > +			controls.add(V4L2_CID_CONTRAST, value.get<int32_t>());
> >  			break;
> >
> >  		case Saturation:
> > -			controls.add(V4L2_CID_SATURATION, value.get<int>());
> > +			controls.add(V4L2_CID_SATURATION, value.get<int32_t>());
> >  			break;
> >
> >  		case ManualExposure:
> >  			controls.add(V4L2_CID_EXPOSURE_AUTO, 1);
> > -			controls.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get<int>());
> > +			controls.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get<int32_t>());
> >  			break;
> >
> >  		case ManualGain:
> > -			controls.add(V4L2_CID_GAIN, value.get<int>());
> > +			controls.add(V4L2_CID_GAIN, value.get<int32_t>());
> >  			break;
> >
> >  		default:
> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> > index 3e34e7a0edbf..e549dc64b996 100644
> > --- a/src/libcamera/pipeline/vimc.cpp
> > +++ b/src/libcamera/pipeline/vimc.cpp
> > @@ -288,15 +288,15 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
> >
> >  		switch (ci->id()) {
> >  		case Brightness:
> > -			controls.add(V4L2_CID_BRIGHTNESS, value.get<int>());
> > +			controls.add(V4L2_CID_BRIGHTNESS, value.get<int32_t>());
> >  			break;
> >
> >  		case Contrast:
> > -			controls.add(V4L2_CID_CONTRAST, value.get<int>());
> > +			controls.add(V4L2_CID_CONTRAST, value.get<int32_t>());
> >  			break;
> >
> >  		case Saturation:
> > -			controls.add(V4L2_CID_SATURATION, value.get<int>());
> > +			controls.add(V4L2_CID_SATURATION, value.get<int32_t>());
> >  			break;
> >
> >  		default:
> > diff --git a/test/controls/control_info.cpp b/test/controls/control_info.cpp
> > index faefaaa444d9..2aba568a302e 100644
> > --- a/test/controls/control_info.cpp
> > +++ b/test/controls/control_info.cpp
> > @@ -26,13 +26,14 @@ protected:
> >  		ControlInfo info(Brightness);
> >
> >  		if (info.id() != Brightness ||
> > -		    info.type() != ControlTypeInteger ||
> > +		    info.type() != ControlTypeInteger32 ||
> >  		    info.name() != std::string("Brightness")) {
> >  			cout << "Invalid control identification for Brightness" << endl;
> >  			return TestFail;
> >  		}
> >
> > -		if (info.min().get<int>() != 0 || info.max().get<int>() != 0) {
> > +		if (info.min().get<int32_t>() != 0 ||
> > +		    info.max().get<int32_t>() != 0) {
> >  			cout << "Invalid control range for Brightness" << endl;
> >  			return TestFail;
> >  		}
> > @@ -44,13 +45,14 @@ protected:
> >  		info = ControlInfo(Contrast, 10, 200);
> >
> >  		if (info.id() != Contrast ||
> > -		    info.type() != ControlTypeInteger ||
> > +		    info.type() != ControlTypeInteger32 ||
> >  		    info.name() != std::string("Contrast")) {
> >  			cout << "Invalid control identification for Contrast" << endl;
> >  			return TestFail;
> >  		}
> >
> > -		if (info.min().get<int>() != 10 || info.max().get<int>() != 200) {
> > +		if (info.min().get<int32_t>() != 10 ||
> > +		    info.max().get<int32_t>() != 200) {
> >  			cout << "Invalid control range for Contrast" << endl;
> >  			return TestFail;
> >  		}
> > diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp
> > index 0402e7c23dba..5215840b1c4e 100644
> > --- a/test/controls/control_list.cpp
> > +++ b/test/controls/control_list.cpp
> > @@ -96,7 +96,7 @@ protected:
> >  			return TestFail;
> >  		}
> >
> > -		if (list[Brightness].get<int>() != 255) {
> > +		if (list[Brightness].get<int32_t>() != 255) {
> >  			cout << "Incorrest Brightness control value" << endl;
> >  			return TestFail;
> >  		}
> > @@ -125,8 +125,8 @@ protected:
> >  		/*
> >  		 * Test control value retrieval and update through ControlInfo.
> >  		 */
> > -		if (list[brightness].get<int>() != 64 ||
> > -		    list[contrast].get<int>() != 128) {
> > +		if (list[brightness].get<int32_t>() != 64 ||
> > +		    list[contrast].get<int32_t>() != 128) {
> >  			cout << "Failed to retrieve control value" << endl;
> >  			return TestFail;
> >  		}
> > @@ -134,8 +134,8 @@ protected:
> >  		list[brightness] = 10;
> >  		list[contrast] = 20;
> >
> > -		if (list[brightness].get<int>() != 10 ||
> > -		    list[contrast].get<int>() != 20) {
> > +		if (list[brightness].get<int32_t>() != 10 ||
> > +		    list[contrast].get<int32_t>() != 20) {
> >  			cout << "Failed to update control value" << endl;
> >  			return TestFail;
> >  		}
> > @@ -185,9 +185,9 @@ protected:
> >  			return TestFail;
> >  		}
> >
> > -		if (newList[Brightness].get<int>() != 10 ||
> > -		    newList[Contrast].get<int>() != 20 ||
> > -		    newList[Saturation].get<int>() != 255) {
> > +		if (newList[Brightness].get<int32_t>() != 10 ||
> > +		    newList[Contrast].get<int32_t>() != 20 ||
> > +		    newList[Saturation].get<int32_t>() != 255) {
> >  			cout << "New list contains incorrect values" << endl;
> >  			return TestFail;
> >  		}
> > diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp
> > index 397c43f799ad..a1ffa842f29e 100644
> > --- a/test/controls/control_value.cpp
> > +++ b/test/controls/control_value.cpp
> > @@ -27,7 +27,7 @@ protected:
> >  		     << " Bool: " << boolean.toString()
> >  		     << endl;
> >
> > -		if (integer.get<int>() != 1234) {
> > +		if (integer.get<int32_t>() != 1234) {
> >  			cerr << "Failed to get Integer" << endl;
> >  			return TestFail;
> >  		}
> > @@ -56,8 +56,8 @@ protected:
> >  			return TestFail;
> >  		}
> >
> > -		value.set<int>(10);
> > -		if (value.get<int>() != 10) {
> > +		value.set<int32_t>(10);
> > +		if (value.get<int32_t>() != 10) {
> >  			cerr << "Failed to get Integer" << endl;
> >  			return TestFail;
> >  		}

Patch
diff mbox series 

diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
index 0886370f0901..a3089064c4b5 100644
--- a/include/libcamera/controls.h
+++ b/include/libcamera/controls.h
@@ -21,7 +21,7 @@  class Camera;
 enum ControlType {
 	ControlTypeNone,
 	ControlTypeBool,
-	ControlTypeInteger,
+	ControlTypeInteger32,
 	ControlTypeInteger64,
 };
 
@@ -30,7 +30,7 @@  class ControlValue
 public:
 	ControlValue();
 	ControlValue(bool value);
-	ControlValue(int value);
+	ControlValue(int32_t value);
 	ControlValue(int64_t value);
 
 	ControlType type() const { return type_; };
@@ -48,7 +48,7 @@  private:
 
 	union {
 		bool bool_;
-		int integer_;
+		int32_t integer32_;
 		int64_t integer64_;
 	};
 };
diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
index 88aab88db327..295ccd55a622 100644
--- a/src/libcamera/controls.cpp
+++ b/src/libcamera/controls.cpp
@@ -31,8 +31,8 @@  LOG_DEFINE_CATEGORY(Controls)
  * Invalid type, for empty values
  * \var ControlTypeBool
  * The control stores a boolean value
- * \var ControlTypeInteger
- * The control stores an integer value
+ * \var ControlTypeInteger32
+ * The control stores a 32-bit integer value
  * \var ControlTypeInteger64
  * The control stores a 64-bit integer value
  */
@@ -63,8 +63,8 @@  ControlValue::ControlValue(bool value)
  * \brief Construct an integer ControlValue
  * \param[in] value Integer value to store
  */
-ControlValue::ControlValue(int value)
-	: type_(ControlTypeInteger), integer_(value)
+ControlValue::ControlValue(int32_t value)
+	: type_(ControlTypeInteger32), integer32_(value)
 {
 }
 
@@ -115,17 +115,17 @@  const bool &ControlValue::get<bool>() const
 }
 
 template<>
-const int &ControlValue::get<int>() const
+const int32_t &ControlValue::get<int32_t>() const
 {
-	ASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64);
+	ASSERT(type_ == ControlTypeInteger32 || type_ == ControlTypeInteger64);
 
-	return integer_;
+	return integer32_;
 }
 
 template<>
 const int64_t &ControlValue::get<int64_t>() const
 {
-	ASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64);
+	ASSERT(type_ == ControlTypeInteger32 || type_ == ControlTypeInteger64);
 
 	return integer64_;
 }
@@ -138,10 +138,10 @@  void ControlValue::set<bool>(const bool &value)
 }
 
 template<>
-void ControlValue::set<int>(const int &value)
+void ControlValue::set<int32_t>(const int32_t &value)
 {
-	type_ = ControlTypeInteger;
-	integer_ = value;
+	type_ = ControlTypeInteger32;
+	integer32_ = value;
 }
 
 template<>
@@ -163,8 +163,8 @@  std::string ControlValue::toString() const
 		return "<None>";
 	case ControlTypeBool:
 		return bool_ ? "True" : "False";
-	case ControlTypeInteger:
-		return std::to_string(integer_);
+	case ControlTypeInteger32:
+		return std::to_string(integer32_);
 	case ControlTypeInteger64:
 		return std::to_string(integer64_);
 	}
@@ -186,35 +186,35 @@  std::string ControlValue::toString() const
 
 /**
  * \var Brightness
- * ControlType: Integer
+ * ControlType: Integer32
  *
  * Specify a fixed brightness parameter.
  */
 
 /**
  * \var Contrast
- * ControlType: Integer
+ * ControlType: Integer32
  *
  * Specify a fixed contrast parameter.
  */
 
 /**
  * \var Saturation
- * ControlType: Integer
+ * ControlType: Integer32
  *
  * Specify a fixed saturation parameter.
  */
 
 /**
  * \var ManualExposure
- * ControlType: Integer
+ * ControlType: Integer32
  *
  * Specify a fixed exposure time in milli-seconds
  */
 
 /**
  * \var ManualGain
- * ControlType: Integer
+ * ControlType: Integer32
  *
  * Specify a fixed gain parameter
  */
diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
index 81c548af2c64..0d56758e3e1d 100644
--- a/src/libcamera/pipeline/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo.cpp
@@ -235,24 +235,24 @@  int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)
 
 		switch (ci->id()) {
 		case Brightness:
-			controls.add(V4L2_CID_BRIGHTNESS, value.get<int>());
+			controls.add(V4L2_CID_BRIGHTNESS, value.get<int32_t>());
 			break;
 
 		case Contrast:
-			controls.add(V4L2_CID_CONTRAST, value.get<int>());
+			controls.add(V4L2_CID_CONTRAST, value.get<int32_t>());
 			break;
 
 		case Saturation:
-			controls.add(V4L2_CID_SATURATION, value.get<int>());
+			controls.add(V4L2_CID_SATURATION, value.get<int32_t>());
 			break;
 
 		case ManualExposure:
 			controls.add(V4L2_CID_EXPOSURE_AUTO, 1);
-			controls.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get<int>());
+			controls.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get<int32_t>());
 			break;
 
 		case ManualGain:
-			controls.add(V4L2_CID_GAIN, value.get<int>());
+			controls.add(V4L2_CID_GAIN, value.get<int32_t>());
 			break;
 
 		default:
diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index 3e34e7a0edbf..e549dc64b996 100644
--- a/src/libcamera/pipeline/vimc.cpp
+++ b/src/libcamera/pipeline/vimc.cpp
@@ -288,15 +288,15 @@  int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
 
 		switch (ci->id()) {
 		case Brightness:
-			controls.add(V4L2_CID_BRIGHTNESS, value.get<int>());
+			controls.add(V4L2_CID_BRIGHTNESS, value.get<int32_t>());
 			break;
 
 		case Contrast:
-			controls.add(V4L2_CID_CONTRAST, value.get<int>());
+			controls.add(V4L2_CID_CONTRAST, value.get<int32_t>());
 			break;
 
 		case Saturation:
-			controls.add(V4L2_CID_SATURATION, value.get<int>());
+			controls.add(V4L2_CID_SATURATION, value.get<int32_t>());
 			break;
 
 		default:
diff --git a/test/controls/control_info.cpp b/test/controls/control_info.cpp
index faefaaa444d9..2aba568a302e 100644
--- a/test/controls/control_info.cpp
+++ b/test/controls/control_info.cpp
@@ -26,13 +26,14 @@  protected:
 		ControlInfo info(Brightness);
 
 		if (info.id() != Brightness ||
-		    info.type() != ControlTypeInteger ||
+		    info.type() != ControlTypeInteger32 ||
 		    info.name() != std::string("Brightness")) {
 			cout << "Invalid control identification for Brightness" << endl;
 			return TestFail;
 		}
 
-		if (info.min().get<int>() != 0 || info.max().get<int>() != 0) {
+		if (info.min().get<int32_t>() != 0 ||
+		    info.max().get<int32_t>() != 0) {
 			cout << "Invalid control range for Brightness" << endl;
 			return TestFail;
 		}
@@ -44,13 +45,14 @@  protected:
 		info = ControlInfo(Contrast, 10, 200);
 
 		if (info.id() != Contrast ||
-		    info.type() != ControlTypeInteger ||
+		    info.type() != ControlTypeInteger32 ||
 		    info.name() != std::string("Contrast")) {
 			cout << "Invalid control identification for Contrast" << endl;
 			return TestFail;
 		}
 
-		if (info.min().get<int>() != 10 || info.max().get<int>() != 200) {
+		if (info.min().get<int32_t>() != 10 ||
+		    info.max().get<int32_t>() != 200) {
 			cout << "Invalid control range for Contrast" << endl;
 			return TestFail;
 		}
diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp
index 0402e7c23dba..5215840b1c4e 100644
--- a/test/controls/control_list.cpp
+++ b/test/controls/control_list.cpp
@@ -96,7 +96,7 @@  protected:
 			return TestFail;
 		}
 
-		if (list[Brightness].get<int>() != 255) {
+		if (list[Brightness].get<int32_t>() != 255) {
 			cout << "Incorrest Brightness control value" << endl;
 			return TestFail;
 		}
@@ -125,8 +125,8 @@  protected:
 		/*
 		 * Test control value retrieval and update through ControlInfo.
 		 */
-		if (list[brightness].get<int>() != 64 ||
-		    list[contrast].get<int>() != 128) {
+		if (list[brightness].get<int32_t>() != 64 ||
+		    list[contrast].get<int32_t>() != 128) {
 			cout << "Failed to retrieve control value" << endl;
 			return TestFail;
 		}
@@ -134,8 +134,8 @@  protected:
 		list[brightness] = 10;
 		list[contrast] = 20;
 
-		if (list[brightness].get<int>() != 10 ||
-		    list[contrast].get<int>() != 20) {
+		if (list[brightness].get<int32_t>() != 10 ||
+		    list[contrast].get<int32_t>() != 20) {
 			cout << "Failed to update control value" << endl;
 			return TestFail;
 		}
@@ -185,9 +185,9 @@  protected:
 			return TestFail;
 		}
 
-		if (newList[Brightness].get<int>() != 10 ||
-		    newList[Contrast].get<int>() != 20 ||
-		    newList[Saturation].get<int>() != 255) {
+		if (newList[Brightness].get<int32_t>() != 10 ||
+		    newList[Contrast].get<int32_t>() != 20 ||
+		    newList[Saturation].get<int32_t>() != 255) {
 			cout << "New list contains incorrect values" << endl;
 			return TestFail;
 		}
diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp
index 397c43f799ad..a1ffa842f29e 100644
--- a/test/controls/control_value.cpp
+++ b/test/controls/control_value.cpp
@@ -27,7 +27,7 @@  protected:
 		     << " Bool: " << boolean.toString()
 		     << endl;
 
-		if (integer.get<int>() != 1234) {
+		if (integer.get<int32_t>() != 1234) {
 			cerr << "Failed to get Integer" << endl;
 			return TestFail;
 		}
@@ -56,8 +56,8 @@  protected:
 			return TestFail;
 		}
 
-		value.set<int>(10);
-		if (value.get<int>() != 10) {
+		value.set<int32_t>(10);
+		if (value.get<int32_t>() != 10) {
 			cerr << "Failed to get Integer" << endl;
 			return TestFail;
 		}