[v3,1/2] libcamera: Extend u32 control type
diff mbox series

Message ID 20241025053111.519449-2-chenghaoyang@chromium.org
State New
Headers show
Series
  • Add U16 & U32 support in controls
Related show

Commit Message

Harvey Yang Oct. 25, 2024, 5:30 a.m. UTC
From: Yudhistira Erlandinata <yerlandinata@chromium.org>

V4L2 Controls support a wide variety of types not yet supported by the
ControlValue type system.

Extend the libcamera ControlValue types to support an explicit 32 bit
unsigned integer type, and map that to the corresponding
V4L2_CTRL_TYPE_U32 type within the v4l2_device support class.

Signed-off-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
---
 include/libcamera/controls.h    |  7 ++++++
 src/libcamera/controls.cpp      | 12 ++++++++--
 src/libcamera/v4l2_device.cpp   | 13 +++++++++++
 test/controls/control_value.cpp | 40 +++++++++++++++++++++++++++++++++
 4 files changed, 70 insertions(+), 2 deletions(-)

Comments

Jacopo Mondi Oct. 25, 2024, 9:15 a.m. UTC | #1
Hi Harvey

On Fri, Oct 25, 2024 at 05:30:13AM +0000, Harvey Yang wrote:
> From: Yudhistira Erlandinata <yerlandinata@chromium.org>
>
> V4L2 Controls support a wide variety of types not yet supported by the
> ControlValue type system.
>
> Extend the libcamera ControlValue types to support an explicit 32 bit
> unsigned integer type, and map that to the corresponding
> V4L2_CTRL_TYPE_U32 type within the v4l2_device support class.

V4L2_CTRL_TYPE_U32 is a control compound type.

	/* Compound types are >= 0x0100 */
	V4L2_CTRL_COMPOUND_TYPES     = 0x0100,
	V4L2_CTRL_TYPE_U8	     = 0x0100,
	V4L2_CTRL_TYPE_U16	     = 0x0101,
	V4L2_CTRL_TYPE_U32	     = 0x0102,
	V4L2_CTRL_TYPE_AREA          = 0x0106,

This means it carries a payload and needs to be handled like
V4L2_CTRL_TYPE_U8 is, specifcially when it comes to parsing it from
the V4L2 interface into a libcamera control.

