[{"id":16161,"web_url":"https://patchwork.libcamera.org/comment/16161/","msgid":"<20210409085537.ziorybz6ynhnkgfz@uno.localdomain>","date":"2021-04-09T08:55:37","subject":"Re: [libcamera-devel] [RFC PATCH 2/5] libcamera: V4L2Subdevice: Add\n\tgetter/setter function for test pattern mode","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hiro,\n\nOn Fri, Apr 09, 2021 at 01:32:05PM +0900, Hirokazu Honda wrote:\n> The supported test pattern modes can be obtained by calling\n> VIDIOC_QUERYMENU to a camera sensor device. This implements the\n> getter and setter functions for the test pattern mode in\n> V4L2SubDevice.\n>\n> Signed-off-by: Hirokazu Honda \n> ---\n> include/libcamera/internal/v4l2_subdevice.h | 5 +\n> src/libcamera/v4l2_subdevice.cpp | 104 ++++++++++++++++++++\n> 2 files changed, 109 insertions(+)\n>\n> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> index d2b9ca55..f2f5d3f6 100644\n> --- a/include/libcamera/internal/v4l2_subdevice.h\n> +++ b/include/libcamera/internal/v4l2_subdevice.h\n> @@ -60,6 +60,9 @@ public:\n> \tint setFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> \t\t Whence whence = ActiveFormat);\n>\n> +\tstd::vector getAvailableTestPatternModes();\n> +\tint setTestPatternMode(int32_t testPatternMode);\n> +\n> \tstatic std::unique_ptr\n> \tfromEntityName(const MediaDevice *media, const std::string &entity);\n>\n> @@ -74,6 +77,8 @@ private:\n> \t\t\t\t\t unsigned int code);\n>\n> \tconst MediaEntity *entity_;\n> +\n> +\tstd::map testPatternModeMap_;\n> };\n>\n> } /* namespace libcamera */\n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index 721ff5a9..130e9c4d 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -24,6 +24,7 @@\n> #include \"libcamera/internal/media_object.h\"\n> #include \"libcamera/internal/utils.h\"\n>\n> +#include \"android/metadata/system/camera_metadata_tags.h\"\n\nI'm afraid this is not correct. We don't want any android specific\ndefinition in libcamera core. libcamera run on non-android systems,\nand we don't want generic application to depend on anything Andoroid.\n\n> /**\n> * \\file v4l2_subdevice.h\n> * \\brief V4L2 Subdevice API\n> @@ -523,4 +524,107 @@ std::vector V4L2Subdevice::enumPadSizes(unsigned int pad,\n> \treturn sizes;\n> }\n>\n> +/**\n> + * \\var V4L2Subdevice::testPatternModeMap_\n> + * \\brief The map from controls::SensorTestPatternMode to an index value to be\n> + * used for V4L2_CID_TEST_PATTERN.\n> + *\n> + * The map is constructed in getAvailableTestPatternModes() and referred in\n> + * setTestPatternMode() to find a value associated with the requested test\n> + * pattern mode.\n> + */\n> +/**\n> + * \\fn V4L2Subdevice::getAvailableTestPatternModes()\n> + * \\brief Retrieve the available test pattern modes.\n> + *\n> + * \\return The available control::SensorTestPatternMode values.\n> + */\n> +std::vector V4L2Subdevice::getAvailableTestPatternModes()\n> +{\n> +\tstd::vector patternModes;\n> +\tif (!testPatternModeMap_.empty()) {\n> +\t\tfor (const auto &mode : testPatternModeMap_)\n> +\t\t\tpatternModes.push_back(mode.first);\n> +\t\treturn patternModes;\n> +\t}\n> +\n> +\tv4l2_queryctrl ctrl;\n> +\tmemset(&ctrl, 0, sizeof(ctrl));\n> +\tctrl.id = V4L2_CID_TEST_PATTERN;\n> +\tint ret = ioctl(VIDIOC_QUERYCTRL, &ctrl);\n\nI'm not sure if you have considered the following and found any\nblocker, but:\n\n- Controls are enumerated in V4L2Device::listControls() with\n VIDIOC_QUERY_EXT_CTRL\n- Menu controls are accepted but not really handled there yet. I think\n you should modify listControls() to handle menu controls properly\n and store them as ControlInfo. ControlInfo currently support being\n constructed with a Span<> of values but as far as I can tell\n V4l2ControlInfo does not.\n- Once you have valid ControlInfo for the menu control, it should\n contain the V4L2 identifers for the menu entries\n- The pipeline handler should then populate the libcamera controls in\n Camera::controls_ by inspecting the V4L2 controls returned by the\n v4l2 subdevice as we do today in IPU3::listControls(). You should\n use the libcamera controls identifiers that you have defined in\n patch #1, not the android defined values\n- The camera HAL inspects the Camera::controls() to translate from\n libcamera defined controls to Android defined metadata\n\nDoes this make sense to you ?\n\nThanks\n j\n\n> +\tif (ret < 0) {\n> +\t\tLOG(V4L2, Error) << \"Unable to get test pattern mode :\"\n> +\t\t\t\t << strerror(-ret);\n> +\t\treturn {};\n> +\t}\n\n> +\n> +\tv4l2_querymenu menu;\n> +\tmemset(&menu, 0, sizeof(menu));\n> +\tmenu.id = ctrl.id;\n> +\tfor (menu.index = ctrl.minimum;\n> +\t static_cast(menu.index) <= ctrl.maximum; menu.index++) {\n> +\t\tif (ioctl(VIDIOC_QUERYMENU, &menu) != 0) {\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tconst std::string modeName(reinterpret_cast(menu.name));\n> +\t\tstd::optional androidTestPatternMode;\n> +\t\t/*\n> +\t\t * ov13858 and ov5670.\n> +\t\t * No corresponding value for \"Vertical Color Bar Type 3\" and\n> +\t\t * \"Vertical Color Bar Type 4\".\n> +\t\t */\n> +\t\tif (modeName == \"Disabled\") {\n> +\t\t\tandroidTestPatternMode =\n> +\t\t\t\tANDROID_SENSOR_TEST_PATTERN_MODE_OFF;\n> +\t\t} else if (modeName == \"Vertical Color Bar Type 1\") {\n> +\t\t\tandroidTestPatternMode =\n> +\t\t\t\tANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS;\n> +\t\t} else if (modeName == \"Vertical Color Bar Type 2\") {\n> +\t\t\tandroidTestPatternMode =\n> +\t\t\t\tANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS_FADE_TO_GRAY;\n> +\t\t}\n> +\n> +\t\tif (androidTestPatternMode) {\n> +\t\t\ttestPatternModeMap_[*androidTestPatternMode] = menu.index;\n> +\t\t\tpatternModes.push_back(*androidTestPatternMode);\n> +\t\t} else {\n> +\t\t\tLOG(V4L2, Debug) << \"Skip test pattern mode: \"\n> +\t\t\t\t\t << modeName;\n> +\t\t}\n> +\t}\n> +\n> +\treturn patternModes;\n> +}\n> +\n> +/**\n> + * \\fn V4L2Subdevice::getAvailableTestPatternModes()\n> + * \\brief Set the test pattern mode.\n> + *\n> + * \\return 0 on success or a negative error code otherwise.\n> + */\n> +int V4L2Subdevice::setTestPatternMode(int32_t testPatternMode)\n> +{\n> +\tauto it = testPatternModeMap_.find(testPatternMode);\n> +\tif (it != testPatternModeMap_.end()) {\n> +\t\tLOG(V4L2, Error) << \"Unsupported test pattern mode: \"\n> +\t\t\t\t << testPatternMode;\n> +\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tv4l2_control ctrl;\n> +\tmemset(&ctrl, 0, sizeof(ctrl));\n> +\tctrl.id = V4L2_CID_TEST_PATTERN;\n> +\tctrl.value = it->second;\n> +\tint ret = ioctl(VIDIOC_S_CTRL, &ctrl);\n> +\tif (ret < 0) {\n> +\t\tLOG(V4L2, Error) << \"Unable to set test pattern mode: \"\n> +\t\t\t\t << strerror(-ret);\n> +\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> } /* namespace libcamera */\n> --\n> 2.31.1.295.g9ea45b61b8-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 293BDBD16B\n\tfor ;\n\tFri, 9 Apr 2021 08:55:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 85D37687F8;\n\tFri, 9 Apr 2021 10:55:01 +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 803D0687D6\n\tfor ;\n\tFri, 9 Apr 2021 10:55:00 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id D4612FF811;\n\tFri, 9 Apr 2021 08:54:59 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Fri, 9 Apr 2021 10:55:37 +0200","From":"Jacopo Mondi ","To":"Hirokazu Honda ","Message-ID":"<20210409085537.ziorybz6ynhnkgfz@uno.localdomain>","References":"<20210409043208.1823330-1-hiroh@chromium.org>\n\t<20210409043208.1823330-3-hiroh@chromium.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210409043208.1823330-3-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [RFC PATCH 2/5] libcamera: V4L2Subdevice: Add\n\tgetter/setter function for test pattern mode","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":16179,"web_url":"https://patchwork.libcamera.org/comment/16179/","msgid":"","date":"2021-04-12T13:12:07","subject":"Re: [libcamera-devel] [RFC PATCH 2/5] libcamera: V4L2Subdevice: Add\n\tgetter/setter function for test pattern mode","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Jacopo, thanks for reviewing.\n\nOn Fri, Apr 9, 2021 at 5:55 PM Jacopo Mondi wrote:\n>\n> Hi Hiro,\n>\n> On Fri, Apr 09, 2021 at 01:32:05PM +0900, Hirokazu Honda wrote:\n> > The supported test pattern modes can be obtained by calling\n> > VIDIOC_QUERYMENU to a camera sensor device. This implements the\n> > getter and setter functions for the test pattern mode in\n> > V4L2SubDevice.\n> >\n> > Signed-off-by: Hirokazu Honda \n> > ---\n> > include/libcamera/internal/v4l2_subdevice.h | 5 +\n> > src/libcamera/v4l2_subdevice.cpp | 104 ++++++++++++++++++++\n> > 2 files changed, 109 insertions(+)\n> >\n> > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> > index d2b9ca55..f2f5d3f6 100644\n> > --- a/include/libcamera/internal/v4l2_subdevice.h\n> > +++ b/include/libcamera/internal/v4l2_subdevice.h\n> > @@ -60,6 +60,9 @@ public:\n> > int setFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> > Whence whence = ActiveFormat);\n> >\n> > + std::vector getAvailableTestPatternModes();\n> > + int setTestPatternMode(int32_t testPatternMode);\n> > +\n> > static std::unique_ptr\n> > fromEntityName(const MediaDevice *media, const std::string &entity);\n> >\n> > @@ -74,6 +77,8 @@ private:\n> > unsigned int code);\n> >\n> > const MediaEntity *entity_;\n> > +\n> > + std::map testPatternModeMap_;\n> > };\n> >\n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > index 721ff5a9..130e9c4d 100644\n> > --- a/src/libcamera/v4l2_subdevice.cpp\n> > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > @@ -24,6 +24,7 @@\n> > #include \"libcamera/internal/media_object.h\"\n> > #include \"libcamera/internal/utils.h\"\n> >\n> > +#include \"android/metadata/system/camera_metadata_tags.h\"\n>\n> I'm afraid this is not correct. We don't want any android specific\n> definition in libcamera core. libcamera run on non-android systems,\n> and we don't want generic application to depend on anything Andoroid.\n>\n> > /**\n> > * \\file v4l2_subdevice.h\n> > * \\brief V4L2 Subdevice API\n> > @@ -523,4 +524,107 @@ std::vector V4L2Subdevice::enumPadSizes(unsigned int pad,\n> > return sizes;\n> > }\n> >\n> > +/**\n> > + * \\var V4L2Subdevice::testPatternModeMap_\n> > + * \\brief The map from controls::SensorTestPatternMode to an index value to be\n> > + * used for V4L2_CID_TEST_PATTERN.\n> > + *\n> > + * The map is constructed in getAvailableTestPatternModes() and referred in\n> > + * setTestPatternMode() to find a value associated with the requested test\n> > + * pattern mode.\n> > + */\n> > +/**\n> > + * \\fn V4L2Subdevice::getAvailableTestPatternModes()\n> > + * \\brief Retrieve the available test pattern modes.\n> > + *\n> > + * \\return The available control::SensorTestPatternMode values.\n> > + */\n> > +std::vector V4L2Subdevice::getAvailableTestPatternModes()\n> > +{\n> > + std::vector patternModes;\n> > + if (!testPatternModeMap_.empty()) {\n> > + for (const auto &mode : testPatternModeMap_)\n> > + patternModes.push_back(mode.first);\n> > + return patternModes;\n> > + }\n> > +\n> > + v4l2_queryctrl ctrl;\n> > + memset(&ctrl, 0, sizeof(ctrl));\n> > + ctrl.id = V4L2_CID_TEST_PATTERN;\n> > + int ret = ioctl(VIDIOC_QUERYCTRL, &ctrl);\n>\n> I'm not sure if you have considered the following and found any\n> blocker, but:\n>\n> - Controls are enumerated in V4L2Device::listControls() with\n> VIDIOC_QUERY_EXT_CTRL\n> - Menu controls are accepted but not really handled there yet. I think\n> you should modify listControls() to handle menu controls properly\n> and store them as ControlInfo. ControlInfo currently support being\n> constructed with a Span<> of values but as far as I can tell\n> V4l2ControlInfo does not.\n> - Once you have valid ControlInfo for the menu control, it should\n> contain the V4L2 identifers for the menu entries\n> - The pipeline handler should then populate the libcamera controls in\n> Camera::controls_ by inspecting the V4L2 controls returned by the\n> v4l2 subdevice as we do today in IPU3::listControls(). You should\n> use the libcamera controls identifiers that you have defined in\n> patch #1, not the android defined values\n> - The camera HAL inspects the Camera::controls() to translate from\n> libcamera defined controls to Android defined metadata\n>\n> Does this make sense to you ?\n>\n\nThat sounds good to me.\nHowever, do you think it makes more sense to add available test\npattern modes to CameraSensorDataBase?\nThe reason is, as you saw in this patch, there is no way of mapping to\nV4L2_CID_TEST_PATTERN value to android one.\nThis is a problem in v4l2 api. An app has to know beforehand what test\npattern mode the name returned by a driver represents.\n\nWhat do you think?\n-Hiro\n\n> Thanks\n> j\n>\n> > + if (ret < 0) {\n> > + LOG(V4L2, Error) << \"Unable to get test pattern mode :\"\n> > + << strerror(-ret);\n> > + return {};\n> > + }\n>\n> > +\n> > + v4l2_querymenu menu;\n> > + memset(&menu, 0, sizeof(menu));\n> > + menu.id = ctrl.id;\n> > + for (menu.index = ctrl.minimum;\n> > + static_cast(menu.index) <= ctrl.maximum; menu.index++) {\n> > + if (ioctl(VIDIOC_QUERYMENU, &menu) != 0) {\n> > + continue;\n> > + }\n> > +\n> > + const std::string modeName(reinterpret_cast(menu.name));\n> > + std::optional androidTestPatternMode;\n> > + /*\n> > + * ov13858 and ov5670.\n> > + * No corresponding value for \"Vertical Color Bar Type 3\" and\n> > + * \"Vertical Color Bar Type 4\".\n> > + */\n> > + if (modeName == \"Disabled\") {\n> > + androidTestPatternMode =\n> > + ANDROID_SENSOR_TEST_PATTERN_MODE_OFF;\n> > + } else if (modeName == \"Vertical Color Bar Type 1\") {\n> > + androidTestPatternMode =\n> > + ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS;\n> > + } else if (modeName == \"Vertical Color Bar Type 2\") {\n> > + androidTestPatternMode =\n> > + ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS_FADE_TO_GRAY;\n> > + }\n> > +\n> > + if (androidTestPatternMode) {\n> > + testPatternModeMap_[*androidTestPatternMode] = menu.index;\n> > + patternModes.push_back(*androidTestPatternMode);\n> > + } else {\n> > + LOG(V4L2, Debug) << \"Skip test pattern mode: \"\n> > + << modeName;\n> > + }\n> > + }\n> > +\n> > + return patternModes;\n> > +}\n> > +\n> > +/**\n> > + * \\fn V4L2Subdevice::getAvailableTestPatternModes()\n> > + * \\brief Set the test pattern mode.\n> > + *\n> > + * \\return 0 on success or a negative error code otherwise.\n> > + */\n> > +int V4L2Subdevice::setTestPatternMode(int32_t testPatternMode)\n> > +{\n> > + auto it = testPatternModeMap_.find(testPatternMode);\n> > + if (it != testPatternModeMap_.end()) {\n> > + LOG(V4L2, Error) << \"Unsupported test pattern mode: \"\n> > + << testPatternMode;\n> > +\n> > + return -EINVAL;\n> > + }\n> > +\n> > + v4l2_control ctrl;\n> > + memset(&ctrl, 0, sizeof(ctrl));\n> > + ctrl.id = V4L2_CID_TEST_PATTERN;\n> > + ctrl.value = it->second;\n> > + int ret = ioctl(VIDIOC_S_CTRL, &ctrl);\n> > + if (ret < 0) {\n> > + LOG(V4L2, Error) << \"Unable to set test pattern mode: \"\n> > + << strerror(-ret);\n> > +\n> > + return -EINVAL;\n> > + }\n> > +\n> > + return 0;\n> > +}\n> > } /* namespace libcamera */\n> > --\n> > 2.31.1.295.g9ea45b61b8-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 B0110BD224\n\tfor ;\n\tMon, 12 Apr 2021 13:12:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2093C687F3;\n\tMon, 12 Apr 2021 15:12:21 +0200 (CEST)","from mail-ed1-x529.google.com (mail-ed1-x529.google.com\n\t[IPv6:2a00:1450:4864:20::529])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 18A22602C8\n\tfor ;\n\tMon, 12 Apr 2021 15:12:20 +0200 (CEST)","by mail-ed1-x529.google.com with SMTP id x4so15042815edd.2\n\tfor ;\n\tMon, 12 Apr 2021 06:12:20 -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=\"Jrd3mCIy\"; 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=1UN3kersX7bGVAXvSu2Degzwm3qWRkOsYc9mbaUH8o4=;\n\tb=Jrd3mCIyTqwFo39MxCleKeEsOJyC5FXifPf9wK2JL7xsSJl+mqd66sG8Cpz1t6X7eh\n\tIWAbwBcYXH2wkqOQtvZC9oowHNny4ah/a/3zS/oKEP2a05Za8/xRvyySFtwmZOXQ4gR3\n\tQVbMzZgby6el7fL3hPVuxOw62dsUL1X7McupY=","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=1UN3kersX7bGVAXvSu2Degzwm3qWRkOsYc9mbaUH8o4=;\n\tb=mW00E+A0VlhJTM6fRPv0+Vey5CxWm8FdRcEX1/8D/h2oRseq0+Rl+WC8i3W3SdPpMe\n\t6YrtCaG/c6ZgtIfKjFSNs5T9DfbF9Hl9JUZIvVVlZ3RRjp+tu43Qs4yoYhHZpCuhnylU\n\twcwpc4POZnxBdMSicnP8uUbuV0/MLAx2wZWtuF8Ao0wPoHtiSCvT+dLbsOv3+SlrU/pK\n\tVu4Hod7rY0SEJRO5Gh7xObB/b+oAe4Evfz9NJnTOFOZxEiQMhKps7GCQSJ3lcDz1ru41\n\tPteWMN+0pmrd3EeWqHQG0l5AfYIsXmQdjNXXFf79byupm8sZIQju9AlhFUWMwfYk3JvV\n\tyLpA==","X-Gm-Message-State":"AOAM5325eUwcRyS1MBlTSpsnjU2I2HHpoN556rQxidTNAG5wr79g362p\n\txEns5ZEDIXDf3gROAc1A9HfXPA5MfiDwSsna/fMxAg==","X-Google-Smtp-Source":"ABdhPJzj0Mar/klRq3CZ6PLXZNkVY9KBbCEtqchvpx7aKhhN7CnwAv41GH0DHW1zYyg/NyWnvpFZv6M9wd88Bq57jwo=","X-Received":"by 2002:a05:6402:34cd:: with SMTP id\n\tw13mr29708564edc.73.1618233139741; \n\tMon, 12 Apr 2021 06:12:19 -0700 (PDT)","MIME-Version":"1.0","References":"<20210409043208.1823330-1-hiroh@chromium.org>\n\t<20210409043208.1823330-3-hiroh@chromium.org>\n\t<20210409085537.ziorybz6ynhnkgfz@uno.localdomain>","In-Reply-To":"<20210409085537.ziorybz6ynhnkgfz@uno.localdomain>","From":"Hirokazu Honda ","Date":"Mon, 12 Apr 2021 22:12:07 +0900","Message-ID":"","To":"Jacopo Mondi ","Subject":"Re: [libcamera-devel] [RFC PATCH 2/5] libcamera: V4L2Subdevice: Add\n\tgetter/setter function for test pattern mode","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":16181,"web_url":"https://patchwork.libcamera.org/comment/16181/","msgid":"<20210412133217.5a6gurqmskf6hpx3@uno.localdomain>","date":"2021-04-12T13:32:17","subject":"Re: [libcamera-devel] [RFC PATCH 2/5] libcamera: V4L2Subdevice: Add\n\tgetter/setter function for test pattern mode","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hiro,\n\nOn Mon, Apr 12, 2021 at 10:12:07PM +0900, Hirokazu Honda wrote:\n> Hi Jacopo, thanks for reviewing.\n>\n> On Fri, Apr 9, 2021 at 5:55 PM Jacopo Mondi wrote:\n> >\n> > Hi Hiro,\n> >\n> > On Fri, Apr 09, 2021 at 01:32:05PM +0900, Hirokazu Honda wrote:\n> > > The supported test pattern modes can be obtained by calling\n> > > VIDIOC_QUERYMENU to a camera sensor device. This implements the\n> > > getter and setter functions for the test pattern mode in\n> > > V4L2SubDevice.\n> > >\n> > > Signed-off-by: Hirokazu Honda \n> > > ---\n> > > include/libcamera/internal/v4l2_subdevice.h | 5 +\n> > > src/libcamera/v4l2_subdevice.cpp | 104 ++++++++++++++++++++\n> > > 2 files changed, 109 insertions(+)\n> > >\n> > > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> > > index d2b9ca55..f2f5d3f6 100644\n> > > --- a/include/libcamera/internal/v4l2_subdevice.h\n> > > +++ b/include/libcamera/internal/v4l2_subdevice.h\n> > > @@ -60,6 +60,9 @@ public:\n> > > int setFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> > > Whence whence = ActiveFormat);\n> > >\n> > > + std::vector getAvailableTestPatternModes();\n> > > + int setTestPatternMode(int32_t testPatternMode);\n> > > +\n> > > static std::unique_ptr\n> > > fromEntityName(const MediaDevice *media, const std::string &entity);\n> > >\n> > > @@ -74,6 +77,8 @@ private:\n> > > unsigned int code);\n> > >\n> > > const MediaEntity *entity_;\n> > > +\n> > > + std::map testPatternModeMap_;\n> > > };\n> > >\n> > > } /* namespace libcamera */\n> > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > > index 721ff5a9..130e9c4d 100644\n> > > --- a/src/libcamera/v4l2_subdevice.cpp\n> > > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > > @@ -24,6 +24,7 @@\n> > > #include \"libcamera/internal/media_object.h\"\n> > > #include \"libcamera/internal/utils.h\"\n> > >\n> > > +#include \"android/metadata/system/camera_metadata_tags.h\"\n> >\n> > I'm afraid this is not correct. We don't want any android specific\n> > definition in libcamera core. libcamera run on non-android systems,\n> > and we don't want generic application to depend on anything Andoroid.\n> >\n> > > /**\n> > > * \\file v4l2_subdevice.h\n> > > * \\brief V4L2 Subdevice API\n> > > @@ -523,4 +524,107 @@ std::vector V4L2Subdevice::enumPadSizes(unsigned int pad,\n> > > return sizes;\n> > > }\n> > >\n> > > +/**\n> > > + * \\var V4L2Subdevice::testPatternModeMap_\n> > > + * \\brief The map from controls::SensorTestPatternMode to an index value to be\n> > > + * used for V4L2_CID_TEST_PATTERN.\n> > > + *\n> > > + * The map is constructed in getAvailableTestPatternModes() and referred in\n> > > + * setTestPatternMode() to find a value associated with the requested test\n> > > + * pattern mode.\n> > > + */\n> > > +/**\n> > > + * \\fn V4L2Subdevice::getAvailableTestPatternModes()\n> > > + * \\brief Retrieve the available test pattern modes.\n> > > + *\n> > > + * \\return The available control::SensorTestPatternMode values.\n> > > + */\n> > > +std::vector V4L2Subdevice::getAvailableTestPatternModes()\n> > > +{\n> > > + std::vector patternModes;\n> > > + if (!testPatternModeMap_.empty()) {\n> > > + for (const auto &mode : testPatternModeMap_)\n> > > + patternModes.push_back(mode.first);\n> > > + return patternModes;\n> > > + }\n> > > +\n> > > + v4l2_queryctrl ctrl;\n> > > + memset(&ctrl, 0, sizeof(ctrl));\n> > > + ctrl.id = V4L2_CID_TEST_PATTERN;\n> > > + int ret = ioctl(VIDIOC_QUERYCTRL, &ctrl);\n> >\n> > I'm not sure if you have considered the following and found any\n> > blocker, but:\n> >\n> > - Controls are enumerated in V4L2Device::listControls() with\n> > VIDIOC_QUERY_EXT_CTRL\n> > - Menu controls are accepted but not really handled there yet. I think\n> > you should modify listControls() to handle menu controls properly\n> > and store them as ControlInfo. ControlInfo currently support being\n> > constructed with a Span<> of values but as far as I can tell\n> > V4l2ControlInfo does not.\n> > - Once you have valid ControlInfo for the menu control, it should\n> > contain the V4L2 identifers for the menu entries\n> > - The pipeline handler should then populate the libcamera controls in\n> > Camera::controls_ by inspecting the V4L2 controls returned by the\n> > v4l2 subdevice as we do today in IPU3::listControls(). You should\n> > use the libcamera controls identifiers that you have defined in\n> > patch #1, not the android defined values\n> > - The camera HAL inspects the Camera::controls() to translate from\n> > libcamera defined controls to Android defined metadata\n> >\n> > Does this make sense to you ?\n> >\n>\n> That sounds good to me.\n> However, do you think it makes more sense to add available test\n> pattern modes to CameraSensorDataBase?\n\nDo you mean recording the V4L2 test pattern modes there ? What would we gain\ncompared to fetching them from the kernel interface ?\n\nIf you mean the Android test pattern mode then no, the sensor database\nis a core libcamera construct, it knows nothing about Android. One\noption for Android-specific values is instead the HAL configuration\nfile.\n\n> The reason is, as you saw in this patch, there is no way of mapping to\n> V4L2_CID_TEST_PATTERN value to android one.\n> This is a problem in v4l2 api. An app has to know beforehand what test\n> pattern mode the name returned by a driver represents.\n\nAh do you mean that the test patter names are driver specific ? Good\njob V4L2! I see your point there. I won't be ashamed of having them in\nthe HAL configuration file, or to perform a best-effort mapping the\nCamera HAL. Not sure what's best tbh\n\n>\n> What do you think?\n> -Hiro\n>\n> > Thanks\n> > j\n> >\n> > > + if (ret < 0) {\n> > > + LOG(V4L2, Error) << \"Unable to get test pattern mode :\"\n> > > + << strerror(-ret);\n> > > + return {};\n> > > + }\n> >\n> > > +\n> > > + v4l2_querymenu menu;\n> > > + memset(&menu, 0, sizeof(menu));\n> > > + menu.id = ctrl.id;\n> > > + for (menu.index = ctrl.minimum;\n> > > + static_cast(menu.index) <= ctrl.maximum; menu.index++) {\n> > > + if (ioctl(VIDIOC_QUERYMENU, &menu) != 0) {\n> > > + continue;\n> > > + }\n> > > +\n> > > + const std::string modeName(reinterpret_cast(menu.name));\n> > > + std::optional androidTestPatternMode;\n> > > + /*\n> > > + * ov13858 and ov5670.\n> > > + * No corresponding value for \"Vertical Color Bar Type 3\" and\n> > > + * \"Vertical Color Bar Type 4\".\n> > > + */\n> > > + if (modeName == \"Disabled\") {\n> > > + androidTestPatternMode =\n> > > + ANDROID_SENSOR_TEST_PATTERN_MODE_OFF;\n> > > + } else if (modeName == \"Vertical Color Bar Type 1\") {\n> > > + androidTestPatternMode =\n> > > + ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS;\n> > > + } else if (modeName == \"Vertical Color Bar Type 2\") {\n> > > + androidTestPatternMode =\n> > > + ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS_FADE_TO_GRAY;\n> > > + }\n> > > +\n> > > + if (androidTestPatternMode) {\n> > > + testPatternModeMap_[*androidTestPatternMode] = menu.index;\n> > > + patternModes.push_back(*androidTestPatternMode);\n> > > + } else {\n> > > + LOG(V4L2, Debug) << \"Skip test pattern mode: \"\n> > > + << modeName;\n> > > + }\n> > > + }\n> > > +\n> > > + return patternModes;\n> > > +}\n> > > +\n> > > +/**\n> > > + * \\fn V4L2Subdevice::getAvailableTestPatternModes()\n> > > + * \\brief Set the test pattern mode.\n> > > + *\n> > > + * \\return 0 on success or a negative error code otherwise.\n> > > + */\n> > > +int V4L2Subdevice::setTestPatternMode(int32_t testPatternMode)\n> > > +{\n> > > + auto it = testPatternModeMap_.find(testPatternMode);\n> > > + if (it != testPatternModeMap_.end()) {\n> > > + LOG(V4L2, Error) << \"Unsupported test pattern mode: \"\n> > > + << testPatternMode;\n> > > +\n> > > + return -EINVAL;\n> > > + }\n> > > +\n> > > + v4l2_control ctrl;\n> > > + memset(&ctrl, 0, sizeof(ctrl));\n> > > + ctrl.id = V4L2_CID_TEST_PATTERN;\n> > > + ctrl.value = it->second;\n> > > + int ret = ioctl(VIDIOC_S_CTRL, &ctrl);\n> > > + if (ret < 0) {\n> > > + LOG(V4L2, Error) << \"Unable to set test pattern mode: \"\n> > > + << strerror(-ret);\n> > > +\n> > > + return -EINVAL;\n> > > + }\n> > > +\n> > > + return 0;\n> > > +}\n> > > } /* namespace libcamera */\n> > > --\n> > > 2.31.1.295.g9ea45b61b8-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 0DD95BD1F6\n\tfor ;\n\tMon, 12 Apr 2021 13:31:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8A488687F3;\n\tMon, 12 Apr 2021 15:31:40 +0200 (CEST)","from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 12A22602C8\n\tfor ;\n\tMon, 12 Apr 2021 15:31:39 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 8BFF91C0006;\n\tMon, 12 Apr 2021 13:31:38 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Mon, 12 Apr 2021 15:32:17 +0200","From":"Jacopo Mondi ","To":"Hirokazu Honda ","Message-ID":"<20210412133217.5a6gurqmskf6hpx3@uno.localdomain>","References":"<20210409043208.1823330-1-hiroh@chromium.org>\n\t<20210409043208.1823330-3-hiroh@chromium.org>\n\t<20210409085537.ziorybz6ynhnkgfz@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH 2/5] libcamera: V4L2Subdevice: Add\n\tgetter/setter function for test pattern mode","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":16182,"web_url":"https://patchwork.libcamera.org/comment/16182/","msgid":"","date":"2021-04-12T13:38:54","subject":"Re: [libcamera-devel] [RFC PATCH 2/5] libcamera: V4L2Subdevice: Add\n\tgetter/setter function for test pattern mode","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Jacopo,\n\nOn Mon, Apr 12, 2021 at 10:31 PM Jacopo Mondi wrote:\n>\n> Hi Hiro,\n>\n> On Mon, Apr 12, 2021 at 10:12:07PM +0900, Hirokazu Honda wrote:\n> > Hi Jacopo, thanks for reviewing.\n> >\n> > On Fri, Apr 9, 2021 at 5:55 PM Jacopo Mondi wrote:\n> > >\n> > > Hi Hiro,\n> > >\n> > > On Fri, Apr 09, 2021 at 01:32:05PM +0900, Hirokazu Honda wrote:\n> > > > The supported test pattern modes can be obtained by calling\n> > > > VIDIOC_QUERYMENU to a camera sensor device. This implements the\n> > > > getter and setter functions for the test pattern mode in\n> > > > V4L2SubDevice.\n> > > >\n> > > > Signed-off-by: Hirokazu Honda \n> > > > ---\n> > > > include/libcamera/internal/v4l2_subdevice.h | 5 +\n> > > > src/libcamera/v4l2_subdevice.cpp | 104 ++++++++++++++++++++\n> > > > 2 files changed, 109 insertions(+)\n> > > >\n> > > > diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> > > > index d2b9ca55..f2f5d3f6 100644\n> > > > --- a/include/libcamera/internal/v4l2_subdevice.h\n> > > > +++ b/include/libcamera/internal/v4l2_subdevice.h\n> > > > @@ -60,6 +60,9 @@ public:\n> > > > int setFormat(unsigned int pad, V4L2SubdeviceFormat *format,\n> > > > Whence whence = ActiveFormat);\n> > > >\n> > > > + std::vector getAvailableTestPatternModes();\n> > > > + int setTestPatternMode(int32_t testPatternMode);\n> > > > +\n> > > > static std::unique_ptr\n> > > > fromEntityName(const MediaDevice *media, const std::string &entity);\n> > > >\n> > > > @@ -74,6 +77,8 @@ private:\n> > > > unsigned int code);\n> > > >\n> > > > const MediaEntity *entity_;\n> > > > +\n> > > > + std::map testPatternModeMap_;\n> > > > };\n> > > >\n> > > > } /* namespace libcamera */\n> > > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > > > index 721ff5a9..130e9c4d 100644\n> > > > --- a/src/libcamera/v4l2_subdevice.cpp\n> > > > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > > > @@ -24,6 +24,7 @@\n> > > > #include \"libcamera/internal/media_object.h\"\n> > > > #include \"libcamera/internal/utils.h\"\n> > > >\n> > > > +#include \"android/metadata/system/camera_metadata_tags.h\"\n> > >\n> > > I'm afraid this is not correct. We don't want any android specific\n> > > definition in libcamera core. libcamera run on non-android systems,\n> > > and we don't want generic application to depend on anything Andoroid.\n> > >\n> > > > /**\n> > > > * \\file v4l2_subdevice.h\n> > > > * \\brief V4L2 Subdevice API\n> > > > @@ -523,4 +524,107 @@ std::vector V4L2Subdevice::enumPadSizes(unsigned int pad,\n> > > > return sizes;\n> > > > }\n> > > >\n> > > > +/**\n> > > > + * \\var V4L2Subdevice::testPatternModeMap_\n> > > > + * \\brief The map from controls::SensorTestPatternMode to an index value to be\n> > > > + * used for V4L2_CID_TEST_PATTERN.\n> > > > + *\n> > > > + * The map is constructed in getAvailableTestPatternModes() and referred in\n> > > > + * setTestPatternMode() to find a value associated with the requested test\n> > > > + * pattern mode.\n> > > > + */\n> > > > +/**\n> > > > + * \\fn V4L2Subdevice::getAvailableTestPatternModes()\n> > > > + * \\brief Retrieve the available test pattern modes.\n> > > > + *\n> > > > + * \\return The available control::SensorTestPatternMode values.\n> > > > + */\n> > > > +std::vector V4L2Subdevice::getAvailableTestPatternModes()\n> > > > +{\n> > > > + std::vector patternModes;\n> > > > + if (!testPatternModeMap_.empty()) {\n> > > > + for (const auto &mode : testPatternModeMap_)\n> > > > + patternModes.push_back(mode.first);\n> > > > + return patternModes;\n> > > > + }\n> > > > +\n> > > > + v4l2_queryctrl ctrl;\n> > > > + memset(&ctrl, 0, sizeof(ctrl));\n> > > > + ctrl.id = V4L2_CID_TEST_PATTERN;\n> > > > + int ret = ioctl(VIDIOC_QUERYCTRL, &ctrl);\n> > >\n> > > I'm not sure if you have considered the following and found any\n> > > blocker, but:\n> > >\n> > > - Controls are enumerated in V4L2Device::listControls() with\n> > > VIDIOC_QUERY_EXT_CTRL\n> > > - Menu controls are accepted but not really handled there yet. I think\n> > > you should modify listControls() to handle menu controls properly\n> > > and store them as ControlInfo. ControlInfo currently support being\n> > > constructed with a Span<> of values but as far as I can tell\n> > > V4l2ControlInfo does not.\n> > > - Once you have valid ControlInfo for the menu control, it should\n> > > contain the V4L2 identifers for the menu entries\n> > > - The pipeline handler should then populate the libcamera controls in\n> > > Camera::controls_ by inspecting the V4L2 controls returned by the\n> > > v4l2 subdevice as we do today in IPU3::listControls(). You should\n> > > use the libcamera controls identifiers that you have defined in\n> > > patch #1, not the android defined values\n> > > - The camera HAL inspects the Camera::controls() to translate from\n> > > libcamera defined controls to Android defined metadata\n> > >\n> > > Does this make sense to you ?\n> > >\n> >\n> > That sounds good to me.\n> > However, do you think it makes more sense to add available test\n> > pattern modes to CameraSensorDataBase?\n>\n> Do you mean recording the V4L2 test pattern modes there ? What would we gain\n> compared to fetching them from the kernel interface ?\n>\n> If you mean the Android test pattern mode then no, the sensor database\n> is a core libcamera construct, it knows nothing about Android. One\n> option for Android-specific values is instead the HAL configuration\n> file.\n>\n\nHmm, so should we have our own test pattern mode definition a part of\nwhich are equivalent to some useful Android test pattern modes?\nThen the definitions are converted to Android ones in HAL configuration?\n\n> > The reason is, as you saw in this patch, there is no way of mapping to\n> > V4L2_CID_TEST_PATTERN value to android one.\n> > This is a problem in v4l2 api. An app has to know beforehand what test\n> > pattern mode the name returned by a driver represents.\n>\n> Ah do you mean that the test patter names are driver specific ? Good\n> job V4L2! I see your point there. I won't be ashamed of having them in\n> the HAL configuration file, or to perform a best-effort mapping the\n> Camera HAL. Not sure what's best tbh\n>\n\nRight. The available test pattern modes are not device specific, like location.\nSo I think it is more natural to put the info to CameraSensorDatabase.\n\n-Hiro\n\n> >\n> > What do you think?\n> > -Hiro\n> >\n> > > Thanks\n> > > j\n> > >\n> > > > + if (ret < 0) {\n> > > > + LOG(V4L2, Error) << \"Unable to get test pattern mode :\"\n> > > > + << strerror(-ret);\n> > > > + return {};\n> > > > + }\n> > >\n> > > > +\n> > > > + v4l2_querymenu menu;\n> > > > + memset(&menu, 0, sizeof(menu));\n> > > > + menu.id = ctrl.id;\n> > > > + for (menu.index = ctrl.minimum;\n> > > > + static_cast(menu.index) <= ctrl.maximum; menu.index++) {\n> > > > + if (ioctl(VIDIOC_QUERYMENU, &menu) != 0) {\n> > > > + continue;\n> > > > + }\n> > > > +\n> > > > + const std::string modeName(reinterpret_cast(menu.name));\n> > > > + std::optional androidTestPatternMode;\n> > > > + /*\n> > > > + * ov13858 and ov5670.\n> > > > + * No corresponding value for \"Vertical Color Bar Type 3\" and\n> > > > + * \"Vertical Color Bar Type 4\".\n> > > > + */\n> > > > + if (modeName == \"Disabled\") {\n> > > > + androidTestPatternMode =\n> > > > + ANDROID_SENSOR_TEST_PATTERN_MODE_OFF;\n> > > > + } else if (modeName == \"Vertical Color Bar Type 1\") {\n> > > > + androidTestPatternMode =\n> > > > + ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS;\n> > > > + } else if (modeName == \"Vertical Color Bar Type 2\") {\n> > > > + androidTestPatternMode =\n> > > > + ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS_FADE_TO_GRAY;\n> > > > + }\n> > > > +\n> > > > + if (androidTestPatternMode) {\n> > > > + testPatternModeMap_[*androidTestPatternMode] = menu.index;\n> > > > + patternModes.push_back(*androidTestPatternMode);\n> > > > + } else {\n> > > > + LOG(V4L2, Debug) << \"Skip test pattern mode: \"\n> > > > + << modeName;\n> > > > + }\n> > > > + }\n> > > > +\n> > > > + return patternModes;\n> > > > +}\n> > > > +\n> > > > +/**\n> > > > + * \\fn V4L2Subdevice::getAvailableTestPatternModes()\n> > > > + * \\brief Set the test pattern mode.\n> > > > + *\n> > > > + * \\return 0 on success or a negative error code otherwise.\n> > > > + */\n> > > > +int V4L2Subdevice::setTestPatternMode(int32_t testPatternMode)\n> > > > +{\n> > > > + auto it = testPatternModeMap_.find(testPatternMode);\n> > > > + if (it != testPatternModeMap_.end()) {\n> > > > + LOG(V4L2, Error) << \"Unsupported test pattern mode: \"\n> > > > + << testPatternMode;\n> > > > +\n> > > > + return -EINVAL;\n> > > > + }\n> > > > +\n> > > > + v4l2_control ctrl;\n> > > > + memset(&ctrl, 0, sizeof(ctrl));\n> > > > + ctrl.id = V4L2_CID_TEST_PATTERN;\n> > > > + ctrl.value = it->second;\n> > > > + int ret = ioctl(VIDIOC_S_CTRL, &ctrl);\n> > > > + if (ret < 0) {\n> > > > + LOG(V4L2, Error) << \"Unable to set test pattern mode: \"\n> > > > + << strerror(-ret);\n> > > > +\n> > > > + return -EINVAL;\n> > > > + }\n> > > > +\n> > > > + return 0;\n> > > > +}\n> > > > } /* namespace libcamera */\n> > > > --\n> > > > 2.31.1.295.g9ea45b61b8-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 01649BD224\n\tfor ;\n\tMon, 12 Apr 2021 13:39:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 58355602CB;\n\tMon, 12 Apr 2021 15:39:08 +0200 (CEST)","from mail-ed1-x533.google.com (mail-ed1-x533.google.com\n\t[IPv6:2a00:1450:4864:20::533])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F0DC9602C8\n\tfor ;\n\tMon, 12 Apr 2021 15:39:06 +0200 (CEST)","by mail-ed1-x533.google.com with SMTP id h10so15089990edt.13\n\tfor ;\n\tMon, 12 Apr 2021 06:39:06 -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=\"e1Io2AAU\"; 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=D0rVPiVS+08qMBuNOOOUJQZzXc9/Lvc2nTmO6RikdCA=;\n\tb=e1Io2AAU9xCYSic7BxqZxuM9n2HCblXI1uzpFiZIrwko0VEfsZ2hcFoWcrnz1LCzaT\n\tlblPbVs9TtH9hwgxy0oRmUeUyF+U7NoRffA2tRRwrke3GUpKaxaBUMRbr570ooVOfNDH\n\tGS2i4ZtuVgvLiE20n6cmjy0Hm1UwGJR4PxV7k=","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=D0rVPiVS+08qMBuNOOOUJQZzXc9/Lvc2nTmO6RikdCA=;\n\tb=t8atQT1trAr3wcMNH5ZPOHJ8sgiow1rsDeFrxOfLqjic1PXP8AJzSPTpq3bX3hCo9g\n\tyIE1hyFqePeZYrG5QG+NWQSfRB+Vue0SPKMx09NMriDQ+xt3c+bERcajEDYBMsEGrQE+\n\tJa8g1EZ3iCDIC6SIHcpMP0dkHmMRAZP1XclrRr8zSa7LS+JXdtp9DUs9UWhQJL+uG+lN\n\tPxHxLAimQORhiaSMfy7/SoW2eiUEy01LaQtuNPt5iBSRy5CTWIQiSQTK19EthpMd2x1k\n\txKuwR90uFDC2Sn0vT5rpQrhk+k12aetAVU3al9WNVc2yelLYMjV9L0W4RbFqLT2hYgif\n\twcCA==","X-Gm-Message-State":"AOAM532Jk0OvJClkrIYD+tZJlgyB2ClDWeF01ECXs9zF/0jeewlptqpf\n\t7NlMBgxp6Dj6N2TaNPaPVBMMLo4tVuuYPvHtDyF1xQ==","X-Google-Smtp-Source":"ABdhPJzSn/4IEys7MPPYSWrnSbV4WOAnuEwF8f5rnZ31pCvqzIzbElGSqcdEoiyKggPVFEE6zduD0dNaQRTlJ1JeYyM=","X-Received":"by 2002:a05:6402:5211:: with SMTP id\n\ts17mr29765481edd.327.1618234746607; \n\tMon, 12 Apr 2021 06:39:06 -0700 (PDT)","MIME-Version":"1.0","References":"<20210409043208.1823330-1-hiroh@chromium.org>\n\t<20210409043208.1823330-3-hiroh@chromium.org>\n\t<20210409085537.ziorybz6ynhnkgfz@uno.localdomain>\n\t\n\t<20210412133217.5a6gurqmskf6hpx3@uno.localdomain>","In-Reply-To":"<20210412133217.5a6gurqmskf6hpx3@uno.localdomain>","From":"Hirokazu Honda ","Date":"Mon, 12 Apr 2021 22:38:54 +0900","Message-ID":"","To":"Jacopo Mondi ","Subject":"Re: [libcamera-devel] [RFC PATCH 2/5] libcamera: V4L2Subdevice: Add\n\tgetter/setter function for test pattern mode","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":16183,"web_url":"https://patchwork.libcamera.org/comment/16183/","msgid":"<20210412135002.2gh6pjomaxvmaun6@uno.localdomain>","date":"2021-04-12T13:50:02","subject":"Re: [libcamera-devel] [RFC PATCH 2/5] libcamera: V4L2Subdevice: Add\n\tgetter/setter function for test pattern mode","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hiro,\n\nOn Mon, Apr 12, 2021 at 10:38:54PM +0900, Hirokazu Honda wrote:\n> Hi Jacopo,\n>\n> On Mon, Apr 12, 2021 at 10:31 PM Jacopo Mondi wrote:\n> >\n> > Hi Hiro,\n> >\n\n[snip]\n\n> > > > > +/**\n> > > > > + * \\fn V4L2Subdevice::getAvailableTestPatternModes()\n> > > > > + * \\brief Retrieve the available test pattern modes.\n> > > > > + *\n> > > > > + * \\return The available control::SensorTestPatternMode values.\n> > > > > + */\n> > > > > +std::vector V4L2Subdevice::getAvailableTestPatternModes()\n> > > > > +{\n> > > > > + std::vector patternModes;\n> > > > > + if (!testPatternModeMap_.empty()) {\n> > > > > + for (const auto &mode : testPatternModeMap_)\n> > > > > + patternModes.push_back(mode.first);\n> > > > > + return patternModes;\n> > > > > + }\n> > > > > +\n> > > > > + v4l2_queryctrl ctrl;\n> > > > > + memset(&ctrl, 0, sizeof(ctrl));\n> > > > > + ctrl.id = V4L2_CID_TEST_PATTERN;\n> > > > > + int ret = ioctl(VIDIOC_QUERYCTRL, &ctrl);\n> > > >\n> > > > I'm not sure if you have considered the following and found any\n> > > > blocker, but:\n> > > >\n> > > > - Controls are enumerated in V4L2Device::listControls() with\n> > > > VIDIOC_QUERY_EXT_CTRL\n> > > > - Menu controls are accepted but not really handled there yet. I think\n> > > > you should modify listControls() to handle menu controls properly\n> > > > and store them as ControlInfo. ControlInfo currently support being\n> > > > constructed with a Span<> of values but as far as I can tell\n> > > > V4l2ControlInfo does not.\n> > > > - Once you have valid ControlInfo for the menu control, it should\n> > > > contain the V4L2 identifers for the menu entries\n> > > > - The pipeline handler should then populate the libcamera controls in\n> > > > Camera::controls_ by inspecting the V4L2 controls returned by the\n> > > > v4l2 subdevice as we do today in IPU3::listControls(). You should\n> > > > use the libcamera controls identifiers that you have defined in\n> > > > patch #1, not the android defined values\n> > > > - The camera HAL inspects the Camera::controls() to translate from\n> > > > libcamera defined controls to Android defined metadata\n> > > >\n> > > > Does this make sense to you ?\n> > > >\n> > >\n> > > That sounds good to me.\n> > > However, do you think it makes more sense to add available test\n> > > pattern modes to CameraSensorDataBase?\n> >\n> > Do you mean recording the V4L2 test pattern modes there ? What would we gain\n> > compared to fetching them from the kernel interface ?\n> >\n> > If you mean the Android test pattern mode then no, the sensor database\n> > is a core libcamera construct, it knows nothing about Android. One\n> > option for Android-specific values is instead the HAL configuration\n> > file.\n> >\n>\n> Hmm, so should we have our own test pattern mode definition a part of\n> which are equivalent to some useful Android test pattern modes?\n\nIf you mean recording them in the HAL configuration file you can\nrecord the android values (or better, their numeric values). There is\nnot need to have them defined as libcamera controls if we have no\nstrict need to do so.\n\n> Then the definitions are converted to Android ones in HAL configuration?\n>\n\nThat would happen if the Camera HAL has to convert from the libcamera\ncontrols to android ones. It really depends if libcamera wants a\ncontrol for the test pattern modes.\n\nCC-ed Laurent\n\n> > > The reason is, as you saw in this patch, there is no way of mapping to\n> > > V4L2_CID_TEST_PATTERN value to android one.\n> > > This is a problem in v4l2 api. An app has to know beforehand what test\n> > > pattern mode the name returned by a driver represents.\n> >\n> > Ah do you mean that the test patter names are driver specific ? Good\n> > job V4L2! I see your point there. I won't be ashamed of having them in\n> > the HAL configuration file, or to perform a best-effort mapping the\n> > Camera HAL. Not sure what's best tbh\n> >\n>\n> Right. The available test pattern modes are not device specific, like location.\n\nWell, not really. Location has a set of values it can be assigned to.\nThe test pattern modes, if I got you right are free formed text, which\nmakes it very hard to translate them to a fixed set of values like the\nones android defines.\n\n> So I think it is more natural to put the info to CameraSensorDatabase.\n>\n\nI still don't get what you would record in the sensor database, maybe\nI'm still asleep :)\n\nCould you provide an example ?\n\nThanks\n j\n\n> -Hiro\n>\n> > >\n> > > What do you think?\n> > > -Hiro\n> > >\n> > > > Thanks\n> > > > j\n> > > >\n> > > > > + if (ret < 0) {\n> > > > > + LOG(V4L2, Error) << \"Unable to get test pattern mode :\"\n> > > > > + << strerror(-ret);\n> > > > > + return {};\n> > > > > + }\n> > > >\n> > > > > +\n> > > > > + v4l2_querymenu menu;\n> > > > > + memset(&menu, 0, sizeof(menu));\n> > > > > + menu.id = ctrl.id;\n> > > > > + for (menu.index = ctrl.minimum;\n> > > > > + static_cast(menu.index) <= ctrl.maximum; menu.index++) {\n> > > > > + if (ioctl(VIDIOC_QUERYMENU, &menu) != 0) {\n> > > > > + continue;\n> > > > > + }\n> > > > > +\n> > > > > + const std::string modeName(reinterpret_cast(menu.name));\n> > > > > + std::optional androidTestPatternMode;\n> > > > > + /*\n> > > > > + * ov13858 and ov5670.\n> > > > > + * No corresponding value for \"Vertical Color Bar Type 3\" and\n> > > > > + * \"Vertical Color Bar Type 4\".\n> > > > > + */\n> > > > > + if (modeName == \"Disabled\") {\n> > > > > + androidTestPatternMode =\n> > > > > + ANDROID_SENSOR_TEST_PATTERN_MODE_OFF;\n> > > > > + } else if (modeName == \"Vertical Color Bar Type 1\") {\n> > > > > + androidTestPatternMode =\n> > > > > + ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS;\n> > > > > + } else if (modeName == \"Vertical Color Bar Type 2\") {\n> > > > > + androidTestPatternMode =\n> > > > > + ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS_FADE_TO_GRAY;\n> > > > > + }\n> > > > > +\n> > > > > + if (androidTestPatternMode) {\n> > > > > + testPatternModeMap_[*androidTestPatternMode] = menu.index;\n> > > > > + patternModes.push_back(*androidTestPatternMode);\n> > > > > + } else {\n> > > > > + LOG(V4L2, Debug) << \"Skip test pattern mode: \"\n> > > > > + << modeName;\n> > > > > + }\n> > > > > + }\n> > > > > +\n> > > > > + return patternModes;\n> > > > > +}\n> > > > > +\n> > > > > +/**\n> > > > > + * \\fn V4L2Subdevice::getAvailableTestPatternModes()\n> > > > > + * \\brief Set the test pattern mode.\n> > > > > + *\n> > > > > + * \\return 0 on success or a negative error code otherwise.\n> > > > > + */\n> > > > > +int V4L2Subdevice::setTestPatternMode(int32_t testPatternMode)\n> > > > > +{\n> > > > > + auto it = testPatternModeMap_.find(testPatternMode);\n> > > > > + if (it != testPatternModeMap_.end()) {\n> > > > > + LOG(V4L2, Error) << \"Unsupported test pattern mode: \"\n> > > > > + << testPatternMode;\n> > > > > +\n> > > > > + return -EINVAL;\n> > > > > + }\n> > > > > +\n> > > > > + v4l2_control ctrl;\n> > > > > + memset(&ctrl, 0, sizeof(ctrl));\n> > > > > + ctrl.id = V4L2_CID_TEST_PATTERN;\n> > > > > + ctrl.value = it->second;\n> > > > > + int ret = ioctl(VIDIOC_S_CTRL, &ctrl);\n> > > > > + if (ret < 0) {\n> > > > > + LOG(V4L2, Error) << \"Unable to set test pattern mode: \"\n> > > > > + << strerror(-ret);\n> > > > > +\n> > > > > + return -EINVAL;\n> > > > > + }\n> > > > > +\n> > > > > + return 0;\n> > > > > +}\n> > > > > } /* namespace libcamera */\n> > > > > --\n> > > > > 2.31.1.295.g9ea45b61b8-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 EAAD5BD1F6\n\tfor ;\n\tMon, 12 Apr 2021 13:49:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 796FF687FE;\n\tMon, 12 Apr 2021 15:49:25 +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 7C99A602C8\n\tfor ;\n\tMon, 12 Apr 2021 15:49:24 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id D8189FF811;\n\tMon, 12 Apr 2021 13:49:23 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Mon, 12 Apr 2021 15:50:02 +0200","From":"Jacopo Mondi ","To":"Hirokazu Honda ","Message-ID":"<20210412135002.2gh6pjomaxvmaun6@uno.localdomain>","References":"<20210409043208.1823330-1-hiroh@chromium.org>\n\t<20210409043208.1823330-3-hiroh@chromium.org>\n\t<20210409085537.ziorybz6ynhnkgfz@uno.localdomain>\n\t\n\t<20210412133217.5a6gurqmskf6hpx3@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [RFC PATCH 2/5] libcamera: V4L2Subdevice: Add\n\tgetter/setter function for test pattern mode","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":16189,"web_url":"https://patchwork.libcamera.org/comment/16189/","msgid":"<6d1ae418-f009-9d8c-8b61-335d3124c990@ideasonboard.com>","date":"2021-04-12T19:45:35","subject":"Re: [libcamera-devel] [RFC PATCH 2/5] libcamera: V4L2Subdevice: Add\n\tgetter/setter function for test pattern mode","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Jacopo, Hiro,\n\nOn 12/04/2021 14:50, Jacopo Mondi wrote:\n> Hi Hiro,\n> \n> On Mon, Apr 12, 2021 at 10:38:54PM +0900, Hirokazu Honda wrote:\n>> Hi Jacopo,\n>>\n>> On Mon, Apr 12, 2021 at 10:31 PM Jacopo Mondi wrote:\n>>>\n>>> Hi Hiro,\n>>>\n> \n> [snip]\n> \n>>>>>> +/**\n>>>>>> + * \\fn V4L2Subdevice::getAvailableTestPatternModes()\n>>>>>> + * \\brief Retrieve the available test pattern modes.\n>>>>>> + *\n>>>>>> + * \\return The available control::SensorTestPatternMode values.\n>>>>>> + */\n>>>>>> +std::vector V4L2Subdevice::getAvailableTestPatternModes()\n>>>>>> +{\n>>>>>> + std::vector patternModes;\n>>>>>> + if (!testPatternModeMap_.empty()) {\n>>>>>> + for (const auto &mode : testPatternModeMap_)\n>>>>>> + patternModes.push_back(mode.first);\n>>>>>> + return patternModes;\n>>>>>> + }\n>>>>>> +\n>>>>>> + v4l2_queryctrl ctrl;\n>>>>>> + memset(&ctrl, 0, sizeof(ctrl));\n>>>>>> + ctrl.id = V4L2_CID_TEST_PATTERN;\n>>>>>> + int ret = ioctl(VIDIOC_QUERYCTRL, &ctrl);\n>>>>>\n>>>>> I'm not sure if you have considered the following and found any\n>>>>> blocker, but:\n>>>>>\n>>>>> - Controls are enumerated in V4L2Device::listControls() with\n>>>>> VIDIOC_QUERY_EXT_CTRL\n>>>>> - Menu controls are accepted but not really handled there yet. I think\n>>>>> you should modify listControls() to handle menu controls properly\n>>>>> and store them as ControlInfo. ControlInfo currently support being\n>>>>> constructed with a Span<> of values but as far as I can tell\n>>>>> V4l2ControlInfo does not.\n>>>>> - Once you have valid ControlInfo for the menu control, it should\n>>>>> contain the V4L2 identifers for the menu entries\n>>>>> - The pipeline handler should then populate the libcamera controls in\n>>>>> Camera::controls_ by inspecting the V4L2 controls returned by the\n>>>>> v4l2 subdevice as we do today in IPU3::listControls(). You should\n>>>>> use the libcamera controls identifiers that you have defined in\n>>>>> patch #1, not the android defined values\n>>>>> - The camera HAL inspects the Camera::controls() to translate from\n>>>>> libcamera defined controls to Android defined metadata\n>>>>>\n>>>>> Does this make sense to you ?\n>>>>>\n>>>>\n>>>> That sounds good to me.\n>>>> However, do you think it makes more sense to add available test\n>>>> pattern modes to CameraSensorDataBase?\n>>>\n>>> Do you mean recording the V4L2 test pattern modes there ? What would we gain\n>>> compared to fetching them from the kernel interface ?\n>>>\n>>> If you mean the Android test pattern mode then no, the sensor database\n>>> is a core libcamera construct, it knows nothing about Android. One\n>>> option for Android-specific values is instead the HAL configuration\n>>> file.\n>>>\n>>\n>> Hmm, so should we have our own test pattern mode definition a part of\n>> which are equivalent to some useful Android test pattern modes?\n> \n> If you mean recording them in the HAL configuration file you can\n> record the android values (or better, their numeric values). There is\n> not need to have them defined as libcamera controls if we have no\n> strict need to do so.\n> \n>> Then the definitions are converted to Android ones in HAL configuration?\n>>\n> \n> That would happen if the Camera HAL has to convert from the libcamera\n> controls to android ones. It really depends if libcamera wants a\n> control for the test pattern modes.\n\nPersonally, I think test patterns are a feature of the camera, and if\navailable should be exposed (as a libcamera control).\n\nIndeed the difficulty might be mapping the menu type options to specific\nlibcamera controls though in a generic way.\n\n\n\n\n> CC-ed Laurent\n> \n>>>> The reason is, as you saw in this patch, there is no way of mapping to\n>>>> V4L2_CID_TEST_PATTERN value to android one.\n>>>> This is a problem in v4l2 api. An app has to know beforehand what test\n>>>> pattern mode the name returned by a driver represents.\n>>>\n>>> Ah do you mean that the test patter names are driver specific ? Good\n>>> job V4L2! I see your point there. I won't be ashamed of having them in\n>>> the HAL configuration file, or to perform a best-effort mapping the\n>>> Camera HAL. Not sure what's best tbh\n>>>\n>>\n>> Right. The available test pattern modes are not device specific, like location.\n> \n> Well, not really. Location has a set of values it can be assigned to.\n> The test pattern modes, if I got you right are free formed text, which\n> makes it very hard to translate them to a fixed set of values like the\n> ones android defines.\n> \n>> So I think it is more natural to put the info to CameraSensorDatabase.\n>>\n> \n> I still don't get what you would record in the sensor database, maybe\n> I'm still asleep :)\n\nPerhaps there is some mapping of custom menu items to libcamera controls\nrequired:\n\n> v4l2-ctl -d /dev/v4l-subdev0 --list-ctrls\n> VIMC Controls\n> \n> test_pattern 0x00f0f000 (menu) : min=0 max=21 default=0 value=0\n\n> v4l2-ctl -d /dev/v4l-subdev0 --list-ctrls-menus\n> VIMC Controls\n> \n> test_pattern 0x00f0f000 (menu) : min=0 max=21 default=0 value=0\n> \t\t\t\t0: 75% Colorbar\n> \t\t\t\t1: 100% Colorbar\n> \t\t\t\t2: CSC Colorbar\n> \t\t\t\t3: Horizontal 100% Colorbar\n> \t\t\t\t4: 100% Color Squares\n> \t\t\t\t5: 100% Black\n> \t\t\t\t6: 100% White\n> \t\t\t\t7: 100% Red\n> \t\t\t\t8: 100% Green\n> \t\t\t\t9: 100% Blue\n> \t\t\t\t10: 16x16 Checkers\n> \t\t\t\t11: 2x2 Checkers\n> \t\t\t\t12: 1x1 Checkers\n> \t\t\t\t13: 2x2 Red/Green Checkers\n> \t\t\t\t14: 1x1 Red/Green Checkers\n> \t\t\t\t15: Alternating Hor Lines\n> \t\t\t\t16: Alternating Vert Lines\n> \t\t\t\t17: One Pixel Wide Cross\n> \t\t\t\t18: Two Pixels Wide Cross\n> \t\t\t\t19: Ten Pixels Wide Cross\n> \t\t\t\t20: Gray Ramp\n> \t\t\t\t21: Noise\n\n\nSomehow we would have to map which of those is appropriate for a\nspecific Android test pattern?\n\nBut that should certainly be done in the android layer - not the\nlibcamera layer ...\n\nThis is not looking very easy to make generic :-(\n\n\n\n> Could you provide an example ?\n> \n> Thanks\n> j\n> \n>> -Hiro\n>>\n>>>>\n>>>> What do you think?\n>>>> -Hiro\n>>>>\n>>>>> Thanks\n>>>>> j\n>>>>>\n>>>>>> + if (ret < 0) {\n>>>>>> + LOG(V4L2, Error) << \"Unable to get test pattern mode :\"\n>>>>>> + << strerror(-ret);\n>>>>>> + return {};\n>>>>>> + }\n>>>>>\n>>>>>> +\n>>>>>> + v4l2_querymenu menu;\n>>>>>> + memset(&menu, 0, sizeof(menu));\n>>>>>> + menu.id = ctrl.id;\n>>>>>> + for (menu.index = ctrl.minimum;\n>>>>>> + static_cast(menu.index) <= ctrl.maximum; menu.index++) {\n>>>>>> + if (ioctl(VIDIOC_QUERYMENU, &menu) != 0) {\n>>>>>> + continue;\n>>>>>> + }\n>>>>>> +\n>>>>>> + const std::string modeName(reinterpret_cast(menu.name));\n>>>>>> + std::optional androidTestPatternMode;\n>>>>>> + /*\n>>>>>> + * ov13858 and ov5670.\n>>>>>> + * No corresponding value for \"Vertical Color Bar Type 3\" and\n>>>>>> + * \"Vertical Color Bar Type 4\".\n>>>>>> + */\n>>>>>> + if (modeName == \"Disabled\") {\n>>>>>> + androidTestPatternMode =\n>>>>>> + ANDROID_SENSOR_TEST_PATTERN_MODE_OFF;\n>>>>>> + } else if (modeName == \"Vertical Color Bar Type 1\") {\n>>>>>> + androidTestPatternMode =\n>>>>>> + ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS;\n>>>>>> + } else if (modeName == \"Vertical Color Bar Type 2\") {\n>>>>>> + androidTestPatternMode =\n>>>>>> + ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS_FADE_TO_GRAY;\n>>>>>> + }\n>>>>>> +\n>>>>>> + if (androidTestPatternMode) {\n>>>>>> + testPatternModeMap_[*androidTestPatternMode] = menu.index;\n>>>>>> + patternModes.push_back(*androidTestPatternMode);\n>>>>>> + } else {\n>>>>>> + LOG(V4L2, Debug) << \"Skip test pattern mode: \"\n>>>>>> + << modeName;\n>>>>>> + }\n>>>>>> + }\n>>>>>> +\n>>>>>> + return patternModes;\n>>>>>> +}\n>>>>>> +\n>>>>>> +/**\n>>>>>> + * \\fn V4L2Subdevice::getAvailableTestPatternModes()\n>>>>>> + * \\brief Set the test pattern mode.\n>>>>>> + *\n>>>>>> + * \\return 0 on success or a negative error code otherwise.\n>>>>>> + */\n>>>>>> +int V4L2Subdevice::setTestPatternMode(int32_t testPatternMode)\n>>>>>> +{\n>>>>>> + auto it = testPatternModeMap_.find(testPatternMode);\n>>>>>> + if (it != testPatternModeMap_.end()) {\n>>>>>> + LOG(V4L2, Error) << \"Unsupported test pattern mode: \"\n>>>>>> + << testPatternMode;\n>>>>>> +\n>>>>>> + return -EINVAL;\n>>>>>> + }\n>>>>>> +\n>>>>>> + v4l2_control ctrl;\n>>>>>> + memset(&ctrl, 0, sizeof(ctrl));\n>>>>>> + ctrl.id = V4L2_CID_TEST_PATTERN;\n>>>>>> + ctrl.value = it->second;\n>>>>>> + int ret = ioctl(VIDIOC_S_CTRL, &ctrl);\n>>>>>> + if (ret < 0) {\n>>>>>> + LOG(V4L2, Error) << \"Unable to set test pattern mode: \"\n>>>>>> + << strerror(-ret);\n>>>>>> +\n>>>>>> + return -EINVAL;\n>>>>>> + }\n>>>>>> +\n>>>>>> + return 0;\n>>>>>> +}\n>>>>>> } /* namespace libcamera */\n>>>>>> --\n>>>>>> 2.31.1.295.g9ea45b61b8-goog\n>>>>>>\n>>>>>> _______________________________________________\n>>>>>> libcamera-devel mailing list\n>>>>>> libcamera-devel@lists.libcamera.org\n>>>>>> https://lists.libcamera.org/listinfo/libcamera-devel\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 BEAA5BD224\n\tfor ;\n\tMon, 12 Apr 2021 19:45:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 26E9D687F6;\n\tMon, 12 Apr 2021 21:45:40 +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 3C9BE602CD\n\tfor ;\n\tMon, 12 Apr 2021 21:45:39 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9B0EA3F0;\n\tMon, 12 Apr 2021 21:45:38 +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=\"sPB25NXt\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1618256738;\n\tbh=fkS8MLuWTeOkCVnmsC5ESNYOdVgojbbQmh0tcRmgw60=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=sPB25NXtYNerMSqoa43mOAcx+CMfaBFaGmGNl0MqmmGQ/Cgd0PSMurGxeohA5VmZj\n\tzDnkVOX4jVYqG7/Dtx/6P6JF64SlphPJ7VWmdQvD0rSOFNih2Jd3OngUVNLB0A77Xk\n\tu3xQTppoB+jA+sYWIfAL3Tz6N9WEg1z1KcFAvpc4=","To":"Jacopo Mondi , Hirokazu Honda ","References":"<20210409043208.1823330-1-hiroh@chromium.org>\n\t<20210409043208.1823330-3-hiroh@chromium.org>\n\t<20210409085537.ziorybz6ynhnkgfz@uno.localdomain>\n\t\n\t<20210412133217.5a6gurqmskf6hpx3@uno.localdomain>\n\t\n\t<20210412135002.2gh6pjomaxvmaun6@uno.localdomain>","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<6d1ae418-f009-9d8c-8b61-335d3124c990@ideasonboard.com>","Date":"Mon, 12 Apr 2021 20:45:35 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20210412135002.2gh6pjomaxvmaun6@uno.localdomain>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [RFC PATCH 2/5] libcamera: V4L2Subdevice: Add\n\tgetter/setter function for test pattern mode","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","Reply-To":"kieran.bingham@ideasonboard.com","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":16198,"web_url":"https://patchwork.libcamera.org/comment/16198/","msgid":"","date":"2021-04-13T00:54:16","subject":"Re: [libcamera-devel] [RFC PATCH 2/5] libcamera: V4L2Subdevice: Add\n\tgetter/setter function for test pattern mode","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Mon, Apr 12, 2021 at 08:45:35PM +0100, Kieran Bingham wrote:\n> On 12/04/2021 14:50, Jacopo Mondi wrote:\n> > On Mon, Apr 12, 2021 at 10:38:54PM +0900, Hirokazu Honda wrote:\n> >> On Mon, Apr 12, 2021 at 10:31 PM Jacopo Mondi wrote:\n> > \n> > [snip]\n> > \n> >>>>>> +/**\n> >>>>>> + * \\fn V4L2Subdevice::getAvailableTestPatternModes()\n> >>>>>> + * \\brief Retrieve the available test pattern modes.\n> >>>>>> + *\n> >>>>>> + * \\return The available control::SensorTestPatternMode values.\n> >>>>>> + */\n> >>>>>> +std::vector V4L2Subdevice::getAvailableTestPatternModes()\n> >>>>>> +{\n> >>>>>> + std::vector patternModes;\n> >>>>>> + if (!testPatternModeMap_.empty()) {\n> >>>>>> + for (const auto &mode : testPatternModeMap_)\n> >>>>>> + patternModes.push_back(mode.first);\n> >>>>>> + return patternModes;\n> >>>>>> + }\n> >>>>>> +\n> >>>>>> + v4l2_queryctrl ctrl;\n> >>>>>> + memset(&ctrl, 0, sizeof(ctrl));\n> >>>>>> + ctrl.id = V4L2_CID_TEST_PATTERN;\n> >>>>>> + int ret = ioctl(VIDIOC_QUERYCTRL, &ctrl);\n> >>>>>\n> >>>>> I'm not sure if you have considered the following and found any\n> >>>>> blocker, but:\n> >>>>>\n> >>>>> - Controls are enumerated in V4L2Device::listControls() with\n> >>>>> VIDIOC_QUERY_EXT_CTRL\n> >>>>> - Menu controls are accepted but not really handled there yet. I think\n> >>>>> you should modify listControls() to handle menu controls properly\n> >>>>> and store them as ControlInfo. ControlInfo currently support being\n> >>>>> constructed with a Span<> of values but as far as I can tell\n> >>>>> V4l2ControlInfo does not.\n> >>>>> - Once you have valid ControlInfo for the menu control, it should\n> >>>>> contain the V4L2 identifers for the menu entries\n> >>>>> - The pipeline handler should then populate the libcamera controls in\n> >>>>> Camera::controls_ by inspecting the V4L2 controls returned by the\n> >>>>> v4l2 subdevice as we do today in IPU3::listControls(). You should\n> >>>>> use the libcamera controls identifiers that you have defined in\n> >>>>> patch #1, not the android defined values\n> >>>>> - The camera HAL inspects the Camera::controls() to translate from\n> >>>>> libcamera defined controls to Android defined metadata\n> >>>>>\n> >>>>> Does this make sense to you ?\n> >>>>\n> >>>> That sounds good to me.\n> >>>> However, do you think it makes more sense to add available test\n> >>>> pattern modes to CameraSensorDataBase?\n> >>>\n> >>> Do you mean recording the V4L2 test pattern modes there ? What would we gain\n> >>> compared to fetching them from the kernel interface ?\n> >>>\n> >>> If you mean the Android test pattern mode then no, the sensor database\n> >>> is a core libcamera construct, it knows nothing about Android. One\n> >>> option for Android-specific values is instead the HAL configuration\n> >>> file.\n> >>\n> >> Hmm, so should we have our own test pattern mode definition a part of\n> >> which are equivalent to some useful Android test pattern modes?\n> > \n> > If you mean recording them in the HAL configuration file you can\n> > record the android values (or better, their numeric values). There is\n> > not need to have them defined as libcamera controls if we have no\n> > strict need to do so.\n> > \n> >> Then the definitions are converted to Android ones in HAL configuration?\n> > \n> > That would happen if the Camera HAL has to convert from the libcamera\n> > controls to android ones. It really depends if libcamera wants a\n> > control for the test pattern modes.\n> \n> Personally, I think test patterns are a feature of the camera, and if\n> available should be exposed (as a libcamera control).\n\nAgreed. Conversion to Android camera metadata values should happen in\nthe HAL (assuming the numerical values defined by the libcamera control\nwould differ from the Android values).\n\n> Indeed the difficulty might be mapping the menu type options to specific\n> libcamera controls though in a generic way.\n> \n> > CC-ed Laurent\n> > \n> >>>> The reason is, as you saw in this patch, there is no way of mapping to\n> >>>> V4L2_CID_TEST_PATTERN value to android one.\n> >>>> This is a problem in v4l2 api. An app has to know beforehand what test\n> >>>> pattern mode the name returned by a driver represents.\n> >>>\n> >>> Ah do you mean that the test patter names are driver specific ? Good\n> >>> job V4L2! I see your point there. I won't be ashamed of having them in\n> >>> the HAL configuration file, or to perform a best-effort mapping the\n> >>> Camera HAL. Not sure what's best tbh\n> >>\n> >> Right. The available test pattern modes are not device specific, like location.\n> > \n> > Well, not really. Location has a set of values it can be assigned to.\n> > The test pattern modes, if I got you right are free formed text, which\n> > makes it very hard to translate them to a fixed set of values like the\n> > ones android defines.\n> > \n> >> So I think it is more natural to put the info to CameraSensorDatabase.\n> > \n> > I still don't get what you would record in the sensor database, maybe\n> > I'm still asleep :)\n> \n> Perhaps there is some mapping of custom menu items to libcamera controls\n> required:\n> \n> > v4l2-ctl -d /dev/v4l-subdev0 --list-ctrls\n> > VIMC Controls\n> > \n> > test_pattern 0x00f0f000 (menu) : min=0 max=21 default=0 value=0\n> \n> > v4l2-ctl -d /dev/v4l-subdev0 --list-ctrls-menus\n> > VIMC Controls\n> > \n> > test_pattern 0x00f0f000 (menu) : min=0 max=21 default=0 value=0\n> > \t\t\t\t0: 75% Colorbar\n> > \t\t\t\t1: 100% Colorbar\n> > \t\t\t\t2: CSC Colorbar\n> > \t\t\t\t3: Horizontal 100% Colorbar\n> > \t\t\t\t4: 100% Color Squares\n> > \t\t\t\t5: 100% Black\n> > \t\t\t\t6: 100% White\n> > \t\t\t\t7: 100% Red\n> > \t\t\t\t8: 100% Green\n> > \t\t\t\t9: 100% Blue\n> > \t\t\t\t10: 16x16 Checkers\n> > \t\t\t\t11: 2x2 Checkers\n> > \t\t\t\t12: 1x1 Checkers\n> > \t\t\t\t13: 2x2 Red/Green Checkers\n> > \t\t\t\t14: 1x1 Red/Green Checkers\n> > \t\t\t\t15: Alternating Hor Lines\n> > \t\t\t\t16: Alternating Vert Lines\n> > \t\t\t\t17: One Pixel Wide Cross\n> > \t\t\t\t18: Two Pixels Wide Cross\n> > \t\t\t\t19: Ten Pixels Wide Cross\n> > \t\t\t\t20: Gray Ramp\n> > \t\t\t\t21: Noise\n> \n> Somehow we would have to map which of those is appropriate for a\n> specific Android test pattern?\n> \n> But that should certainly be done in the android layer - not the\n> libcamera layer ...\n> \n> This is not looking very easy to make generic :-(\n\nIf we define standard test patterns for the libcamera test pattern\ncontrol, then mapping those test patterns to the V4L2 control values\nwould belong to the sensor database. If we make the libcamera test\npattern control a device-specific value without any standardization,\nthen it wouldn't belong to the sensor database but in the HAL.\n\nAn interesting point from the Android camera HAL documentation:\n\n\"The HAL may choose to substitute test patterns from the sensor with\ntest patterns from on-device memory. In that case, it should be\nindistinguishable to the ISP whether the data came from the sensor\ninterconnect bus (such as CSI2) or memory.\"\n\nI wonder if that's what most implementations end up doing, and maybe it\nwould make sense, given that there's no standardization of test patterns\nacross different sensor models.\n\nAt this point my feelign is that we should define libcamera test pattern\ncontrol values based on how we expect those patterns to be used. The\nmain use case (but I may be missing other use cases) is to support\ntesting of the HAL, and to be able to implement standard tests, we need\nstandard test patterns. Generating them in software and feeding them to\nthe ISP would result in a more standard behaviour. What should we do,\nhowever, when the platform only has an inline ISP ?\n\nHiro, could you provide a list (as exhaustive as possible) of use cases\nfor test patterns ?\n\n> > Could you provide an example ?\n> > \n> >>>> What do you think?\n> >>>>\n> >>>>>> + if (ret < 0) {\n> >>>>>> + LOG(V4L2, Error) << \"Unable to get test pattern mode :\"\n> >>>>>> + << strerror(-ret);\n> >>>>>> + return {};\n> >>>>>> + }\n> >>>>>\n> >>>>>> +\n> >>>>>> + v4l2_querymenu menu;\n> >>>>>> + memset(&menu, 0, sizeof(menu));\n> >>>>>> + menu.id = ctrl.id;\n> >>>>>> + for (menu.index = ctrl.minimum;\n> >>>>>> + static_cast(menu.index) <= ctrl.maximum; menu.index++) {\n> >>>>>> + if (ioctl(VIDIOC_QUERYMENU, &menu) != 0) {\n> >>>>>> + continue;\n> >>>>>> + }\n> >>>>>> +\n> >>>>>> + const std::string modeName(reinterpret_cast(menu.name));\n> >>>>>> + std::optional androidTestPatternMode;\n> >>>>>> + /*\n> >>>>>> + * ov13858 and ov5670.\n> >>>>>> + * No corresponding value for \"Vertical Color Bar Type 3\" and\n> >>>>>> + * \"Vertical Color Bar Type 4\".\n> >>>>>> + */\n> >>>>>> + if (modeName == \"Disabled\") {\n> >>>>>> + androidTestPatternMode =\n> >>>>>> + ANDROID_SENSOR_TEST_PATTERN_MODE_OFF;\n> >>>>>> + } else if (modeName == \"Vertical Color Bar Type 1\") {\n> >>>>>> + androidTestPatternMode =\n> >>>>>> + ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS;\n> >>>>>> + } else if (modeName == \"Vertical Color Bar Type 2\") {\n> >>>>>> + androidTestPatternMode =\n> >>>>>> + ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS_FADE_TO_GRAY;\n> >>>>>> + }\n> >>>>>> +\n> >>>>>> + if (androidTestPatternMode) {\n> >>>>>> + testPatternModeMap_[*androidTestPatternMode] = menu.index;\n> >>>>>> + patternModes.push_back(*androidTestPatternMode);\n> >>>>>> + } else {\n> >>>>>> + LOG(V4L2, Debug) << \"Skip test pattern mode: \"\n> >>>>>> + << modeName;\n> >>>>>> + }\n> >>>>>> + }\n> >>>>>> +\n> >>>>>> + return patternModes;\n> >>>>>> +}\n> >>>>>> +\n> >>>>>> +/**\n> >>>>>> + * \\fn V4L2Subdevice::getAvailableTestPatternModes()\n> >>>>>> + * \\brief Set the test pattern mode.\n> >>>>>> + *\n> >>>>>> + * \\return 0 on success or a negative error code otherwise.\n> >>>>>> + */\n> >>>>>> +int V4L2Subdevice::setTestPatternMode(int32_t testPatternMode)\n> >>>>>> +{\n> >>>>>> + auto it = testPatternModeMap_.find(testPatternMode);\n> >>>>>> + if (it != testPatternModeMap_.end()) {\n> >>>>>> + LOG(V4L2, Error) << \"Unsupported test pattern mode: \"\n> >>>>>> + << testPatternMode;\n> >>>>>> +\n> >>>>>> + return -EINVAL;\n> >>>>>> + }\n> >>>>>> +\n> >>>>>> + v4l2_control ctrl;\n> >>>>>> + memset(&ctrl, 0, sizeof(ctrl));\n> >>>>>> + ctrl.id = V4L2_CID_TEST_PATTERN;\n> >>>>>> + ctrl.value = it->second;\n> >>>>>> + int ret = ioctl(VIDIOC_S_CTRL, &ctrl);\n> >>>>>> + if (ret < 0) {\n> >>>>>> + LOG(V4L2, Error) << \"Unable to set test pattern mode: \"\n> >>>>>> + << strerror(-ret);\n> >>>>>> +\n> >>>>>> + return -EINVAL;\n> >>>>>> + }\n> >>>>>> +\n> >>>>>> + return 0;\n> >>>>>> +}\n> >>>>>> } /* namespace libcamera */","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 C55CEBD1F6\n\tfor ;\n\tTue, 13 Apr 2021 00:55:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3EFA068801;\n\tTue, 13 Apr 2021 02:55:08 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9E733687EC\n\tfor ;\n\tTue, 13 Apr 2021 02:55:06 +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 D0F186F2;\n\tTue, 13 Apr 2021 02:55:05 +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=\"LiDUvEWf\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1618275306;\n\tbh=fu/J5Wv/dpDP1Nt5dCCtZfAt9FK1txcoWR5jVUP9Ns4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=LiDUvEWfBlLXea5TgY4GT9H7s4w80RljFwA4nObRa+216AhXLEuEdKh+q37Diz4Jk\n\t+pFw9gYOWbAiyQTfVguL9AGs3oI2oXBiCFerJqACHtRVGQjVCGxCKOsDR7aK49RTuu\n\twu+NVEQXSRhd7jGIawSZ/21ETMJRK+HhBJSvrCO4=","Date":"Tue, 13 Apr 2021 03:54:16 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210409043208.1823330-1-hiroh@chromium.org>\n\t<20210409043208.1823330-3-hiroh@chromium.org>\n\t<20210409085537.ziorybz6ynhnkgfz@uno.localdomain>\n\t\n\t<20210412133217.5a6gurqmskf6hpx3@uno.localdomain>\n\t\n\t<20210412135002.2gh6pjomaxvmaun6@uno.localdomain>\n\t<6d1ae418-f009-9d8c-8b61-335d3124c990@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<6d1ae418-f009-9d8c-8b61-335d3124c990@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH 2/5] libcamera: V4L2Subdevice: Add\n\tgetter/setter function for test pattern mode","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":16218,"web_url":"https://patchwork.libcamera.org/comment/16218/","msgid":"","date":"2021-04-13T07:40:40","subject":"Re: [libcamera-devel] [RFC PATCH 2/5] libcamera: V4L2Subdevice: Add\n\tgetter/setter function for test pattern mode","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Jacopo, Kieran and Laurent, thanks for comments.\n\nOn Tue, Apr 13, 2021 at 9:55 AM Laurent Pinchart\n wrote:\n>\n> Hello,\n>\n> On Mon, Apr 12, 2021 at 08:45:35PM +0100, Kieran Bingham wrote:\n> > On 12/04/2021 14:50, Jacopo Mondi wrote:\n> > > On Mon, Apr 12, 2021 at 10:38:54PM +0900, Hirokazu Honda wrote:\n> > >> On Mon, Apr 12, 2021 at 10:31 PM Jacopo Mondi wrote:\n> > >\n> > > [snip]\n> > >\n> > >>>>>> +/**\n> > >>>>>> + * \\fn V4L2Subdevice::getAvailableTestPatternModes()\n> > >>>>>> + * \\brief Retrieve the available test pattern modes.\n> > >>>>>> + *\n> > >>>>>> + * \\return The available control::SensorTestPatternMode values.\n> > >>>>>> + */\n> > >>>>>> +std::vector V4L2Subdevice::getAvailableTestPatternModes()\n> > >>>>>> +{\n> > >>>>>> + std::vector patternModes;\n> > >>>>>> + if (!testPatternModeMap_.empty()) {\n> > >>>>>> + for (const auto &mode : testPatternModeMap_)\n> > >>>>>> + patternModes.push_back(mode.first);\n> > >>>>>> + return patternModes;\n> > >>>>>> + }\n> > >>>>>> +\n> > >>>>>> + v4l2_queryctrl ctrl;\n> > >>>>>> + memset(&ctrl, 0, sizeof(ctrl));\n> > >>>>>> + ctrl.id = V4L2_CID_TEST_PATTERN;\n> > >>>>>> + int ret = ioctl(VIDIOC_QUERYCTRL, &ctrl);\n> > >>>>>\n> > >>>>> I'm not sure if you have considered the following and found any\n> > >>>>> blocker, but:\n> > >>>>>\n> > >>>>> - Controls are enumerated in V4L2Device::listControls() with\n> > >>>>> VIDIOC_QUERY_EXT_CTRL\n> > >>>>> - Menu controls are accepted but not really handled there yet. I think\n> > >>>>> you should modify listControls() to handle menu controls properly\n> > >>>>> and store them as ControlInfo. ControlInfo currently support being\n> > >>>>> constructed with a Span<> of values but as far as I can tell\n> > >>>>> V4l2ControlInfo does not.\n> > >>>>> - Once you have valid ControlInfo for the menu control, it should\n> > >>>>> contain the V4L2 identifers for the menu entries\n> > >>>>> - The pipeline handler should then populate the libcamera controls in\n> > >>>>> Camera::controls_ by inspecting the V4L2 controls returned by the\n> > >>>>> v4l2 subdevice as we do today in IPU3::listControls(). You should\n> > >>>>> use the libcamera controls identifiers that you have defined in\n> > >>>>> patch #1, not the android defined values\n> > >>>>> - The camera HAL inspects the Camera::controls() to translate from\n> > >>>>> libcamera defined controls to Android defined metadata\n> > >>>>>\n> > >>>>> Does this make sense to you ?\n> > >>>>\n> > >>>> That sounds good to me.\n> > >>>> However, do you think it makes more sense to add available test\n> > >>>> pattern modes to CameraSensorDataBase?\n> > >>>\n> > >>> Do you mean recording the V4L2 test pattern modes there ? What would we gain\n> > >>> compared to fetching them from the kernel interface ?\n> > >>>\n> > >>> If you mean the Android test pattern mode then no, the sensor database\n> > >>> is a core libcamera construct, it knows nothing about Android. One\n> > >>> option for Android-specific values is instead the HAL configuration\n> > >>> file.\n> > >>\n> > >> Hmm, so should we have our own test pattern mode definition a part of\n> > >> which are equivalent to some useful Android test pattern modes?\n> > >\n> > > If you mean recording them in the HAL configuration file you can\n> > > record the android values (or better, their numeric values). There is\n> > > not need to have them defined as libcamera controls if we have no\n> > > strict need to do so.\n> > >\n> > >> Then the definitions are converted to Android ones in HAL configuration?\n> > >\n> > > That would happen if the Camera HAL has to convert from the libcamera\n> > > controls to android ones. It really depends if libcamera wants a\n> > > control for the test pattern modes.\n> >\n> > Personally, I think test patterns are a feature of the camera, and if\n> > available should be exposed (as a libcamera control).\n>\n> Agreed. Conversion to Android camera metadata values should happen in\n> the HAL (assuming the numerical values defined by the libcamera control\n> would differ from the Android values).\n>\n> > Indeed the difficulty might be mapping the menu type options to specific\n> > libcamera controls though in a generic way.\n> >\n> > > CC-ed Laurent\n> > >\n> > >>>> The reason is, as you saw in this patch, there is no way of mapping to\n> > >>>> V4L2_CID_TEST_PATTERN value to android one.\n> > >>>> This is a problem in v4l2 api. An app has to know beforehand what test\n> > >>>> pattern mode the name returned by a driver represents.\n> > >>>\n> > >>> Ah do you mean that the test patter names are driver specific ? Good\n> > >>> job V4L2! I see your point there. I won't be ashamed of having them in\n> > >>> the HAL configuration file, or to perform a best-effort mapping the\n> > >>> Camera HAL. Not sure what's best tbh\n> > >>\n> > >> Right. The available test pattern modes are not device specific, like location.\n> > >\n> > > Well, not really. Location has a set of values it can be assigned to.\n> > > The test pattern modes, if I got you right are free formed text, which\n> > > makes it very hard to translate them to a fixed set of values like the\n> > > ones android defines.\n> > >\n> > >> So I think it is more natural to put the info to CameraSensorDatabase.\n> > >\n> > > I still don't get what you would record in the sensor database, maybe\n> > > I'm still asleep :)\n> >\n> > Perhaps there is some mapping of custom menu items to libcamera controls\n> > required:\n> >\n> > > v4l2-ctl -d /dev/v4l-subdev0 --list-ctrls\n> > > VIMC Controls\n> > >\n> > > test_pattern 0x00f0f000 (menu) : min=0 max=21 default=0 value=0\n> >\n> > > v4l2-ctl -d /dev/v4l-subdev0 --list-ctrls-menus\n> > > VIMC Controls\n> > >\n> > > test_pattern 0x00f0f000 (menu) : min=0 max=21 default=0 value=0\n> > > 0: 75% Colorbar\n> > > 1: 100% Colorbar\n> > > 2: CSC Colorbar\n> > > 3: Horizontal 100% Colorbar\n> > > 4: 100% Color Squares\n> > > 5: 100% Black\n> > > 6: 100% White\n> > > 7: 100% Red\n> > > 8: 100% Green\n> > > 9: 100% Blue\n> > > 10: 16x16 Checkers\n> > > 11: 2x2 Checkers\n> > > 12: 1x1 Checkers\n> > > 13: 2x2 Red/Green Checkers\n> > > 14: 1x1 Red/Green Checkers\n> > > 15: Alternating Hor Lines\n> > > 16: Alternating Vert Lines\n> > > 17: One Pixel Wide Cross\n> > > 18: Two Pixels Wide Cross\n> > > 19: Ten Pixels Wide Cross\n> > > 20: Gray Ramp\n> > > 21: Noise\n> >\n> > Somehow we would have to map which of those is appropriate for a\n> > specific Android test pattern?\n> >\n> > But that should certainly be done in the android layer - not the\n> > libcamera layer ...\n> >\n> > This is not looking very easy to make generic :-(\n>\n> If we define standard test patterns for the libcamera test pattern\n> control, then mapping those test patterns to the V4L2 control values\n> would belong to the sensor database. If we make the libcamera test\n> pattern control a device-specific value without any standardization,\n> then it wouldn't belong to the sensor database but in the HAL.\n>\n> An interesting point from the Android camera HAL documentation:\n>\n> \"The HAL may choose to substitute test patterns from the sensor with\n> test patterns from on-device memory. In that case, it should be\n> indistinguishable to the ISP whether the data came from the sensor\n> interconnect bus (such as CSI2) or memory.\"\n>\n> I wonder if that's what most implementations end up doing, and maybe it\n> would make sense, given that there's no standardization of test patterns\n> across different sensor models.\n>\n> At this point my feelign is that we should define libcamera test pattern\n> control values based on how we expect those patterns to be used. The\n> main use case (but I may be missing other use cases) is to support\n> testing of the HAL, and to be able to implement standard tests, we need\n> standard test patterns. Generating them in software and feeding them to\n> the ISP would result in a more standard behaviour. What should we do,\n> however, when the platform only has an inline ISP ?\n>\n> Hiro, could you provide a list (as exhaustive as possible) of use cases\n> for test patterns ?\n>\n\nIn our test, ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS_FADE_TO_GRAY\nand ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS are used.\nWith test pattern mode frames, we test no corruption in frames.\nhttps://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/platform2/camera/camera3_test/camera3_frame_fixture.h;l=100;drc=2837ddd0fde71236264c417fc5874ba3646d9a46\n\nI discussed with Jacopo via IRC chat.\nThe proper solution is the following:\n1.) Add menu support to controls.\n2.) V4L2Device store all supported test pattern values with controls.\n3.) CameraSensor gets the test pattern values (name, etc) via\nV4L2Device::controls().\n4.) CameraSensor converts them to libcamera test pattern control\nvalues by using a conversion table in CameraSensorDatabase\n5.) IPU3 reports the libcamera test pattern control values to Android HAL.\n6.) Android HAL convers the libcamera test pattern control values to\nAndroid test pattern values.\n\nI will submit the next patch series with this solution (except 6) as\nRFC with ccing all of you.\n\n-Hiro\n\n> > > Could you provide an example ?\n> > >\n> > >>>> What do you think?\n> > >>>>\n> > >>>>>> + if (ret < 0) {\n> > >>>>>> + LOG(V4L2, Error) << \"Unable to get test pattern mode :\"\n> > >>>>>> + << strerror(-ret);\n> > >>>>>> + return {};\n> > >>>>>> + }\n> > >>>>>\n> > >>>>>> +\n> > >>>>>> + v4l2_querymenu menu;\n> > >>>>>> + memset(&menu, 0, sizeof(menu));\n> > >>>>>> + menu.id = ctrl.id;\n> > >>>>>> + for (menu.index = ctrl.minimum;\n> > >>>>>> + static_cast(menu.index) <= ctrl.maximum; menu.index++) {\n> > >>>>>> + if (ioctl(VIDIOC_QUERYMENU, &menu) != 0) {\n> > >>>>>> + continue;\n> > >>>>>> + }\n> > >>>>>> +\n> > >>>>>> + const std::string modeName(reinterpret_cast(menu.name));\n> > >>>>>> + std::optional androidTestPatternMode;\n> > >>>>>> + /*\n> > >>>>>> + * ov13858 and ov5670.\n> > >>>>>> + * No corresponding value for \"Vertical Color Bar Type 3\" and\n> > >>>>>> + * \"Vertical Color Bar Type 4\".\n> > >>>>>> + */\n> > >>>>>> + if (modeName == \"Disabled\") {\n> > >>>>>> + androidTestPatternMode =\n> > >>>>>> + ANDROID_SENSOR_TEST_PATTERN_MODE_OFF;\n> > >>>>>> + } else if (modeName == \"Vertical Color Bar Type 1\") {\n> > >>>>>> + androidTestPatternMode =\n> > >>>>>> + ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS;\n> > >>>>>> + } else if (modeName == \"Vertical Color Bar Type 2\") {\n> > >>>>>> + androidTestPatternMode =\n> > >>>>>> + ANDROID_SENSOR_TEST_PATTERN_MODE_COLOR_BARS_FADE_TO_GRAY;\n> > >>>>>> + }\n> > >>>>>> +\n> > >>>>>> + if (androidTestPatternMode) {\n> > >>>>>> + testPatternModeMap_[*androidTestPatternMode] = menu.index;\n> > >>>>>> + patternModes.push_back(*androidTestPatternMode);\n> > >>>>>> + } else {\n> > >>>>>> + LOG(V4L2, Debug) << \"Skip test pattern mode: \"\n> > >>>>>> + << modeName;\n> > >>>>>> + }\n> > >>>>>> + }\n> > >>>>>> +\n> > >>>>>> + return patternModes;\n> > >>>>>> +}\n> > >>>>>> +\n> > >>>>>> +/**\n> > >>>>>> + * \\fn V4L2Subdevice::getAvailableTestPatternModes()\n> > >>>>>> + * \\brief Set the test pattern mode.\n> > >>>>>> + *\n> > >>>>>> + * \\return 0 on success or a negative error code otherwise.\n> > >>>>>> + */\n> > >>>>>> +int V4L2Subdevice::setTestPatternMode(int32_t testPatternMode)\n> > >>>>>> +{\n> > >>>>>> + auto it = testPatternModeMap_.find(testPatternMode);\n> > >>>>>> + if (it != testPatternModeMap_.end()) {\n> > >>>>>> + LOG(V4L2, Error) << \"Unsupported test pattern mode: \"\n> > >>>>>> + << testPatternMode;\n> > >>>>>> +\n> > >>>>>> + return -EINVAL;\n> > >>>>>> + }\n> > >>>>>> +\n> > >>>>>> + v4l2_control ctrl;\n> > >>>>>> + memset(&ctrl, 0, sizeof(ctrl));\n> > >>>>>> + ctrl.id = V4L2_CID_TEST_PATTERN;\n> > >>>>>> + ctrl.value = it->second;\n> > >>>>>> + int ret = ioctl(VIDIOC_S_CTRL, &ctrl);\n> > >>>>>> + if (ret < 0) {\n> > >>>>>> + LOG(V4L2, Error) << \"Unable to set test pattern mode: \"\n> > >>>>>> + << strerror(-ret);\n> > >>>>>> +\n> > >>>>>> + return -EINVAL;\n> > >>>>>> + }\n> > >>>>>> +\n> > >>>>>> + return 0;\n> > >>>>>> +}\n> > >>>>>> } /* namespace libcamera */\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 4B8D2BD224\n\tfor ;\n\tTue, 13 Apr 2021 07:40:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 02612687F4;\n\tTue, 13 Apr 2021 09:40:54 +0200 (CEST)","from mail-ej1-x62f.google.com (mail-ej1-x62f.google.com\n\t[IPv6:2a00:1450:4864:20::62f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CF65D687F3\n\tfor ;\n\tTue, 13 Apr 2021 09:40:51 +0200 (CEST)","by mail-ej1-x62f.google.com with SMTP id sd23so15706012ejb.12\n\tfor ;\n\tTue, 13 Apr 2021 00:40:51 -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=\"K0iKOM3M\"; 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=9LVyLEfj0xcB9okYlYIMbPmMfeeBzYqhxqAKASTKXVM=;\n\tb=K0iKOM3MzoTrUNE+gZCzq4PF4cxFG7SijF9MX8WLJeTsSteOVMvmRw2V7vJdmPEiVb\n\tlsNyRML1UOkKtyNI/06KRhmNKO45+vdtyuj64iowo/ngS70WeN2fesmxUrk3yCjfVUis\n\tWShQDZuCOQFdr+1zRcXQvKxOU7hNTpj73D0O8=","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=9LVyLEfj0xcB9okYlYIMbPmMfeeBzYqhxqAKASTKXVM=;\n\tb=lmhatFNVwbXaGh1/utXXjf7aC5DMNnBIXfnbXXsXNhISUfEmMvB/qmVPzkLe5FoG5N\n\t6GZ1DAch8yHOkhpM2rBsBDLpNffpLpXEX8GRh+gXP0F1H5f/Asl7E3J5dLyxDGidE1nk\n\tROYd4EZE4oFbDD9SxR8bUJub6hi95GlKfuY+1qAIJndh4vhN6YvWNCUL3/JxL5pkSabb\n\tfZx4ZNS/NrskeLxKMLDrHdareXAgmE0t+GPipT4vYoQM7EyAaDcHBitpoE1hBElJCioV\n\tOULycqKeaM+3UnFBQS69j+u66Ss1T0sYs2YyoAe4si25FT1+cU24ScaYpfdqG5YfXZtq\n\tSpKA==","X-Gm-Message-State":"AOAM533KktuImFHWgweK93zenkbfZrvaXHAy8AmCYCYf80ub2VlN9vVH\n\tZnhFQMyUuxsTmu44lBKluXR1c37qjDB5sIc8fRiytQ==","X-Google-Smtp-Source":"ABdhPJzjsxWL7qRzeKsDrGeZU59EAMsRGK+cN/xnKsLuD96Uouu07ZUTRHG7RtFpHSlGbAjUC/Fv3HN3kSdb8zzqHeU=","X-Received":"by 2002:a17:906:b890:: with SMTP id\n\thb16mr8226796ejb.221.1618299651391; \n\tTue, 13 Apr 2021 00:40:51 -0700 (PDT)","MIME-Version":"1.0","References":"<20210409043208.1823330-1-hiroh@chromium.org>\n\t<20210409043208.1823330-3-hiroh@chromium.org>\n\t<20210409085537.ziorybz6ynhnkgfz@uno.localdomain>\n\t\n\t<20210412133217.5a6gurqmskf6hpx3@uno.localdomain>\n\t\n\t<20210412135002.2gh6pjomaxvmaun6@uno.localdomain>\n\t<6d1ae418-f009-9d8c-8b61-335d3124c990@ideasonboard.com>\n\t","In-Reply-To":"","From":"Hirokazu Honda ","Date":"Tue, 13 Apr 2021 16:40:40 +0900","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [RFC PATCH 2/5] libcamera: V4L2Subdevice: Add\n\tgetter/setter function for test pattern mode","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\" "}}]