[{"id":17507,"web_url":"https://patchwork.libcamera.org/comment/17507/","msgid":"<20210610160544.yhzm5ck5nb54qflh@uno.localdomain>","date":"2021-06-10T16:05:44","subject":"Re: [libcamera-devel] [PATCH v8 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 Thu, Jun 10, 2021 at 05:25:35PM +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> Reviewed-by: Jacopo Mondi \n> Reviewed-by: Laurent Pinchart \n> ---\n> include/libcamera/internal/v4l2_device.h | 5 +\n> src/libcamera/v4l2_device.cpp | 186 +++++++++++++++--------\n> 2 files changed, 127 insertions(+), 64 deletions(-)\n>\n> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> index b82e2a14..c318e976 100644\n> --- a/include/libcamera/internal/v4l2_device.h\n> +++ b/include/libcamera/internal/v4l2_device.h\n> @@ -56,6 +56,11 @@ protected:\n> \tint fd() const { return fd_; }\n>\n> private:\n> +\tstatic ControlType v4l2CtrlType(uint32_t ctrlType);\n> +\tstatic std::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> \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..cda3769a 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -32,69 +32,6 @@ 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\nThanks for addressing my request, but now we have an empty namespace\nwhich could be dropped.\n\nI have a few more picky comments on the newly added documentation.\nWith your ack they can be applied when merging.\n\n>\n> /**\n> @@ -524,6 +461,121 @@ 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 Retrieve ControlType for V4L2 control type\n\nRetrieve the libcamera control type associated with the V4L2 control type\n\n> + * \\param[in] ctrlType V4L2 control type\n> + *\n> + * \\return ControlType for \\a ctrlType\n\nThe ControlType corresponding to \\a ctrlType\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> +/**\n> + * \\brief Create ControlId for v4l2_query_ext_ctrl\n\n\\brief Create a ControlId for a V4L2 control\n\n> + * \\param[in] ctrl v4l2_query_ext_ctrl\n\nYour below description sound better:\n\n\\param[in] ctrl The v4l2_query_ext_ctrl that represents a V4L2 control\n\n> + *\n> + * \\return ControlId for \\a ctrl\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 from v4l2_query_ext_ctrl\n> + * \\param[in] ctrl v4l2_query_ext_ctrl\n\nYour below description sound better:\n\n\\param[in] ctrl The v4l2_query_ext_ctrl that represents a V4L2 control\n\n\n> + *\n> + * \\return ControlInfo from \\a v4l2_query_ext_ctrl\n\nA ControlInfo created from \\a ctrl\n\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> +\tcase V4L2_CTRL_TYPE_INTEGER_MENU:\n> +\tcase V4L2_CTRL_TYPE_MENU:\n> +\t\treturn v4l2MenuControlInfo(ctrl);\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> + * \\brief Create ControlInfo for a V4L2 menu control\n> + * \\param[in] ctrl The v4l2_query_ext_ctrl that represents a menu\n\n.. a V4L2 menu control\n\n> + *\n> + * The created ControlInfo contains indices acquired by VIDIOC_QUERYMENU.\n> + *\n> + * \\return ControlInfo for a V4L2 menu control\n\nI would apply the same suggestion given above\n\n> + */\n> +ControlInfo V4L2Device::v4l2MenuControlInfo(const 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> +\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}\n> +\n> +\treturn ControlInfo(indices,\n> +\t\t\t ControlValue(static_cast(ctrl.default_value)));\n> +}\n> +\n> /*\n> * \\brief List and store information about all controls supported by the\n> * V4L2 device\n> @@ -533,7 +585,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> @@ -562,6 +613,9 @@ 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> @@ -630,6 +684,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 9F8BCBD78E\n\tfor ;\n\tThu, 10 Jun 2021 16:04:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0364F6892E;\n\tThu, 10 Jun 2021 18:04:58 +0200 (CEST)","from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[217.70.183.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DD9AE6029B\n\tfor ;\n\tThu, 10 Jun 2021 18:04:55 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay3-d.mail.gandi.net (Postfix) with ESMTPSA id 1F20160007;\n\tThu, 10 Jun 2021 16:04:54 +0000 (UTC)"],"Date":"Thu, 10 Jun 2021 18:05:44 +0200","From":"Jacopo Mondi ","To":"Hirokazu Honda ","Message-ID":"<20210610160544.yhzm5ck5nb54qflh@uno.localdomain>","References":"<20210610082539.529739-1-hiroh@chromium.org>\n\t<20210610082539.529739-2-hiroh@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210610082539.529739-2-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [PATCH v8 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":17511,"web_url":"https://patchwork.libcamera.org/comment/17511/","msgid":"","date":"2021-06-11T03:54:41","subject":"Re: [libcamera-devel] [PATCH v8 2/6] libcamera: V4L2Device: Support\n\tv4l2 menu control","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Jacopo,\n\nOn Fri, Jun 11, 2021 at 1:04 AM Jacopo Mondi wrote:\n\n> Hi Hiro,\n>\n> On Thu, Jun 10, 2021 at 05:25:35PM +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> > Reviewed-by: Jacopo Mondi \n> > Reviewed-by: Laurent Pinchart \n> > ---\n> > include/libcamera/internal/v4l2_device.h | 5 +\n> > src/libcamera/v4l2_device.cpp | 186 +++++++++++++++--------\n> > 2 files changed, 127 insertions(+), 64 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/v4l2_device.h\n> b/include/libcamera/internal/v4l2_device.h\n> > index b82e2a14..c318e976 100644\n> > --- a/include/libcamera/internal/v4l2_device.h\n> > +++ b/include/libcamera/internal/v4l2_device.h\n> > @@ -56,6 +56,11 @@ protected:\n> > int fd() const { return fd_; }\n> >\n> > private:\n> > + static ControlType v4l2CtrlType(uint32_t ctrlType);\n> > + static std::unique_ptr v4l2ControlId(const\n> v4l2_query_ext_ctrl &ctrl);\n> > + ControlInfo v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl);\n> > + ControlInfo v4l2MenuControlInfo(const v4l2_query_ext_ctrl &ctrl);\n> > +\n> > void listControls();\n> > void updateControls(ControlList *ctrls,\n> > Span v4l2Ctrls);\n> > diff --git a/src/libcamera/v4l2_device.cpp\n> b/src/libcamera/v4l2_device.cpp\n> > index b39c6266..cda3769a 100644\n> > --- a/src/libcamera/v4l2_device.cpp\n> > +++ b/src/libcamera/v4l2_device.cpp\n> > @@ -32,69 +32,6 @@ LOG_DEFINE_CATEGORY(V4L2)\n> >\n> > namespace {\n> >\n> > -ControlType v4l2CtrlType(uint32_t ctrlType)\n> > -{\n> > - switch (ctrlType) {\n> > - case V4L2_CTRL_TYPE_U8:\n> > - return ControlTypeByte;\n> > -\n> > - case V4L2_CTRL_TYPE_BOOLEAN:\n> > - return ControlTypeBool;\n> > -\n> > - case V4L2_CTRL_TYPE_INTEGER:\n> > - return ControlTypeInteger32;\n> > -\n> > - case V4L2_CTRL_TYPE_INTEGER64:\n> > - return ControlTypeInteger64;\n> > -\n> > - case V4L2_CTRL_TYPE_MENU:\n> > - case V4L2_CTRL_TYPE_BUTTON:\n> > - case V4L2_CTRL_TYPE_BITMASK:\n> > - case V4L2_CTRL_TYPE_INTEGER_MENU:\n> > - /*\n> > - * More precise types may be needed, for now use a 32-bit\n> > - * integer type.\n> > - */\n> > - return ControlTypeInteger32;\n> > -\n> > - default:\n> > - return ControlTypeNone;\n> > - }\n> > -}\n> > -\n> > -std::unique_ptr v4l2ControlId(const v4l2_query_ext_ctrl\n> &ctrl)\n> > -{\n> > - const size_t len = strnlen(ctrl.name, sizeof(ctrl.name));\n> > - const std::string name(static_cast(ctrl.name), len);\n> > - const ControlType type = v4l2CtrlType(ctrl.type);\n> > -\n> > - return std::make_unique(ctrl.id, name, type);\n> > -}\n> > -\n> > -ControlInfo v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl)\n> > -{\n> > - switch (ctrl.type) {\n> > - case V4L2_CTRL_TYPE_U8:\n> > - return ControlInfo(static_cast(ctrl.minimum),\n> > - static_cast(ctrl.maximum),\n> > -\n> static_cast(ctrl.default_value));\n> > -\n> > - case V4L2_CTRL_TYPE_BOOLEAN:\n> > - return ControlInfo(static_cast(ctrl.minimum),\n> > - static_cast(ctrl.maximum),\n> > - static_cast(ctrl.default_value));\n> > -\n> > - case V4L2_CTRL_TYPE_INTEGER64:\n> > - return ControlInfo(static_cast(ctrl.minimum),\n> > - static_cast(ctrl.maximum),\n> > -\n> static_cast(ctrl.default_value));\n> > -\n> > - default:\n> > - return ControlInfo(static_cast(ctrl.minimum),\n> > - static_cast(ctrl.maximum),\n> > -\n> static_cast(ctrl.default_value));\n> > - }\n> > -}\n> > } /* namespace */\n>\n> Thanks for addressing my request, but now we have an empty namespace\n> which could be dropped.\n>\n> I have a few more picky comments on the newly added documentation.\n> With your ack they can be applied when merging.\n>\n> >\n> > /**\n> > @@ -524,6 +461,121 @@ int V4L2Device::ioctl(unsigned long request, void\n> *argp)\n> > * \\return The V4L2 device file descriptor, -1 if the device node is\n> not open\n> > */\n> >\n> > +/**\n> > + * \\brief Retrieve ControlType for V4L2 control type\n>\n> Retrieve the libcamera control type associated with the V4L2 control type\n>\n> > + * \\param[in] ctrlType V4L2 control type\n> > + *\n> > + * \\return ControlType for \\a ctrlType\n>\n> The ControlType corresponding to \\a ctrlType\n>\n> > + */\n> > +ControlType V4L2Device::v4l2CtrlType(uint32_t ctrlType)\n> > +{\n> > + switch (ctrlType) {\n> > + case V4L2_CTRL_TYPE_U8:\n> > + return ControlTypeByte;\n> > +\n> > + case V4L2_CTRL_TYPE_BOOLEAN:\n> > + return ControlTypeBool;\n> > +\n> > + case V4L2_CTRL_TYPE_INTEGER:\n> > + return ControlTypeInteger32;\n> > +\n> > + case V4L2_CTRL_TYPE_INTEGER64:\n> > + return ControlTypeInteger64;\n> > +\n> > + case V4L2_CTRL_TYPE_MENU:\n> > + case V4L2_CTRL_TYPE_BUTTON:\n> > + case V4L2_CTRL_TYPE_BITMASK:\n> > + case V4L2_CTRL_TYPE_INTEGER_MENU:\n> > + /*\n> > + * More precise types may be needed, for now use a 32-bit\n> > + * integer type.\n> > + */\n> > + return ControlTypeInteger32;\n> > +\n> > + default:\n> > + return ControlTypeNone;\n> > + }\n> > +}\n> > +\n> > +/**\n> > + * \\brief Create ControlId for v4l2_query_ext_ctrl\n>\n> \\brief Create a ControlId for a V4L2 control\n>\n> > + * \\param[in] ctrl v4l2_query_ext_ctrl\n>\n> Your below description sound better:\n>\n> \\param[in] ctrl The v4l2_query_ext_ctrl that represents a V4L2 control\n>\n> > + *\n> > + * \\return ControlId for \\a ctrl\n> > + */\n> > +std::unique_ptr V4L2Device::v4l2ControlId(const\n> v4l2_query_ext_ctrl &ctrl)\n> > +{\n> > + const size_t len = strnlen(ctrl.name, sizeof(ctrl.name));\n> > + const std::string name(static_cast(ctrl.name), len);\n> > + const ControlType type = v4l2CtrlType(ctrl.type);\n> > +\n> > + return std::make_unique(ctrl.id, name, type);\n> > +}\n> > +\n> > +/**\n> > + * \\brief Create ControlInfo from v4l2_query_ext_ctrl\n> > + * \\param[in] ctrl v4l2_query_ext_ctrl\n>\n> Your below description sound better:\n>\n> \\param[in] ctrl The v4l2_query_ext_ctrl that represents a V4L2 control\n>\n>\n> > + *\n> > + * \\return ControlInfo from \\a v4l2_query_ext_ctrl\n>\n> A ControlInfo created from \\a ctrl\n>\n> > + */\n> > +ControlInfo V4L2Device::v4l2ControlInfo(const v4l2_query_ext_ctrl &ctrl)\n> > +{\n> > + switch (ctrl.type) {\n> > + case V4L2_CTRL_TYPE_U8:\n> > + return ControlInfo(static_cast(ctrl.minimum),\n> > + static_cast(ctrl.maximum),\n> > +\n> static_cast(ctrl.default_value));\n> > +\n> > + case V4L2_CTRL_TYPE_BOOLEAN:\n> > + return ControlInfo(static_cast(ctrl.minimum),\n> > + static_cast(ctrl.maximum),\n> > + static_cast(ctrl.default_value));\n> > +\n> > + case V4L2_CTRL_TYPE_INTEGER64:\n> > + return ControlInfo(static_cast(ctrl.minimum),\n> > + static_cast(ctrl.maximum),\n> > +\n> static_cast(ctrl.default_value));\n> > +\n> > + case V4L2_CTRL_TYPE_INTEGER_MENU:\n> > + case V4L2_CTRL_TYPE_MENU:\n> > + return v4l2MenuControlInfo(ctrl);\n> > +\n> > + default:\n> > + return ControlInfo(static_cast(ctrl.minimum),\n> > + static_cast(ctrl.maximum),\n> > +\n> static_cast(ctrl.default_value));\n> > + }\n> > +}\n> > +\n> > +/**\n> > + * \\brief Create ControlInfo for a V4L2 menu control\n> > + * \\param[in] ctrl The v4l2_query_ext_ctrl that represents a menu\n>\n> .. a V4L2 menu control\n>\n> > + *\n> > + * The created ControlInfo contains indices acquired by\n> VIDIOC_QUERYMENU.\n> > + *\n> > + * \\return ControlInfo for a V4L2 menu control\n>\n> I would apply the same suggestion given above\n>\n>\nAll of them sound good to me.\nThanks.\n\n-Hiro\n\n\n> > + */\n> > +ControlInfo V4L2Device::v4l2MenuControlInfo(const struct\n> v4l2_query_ext_ctrl &ctrl)\n> > +{\n> > + std::vector indices;\n> > + struct v4l2_querymenu menu = {};\n> > + menu.id = ctrl.id;\n> > +\n> > + if (ctrl.minimum < 0)\n> > + return ControlInfo();\n> > +\n> > + for (int32_t index = ctrl.minimum; index <= ctrl.maximum; ++index)\n> {\n> > + menu.index = index;\n> > + if (ioctl(VIDIOC_QUERYMENU, &menu) != 0)\n> > + continue;\n> > +\n> > + indices.push_back(index);\n> > + }\n> > +\n> > + return ControlInfo(indices,\n> > +\n> ControlValue(static_cast(ctrl.default_value)));\n> > +}\n> > +\n> > /*\n> > * \\brief List and store information about all controls supported by the\n> > * V4L2 device\n> > @@ -533,7 +585,6 @@ void V4L2Device::listControls()\n> > ControlInfoMap::Map ctrls;\n> > struct v4l2_query_ext_ctrl ctrl = {};\n> >\n> > - /* \\todo Add support for menu controls. */\n> > while (1) {\n> > ctrl.id |= V4L2_CTRL_FLAG_NEXT_CTRL |\n> > V4L2_CTRL_FLAG_NEXT_COMPOUND;\n> > @@ -562,6 +613,9 @@ void V4L2Device::listControls()\n> > continue;\n> > }\n> >\n> > + LOG(V4L2, Debug) << \"Control: \" << ctrl.name\n> > + << \" (\" << utils::hex(ctrl.id) << \")\";\n> > +\n> > controlIds_.emplace_back(v4l2ControlId(ctrl));\n> > controlInfo_.emplace(ctrl.id, ctrl);\n> >\n> > @@ -630,6 +684,10 @@ void V4L2Device::updateControls(ControlList *ctrls,\n> > value.set(v4l2Ctrl.value64);\n> > break;\n> >\n> > + case ControlTypeInteger32:\n> > + value.set(v4l2Ctrl.value);\n> > + break;\n> > +\n> > case ControlTypeByte:\n> > /*\n> > * No action required, the VIDIOC_[GS]_EXT_CTRLS\n> ioctl\n> > --\n> > 2.32.0.rc1.229.g3e70b5a671-goog\n> >\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 8B04ABD78E\n\tfor ;\n\tFri, 11 Jun 2021 03:54:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C4BEB68931;\n\tFri, 11 Jun 2021 05:54:53 +0200 (CEST)","from mail-ed1-x52c.google.com (mail-ed1-x52c.google.com\n\t[IPv6:2a00:1450:4864:20::52c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CCC716029A\n\tfor ;\n\tFri, 11 Jun 2021 05:54:52 +0200 (CEST)","by mail-ed1-x52c.google.com with SMTP id r11so35470347edt.13\n\tfor ;\n\tThu, 10 Jun 2021 20:54:52 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"QAat4nEf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=xngxKPXxOeue70hgkD9Kjs1em8pshhfr5g0JFyfmR3A=;\n\tb=QAat4nEfxiF2gNfH9doLrhaEOBM1tg8VIY+GGLaVVjlIJikmFl6oPbU+lbw+y8HRfq\n\tn335+7uM2ZW3oVuNnR43zPUuUKtKOs34ZJrf2NCA7lcOPD2Gfl0jxdb82YlIFxi0lu2p\n\tB9JwLkMs0pSt+eNIdkQZ4GKHh9FflPd4ueP0k=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=xngxKPXxOeue70hgkD9Kjs1em8pshhfr5g0JFyfmR3A=;\n\tb=MjNjMOn7sI/p1lq/6jKCU4jV4GzBl6DLh6cV4biDcZEpenvMTioYRgDLboSjAF7wir\n\tDAq2X3XQD1FXDSmEe6cPG0x7RcRo4I/I2Epcw0Bcj1ywDR+aczLYwsGUEMouPnMv/DdQ\n\tm3HNEHrv5pin+JH82h9wNj6Hn27gb4zlxY68QJx20CPWAHwseEjaoA3rn5ouml8ijqDk\n\tCXhCBWeOeh8zgcLyhZaxc+hIakoLQkPhG2AVNidLj5k4Q2GvNyEOl9jVUnwCu52AIg7G\n\tzazyj5hr+Q8nRGS9rdCJNX+wfUqLiqWuM+RB76BWpYmRtcvhkS15WVqa9gkOPVwo33i+\n\tdxHQ==","X-Gm-Message-State":"AOAM531XT6cflnGOPJGJnFB7Xtpqrrq6JwjBl65c/beewR52W4tL/nbO\n\tWqirBVKjsChiVjhkHllqEkwxOlB+lLYLlSPRwpdWEjE0b444KQ==","X-Google-Smtp-Source":"ABdhPJxciHbLFaX4bBGFr4vOjv22vH6ZStXckerVILGm/W6cAk4qFATV4j5Dqqw1GJcZ0nP0BErrCistRjhWzs9M774=","X-Received":"by 2002:a50:ee05:: with SMTP id g5mr1553562eds.73.1623383692345; \n\tThu, 10 Jun 2021 20:54:52 -0700 (PDT)","MIME-Version":"1.0","References":"<20210610082539.529739-1-hiroh@chromium.org>\n\t<20210610082539.529739-2-hiroh@chromium.org>\n\t<20210610160544.yhzm5ck5nb54qflh@uno.localdomain>","In-Reply-To":"<20210610160544.yhzm5ck5nb54qflh@uno.localdomain>","From":"Hirokazu Honda ","Date":"Fri, 11 Jun 2021 12:54:41 +0900","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"multipart/alternative; boundary=\"000000000000511e3805c4757863\"","Subject":"Re: [libcamera-devel] [PATCH v8 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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]