[libcamera-devel] [PATCH v2 3/7] libcamera: V4L2Device: Support v4l2 menu control
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Apr 27 05:46:11 CEST 2021
Hi Hiro,
Thank you for the patch.
On Wed, Apr 21, 2021 at 01:23:42PM +0900, Hirokazu Honda wrote:
> This adds a support of v4l2 menu.
>
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> ---
> include/libcamera/internal/v4l2_controls.h | 1 +
> include/libcamera/internal/v4l2_device.h | 3 +
> src/libcamera/v4l2_controls.cpp | 10 ++-
> src/libcamera/v4l2_device.cpp | 90 ++++++++++++++++++++--
> 4 files changed, 98 insertions(+), 6 deletions(-)
>
> diff --git a/include/libcamera/internal/v4l2_controls.h b/include/libcamera/internal/v4l2_controls.h
> index 0851b8dd..c42f2529 100644
> --- a/include/libcamera/internal/v4l2_controls.h
> +++ b/include/libcamera/internal/v4l2_controls.h
> @@ -24,6 +24,7 @@ class V4L2ControlInfo : public ControlInfo
> {
> public:
> V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl);
> + V4L2ControlInfo(const ControlInfo &ctrl);
The purpose of the V4L2ControlInfo class is to provide an easy way to
construct a ControlInfo from v4l2_query_ext_ctrl. It has no data member,
so it's really only a convenience constructor. If we need to make
copies, let's use the base ControlInfo class. That's what we store in
the ControlInfoMap.
> };
>
> } /* namespace libcamera */
> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h
> index 4cce3840..2436a83c 100644
> --- a/include/libcamera/internal/v4l2_device.h
> +++ b/include/libcamera/internal/v4l2_device.h
> @@ -53,6 +53,9 @@ protected:
> int fd() const { return fd_; }
>
> private:
> + bool createV4L2ControlInfoForMenu(const v4l2_query_ext_ctrl &ctrl,
> + V4L2ControlInfo &v4l2CtrlInfo);
> +
> void listControls();
> void updateControls(ControlList *ctrls,
> const std::vector<v4l2_ext_control> &v4l2Ctrls);
> diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp
> index 3f8ec6ca..2c965cfa 100644
> --- a/src/libcamera/v4l2_controls.cpp
> +++ b/src/libcamera/v4l2_controls.cpp
> @@ -73,9 +73,12 @@ ControlType v4l2_ctrl_type(const struct v4l2_query_ext_ctrl &ctrl)
> return ControlTypeInteger64;
>
> case V4L2_CTRL_TYPE_MENU:
> + case V4L2_CTRL_TYPE_INTEGER_MENU:
> + return ControlTypeMenu;
> +
> 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.
> @@ -148,4 +151,9 @@ V4L2ControlInfo::V4L2ControlInfo(const struct v4l2_query_ext_ctrl &ctrl)
> }
> }
>
> +V4L2ControlInfo::V4L2ControlInfo(const ControlInfo &ctrl)
> + : ControlInfo(ctrl)
> +{
> +}
> +
> } /* namespace libcamera */
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 8f29cd7f..585e527b 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -459,6 +459,47 @@ int V4L2Device::ioctl(unsigned long request, void *argp)
> * \return The device node path
> */
>
> +bool V4L2Device::createV4L2ControlInfoForMenu(const v4l2_query_ext_ctrl &ctrl,
> + V4L2ControlInfo &v4l2CtrlInfo)
> +{
> + ASSERT(ctrl.type == V4L2_CTRL_TYPE_MENU ||
> + ctrl.type == V4L2_CTRL_TYPE_INTEGER_MENU);
> + const bool isName = ctrl.type == V4L2_CTRL_TYPE_MENU;
> +
> + std::vector<ControlValue> values;
> + v4l2_querymenu menu;
> + memset(&menu, 0, sizeof(menu));
> + menu.id = ctrl.id;
> + for (menu.index = ctrl.minimum;
> + static_cast<int>(menu.index) <= ctrl.maximum; menu.index++) {
> + if (ioctl(VIDIOC_QUERYMENU, &menu) != 0)
> + continue;
> + ControlMenu ctrlMenu;
> + ctrlMenu.index = static_cast<int32_t>(menu.index);
> + ctrlMenu.isName = isName;
> + if (isName) {
> + strcpy(ctrlMenu.name,
> + reinterpret_cast<const char *>(menu.name));
> + } else {
> + ctrlMenu.value = menu.value;
> + }
> +
> + values.push_back(ControlValue(ctrlMenu));
> + }
> +
> + if (values.empty()) {
> + LOG(V4L2, Error)
> + << "No applicable value: " << utils::hex(ctrl.id);
> + return false;
> + }
> +
> + v4l2CtrlInfo = V4L2ControlInfo(
> + ControlInfo(libcamera::Span<const ControlValue>(
> + values.data(), values.size())));
There's no need to go through V4L2ControlInfo here, ControlInfo is all
you need.
> +
> + return true;
> +}
> +
> /**
> * \fn V4L2Device::fd()
> * \brief Retrieve the V4L2 device file descriptor
> @@ -474,7 +515,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;
> @@ -485,15 +525,20 @@ void V4L2Device::listControls()
> ctrl.flags & V4L2_CTRL_FLAG_DISABLED)
> continue;
>
> + V4L2ControlInfo v4l2CtrlInfo(ControlInfo(0));
That seems like a hack :-)
> 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:
> + v4l2CtrlInfo = V4L2ControlInfo(ctrl);
> + break;
> + case V4L2_CTRL_TYPE_MENU:
> + case V4L2_CTRL_TYPE_INTEGER_MENU:
> + if (!createV4L2ControlInfoForMenu(ctrl, v4l2CtrlInfo))
> + continue;
> break;
> /* \todo Support other control types. */
> default:
> @@ -503,10 +548,13 @@ void V4L2Device::listControls()
> continue;
> }
>
> + LOG(V4L2, Debug) << "Control: " << ctrl.name
> + << " (" << utils::hex(ctrl.id) << ")";
> +
> controlIds_.emplace_back(std::make_unique<V4L2ControlId>(ctrl));
> controlInfo_.emplace(ctrl.id, ctrl);
>
> - ctrls.emplace(controlIds_.back().get(), V4L2ControlInfo(ctrl));
> + ctrls.emplace(controlIds_.back().get(), std::move(v4l2CtrlInfo));
> }
>
> controls_ = std::move(ctrls);
> @@ -539,7 +587,39 @@ void V4L2Device::updateControls(ControlList *ctrls,
> case ControlTypeInteger64:
> newValue.set<int64_t>(v4l2Ctrl.value64);
> break;
> -
> + case ControlTypeMenu:
> + switch (v4l2Ctrl.id) {
> + case V4L2_CID_TEST_PATTERN: {
Let's keep specific control IDs out of this code, it should be
implemented generically for all menu controls.
> + ControlMenu menu;
> + menu.index = v4l2Ctrl.value;
> + menu.isName = true;
> +
> + bool found = false;
> + for (const ControlValue &validValue : it->second.values()) {
> + const auto &ctrlMenu = validValue.get<ControlMenu>();
> + if (!ctrlMenu.isName)
> + continue;
> + if (menu.index == ctrlMenu.index) {
> + strcpy(menu.name, ctrlMenu.name);
> + found = true;
> + break;
> + }
> + }
> +
> + if (!found) {
> + LOG(V4L2, Error)
> + << "Matched value is not found"
> + << ", index=" << menu.index;
> + continue;
> + }
> +
> + newValue.set<ControlMenu>(menu);
> + break;
> + }
> + default:
> + LOG(V4L2, Error) << "Unknown id: " << v4l2Ctrl.id;
> + break;
> + }
> case ControlTypeByte:
> /*
> * No action required, the VIDIOC_[GS]_EXT_CTRLS ioctl
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list