Patch Detail
Show a patch.
GET /api/1.1/patches/2044/?format=api
{ "id": 2044, "url": "https://patchwork.libcamera.org/api/1.1/patches/2044/?format=api", "web_url": "https://patchwork.libcamera.org/patch/2044/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20190928152238.23752-3-laurent.pinchart@ideasonboard.com>", "date": "2019-09-28T15:22:28", "name": "[libcamera-devel,02/12] libcamera: controls: Make ControlValue get/set accessors template methods", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "61da17feadd8e96829d0fcde79d94a9d6bef7347", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/1.1/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/2044/mbox/", "series": [ { "id": 511, "url": "https://patchwork.libcamera.org/api/1.1/series/511/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=511", "date": "2019-09-28T15:22:26", "name": "Improve the application-facing controls API", "version": 1, "mbox": "https://patchwork.libcamera.org/series/511/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/2044/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/2044/checks/", "tags": {}, "headers": { "Return-Path": "<laurent.pinchart@ideasonboard.com>", "Received": [ "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9539160BBA\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 28 Sep 2019 17:22:56 +0200 (CEST)", "from pendragon.bb.dnainternet.fi\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 30EBF53B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 28 Sep 2019 17:22:56 +0200 (CEST)" ], "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1569684176;\n\tbh=PoOdE+x3OxtK1ydoqaI/SZ6oEj+TDu7AxkOEg/JQYEE=;\n\th=From:To:Subject:Date:In-Reply-To:References:From;\n\tb=cIlWx4KYRxfcoDjByGD76VIREGCdmKfhYQpFbC6qBtNdmH+aRA9QBuKIpWmo6+YI9\n\tdoOWC6esaDK0DZCEAmc7p40LrW9/NV1R2Qr1VafMzAiRxViuzTCVOFqOeXbJBY+Ajw\n\tNY0toxl+HhKi/iGOLfq9hGUJdtOiWgo+x1syS9Ug=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Sat, 28 Sep 2019 18:22:28 +0300", "Message-Id": "<20190928152238.23752-3-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.21.0", "In-Reply-To": "<20190928152238.23752-1-laurent.pinchart@ideasonboard.com>", "References": "<20190928152238.23752-1-laurent.pinchart@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH 02/12] libcamera: controls: Make\n\tControlValue get/set accessors template methods", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "X-List-Received-Date": "Sat, 28 Sep 2019 15:22:56 -0000" }, "content": "The ControlValue get accessors are implemented with functions of\ndifferent names, whlie the set accessors use polymorphism to support\ndifferent control types. This isn't very consistent and intuitive. Make\nthe API clearer by using template methods. This will also have the added\nadvantage that support for the new types will only require adding\ntemplate specialisations, without adding new methods.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n include/libcamera/controls.h | 11 ++-\n src/libcamera/controls.cpp | 101 +++++++++++++---------------\n src/libcamera/pipeline/uvcvideo.cpp | 10 +--\n src/libcamera/pipeline/vimc.cpp | 6 +-\n test/controls/control_info.cpp | 4 +-\n test/controls/control_list.cpp | 16 ++---\n test/controls/control_value.cpp | 12 ++--\n 7 files changed, 74 insertions(+), 86 deletions(-)", "diff": "diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\nindex ffba880a66ff..0886370f0901 100644\n--- a/include/libcamera/controls.h\n+++ b/include/libcamera/controls.h\n@@ -36,13 +36,10 @@ public:\n \tControlType type() const { return type_; };\n \tbool isNone() const { return type_ == ControlTypeNone; };\n \n-\tvoid set(bool value);\n-\tvoid set(int value);\n-\tvoid set(int64_t value);\n-\n-\tbool getBool() const;\n-\tint getInt() const;\n-\tint64_t getInt64() const;\n+\ttemplate<typename T>\n+\tconst T &get() const;\n+\ttemplate<typename T>\n+\tvoid set(const T &value);\n \n \tstd::string toString() const;\n \ndiff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp\nindex 9960a30dfa03..88aab88db327 100644\n--- a/src/libcamera/controls.cpp\n+++ b/src/libcamera/controls.cpp\n@@ -90,76 +90,67 @@ ControlValue::ControlValue(int64_t value)\n */\n \n /**\n- * \\brief Set the value with a boolean\n- * \\param[in] value Boolean value to store\n+ * \\fn template<typename T> const T &ControlValue::get() const\n+ * \\brief Get the control value\n+ *\n+ * The control value type shall match the type T, otherwise the behaviour is\n+ * undefined.\n+ *\n+ * \\return The control value\n */\n-void ControlValue::set(bool value)\n+\n+/**\n+ * \\fn template<typename T> void ControlValue::set(const T &value)\n+ * \\brief Set the control value to \\a value\n+ * \\param[in] value The control value\n+ */\n+\n+#ifndef __DOXYGEN__\n+template<>\n+const bool &ControlValue::get<bool>() const\n+{\n+\tASSERT(type_ == ControlTypeBool);\n+\n+\treturn bool_;\n+}\n+\n+template<>\n+const int &ControlValue::get<int>() const\n+{\n+\tASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64);\n+\n+\treturn integer_;\n+}\n+\n+template<>\n+const int64_t &ControlValue::get<int64_t>() const\n+{\n+\tASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64);\n+\n+\treturn integer64_;\n+}\n+\n+template<>\n+void ControlValue::set<bool>(const bool &value)\n {\n \ttype_ = ControlTypeBool;\n \tbool_ = value;\n }\n \n-/**\n- * \\brief Set the value with an integer\n- * \\param[in] value Integer value to store\n- */\n-void ControlValue::set(int value)\n+template<>\n+void ControlValue::set<int>(const int &value)\n {\n \ttype_ = ControlTypeInteger;\n \tinteger_ = value;\n }\n \n-/**\n- * \\brief Set the value with a 64 bit integer\n- * \\param[in] value 64 bit integer value to store\n- */\n-void ControlValue::set(int64_t value)\n+template<>\n+void ControlValue::set<int64_t>(const int64_t &value)\n {\n \ttype_ = ControlTypeInteger64;\n \tinteger64_ = value;\n }\n-\n-/**\n- * \\brief Get the boolean value\n- *\n- * The value type must be Boolean.\n- *\n- * \\return The boolean value\n- */\n-bool ControlValue::getBool() const\n-{\n-\tASSERT(type_ == ControlTypeBool);\n-\n-\treturn bool_;\n-}\n-\n-/**\n- * \\brief Get the integer value\n- *\n- * The value type must be Integer or Integer64.\n- *\n- * \\return The integer value\n- */\n-int ControlValue::getInt() const\n-{\n-\tASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64);\n-\n-\treturn integer_;\n-}\n-\n-/**\n- * \\brief Get the 64-bit integer value\n- *\n- * The value type must be Integer or Integer64.\n- *\n- * \\return The 64-bit integer value\n- */\n-int64_t ControlValue::getInt64() const\n-{\n-\tASSERT(type_ == ControlTypeInteger || type_ == ControlTypeInteger64);\n-\n-\treturn integer64_;\n-}\n+#endif /* __DOXYGEN__ */\n \n /**\n * \\brief Assemble and return a string describing the value\ndiff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp\nindex 8965210550d2..81c548af2c64 100644\n--- a/src/libcamera/pipeline/uvcvideo.cpp\n+++ b/src/libcamera/pipeline/uvcvideo.cpp\n@@ -235,24 +235,24 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)\n \n \t\tswitch (ci->id()) {\n \t\tcase Brightness:\n-\t\t\tcontrols.add(V4L2_CID_BRIGHTNESS, value.getInt());\n+\t\t\tcontrols.add(V4L2_CID_BRIGHTNESS, value.get<int>());\n \t\t\tbreak;\n \n \t\tcase Contrast:\n-\t\t\tcontrols.add(V4L2_CID_CONTRAST, value.getInt());\n+\t\t\tcontrols.add(V4L2_CID_CONTRAST, value.get<int>());\n \t\t\tbreak;\n \n \t\tcase Saturation:\n-\t\t\tcontrols.add(V4L2_CID_SATURATION, value.getInt());\n+\t\t\tcontrols.add(V4L2_CID_SATURATION, value.get<int>());\n \t\t\tbreak;\n \n \t\tcase ManualExposure:\n \t\t\tcontrols.add(V4L2_CID_EXPOSURE_AUTO, 1);\n-\t\t\tcontrols.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.getInt());\n+\t\t\tcontrols.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get<int>());\n \t\t\tbreak;\n \n \t\tcase ManualGain:\n-\t\t\tcontrols.add(V4L2_CID_GAIN, value.getInt());\n+\t\t\tcontrols.add(V4L2_CID_GAIN, value.get<int>());\n \t\t\tbreak;\n \n \t\tdefault:\ndiff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp\nindex f26a91f86ec1..3e34e7a0edbf 100644\n--- a/src/libcamera/pipeline/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc.cpp\n@@ -288,15 +288,15 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)\n \n \t\tswitch (ci->id()) {\n \t\tcase Brightness:\n-\t\t\tcontrols.add(V4L2_CID_BRIGHTNESS, value.getInt());\n+\t\t\tcontrols.add(V4L2_CID_BRIGHTNESS, value.get<int>());\n \t\t\tbreak;\n \n \t\tcase Contrast:\n-\t\t\tcontrols.add(V4L2_CID_CONTRAST, value.getInt());\n+\t\t\tcontrols.add(V4L2_CID_CONTRAST, value.get<int>());\n \t\t\tbreak;\n \n \t\tcase Saturation:\n-\t\t\tcontrols.add(V4L2_CID_SATURATION, value.getInt());\n+\t\t\tcontrols.add(V4L2_CID_SATURATION, value.get<int>());\n \t\t\tbreak;\n \n \t\tdefault:\ndiff --git a/test/controls/control_info.cpp b/test/controls/control_info.cpp\nindex 8cda860b9fe9..faefaaa444d9 100644\n--- a/test/controls/control_info.cpp\n+++ b/test/controls/control_info.cpp\n@@ -32,7 +32,7 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tif (info.min().getInt() != 0 || info.max().getInt() != 0) {\n+\t\tif (info.min().get<int>() != 0 || info.max().get<int>() != 0) {\n \t\t\tcout << \"Invalid control range for Brightness\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n@@ -50,7 +50,7 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tif (info.min().getInt() != 10 || info.max().getInt() != 200) {\n+\t\tif (info.min().get<int>() != 10 || info.max().get<int>() != 200) {\n \t\t\tcout << \"Invalid control range for Contrast\" << endl;\n \t\t\treturn TestFail;\n \t\t}\ndiff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp\nindex f1d79ff8fcfd..0402e7c23dba 100644\n--- a/test/controls/control_list.cpp\n+++ b/test/controls/control_list.cpp\n@@ -96,7 +96,7 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tif (list[Brightness].getInt() != 255) {\n+\t\tif (list[Brightness].get<int>() != 255) {\n \t\t\tcout << \"Incorrest Brightness control value\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n@@ -125,8 +125,8 @@ protected:\n \t\t/*\n \t\t * Test control value retrieval and update through ControlInfo.\n \t\t */\n-\t\tif (list[brightness].getInt() != 64 ||\n-\t\t list[contrast].getInt() != 128) {\n+\t\tif (list[brightness].get<int>() != 64 ||\n+\t\t list[contrast].get<int>() != 128) {\n \t\t\tcout << \"Failed to retrieve control value\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n@@ -134,8 +134,8 @@ protected:\n \t\tlist[brightness] = 10;\n \t\tlist[contrast] = 20;\n \n-\t\tif (list[brightness].getInt() != 10 ||\n-\t\t list[contrast].getInt() != 20) {\n+\t\tif (list[brightness].get<int>() != 10 ||\n+\t\t list[contrast].get<int>() != 20) {\n \t\t\tcout << \"Failed to update control value\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n@@ -185,9 +185,9 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tif (newList[Brightness].getInt() != 10 ||\n-\t\t newList[Contrast].getInt() != 20 ||\n-\t\t newList[Saturation].getInt() != 255) {\n+\t\tif (newList[Brightness].get<int>() != 10 ||\n+\t\t newList[Contrast].get<int>() != 20 ||\n+\t\t newList[Saturation].get<int>() != 255) {\n \t\t\tcout << \"New list contains incorrect values\" << endl;\n \t\t\treturn TestFail;\n \t\t}\ndiff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp\nindex 778efe5c115f..397c43f799ad 100644\n--- a/test/controls/control_value.cpp\n+++ b/test/controls/control_value.cpp\n@@ -27,12 +27,12 @@ protected:\n \t\t << \" Bool: \" << boolean.toString()\n \t\t << endl;\n \n-\t\tif (integer.getInt() != 1234) {\n+\t\tif (integer.get<int>() != 1234) {\n \t\t\tcerr << \"Failed to get Integer\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tif (boolean.getBool() != true) {\n+\t\tif (boolean.get<bool>() != true) {\n \t\t\tcerr << \"Failed to get Boolean\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n@@ -45,19 +45,19 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tvalue.set(true);\n+\t\tvalue.set<bool>(true);\n \t\tif (value.isNone()) {\n \t\t\tcerr << \"Failed to set an empty object\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tif (value.getBool() != true) {\n+\t\tif (value.get<bool>() != true) {\n \t\t\tcerr << \"Failed to get Booleans\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tvalue.set(10);\n-\t\tif (value.getInt() != 10) {\n+\t\tvalue.set<int>(10);\n+\t\tif (value.get<int>() != 10) {\n \t\t\tcerr << \"Failed to get Integer\" << endl;\n \t\t\treturn TestFail;\n \t\t}\n", "prefixes": [ "libcamera-devel", "02/12" ] }