[{"id":17687,"web_url":"https://patchwork.libcamera.org/comment/17687/","msgid":"<20210622100811.hp7q2ocsy62g6pry@uno.localdomain>","date":"2021-06-22T10:08:11","subject":"Re: [libcamera-devel] [RFC PATCH v2 1/5] libcamera: camera_sensor:\n\tReverse the key and value of test pattern mode map","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Hiro,\n\nOn Tue, Jun 22, 2021 at 11:36:50AM +0900, Hirokazu Honda wrote:\n> The key and value of the test pattern mode are originally the index of\n> v4l2 control and the corresponding test pattern mode control value.\n> This key and value are useful in the initialization for reporting\n> available test pattern modes. However, the map of the reversed key and\n> value is much more useful in applying a requested test pattern mode.\n> Reverses the key and value of the map as the initialization is one\n> time but the test pattern mode request will be multiple times.\n>\n> Signed-off-by: Hirokazu Honda \n> ---\n> src/libcamera/camera_sensor.cpp | 8 +++++--\n> src/libcamera/camera_sensor_properties.cpp | 28 +++++++++++-----------\n> 2 files changed, 20 insertions(+), 16 deletions(-)\n>\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 3e135353..70bbd97a 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -319,11 +319,15 @@ void CameraSensor::initTestPatternModes(\n> \t\treturn;\n> \t}\n>\n> +\tstd::map indexToTestPatternMode;\n> +\tfor (const auto& it : testPatternModes)\n> +\t\tindexToTestPatternMode[it.second] = it.first;\n> +\n\nAt the expense of this additional vector, it is probably more\nefficient to create it here instead of walking the testPatternModes\nmap looking for a value matching the index in all the iterations of\nthe below for loop like I would have done.\n\nHowever, is this worth a comment ?\n\n /*\n * Create a map that associates the V4L2 control index to the\n * test pattern mode by reversing the testPatternModes map\n * provided by the camera sensor properties. This makes it\n * easier to verify if the control index is supported in the below\n * for loop that creates the list of supported test patterns.\n */\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> \tfor (const ControlValue &value : v4l2TestPattern->second.values()) {\n> \t\tconst int32_t index = value.get();\n>\n> -\t\tconst auto it = testPatternModes.find(index);\n> -\t\tif (it == testPatternModes.end()) {\n> +\t\tconst auto it = indexToTestPatternMode.find(index);\n> +\t\tif (it == indexToTestPatternMode.end()) {\n> \t\t\tLOG(CameraSensor, Debug)\n> \t\t\t\t<< \"Test pattern mode \" << index << \" ignored\";\n> \t\t\tcontinue;\n> diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp\n> index f660743a..d0b2ae0e 100644\n> --- a/src/libcamera/camera_sensor_properties.cpp\n> +++ b/src/libcamera/camera_sensor_properties.cpp\n> @@ -38,9 +38,9 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties)\n> * \\brief The physical size of a pixel, including pixel edges, in nanometers.\n> *\n> * \\var CameraSensorProperties::testPatternModes\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> + * \\brief Map that associates the TestPattern control value with the indexes of\n> + * the corresponding sensor test pattern modes as returned by\n> + * V4L2_CID_TEST_PATTERN.\n> */\n>\n> /**\n> @@ -55,11 +55,11 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t{ \"imx219\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> \t\t\t.testPatternModes = {\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\t{ controls::draft::TestPatternModeOff, 0 },\n> +\t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n> +\t\t\t\t{ controls::draft::TestPatternModeSolidColor, 2 },\n> +\t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n> +\t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n> \t\t\t},\n> \t\t} },\n> \t\t{ \"imx258\", {\n> @@ -70,22 +70,22 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t{ \"ov5670\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> \t\t\t.testPatternModes = {\n> -\t\t\t\t{ 0, controls::draft::TestPatternModeOff },\n> -\t\t\t\t{ 1, controls::draft::TestPatternModeColorBars },\n> +\t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n> +\t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n> \t\t\t},\n> \t\t} },\n> \t\t{ \"ov13858\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> \t\t\t.testPatternModes = {\n> -\t\t\t\t{ 0, controls::draft::TestPatternModeOff },\n> -\t\t\t\t{ 1, controls::draft::TestPatternModeColorBars },\n> +\t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n> +\t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n> \t\t\t},\n> \t\t} },\n> \t\t{ \"ov5693\", {\n> \t\t\t.unitCellSize = { 1400, 1400 },\n> \t\t\t.testPatternModes = {\n> -\t\t\t\t{ 0, controls::draft::TestPatternModeOff },\n> -\t\t\t\t{ 2, controls::draft::TestPatternModeColorBars },\n> +\t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n> +\t\t\t\t{ controls::draft::TestPatternModeColorBars, 2 },\n> \t\t\t\t/*\n> \t\t\t\t * No corresponding test pattern mode for\n> \t\t\t\t * 1: \"Random data\" and 3: \"Colour Bars with\n> --\n> 2.32.0.288.g62a8d224e6-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 1325EC321B\n\tfor ;\n\tTue, 22 Jun 2021 10:07:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 241AD68932;\n\tTue, 22 Jun 2021 12:07:24 +0200 (CEST)","from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[217.70.183.200])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B687960292\n\tfor ;\n\tTue, 22 Jun 2021 12:07:22 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay7-d.mail.gandi.net (Postfix) with ESMTPSA id 0581220015;\n\tTue, 22 Jun 2021 10:07:21 +0000 (UTC)"],"Date":"Tue, 22 Jun 2021 12:08:11 +0200","From":"Jacopo Mondi ","To":"Hirokazu Honda ","Message-ID":"<20210622100811.hp7q2ocsy62g6pry@uno.localdomain>","References":"<20210622023654.969162-1-hiroh@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210622023654.969162-1-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [RFC PATCH v2 1/5] libcamera: camera_sensor:\n\tReverse the key and value of test pattern mode map","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":17688,"web_url":"https://patchwork.libcamera.org/comment/17688/","msgid":"","date":"2021-06-22T10:14:41","subject":"Re: [libcamera-devel] [RFC PATCH v2 1/5] libcamera: camera_sensor:\n\tReverse the key and value of test pattern mode map","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 Tue, Jun 22, 2021 at 11:36:50AM +0900, Hirokazu Honda wrote:\n> The key and value of the test pattern mode are originally the index of\n> v4l2 control and the corresponding test pattern mode control value.\n> This key and value are useful in the initialization for reporting\n> available test pattern modes. However, the map of the reversed key and\n> value is much more useful in applying a requested test pattern mode.\n> Reverses the key and value of the map as the initialization is one\n> time but the test pattern mode request will be multiple times.\n> \n> Signed-off-by: Hirokazu Honda \n\nReviewed-by: Laurent Pinchart \n\n> ---\n> src/libcamera/camera_sensor.cpp | 8 +++++--\n> src/libcamera/camera_sensor_properties.cpp | 28 +++++++++++-----------\n> 2 files changed, 20 insertions(+), 16 deletions(-)\n> \n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 3e135353..70bbd97a 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -319,11 +319,15 @@ void CameraSensor::initTestPatternModes(\n> \t\treturn;\n> \t}\n> \n> +\tstd::map indexToTestPatternMode;\n> +\tfor (const auto& it : testPatternModes)\n> +\t\tindexToTestPatternMode[it.second] = it.first;\n> +\n> \tfor (const ControlValue &value : v4l2TestPattern->second.values()) {\n> \t\tconst int32_t index = value.get();\n> \n> -\t\tconst auto it = testPatternModes.find(index);\n> -\t\tif (it == testPatternModes.end()) {\n> +\t\tconst auto it = indexToTestPatternMode.find(index);\n> +\t\tif (it == indexToTestPatternMode.end()) {\n> \t\t\tLOG(CameraSensor, Debug)\n> \t\t\t\t<< \"Test pattern mode \" << index << \" ignored\";\n> \t\t\tcontinue;\n> diff --git a/src/libcamera/camera_sensor_properties.cpp b/src/libcamera/camera_sensor_properties.cpp\n> index f660743a..d0b2ae0e 100644\n> --- a/src/libcamera/camera_sensor_properties.cpp\n> +++ b/src/libcamera/camera_sensor_properties.cpp\n> @@ -38,9 +38,9 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties)\n> * \\brief The physical size of a pixel, including pixel edges, in nanometers.\n> *\n> * \\var CameraSensorProperties::testPatternModes\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> + * \\brief Map that associates the TestPattern control value with the indexes of\n> + * the corresponding sensor test pattern modes as returned by\n> + * V4L2_CID_TEST_PATTERN.\n> */\n> \n> /**\n> @@ -55,11 +55,11 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t{ \"imx219\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> \t\t\t.testPatternModes = {\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\t{ controls::draft::TestPatternModeOff, 0 },\n> +\t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n> +\t\t\t\t{ controls::draft::TestPatternModeSolidColor, 2 },\n> +\t\t\t\t{ controls::draft::TestPatternModeColorBarsFadeToGray, 3 },\n> +\t\t\t\t{ controls::draft::TestPatternModePn9, 4 },\n> \t\t\t},\n> \t\t} },\n> \t\t{ \"imx258\", {\n> @@ -70,22 +70,22 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen\n> \t\t{ \"ov5670\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> \t\t\t.testPatternModes = {\n> -\t\t\t\t{ 0, controls::draft::TestPatternModeOff },\n> -\t\t\t\t{ 1, controls::draft::TestPatternModeColorBars },\n> +\t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n> +\t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n> \t\t\t},\n> \t\t} },\n> \t\t{ \"ov13858\", {\n> \t\t\t.unitCellSize = { 1120, 1120 },\n> \t\t\t.testPatternModes = {\n> -\t\t\t\t{ 0, controls::draft::TestPatternModeOff },\n> -\t\t\t\t{ 1, controls::draft::TestPatternModeColorBars },\n> +\t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n> +\t\t\t\t{ controls::draft::TestPatternModeColorBars, 1 },\n> \t\t\t},\n> \t\t} },\n> \t\t{ \"ov5693\", {\n> \t\t\t.unitCellSize = { 1400, 1400 },\n> \t\t\t.testPatternModes = {\n> -\t\t\t\t{ 0, controls::draft::TestPatternModeOff },\n> -\t\t\t\t{ 2, controls::draft::TestPatternModeColorBars },\n> +\t\t\t\t{ controls::draft::TestPatternModeOff, 0 },\n> +\t\t\t\t{ controls::draft::TestPatternModeColorBars, 2 },\n> \t\t\t\t/*\n> \t\t\t\t * No corresponding test pattern mode for\n> \t\t\t\t * 1: \"Random data\" and 3: \"Colour Bars with","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 7B8D7C321A\n\tfor ;\n\tTue, 22 Jun 2021 10:15:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DBF2260294;\n\tTue, 22 Jun 2021 12:15:10 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 97B2B60292\n\tfor ;\n\tTue, 22 Jun 2021 12:15:09 +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 145DFA66;\n\tTue, 22 Jun 2021 12:15:09 +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=\"QnVgovbE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1624356909;\n\tbh=K5S+vqpF96H8LLCUkxufbeHCwXWVBwCb2zPQa0QlAfQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=QnVgovbEF8G753QzXh2EWya+vVOTAqgYTUtbjEHtj4GkZA/04ry5x0Md8076Xn3yl\n\txJ/KQUoRypu+VPeEAwD0YafxM78KEEM5oax4pWkoTXrbUzBBpNACcyqhhl6pQgf9Qr\n\t4EybSQSx5hKbQ3ptc4KufMfLPt6vGsHmIFMoNgDw=","Date":"Tue, 22 Jun 2021 13:14:41 +0300","From":"Laurent Pinchart ","To":"Hirokazu Honda ","Message-ID":"","References":"<20210622023654.969162-1-hiroh@chromium.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210622023654.969162-1-hiroh@chromium.org>","Subject":"Re: [libcamera-devel] [RFC PATCH v2 1/5] libcamera: camera_sensor:\n\tReverse the key and value of test pattern mode map","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\" "}}]