[{"id":23191,"web_url":"https://patchwork.libcamera.org/comment/23191/","msgid":"","date":"2022-05-27T10:52:34","subject":"Re: [libcamera-devel] [PATCH v2 16/19] py: Re-structure the\n\tcontrols API","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Tomi,\n\nThank you for the patch.\n\nOn Tue, May 24, 2022 at 02:46:07PM +0300, Tomi Valkeinen wrote:\n> Add ControlInfo class and change the controls related methods to\n> resemble the C++ API (e.g. no more string based control methods).\n> \n> We don't implement ControlList or ControlInfoMap but just expose the\n> same data via standard Python dict.\n> \n> Signed-off-by: Tomi Valkeinen \n> ---\n> src/py/cam/cam.py | 8 +--\n> src/py/cam/cam_qt.py | 9 ++--\n> src/py/libcamera/py_main.cpp | 100 ++++++++++++++++++-----------------\n> 3 files changed, 61 insertions(+), 56 deletions(-)\n> \n> diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py\n> index f6e8232c..f464bd01 100755\n> --- a/src/py/cam/cam.py\n> +++ b/src/py/cam/cam.py\n> @@ -48,16 +48,16 @@ class CameraContext:\n> \n> print('Properties for', self.id)\n> \n> - for name, prop in camera.properties.items():\n> - print('\\t{}: {}'.format(name, prop))\n> + for cid, val in camera.properties.items():\n> + print('\\t{}: {}'.format(cid, val))\n> \n> def do_cmd_list_controls(self):\n> camera = self.camera\n> \n> print('Controls for', self.id)\n> \n> - for name, prop in camera.controls.items():\n> - print('\\t{}: {}'.format(name, prop))\n> + for cid, info in camera.controls.items():\n> + print('\\t{}: {}'.format(cid, info))\n> \n> def do_cmd_info(self):\n> camera = self.camera\n> diff --git a/src/py/cam/cam_qt.py b/src/py/cam/cam_qt.py\n> index d638e9cc..739e9749 100644\n> --- a/src/py/cam/cam_qt.py\n> +++ b/src/py/cam/cam_qt.py\n> @@ -267,9 +267,9 @@ class MainWindow(QtWidgets.QWidget):\n> \n> camera = ctx.camera\n> \n> - for k, v in camera.properties.items():\n> + for cid, cv in camera.properties.items():\n> lab = QtWidgets.QLabel()\n> - lab.setText(k + ' = ' + str(v))\n> + lab.setText(\"{} = {}\".format(cid, cv))\n\ns/\"/'/g\n\n> groupLayout.addWidget(lab)\n> \n> group = QtWidgets.QGroupBox('Controls')\n> @@ -277,9 +277,10 @@ class MainWindow(QtWidgets.QWidget):\n> group.setLayout(groupLayout)\n> controlsLayout.addWidget(group)\n> \n> - for k, (min, max, default) in camera.controls.items():\n> + for cid, cinfo in camera.controls.items():\n> lab = QtWidgets.QLabel()\n> - lab.setText('{} = {}/{}/{}'.format(k, min, max, default))\n> + lab.setText('{} = {}/{}/{}'\n> + .format(cid, cinfo.min, cinfo.max, cinfo.default))\n> groupLayout.addWidget(lab)\n> \n> controlsLayout.addStretch()\n> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n> index 33ecc1cd..80f26c12 100644\n> --- a/src/py/libcamera/py_main.cpp\n> +++ b/src/py/libcamera/py_main.cpp\n> @@ -159,6 +159,7 @@ PYBIND11_MODULE(_libcamera, m)\n> \tauto pyFrameBuffer = py::class_(m, \"FrameBuffer\");\n> \tauto pyStream = py::class_(m, \"Stream\");\n> \tauto pyControlId = py::class_(m, \"ControlId\");\n> +\tauto pyControlInfo = py::class_(m, \"ControlInfo\");\n> \tauto pyRequest = py::class_(m, \"Request\");\n> \tauto pyRequestStatus = py::enum_(pyRequest, \"Status\");\n> \tauto pyRequestReuse = py::enum_(pyRequest, \"Reuse\");\n> @@ -260,28 +261,17 @@ PYBIND11_MODULE(_libcamera, m)\n> \t\t.def_property_readonly(\"id\", &Camera::id)\n> \t\t.def(\"acquire\", &Camera::acquire)\n> \t\t.def(\"release\", &Camera::release)\n> -\t\t.def(\"start\", [](Camera &self, py::dict controls) {\n> +\t\t.def(\"start\", [](Camera &self,\n> +\t\t const std::unordered_map &controls) {\n> \t\t\t/* \\todo What happens if someone calls start() multiple times? */\n> \n> \t\t\tself.requestCompleted.connect(handleRequestCompleted);\n> \n> -\t\t\tconst ControlInfoMap &controlMap = self.controls();\n> -\t\t\tControlList controlList(controlMap);\n> -\t\t\tfor (const auto& [hkey, hval]: controls) {\n> -\t\t\t\tauto key = hkey.cast();\n> +\t\t\tControlList controlList(self.controls());\n> \n> -\t\t\t\tauto it = std::find_if(controlMap.begin(), controlMap.end(),\n> -\t\t\t\t\t\t [&key](const auto &kvp) {\n> -\t\t\t\t\t\t\t\treturn kvp.first->name() == key;\n> -\t\t\t\t\t\t });\n> -\n> -\t\t\t\tif (it == controlMap.end())\n> -\t\t\t\t\tthrow std::runtime_error(\"Control \" + key + \" not found\");\n> -\n> -\t\t\t\tconst auto &id = it->first;\n> -\t\t\t\tauto obj = py::cast(hval);\n> -\n> -\t\t\t\tcontrolList.set(id->id(), pyToControlValue(obj, id->type()));\n> +\t\t\tfor (const auto& [id, obj]: controls) {\n> +\t\t\t\tauto val = pyToControlValue(obj, id->type());\n> +\t\t\t\tcontrolList.set(id->id(), val);\n> \t\t\t}\n> \n> \t\t\tint ret = self.start(&controlList);\n> @@ -291,7 +281,7 @@ PYBIND11_MODULE(_libcamera, m)\n> \t\t\t}\n> \n> \t\t\treturn 0;\n> -\t\t}, py::arg(\"controls\") = py::dict())\n> +\t\t}, py::arg(\"controls\") = std::unordered_map())\n> \n> \t\t.def(\"stop\", [](Camera &self) {\n> \t\t\tint ret = self.stop();\n> @@ -341,40 +331,26 @@ PYBIND11_MODULE(_libcamera, m)\n> \t\t\treturn set;\n> \t\t})\n> \n> -\t\t.def(\"find_control\", [](Camera &self, const std::string &name) {\n> -\t\t\tconst auto &controls = self.controls();\n> -\n> -\t\t\tauto it = std::find_if(controls.begin(), controls.end(),\n> -\t\t\t\t\t [&name](const auto &kvp) {\n> -\t\t\t\t\t\t\treturn kvp.first->name() == name;\n> -\t\t\t\t\t });\n> -\n> -\t\t\tif (it == controls.end())\n> -\t\t\t\tthrow std::runtime_error(\"Control '\" + name + \"' not found\");\n> -\n> -\t\t\treturn it->first;\n> -\t\t}, py::return_value_policy::reference_internal)\n> -\n> \t\t.def_property_readonly(\"controls\", [](Camera &self) {\n> -\t\t\tpy::dict ret;\n> +\t\t\t/* Convert ControlInfoMap to std container */\n> \n> -\t\t\tfor (const auto &[id, ci] : self.controls()) {\n> -\t\t\t\tret[id->name().c_str()] = std::make_tuple(controlValueToPy(ci.min()),\n> -\t\t\t\t\t\t\t\t\t\t controlValueToPy(ci.max()),\n> -\t\t\t\t\t\t\t\t\t\t controlValueToPy(ci.def()));\n> -\t\t\t}\n> +\t\t\tstd::unordered_map ret;\n> +\n> +\t\t\tfor (const auto &[k, cv] : self.controls())\n> +\t\t\t\tret[k] = cv;\n> \n> \t\t\treturn ret;\n> \t\t})\n> \n> \t\t.def_property_readonly(\"properties\", [](Camera &self) {\n> -\t\t\tpy::dict ret;\n> +\t\t\t/* Convert ControlList to std container */\n> \n> -\t\t\tfor (const auto &[key, cv] : self.properties()) {\n> -\t\t\t\tconst ControlId *id = properties::properties.at(key);\n> -\t\t\t\tpy::object ob = controlValueToPy(cv);\n> +\t\t\tstd::unordered_map ret;\n> \n> -\t\t\t\tret[id->name().c_str()] = ob;\n> +\t\t\tfor (const auto &[k, cv] : self.properties()) {\n> +\t\t\t\tconst ControlId *id = properties::properties.at(k);\n> +\t\t\t\tpy::object ob = controlValueToPy(cv);\n> +\t\t\t\tret[id] = ob;\n> \t\t\t}\n> \n> \t\t\treturn ret;\n> @@ -464,7 +440,34 @@ PYBIND11_MODULE(_libcamera, m)\n> \tpyControlId\n> \t\t.def_property_readonly(\"id\", &ControlId::id)\n> \t\t.def_property_readonly(\"name\", &ControlId::name)\n> -\t\t.def_property_readonly(\"type\", &ControlId::type);\n> +\t\t.def_property_readonly(\"type\", &ControlId::type)\n> +\t\t.def(\"__str__\", [](const ControlId &self) { return self.name(); })\n> +\t\t.def(\"__repr__\", [](const ControlId &self) {\n> +\t\t\treturn py::str(\"libcamera.ControlId({}, {}, {})\")\n> +\t\t\t\t.format(self.id(), self.name(), self.type());\n\nShould this be py::str(\"libcamera.controls.{}\").format(self.name()) ?\nThat may not work for draft controls though.\n\n> +\t\t});\n> +\n> +\tpyControlInfo\n> +\t\t.def_property_readonly(\"min\", [](const ControlInfo& self) {\n\ns/& self/ &self/\n\nSame below.\n\nReviewed-by: Laurent Pinchart \n\n\n> +\t\t\treturn controlValueToPy(self.min());\n> +\t\t})\n> +\t\t.def_property_readonly(\"max\", [](const ControlInfo& self) {\n> +\t\t\treturn controlValueToPy(self.max());\n> +\t\t})\n> +\t\t.def_property_readonly(\"default\", [](const ControlInfo& self) {\n> +\t\t\treturn controlValueToPy(self.def());\n> +\t\t})\n> +\t\t.def_property_readonly(\"values\", [](const ControlInfo& self) {\n> +\t\t\tpy::list l;\n> +\t\t\tfor (const auto &v : self.values())\n> +\t\t\t\tl.append(controlValueToPy(v));\n> +\t\t\treturn l;\n> +\t\t})\n> +\t\t.def(\"__str__\", &ControlInfo::toString)\n> +\t\t.def(\"__repr__\", [](const ControlInfo& self) {\n> +\t\t\treturn py::str(\"libcamera.ControlInfo({})\")\n> +\t\t\t\t.format(self.toString());\n> +\t\t});\n> \n> \tpyRequest\n> \t\t/* \\todo Fence is not supported, so we cannot expose addBuffer() directly */\n> @@ -475,17 +478,18 @@ PYBIND11_MODULE(_libcamera, m)\n> \t\t.def_property_readonly(\"buffers\", &Request::buffers)\n> \t\t.def_property_readonly(\"cookie\", &Request::cookie)\n> \t\t.def_property_readonly(\"has_pending_buffers\", &Request::hasPendingBuffers)\n> -\t\t.def(\"set_control\", [](Request &self, ControlId &id, py::object value) {\n> +\t\t.def(\"set_control\", [](Request &self, const ControlId &id, py::object value) {\n> \t\t\tself.controls().set(id.id(), pyToControlValue(value, id.type()));\n> \t\t})\n> \t\t.def_property_readonly(\"metadata\", [](Request &self) {\n> -\t\t\tpy::dict ret;\n> +\t\t\t/* Convert ControlList to std container */\n> +\n> +\t\t\tstd::unordered_map ret;\n> \n> \t\t\tfor (const auto &[key, cv] : self.metadata()) {\n> \t\t\t\tconst ControlId *id = controls::controls.at(key);\n> \t\t\t\tpy::object ob = controlValueToPy(cv);\n> -\n> -\t\t\t\tret[id->name().c_str()] = ob;\n> +\t\t\t\tret[id] = ob;\n> \t\t\t}\n> \n> \t\t\treturn ret;","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 2D5A0BD161\n\tfor ;\n\tFri, 27 May 2022 10:52:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6B137633A5;\n\tFri, 27 May 2022 12:52:54 +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 EC85A633A2\n\tfor ;\n\tFri, 27 May 2022 12:52:52 +0200 (CEST)","from pendragon.ideasonboard.com (unknown [46.183.103.8])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 524DA31A;\n\tFri, 27 May 2022 12:52:50 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1653648774;\n\tbh=D9BFXieGtP+cGcXkbi8HuGQeXaQqebtcZuNVYK0Ti0A=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=fLQ2DLmQBqTazss0bspuwo/aNbpKW1G23G21J9O0ulK+HJRqfA0R1mGSOtPg99sO5\n\t7Lr6nNXNJFuwW2mBGWgHitqKMeDlrfaFAyIJdVfow2yz74Ly0ITt+V6vkKM2+Eu3Fg\n\tyMvpctGTJulk0An0DjaT+24G72gAY35qYRPEajXW8gdd9WV0GUUs47qCFqzIEL7Ia+\n\t6bflOAAEIUK7v9dh2eSVSTnA96gQBFaVa5BNAU+5XnRBAbKGw7rFbMezMhKy5Pq6C3\n\tWjrrkf3ZQUJeSvq7e3Tpv3W935Nj1zko2ug1tB8U1dh+VBwc0e8R/sHeKwe/uoUcu4\n\tAbZAC4Jzz2WYg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1653648772;\n\tbh=D9BFXieGtP+cGcXkbi8HuGQeXaQqebtcZuNVYK0Ti0A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=wO9iT+WVJXl9fWwm72sRZcJlYimcsk4SwwlzQv/3/Ni1K2BDW+KOYvRAI99RamSnk\n\thPNGcdhULlilGjLY8roiiTlClI+OJhrwWo73s2tU63PIYAUkU0YB0PzL+x8pEAi2Ej\n\tCh7Jf4XjuBP9meRl7AwNzd8OvV3X+KusuieQ60NA="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"wO9iT+WV\"; dkim-atps=neutral","Date":"Fri, 27 May 2022 13:52:34 +0300","To":"Tomi Valkeinen ","Message-ID":"","References":"<20220524114610.41848-1-tomi.valkeinen@ideasonboard.com>\n\t<20220524114610.41848-17-tomi.valkeinen@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220524114610.41848-17-tomi.valkeinen@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 16/19] py: Re-structure the\n\tcontrols API","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23198,"web_url":"https://patchwork.libcamera.org/comment/23198/","msgid":"<4e5deacb-5480-0231-c9a2-8d57fc2a00fe@ideasonboard.com>","date":"2022-05-27T13:16:55","subject":"Re: [libcamera-devel] [PATCH v2 16/19] py: Re-structure the\n\tcontrols API","submitter":{"id":109,"url":"https://patchwork.libcamera.org/api/people/109/","name":"Tomi Valkeinen","email":"tomi.valkeinen@ideasonboard.com"},"content":"On 27/05/2022 13:52, Laurent Pinchart wrote:\n\n>> @@ -464,7 +440,34 @@ PYBIND11_MODULE(_libcamera, m)\n>> \tpyControlId\n>> \t\t.def_property_readonly(\"id\", &ControlId::id)\n>> \t\t.def_property_readonly(\"name\", &ControlId::name)\n>> -\t\t.def_property_readonly(\"type\", &ControlId::type);\n>> +\t\t.def_property_readonly(\"type\", &ControlId::type)\n>> +\t\t.def(\"__str__\", [](const ControlId &self) { return self.name(); })\n>> +\t\t.def(\"__repr__\", [](const ControlId &self) {\n>> +\t\t\treturn py::str(\"libcamera.ControlId({}, {}, {})\")\n>> +\t\t\t\t.format(self.id(), self.name(), self.type());\n> \n> Should this be py::str(\"libcamera.controls.{}\").format(self.name()) ?\n> That may not work for draft controls though.\n\nDoesn't work for properties either.\n\nGenerally speaking, I'm not quite sure what to do with __repr__ in cases \nwhere you can't create the object yourself.\n\n Tomi","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 841F0BD160\n\tfor ;\n\tFri, 27 May 2022 13:17:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C4C97633A5;\n\tFri, 27 May 2022 15:17:01 +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 107CE633A2\n\tfor ;\n\tFri, 27 May 2022 15:17:00 +0200 (CEST)","from [192.168.1.111] (91-156-85-209.elisa-laajakaista.fi\n\t[91.156.85.209])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 24A0530A;\n\tFri, 27 May 2022 15:16:59 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1653657421;\n\tbh=jPvDQK/oSN9LtUz5oLASAppjbre5eUeCOEkpeX/01tY=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=W0T3cAbta6RWslxlGEcIc6DDdoBMOmbR+2Cy4RBNCWXIGIDOz9gfS3nnAC1ez+PMf\n\tOPNGKEjuAy8xQhq3DF0OwUdhxuLsxkwUIoBwEzYpXxlOU/yC6nN3OHzOxjclquJAk1\n\tgRR/nWxmxLrAw1hNwPgbpyXVRADf7xthRsKe01+shZRk5hKMk6dEpkvmsCF1rhODzw\n\t6Rbc5W6w4MoJ+sFwyR7+J7cd2pGz2C6AUsnZspaoZFXV5eR/ljdos2b1UkRyb8DtDW\n\t3nSLUkaTp7ltFcagAndXnJuoVHYAcnva8wjB4q+81x1ay+mVYVkXWN8WEc3miK09mj\n\tVXONJfXFPYcrA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1653657419;\n\tbh=jPvDQK/oSN9LtUz5oLASAppjbre5eUeCOEkpeX/01tY=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=cTqr0ZDx16WWTtdXnJFCwG5sIaLSNUJYLZe/ZMYFAs6gHbeHfHS3VXreb9Om1qYxN\n\tqW0vWuIJQNXr75Fl7w01sTCp4BNBV8OSbTVazPhhyoqAGzomcJJYJMAwxIWlXHcSF4\n\tNM3oIEo4TK8+hZbXaxemuBIbPoHblvIvBshOkJu0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"cTqr0ZDx\"; dkim-atps=neutral","Message-ID":"<4e5deacb-5480-0231-c9a2-8d57fc2a00fe@ideasonboard.com>","Date":"Fri, 27 May 2022 16:16:55 +0300","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.9.1","Content-Language":"en-US","To":"Laurent Pinchart ","References":"<20220524114610.41848-1-tomi.valkeinen@ideasonboard.com>\n\t<20220524114610.41848-17-tomi.valkeinen@ideasonboard.com>\n\t","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v2 16/19] py: Re-structure the\n\tcontrols API","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","From":"Tomi Valkeinen via libcamera-devel\n\t","Reply-To":"Tomi Valkeinen ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]