[{"id":17439,"web_url":"https://patchwork.libcamera.org/comment/17439/","msgid":"<20210607141558.du66wb4dubfvlbmm@uno.localdomain>","date":"2021-06-07T14:15:58","subject":"Re: [libcamera-devel] [PATCH v7 2/6] libcamera: V4L2Device: Support\n\tv4l2 menu control","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hiro,\n\nOn Mon, Jun 07, 2021 at 10:13:58AM +0900, Hirokazu Honda wrote:\n> This adds a support of v4l2 menu. The control info for v4l2 menu\n> contains indices without names and 64bit values of querymenu.\n>\n> Signed-off-by: Hirokazu Honda \n> ---\n> include/libcamera/internal/v4l2_device.h | 2 +\n> src/libcamera/v4l2_device.cpp | 53 ++++++++++++++++++++++--\n> 2 files changed, 51 insertions(+), 4 deletions(-)\n>\n> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> index b82e2a14..687be9b2 100644\n> --- a/include/libcamera/internal/v4l2_device.h\n> +++ b/include/libcamera/internal/v4l2_device.h\n> @@ -56,6 +56,8 @@ protected:\n> \tint fd() const { return fd_; }\n>\n> private:\n> +\tControlInfo v4l2MenuControlInfo(const v4l2_query_ext_ctrl &ctrl);\n> +\n> \tvoid listControls();\n> \tvoid updateControls(ControlList *ctrls,\n> \t\t\t Span v4l2Ctrls);\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index b39c6266..40e2c74c 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -524,6 +524,38 @@ int V4L2Device::ioctl(unsigned long request, void *argp)\n> * \\return The V4L2 device file descriptor, -1 if the device node is not open\n> */\n>\n> +/**\n> + * \\brief Create ControlInfo for a V4L2 menu ctrl\n> + * \\param[in] ctrl The v4l2_query_ext_ctrl that represents a menu\n> + *\n> + * The created ControlInfo contains indices acquired by VIDIOC_QUERYMENU.\n> + *\n> + * \\return ControlInfo for a V4L2 menu ctrl.\n\nnit: no full stop at the end\nCan be changed when applying!\n\nThanks for keep pushing this series\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n\n> + */\n> +ControlInfo V4L2Device::v4l2MenuControlInfo(\n> +\tconst struct v4l2_query_ext_ctrl &ctrl)\n> +{\n> +\tstd::vector indices;\n> +\tstruct v4l2_querymenu menu = {};\n> +\tmenu.id = ctrl.id;\n> +\n> +\tif (ctrl.minimum < 0)\n> +\t\treturn ControlInfo();\n> +\n> +\tControlValue def = {};\n> +\tfor (int32_t index = ctrl.minimum; index <= ctrl.maximum; ++index) {\n> +\t\tmenu.index = index;\n> +\t\tif (ioctl(VIDIOC_QUERYMENU, &menu) != 0)\n> +\t\t\tcontinue;\n> +\n> +\t\tindices.push_back(index);\n> +\t\tif (index == ctrl.default_value)\n> +\t\t\tdef = ControlValue(index);\n> +\t}\n> +\n> +\treturn ControlInfo(indices, def);\n> +}\n> +\n> /*\n> * \\brief List and store information about all controls supported by the\n> * V4L2 device\n> @@ -533,7 +565,6 @@ void V4L2Device::listControls()\n> \tControlInfoMap::Map ctrls;\n> \tstruct v4l2_query_ext_ctrl ctrl = {};\n>\n> -\t/* \\todo Add support for menu controls. */\n> \twhile (1) {\n> \t\tctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL |\n> \t\t\t V4L2_CTRL_FLAG_NEXT_COMPOUND;\n> @@ -544,16 +575,23 @@ void V4L2Device::listControls()\n> \t\t ctrl.flags & V4L2_CTRL_FLAG_DISABLED)\n> \t\t\tcontinue;\n>\n> +\t\tControlInfo ctrlInfo;\n> +\n> \t\tswitch (ctrl.type) {\n> \t\tcase V4L2_CTRL_TYPE_INTEGER:\n> \t\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> -\t\tcase V4L2_CTRL_TYPE_MENU:\n> \t\tcase V4L2_CTRL_TYPE_BUTTON:\n> \t\tcase V4L2_CTRL_TYPE_INTEGER64:\n> \t\tcase V4L2_CTRL_TYPE_BITMASK:\n> -\t\tcase V4L2_CTRL_TYPE_INTEGER_MENU:\n> \t\tcase V4L2_CTRL_TYPE_U8:\n> +\t\t\tctrlInfo = v4l2ControlInfo(ctrl);\n> +\t\t\tbreak;\n> +\n> +\t\tcase V4L2_CTRL_TYPE_INTEGER_MENU:\n> +\t\tcase V4L2_CTRL_TYPE_MENU:\n> +\t\t\tctrlInfo = v4l2MenuControlInfo(ctrl);\n> \t\t\tbreak;\n> +\n> \t\t/* \\todo Support other control types. */\n> \t\tdefault:\n> \t\t\tLOG(V4L2, Debug)\n> @@ -562,10 +600,13 @@ void V4L2Device::listControls()\n> \t\t\tcontinue;\n> \t\t}\n>\n> +\t\tLOG(V4L2, Debug) << \"Control: \" << ctrl.name\n> +\t\t\t\t << \" (\" << utils::hex(ctrl.id) << \")\";\n> +\n> \t\tcontrolIds_.emplace_back(v4l2ControlId(ctrl));\n> \t\tcontrolInfo_.emplace(ctrl.id, ctrl);\n>\n> -\t\tctrls.emplace(controlIds_.back().get(), v4l2ControlInfo(ctrl));\n> +\t\tctrls.emplace(controlIds_.back().get(), std::move(ctrlInfo));\n> \t}\n>\n> \tcontrols_ = std::move(ctrls);\n> @@ -630,6 +671,10 @@ void V4L2Device::updateControls(ControlList *ctrls,\n> \t\t\tvalue.set(v4l2Ctrl.value64);\n> \t\t\tbreak;\n>\n> +\t\tcase ControlTypeInteger32:\n> +\t\t\tvalue.set(v4l2Ctrl.value);\n> +\t\t\tbreak;\n> +\n> \t\tcase ControlTypeByte:\n> \t\t\t/*\n> \t\t\t * No action required, the VIDIOC_[GS]_EXT_CTRLS ioctl\n> --\n> 2.32.0.rc1.229.g3e70b5a671-goog\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id BA88DC320B\n\tfor ;\n\tMon, 7 Jun 2021 14:15:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CCA8668928;\n\tMon, 7 Jun 2021 16:15:10 +0200 (CEST)","from relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 66289602A7\n\tfor ;\n\tMon, 7 Jun 2021 16:15:10 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay12.mail.gandi.net (Postfix) with ESMTPSA id 781FB20000A;\n\tMon, 7 Jun 2021 14:15:09 +0000 (UTC)"],"Date":"Mon, 7 Jun 2021 16:15:58 +0200","From":"Jacopo Mondi ","To":"Hirokazu Honda ","Message-ID":"<20210607141558.du66wb4dubfvlbmm@uno.localdomain>","References":"<20210607011402.55331-1-hiroh@chromium.org>\n\t<20210607011402.55331-2-hiroh@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210607011402.55331-2-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [PATCH v7 2/6] libcamera: V4L2Device: Support\n\tv4l2 menu control","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17441,"web_url":"https://patchwork.libcamera.org/comment/17441/","msgid":"<20210607144653.53ertd5kjl6cible@uno.localdomain>","date":"2021-06-07T14:46:53","subject":"Re: [libcamera-devel] [PATCH v7 2/6] libcamera: V4L2Device: Support\n\tv4l2 menu control","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hiro,\n\nOn Mon, Jun 07, 2021 at 04:16:01PM +0200, Jacopo Mondi wrote:\n> Hi Hiro,\n>\n> On Mon, Jun 07, 2021 at 10:13:58AM +0900, Hirokazu Honda wrote:\n> > This adds a support of v4l2 menu. The control info for v4l2 menu\n> > contains indices without names and 64bit values of querymenu.\n> >\n> > Signed-off-by: Hirokazu Honda \n> > ---\n> > include/libcamera/internal/v4l2_device.h | 2 +\n> > src/libcamera/v4l2_device.cpp | 53 ++++++++++++++++++++++--\n> > 2 files changed, 51 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> > index b82e2a14..687be9b2 100644\n> > --- a/include/libcamera/internal/v4l2_device.h\n> > +++ b/include/libcamera/internal/v4l2_device.h\n> > @@ -56,6 +56,8 @@ protected:\n> > \tint fd() const { return fd_; }\n> >\n> > private:\n> > +\tControlInfo v4l2MenuControlInfo(const v4l2_query_ext_ctrl &ctrl);\n> > +\n> > \tvoid listControls();\n> > \tvoid updateControls(ControlList *ctrls,\n> > \t\t\t Span v4l2Ctrls);\n> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > index b39c6266..40e2c74c 100644\n> > --- a/src/libcamera/v4l2_device.cpp\n> > +++ b/src/libcamera/v4l2_device.cpp\n> > @@ -524,6 +524,38 @@ int V4L2Device::ioctl(unsigned long request, void *argp)\n> > * \\return The V4L2 device file descriptor, -1 if the device node is not open\n> > */\n> >\n> > +/**\n> > + * \\brief Create ControlInfo for a V4L2 menu ctrl\n> > + * \\param[in] ctrl The v4l2_query_ext_ctrl that represents a menu\n> > + *\n> > + * The created ControlInfo contains indices acquired by VIDIOC_QUERYMENU.\n> > + *\n> > + * \\return ControlInfo for a V4L2 menu ctrl.\n>\n> nit: no full stop at the end\n> Can be changed when applying!\n>\n> Thanks for keep pushing this series\n> Reviewed-by: Jacopo Mondi \n>\n> Thanks\n> j\n>\n>\n> > + */\n> > +ControlInfo V4L2Device::v4l2MenuControlInfo(\n> > +\tconst struct v4l2_query_ext_ctrl &ctrl)\n\nYou know, it looks better on a single line even if it's 84 cols (we\nhave an hard limit at 120 cols, soft one at 80)\n\nI just noticed a little issue, with this patch:\nThis new function is defined as a class member, while the other\nv4l2-ctrl related function are defined in an anonymous namespace.\n\nWould you consider this patch on top to make all functions class\nmembers ?\n\n-------------------------------------------------------------------------------\ndiff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\nindex 687be9b25273..5a8b99e067ab 100644\n--- a/include/libcamera/internal/v4l2_device.h\n+++ b/include/libcamera/internal/v4l2_device.h\n@@ -56,6 +56,9 @@ protected:\n \tint fd() const { return fd_; }\n\n private:\n+\tControlType v4l2CtrlType(uint32_t ctrlType);\n+\tstd::unique_ptr v4l2ControlId(const v4l2_query_ext_ctrl &ctrl);\n+\tControlInfo v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl);\n \tControlInfo v4l2MenuControlInfo(const v4l2_query_ext_ctrl &ctrl);\n\n \tvoid listControls();\ndiff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\nindex 40e2c74c401a..452cc9b5e430 100644\n--- a/src/libcamera/v4l2_device.cpp\n+++ b/src/libcamera/v4l2_device.cpp\n@@ -30,73 +30,6 @@ namespace libcamera {\n\n LOG_DEFINE_CATEGORY(V4L2)\n\n-namespace {\n-\n-ControlType v4l2CtrlType(uint32_t ctrlType)\n-{\n-\tswitch (ctrlType) {\n-\tcase V4L2_CTRL_TYPE_U8:\n-\t\treturn ControlTypeByte;\n-\n-\tcase V4L2_CTRL_TYPE_BOOLEAN:\n-\t\treturn ControlTypeBool;\n-\n-\tcase V4L2_CTRL_TYPE_INTEGER:\n-\t\treturn ControlTypeInteger32;\n-\n-\tcase V4L2_CTRL_TYPE_INTEGER64:\n-\t\treturn ControlTypeInteger64;\n-\n-\tcase V4L2_CTRL_TYPE_MENU:\n-\tcase V4L2_CTRL_TYPE_BUTTON:\n-\tcase V4L2_CTRL_TYPE_BITMASK:\n-\tcase V4L2_CTRL_TYPE_INTEGER_MENU:\n-\t\t/*\n-\t\t * More precise types may be needed, for now use a 32-bit\n-\t\t * integer type.\n-\t\t */\n-\t\treturn ControlTypeInteger32;\n-\n-\tdefault:\n-\t\treturn ControlTypeNone;\n-\t}\n-}\n-\n-std::unique_ptr v4l2ControlId(const v4l2_query_ext_ctrl &ctrl)\n-{\n-\tconst size_t len = strnlen(ctrl.name, sizeof(ctrl.name));\n-\tconst std::string name(static_cast(ctrl.name), len);\n-\tconst ControlType type = v4l2CtrlType(ctrl.type);\n-\n-\treturn std::make_unique(ctrl.id, name, type);\n-}\n-\n-ControlInfo v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl)\n-{\n-\tswitch (ctrl.type) {\n-\tcase V4L2_CTRL_TYPE_U8:\n-\t\treturn ControlInfo(static_cast(ctrl.minimum),\n-\t\t\t\t static_cast(ctrl.maximum),\n-\t\t\t\t static_cast(ctrl.default_value));\n-\n-\tcase V4L2_CTRL_TYPE_BOOLEAN:\n-\t\treturn ControlInfo(static_cast(ctrl.minimum),\n-\t\t\t\t static_cast(ctrl.maximum),\n-\t\t\t\t static_cast(ctrl.default_value));\n-\n-\tcase V4L2_CTRL_TYPE_INTEGER64:\n-\t\treturn ControlInfo(static_cast(ctrl.minimum),\n-\t\t\t\t static_cast(ctrl.maximum),\n-\t\t\t\t static_cast(ctrl.default_value));\n-\n-\tdefault:\n-\t\treturn ControlInfo(static_cast(ctrl.minimum),\n-\t\t\t\t static_cast(ctrl.maximum),\n-\t\t\t\t static_cast(ctrl.default_value));\n-\t}\n-}\n-} /* namespace */\n-\n /**\n * \\class V4L2Device\n * \\brief Base class for V4L2VideoDevice and V4L2Subdevice\n@@ -524,6 +457,77 @@ int V4L2Device::ioctl(unsigned long request, void *argp)\n * \\return The V4L2 device file descriptor, -1 if the device node is not open\n */\n\n+ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType)\n+{\n+\tswitch (ctrlType) {\n+\tcase V4L2_CTRL_TYPE_U8:\n+\t\treturn ControlTypeByte;\n+\n+\tcase V4L2_CTRL_TYPE_BOOLEAN:\n+\t\treturn ControlTypeBool;\n+\n+\tcase V4L2_CTRL_TYPE_INTEGER:\n+\t\treturn ControlTypeInteger32;\n+\n+\tcase V4L2_CTRL_TYPE_INTEGER64:\n+\t\treturn ControlTypeInteger64;\n+\n+\tcase V4L2_CTRL_TYPE_MENU:\n+\tcase V4L2_CTRL_TYPE_BUTTON:\n+\tcase V4L2_CTRL_TYPE_BITMASK:\n+\tcase V4L2_CTRL_TYPE_INTEGER_MENU:\n+\t\t/*\n+\t\t * More precise types may be needed, for now use a 32-bit\n+\t\t * integer type.\n+\t\t */\n+\t\treturn ControlTypeInteger32;\n+\n+\tdefault:\n+\t\treturn ControlTypeNone;\n+\t}\n+}\n+\n+std::unique_ptr V4L2Device::v4l2ControlId(const v4l2_query_ext_ctrl &ctrl)\n+{\n+\tconst size_t len = strnlen(ctrl.name, sizeof(ctrl.name));\n+\tconst std::string name(static_cast(ctrl.name), len);\n+\tconst ControlType type = v4l2CtrlType(ctrl.type);\n+\n+\treturn std::make_unique(ctrl.id, name, type);\n+}\n+\n+/**\n+ * \\brief Create ControlInfo for a V4L2 ctrl\n+ * \\param[in] ctrl The v4l2_query_ext_ctrl that represents a control\n+ *\n+ * \\return ControlInfo for a V4L2 ctrl\n+ */\n+ControlInfo V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl)\n+{\n+\tswitch (ctrl.type) {\n+\tcase V4L2_CTRL_TYPE_U8:\n+\t\treturn ControlInfo(static_cast(ctrl.minimum),\n+\t\t\t\t static_cast(ctrl.maximum),\n+\t\t\t\t static_cast(ctrl.default_value));\n+\n+\tcase V4L2_CTRL_TYPE_BOOLEAN:\n+\t\treturn ControlInfo(static_cast(ctrl.minimum),\n+\t\t\t\t static_cast(ctrl.maximum),\n+\t\t\t\t static_cast(ctrl.default_value));\n+\n+\tcase V4L2_CTRL_TYPE_INTEGER64:\n+\t\treturn ControlInfo(static_cast(ctrl.minimum),\n+\t\t\t\t static_cast(ctrl.maximum),\n+\t\t\t\t static_cast(ctrl.default_value));\n+\n+\tdefault:\n+\t\treturn ControlInfo(static_cast(ctrl.minimum),\n+\t\t\t\t static_cast(ctrl.maximum),\n+\t\t\t\t static_cast(ctrl.default_value));\n+\t}\n+}\n+\n+\n /**\n * \\brief Create ControlInfo for a V4L2 menu ctrl\n * \\param[in] ctrl The v4l2_query_ext_ctrl that represents a menu\n\n-------------------------------------------------------------------------------\n> > +{\n> > +\tstd::vector indices;\n> > +\tstruct v4l2_querymenu menu = {};\n> > +\tmenu.id = ctrl.id;\n> > +\n> > +\tif (ctrl.minimum < 0)\n> > +\t\treturn ControlInfo();\n> > +\n> > +\tControlValue def = {};\n> > +\tfor (int32_t index = ctrl.minimum; index <= ctrl.maximum; ++index) {\n> > +\t\tmenu.index = index;\n> > +\t\tif (ioctl(VIDIOC_QUERYMENU, &menu) != 0)\n> > +\t\t\tcontinue;\n> > +\n> > +\t\tindices.push_back(index);\n> > +\t\tif (index == ctrl.default_value)\n> > +\t\t\tdef = ControlValue(index);\n> > +\t}\n> > +\n> > +\treturn ControlInfo(indices, def);\n> > +}\n> > +\n> > /*\n> > * \\brief List and store information about all controls supported by the\n> > * V4L2 device\n> > @@ -533,7 +565,6 @@ void V4L2Device::listControls()\n> > \tControlInfoMap::Map ctrls;\n> > \tstruct v4l2_query_ext_ctrl ctrl = {};\n> >\n> > -\t/* \\todo Add support for menu controls. */\n> > \twhile (1) {\n> > \t\tctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL |\n> > \t\t\t V4L2_CTRL_FLAG_NEXT_COMPOUND;\n> > @@ -544,16 +575,23 @@ void V4L2Device::listControls()\n> > \t\t ctrl.flags & V4L2_CTRL_FLAG_DISABLED)\n> > \t\t\tcontinue;\n> >\n> > +\t\tControlInfo ctrlInfo;\n> > +\n> > \t\tswitch (ctrl.type) {\n> > \t\tcase V4L2_CTRL_TYPE_INTEGER:\n> > \t\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> > -\t\tcase V4L2_CTRL_TYPE_MENU:\n> > \t\tcase V4L2_CTRL_TYPE_BUTTON:\n> > \t\tcase V4L2_CTRL_TYPE_INTEGER64:\n> > \t\tcase V4L2_CTRL_TYPE_BITMASK:\n> > -\t\tcase V4L2_CTRL_TYPE_INTEGER_MENU:\n> > \t\tcase V4L2_CTRL_TYPE_U8:\n> > +\t\t\tctrlInfo = v4l2ControlInfo(ctrl);\n> > +\t\t\tbreak;\n> > +\n> > +\t\tcase V4L2_CTRL_TYPE_INTEGER_MENU:\n> > +\t\tcase V4L2_CTRL_TYPE_MENU:\n> > +\t\t\tctrlInfo = v4l2MenuControlInfo(ctrl);\n> > \t\t\tbreak;\n> > +\n> > \t\t/* \\todo Support other control types. */\n> > \t\tdefault:\n> > \t\t\tLOG(V4L2, Debug)\n> > @@ -562,10 +600,13 @@ void V4L2Device::listControls()\n> > \t\t\tcontinue;\n> > \t\t}\n> >\n> > +\t\tLOG(V4L2, Debug) << \"Control: \" << ctrl.name\n> > +\t\t\t\t << \" (\" << utils::hex(ctrl.id) << \")\";\n> > +\n> > \t\tcontrolIds_.emplace_back(v4l2ControlId(ctrl));\n> > \t\tcontrolInfo_.emplace(ctrl.id, ctrl);\n> >\n> > -\t\tctrls.emplace(controlIds_.back().get(), v4l2ControlInfo(ctrl));\n> > +\t\tctrls.emplace(controlIds_.back().get(), std::move(ctrlInfo));\n> > \t}\n> >\n> > \tcontrols_ = std::move(ctrls);\n> > @@ -630,6 +671,10 @@ void V4L2Device::updateControls(ControlList *ctrls,\n> > \t\t\tvalue.set(v4l2Ctrl.value64);\n> > \t\t\tbreak;\n> >\n> > +\t\tcase ControlTypeInteger32:\n> > +\t\t\tvalue.set(v4l2Ctrl.value);\n> > +\t\t\tbreak;\n> > +\n> > \t\tcase ControlTypeByte:\n> > \t\t\t/*\n> > \t\t\t * No action required, the VIDIOC_[GS]_EXT_CTRLS ioctl\n> > --\n> > 2.32.0.rc1.229.g3e70b5a671-goog\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 51A31C320B\n\tfor ;\n\tMon, 7 Jun 2021 14:46:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9070868928;\n\tMon, 7 Jun 2021 16:46:06 +0200 (CEST)","from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F4140602A7\n\tfor ;\n\tMon, 7 Jun 2021 16:46:04 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 2B348E0009;\n\tMon, 7 Jun 2021 14:46:03 +0000 (UTC)"],"Date":"Mon, 7 Jun 2021 16:46:53 +0200","From":"Jacopo Mondi ","To":"Hirokazu Honda ","Message-ID":"<20210607144653.53ertd5kjl6cible@uno.localdomain>","References":"<20210607011402.55331-1-hiroh@chromium.org>\n\t<20210607011402.55331-2-hiroh@chromium.org>\n\t<20210607141558.du66wb4dubfvlbmm@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210607141558.du66wb4dubfvlbmm@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v7 2/6] libcamera: V4L2Device: Support\n\tv4l2 menu control","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17449,"web_url":"https://patchwork.libcamera.org/comment/17449/","msgid":"","date":"2021-06-07T21:38:56","subject":"Re: [libcamera-devel] [PATCH v7 2/6] libcamera: V4L2Device: Support\n\tv4l2 menu control","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hiro,\n\nThank you for the patch.\n\nOn Mon, Jun 07, 2021 at 10:13:58AM +0900, Hirokazu Honda wrote:\n> This adds a support of v4l2 menu. The control info for v4l2 menu\n> contains indices without names and 64bit values of querymenu.\n> \n> Signed-off-by: Hirokazu Honda \n> ---\n> include/libcamera/internal/v4l2_device.h | 2 +\n> src/libcamera/v4l2_device.cpp | 53 ++++++++++++++++++++++--\n> 2 files changed, 51 insertions(+), 4 deletions(-)\n> \n> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> index b82e2a14..687be9b2 100644\n> --- a/include/libcamera/internal/v4l2_device.h\n> +++ b/include/libcamera/internal/v4l2_device.h\n> @@ -56,6 +56,8 @@ protected:\n> \tint fd() const { return fd_; }\n> \n> private:\n> +\tControlInfo v4l2MenuControlInfo(const v4l2_query_ext_ctrl &ctrl);\n> +\n> \tvoid listControls();\n> \tvoid updateControls(ControlList *ctrls,\n> \t\t\t Span v4l2Ctrls);\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index b39c6266..40e2c74c 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -524,6 +524,38 @@ int V4L2Device::ioctl(unsigned long request, void *argp)\n> * \\return The V4L2 device file descriptor, -1 if the device node is not open\n> */\n> \n> +/**\n> + * \\brief Create ControlInfo for a V4L2 menu ctrl\n\ns/ctrl/control/\n\n> + * \\param[in] ctrl The v4l2_query_ext_ctrl that represents a menu\n> + *\n> + * The created ControlInfo contains indices acquired by VIDIOC_QUERYMENU.\n> + *\n> + * \\return ControlInfo for a V4L2 menu ctrl.\n\ns/a V4L2 menu ctrl./the V4L2 menu control/\n\n> + */\n> +ControlInfo V4L2Device::v4l2MenuControlInfo(\n> +\tconst struct v4l2_query_ext_ctrl &ctrl)\n\nI agree with Jacopo, I wouldn't break the line here.\n\n> +{\n> +\tstd::vector indices;\n> +\tstruct v4l2_querymenu menu = {};\n> +\tmenu.id = ctrl.id;\n> +\n> +\tif (ctrl.minimum < 0)\n> +\t\treturn ControlInfo();\n> +\n> +\tControlValue def = {};\n\nYou can drop = {}, there's a default constructor.\n\n> +\tfor (int32_t index = ctrl.minimum; index <= ctrl.maximum; ++index) {\n> +\t\tmenu.index = index;\n> +\t\tif (ioctl(VIDIOC_QUERYMENU, &menu) != 0)\n> +\t\t\tcontinue;\n> +\n> +\t\tindices.push_back(index);\n> +\t\tif (index == ctrl.default_value)\n> +\t\t\tdef = ControlValue(index);\n> +\t}\n> +\n> +\treturn ControlInfo(indices, def);\n\nCould this be\n\n\treturn ControlInfo(indices, ControlValue(ctrl.default_value));\n\n?\n\nReviewed-by: Laurent Pinchart \n\n> +}\n> +\n> /*\n> * \\brief List and store information about all controls supported by the\n> * V4L2 device\n> @@ -533,7 +565,6 @@ void V4L2Device::listControls()\n> \tControlInfoMap::Map ctrls;\n> \tstruct v4l2_query_ext_ctrl ctrl = {};\n> \n> -\t/* \\todo Add support for menu controls. */\n> \twhile (1) {\n> \t\tctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL |\n> \t\t\t V4L2_CTRL_FLAG_NEXT_COMPOUND;\n> @@ -544,16 +575,23 @@ void V4L2Device::listControls()\n> \t\t ctrl.flags & V4L2_CTRL_FLAG_DISABLED)\n> \t\t\tcontinue;\n> \n> +\t\tControlInfo ctrlInfo;\n> +\n> \t\tswitch (ctrl.type) {\n> \t\tcase V4L2_CTRL_TYPE_INTEGER:\n> \t\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> -\t\tcase V4L2_CTRL_TYPE_MENU:\n> \t\tcase V4L2_CTRL_TYPE_BUTTON:\n> \t\tcase V4L2_CTRL_TYPE_INTEGER64:\n> \t\tcase V4L2_CTRL_TYPE_BITMASK:\n> -\t\tcase V4L2_CTRL_TYPE_INTEGER_MENU:\n> \t\tcase V4L2_CTRL_TYPE_U8:\n> +\t\t\tctrlInfo = v4l2ControlInfo(ctrl);\n> +\t\t\tbreak;\n> +\n> +\t\tcase V4L2_CTRL_TYPE_INTEGER_MENU:\n> +\t\tcase V4L2_CTRL_TYPE_MENU:\n> +\t\t\tctrlInfo = v4l2MenuControlInfo(ctrl);\n> \t\t\tbreak;\n> +\n> \t\t/* \\todo Support other control types. */\n> \t\tdefault:\n> \t\t\tLOG(V4L2, Debug)\n> @@ -562,10 +600,13 @@ void V4L2Device::listControls()\n> \t\t\tcontinue;\n> \t\t}\n> \n> +\t\tLOG(V4L2, Debug) << \"Control: \" << ctrl.name\n> +\t\t\t\t << \" (\" << utils::hex(ctrl.id) << \")\";\n> +\n> \t\tcontrolIds_.emplace_back(v4l2ControlId(ctrl));\n> \t\tcontrolInfo_.emplace(ctrl.id, ctrl);\n> \n> -\t\tctrls.emplace(controlIds_.back().get(), v4l2ControlInfo(ctrl));\n> +\t\tctrls.emplace(controlIds_.back().get(), std::move(ctrlInfo));\n> \t}\n> \n> \tcontrols_ = std::move(ctrls);\n> @@ -630,6 +671,10 @@ void V4L2Device::updateControls(ControlList *ctrls,\n> \t\t\tvalue.set(v4l2Ctrl.value64);\n> \t\t\tbreak;\n> \n> +\t\tcase ControlTypeInteger32:\n> +\t\t\tvalue.set(v4l2Ctrl.value);\n> +\t\t\tbreak;\n> +\n> \t\tcase ControlTypeByte:\n> \t\t\t/*\n> \t\t\t * No action required, the VIDIOC_[GS]_EXT_CTRLS ioctl","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 09B06BD22E\n\tfor ;\n\tMon, 7 Jun 2021 21:39:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 322CE6892F;\n\tMon, 7 Jun 2021 23:39:12 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E286A6891F\n\tfor ;\n\tMon, 7 Jun 2021 23:39:10 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 55FBE8DB;\n\tMon, 7 Jun 2021 23:39:10 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"oVbfXobl\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1623101950;\n\tbh=qiPVsvBY27L8wZZhd77W7chh9gco6Jgu8YhXwFqM4AU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=oVbfXoblHDmZvOLS4gc58cs3ptMeBLhCaazMtaxKBDcHUd5MkCnXq8lf9QoQnelZQ\n\t8Wj2gJAF3kybUk/xDNtGrmYQfmrwVsNfm/rF1bqLTVjwUajvz9kBjqJl5Im3x4kAzD\n\tStuuJ5D3Rh4eH1spnPjOIXVTF5Qz5ViQEhPheLwI=","Date":"Tue, 8 Jun 2021 00:38:56 +0300","From":"Laurent Pinchart ","To":"Hirokazu Honda ","Message-ID":"","References":"<20210607011402.55331-1-hiroh@chromium.org>\n\t<20210607011402.55331-2-hiroh@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210607011402.55331-2-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [PATCH v7 2/6] libcamera: V4L2Device: Support\n\tv4l2 menu control","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17450,"web_url":"https://patchwork.libcamera.org/comment/17450/","msgid":"","date":"2021-06-07T21:39:33","subject":"Re: [libcamera-devel] [PATCH v7 2/6] libcamera: V4L2Device: Support\n\tv4l2 menu control","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Jun 07, 2021 at 04:46:53PM +0200, Jacopo Mondi wrote:\n> On Mon, Jun 07, 2021 at 04:16:01PM +0200, Jacopo Mondi wrote:\n> > On Mon, Jun 07, 2021 at 10:13:58AM +0900, Hirokazu Honda wrote:\n> > > This adds a support of v4l2 menu. The control info for v4l2 menu\n> > > contains indices without names and 64bit values of querymenu.\n> > >\n> > > Signed-off-by: Hirokazu Honda \n> > > ---\n> > > include/libcamera/internal/v4l2_device.h | 2 +\n> > > src/libcamera/v4l2_device.cpp | 53 ++++++++++++++++++++++--\n> > > 2 files changed, 51 insertions(+), 4 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> > > index b82e2a14..687be9b2 100644\n> > > --- a/include/libcamera/internal/v4l2_device.h\n> > > +++ b/include/libcamera/internal/v4l2_device.h\n> > > @@ -56,6 +56,8 @@ protected:\n> > > \tint fd() const { return fd_; }\n> > >\n> > > private:\n> > > +\tControlInfo v4l2MenuControlInfo(const v4l2_query_ext_ctrl &ctrl);\n> > > +\n> > > \tvoid listControls();\n> > > \tvoid updateControls(ControlList *ctrls,\n> > > \t\t\t Span v4l2Ctrls);\n> > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > > index b39c6266..40e2c74c 100644\n> > > --- a/src/libcamera/v4l2_device.cpp\n> > > +++ b/src/libcamera/v4l2_device.cpp\n> > > @@ -524,6 +524,38 @@ int V4L2Device::ioctl(unsigned long request, void *argp)\n> > > * \\return The V4L2 device file descriptor, -1 if the device node is not open\n> > > */\n> > >\n> > > +/**\n> > > + * \\brief Create ControlInfo for a V4L2 menu ctrl\n> > > + * \\param[in] ctrl The v4l2_query_ext_ctrl that represents a menu\n> > > + *\n> > > + * The created ControlInfo contains indices acquired by VIDIOC_QUERYMENU.\n> > > + *\n> > > + * \\return ControlInfo for a V4L2 menu ctrl.\n> >\n> > nit: no full stop at the end\n> > Can be changed when applying!\n> >\n> > Thanks for keep pushing this series\n> > Reviewed-by: Jacopo Mondi \n> >\n> > Thanks\n> > j\n> >\n> >\n> > > + */\n> > > +ControlInfo V4L2Device::v4l2MenuControlInfo(\n> > > +\tconst struct v4l2_query_ext_ctrl &ctrl)\n> \n> You know, it looks better on a single line even if it's 84 cols (we\n> have an hard limit at 120 cols, soft one at 80)\n> \n> I just noticed a little issue, with this patch:\n> This new function is defined as a class member, while the other\n> v4l2-ctrl related function are defined in an anonymous namespace.\n> \n> Would you consider this patch on top to make all functions class\n> members ?\n\nI've considered this too, I don't mind much either way.\n\n> -------------------------------------------------------------------------------\n> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> index 687be9b25273..5a8b99e067ab 100644\n> --- a/include/libcamera/internal/v4l2_device.h\n> +++ b/include/libcamera/internal/v4l2_device.h\n> @@ -56,6 +56,9 @@ protected:\n> \tint fd() const { return fd_; }\n> \n> private:\n> +\tControlType v4l2CtrlType(uint32_t ctrlType);\n> +\tstd::unique_ptr v4l2ControlId(const v4l2_query_ext_ctrl &ctrl);\n> +\tControlInfo v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl);\n> \tControlInfo v4l2MenuControlInfo(const v4l2_query_ext_ctrl &ctrl);\n> \n> \tvoid listControls();\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 40e2c74c401a..452cc9b5e430 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -30,73 +30,6 @@ namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(V4L2)\n> \n> -namespace {\n> -\n> -ControlType v4l2CtrlType(uint32_t ctrlType)\n> -{\n> -\tswitch (ctrlType) {\n> -\tcase V4L2_CTRL_TYPE_U8:\n> -\t\treturn ControlTypeByte;\n> -\n> -\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> -\t\treturn ControlTypeBool;\n> -\n> -\tcase V4L2_CTRL_TYPE_INTEGER:\n> -\t\treturn ControlTypeInteger32;\n> -\n> -\tcase V4L2_CTRL_TYPE_INTEGER64:\n> -\t\treturn ControlTypeInteger64;\n> -\n> -\tcase V4L2_CTRL_TYPE_MENU:\n> -\tcase V4L2_CTRL_TYPE_BUTTON:\n> -\tcase V4L2_CTRL_TYPE_BITMASK:\n> -\tcase V4L2_CTRL_TYPE_INTEGER_MENU:\n> -\t\t/*\n> -\t\t * More precise types may be needed, for now use a 32-bit\n> -\t\t * integer type.\n> -\t\t */\n> -\t\treturn ControlTypeInteger32;\n> -\n> -\tdefault:\n> -\t\treturn ControlTypeNone;\n> -\t}\n> -}\n> -\n> -std::unique_ptr v4l2ControlId(const v4l2_query_ext_ctrl &ctrl)\n> -{\n> -\tconst size_t len = strnlen(ctrl.name, sizeof(ctrl.name));\n> -\tconst std::string name(static_cast(ctrl.name), len);\n> -\tconst ControlType type = v4l2CtrlType(ctrl.type);\n> -\n> -\treturn std::make_unique(ctrl.id, name, type);\n> -}\n> -\n> -ControlInfo v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl)\n> -{\n> -\tswitch (ctrl.type) {\n> -\tcase V4L2_CTRL_TYPE_U8:\n> -\t\treturn ControlInfo(static_cast(ctrl.minimum),\n> -\t\t\t\t static_cast(ctrl.maximum),\n> -\t\t\t\t static_cast(ctrl.default_value));\n> -\n> -\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> -\t\treturn ControlInfo(static_cast(ctrl.minimum),\n> -\t\t\t\t static_cast(ctrl.maximum),\n> -\t\t\t\t static_cast(ctrl.default_value));\n> -\n> -\tcase V4L2_CTRL_TYPE_INTEGER64:\n> -\t\treturn ControlInfo(static_cast(ctrl.minimum),\n> -\t\t\t\t static_cast(ctrl.maximum),\n> -\t\t\t\t static_cast(ctrl.default_value));\n> -\n> -\tdefault:\n> -\t\treturn ControlInfo(static_cast(ctrl.minimum),\n> -\t\t\t\t static_cast(ctrl.maximum),\n> -\t\t\t\t static_cast(ctrl.default_value));\n> -\t}\n> -}\n> -} /* namespace */\n> -\n> /**\n> * \\class V4L2Device\n> * \\brief Base class for V4L2VideoDevice and V4L2Subdevice\n> @@ -524,6 +457,77 @@ int V4L2Device::ioctl(unsigned long request, void *argp)\n> * \\return The V4L2 device file descriptor, -1 if the device node is not open\n> */\n> \n> +ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType)\n> +{\n> +\tswitch (ctrlType) {\n> +\tcase V4L2_CTRL_TYPE_U8:\n> +\t\treturn ControlTypeByte;\n> +\n> +\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> +\t\treturn ControlTypeBool;\n> +\n> +\tcase V4L2_CTRL_TYPE_INTEGER:\n> +\t\treturn ControlTypeInteger32;\n> +\n> +\tcase V4L2_CTRL_TYPE_INTEGER64:\n> +\t\treturn ControlTypeInteger64;\n> +\n> +\tcase V4L2_CTRL_TYPE_MENU:\n> +\tcase V4L2_CTRL_TYPE_BUTTON:\n> +\tcase V4L2_CTRL_TYPE_BITMASK:\n> +\tcase V4L2_CTRL_TYPE_INTEGER_MENU:\n> +\t\t/*\n> +\t\t * More precise types may be needed, for now use a 32-bit\n> +\t\t * integer type.\n> +\t\t */\n> +\t\treturn ControlTypeInteger32;\n> +\n> +\tdefault:\n> +\t\treturn ControlTypeNone;\n> +\t}\n> +}\n> +\n> +std::unique_ptr V4L2Device::v4l2ControlId(const v4l2_query_ext_ctrl &ctrl)\n> +{\n> +\tconst size_t len = strnlen(ctrl.name, sizeof(ctrl.name));\n> +\tconst std::string name(static_cast(ctrl.name), len);\n> +\tconst ControlType type = v4l2CtrlType(ctrl.type);\n> +\n> +\treturn std::make_unique(ctrl.id, name, type);\n> +}\n> +\n> +/**\n> + * \\brief Create ControlInfo for a V4L2 ctrl\n> + * \\param[in] ctrl The v4l2_query_ext_ctrl that represents a control\n> + *\n> + * \\return ControlInfo for a V4L2 ctrl\n> + */\n> +ControlInfo V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl)\n> +{\n> +\tswitch (ctrl.type) {\n> +\tcase V4L2_CTRL_TYPE_U8:\n> +\t\treturn ControlInfo(static_cast(ctrl.minimum),\n> +\t\t\t\t static_cast(ctrl.maximum),\n> +\t\t\t\t static_cast(ctrl.default_value));\n> +\n> +\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> +\t\treturn ControlInfo(static_cast(ctrl.minimum),\n> +\t\t\t\t static_cast(ctrl.maximum),\n> +\t\t\t\t static_cast(ctrl.default_value));\n> +\n> +\tcase V4L2_CTRL_TYPE_INTEGER64:\n> +\t\treturn ControlInfo(static_cast(ctrl.minimum),\n> +\t\t\t\t static_cast(ctrl.maximum),\n> +\t\t\t\t static_cast(ctrl.default_value));\n> +\n> +\tdefault:\n> +\t\treturn ControlInfo(static_cast(ctrl.minimum),\n> +\t\t\t\t static_cast(ctrl.maximum),\n> +\t\t\t\t static_cast(ctrl.default_value));\n> +\t}\n> +}\n> +\n> +\n> /**\n> * \\brief Create ControlInfo for a V4L2 menu ctrl\n> * \\param[in] ctrl The v4l2_query_ext_ctrl that represents a menu\n> \n> -------------------------------------------------------------------------------\n> > > +{\n> > > +\tstd::vector indices;\n> > > +\tstruct v4l2_querymenu menu = {};\n> > > +\tmenu.id = ctrl.id;\n> > > +\n> > > +\tif (ctrl.minimum < 0)\n> > > +\t\treturn ControlInfo();\n> > > +\n> > > +\tControlValue def = {};\n> > > +\tfor (int32_t index = ctrl.minimum; index <= ctrl.maximum; ++index) {\n> > > +\t\tmenu.index = index;\n> > > +\t\tif (ioctl(VIDIOC_QUERYMENU, &menu) != 0)\n> > > +\t\t\tcontinue;\n> > > +\n> > > +\t\tindices.push_back(index);\n> > > +\t\tif (index == ctrl.default_value)\n> > > +\t\t\tdef = ControlValue(index);\n> > > +\t}\n> > > +\n> > > +\treturn ControlInfo(indices, def);\n> > > +}\n> > > +\n> > > /*\n> > > * \\brief List and store information about all controls supported by the\n> > > * V4L2 device\n> > > @@ -533,7 +565,6 @@ void V4L2Device::listControls()\n> > > \tControlInfoMap::Map ctrls;\n> > > \tstruct v4l2_query_ext_ctrl ctrl = {};\n> > >\n> > > -\t/* \\todo Add support for menu controls. */\n> > > \twhile (1) {\n> > > \t\tctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL |\n> > > \t\t\t V4L2_CTRL_FLAG_NEXT_COMPOUND;\n> > > @@ -544,16 +575,23 @@ void V4L2Device::listControls()\n> > > \t\t ctrl.flags & V4L2_CTRL_FLAG_DISABLED)\n> > > \t\t\tcontinue;\n> > >\n> > > +\t\tControlInfo ctrlInfo;\n> > > +\n> > > \t\tswitch (ctrl.type) {\n> > > \t\tcase V4L2_CTRL_TYPE_INTEGER:\n> > > \t\tcase V4L2_CTRL_TYPE_BOOLEAN:\n> > > -\t\tcase V4L2_CTRL_TYPE_MENU:\n> > > \t\tcase V4L2_CTRL_TYPE_BUTTON:\n> > > \t\tcase V4L2_CTRL_TYPE_INTEGER64:\n> > > \t\tcase V4L2_CTRL_TYPE_BITMASK:\n> > > -\t\tcase V4L2_CTRL_TYPE_INTEGER_MENU:\n> > > \t\tcase V4L2_CTRL_TYPE_U8:\n> > > +\t\t\tctrlInfo = v4l2ControlInfo(ctrl);\n> > > +\t\t\tbreak;\n> > > +\n> > > +\t\tcase V4L2_CTRL_TYPE_INTEGER_MENU:\n> > > +\t\tcase V4L2_CTRL_TYPE_MENU:\n> > > +\t\t\tctrlInfo = v4l2MenuControlInfo(ctrl);\n> > > \t\t\tbreak;\n> > > +\n> > > \t\t/* \\todo Support other control types. */\n> > > \t\tdefault:\n> > > \t\t\tLOG(V4L2, Debug)\n> > > @@ -562,10 +600,13 @@ void V4L2Device::listControls()\n> > > \t\t\tcontinue;\n> > > \t\t}\n> > >\n> > > +\t\tLOG(V4L2, Debug) << \"Control: \" << ctrl.name\n> > > +\t\t\t\t << \" (\" << utils::hex(ctrl.id) << \")\";\n> > > +\n> > > \t\tcontrolIds_.emplace_back(v4l2ControlId(ctrl));\n> > > \t\tcontrolInfo_.emplace(ctrl.id, ctrl);\n> > >\n> > > -\t\tctrls.emplace(controlIds_.back().get(), v4l2ControlInfo(ctrl));\n> > > +\t\tctrls.emplace(controlIds_.back().get(), std::move(ctrlInfo));\n> > > \t}\n> > >\n> > > \tcontrols_ = std::move(ctrls);\n> > > @@ -630,6 +671,10 @@ void V4L2Device::updateControls(ControlList *ctrls,\n> > > \t\t\tvalue.set(v4l2Ctrl.value64);\n> > > \t\t\tbreak;\n> > >\n> > > +\t\tcase ControlTypeInteger32:\n> > > +\t\t\tvalue.set(v4l2Ctrl.value);\n> > > +\t\t\tbreak;\n> > > +\n> > > \t\tcase ControlTypeByte:\n> > > \t\t\t/*\n> > > \t\t\t * No action required, the VIDIOC_[GS]_EXT_CTRLS ioctl","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 37075BD22E\n\tfor ;\n\tMon, 7 Jun 2021 21:39:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F10556891F;\n\tMon, 7 Jun 2021 23:39:48 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6FC9F6891F\n\tfor ;\n\tMon, 7 Jun 2021 23:39:47 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DB1E58DB;\n\tMon, 7 Jun 2021 23:39:46 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Tn+5DdO2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1623101987;\n\tbh=EceoJoSGHgEqz/swmnnUKspI05fgX/yFNtLQv/GHhvw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Tn+5DdO2/cpYbKvYVj6siheSKwkZIQl8SMrUHPF88Chh4/CvYZ0LKeYOg2QzUk8io\n\tqsRgiU8CokF5IWGPjrvAvjpvTghrlbE/EoO+4qGxArtI+jY5l+Rf/H5tDVC06gU7zq\n\tPCLwqB0PQfwuige0XVsMJHuhxexkkKfn+wqrCqNg=","Date":"Tue, 8 Jun 2021 00:39:33 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210607011402.55331-1-hiroh@chromium.org>\n\t<20210607011402.55331-2-hiroh@chromium.org>\n\t<20210607141558.du66wb4dubfvlbmm@uno.localdomain>\n\t<20210607144653.53ertd5kjl6cible@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210607144653.53ertd5kjl6cible@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v7 2/6] libcamera: V4L2Device: Support\n\tv4l2 menu control","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]