[RFC PATCH 1/6] libcamera: v4l2_device: Add support for V4L2_CTRL_TYPE_U32

[Jacopo Mondi]

Hi Umang

On Fri, Jul 12, 2024 at 10:59:15AM GMT, Umang Jain wrote:
> Support for the U16 and U32 compound control types is missing. U16 will
> require a new libcamera control type, but U32 maps to the existing
> ControlTypeInteger32 and can be added easily.

Can this patch be broken out and fast-tracked maybe ?

>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
>  include/libcamera/controls.h  |  5 +++++
>  src/libcamera/v4l2_device.cpp | 11 +++++++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 7c2bb287..c6db4ebb 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -57,6 +57,11 @@ struct control_type<uint8_t> {
>  	static constexpr ControlType value = ControlTypeByte;
>  };
>
> +template<>
> +struct control_type<uint32_t> {
> +	static constexpr ControlType value = ControlTypeInteger32;
> +};
> +
>  template<>
>  struct control_type<int32_t> {
>  	static constexpr ControlType value = ControlTypeInteger32;
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 4a2048cf..db20f31c 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -212,6 +212,10 @@ ControlList V4L2Device::getControls(const std::vector<uint32_t> &ids)
>  				type = ControlTypeByte;
>  				break;
>
> +			case V4L2_CTRL_TYPE_U32:
> +				type = ControlTypeInteger32;
> +				break;
> +
>  			default:
>  				LOG(V4L2, Error)
>  					<< "Unsupported payload control type "

I was about to say that you should also have changed the below

			v4l2Ctrl.p_u8 = data.data();

to

			v4l2Ctrl.p_u32 = data.data();

But as this is a union it probably doesn't make a difference as we
don't access the control value by pointer but the content will be
stored in ControlValue::data ?

Have you verified you can actually read the content of
the array control using p_u8 unconditionally ?


> @@ -491,6 +495,7 @@ ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType)
>  		return ControlTypeBool;
>
>  	case V4L2_CTRL_TYPE_INTEGER:
> +	case V4L2_CTRL_TYPE_U32:
>  		return ControlTypeInteger32;
>
>  	case V4L2_CTRL_TYPE_INTEGER64:
> @@ -543,6 +548,11 @@ std::optional<ControlInfo> V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl
>  				   static_cast<bool>(ctrl.maximum),
>  				   static_cast<bool>(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_INTEGER64:
>  		return ControlInfo(static_cast<int64_t>(ctrl.minimum),
>  				   static_cast<int64_t>(ctrl.maximum),
> @@ -624,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:
> --
> 2.45.0
>