[PATCH v5 2/2] libcamera: Extend u16 control type
Kieran Bingham
kieran.bingham at ideasonboard.com
Thu Nov 28 18:45:47 CET 2024
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 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 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 at chromium.org>
> > Co-developed-by: Harvey Yang <chenghaoyang at chromium.org>
> > Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
>
> Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham at 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
> >
More information about the libcamera-devel
mailing list