I'm surprised you don't hit this in V4L2Device::getControls()

		if (info.flags & V4L2_CTRL_FLAG_HAS_PAYLOAD) {
			ControlType type;

			switch (info.type) {
			case V4L2_CTRL_TYPE_U8:
				type = ControlTypeByte;
				break;

			default:
				LOG(V4L2, Error)
					<< "Unsupported payload control type "
					<< info.type;
				return {};
			}

>
> Signed-off-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
> Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> ---
>  include/libcamera/controls.h    |  7 ++++++
>  src/libcamera/controls.cpp      | 12 ++++++++--
>  src/libcamera/v4l2_device.cpp   | 13 +++++++++++
>  test/controls/control_value.cpp | 40 +++++++++++++++++++++++++++++++++
>  4 files changed, 70 insertions(+), 2 deletions(-)
>
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index ca60bbaca..6da8ad2c3 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -29,6 +29,7 @@ enum ControlType {
>  	ControlTypeNone,
>  	ControlTypeBool,
>  	ControlTypeByte,
> +	ControlTypeUnsigned32,
>  	ControlTypeInteger32,
>  	ControlTypeInteger64,
>  	ControlTypeFloat,
> @@ -62,6 +63,12 @@ struct control_type<uint8_t> {
>  	static constexpr std::size_t size = 0;
>  };
>
> +template<>
> +struct control_type<uint32_t> {
> +	static constexpr ControlType value = ControlTypeUnsigned32;
> +	static constexpr std::size_t size = 0;
> +};
> +
>  template<>
>  struct control_type<int32_t> {
>  	static constexpr ControlType value = ControlTypeInteger32;
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index 62185d643..8ae295191 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -54,6 +54,7 @@ static constexpr size_t ControlValueSize[] = {
>  	[ControlTypeNone]		= 0,
>  	[ControlTypeBool]		= sizeof(bool),
>  	[ControlTypeByte]		= sizeof(uint8_t),
> +	[ControlTypeUnsigned32]		= sizeof(uint32_t),
>  	[ControlTypeInteger32]		= sizeof(int32_t),
>  	[ControlTypeInteger64]		= sizeof(int64_t),
>  	[ControlTypeFloat]		= sizeof(float),
> @@ -74,10 +75,12 @@ static constexpr size_t ControlValueSize[] = {
>   * The control stores a boolean value
>   * \var ControlTypeByte
>   * The control stores a byte value as an unsigned 8-bit integer
> + * \var ControlTypeUnsigned32
> + * The control stores an unsigned 32-bit integer value
>   * \var ControlTypeInteger32
> - * The control stores a 32-bit integer value
> + * The control stores a signed 32-bit integer value
>   * \var ControlTypeInteger64
> - * The control stores a 64-bit integer value
> + * The control stores a signed 64-bit integer value
>   * \var ControlTypeFloat
>   * The control stores a 32-bit floating point value
>   * \var ControlTypeString
> @@ -230,6 +233,11 @@ std::string ControlValue::toString() const
>  			str += std::to_string(*value);
>  			break;
>  		}
> +		case ControlTypeUnsigned32: {
> +			const uint32_t *value = reinterpret_cast<const uint32_t *>(data);
> +			str += std::to_string(*value);
> +			break;
> +		}
>  		case ControlTypeInteger32: {
>  			const int32_t *value = reinterpret_cast<const int32_t *>(data);
>  			str += std::to_string(*value);
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 68add4f2e..0ba8dcfa0 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -9,6 +9,7 @@
>
>  #include <fcntl.h>
>  #include <map>
> +#include <stdint.h>
>  #include <stdlib.h>
>  #include <string.h>
>  #include <sys/ioctl.h>
> @@ -17,11 +18,14 @@
>  #include <vector>
>
>  #include <linux/v4l2-mediabus.h>
> +#include <linux/videodev2.h>

Included by the header file

>
>  #include <libcamera/base/event_notifier.h>
>  #include <libcamera/base/log.h>
>  #include <libcamera/base/utils.h>
>
> +#include <libcamera/controls.h>
> +

This as well

>  #include "libcamera/internal/formats.h"
>  #include "libcamera/internal/sysfs.h"
>
> @@ -488,6 +492,9 @@ ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType)
>  	case V4L2_CTRL_TYPE_BOOLEAN:
>  		return ControlTypeBool;
>
> +	case V4L2_CTRL_TYPE_U32:
> +		return ControlTypeUnsigned32;
> +
>  	case V4L2_CTRL_TYPE_INTEGER:
>  		return ControlTypeInteger32;
>
> @@ -536,6 +543,11 @@ std::optional<ControlInfo> V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl
>  				   static_cast<uint8_t>(ctrl.maximum),
>  				   static_cast<uint8_t>(ctrl.default_value));
>
> +	case V4L2_CTRL_TYPE_U32:
> +		return ControlInfo(static_cast<uint32_t>(ctrl.minimum),
> +				   static_cast<uint32_t>(ctrl.maximum),
> +				   static_cast<uint32_t>(ctrl.default_value));
> +
>  	case V4L2_CTRL_TYPE_BOOLEAN:
>  		return ControlInfo(static_cast<bool>(ctrl.minimum),
>  				   static_cast<bool>(ctrl.maximum),
> @@ -622,6 +634,7 @@ void V4L2Device::listControls()
>  		case V4L2_CTRL_TYPE_BITMASK:
>  		case V4L2_CTRL_TYPE_INTEGER_MENU:
>  		case V4L2_CTRL_TYPE_U8:
> +		case V4L2_CTRL_TYPE_U32:
>  			break;
>  		/* \todo Support other control types. */
>  		default:
> diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp
> index 344107fae..6ca85b739 100644
> --- a/test/controls/control_value.cpp
> +++ b/test/controls/control_value.cpp
> @@ -109,6 +109,46 @@ protected:
>  			return TestFail;
>  		}
>
> +		/*
> +		 * Unsigned Integer32 type.
> +		 */
> +		value.set(static_cast<uint32_t>(42));
> +		if (value.isNone() || value.isArray() ||
> +		    value.type() != ControlTypeUnsigned32) {
> +			cerr << "Control type mismatch after setting to uint32_t" << endl;
> +			return TestFail;
> +		}
> +
> +		if (value.get<uint32_t>() != 42) {
> +			cerr << "Control value mismatch after setting to uint32_t" << endl;
> +			return TestFail;
> +		}
> +
> +		if (value.toString() != "42") {
> +			cerr << "Control string mismatch after setting to uint32_t" << endl;
> +			return TestFail;
> +		}
> +
> +		std::array<uint32_t, 4> uint32s{ 3, 14, 15, 9 };
> +		value.set(Span<uint32_t>(uint32s));
> +		if (value.isNone() || !value.isArray() ||
> +		    value.type() != ControlTypeUnsigned32) {
> +			cerr << "Control type mismatch after setting to uint32_t array" << endl;
> +			return TestFail;
> +		}
> +
> +		Span<const uint32_t> uint32sResult = value.get<Span<const uint32_t>>();
> +		if (uint32s.size() != uint32sResult.size() ||
> +		    !std::equal(uint32s.begin(), uint32s.end(), uint32sResult.begin())) {
> +			cerr << "Control value mismatch after setting to uint32_t array" << endl;
> +			return TestFail;
> +		}
> +
> +		if (value.toString() != "[ 3, 14, 15, 9 ]") {
> +			cerr << "Control string mismatch after setting to uint32_t array" << endl;
> +			return TestFail;
> +		}
> +
>  		/*
>  		 * Integer32 type.
>  		 */
> --
> 2.47.0.163.g1226f6d8fa-goog
>
Harvey Yang Oct. 25, 2024, 3:40 p.m. UTC | #2
Hi Jacopo,

On Fri, Oct 25, 2024 at 5:15 PM Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> Hi Harvey
>
> On Fri, Oct 25, 2024 at 05:30:13AM +0000, Harvey Yang wrote:
> > From: Yudhistira Erlandinata <yerlandinata@chromium.org>
> >
> > V4L2 Controls support a wide variety of types not yet supported by the
> > ControlValue type system.
> >
> > Extend the libcamera ControlValue types to support an explicit 32 bit
> > unsigned integer type, and map that to the corresponding
> > V4L2_CTRL_TYPE_U32 type within the v4l2_device support class.
>
> V4L2_CTRL_TYPE_U32 is a control compound type.
>
>         /* Compound types are >= 0x0100 */
>         V4L2_CTRL_COMPOUND_TYPES     = 0x0100,
>         V4L2_CTRL_TYPE_U8            = 0x0100,
>         V4L2_CTRL_TYPE_U16           = 0x0101,
>         V4L2_CTRL_TYPE_U32           = 0x0102,
>         V4L2_CTRL_TYPE_AREA          = 0x0106,
>
> This means it carries a payload and needs to be handled like
> V4L2_CTRL_TYPE_U8 is, specifcially when it comes to parsing it from
> the V4L2 interface into a libcamera control.
>
> I'm surprised you don't hit this in V4L2Device::getControls()
>
>                 if (info.flags & V4L2_CTRL_FLAG_HAS_PAYLOAD) {
>                         ControlType type;
>
>                         switch (info.type) {
>                         case V4L2_CTRL_TYPE_U8:
>                                 type = ControlTypeByte;
>                                 break;
>
>                         default:
>                                 LOG(V4L2, Error)
>                                         << "Unsupported payload control type "
>                                         << info.type;
>                                 return {};
>                         }
>

Thanks! Added for U32 and U16.

> >
> > Signed-off-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
> > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> > ---
> >  include/libcamera/controls.h    |  7 ++++++
> >  src/libcamera/controls.cpp      | 12 ++++++++--
> >  src/libcamera/v4l2_device.cpp   | 13 +++++++++++
> >  test/controls/control_value.cpp | 40 +++++++++++++++++++++++++++++++++
> >  4 files changed, 70 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> > index ca60bbaca..6da8ad2c3 100644
> > --- a/include/libcamera/controls.h
> > +++ b/include/libcamera/controls.h
> > @@ -29,6 +29,7 @@ enum ControlType {
> >       ControlTypeNone,
> >       ControlTypeBool,
> >       ControlTypeByte,
> > +     ControlTypeUnsigned32,
> >       ControlTypeInteger32,
> >       ControlTypeInteger64,
> >       ControlTypeFloat,
> > @@ -62,6 +63,12 @@ struct control_type<uint8_t> {
> >       static constexpr std::size_t size = 0;
> >  };
> >
> > +template<>
> > +struct control_type<uint32_t> {
> > +     static constexpr ControlType value = ControlTypeUnsigned32;
> > +     static constexpr std::size_t size = 0;
> > +};
> > +
> >  template<>
> >  struct control_type<int32_t> {
> >       static constexpr ControlType value = ControlTypeInteger32;
> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> > index 62185d643..8ae295191 100644
> > --- a/src/libcamera/controls.cpp
> > +++ b/src/libcamera/controls.cpp
> > @@ -54,6 +54,7 @@ static constexpr size_t ControlValueSize[] = {
> >       [ControlTypeNone]               = 0,
> >       [ControlTypeBool]               = sizeof(bool),
> >       [ControlTypeByte]               = sizeof(uint8_t),
> > +     [ControlTypeUnsigned32]         = sizeof(uint32_t),
> >       [ControlTypeInteger32]          = sizeof(int32_t),
> >       [ControlTypeInteger64]          = sizeof(int64_t),
> >       [ControlTypeFloat]              = sizeof(float),
> > @@ -74,10 +75,12 @@ static constexpr size_t ControlValueSize[] = {
> >   * The control stores a boolean value
> >   * \var ControlTypeByte
> >   * The control stores a byte value as an unsigned 8-bit integer
> > + * \var ControlTypeUnsigned32
> > + * The control stores an unsigned 32-bit integer value
> >   * \var ControlTypeInteger32
> > - * The control stores a 32-bit integer value
> > + * The control stores a signed 32-bit integer value
> >   * \var ControlTypeInteger64
> > - * The control stores a 64-bit integer value
> > + * The control stores a signed 64-bit integer value
> >   * \var ControlTypeFloat
> >   * The control stores a 32-bit floating point value
> >   * \var ControlTypeString
> > @@ -230,6 +233,11 @@ std::string ControlValue::toString() const
> >                       str += std::to_string(*value);
> >                       break;
> >               }
> > +             case ControlTypeUnsigned32: {
> > +                     const uint32_t *value = reinterpret_cast<const uint32_t *>(data);
> > +                     str += std::to_string(*value);
> > +                     break;
> > +             }
> >               case ControlTypeInteger32: {
> >                       const int32_t *value = reinterpret_cast<const int32_t *>(data);
> >                       str += std::to_string(*value);
> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> > index 68add4f2e..0ba8dcfa0 100644
> > --- a/src/libcamera/v4l2_device.cpp
> > +++ b/src/libcamera/v4l2_device.cpp
> > @@ -9,6 +9,7 @@
> >
> >  #include <fcntl.h>
> >  #include <map>
> > +#include <stdint.h>
> >  #include <stdlib.h>
> >  #include <string.h>
> >  #include <sys/ioctl.h>
> > @@ -17,11 +18,14 @@
> >  #include <vector>
> >
> >  #include <linux/v4l2-mediabus.h>
> > +#include <linux/videodev2.h>
>
> Included by the header file

Removed.

>
> >
> >  #include <libcamera/base/event_notifier.h>
> >  #include <libcamera/base/log.h>
> >  #include <libcamera/base/utils.h>
> >
> > +#include <libcamera/controls.h>
> > +
>
> This as well

Removed.

BR,
Harvey

>
> >  #include "libcamera/internal/formats.h"
> >  #include "libcamera/internal/sysfs.h"
> >
> > @@ -488,6 +492,9 @@ ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType)
> >       case V4L2_CTRL_TYPE_BOOLEAN:
> >               return ControlTypeBool;
> >
> > +     case V4L2_CTRL_TYPE_U32:
> > +             return ControlTypeUnsigned32;
> > +
> >       case V4L2_CTRL_TYPE_INTEGER:
> >               return ControlTypeInteger32;
> >
> > @@ -536,6 +543,11 @@ std::optional<ControlInfo> V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl
> >                                  static_cast<uint8_t>(ctrl.maximum),
> >                                  static_cast<uint8_t>(ctrl.default_value));
> >
> > +     case V4L2_CTRL_TYPE_U32:
> > +             return ControlInfo(static_cast<uint32_t>(ctrl.minimum),
> > +                                static_cast<uint32_t>(ctrl.maximum),
> > +                                static_cast<uint32_t>(ctrl.default_value));
> > +
> >       case V4L2_CTRL_TYPE_BOOLEAN:
> >               return ControlInfo(static_cast<bool>(ctrl.minimum),
> >                                  static_cast<bool>(ctrl.maximum),
> > @@ -622,6 +634,7 @@ void V4L2Device::listControls()
> >               case V4L2_CTRL_TYPE_BITMASK:
> >               case V4L2_CTRL_TYPE_INTEGER_MENU:
> >               case V4L2_CTRL_TYPE_U8:
> > +             case V4L2_CTRL_TYPE_U32:
> >                       break;
> >               /* \todo Support other control types. */
> >               default:
> > diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp
> > index 344107fae..6ca85b739 100644
> > --- a/test/controls/control_value.cpp
> > +++ b/test/controls/control_value.cpp
> > @@ -109,6 +109,46 @@ protected:
> >                       return TestFail;
> >               }
> >
> > +             /*
> > +              * Unsigned Integer32 type.
> > +              */
> > +             value.set(static_cast<uint32_t>(42));
> > +             if (value.isNone() || value.isArray() ||
> > +                 value.type() != ControlTypeUnsigned32) {
> > +                     cerr << "Control type mismatch after setting to uint32_t" << endl;
> > +                     return TestFail;
> > +             }
> > +
> > +             if (value.get<uint32_t>() != 42) {
> > +                     cerr << "Control value mismatch after setting to uint32_t" << endl;
> > +                     return TestFail;
> > +             }
> > +
> > +             if (value.toString() != "42") {
> > +                     cerr << "Control string mismatch after setting to uint32_t" << endl;
> > +                     return TestFail;
> > +             }
> > +
> > +             std::array<uint32_t, 4> uint32s{ 3, 14, 15, 9 };
> > +             value.set(Span<uint32_t>(uint32s));
> > +             if (value.isNone() || !value.isArray() ||
> > +                 value.type() != ControlTypeUnsigned32) {
> > +                     cerr << "Control type mismatch after setting to uint32_t array" << endl;
> > +                     return TestFail;
> > +             }
> > +
> > +             Span<const uint32_t> uint32sResult = value.get<Span<const uint32_t>>();
> > +             if (uint32s.size() != uint32sResult.size() ||
> > +                 !std::equal(uint32s.begin(), uint32s.end(), uint32sResult.begin())) {
> > +                     cerr << "Control value mismatch after setting to uint32_t array" << endl;
> > +                     return TestFail;
> > +             }
> > +
> > +             if (value.toString() != "[ 3, 14, 15, 9 ]") {
> > +                     cerr << "Control string mismatch after setting to uint32_t array" << endl;
> > +                     return TestFail;
> > +             }
> > +
> >               /*
> >                * Integer32 type.
> >                */
> > --
> > 2.47.0.163.g1226f6d8fa-goog
> >
Jacopo Mondi Oct. 25, 2024, 4:46 p.m. UTC | #3
Hi Harvey

On Fri, Oct 25, 2024 at 11:40:02PM +0800, Cheng-Hao Yang wrote:
> Hi Jacopo,
>
> On Fri, Oct 25, 2024 at 5:15 PM Jacopo Mondi
> <jacopo.mondi@ideasonboard.com> wrote:
> >
> > Hi Harvey
> >
> > On Fri, Oct 25, 2024 at 05:30:13AM +0000, Harvey Yang wrote:
> > > From: Yudhistira Erlandinata <yerlandinata@chromium.org>
> > >
> > > V4L2 Controls support a wide variety of types not yet supported by the
> > > ControlValue type system.
> > >
> > > Extend the libcamera ControlValue types to support an explicit 32 bit
> > > unsigned integer type, and map that to the corresponding
> > > V4L2_CTRL_TYPE_U32 type within the v4l2_device support class.
> >
> > V4L2_CTRL_TYPE_U32 is a control compound type.
> >
> >         /* Compound types are >= 0x0100 */
> >         V4L2_CTRL_COMPOUND_TYPES     = 0x0100,
> >         V4L2_CTRL_TYPE_U8            = 0x0100,
> >         V4L2_CTRL_TYPE_U16           = 0x0101,
> >         V4L2_CTRL_TYPE_U32           = 0x0102,
> >         V4L2_CTRL_TYPE_AREA          = 0x0106,
> >
> > This means it carries a payload and needs to be handled like
> > V4L2_CTRL_TYPE_U8 is, specifcially when it comes to parsing it from
> > the V4L2 interface into a libcamera control.
> >
> > I'm surprised you don't hit this in V4L2Device::getControls()
> >
> >                 if (info.flags & V4L2_CTRL_FLAG_HAS_PAYLOAD) {
> >                         ControlType type;
> >
> >                         switch (info.type) {
> >                         case V4L2_CTRL_TYPE_U8:
> >                                 type = ControlTypeByte;
> >                                 break;
> >
> >                         default:
> >                                 LOG(V4L2, Error)
> >                                         << "Unsupported payload control type "
> >                                         << info.type;
> >                                 return {};
> >                         }
> >
>
> Thanks! Added for U32 and U16.

Thank you.

Any idea why

                 LOG(V4L2, Error)
                         << "Unsupported payload control type "
                         << info.type;
                 return {};

wasn't hit ?

>
> > >
> > > Signed-off-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
> > > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> > > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> > > ---
> > >  include/libcamera/controls.h    |  7 ++++++
> > >  src/libcamera/controls.cpp      | 12 ++++++++--
> > >  src/libcamera/v4l2_device.cpp   | 13 +++++++++++
> > >  test/controls/control_value.cpp | 40 +++++++++++++++++++++++++++++++++
> > >  4 files changed, 70 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> > > index ca60bbaca..6da8ad2c3 100644
> > > --- a/include/libcamera/controls.h
> > > +++ b/include/libcamera/controls.h
> > > @@ -29,6 +29,7 @@ enum ControlType {
> > >       ControlTypeNone,
> > >       ControlTypeBool,
> > >       ControlTypeByte,
> > > +     ControlTypeUnsigned32,
> > >       ControlTypeInteger32,
> > >       ControlTypeInteger64,
> > >       ControlTypeFloat,
> > > @@ -62,6 +63,12 @@ struct control_type<uint8_t> {
> > >       static constexpr std::size_t size = 0;
> > >  };
> > >
> > > +template<>
> > > +struct control_type<uint32_t> {
> > > +     static constexpr ControlType value = ControlTypeUnsigned32;
> > > +     static constexpr std::size_t size = 0;
> > > +};
> > > +
> > >  template<>
> > >  struct control_type<int32_t> {
> > >       static constexpr ControlType value = ControlTypeInteger32;
> > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> > > index 62185d643..8ae295191 100644
> > > --- a/src/libcamera/controls.cpp
> > > +++ b/src/libcamera/controls.cpp
> > > @@ -54,6 +54,7 @@ static constexpr size_t ControlValueSize[] = {
> > >       [ControlTypeNone]               = 0,
> > >       [ControlTypeBool]               = sizeof(bool),
> > >       [ControlTypeByte]               = sizeof(uint8_t),
> > > +     [ControlTypeUnsigned32]         = sizeof(uint32_t),
> > >       [ControlTypeInteger32]          = sizeof(int32_t),
> > >       [ControlTypeInteger64]          = sizeof(int64_t),
> > >       [ControlTypeFloat]              = sizeof(float),
> > > @@ -74,10 +75,12 @@ static constexpr size_t ControlValueSize[] = {
> > >   * The control stores a boolean value
> > >   * \var ControlTypeByte
> > >   * The control stores a byte value as an unsigned 8-bit integer
> > > + * \var ControlTypeUnsigned32
> > > + * The control stores an unsigned 32-bit integer value
> > >   * \var ControlTypeInteger32
> > > - * The control stores a 32-bit integer value
> > > + * The control stores a signed 32-bit integer value
> > >   * \var ControlTypeInteger64
> > > - * The control stores a 64-bit integer value
> > > + * The control stores a signed 64-bit integer value
> > >   * \var ControlTypeFloat
> > >   * The control stores a 32-bit floating point value
> > >   * \var ControlTypeString
> > > @@ -230,6 +233,11 @@ std::string ControlValue::toString() const
> > >                       str += std::to_string(*value);
> > >                       break;
> > >               }
> > > +             case ControlTypeUnsigned32: {
> > > +                     const uint32_t *value = reinterpret_cast<const uint32_t *>(data);
> > > +                     str += std::to_string(*value);
> > > +                     break;
> > > +             }
> > >               case ControlTypeInteger32: {
> > >                       const int32_t *value = reinterpret_cast<const int32_t *>(data);
> > >                       str += std::to_string(*value);
> > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> > > index 68add4f2e..0ba8dcfa0 100644
> > > --- a/src/libcamera/v4l2_device.cpp
> > > +++ b/src/libcamera/v4l2_device.cpp
> > > @@ -9,6 +9,7 @@
> > >
> > >  #include <fcntl.h>
> > >  #include <map>
> > > +#include <stdint.h>
> > >  #include <stdlib.h>
> > >  #include <string.h>
> > >  #include <sys/ioctl.h>
> > > @@ -17,11 +18,14 @@
> > >  #include <vector>
> > >
> > >  #include <linux/v4l2-mediabus.h>
> > > +#include <linux/videodev2.h>
> >
> > Included by the header file
>
> Removed.
>
> >
> > >
> > >  #include <libcamera/base/event_notifier.h>
> > >  #include <libcamera/base/log.h>
> > >  #include <libcamera/base/utils.h>
> > >
> > > +#include <libcamera/controls.h>
> > > +
> >
> > This as well
>
> Removed.
>
> BR,
> Harvey
>
> >
> > >  #include "libcamera/internal/formats.h"
> > >  #include "libcamera/internal/sysfs.h"
> > >
> > > @@ -488,6 +492,9 @@ ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType)
> > >       case V4L2_CTRL_TYPE_BOOLEAN:
> > >               return ControlTypeBool;
> > >
> > > +     case V4L2_CTRL_TYPE_U32:
> > > +             return ControlTypeUnsigned32;
> > > +
> > >       case V4L2_CTRL_TYPE_INTEGER:
> > >               return ControlTypeInteger32;
> > >
> > > @@ -536,6 +543,11 @@ std::optional<ControlInfo> V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl
> > >                                  static_cast<uint8_t>(ctrl.maximum),
> > >                                  static_cast<uint8_t>(ctrl.default_value));
> > >
> > > +     case V4L2_CTRL_TYPE_U32:
> > > +             return ControlInfo(static_cast<uint32_t>(ctrl.minimum),
> > > +                                static_cast<uint32_t>(ctrl.maximum),
> > > +                                static_cast<uint32_t>(ctrl.default_value));
> > > +
> > >       case V4L2_CTRL_TYPE_BOOLEAN:
> > >               return ControlInfo(static_cast<bool>(ctrl.minimum),
> > >                                  static_cast<bool>(ctrl.maximum),
> > > @@ -622,6 +634,7 @@ void V4L2Device::listControls()
> > >               case V4L2_CTRL_TYPE_BITMASK:
> > >               case V4L2_CTRL_TYPE_INTEGER_MENU:
> > >               case V4L2_CTRL_TYPE_U8:
> > > +             case V4L2_CTRL_TYPE_U32:
> > >                       break;
> > >               /* \todo Support other control types. */
> > >               default:
> > > diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp
> > > index 344107fae..6ca85b739 100644
> > > --- a/test/controls/control_value.cpp
> > > +++ b/test/controls/control_value.cpp
> > > @@ -109,6 +109,46 @@ protected:
> > >                       return TestFail;
> > >               }
> > >
> > > +             /*
> > > +              * Unsigned Integer32 type.
> > > +              */
> > > +             value.set(static_cast<uint32_t>(42));
> > > +             if (value.isNone() || value.isArray() ||
> > > +                 value.type() != ControlTypeUnsigned32) {
> > > +                     cerr << "Control type mismatch after setting to uint32_t" << endl;
> > > +                     return TestFail;
> > > +             }
> > > +
> > > +             if (value.get<uint32_t>() != 42) {
> > > +                     cerr << "Control value mismatch after setting to uint32_t" << endl;
> > > +                     return TestFail;
> > > +             }
> > > +
> > > +             if (value.toString() != "42") {
> > > +                     cerr << "Control string mismatch after setting to uint32_t" << endl;
> > > +                     return TestFail;
> > > +             }
> > > +
> > > +             std::array<uint32_t, 4> uint32s{ 3, 14, 15, 9 };
> > > +             value.set(Span<uint32_t>(uint32s));
> > > +             if (value.isNone() || !value.isArray() ||
> > > +                 value.type() != ControlTypeUnsigned32) {
> > > +                     cerr << "Control type mismatch after setting to uint32_t array" << endl;
> > > +                     return TestFail;
> > > +             }
> > > +
> > > +             Span<const uint32_t> uint32sResult = value.get<Span<const uint32_t>>();
> > > +             if (uint32s.size() != uint32sResult.size() ||
> > > +                 !std::equal(uint32s.begin(), uint32s.end(), uint32sResult.begin())) {
> > > +                     cerr << "Control value mismatch after setting to uint32_t array" << endl;
> > > +                     return TestFail;
> > > +             }
> > > +
> > > +             if (value.toString() != "[ 3, 14, 15, 9 ]") {
> > > +                     cerr << "Control string mismatch after setting to uint32_t array" << endl;
> > > +                     return TestFail;
> > > +             }
> > > +
> > >               /*
> > >                * Integer32 type.
> > >                */
> > > --
> > > 2.47.0.163.g1226f6d8fa-goog
> > >
Harvey Yang Oct. 25, 2024, 5:40 p.m. UTC | #4
Hi Jacopo,

On Sat, Oct 26, 2024 at 12:46 AM Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> Hi Harvey
>
> On Fri, Oct 25, 2024 at 11:40:02PM +0800, Cheng-Hao Yang wrote:
> > Hi Jacopo,
> >
> > On Fri, Oct 25, 2024 at 5:15 PM Jacopo Mondi
> > <jacopo.mondi@ideasonboard.com> wrote:
> > >
> > > Hi Harvey
> > >
> > > On Fri, Oct 25, 2024 at 05:30:13AM +0000, Harvey Yang wrote:
> > > > From: Yudhistira Erlandinata <yerlandinata@chromium.org>
> > > >
> > > > V4L2 Controls support a wide variety of types not yet supported by the
> > > > ControlValue type system.
> > > >
> > > > Extend the libcamera ControlValue types to support an explicit 32 bit
> > > > unsigned integer type, and map that to the corresponding
> > > > V4L2_CTRL_TYPE_U32 type within the v4l2_device support class.
> > >
> > > V4L2_CTRL_TYPE_U32 is a control compound type.
> > >
> > >         /* Compound types are >= 0x0100 */
> > >         V4L2_CTRL_COMPOUND_TYPES     = 0x0100,
> > >         V4L2_CTRL_TYPE_U8            = 0x0100,
> > >         V4L2_CTRL_TYPE_U16           = 0x0101,
> > >         V4L2_CTRL_TYPE_U32           = 0x0102,
> > >         V4L2_CTRL_TYPE_AREA          = 0x0106,
> > >
> > > This means it carries a payload and needs to be handled like
> > > V4L2_CTRL_TYPE_U8 is, specifcially when it comes to parsing it from
> > > the V4L2 interface into a libcamera control.
> > >
> > > I'm surprised you don't hit this in V4L2Device::getControls()
> > >
> > >                 if (info.flags & V4L2_CTRL_FLAG_HAS_PAYLOAD) {
> > >                         ControlType type;
> > >
> > >                         switch (info.type) {
> > >                         case V4L2_CTRL_TYPE_U8:
> > >                                 type = ControlTypeByte;
> > >                                 break;
> > >
> > >                         default:
> > >                                 LOG(V4L2, Error)
> > >                                         << "Unsupported payload control type "
> > >                                         << info.type;
> > >                                 return {};
> > >                         }
> > >
> >
> > Thanks! Added for U32 and U16.
>
> Thank you.
>
> Any idea why
>
>                  LOG(V4L2, Error)
>                          << "Unsupported payload control type "
>                          << info.type;
>                  return {};
>
> wasn't hit ?

I think the original purpose of adding it is not for getControls/setControls:
https://chromium-review.googlesource.com/c/chromiumos/third_party/libcamera/+/5669226/1//COMMIT_MSG#12

BR,
Harvey

>
> >
> > > >
> > > > Signed-off-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
> > > > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> > > > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> > > > ---
> > > >  include/libcamera/controls.h    |  7 ++++++
> > > >  src/libcamera/controls.cpp      | 12 ++++++++--
> > > >  src/libcamera/v4l2_device.cpp   | 13 +++++++++++
> > > >  test/controls/control_value.cpp | 40 +++++++++++++++++++++++++++++++++
> > > >  4 files changed, 70 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> > > > index ca60bbaca..6da8ad2c3 100644
> > > > --- a/include/libcamera/controls.h
> > > > +++ b/include/libcamera/controls.h
> > > > @@ -29,6 +29,7 @@ enum ControlType {
> > > >       ControlTypeNone,
> > > >       ControlTypeBool,
> > > >       ControlTypeByte,
> > > > +     ControlTypeUnsigned32,
> > > >       ControlTypeInteger32,
> > > >       ControlTypeInteger64,
> > > >       ControlTypeFloat,
> > > > @@ -62,6 +63,12 @@ struct control_type<uint8_t> {
> > > >       static constexpr std::size_t size = 0;
> > > >  };
> > > >
> > > > +template<>
> > > > +struct control_type<uint32_t> {
> > > > +     static constexpr ControlType value = ControlTypeUnsigned32;
> > > > +     static constexpr std::size_t size = 0;
> > > > +};
> > > > +
> > > >  template<>
> > > >  struct control_type<int32_t> {
> > > >       static constexpr ControlType value = ControlTypeInteger32;
> > > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> > > > index 62185d643..8ae295191 100644
> > > > --- a/src/libcamera/controls.cpp
> > > > +++ b/src/libcamera/controls.cpp
> > > > @@ -54,6 +54,7 @@ static constexpr size_t ControlValueSize[] = {
> > > >       [ControlTypeNone]               = 0,
> > > >       [ControlTypeBool]               = sizeof(bool),
> > > >       [ControlTypeByte]               = sizeof(uint8_t),
> > > > +     [ControlTypeUnsigned32]         = sizeof(uint32_t),
> > > >       [ControlTypeInteger32]          = sizeof(int32_t),
> > > >       [ControlTypeInteger64]          = sizeof(int64_t),
> > > >       [ControlTypeFloat]              = sizeof(float),
> > > > @@ -74,10 +75,12 @@ static constexpr size_t ControlValueSize[] = {
> > > >   * The control stores a boolean value
> > > >   * \var ControlTypeByte
> > > >   * The control stores a byte value as an unsigned 8-bit integer
> > > > + * \var ControlTypeUnsigned32
> > > > + * The control stores an unsigned 32-bit integer value
> > > >   * \var ControlTypeInteger32
> > > > - * The control stores a 32-bit integer value
> > > > + * The control stores a signed 32-bit integer value
> > > >   * \var ControlTypeInteger64
> > > > - * The control stores a 64-bit integer value
> > > > + * The control stores a signed 64-bit integer value
> > > >   * \var ControlTypeFloat
> > > >   * The control stores a 32-bit floating point value
> > > >   * \var ControlTypeString
> > > > @@ -230,6 +233,11 @@ std::string ControlValue::toString() const
> > > >                       str += std::to_string(*value);
> > > >                       break;
> > > >               }
> > > > +             case ControlTypeUnsigned32: {
> > > > +                     const uint32_t *value = reinterpret_cast<const uint32_t *>(data);
> > > > +                     str += std::to_string(*value);
> > > > +                     break;
> > > > +             }
> > > >               case ControlTypeInteger32: {
> > > >                       const int32_t *value = reinterpret_cast<const int32_t *>(data);
> > > >                       str += std::to_string(*value);
> > > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> > > > index 68add4f2e..0ba8dcfa0 100644
> > > > --- a/src/libcamera/v4l2_device.cpp
> > > > +++ b/src/libcamera/v4l2_device.cpp
> > > > @@ -9,6 +9,7 @@
> > > >
> > > >  #include <fcntl.h>
> > > >  #include <map>
> > > > +#include <stdint.h>
> > > >  #include <stdlib.h>
> > > >  #include <string.h>
> > > >  #include <sys/ioctl.h>
> > > > @@ -17,11 +18,14 @@
> > > >  #include <vector>
> > > >
> > > >  #include <linux/v4l2-mediabus.h>
> > > > +#include <linux/videodev2.h>
> > >
> > > Included by the header file
> >
> > Removed.
> >
> > >
> > > >
> > > >  #include <libcamera/base/event_notifier.h>
> > > >  #include <libcamera/base/log.h>
> > > >  #include <libcamera/base/utils.h>
> > > >
> > > > +#include <libcamera/controls.h>
> > > > +
> > >
> > > This as well
> >
> > Removed.
> >
> > BR,
> > Harvey
> >
> > >
> > > >  #include "libcamera/internal/formats.h"
> > > >  #include "libcamera/internal/sysfs.h"
> > > >
> > > > @@ -488,6 +492,9 @@ ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType)
> > > >       case V4L2_CTRL_TYPE_BOOLEAN:
> > > >               return ControlTypeBool;
> > > >
> > > > +     case V4L2_CTRL_TYPE_U32:
> > > > +             return ControlTypeUnsigned32;
> > > > +
> > > >       case V4L2_CTRL_TYPE_INTEGER:
> > > >               return ControlTypeInteger32;
> > > >
> > > > @@ -536,6 +543,11 @@ std::optional<ControlInfo> V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl
> > > >                                  static_cast<uint8_t>(ctrl.maximum),
> > > >                                  static_cast<uint8_t>(ctrl.default_value));
> > > >
> > > > +     case V4L2_CTRL_TYPE_U32:
> > > > +             return ControlInfo(static_cast<uint32_t>(ctrl.minimum),
> > > > +                                static_cast<uint32_t>(ctrl.maximum),
> > > > +                                static_cast<uint32_t>(ctrl.default_value));
> > > > +
> > > >       case V4L2_CTRL_TYPE_BOOLEAN:
> > > >               return ControlInfo(static_cast<bool>(ctrl.minimum),
> > > >                                  static_cast<bool>(ctrl.maximum),
> > > > @@ -622,6 +634,7 @@ void V4L2Device::listControls()
> > > >               case V4L2_CTRL_TYPE_BITMASK:
> > > >               case V4L2_CTRL_TYPE_INTEGER_MENU:
> > > >               case V4L2_CTRL_TYPE_U8:
> > > > +             case V4L2_CTRL_TYPE_U32:
> > > >                       break;
> > > >               /* \todo Support other control types. */
> > > >               default:
> > > > diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp
> > > > index 344107fae..6ca85b739 100644
> > > > --- a/test/controls/control_value.cpp
> > > > +++ b/test/controls/control_value.cpp
> > > > @@ -109,6 +109,46 @@ protected:
> > > >                       return TestFail;
> > > >               }
> > > >
> > > > +             /*
> > > > +              * Unsigned Integer32 type.
> > > > +              */
> > > > +             value.set(static_cast<uint32_t>(42));
> > > > +             if (value.isNone() || value.isArray() ||
> > > > +                 value.type() != ControlTypeUnsigned32) {
> > > > +                     cerr << "Control type mismatch after setting to uint32_t" << endl;
> > > > +                     return TestFail;
> > > > +             }
> > > > +
> > > > +             if (value.get<uint32_t>() != 42) {
> > > > +                     cerr << "Control value mismatch after setting to uint32_t" << endl;
> > > > +                     return TestFail;
> > > > +             }
> > > > +
> > > > +             if (value.toString() != "42") {
> > > > +                     cerr << "Control string mismatch after setting to uint32_t" << endl;
> > > > +                     return TestFail;
> > > > +             }
> > > > +
> > > > +             std::array<uint32_t, 4> uint32s{ 3, 14, 15, 9 };
> > > > +             value.set(Span<uint32_t>(uint32s));
> > > > +             if (value.isNone() || !value.isArray() ||
> > > > +                 value.type() != ControlTypeUnsigned32) {
> > > > +                     cerr << "Control type mismatch after setting to uint32_t array" << endl;
> > > > +                     return TestFail;
> > > > +             }
> > > > +
> > > > +             Span<const uint32_t> uint32sResult = value.get<Span<const uint32_t>>();
> > > > +             if (uint32s.size() != uint32sResult.size() ||
> > > > +                 !std::equal(uint32s.begin(), uint32s.end(), uint32sResult.begin())) {
> > > > +                     cerr << "Control value mismatch after setting to uint32_t array" << endl;
> > > > +                     return TestFail;
> > > > +             }
> > > > +
> > > > +             if (value.toString() != "[ 3, 14, 15, 9 ]") {
> > > > +                     cerr << "Control string mismatch after setting to uint32_t array" << endl;
> > > > +                     return TestFail;
> > > > +             }
> > > > +
> > > >               /*
> > > >                * Integer32 type.
> > > >                */
> > > > --
> > > > 2.47.0.163.g1226f6d8fa-goog
> > > >

Patch
diff mbox series

diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
index ca60bbaca..6da8ad2c3 100644
--- a/include/libcamera/controls.h
+++ b/include/libcamera/controls.h
@@ -29,6 +29,7 @@  enum ControlType {
 	ControlTypeNone,
 	ControlTypeBool,
 	ControlTypeByte,
+	ControlTypeUnsigned32,
 	ControlTypeInteger32,
 	ControlTypeInteger64,
 	ControlTypeFloat,
@@ -62,6 +63,12 @@  struct control_type<uint8_t> {
 	static constexpr std::size_t size = 0;
 };
 
+template<>
+struct control_type<uint32_t> {
+	static constexpr ControlType value = ControlTypeUnsigned32;
+	static constexpr std::size_t size = 0;
+};
+
 template<>
 struct control_type<int32_t> {
 	static constexpr ControlType value = ControlTypeInteger32;
diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
index 62185d643..8ae295191 100644
--- a/src/libcamera/controls.cpp
+++ b/src/libcamera/controls.cpp
@@ -54,6 +54,7 @@  static constexpr size_t ControlValueSize[] = {
 	[ControlTypeNone]		= 0,
 	[ControlTypeBool]		= sizeof(bool),
 	[ControlTypeByte]		= sizeof(uint8_t),
+	[ControlTypeUnsigned32]		= sizeof(uint32_t),
 	[ControlTypeInteger32]		= sizeof(int32_t),
 	[ControlTypeInteger64]		= sizeof(int64_t),
 	[ControlTypeFloat]		= sizeof(float),
@@ -74,10 +75,12 @@  static constexpr size_t ControlValueSize[] = {
  * The control stores a boolean value
  * \var ControlTypeByte
  * The control stores a byte value as an unsigned 8-bit integer
+ * \var ControlTypeUnsigned32
+ * The control stores an unsigned 32-bit integer value
  * \var ControlTypeInteger32
- * The control stores a 32-bit integer value
+ * The control stores a signed 32-bit integer value
  * \var ControlTypeInteger64
- * The control stores a 64-bit integer value
+ * The control stores a signed 64-bit integer value
  * \var ControlTypeFloat
  * The control stores a 32-bit floating point value
  * \var ControlTypeString
@@ -230,6 +233,11 @@  std::string ControlValue::toString() const
 			str += std::to_string(*value);
 			break;
 		}
+		case ControlTypeUnsigned32: {
+			const uint32_t *value = reinterpret_cast<const uint32_t *>(data);
+			str += std::to_string(*value);
+			break;
+		}
 		case ControlTypeInteger32: {
 			const int32_t *value = reinterpret_cast<const int32_t *>(data);
 			str += std::to_string(*value);
diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
index 68add4f2e..0ba8dcfa0 100644
--- a/src/libcamera/v4l2_device.cpp
+++ b/src/libcamera/v4l2_device.cpp
@@ -9,6 +9,7 @@ 
 
 #include <fcntl.h>
 #include <map>
+#include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
 #include <sys/ioctl.h>
@@ -17,11 +18,14 @@ 
 #include <vector>
 
 #include <linux/v4l2-mediabus.h>
+#include <linux/videodev2.h>
 
 #include <libcamera/base/event_notifier.h>
 #include <libcamera/base/log.h>
 #include <libcamera/base/utils.h>
 
+#include <libcamera/controls.h>
+
 #include "libcamera/internal/formats.h"
 #include "libcamera/internal/sysfs.h"
 
@@ -488,6 +492,9 @@  ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType)
 	case V4L2_CTRL_TYPE_BOOLEAN:
 		return ControlTypeBool;
 
+	case V4L2_CTRL_TYPE_U32:
+		return ControlTypeUnsigned32;
+
 	case V4L2_CTRL_TYPE_INTEGER:
 		return ControlTypeInteger32;
 
@@ -536,6 +543,11 @@  std::optional<ControlInfo> V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl
 				   static_cast<uint8_t>(ctrl.maximum),
 				   static_cast<uint8_t>(ctrl.default_value));
 
+	case V4L2_CTRL_TYPE_U32:
+		return ControlInfo(static_cast<uint32_t>(ctrl.minimum),
+				   static_cast<uint32_t>(ctrl.maximum),
+				   static_cast<uint32_t>(ctrl.default_value));
+
 	case V4L2_CTRL_TYPE_BOOLEAN:
 		return ControlInfo(static_cast<bool>(ctrl.minimum),
 				   static_cast<bool>(ctrl.maximum),
@@ -622,6 +634,7 @@  void V4L2Device::listControls()
 		case V4L2_CTRL_TYPE_BITMASK:
 		case V4L2_CTRL_TYPE_INTEGER_MENU:
 		case V4L2_CTRL_TYPE_U8:
+		case V4L2_CTRL_TYPE_U32:
 			break;
 		/* \todo Support other control types. */
 		default:
diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp
index 344107fae..6ca85b739 100644
--- a/test/controls/control_value.cpp
+++ b/test/controls/control_value.cpp
@@ -109,6 +109,46 @@  protected:
 			return TestFail;
 		}
 
+		/*
+		 * Unsigned Integer32 type.
+		 */
+		value.set(static_cast<uint32_t>(42));
+		if (value.isNone() || value.isArray() ||
+		    value.type() != ControlTypeUnsigned32) {
+			cerr << "Control type mismatch after setting to uint32_t" << endl;
+			return TestFail;
+		}
+
+		if (value.get<uint32_t>() != 42) {
+			cerr << "Control value mismatch after setting to uint32_t" << endl;
+			return TestFail;
+		}
+
+		if (value.toString() != "42") {
+			cerr << "Control string mismatch after setting to uint32_t" << endl;
+			return TestFail;
+		}
+
+		std::array<uint32_t, 4> uint32s{ 3, 14, 15, 9 };
+		value.set(Span<uint32_t>(uint32s));
+		if (value.isNone() || !value.isArray() ||
+		    value.type() != ControlTypeUnsigned32) {
+			cerr << "Control type mismatch after setting to uint32_t array" << endl;
+			return TestFail;
+		}
+
+		Span<const uint32_t> uint32sResult = value.get<Span<const uint32_t>>();
+		if (uint32s.size() != uint32sResult.size() ||
+		    !std::equal(uint32s.begin(), uint32s.end(), uint32sResult.begin())) {
+			cerr << "Control value mismatch after setting to uint32_t array" << endl;
+			return TestFail;
+		}
+
+		if (value.toString() != "[ 3, 14, 15, 9 ]") {
+			cerr << "Control string mismatch after setting to uint32_t array" << endl;
+			return TestFail;
+		}
+
 		/*
 		 * Integer32 type.
 		 */