[PATCH 1/2] libcamera: Allow enumerating u32 control type
Cheng-Hao Yang
chenghaoyang at chromium.org
Tue Oct 22 11:44:34 CEST 2024
Hi Kieran,
On Tue, Oct 22, 2024 at 5:39 PM Cheng-Hao Yang
<chenghaoyang at chromium.org> wrote:
>
> Hi Kieran,
>
> On Tue, Oct 22, 2024 at 12:27 AM Kieran Bingham
> <kieran.bingham at ideasonboard.com> wrote:
> >
> > 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; ?
>
> Sorry, I don't get what you mean...?
Ah sorry, CrOS mtkisp7 branch doesn't have the patch from Paul.
Added in the next version.
>
> >
> > > +};
> > > +
> > > 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 ...
>
> Right, I'll remove this hunk from the patch.
>
> BR,
> Harvey
>
> >
> >
> > > continue;
> > > }
> > >
> > > --
> > > 2.47.0.rc1.288.g06298d1525-goog
> > >
More information about the libcamera-devel
mailing list