[libcamera-devel] [PATCH v7 2/6] libcamera: V4L2Device: Support v4l2 menu control
Jacopo Mondi
jacopo at jmondi.org
Mon Jun 7 16:46:53 CEST 2021
Hi Hiro,
On Mon, Jun 07, 2021 at 04:16:01PM +0200, Jacopo Mondi wrote:
> Hi Hiro,
>
> On Mon, Jun 07, 2021 at 10:13:58AM +0900, Hirokazu Honda wrote:
> > This adds a support of v4l2 menu. The control info for v4l2 menu
> > contains indices without names and 64bit values of querymenu.
> >
> > Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> > ---
> > include/libcamera/internal/v4l2_device.h | 2 +
> > src/libcamera/v4l2_device.cpp | 53 ++++++++++++++++++++++--
> > 2 files changed, 51 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h
> > index b82e2a14..687be9b2 100644
> > --- a/include/libcamera/internal/v4l2_device.h
> > +++ b/include/libcamera/internal/v4l2_device.h
> > @@ -56,6 +56,8 @@ protected:
> > int fd() const { return fd_; }
> >
> > private:
> > + ControlInfo v4l2MenuControlInfo(const v4l2_query_ext_ctrl &ctrl);
> > +
> > void listControls();
> > void updateControls(ControlList *ctrls,
> > Span<const v4l2_ext_control> v4l2Ctrls);
> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> > index b39c6266..40e2c74c 100644
> > --- a/src/libcamera/v4l2_device.cpp
> > +++ b/src/libcamera/v4l2_device.cpp
> > @@ -524,6 +524,38 @@ int V4L2Device::ioctl(unsigned long request, void *argp)
> > * \return The V4L2 device file descriptor, -1 if the device node is not open
> > */
> >
> > +/**
> > + * \brief Create ControlInfo for a V4L2 menu ctrl
> > + * \param[in] ctrl The v4l2_query_ext_ctrl that represents a menu
> > + *
> > + * The created ControlInfo contains indices acquired by VIDIOC_QUERYMENU.
> > + *
> > + * \return ControlInfo for a V4L2 menu ctrl.
>
> nit: no full stop at the end
> Can be changed when applying!
>
> Thanks for keep pushing this series
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>
> Thanks
> j
>
>
> > + */
> > +ControlInfo V4L2Device::v4l2MenuControlInfo(
> > + const struct v4l2_query_ext_ctrl &ctrl)
You know, it looks better on a single line even if it's 84 cols (we
have an hard limit at 120 cols, soft one at 80)
I just noticed a little issue, with this patch:
This new function is defined as a class member, while the other
v4l2-ctrl related function are defined in an anonymous namespace.
Would you consider this patch on top to make all functions class
members ?
-------------------------------------------------------------------------------
diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h
index 687be9b25273..5a8b99e067ab 100644
--- a/include/libcamera/internal/v4l2_device.h
+++ b/include/libcamera/internal/v4l2_device.h
@@ -56,6 +56,9 @@ protected:
int fd() const { return fd_; }
private:
+ ControlType v4l2CtrlType(uint32_t ctrlType);
+ std::unique_ptr<ControlId> v4l2ControlId(const v4l2_query_ext_ctrl &ctrl);
+ ControlInfo v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl);
ControlInfo v4l2MenuControlInfo(const v4l2_query_ext_ctrl &ctrl);
void listControls();
diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
index 40e2c74c401a..452cc9b5e430 100644
--- a/src/libcamera/v4l2_device.cpp
+++ b/src/libcamera/v4l2_device.cpp
@@ -30,73 +30,6 @@ namespace libcamera {
LOG_DEFINE_CATEGORY(V4L2)
-namespace {
-
-ControlType v4l2CtrlType(uint32_t ctrlType)
-{
- switch (ctrlType) {
- case V4L2_CTRL_TYPE_U8:
- return ControlTypeByte;
-
- case V4L2_CTRL_TYPE_BOOLEAN:
- return ControlTypeBool;
-
- case V4L2_CTRL_TYPE_INTEGER:
- return ControlTypeInteger32;
-
- case V4L2_CTRL_TYPE_INTEGER64:
- return ControlTypeInteger64;
-
- case V4L2_CTRL_TYPE_MENU:
- case V4L2_CTRL_TYPE_BUTTON:
- case V4L2_CTRL_TYPE_BITMASK:
- case V4L2_CTRL_TYPE_INTEGER_MENU:
- /*
- * More precise types may be needed, for now use a 32-bit
- * integer type.
- */
- return ControlTypeInteger32;
-
- default:
- return ControlTypeNone;
- }
-}
-
-std::unique_ptr<ControlId> v4l2ControlId(const v4l2_query_ext_ctrl &ctrl)
-{
- const size_t len = strnlen(ctrl.name, sizeof(ctrl.name));
- const std::string name(static_cast<const char *>(ctrl.name), len);
- const ControlType type = v4l2CtrlType(ctrl.type);
-
- return std::make_unique<ControlId>(ctrl.id, name, type);
-}
-
-ControlInfo v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl)
-{
- switch (ctrl.type) {
- case V4L2_CTRL_TYPE_U8:
- return ControlInfo(static_cast<uint8_t>(ctrl.minimum),
- static_cast<uint8_t>(ctrl.maximum),
- static_cast<uint8_t>(ctrl.default_value));
-
- case V4L2_CTRL_TYPE_BOOLEAN:
- return ControlInfo(static_cast<bool>(ctrl.minimum),
- static_cast<bool>(ctrl.maximum),
- static_cast<bool>(ctrl.default_value));
-
- case V4L2_CTRL_TYPE_INTEGER64:
- return ControlInfo(static_cast<int64_t>(ctrl.minimum),
- static_cast<int64_t>(ctrl.maximum),
- static_cast<int64_t>(ctrl.default_value));
-
- default:
- return ControlInfo(static_cast<int32_t>(ctrl.minimum),
- static_cast<int32_t>(ctrl.maximum),
- static_cast<int32_t>(ctrl.default_value));
- }
-}
-} /* namespace */
-
/**
* \class V4L2Device
* \brief Base class for V4L2VideoDevice and V4L2Subdevice
@@ -524,6 +457,77 @@ int V4L2Device::ioctl(unsigned long request, void *argp)
* \return The V4L2 device file descriptor, -1 if the device node is not open
*/
+ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType)
+{
+ switch (ctrlType) {
+ case V4L2_CTRL_TYPE_U8:
+ return ControlTypeByte;
+
+ case V4L2_CTRL_TYPE_BOOLEAN:
+ return ControlTypeBool;
+
+ case V4L2_CTRL_TYPE_INTEGER:
+ return ControlTypeInteger32;
+
+ case V4L2_CTRL_TYPE_INTEGER64:
+ return ControlTypeInteger64;
+
+ case V4L2_CTRL_TYPE_MENU:
+ case V4L2_CTRL_TYPE_BUTTON:
+ case V4L2_CTRL_TYPE_BITMASK:
+ case V4L2_CTRL_TYPE_INTEGER_MENU:
+ /*
+ * More precise types may be needed, for now use a 32-bit
+ * integer type.
+ */
+ return ControlTypeInteger32;
+
+ default:
+ return ControlTypeNone;
+ }
+}
+
+std::unique_ptr<ControlId> V4L2Device::v4l2ControlId(const v4l2_query_ext_ctrl &ctrl)
+{
+ const size_t len = strnlen(ctrl.name, sizeof(ctrl.name));
+ const std::string name(static_cast<const char *>(ctrl.name), len);
+ const ControlType type = v4l2CtrlType(ctrl.type);
+
+ return std::make_unique<ControlId>(ctrl.id, name, type);
+}
+
+/**
+ * \brief Create ControlInfo for a V4L2 ctrl
+ * \param[in] ctrl The v4l2_query_ext_ctrl that represents a control
+ *
+ * \return ControlInfo for a V4L2 ctrl
+ */
+ControlInfo V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl)
+{
+ switch (ctrl.type) {
+ case V4L2_CTRL_TYPE_U8:
+ return ControlInfo(static_cast<uint8_t>(ctrl.minimum),
+ static_cast<uint8_t>(ctrl.maximum),
+ static_cast<uint8_t>(ctrl.default_value));
+
+ case V4L2_CTRL_TYPE_BOOLEAN:
+ return ControlInfo(static_cast<bool>(ctrl.minimum),
+ static_cast<bool>(ctrl.maximum),
+ static_cast<bool>(ctrl.default_value));
+
+ case V4L2_CTRL_TYPE_INTEGER64:
+ return ControlInfo(static_cast<int64_t>(ctrl.minimum),
+ static_cast<int64_t>(ctrl.maximum),
+ static_cast<int64_t>(ctrl.default_value));
+
+ default:
+ return ControlInfo(static_cast<int32_t>(ctrl.minimum),
+ static_cast<int32_t>(ctrl.maximum),
+ static_cast<int32_t>(ctrl.default_value));
+ }
+}
+
+
/**
* \brief Create ControlInfo for a V4L2 menu ctrl
* \param[in] ctrl The v4l2_query_ext_ctrl that represents a menu
-------------------------------------------------------------------------------
> > +{
> > + std::vector<ControlValue> indices;
> > + struct v4l2_querymenu menu = {};
> > + menu.id = ctrl.id;
> > +
> > + if (ctrl.minimum < 0)
> > + return ControlInfo();
> > +
> > + ControlValue def = {};
> > + for (int32_t index = ctrl.minimum; index <= ctrl.maximum; ++index) {
> > + menu.index = index;
> > + if (ioctl(VIDIOC_QUERYMENU, &menu) != 0)
> > + continue;
> > +
> > + indices.push_back(index);
> > + if (index == ctrl.default_value)
> > + def = ControlValue(index);
> > + }
> > +
> > + return ControlInfo(indices, def);
> > +}
> > +
> > /*
> > * \brief List and store information about all controls supported by the
> > * V4L2 device
> > @@ -533,7 +565,6 @@ void V4L2Device::listControls()
> > ControlInfoMap::Map ctrls;
> > struct v4l2_query_ext_ctrl ctrl = {};
> >
> > - /* \todo Add support for menu controls. */
> > while (1) {
> > ctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL |
> > V4L2_CTRL_FLAG_NEXT_COMPOUND;
> > @@ -544,16 +575,23 @@ void V4L2Device::listControls()
> > ctrl.flags & V4L2_CTRL_FLAG_DISABLED)
> > continue;
> >
> > + ControlInfo ctrlInfo;
> > +
> > switch (ctrl.type) {
> > case V4L2_CTRL_TYPE_INTEGER:
> > case V4L2_CTRL_TYPE_BOOLEAN:
> > - case V4L2_CTRL_TYPE_MENU:
> > case V4L2_CTRL_TYPE_BUTTON:
> > case V4L2_CTRL_TYPE_INTEGER64:
> > case V4L2_CTRL_TYPE_BITMASK:
> > - case V4L2_CTRL_TYPE_INTEGER_MENU:
> > case V4L2_CTRL_TYPE_U8:
> > + ctrlInfo = v4l2ControlInfo(ctrl);
> > + break;
> > +
> > + case V4L2_CTRL_TYPE_INTEGER_MENU:
> > + case V4L2_CTRL_TYPE_MENU:
> > + ctrlInfo = v4l2MenuControlInfo(ctrl);
> > break;
> > +
> > /* \todo Support other control types. */
> > default:
> > LOG(V4L2, Debug)
> > @@ -562,10 +600,13 @@ void V4L2Device::listControls()
> > continue;
> > }
> >
> > + LOG(V4L2, Debug) << "Control: " << ctrl.name
> > + << " (" << utils::hex(ctrl.id) << ")";
> > +
> > controlIds_.emplace_back(v4l2ControlId(ctrl));
> > controlInfo_.emplace(ctrl.id, ctrl);
> >
> > - ctrls.emplace(controlIds_.back().get(), v4l2ControlInfo(ctrl));
> > + ctrls.emplace(controlIds_.back().get(), std::move(ctrlInfo));
> > }
> >
> > controls_ = std::move(ctrls);
> > @@ -630,6 +671,10 @@ void V4L2Device::updateControls(ControlList *ctrls,
> > value.set<int64_t>(v4l2Ctrl.value64);
> > break;
> >
> > + case ControlTypeInteger32:
> > + value.set<int32_t>(v4l2Ctrl.value);
> > + break;
> > +
> > case ControlTypeByte:
> > /*
> > * No action required, the VIDIOC_[GS]_EXT_CTRLS ioctl
> > --
> > 2.32.0.rc1.229.g3e70b5a671-goog
> >
More information about the libcamera-devel
mailing list