[PATCH 1/2] libcamera: Allow enumerating u32 control type

[Kieran Bingham]

Quoting Harvey Yang (2024-10-11 18:55:11)
> From: Yudhistira Erlandinata <yerlandinata at google.com>
> 
> Only allowing V4L2_CTRL_TYPE_U32 control type to be listed in the
> V4L2Device::controls_, so it can be used together with function
> V4L2Device::setExtControl. Like many other control types, this type
> is still not supported in the V4L2Device::getControls and
> V4L2Device::setControls.
> 
> 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  |  6 ++++++
>  src/libcamera/controls.cpp    | 12 ++++++++++--
>  src/libcamera/v4l2_device.cpp | 16 +++++++++++++++-
>  3 files changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index ca60bbaca..25f68040d 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,11 @@ struct control_type<uint8_t> {
>         static constexpr std::size_t size = 0;
>  };
>  
> +template<>
> +struct control_type<uint32_t> {
> +       static constexpr ControlType value = ControlTypeUnsigned32;

Does this need 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..f3454ba24 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,12 +634,14 @@ 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:
>                         LOG(V4L2, Debug)
>                                 << "Control " << utils::hex(ctrl.id)
> -                               << " has unsupported type " << ctrl.type;
> +                               << " has unsupported type " << ctrl.type
> +                               << ". Name: " << ctrl.name;

This hunk looks like a good change that should be in an independent
patch. And preumably - if ctrl.name is valid here - then it could
replace the hexdump of ctrl.id ...


>                         continue;
>                 }
>  
> -- 
> 2.47.0.rc1.288.g06298d1525-goog
>