[PATCH v3 1/2] libcamera: Extend u32 control type

Jacopo Mondi jacopo.mondi at ideasonboard.com
Fri Oct 25 11:15:12 CEST 2024


Hi Harvey

On Fri, Oct 25, 2024 at 05:30:13AM +0000, Harvey Yang wrote:
> From: Yudhistira Erlandinata <yerlandinata at 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 at chromium.org>
> Co-developed-by: Harvey Yang <chenghaoyang at chromium.org>
> Signed-off-by: Harvey Yang <chenghaoyang at 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
>


More information about the libcamera-devel mailing list