| Message ID | 20240712052920.33396-2-umang.jain@ideasonboard.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series |
|
| Related | show |
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@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 >
Hi Umang, Thank you for the patch. On Fri, Jul 12, 2024 at 10:59:15AM +0530, 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. > > Signed-off-by: Umang Jain <umang.jain@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 " > @@ -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)); What happens if you left TYPE_U32 being handled by the default case, and drop the change to include/libcamera/controls.h ? > + > 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:
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 " @@ -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:
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. Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> --- include/libcamera/controls.h | 5 +++++ src/libcamera/v4l2_device.cpp | 11 +++++++++++ 2 files changed, 16 insertions(+)