Message ID | 20241029161117.1197603-3-chenghaoyang@chromium.org |
---|---|
State | Accepted |
Commit | d711a4c015e74c7a77cfeba17b621909385f4494 |
Headers | show |
Series |
|
Related | show |
Hi Harvey On Tue, Oct 29, 2024 at 04:07:13PM +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 16 bit > unsigned integer type, and map that to the corresponding > V4L2_CTRL_TYPE_U16 type within the v4l2_device support class. > > It's used on some camera metadata that is of length 16-bits, > for example JPEG metadata headers. > > Signed-off-by: Yudhistira Erlandinata <yerlandinata@chromium.org> > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org> > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks j > --- > include/libcamera/controls.h | 7 ++++++ > src/libcamera/controls.cpp | 8 +++++++ > src/libcamera/v4l2_device.cpp | 28 +++++++++++++++++++++++ > test/controls/control_value.cpp | 40 +++++++++++++++++++++++++++++++++ > 4 files changed, 83 insertions(+) > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > index 6da8ad2c3..162115bb5 100644 > --- a/include/libcamera/controls.h > +++ b/include/libcamera/controls.h > @@ -29,6 +29,7 @@ enum ControlType { > ControlTypeNone, > ControlTypeBool, > ControlTypeByte, > + ControlTypeUnsigned16, > ControlTypeUnsigned32, > ControlTypeInteger32, > ControlTypeInteger64, > @@ -63,6 +64,12 @@ struct control_type<uint8_t> { > static constexpr std::size_t size = 0; > }; > > +template<> > +struct control_type<uint16_t> { > + static constexpr ControlType value = ControlTypeUnsigned16; > + static constexpr std::size_t size = 0; > +}; > + > template<> > struct control_type<uint32_t> { > static constexpr ControlType value = ControlTypeUnsigned32; > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp > index 8ae295191..3a840bb22 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), > + [ControlTypeUnsigned16] = sizeof(uint16_t), > [ControlTypeUnsigned32] = sizeof(uint32_t), > [ControlTypeInteger32] = sizeof(int32_t), > [ControlTypeInteger64] = sizeof(int64_t), > @@ -75,6 +76,8 @@ 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 ControlTypeUnsigned16 > + * The control stores an unsigned 16-bit integer value > * \var ControlTypeUnsigned32 > * The control stores an unsigned 32-bit integer value > * \var ControlTypeInteger32 > @@ -233,6 +236,11 @@ std::string ControlValue::toString() const > str += std::to_string(*value); > break; > } > + case ControlTypeUnsigned16: { > + const uint16_t *value = reinterpret_cast<const uint16_t *>(data); > + str += std::to_string(*value); > + break; > + } > case ControlTypeUnsigned32: { > const uint32_t *value = reinterpret_cast<const uint32_t *>(data); > str += std::to_string(*value); > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > index 90fa6b278..f0c6fa63d 100644 > --- a/src/libcamera/v4l2_device.cpp > +++ b/src/libcamera/v4l2_device.cpp > @@ -216,6 +216,13 @@ ControlList V4L2Device::getControls(const std::vector<uint32_t> &ids) > v4l2Ctrl.p_u8 = data.data(); > break; > > + case V4L2_CTRL_TYPE_U16: > + type = ControlTypeUnsigned16; > + value.reserve(type, true, info.elems); > + data = value.data(); > + v4l2Ctrl.p_u16 = reinterpret_cast<uint16_t *>(data.data()); > + break; > + > case V4L2_CTRL_TYPE_U32: > type = ControlTypeUnsigned32; > value.reserve(type, true, info.elems); > @@ -307,6 +314,18 @@ int V4L2Device::setControls(ControlList *ctrls) > /* Set the v4l2_ext_control value for the write operation. */ > ControlValue &value = ctrl->second; > switch (iter->first->type()) { > + case ControlTypeUnsigned16: { > + if (value.isArray()) { > + Span<uint8_t> data = value.data(); > + v4l2Ctrl.p_u16 = reinterpret_cast<uint16_t *>(data.data()); > + v4l2Ctrl.size = data.size(); > + } else { > + v4l2Ctrl.value = value.get<uint16_t>(); > + } > + > + break; > + } > + > case ControlTypeUnsigned32: { > if (value.isArray()) { > Span<uint8_t> data = value.data(); > @@ -508,6 +527,9 @@ ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType) > case V4L2_CTRL_TYPE_BOOLEAN: > return ControlTypeBool; > > + case V4L2_CTRL_TYPE_U16: > + return ControlTypeUnsigned16; > + > case V4L2_CTRL_TYPE_U32: > return ControlTypeUnsigned32; > > @@ -559,6 +581,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_U16: > + return ControlInfo(static_cast<uint16_t>(ctrl.minimum), > + static_cast<uint16_t>(ctrl.maximum), > + static_cast<uint16_t>(ctrl.default_value)); > + > case V4L2_CTRL_TYPE_U32: > return ControlInfo(static_cast<uint32_t>(ctrl.minimum), > static_cast<uint32_t>(ctrl.maximum), > @@ -650,6 +677,7 @@ void V4L2Device::listControls() > case V4L2_CTRL_TYPE_BITMASK: > case V4L2_CTRL_TYPE_INTEGER_MENU: > case V4L2_CTRL_TYPE_U8: > + case V4L2_CTRL_TYPE_U16: > case V4L2_CTRL_TYPE_U32: > break; > /* \todo Support other control types. */ > diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp > index 6ca85b739..5084fd0cf 100644 > --- a/test/controls/control_value.cpp > +++ b/test/controls/control_value.cpp > @@ -109,6 +109,46 @@ protected: > return TestFail; > } > > + /* > + * Unsigned Integer16 type. > + */ > + value.set(static_cast<uint16_t>(42)); > + if (value.isNone() || value.isArray() || > + value.type() != ControlTypeUnsigned16) { > + cerr << "Control type mismatch after setting to uint16_t" << endl; > + return TestFail; > + } > + > + if (value.get<uint16_t>() != 42) { > + cerr << "Control value mismatch after setting to uint16_t" << endl; > + return TestFail; > + } > + > + if (value.toString() != "42") { > + cerr << "Control string mismatch after setting to uint16_t" << endl; > + return TestFail; > + } > + > + std::array<uint16_t, 4> uint16s{ 3, 14, 15, 9 }; > + value.set(Span<uint16_t>(uint16s)); > + if (value.isNone() || !value.isArray() || > + value.type() != ControlTypeUnsigned16) { > + cerr << "Control type mismatch after setting to uint16_t array" << endl; > + return TestFail; > + } > + > + Span<const uint16_t> uint16sResult = value.get<Span<const uint16_t>>(); > + if (uint16s.size() != uint16sResult.size() || > + !std::equal(uint16s.begin(), uint16s.end(), uint16sResult.begin())) { > + cerr << "Control value mismatch after setting to uint16_t array" << endl; > + return TestFail; > + } > + > + if (value.toString() != "[ 3, 14, 15, 9 ]") { > + cerr << "Control string mismatch after setting to uint16_t array" << endl; > + return TestFail; > + } > + > /* > * Unsigned Integer32 type. > */ > -- > 2.47.0.163.g1226f6d8fa-goog >
Quoting Jacopo Mondi (2024-11-28 17:39:23) > Hi Harvey > > On Tue, Oct 29, 2024 at 04:07:13PM +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 16 bit > > unsigned integer type, and map that to the corresponding > > V4L2_CTRL_TYPE_U16 type within the v4l2_device support class. > > > > It's used on some camera metadata that is of length 16-bits, > > for example JPEG metadata headers. > > > > Signed-off-by: Yudhistira Erlandinata <yerlandinata@chromium.org> > > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org> > > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org> > > Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Thanks > j > > > --- > > include/libcamera/controls.h | 7 ++++++ > > src/libcamera/controls.cpp | 8 +++++++ > > src/libcamera/v4l2_device.cpp | 28 +++++++++++++++++++++++ > > test/controls/control_value.cpp | 40 +++++++++++++++++++++++++++++++++ > > 4 files changed, 83 insertions(+) > > > > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h > > index 6da8ad2c3..162115bb5 100644 > > --- a/include/libcamera/controls.h > > +++ b/include/libcamera/controls.h > > @@ -29,6 +29,7 @@ enum ControlType { > > ControlTypeNone, > > ControlTypeBool, > > ControlTypeByte, > > + ControlTypeUnsigned16, > > ControlTypeUnsigned32, > > ControlTypeInteger32, > > ControlTypeInteger64, > > @@ -63,6 +64,12 @@ struct control_type<uint8_t> { > > static constexpr std::size_t size = 0; > > }; > > > > +template<> > > +struct control_type<uint16_t> { > > + static constexpr ControlType value = ControlTypeUnsigned16; > > + static constexpr std::size_t size = 0; > > +}; > > + > > template<> > > struct control_type<uint32_t> { > > static constexpr ControlType value = ControlTypeUnsigned32; > > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp > > index 8ae295191..3a840bb22 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), > > + [ControlTypeUnsigned16] = sizeof(uint16_t), > > [ControlTypeUnsigned32] = sizeof(uint32_t), > > [ControlTypeInteger32] = sizeof(int32_t), > > [ControlTypeInteger64] = sizeof(int64_t), > > @@ -75,6 +76,8 @@ 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 ControlTypeUnsigned16 > > + * The control stores an unsigned 16-bit integer value > > * \var ControlTypeUnsigned32 > > * The control stores an unsigned 32-bit integer value > > * \var ControlTypeInteger32 > > @@ -233,6 +236,11 @@ std::string ControlValue::toString() const > > str += std::to_string(*value); > > break; > > } > > + case ControlTypeUnsigned16: { > > + const uint16_t *value = reinterpret_cast<const uint16_t *>(data); > > + str += std::to_string(*value); > > + break; > > + } > > case ControlTypeUnsigned32: { > > const uint32_t *value = reinterpret_cast<const uint32_t *>(data); > > str += std::to_string(*value); > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp > > index 90fa6b278..f0c6fa63d 100644 > > --- a/src/libcamera/v4l2_device.cpp > > +++ b/src/libcamera/v4l2_device.cpp > > @@ -216,6 +216,13 @@ ControlList V4L2Device::getControls(const std::vector<uint32_t> &ids) > > v4l2Ctrl.p_u8 = data.data(); > > break; > > > > + case V4L2_CTRL_TYPE_U16: > > + type = ControlTypeUnsigned16; > > + value.reserve(type, true, info.elems); > > + data = value.data(); > > + v4l2Ctrl.p_u16 = reinterpret_cast<uint16_t *>(data.data()); > > + break; > > + > > case V4L2_CTRL_TYPE_U32: > > type = ControlTypeUnsigned32; > > value.reserve(type, true, info.elems); > > @@ -307,6 +314,18 @@ int V4L2Device::setControls(ControlList *ctrls) > > /* Set the v4l2_ext_control value for the write operation. */ > > ControlValue &value = ctrl->second; > > switch (iter->first->type()) { > > + case ControlTypeUnsigned16: { > > + if (value.isArray()) { > > + Span<uint8_t> data = value.data(); > > + v4l2Ctrl.p_u16 = reinterpret_cast<uint16_t *>(data.data()); > > + v4l2Ctrl.size = data.size(); > > + } else { > > + v4l2Ctrl.value = value.get<uint16_t>(); > > + } > > + > > + break; > > + } > > + > > case ControlTypeUnsigned32: { > > if (value.isArray()) { > > Span<uint8_t> data = value.data(); > > @@ -508,6 +527,9 @@ ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType) > > case V4L2_CTRL_TYPE_BOOLEAN: > > return ControlTypeBool; > > > > + case V4L2_CTRL_TYPE_U16: > > + return ControlTypeUnsigned16; > > + > > case V4L2_CTRL_TYPE_U32: > > return ControlTypeUnsigned32; > > > > @@ -559,6 +581,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_U16: > > + return ControlInfo(static_cast<uint16_t>(ctrl.minimum), > > + static_cast<uint16_t>(ctrl.maximum), > > + static_cast<uint16_t>(ctrl.default_value)); > > + > > case V4L2_CTRL_TYPE_U32: > > return ControlInfo(static_cast<uint32_t>(ctrl.minimum), > > static_cast<uint32_t>(ctrl.maximum), > > @@ -650,6 +677,7 @@ void V4L2Device::listControls() > > case V4L2_CTRL_TYPE_BITMASK: > > case V4L2_CTRL_TYPE_INTEGER_MENU: > > case V4L2_CTRL_TYPE_U8: > > + case V4L2_CTRL_TYPE_U16: > > case V4L2_CTRL_TYPE_U32: > > break; > > /* \todo Support other control types. */ > > diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp > > index 6ca85b739..5084fd0cf 100644 > > --- a/test/controls/control_value.cpp > > +++ b/test/controls/control_value.cpp > > @@ -109,6 +109,46 @@ protected: > > return TestFail; > > } > > > > + /* > > + * Unsigned Integer16 type. > > + */ > > + value.set(static_cast<uint16_t>(42)); > > + if (value.isNone() || value.isArray() || > > + value.type() != ControlTypeUnsigned16) { > > + cerr << "Control type mismatch after setting to uint16_t" << endl; > > + return TestFail; > > + } > > + > > + if (value.get<uint16_t>() != 42) { > > + cerr << "Control value mismatch after setting to uint16_t" << endl; > > + return TestFail; > > + } > > + > > + if (value.toString() != "42") { > > + cerr << "Control string mismatch after setting to uint16_t" << endl; > > + return TestFail; > > + } > > + > > + std::array<uint16_t, 4> uint16s{ 3, 14, 15, 9 }; > > + value.set(Span<uint16_t>(uint16s)); > > + if (value.isNone() || !value.isArray() || > > + value.type() != ControlTypeUnsigned16) { > > + cerr << "Control type mismatch after setting to uint16_t array" << endl; > > + return TestFail; > > + } > > + > > + Span<const uint16_t> uint16sResult = value.get<Span<const uint16_t>>(); > > + if (uint16s.size() != uint16sResult.size() || > > + !std::equal(uint16s.begin(), uint16s.end(), uint16sResult.begin())) { > > + cerr << "Control value mismatch after setting to uint16_t array" << endl; > > + return TestFail; > > + } > > + > > + if (value.toString() != "[ 3, 14, 15, 9 ]") { > > + cerr << "Control string mismatch after setting to uint16_t array" << endl; > > + return TestFail; > > + } > > + > > /* > > * Unsigned Integer32 type. > > */ > > -- > > 2.47.0.163.g1226f6d8fa-goog > >
diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 6da8ad2c3..162115bb5 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -29,6 +29,7 @@ enum ControlType { ControlTypeNone, ControlTypeBool, ControlTypeByte, + ControlTypeUnsigned16, ControlTypeUnsigned32, ControlTypeInteger32, ControlTypeInteger64, @@ -63,6 +64,12 @@ struct control_type<uint8_t> { static constexpr std::size_t size = 0; }; +template<> +struct control_type<uint16_t> { + static constexpr ControlType value = ControlTypeUnsigned16; + static constexpr std::size_t size = 0; +}; + template<> struct control_type<uint32_t> { static constexpr ControlType value = ControlTypeUnsigned32; diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 8ae295191..3a840bb22 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), + [ControlTypeUnsigned16] = sizeof(uint16_t), [ControlTypeUnsigned32] = sizeof(uint32_t), [ControlTypeInteger32] = sizeof(int32_t), [ControlTypeInteger64] = sizeof(int64_t), @@ -75,6 +76,8 @@ 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 ControlTypeUnsigned16 + * The control stores an unsigned 16-bit integer value * \var ControlTypeUnsigned32 * The control stores an unsigned 32-bit integer value * \var ControlTypeInteger32 @@ -233,6 +236,11 @@ std::string ControlValue::toString() const str += std::to_string(*value); break; } + case ControlTypeUnsigned16: { + const uint16_t *value = reinterpret_cast<const uint16_t *>(data); + str += std::to_string(*value); + break; + } case ControlTypeUnsigned32: { const uint32_t *value = reinterpret_cast<const uint32_t *>(data); str += std::to_string(*value); diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index 90fa6b278..f0c6fa63d 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -216,6 +216,13 @@ ControlList V4L2Device::getControls(const std::vector<uint32_t> &ids) v4l2Ctrl.p_u8 = data.data(); break; + case V4L2_CTRL_TYPE_U16: + type = ControlTypeUnsigned16; + value.reserve(type, true, info.elems); + data = value.data(); + v4l2Ctrl.p_u16 = reinterpret_cast<uint16_t *>(data.data()); + break; + case V4L2_CTRL_TYPE_U32: type = ControlTypeUnsigned32; value.reserve(type, true, info.elems); @@ -307,6 +314,18 @@ int V4L2Device::setControls(ControlList *ctrls) /* Set the v4l2_ext_control value for the write operation. */ ControlValue &value = ctrl->second; switch (iter->first->type()) { + case ControlTypeUnsigned16: { + if (value.isArray()) { + Span<uint8_t> data = value.data(); + v4l2Ctrl.p_u16 = reinterpret_cast<uint16_t *>(data.data()); + v4l2Ctrl.size = data.size(); + } else { + v4l2Ctrl.value = value.get<uint16_t>(); + } + + break; + } + case ControlTypeUnsigned32: { if (value.isArray()) { Span<uint8_t> data = value.data(); @@ -508,6 +527,9 @@ ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType) case V4L2_CTRL_TYPE_BOOLEAN: return ControlTypeBool; + case V4L2_CTRL_TYPE_U16: + return ControlTypeUnsigned16; + case V4L2_CTRL_TYPE_U32: return ControlTypeUnsigned32; @@ -559,6 +581,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_U16: + return ControlInfo(static_cast<uint16_t>(ctrl.minimum), + static_cast<uint16_t>(ctrl.maximum), + static_cast<uint16_t>(ctrl.default_value)); + case V4L2_CTRL_TYPE_U32: return ControlInfo(static_cast<uint32_t>(ctrl.minimum), static_cast<uint32_t>(ctrl.maximum), @@ -650,6 +677,7 @@ void V4L2Device::listControls() case V4L2_CTRL_TYPE_BITMASK: case V4L2_CTRL_TYPE_INTEGER_MENU: case V4L2_CTRL_TYPE_U8: + case V4L2_CTRL_TYPE_U16: case V4L2_CTRL_TYPE_U32: break; /* \todo Support other control types. */ diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp index 6ca85b739..5084fd0cf 100644 --- a/test/controls/control_value.cpp +++ b/test/controls/control_value.cpp @@ -109,6 +109,46 @@ protected: return TestFail; } + /* + * Unsigned Integer16 type. + */ + value.set(static_cast<uint16_t>(42)); + if (value.isNone() || value.isArray() || + value.type() != ControlTypeUnsigned16) { + cerr << "Control type mismatch after setting to uint16_t" << endl; + return TestFail; + } + + if (value.get<uint16_t>() != 42) { + cerr << "Control value mismatch after setting to uint16_t" << endl; + return TestFail; + } + + if (value.toString() != "42") { + cerr << "Control string mismatch after setting to uint16_t" << endl; + return TestFail; + } + + std::array<uint16_t, 4> uint16s{ 3, 14, 15, 9 }; + value.set(Span<uint16_t>(uint16s)); + if (value.isNone() || !value.isArray() || + value.type() != ControlTypeUnsigned16) { + cerr << "Control type mismatch after setting to uint16_t array" << endl; + return TestFail; + } + + Span<const uint16_t> uint16sResult = value.get<Span<const uint16_t>>(); + if (uint16s.size() != uint16sResult.size() || + !std::equal(uint16s.begin(), uint16s.end(), uint16sResult.begin())) { + cerr << "Control value mismatch after setting to uint16_t array" << endl; + return TestFail; + } + + if (value.toString() != "[ 3, 14, 15, 9 ]") { + cerr << "Control string mismatch after setting to uint16_t array" << endl; + return TestFail; + } + /* * Unsigned Integer32 type. */