[{"id":20672,"web_url":"https://patchwork.libcamera.org/comment/20672/","msgid":"<20211104091112.v4olxmdfnufmbplf@uno.localdomain>","date":"2021-11-04T09:11:12","subject":"Re: [libcamera-devel] [PATCH v4 1/3] camera_sensor: Deal test\n\tpattern mode control values with its enum","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hiro\n\n s/deal/handle. in subject\n\nOn Thu, Nov 04, 2021 at 03:42:50PM +0900, Hirokazu Honda wrote:\n> This changes the type of test pattern mode control values to\n> controls::draft::TestPatternModeEnum from int32_t\n>\n> Signed-off-by: Hirokazu Honda \n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n> include/libcamera/internal/camera_sensor.h | 10 +++++++---\n> include/libcamera/internal/camera_sensor_properties.h | 3 ++-\n> src/libcamera/camera_sensor.cpp | 4 ++--\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 7 ++++---\n> 4 files changed, 15 insertions(+), 9 deletions(-)\n>\n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index d25a1165..edef2220 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -14,8 +14,10 @@\n> #include \n> #include \n>\n> +#include \n> #include \n> #include \n> +\n> #include \n>\n> #include \"libcamera/internal/formats.h\"\n> @@ -40,7 +42,8 @@ public:\n> \tconst std::vector &mbusCodes() const { return mbusCodes_; }\n> \tconst std::vector sizes(unsigned int mbusCode) const;\n> \tSize resolution() const;\n> -\tconst std::vector &testPatternModes() const\n> +\tconst std::vector\n> +\t\t&testPatternModes() const\n> \t{\n> \t\treturn testPatternModes_;\n> \t}\n> @@ -71,7 +74,8 @@ private:\n> \tvoid initVimcDefaultProperties();\n> \tvoid initStaticProperties();\n> \tvoid initTestPatternModes(\n> -\t\tconst std::map &testPatternModeMap);\n> +\t\tconst std::map\n> +\t\t\t&testPatternModeMap);\n> \tint initProperties();\n>\n> \tconst MediaEntity *entity_;\n> @@ -84,7 +88,7 @@ private:\n> \tV4L2Subdevice::Formats formats_;\n> \tstd::vector mbusCodes_;\n> \tstd::vector sizes_;\n> -\tstd::vector testPatternModes_;\n> +\tstd::vector testPatternModes_;\n>\n> \tSize pixelArraySize_;\n> \tRectangle activeArea_;\n> diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h\n> index 67c77920..5c7e5e87 100644\n> --- a/include/libcamera/internal/camera_sensor_properties.h\n> +++ b/include/libcamera/internal/camera_sensor_properties.h\n> @@ -10,6 +10,7 @@\n> #include \n> #include \n>\n> +#include \n> #include \n>\n> namespace libcamera {\n> @@ -18,7 +19,7 @@ struct CameraSensorProperties {\n> \tstatic const CameraSensorProperties *get(const std::string &sensor);\n>\n> \tSize unitCellSize;\n> -\tstd::map testPatternModes;\n> +\tstd::map testPatternModes;\n> };\n>\n> } /* namespace libcamera */\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 9fdb8c09..f0aa9f24 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -311,7 +311,7 @@ void CameraSensor::initStaticProperties()\n> }\n>\n> void CameraSensor::initTestPatternModes(\n> -\tconst std::map &testPatternModes)\n> +\tconst std::map &testPatternModes)\n> {\n> \tconst auto &v4l2TestPattern = controls().find(V4L2_CID_TEST_PATTERN);\n> \tif (v4l2TestPattern == controls().end()) {\n> @@ -327,7 +327,7 @@ void CameraSensor::initTestPatternModes(\n> \t * control index is supported in the below for loop that creates the\n> \t * list of supported test patterns.\n> \t */\n> -\tstd::map indexToTestPatternMode;\n> +\tstd::map indexToTestPatternMode;\n> \tfor (const auto &it : testPatternModes)\n> \t\tindexToTestPatternMode[it.second] = it.first;\n>\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index eb714aa6..63cb7f11 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -981,13 +981,14 @@ int PipelineHandlerIPU3::updateControls(IPU3CameraData *data)\n> \t\treturn ret;\n>\n> \tControlInfoMap::Map controls = IPU3Controls;\n> -\tconst std::vector &testPatternModes = sensor->testPatternModes();\n> +\tconst std::vector\n> +\t\t&testPatternModes = sensor->testPatternModes();\n> \tif (!testPatternModes.empty()) {\n> \t\tstd::vector values;\n> \t\tvalues.reserve(testPatternModes.size());\n>\n> -\t\tfor (int32_t pattern : testPatternModes)\n> -\t\t\tvalues.emplace_back(pattern);\n> +\t\tfor (auto pattern : testPatternModes)\n> +\t\t\tvalues.emplace_back(static_cast(pattern));\n>\n> \t\tcontrols[&controls::draft::TestPatternMode] = ControlInfo(values);\n> \t}\n> --\n> 2.33.1.1089.g2158813163f-goog\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id DF30FBDB1C\n\tfor ;\n\tThu, 4 Nov 2021 09:10:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 49B616033A;\n\tThu, 4 Nov 2021 10:10:23 +0100 (CET)","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 EA15860122\n\tfor ;\n\tThu, 4 Nov 2021 10:10:21 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 58074FF816;\n\tThu, 4 Nov 2021 09:10:21 +0000 (UTC)"],"Date":"Thu, 4 Nov 2021 10:11:12 +0100","From":"Jacopo Mondi ","To":"Hirokazu Honda ","Message-ID":"<20211104091112.v4olxmdfnufmbplf@uno.localdomain>","References":"<20211104064252.2091330-1-hiroh@chromium.org>\n\t<20211104064252.2091330-2-hiroh@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211104064252.2091330-2-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [PATCH v4 1/3] camera_sensor: Deal test\n\tpattern mode control values with its enum","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":20734,"web_url":"https://patchwork.libcamera.org/comment/20734/","msgid":"<163641191660.948064.15674438387296839033@Monstersaurus>","date":"2021-11-08T22:51:56","subject":"Re: [libcamera-devel] [PATCH v4 1/3] camera_sensor: Deal test\n\tpattern mode control values with its enum","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"The $SUBJECT doesn't have libcamera: before camera_sensor: while patch\n2/3 in this series does.\n\nMaking it consistent, and simplifying I'd write the following: Feel free\nto take or adapt as you like.\n\n\n\"\"\"\nlibcamera: camera_sensor: Reference test pattern modes by enum type\n\nThe CameraSensor stores TestPatternModes as an int32_t. This prevents\nthe compiler from verifying the usage against the defined enum types.\n\nFix references to the TestPatternMode to store the value as the\nTestPatternModeEnum type which is defined by the control generator.\n\"\"\"\n\nQuoting Hirokazu Honda (2021-11-04 06:42:50)\n> This changes the type of test pattern mode control values to\n> controls::draft::TestPatternModeEnum from int32_t\n> \n> Signed-off-by: Hirokazu Honda \n> ---\n> include/libcamera/internal/camera_sensor.h | 10 +++++++---\n> include/libcamera/internal/camera_sensor_properties.h | 3 ++-\n> src/libcamera/camera_sensor.cpp | 4 ++--\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 7 ++++---\n> 4 files changed, 15 insertions(+), 9 deletions(-)\n> \n> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h\n> index d25a1165..edef2220 100644\n> --- a/include/libcamera/internal/camera_sensor.h\n> +++ b/include/libcamera/internal/camera_sensor.h\n> @@ -14,8 +14,10 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> #include \n> +\n> #include \n> \n> #include \"libcamera/internal/formats.h\"\n> @@ -40,7 +42,8 @@ public:\n> const std::vector &mbusCodes() const { return mbusCodes_; }\n> const std::vector sizes(unsigned int mbusCode) const;\n> Size resolution() const;\n> - const std::vector &testPatternModes() const\n> + const std::vector\n\nIt's a shame the generated Enum type is so long. But that's not a fault\nof this patch.\n\nReviewed-by: Kieran Bingham \n\n> + &testPatternModes() const\n> {\n> return testPatternModes_;\n> }\n> @@ -71,7 +74,8 @@ private:\n> void initVimcDefaultProperties();\n> void initStaticProperties();\n> void initTestPatternModes(\n> - const std::map &testPatternModeMap);\n> + const std::map\n> + &testPatternModeMap);\n> int initProperties();\n> \n> const MediaEntity *entity_;\n> @@ -84,7 +88,7 @@ private:\n> V4L2Subdevice::Formats formats_;\n> std::vector mbusCodes_;\n> std::vector sizes_;\n> - std::vector testPatternModes_;\n> + std::vector testPatternModes_;\n> \n> Size pixelArraySize_;\n> Rectangle activeArea_;\n> diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h\n> index 67c77920..5c7e5e87 100644\n> --- a/include/libcamera/internal/camera_sensor_properties.h\n> +++ b/include/libcamera/internal/camera_sensor_properties.h\n> @@ -10,6 +10,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> \n> namespace libcamera {\n> @@ -18,7 +19,7 @@ struct CameraSensorProperties {\n> static const CameraSensorProperties *get(const std::string &sensor);\n> \n> Size unitCellSize;\n> - std::map testPatternModes;\n> + std::map testPatternModes;\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 9fdb8c09..f0aa9f24 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -311,7 +311,7 @@ void CameraSensor::initStaticProperties()\n> }\n> \n> void CameraSensor::initTestPatternModes(\n> - const std::map &testPatternModes)\n> + const std::map &testPatternModes)\n> {\n> const auto &v4l2TestPattern = controls().find(V4L2_CID_TEST_PATTERN);\n> if (v4l2TestPattern == controls().end()) {\n> @@ -327,7 +327,7 @@ void CameraSensor::initTestPatternModes(\n> * control index is supported in the below for loop that creates the\n> * list of supported test patterns.\n> */\n> - std::map indexToTestPatternMode;\n> + std::map indexToTestPatternMode;\n> for (const auto &it : testPatternModes)\n> indexToTestPatternMode[it.second] = it.first;\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index eb714aa6..63cb7f11 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -981,13 +981,14 @@ int PipelineHandlerIPU3::updateControls(IPU3CameraData *data)\n> return ret;\n> \n> ControlInfoMap::Map controls = IPU3Controls;\n> - const std::vector &testPatternModes = sensor->testPatternModes();\n> + const std::vector\n> + &testPatternModes = sensor->testPatternModes();\n> if (!testPatternModes.empty()) {\n> std::vector values;\n> values.reserve(testPatternModes.size());\n> \n> - for (int32_t pattern : testPatternModes)\n> - values.emplace_back(pattern);\n> + for (auto pattern : testPatternModes)\n> + values.emplace_back(static_cast(pattern));\n> \n> controls[&controls::draft::TestPatternMode] = ControlInfo(values);\n> }\n> -- \n> 2.33.1.1089.g2158813163f-goog\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C6371BF415\n\tfor ;\n\tMon, 8 Nov 2021 22:52:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F16E96035F;\n\tMon, 8 Nov 2021 23:52:00 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 47BCE6032C\n\tfor ;\n\tMon, 8 Nov 2021 23:51:59 +0100 (CET)","from pendragon.ideasonboard.com\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 DC93BE7;\n\tMon, 8 Nov 2021 23:51:58 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"qXksdJ6z\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1636411918;\n\tbh=vz98d23Ak2wxCG7cOu4FvJ4Wjwiw4e4/w8T2sMYLxJM=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=qXksdJ6zBi0h7UIrTGVYKPB2ojMgJWxw8qek7J2gd8uPL3AcjZDDbc4CyShVIgs+v\n\tj+BvTsnIqQy/q06Lf9VORDDhqWippDf3VKB9ap46SHY671tGRurAATvhhLzvcdLhZf\n\tu2f0i0fmLeioVqbQlvAmHsB+YM6eG4RCOxdl8idI=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211104064252.2091330-2-hiroh@chromium.org>","References":"<20211104064252.2091330-1-hiroh@chromium.org>\n\t<20211104064252.2091330-2-hiroh@chromium.org>","From":"Kieran Bingham ","To":"Hirokazu Honda , libcamera-devel@lists.libcamera.org","Date":"Mon, 08 Nov 2021 22:51:56 +0000","Message-ID":"<163641191660.948064.15674438387296839033@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH v4 1/3] camera_sensor: Deal test\n\tpattern mode control values with its enum","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]