[libcamera-devel] [RFC PATCH v2 2/5] libcamera: V4L2Device: Support v4l2 menu control
Hirokazu Honda
hiroh at chromium.org
Thu Apr 15 12:48:17 CEST 2021
HI Kieran, Thanks for the comment.
On Wed, Apr 14, 2021 at 5:20 PM Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> Hi Hiro,
>
> On 13/04/2021 08:50, 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 | 104 ++++++++++++++++++++-
> > 4 files changed, 112 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);
> > };
> >
> > } /* 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..7fbca8b1 100644
> > --- a/src/libcamera/v4l2_device.cpp
> > +++ b/src/libcamera/v4l2_device.cpp
> > @@ -10,6 +10,7 @@
> > #include <fcntl.h>
> > #include <iomanip>
> > #include <limits.h>
> > +#include <linux/intel-ipu3.h>
>
>
> The V4L2 device should not be IPU3 specific in any way.
>
>
> > #include <stdlib.h>
> > #include <string.h>
> > #include <sys/ioctl.h>
> > @@ -459,6 +460,60 @@ 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);
> > + bool isName = false;
> > + switch (ctrl.id) {
> > + case V4L2_CID_TEST_PATTERN:
> > + case V4L2_CID_INTEL_IPU3_MODE:
> > + isName = true;
> > + break;
>
> Which means processing like this can't be here.
>
> Is there a way to do this generically from the information you have in
> the v4l2_query_ext_ctrl ?
>
>
Okay, I think this should be
isName = ctrl.type == V4L2_CTRL_TYPE_MENU;
Then we removed this.
I would do it in the next patch series.
Thanks,
-Hiro
>
> > + case V4L2_CID_LINK_FREQ:
> > + isName = false;
> > + break;
> > + default:
> > + LOG(V4L2, Error) << "Unsupported menu: " << ctrl.name
> > + << " (" << utils::hex(ctrl.id) << ")";
> > + return false;
> > + }
> > +
> > + 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())));
> > +
> > + return true;
> > +}
> > +
> > /**
> > * \fn V4L2Device::fd()
> > * \brief Retrieve the V4L2 device file descriptor
> > @@ -474,7 +529,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 +539,20 @@ void V4L2Device::listControls()
> > ctrl.flags & V4L2_CTRL_FLAG_DISABLED)
> > continue;
> >
> > + V4L2ControlInfo v4l2CtrlInfo(ControlInfo(0));
> > 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 +562,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 +601,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: {
> > + 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
> --
> Kieran
More information about the libcamera-devel
mailing list