[{"id":16801,"web_url":"https://patchwork.libcamera.org/comment/16801/","msgid":"<20210506091155.jvn3novrsxus3ohl@uno.localdomain>","date":"2021-05-06T09:11:55","subject":"Re: [libcamera-devel] [PATCH v4 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, May 06, 2021 at 04:54:45PM +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 | 3 ++\n> src/libcamera/v4l2_device.cpp | 65 +++++++++++++++++++++---\n> 2 files changed, 61 insertions(+), 7 deletions(-)\n>\n> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> index 087f07e7..c7a2539c 100644\n> --- a/include/libcamera/internal/v4l2_device.h\n> +++ b/include/libcamera/internal/v4l2_device.h\n> @@ -54,6 +54,9 @@ protected:\n> \tint fd() const { return fd_; }\n>\n> private:\n> +\tbool createControlInfoForMenu(const v4l2_query_ext_ctrl &ctrl,\n> +\t\t\t\t ControlInfo &ctrlInfo);\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 397029ac..7056c71a 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -452,6 +452,7 @@ int V4L2Device::ioctl(unsigned long request, void *argp)\n> \treturn 0;\n> }\n>\n> +\n\nAddional empty line\n\nDoesn't checkstyle.py warn you ?\n\n> /**\n> * \\fn V4L2Device::deviceNode()\n> * \\brief Retrieve the device node path\n> @@ -459,10 +460,47 @@ int V4L2Device::ioctl(unsigned long request, void *argp)\n> */\n>\n> /**\n> - * \\fn V4L2Device::fd()\n> - * \\brief Retrieve the V4L2 device file descriptor\n> - * \\return The V4L2 device file descriptor, -1 if the device node is not open\n\nWhere has the fd() documentation gone ?\n\n> + * \\brief Create ControlInfo for v4l2 menu ctrl.\n> + * \\param[in] ctrl The v4l2_query_ext_ctrl that represents a menu\n> + * \\param[out] ctrlInfo The created controlInfo\n> + *\n> + * The created ControlInfo contains not only values and also extra values, which\n> + * are indices and name/value acquired by VIDIOC_QUERYMENU, respectively. The\n> + * extra values contains std::string if the type of \\a ctrl is\n> + * V4L2_CTRL_TYPE_MENU or int64_t otherwise.\n\nThis part of the documentation doesn't apply anymore\n\n> + *\n> + * \\return True on success or false otherwise\n> */\n> +bool V4L2Device::createControlInfoForMenu(const v4l2_query_ext_ctrl &ctrl,\n> +\t\t\t\t\t ControlInfo &ctrlInfo)\n\nCurrently we construct V4L2ControlInfo passing in a\nv4l2_query_ext_ctrl.\n\nCan't we do the same and detect if we have to enumerate indices if the\ncontrol type is MENU, instead of creating a new function here ?\n\n> +{\n> +\tASSERT(ctrl.type == V4L2_CTRL_TYPE_MENU ||\n> +\t ctrl.type == V4L2_CTRL_TYPE_INTEGER_MENU);\n> +\n> +\tstd::vector indices;\n> +\tv4l2_querymenu menu;\n> +\tmemset(&menu, 0, sizeof(menu));\n> +\tmenu.id = ctrl.id;\n> +\n> +\tfor (menu.index = ctrl.minimum;\n> +\t static_cast(menu.index) <= ctrl.maximum; menu.index++) {\n\nI was about to complain that you could loop until you don't get\nEINVAL, but the V4L2 documentation supports what you have here\n\n\"It is possible for VIDIOC_QUERYMENU to return an EINVAL error code\nfor some indices between minimum and maximum. In that case that\nparticular menu item is not supported by this driver. Also note that\nthe minimum value is not necessarily 0.\"\n\n> +\t\tif (ioctl(VIDIOC_QUERYMENU, &menu) != 0)\n> +\t\t\tcontinue;\n\nI would however be loud if the return code is !-EINVAL\n\n> +\n> +\t\tindices.emplace_back(static_cast(menu.index));\n\nHas emplicing any benefit compared to push_back for POD like an\nint32_t ?\n\n> +\t}\n> +\n> +\tif (indices.empty()) {\n> +\t\tLOG(V4L2, Error)\n> +\t\t\t<< \"No applicable value: \" << utils::hex(ctrl.id);\n> +\n> +\t\treturn false;\n> +\t}\n> +\n> +\tctrlInfo = ControlInfo(indices);\n\nCan't you return the ctrlInfo directly and exploit RVO ? or would it\nbecome difficult to detect errors ? I would howerv consider try moving\nthis to the V4L2ControlInfo constructor first.\n\n> +\n> +\treturn true;\n> +}\n>\n> /*\n> * \\brief List and store information about all controls supported by the\n> @@ -473,7 +511,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> @@ -484,15 +521,22 @@ void V4L2Device::listControls()\n> \t\t ctrl.flags & V4L2_CTRL_FLAG_DISABLED)\n> \t\t\tcontinue;\n>\n> +\t\tControlInfo ctrlInfo;\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\tif (!createControlInfoForMenu(ctrl, ctrlInfo))\n> +\t\t\t\tcontinue;\n> +\n> \t\t\tbreak;\n> \t\t/* \\todo Support other control types. */\n> \t\tdefault:\n> @@ -502,10 +546,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(std::make_unique(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(), ctrlInfo);\n> \t}\n>\n> \tcontrols_ = std::move(ctrls);\n> @@ -535,6 +582,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\nWe have this here below:\n\n\t\tdefault:\n\t\t\t/*\n\t\t\t * \\todo To be changed when support for string controls\n\t\t\t * will be added.\n\t\t\t */\n\t\t\tvalue.set(v4l2Ctrl->value);\n\t\t\tbreak;\n\nwasn't it enough ?\n\nThanks\n j\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.31.1.607.g51e8a6a459-goog\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 8178BBF81D\n\tfor ;\n\tThu, 6 May 2021 09:11:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0CEC368919;\n\tThu, 6 May 2021 11:11:14 +0200 (CEST)","from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0CD4468911\n\tfor ;\n\tThu, 6 May 2021 11:11:12 +0200 (CEST)","from uno.localdomain (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 688D34000C;\n\tThu, 6 May 2021 09:11:11 +0000 (UTC)"],"X-Originating-IP":"93.61.96.190","Date":"Thu, 6 May 2021 11:11:55 +0200","From":"Jacopo Mondi ","To":"Hirokazu Honda ","Message-ID":"<20210506091155.jvn3novrsxus3ohl@uno.localdomain>","References":"<20210506075449.1761752-1-hiroh@chromium.org>\n\t<20210506075449.1761752-3-hiroh@chromium.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210506075449.1761752-3-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [PATCH v4 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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16807,"web_url":"https://patchwork.libcamera.org/comment/16807/","msgid":"","date":"2021-05-06T12:14:52","subject":"Re: [libcamera-devel] [PATCH v4 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 Thu, May 6, 2021 at 6:11 PM Jacopo Mondi wrote:\n\n> Hi Hiro,\n>\n> On Thu, May 06, 2021 at 04:54:45PM +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 | 3 ++\n> > src/libcamera/v4l2_device.cpp | 65 +++++++++++++++++++++---\n> > 2 files changed, 61 insertions(+), 7 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/v4l2_device.h\n> b/include/libcamera/internal/v4l2_device.h\n> > index 087f07e7..c7a2539c 100644\n> > --- a/include/libcamera/internal/v4l2_device.h\n> > +++ b/include/libcamera/internal/v4l2_device.h\n> > @@ -54,6 +54,9 @@ protected:\n> > int fd() const { return fd_; }\n> >\n> > private:\n> > + bool createControlInfoForMenu(const v4l2_query_ext_ctrl &ctrl,\n> > + ControlInfo &ctrlInfo);\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 397029ac..7056c71a 100644\n> > --- a/src/libcamera/v4l2_device.cpp\n> > +++ b/src/libcamera/v4l2_device.cpp\n> > @@ -452,6 +452,7 @@ int V4L2Device::ioctl(unsigned long request, void\n> *argp)\n> > return 0;\n> > }\n> >\n> > +\n>\n> Addional empty line\n>\n> Doesn't checkstyle.py warn you ?\n>\n> > /**\n> > * \\fn V4L2Device::deviceNode()\n> > * \\brief Retrieve the device node path\n> > @@ -459,10 +460,47 @@ int V4L2Device::ioctl(unsigned long request, void\n> *argp)\n> > */\n> >\n> > /**\n> > - * \\fn V4L2Device::fd()\n> > - * \\brief Retrieve the V4L2 device file descriptor\n> > - * \\return The V4L2 device file descriptor, -1 if the device node is\n> not open\n>\n> Where has the fd() documentation gone ?\n>\n> > + * \\brief Create ControlInfo for v4l2 menu ctrl.\n> > + * \\param[in] ctrl The v4l2_query_ext_ctrl that represents a menu\n> > + * \\param[out] ctrlInfo The created controlInfo\n> > + *\n> > + * The created ControlInfo contains not only values and also extra\n> values, which\n> > + * are indices and name/value acquired by VIDIOC_QUERYMENU,\n> respectively. The\n> > + * extra values contains std::string if the type of \\a ctrl is\n> > + * V4L2_CTRL_TYPE_MENU or int64_t otherwise.\n>\n> This part of the documentation doesn't apply anymore\n>\n> > + *\n> > + * \\return True on success or false otherwise\n> > */\n> > +bool V4L2Device::createControlInfoForMenu(const v4l2_query_ext_ctrl\n> &ctrl,\n> > + ControlInfo &ctrlInfo)\n>\n> Currently we construct V4L2ControlInfo passing in a\n> v4l2_query_ext_ctrl.\n>\n> Can't we do the same and detect if we have to enumerate indices if the\n> control type is MENU, instead of creating a new function here ?\n>\n>\nTo do that, we need to pass fd to call QUERYMENU.\nI think the direction is opposite. I would remove V4L2CtrlInfo because it\njust calls a few functions to construct ControlInfo.\nWhat do you think?\n\n-Hiro\n\n> +{\n> > + ASSERT(ctrl.type == V4L2_CTRL_TYPE_MENU ||\n> > + ctrl.type == V4L2_CTRL_TYPE_INTEGER_MENU);\n> > +\n> > + std::vector indices;\n> > + v4l2_querymenu menu;\n> > + memset(&menu, 0, sizeof(menu));\n> > + menu.id = ctrl.id;\n> > +\n> > + for (menu.index = ctrl.minimum;\n> > + static_cast(menu.index) <= ctrl.maximum; menu.index++) {\n>\n> I was about to complain that you could loop until you don't get\n> EINVAL, but the V4L2 documentation supports what you have here\n>\n> \"It is possible for VIDIOC_QUERYMENU to return an EINVAL error code\n> for some indices between minimum and maximum. In that case that\n> particular menu item is not supported by this driver. Also note that\n> the minimum value is not necessarily 0.\"\n>\n> > + if (ioctl(VIDIOC_QUERYMENU, &menu) != 0)\n> > + continue;\n>\n> I would however be loud if the return code is !-EINVAL\n>\n> > +\n> > + indices.emplace_back(static_cast(menu.index));\n>\n> Has emplicing any benefit compared to push_back for POD like an\n> int32_t ?\n>\n> > + }\n> > +\n> > + if (indices.empty()) {\n> > + LOG(V4L2, Error)\n> > + << \"No applicable value: \" << utils::hex(ctrl.id);\n> > +\n> > + return false;\n> > + }\n> > +\n> > + ctrlInfo = ControlInfo(indices);\n>\n> Can't you return the ctrlInfo directly and exploit RVO ? or would it\n> become difficult to detect errors ? I would howerv consider try moving\n> this to the V4L2ControlInfo constructor first.\n>\n> > +\n> > + return true;\n> > +}\n> >\n> > /*\n> > * \\brief List and store information about all controls supported by the\n> > @@ -473,7 +511,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> > @@ -484,15 +521,22 @@ void V4L2Device::listControls()\n> > ctrl.flags & V4L2_CTRL_FLAG_DISABLED)\n> > continue;\n> >\n> > + ControlInfo ctrlInfo;\n> > switch (ctrl.type) {\n> > case V4L2_CTRL_TYPE_INTEGER:\n> > case V4L2_CTRL_TYPE_BOOLEAN:\n> > - case V4L2_CTRL_TYPE_MENU:\n> > case V4L2_CTRL_TYPE_BUTTON:\n> > case V4L2_CTRL_TYPE_INTEGER64:\n> > case V4L2_CTRL_TYPE_BITMASK:\n> > - case V4L2_CTRL_TYPE_INTEGER_MENU:\n> > case V4L2_CTRL_TYPE_U8:\n> > + ctrlInfo = V4L2ControlInfo(ctrl);\n> > + break;\n> > +\n> > + case V4L2_CTRL_TYPE_INTEGER_MENU:\n> > + case V4L2_CTRL_TYPE_MENU:\n> > + if (!createControlInfoForMenu(ctrl, ctrlInfo))\n> > + continue;\n> > +\n> > break;\n> > /* \\todo Support other control types. */\n> > default:\n> > @@ -502,10 +546,13 @@ void V4L2Device::listControls()\n> > continue;\n> > }\n> >\n> > + LOG(V4L2, Debug) << \"Control: \" << ctrl.name\n> > + << \" (\" << utils::hex(ctrl.id) << \")\";\n> > +\n> >\n> controlIds_.emplace_back(std::make_unique(ctrl));\n> > controlInfo_.emplace(ctrl.id, ctrl);\n> >\n> > - ctrls.emplace(controlIds_.back().get(),\n> V4L2ControlInfo(ctrl));\n> > + ctrls.emplace(controlIds_.back().get(), ctrlInfo);\n> > }\n> >\n> > controls_ = std::move(ctrls);\n> > @@ -535,6 +582,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>\n> We have this here below:\n>\n> default:\n> /*\n> * \\todo To be changed when support for string\n> controls\n> * will be added.\n> */\n> value.set(v4l2Ctrl->value);\n> break;\n>\n> wasn't it enough ?\n>\n> Thanks\n> j\n>\n> > case ControlTypeByte:\n> > /*\n> > * No action required, the VIDIOC_[GS]_EXT_CTRLS\n> ioctl\n> > --\n> > 2.31.1.607.g51e8a6a459-goog\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\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 022EBBF81D\n\tfor ;\n\tThu, 6 May 2021 12:15:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 38B2D68915;\n\tThu, 6 May 2021 14:15:05 +0200 (CEST)","from mail-ed1-x530.google.com (mail-ed1-x530.google.com\n\t[IPv6:2a00:1450:4864:20::530])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 35C1068901\n\tfor ;\n\tThu, 6 May 2021 14:15:03 +0200 (CEST)","by mail-ed1-x530.google.com with SMTP id l7so5906819edb.1\n\tfor ;\n\tThu, 06 May 2021 05:15:03 -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=\"KcwfL4sC\"; 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=ViFN8LY32cCn+6o0h+8PXR3uZ2n3bxnARn4VfAWB7so=;\n\tb=KcwfL4sCUvDMXwzTfV8nJWgYHWjSJvS+pqHbxsw/YpCuG66PHj59SeLeB2SK9fSsr9\n\t+8+va9zLDpeQd5v3VmYoXEm9zCV+qD7K0gM7Yf0G7hf77nfLKUy4ej3m1eu+Qwy/vFNX\n\tg/4n1lrd4x5Y1JUQm8u7odq0SgfZOeXCE/mts=","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=ViFN8LY32cCn+6o0h+8PXR3uZ2n3bxnARn4VfAWB7so=;\n\tb=oVA4wGwDgiSKPu5JY/A1uh2tNG6fBHgt8Z10a2K4GRrIq5kiAy4dKkgbhXniNIeKjN\n\tEEQIaHlPms0/VjrcV1ymHVBi+ggMbG+L6RXs/5JrqX54ojjDBgBJPiAiD2R3v9fh5h4A\n\t6pKxkIpZOUSQubMEdx0ZHhkm07oYQ3qJj6h4rqZN3BqKK0MOZ0+L50B9VoSvWSNmuPGU\n\tyx3Pe80kZpWQdLsoAOXYeWLZfDNaMhqnjTJfBNZE59701jKs4+aH+PkhdJ8vcZoubAob\n\tWcrizs6AmwQmxp1cDqoxWXyXd7ZuikmOT+PzD9oeVAh0nQCst4mKRHR47pcEz5opXNz6\n\tCcyw==","X-Gm-Message-State":"AOAM5325/rK9z3X8DDwv6DrijgHm4eH0xcsSjSizxmPU4jQZxQPv4V1X\n\tb/yAjkaYGJm3Ap3fXRxy2KFpAmnkCwo4TiD9/hX84Q==","X-Google-Smtp-Source":"ABdhPJyT32f3f87CxJUlmViDXUWM6FynnFHuoEFt4EnoYo6/+NMoCUqeM1iChX+/sZ8kOkTmakRbn5ROaetcMJm7kew=","X-Received":"by 2002:a50:8e44:: with SMTP id 4mr4838273edx.244.1620303302877; \n\tThu, 06 May 2021 05:15:02 -0700 (PDT)","MIME-Version":"1.0","References":"<20210506075449.1761752-1-hiroh@chromium.org>\n\t<20210506075449.1761752-3-hiroh@chromium.org>\n\t<20210506091155.jvn3novrsxus3ohl@uno.localdomain>","In-Reply-To":"<20210506091155.jvn3novrsxus3ohl@uno.localdomain>","From":"Hirokazu Honda ","Date":"Thu, 6 May 2021 21:14:52 +0900","Message-ID":"","To":"Jacopo Mondi ","Subject":"Re: [libcamera-devel] [PATCH v4 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 ","Content-Type":"multipart/mixed;\n\tboundary=\"===============6962364945821417174==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16812,"web_url":"https://patchwork.libcamera.org/comment/16812/","msgid":"<20210506125656.qepelgny4blecz6f@uno.localdomain>","date":"2021-05-06T12:56:56","subject":"Re: [libcamera-devel] [PATCH v4 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, May 06, 2021 at 09:14:52PM +0900, Hirokazu Honda wrote:\n> Hi Jacopo,\n>\n> On Thu, May 6, 2021 at 6:11 PM Jacopo Mondi wrote:\n>\n> >\n> > Currently we construct V4L2ControlInfo passing in a\n> > v4l2_query_ext_ctrl.\n> >\n> > Can't we do the same and detect if we have to enumerate indices if the\n> > control type is MENU, instead of creating a new function here ?\n> >\n> >\n> To do that, we need to pass fd to call QUERYMENU.\n\nRight, but as we switch on the type, passing to the constructor an\nadditional parameter doesn't seem that bad.\n\n> I think the direction is opposite. I would remove V4L2CtrlInfo because it\n> just calls a few functions to construct ControlInfo.\n\nAnd we do set/get controls in v4l2 device already...\n\nI see your point, and I would not mind that, v4l2_controls.cpp/h is\npretty slim at the moment and I think it can be moved to the\nv4l2_device. I don't see drawbacks, and I would be pleased to see\nv4l2_controls gone tbh\n\n> What do you think?\n\nI like the idea, but I think you should first move everything to the\nv4l2_device, remove v4l2_controls.cpp/h and then add support for menu.\n\nPlease have this yak which needs a fresh shave it seems\n\n _,,,_\n .-'` ( '.\n .-' ,_ ; \\___ _,\n __.' ) \\'.__.'(:;'.__.'/\n __..--\"\" ( '.__{':');}__.'\n .' ( ; ( .-|` ' |-.\n / ( ) ) '-p q-'\n ( ; ; ; ; |.---.|\n ) ( ( ; \\ o o)\n | ) ; | ) ) /'.__/\n ) ; ) ; | ; //\n ( ) _,\\ ; //\n ; ( ,_,,-~\"\"~`\"\" \\ ( //\n \\_.'\\\\_ '. /<_\n \\\\_)--\\ \\ \\--\\\n )--\\\"\"` )--\\\"`\n `\"\"`\n\nJoking aside, I don't want to throw more issues to solve before\naddresing the issue at hand with v4l2 menu controls, but having some\ncontrols constructed here and some others in v4l2 controls feels\nweird...\n\n>\n> -Hiro\n>\n> > +{\n> > > + ASSERT(ctrl.type == V4L2_CTRL_TYPE_MENU ||\n> > > + ctrl.type == V4L2_CTRL_TYPE_INTEGER_MENU);\n> > > +\n> > > + std::vector indices;\n> > > + v4l2_querymenu menu;\n> > > + memset(&menu, 0, sizeof(menu));\n> > > + menu.id = ctrl.id;\n> > > +\n> > > + for (menu.index = ctrl.minimum;\n> > > + static_cast(menu.index) <= ctrl.maximum; menu.index++) {\n> >\n> > I was about to complain that you could loop until you don't get\n> > EINVAL, but the V4L2 documentation supports what you have here\n> >\n> > \"It is possible for VIDIOC_QUERYMENU to return an EINVAL error code\n> > for some indices between minimum and maximum. In that case that\n> > particular menu item is not supported by this driver. Also note that\n> > the minimum value is not necessarily 0.\"\n> >\n> > > + if (ioctl(VIDIOC_QUERYMENU, &menu) != 0)\n> > > + continue;\n> >\n> > I would however be loud if the return code is !-EINVAL\n> >\n> > > +\n> > > + indices.emplace_back(static_cast(menu.index));\n> >\n> > Has emplicing any benefit compared to push_back for POD like an\n> > int32_t ?\n> >\n> > > + }\n> > > +\n> > > + if (indices.empty()) {\n> > > + LOG(V4L2, Error)\n> > > + << \"No applicable value: \" << utils::hex(ctrl.id);\n> > > +\n> > > + return false;\n> > > + }\n> > > +\n> > > + ctrlInfo = ControlInfo(indices);\n> >\n> > Can't you return the ctrlInfo directly and exploit RVO ? or would it\n> > become difficult to detect errors ? I would howerv consider try moving\n> > this to the V4L2ControlInfo constructor first.\n> >\n> > > +\n> > > + return true;\n> > > +}\n> > >\n> > > /*\n> > > * \\brief List and store information about all controls supported by the\n> > > @@ -473,7 +511,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> > > @@ -484,15 +521,22 @@ void V4L2Device::listControls()\n> > > ctrl.flags & V4L2_CTRL_FLAG_DISABLED)\n> > > continue;\n> > >\n> > > + ControlInfo ctrlInfo;\n> > > switch (ctrl.type) {\n> > > case V4L2_CTRL_TYPE_INTEGER:\n> > > case V4L2_CTRL_TYPE_BOOLEAN:\n> > > - case V4L2_CTRL_TYPE_MENU:\n> > > case V4L2_CTRL_TYPE_BUTTON:\n> > > case V4L2_CTRL_TYPE_INTEGER64:\n> > > case V4L2_CTRL_TYPE_BITMASK:\n> > > - case V4L2_CTRL_TYPE_INTEGER_MENU:\n> > > case V4L2_CTRL_TYPE_U8:\n> > > + ctrlInfo = V4L2ControlInfo(ctrl);\n> > > + break;\n> > > +\n> > > + case V4L2_CTRL_TYPE_INTEGER_MENU:\n> > > + case V4L2_CTRL_TYPE_MENU:\n> > > + if (!createControlInfoForMenu(ctrl, ctrlInfo))\n> > > + continue;\n> > > +\n> > > break;\n> > > /* \\todo Support other control types. */\n> > > default:\n> > > @@ -502,10 +546,13 @@ void V4L2Device::listControls()\n> > > continue;\n> > > }\n> > >\n> > > + LOG(V4L2, Debug) << \"Control: \" << ctrl.name\n> > > + << \" (\" << utils::hex(ctrl.id) << \")\";\n> > > +\n> > >\n> > controlIds_.emplace_back(std::make_unique(ctrl));\n> > > controlInfo_.emplace(ctrl.id, ctrl);\n> > >\n> > > - ctrls.emplace(controlIds_.back().get(),\n> > V4L2ControlInfo(ctrl));\n> > > + ctrls.emplace(controlIds_.back().get(), ctrlInfo);\n> > > }\n> > >\n> > > controls_ = std::move(ctrls);\n> > > @@ -535,6 +582,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> >\n> > We have this here below:\n> >\n> > default:\n> > /*\n> > * \\todo To be changed when support for string\n> > controls\n> > * will be added.\n> > */\n> > value.set(v4l2Ctrl->value);\n> > break;\n> >\n> > wasn't it enough ?\n> >\n> > Thanks\n> > j\n> >\n> > > case ControlTypeByte:\n> > > /*\n> > > * No action required, the VIDIOC_[GS]_EXT_CTRLS\n> > ioctl\n> > > --\n> > > 2.31.1.607.g51e8a6a459-goog\n> > >\n> > > _______________________________________________\n> > > libcamera-devel mailing list\n> > > libcamera-devel@lists.libcamera.org\n> > > https://lists.libcamera.org/listinfo/libcamera-devel\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 1F73CBF825\n\tfor ;\n\tThu, 6 May 2021 12:56:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8D1F96891B;\n\tThu, 6 May 2021 14:56:15 +0200 (CEST)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AAA8768909\n\tfor ;\n\tThu, 6 May 2021 14:56:13 +0200 (CEST)","from uno.localdomain (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 1624FFF807;\n\tThu, 6 May 2021 12:56:12 +0000 (UTC)"],"X-Originating-IP":"93.61.96.190","Date":"Thu, 6 May 2021 14:56:56 +0200","From":"Jacopo Mondi ","To":"Hirokazu Honda ","Message-ID":"<20210506125656.qepelgny4blecz6f@uno.localdomain>","References":"<20210506075449.1761752-1-hiroh@chromium.org>\n\t<20210506075449.1761752-3-hiroh@chromium.org>\n\t<20210506091155.jvn3novrsxus3ohl@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v4 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 ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16816,"web_url":"https://patchwork.libcamera.org/comment/16816/","msgid":"","date":"2021-05-07T02:19:13","subject":"Re: [libcamera-devel] [PATCH v4 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\n\nOn Thu, May 6, 2021 at 9:56 PM Jacopo Mondi wrote:\n\n> Hi Hiro,\n>\n> On Thu, May 06, 2021 at 09:14:52PM +0900, Hirokazu Honda wrote:\n> > Hi Jacopo,\n> >\n> > On Thu, May 6, 2021 at 6:11 PM Jacopo Mondi wrote:\n> >\n> > >\n> > > Currently we construct V4L2ControlInfo passing in a\n> > > v4l2_query_ext_ctrl.\n> > >\n> > > Can't we do the same and detect if we have to enumerate indices if the\n> > > control type is MENU, instead of creating a new function here ?\n> > >\n> > >\n> > To do that, we need to pass fd to call QUERYMENU.\n>\n> Right, but as we switch on the type, passing to the constructor an\n> additional parameter doesn't seem that bad.\n>\n> > I think the direction is opposite. I would remove V4L2CtrlInfo because it\n> > just calls a few functions to construct ControlInfo.\n>\n> And we do set/get controls in v4l2 device already...\n>\n> I see your point, and I would not mind that, v4l2_controls.cpp/h is\n> pretty slim at the moment and I think it can be moved to the\n> v4l2_device. I don't see drawbacks, and I would be pleased to see\n> v4l2_controls gone tbh\n>\n> > What do you think?\n>\n> I like the idea, but I think you should first move everything to the\n> v4l2_device, remove v4l2_controls.cpp/h and then add support for menu.\n>\n> Please have this yak which needs a fresh shave it seems\n>\n> _,,,_\n> .-'` ( '.\n> .-' ,_ ; \\___ _,\n> __.' ) \\'.__.'(:;'.__.'/\n> __..--\"\" ( '.__{':');}__.'\n> .' ( ; ( .-|` ' |-.\n> / ( ) ) '-p q-'\n> ( ; ; ; ; |.---.|\n> ) ( ( ; \\ o o)\n> | ) ; | ) ) /'.__/\n> ) ; ) ; | ; //\n> ( ) _,\\ ; //\n> ; ( ,_,,-~\"\"~`\"\" \\ ( //\n> \\_.'\\\\_ '. /<_\n> \\\\_)--\\ \\ \\--\\\n> )--\\\"\"` )--\\\"`\n> `\"\"`\n>\n> Joking aside, I don't want to throw more issues to solve before\n> addresing the issue at hand with v4l2 menu controls, but having some\n> controls constructed here and some others in v4l2 controls feels\n> weird...\n>\n>\nAlright, I submitted the patch series of removing the classes.\nWith that, I expect this code will not be weird.\n\n-Hiro\n\n>\n> > -Hiro\n> >\n> > > +{\n> > > > + ASSERT(ctrl.type == V4L2_CTRL_TYPE_MENU ||\n> > > > + ctrl.type == V4L2_CTRL_TYPE_INTEGER_MENU);\n> > > > +\n> > > > + std::vector indices;\n> > > > + v4l2_querymenu menu;\n> > > > + memset(&menu, 0, sizeof(menu));\n> > > > + menu.id = ctrl.id;\n> > > > +\n> > > > + for (menu.index = ctrl.minimum;\n> > > > + static_cast(menu.index) <= ctrl.maximum;\n> menu.index++) {\n> > >\n> > > I was about to complain that you could loop until you don't get\n> > > EINVAL, but the V4L2 documentation supports what you have here\n> > >\n> > > \"It is possible for VIDIOC_QUERYMENU to return an EINVAL error code\n> > > for some indices between minimum and maximum. In that case that\n> > > particular menu item is not supported by this driver. Also note that\n> > > the minimum value is not necessarily 0.\"\n> > >\n> > > > + if (ioctl(VIDIOC_QUERYMENU, &menu) != 0)\n> > > > + continue;\n> > >\n> > > I would however be loud if the return code is !-EINVAL\n> > >\n> > > > +\n> > > > + indices.emplace_back(static_cast(menu.index));\n> > >\n> > > Has emplicing any benefit compared to push_back for POD like an\n> > > int32_t ?\n> > >\n> > > > + }\n> > > > +\n> > > > + if (indices.empty()) {\n> > > > + LOG(V4L2, Error)\n> > > > + << \"No applicable value: \" << utils::hex(\n> ctrl.id);\n> > > > +\n> > > > + return false;\n> > > > + }\n> > > > +\n> > > > + ctrlInfo = ControlInfo(indices);\n> > >\n> > > Can't you return the ctrlInfo directly and exploit RVO ? or would it\n> > > become difficult to detect errors ? I would howerv consider try moving\n> > > this to the V4L2ControlInfo constructor first.\n> > >\n> > > > +\n> > > > + return true;\n> > > > +}\n> > > >\n> > > > /*\n> > > > * \\brief List and store information about all controls supported\n> by the\n> > > > @@ -473,7 +511,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> > > > @@ -484,15 +521,22 @@ void V4L2Device::listControls()\n> > > > ctrl.flags & V4L2_CTRL_FLAG_DISABLED)\n> > > > continue;\n> > > >\n> > > > + ControlInfo ctrlInfo;\n> > > > switch (ctrl.type) {\n> > > > case V4L2_CTRL_TYPE_INTEGER:\n> > > > case V4L2_CTRL_TYPE_BOOLEAN:\n> > > > - case V4L2_CTRL_TYPE_MENU:\n> > > > case V4L2_CTRL_TYPE_BUTTON:\n> > > > case V4L2_CTRL_TYPE_INTEGER64:\n> > > > case V4L2_CTRL_TYPE_BITMASK:\n> > > > - case V4L2_CTRL_TYPE_INTEGER_MENU:\n> > > > case V4L2_CTRL_TYPE_U8:\n> > > > + ctrlInfo = V4L2ControlInfo(ctrl);\n> > > > + break;\n> > > > +\n> > > > + case V4L2_CTRL_TYPE_INTEGER_MENU:\n> > > > + case V4L2_CTRL_TYPE_MENU:\n> > > > + if (!createControlInfoForMenu(ctrl, ctrlInfo))\n> > > > + continue;\n> > > > +\n> > > > break;\n> > > > /* \\todo Support other control types. */\n> > > > default:\n> > > > @@ -502,10 +546,13 @@ void V4L2Device::listControls()\n> > > > continue;\n> > > > }\n> > > >\n> > > > + LOG(V4L2, Debug) << \"Control: \" << ctrl.name\n> > > > + << \" (\" << utils::hex(ctrl.id) <<\n> \")\";\n> > > > +\n> > > >\n> > > controlIds_.emplace_back(std::make_unique(ctrl));\n> > > > controlInfo_.emplace(ctrl.id, ctrl);\n> > > >\n> > > > - ctrls.emplace(controlIds_.back().get(),\n> > > V4L2ControlInfo(ctrl));\n> > > > + ctrls.emplace(controlIds_.back().get(), ctrlInfo);\n> > > > }\n> > > >\n> > > > controls_ = std::move(ctrls);\n> > > > @@ -535,6 +582,10 @@ void V4L2Device::updateControls(ControlList\n> *ctrls,\n> > > > value.set(v4l2Ctrl->value64);\n> > > > break;\n> > > >\n> > > > + case ControlTypeInteger32:\n> > > > + value.set(v4l2Ctrl->value);\n> > > > + break;\n> > > > +\n> > >\n> > > We have this here below:\n> > >\n> > > default:\n> > > /*\n> > > * \\todo To be changed when support for string\n> > > controls\n> > > * will be added.\n> > > */\n> > > value.set(v4l2Ctrl->value);\n> > > break;\n> > >\n> > > wasn't it enough ?\n> > >\n> > > Thanks\n> > > j\n> > >\n> > > > case ControlTypeByte:\n> > > > /*\n> > > > * No action required, the\n> VIDIOC_[GS]_EXT_CTRLS\n> > > ioctl\n> > > > --\n> > > > 2.31.1.607.g51e8a6a459-goog\n> > > >\n> > > > _______________________________________________\n> > > > libcamera-devel mailing list\n> > > > libcamera-devel@lists.libcamera.org\n> > > > https://lists.libcamera.org/listinfo/libcamera-devel\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 D8328BF831\n\tfor ;\n\tFri, 7 May 2021 02:19:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 545C668915;\n\tFri, 7 May 2021 04:19:25 +0200 (CEST)","from mail-ed1-x52b.google.com (mail-ed1-x52b.google.com\n\t[IPv6:2a00:1450:4864:20::52b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 210E0602BA\n\tfor ;\n\tFri, 7 May 2021 04:19:24 +0200 (CEST)","by mail-ed1-x52b.google.com with SMTP id y26so8384703eds.4\n\tfor ;\n\tThu, 06 May 2021 19:19:24 -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=\"etX+ixbY\"; 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=1hEnOK4ZGv0hrejqiE6+jsitCzy3y9l4RuA9eKxAcf8=;\n\tb=etX+ixbY0s4ixxZVl1IioFOsFoZeP+xuUUf+vd4pd+m0+/g795rtOTukFN97SFS9Hm\n\tlN8rCtAvHyH4YjTexzDTvjOTsvj5QZiVRZcIo50IaA055L9LMywRG86hvW0YC3fS+B+P\n\tCY9+RSuF+4rc/z/AuBqTxydP1x9F6WfqAgP7E=","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=1hEnOK4ZGv0hrejqiE6+jsitCzy3y9l4RuA9eKxAcf8=;\n\tb=LnzXKvIyfkiW3hwwa3Iipqy/sbJopT2fTU5D29fmQN59toZXLFrmFlA7VUBwevMjom\n\tp1P6nc80FOHydlZRKcrFBUpew9pWLqlzlx2wfbla9lQBI27YzTnJ5FRwjPud4OoR8fc/\n\t5XSieMComOa4EtNYoQ8tSY5t8AvFNyLuILujISmV9wrRl6e/BPH6KIEhr1A/f6PsNMrO\n\tqNZcPfN13dlKcda+Hu6PF1/w37zCWTtYNBMOJ5SNmCHtR8VYa3rJ3MUHGT4Lck6KdZ20\n\t8Y4i3B6RBAbbvl0izZK8nMRXlLmE9lbCGd2xHVYPynKnBFBj7KQmaReRfwwQ4b5LXXlG\n\tSuNg==","X-Gm-Message-State":"AOAM533sLUzpX++UIehFA5ilWhyN4cyp+n1rd9NkbjmxZuyQjLxnN2cB\n\t7d3MIr6spoa9d+E3ig1/ussLW0paCTpjtqwM5NXFX54yQBM=","X-Google-Smtp-Source":"ABdhPJxI/Ed9XVJSgVBOtbo/0FDyYeYRr6EEcOx/s9zaGCHXO5o4JCGEpKsg1L5k0yoryL9oje73w3vp9PDSGyyBCmc=","X-Received":"by 2002:a05:6402:50c6:: with SMTP id\n\th6mr8724060edb.327.1620353963721; \n\tThu, 06 May 2021 19:19:23 -0700 (PDT)","MIME-Version":"1.0","References":"<20210506075449.1761752-1-hiroh@chromium.org>\n\t<20210506075449.1761752-3-hiroh@chromium.org>\n\t<20210506091155.jvn3novrsxus3ohl@uno.localdomain>\n\t\n\t<20210506125656.qepelgny4blecz6f@uno.localdomain>","In-Reply-To":"<20210506125656.qepelgny4blecz6f@uno.localdomain>","From":"Hirokazu Honda ","Date":"Fri, 7 May 2021 11:19:13 +0900","Message-ID":"","To":"Jacopo Mondi ","Subject":"Re: [libcamera-devel] [PATCH v4 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 ","Content-Type":"multipart/mixed;\n\tboundary=\"===============0442440796189963216==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]