| Message ID | 20210607011402.55331-2-hiroh@chromium.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
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@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@jmondi.org> Thanks j > + */ > +ControlInfo V4L2Device::v4l2MenuControlInfo( > + const struct v4l2_query_ext_ctrl &ctrl) > +{ > + 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 >
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@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@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 > >
Hi Hiro, Thank you for the patch. 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@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 s/ctrl/control/ > + * \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. s/a V4L2 menu ctrl./the V4L2 menu control/ > + */ > +ControlInfo V4L2Device::v4l2MenuControlInfo( > + const struct v4l2_query_ext_ctrl &ctrl) I agree with Jacopo, I wouldn't break the line here. > +{ > + std::vector<ControlValue> indices; > + struct v4l2_querymenu menu = {}; > + menu.id = ctrl.id; > + > + if (ctrl.minimum < 0) > + return ControlInfo(); > + > + ControlValue def = {}; You can drop = {}, there's a default constructor. > + 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); Could this be return ControlInfo(indices, ControlValue(ctrl.default_value)); ? Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > +} > + > /* > * \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
Hi Jacopo, On Mon, Jun 07, 2021 at 04:46:53PM +0200, Jacopo Mondi wrote: > On Mon, Jun 07, 2021 at 04:16:01PM +0200, Jacopo Mondi wrote: > > 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@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@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 ? I've considered this too, I don't mind much either way. > ------------------------------------------------------------------------------- > 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
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. + */ +ControlInfo V4L2Device::v4l2MenuControlInfo( + const struct v4l2_query_ext_ctrl &ctrl) +{ + 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
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@chromium.org> --- include/libcamera/internal/v4l2_device.h | 2 + src/libcamera/v4l2_device.cpp | 53 ++++++++++++++++++++++-- 2 files changed, 51 insertions(+), 4 deletions(-)