[{"id":17418,"web_url":"https://patchwork.libcamera.org/comment/17418/","msgid":"","date":"2021-06-06T23:37:16","subject":"Re: [libcamera-devel] [PATCH v6 3/6] libcamera:\n\tCameraSensorProperties: Add table of v4l2 index and test pattern","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Hiro,\n\nThank you for the patch.\n\nOn Fri, May 28, 2021 at 12:05:28PM +0900, Hirokazu Honda wrote:\n> The V4L2 specification defines the sensor test pattern modes\n> through a menu control, where a numerical index is associated to\n> a string that describes the test pattern. The index-to-pattern\n> mapping is driver specific and requires a corresponding representation\n> in the library.\n> \n> Add to the static list of CameraSensorProperties a map of indexes to\n> libcamera::controls::TestPatternModes values to be able to map the\n> indexes returned by the driver to the corresponding test pattern mode.\n> \n> Signed-off-by: Hirokazu Honda \n> ---\n> .../internal/camera_sensor_properties.h | 2 ++\n> src/libcamera/camera_sensor_properties.cpp | 32 +++++++++++++++++++\n> 2 files changed, 34 insertions(+)\n> \n> diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h\n> index f5e242cb..88ec7261 100644\n> --- a/include/libcamera/internal/camera_sensor_properties.h\n> +++ b/include/libcamera/internal/camera_sensor_properties.h\n> @@ -7,6 +7,7 @@\n> #ifndef __LIBCAMERA_SENSOR_CAMERA_SENSOR_PROPERTIES_H__\n> #define __LIBCAMERA_SENSOR_CAMERA_SENSOR_PROPERTIES_H__\n> \n> +#include \n> #include \n> \n> #include \n> @@ -17,6 +18,7 @@ struct CameraSensorProperties {\n> \tstatic const CameraSensorProperties *get(const std::string &sensor);\n> \n> \tSize unitCellSize;\n> +\tstd::map testPatternModeMap;\n\nI'd use int32_t for both, given that's the type of both the TestPattern\nlibcamera control and the V4L2 menu controls. The maps are small so it\nwon't consume much memory, and I wouldn't be surprised if uint8_t was\naligned to 32 bits anyway.\n\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp\n> index 2a6e97f7..841564ff 100644\n> --- a/src/libcamera/camera_sensor_properties.cpp\n> +++ b/src/libcamera/camera_sensor_properties.cpp\n> @@ -9,6 +9,8 @@\n> \n> #include \n> \n> +#include \n> +\n> #include \"libcamera/internal/log.h\"\n> \n> /**\n> @@ -34,6 +36,11 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties)\n> *\n> * \\var CameraSensorProperties::unitCellSize\n> * \\brief The physical size of a pixel, including pixel edges, in nanometers.\n> + *\n> + * \\var CameraSensorProperties::testPatternModeMap\n\nI'd name the field testPatternModes (or even testPatterns if you want to\nshorten it), up to you.\n\n> + * \\brief Map that associates the indexes of the sensor test pattern modes as\n> + * returned by V4L2_CID_TEST_PATTERN with the corresponding TestPattern\n> + * control value\n> */\n> \n> /**\n> @@ -47,15 +54,40 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \tstatic const std::map sensorProps = {\n> \t\t{ \"imx219\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> +\t\t\t.testPatternModeMap = {\n> +\t\t\t\t{ 0, controls::draft::TestPatternModeOff },\n> +\t\t\t\t{ 1, controls::draft::TestPatternModeColorBars },\n> +\t\t\t\t{ 2, controls::draft::TestPatternModeSolidColor },\n> +\t\t\t\t{ 3, controls::draft::TestPatternModeColorBarsFadeToGray },\n> +\t\t\t\t{ 4, controls::draft::TestPatternModePn9 },\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov5670\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> +\t\t\t.testPatternModeMap = {\n> +\t\t\t\t{ 0, controls::draft::TestPatternModeOff },\n> +\t\t\t\t{ 1, controls::draft::TestPatternModeColorBars },\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov13858\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> +\t\t\t.testPatternModeMap = {\n> +\t\t\t\t{ 0, controls::draft::TestPatternModeOff },\n> +\t\t\t\t{ 1, controls::draft::TestPatternModeColorBars },\n> +\t\t\t\t{ 2, controls::draft::TestPatternModeColorBarsFadeToGray },\n\nDoes the OV13858 support the fade-to-grey pattern as described in patch\n1/6 ? I'm looking at the OV13850 datasheet (I haven't found a leaked\nversion of the OV13858) and the fade-to-grey pattern doesn't have the\nquantized right half of the colour bars.\n\n> +\t\t\t},\n> \t\t} },\n> \t\t{ \"ov5693\", {\n> \t\t\t.unitCellSize = { 1400, 1400 },\n> +\t\t\t.testPatternModeMap = {\n> +\t\t\t\t{ 0, controls::draft::TestPatternModeOff },\n> +\t\t\t\t{ 2, controls::draft::TestPatternModeColorBars },\n> +\t\t\t\t/*\n> +\t\t\t\t * No correspondence test pattern mode for\n\ns/correspondence/corresponding/\n\n> +\t\t\t\t * 1: \"Random data\" and 3: \"Colour Bars with\n> +\t\t\t\t * Rolling Bar\".\n> +\t\t\t\t */\n> +\t\t\t},\n> \t\t} },\n> \t};\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 BC0D5C320B\n\tfor ;\n\tSun, 6 Jun 2021 23:37:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1AE8468928;\n\tMon, 7 Jun 2021 01:37:32 +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 A376868921\n\tfor ;\n\tMon, 7 Jun 2021 01:37:30 +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 EBEBB80F;\n\tMon, 7 Jun 2021 01:37:29 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"FmyC8N7X\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1623022650;\n\tbh=O3TYQLbrz20nxubGVYBymxz8dBWzf2ccowY3idGW0i0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FmyC8N7X/k9JVUE2CqBKiTxEYrq/vPOmIVn4aC+nX2rIqYmc7qCgCBbwdYS9JvpLR\n\tC+xBEPxxnRiPoqePXW3i7w9h5t+MYQSRY9Z6GjiEWwVut3mEfBh63pt5iwkEjrhGfr\n\tw4hOFa8sboBbRj7QfT8YnAaQ+A61aMeZ3KbQU50M=","Date":"Mon, 7 Jun 2021 02:37:16 +0300","From":"Laurent Pinchart ","To":"Hirokazu Honda ","Message-ID":"","References":"<20210528030531.189492-1-hiroh@chromium.org>\n\t<20210528030531.189492-3-hiroh@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210528030531.189492-3-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [PATCH v6 3/6] libcamera:\n\tCameraSensorProperties: Add table of v4l2 index and test pattern","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17426,"web_url":"https://patchwork.libcamera.org/comment/17426/","msgid":"","date":"2021-06-07T00:20:01","subject":"Re: [libcamera-devel] [PATCH v6 3/6] libcamera:\n\tCameraSensorProperties: Add table of v4l2 index and test pattern","submitter":{"id":63,"url":"https://patchwork.libcamera.org/api/people/63/","name":"Hirokazu Honda","email":"hiroh@chromium.org"},"content":"Hi Laurent,\n\nOn Mon, Jun 7, 2021 at 8:37 AM Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Hiro,\n>\n> Thank you for the patch.\n>\n> On Fri, May 28, 2021 at 12:05:28PM +0900, Hirokazu Honda wrote:\n> > The V4L2 specification defines the sensor test pattern modes\n> > through a menu control, where a numerical index is associated to\n> > a string that describes the test pattern. The index-to-pattern\n> > mapping is driver specific and requires a corresponding representation\n> > in the library.\n> >\n> > Add to the static list of CameraSensorProperties a map of indexes to\n> > libcamera::controls::TestPatternModes values to be able to map the\n> > indexes returned by the driver to the corresponding test pattern mode.\n> >\n> > Signed-off-by: Hirokazu Honda \n> > ---\n> > .../internal/camera_sensor_properties.h | 2 ++\n> > src/libcamera/camera_sensor_properties.cpp | 32 +++++++++++++++++++\n> > 2 files changed, 34 insertions(+)\n> >\n> > diff --git a/include/libcamera/internal/camera_sensor_properties.h\n> b/include/libcamera/internal/camera_sensor_properties.h\n> > index f5e242cb..88ec7261 100644\n> > --- a/include/libcamera/internal/camera_sensor_properties.h\n> > +++ b/include/libcamera/internal/camera_sensor_properties.h\n> > @@ -7,6 +7,7 @@\n> > #ifndef __LIBCAMERA_SENSOR_CAMERA_SENSOR_PROPERTIES_H__\n> > #define __LIBCAMERA_SENSOR_CAMERA_SENSOR_PROPERTIES_H__\n> >\n> > +#include \n> > #include \n> >\n> > #include \n> > @@ -17,6 +18,7 @@ struct CameraSensorProperties {\n> > static const CameraSensorProperties *get(const std::string\n> &sensor);\n> >\n> > Size unitCellSize;\n> > + std::map testPatternModeMap;\n>\n> I'd use int32_t for both, given that's the type of both the TestPattern\n> libcamera control and the V4L2 menu controls. The maps are small so it\n> won't consume much memory, and I wouldn't be surprised if uint8_t was\n> aligned to 32 bits anyway.\n>\n> > };\n> >\n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/camera_sensor_properties.cpp\n> b/src/libcamera/camera_sensor_properties.cpp\n> > index 2a6e97f7..841564ff 100644\n> > --- a/src/libcamera/camera_sensor_properties.cpp\n> > +++ b/src/libcamera/camera_sensor_properties.cpp\n> > @@ -9,6 +9,8 @@\n> >\n> > #include \n> >\n> > +#include \n> > +\n> > #include \"libcamera/internal/log.h\"\n> >\n> > /**\n> > @@ -34,6 +36,11 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties)\n> > *\n> > * \\var CameraSensorProperties::unitCellSize\n> > * \\brief The physical size of a pixel, including pixel edges, in\n> nanometers.\n> > + *\n> > + * \\var CameraSensorProperties::testPatternModeMap\n>\n> I'd name the field testPatternModes (or even testPatterns if you want to\n> shorten it), up to you.\n>\n> > + * \\brief Map that associates the indexes of the sensor test pattern\n> modes as\n> > + * returned by V4L2_CID_TEST_PATTERN with the corresponding TestPattern\n> > + * control value\n> > */\n> >\n> > /**\n> > @@ -47,15 +54,40 @@ const CameraSensorProperties\n> *CameraSensorProperties::get(const std::string &sen\n> > static const std::map\n> sensorProps = {\n> > { \"imx219\", {\n> > .unitCellSize = { 1120, 1120 },\n> > + .testPatternModeMap = {\n> > + { 0, controls::draft::TestPatternModeOff },\n> > + { 1,\n> controls::draft::TestPatternModeColorBars },\n> > + { 2,\n> controls::draft::TestPatternModeSolidColor },\n> > + { 3,\n> controls::draft::TestPatternModeColorBarsFadeToGray },\n> > + { 4, controls::draft::TestPatternModePn9 },\n> > + },\n> > } },\n> > { \"ov5670\", {\n> > .unitCellSize = { 1120, 1120 },\n> > + .testPatternModeMap = {\n> > + { 0, controls::draft::TestPatternModeOff },\n> > + { 1,\n> controls::draft::TestPatternModeColorBars },\n> > + },\n> > } },\n> > { \"ov13858\", {\n> > .unitCellSize = { 1120, 1120 },\n> > + .testPatternModeMap = {\n> > + { 0, controls::draft::TestPatternModeOff },\n> > + { 1,\n> controls::draft::TestPatternModeColorBars },\n> > + { 2,\n> controls::draft::TestPatternModeColorBarsFadeToGray },\n>\n> Does the OV13858 support the fade-to-grey pattern as described in patch\n> 1/6 ? I'm looking at the OV13850 datasheet (I haven't found a leaked\n> version of the OV13858) and the fade-to-grey pattern doesn't have the\n> quantized right half of the colour bars.\n>\n> Thanks. I dropped the fade-to-gray. In fact, our xml doesn't list it.\n\nRegards,\n-Hiro\n\n\n> > + },\n> > } },\n> > { \"ov5693\", {\n> > .unitCellSize = { 1400, 1400 },\n> > + .testPatternModeMap = {\n> > + { 0, controls::draft::TestPatternModeOff },\n> > + { 2,\n> controls::draft::TestPatternModeColorBars },\n> > + /*\n> > + * No correspondence test pattern mode for\n>\n> s/correspondence/corresponding/\n>\n> > + * 1: \"Random data\" and 3: \"Colour Bars\n> with\n> > + * Rolling Bar\".\n> > + */\n> > + },\n> > } },\n> > };\n> >\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 AF804C320B\n\tfor ;\n\tMon, 7 Jun 2021 00:20:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1C90A68928;\n\tMon, 7 Jun 2021 02:20:14 +0200 (CEST)","from mail-ej1-x634.google.com (mail-ej1-x634.google.com\n\t[IPv6:2a00:1450:4864:20::634])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5229568921\n\tfor ;\n\tMon, 7 Jun 2021 02:20:12 +0200 (CEST)","by mail-ej1-x634.google.com with SMTP id my49so7004771ejc.7\n\tfor ;\n\tSun, 06 Jun 2021 17:20:12 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"XNLUB+ZM\"; 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=5wIuDnW0z/jPrOYo8CslsQXbyseYECHhrAS9iCC0kro=;\n\tb=XNLUB+ZMQGy0FiN6ZxRMVtqcxatM8037POs2LJKj8HhjTEBHqq55gVWSqGqaNRFeKt\n\tQ80W8D6rYBmGf942Yuiix9Cf2t1QbQfb6+oCylAMFsRXuxaWKQgkD77kCXAqNvXMbP3x\n\tb9Z6pFqXG7ofM8bg8sMh7YgGtQ3Opa0z2pujA=","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=5wIuDnW0z/jPrOYo8CslsQXbyseYECHhrAS9iCC0kro=;\n\tb=NGHGkkfdordWU5pfpejeQACfe01FyvkzABA2vR0AeKLpF5/eKFeveXIYjnQGLFcDF0\n\tf5etfGcFkzuuQCHmuqwd7U72Eobyq5uBg+zMv4pVrbIJNPhy0R97CuQzdalznGu2mryC\n\t9QFFESO9hlSeBQ8owDhxyCL//o+daU2A6i5dGnXbsaPWhiWOPNKv8P/4gD0nC19g7Iw3\n\tYBMd620WqKxw2WEcx/TTbfU6ZXPsNjT7N+lyBBg3Agp5Ks/F1zBcKg/Nhdy0SupHzhpf\n\tURBSawh4RqhXz0RYeg+HgOZKtHTjWb2GXeO6FIr5z1aeiXvVmmIlzBSQDjOjTmFThaSX\n\tLsEg==","X-Gm-Message-State":"AOAM532Qcyc8Lz/9mTuYUUqRK+SpmIWvBcKi4ACZ1sTAOSbfAMh0a/1W\n\tJI1VCLAuk/bUl+2cDKVHgumLErL+bxS0AghbFwTGMg==","X-Google-Smtp-Source":"ABdhPJyrCvNjB0vcxlbEcVVRZVY8oNK4/ptn3Eo/U1ecUNJDI+IAvc6gq41d7SCV4H6RCnV9ToXbzggZoVodGwLZEbE=","X-Received":"by 2002:a17:906:2459:: with SMTP id\n\ta25mr15110811ejb.306.1623025211989; \n\tSun, 06 Jun 2021 17:20:11 -0700 (PDT)","MIME-Version":"1.0","References":"<20210528030531.189492-1-hiroh@chromium.org>\n\t<20210528030531.189492-3-hiroh@chromium.org>\n\t","In-Reply-To":"","From":"Hirokazu Honda ","Date":"Mon, 7 Jun 2021 09:20:01 +0900","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"00000000000038e46b05c42201d1\"","Subject":"Re: [libcamera-devel] [PATCH v6 3/6] libcamera:\n\tCameraSensorProperties: Add table of v4l2 index and test pattern","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]