[{"id":23015,"web_url":"https://patchwork.libcamera.org/comment/23015/","msgid":"<165280805593.2416244.14811808292332482855@Monstersaurus>","date":"2022-05-17T17:20:55","subject":"Re: [libcamera-devel] [PATCH v2 13/13] py: use geometry classes","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Tomi Valkeinen (2022-05-17 15:33:25)\n> Now that we have proper geometry classes in the Python bindings, change\n> the existing bindings and the .py files accordingly.\n> \n> Signed-off-by: Tomi Valkeinen \n> Reviewed-by: Laurent Pinchart \n> ---\n> src/py/cam/cam.py | 7 +++++--\n> src/py/cam/cam_kms.py | 3 ++-\n> src/py/cam/cam_qt.py | 7 ++++---\n> src/py/cam/cam_qtgl.py | 3 ++-\n> src/py/libcamera/pymain.cpp | 41 ++++++++-----------------------------\n> 5 files changed, 22 insertions(+), 39 deletions(-)\n> \n> diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py\n> index 001fb9de..2f0690b5 100755\n> --- a/src/py/cam/cam.py\n> +++ b/src/py/cam/cam.py\n> @@ -160,8 +160,11 @@ def configure(ctx):\n> for idx, stream_opts in enumerate(streams):\n> stream_config = camconfig.at(idx)\n> \n> - if 'width' in stream_opts and 'height' in stream_opts:\n> - stream_config.size = (stream_opts['width'], stream_opts['height'])\n> + if 'width' in stream_opts:\n> + stream_config.size.width = stream_opts['width']\n> +\n> + if 'height' in stream_opts:\n> + stream_config.size.height = stream_opts['height']\n\nWhat happens here if only one of width, or height is set. Can it be\nenforced to set both? Actually - I suspect the validation phase should\nchoose a (probably small, but more correct) configuration.\n\nIt looks like a good cleanup otherwise though.\n\n\nReviewed-by: Kieran Bingham \n\n\n> \n> if 'pixelformat' in stream_opts:\n> stream_config.pixel_format = libcam.PixelFormat(stream_opts['pixelformat'])\n> diff --git a/src/py/cam/cam_kms.py b/src/py/cam/cam_kms.py\n> index 04381da1..74cd3b38 100644\n> --- a/src/py/cam/cam_kms.py\n> +++ b/src/py/cam/cam_kms.py\n> @@ -126,7 +126,8 @@ class KMSRenderer:\n> })\n> \n> for fb in ctx['allocator'].buffers(stream):\n> - w, h = cfg.size\n> + w = cfg.size.width\n> + h = cfg.size.height\n> fds = []\n> strides = []\n> offsets = []\n> diff --git a/src/py/cam/cam_qt.py b/src/py/cam/cam_qt.py\n> index 45a30aeb..91be2a08 100644\n> --- a/src/py/cam/cam_qt.py\n> +++ b/src/py/cam/cam_qt.py\n> @@ -84,8 +84,8 @@ def demosaic(data, r0, g0, g1, b0):\n> \n> \n> def to_rgb(fmt, size, data):\n> - w = size[0]\n> - h = size[1]\n> + w = size.width\n> + h = size.height\n> \n> fmt = str(fmt)\n> \n> @@ -292,7 +292,8 @@ class MainWindow(QtWidgets.QWidget):\n> def buf_to_qpixmap(self, stream, fb):\n> with fb.mmap() as mfb:\n> cfg = stream.configuration\n> - w, h = cfg.size\n> + w = cfg.size.width\n> + h = cfg.size.height\n> pitch = cfg.stride\n> \n> if str(cfg.pixel_format) == 'MJPEG':\n> diff --git a/src/py/cam/cam_qtgl.py b/src/py/cam/cam_qtgl.py\n> index 261accb8..4bbcda6c 100644\n> --- a/src/py/cam/cam_qtgl.py\n> +++ b/src/py/cam/cam_qtgl.py\n> @@ -268,7 +268,8 @@ class MainWindow(QtWidgets.QWidget):\n> def create_texture(self, stream, fb):\n> cfg = stream.configuration\n> fmt = cfg.pixel_format.fourcc\n> - w, h = cfg.size\n> + w = cfg.size.width\n> + h = cfg.size.height\n> \n> attribs = [\n> EGL_WIDTH, w,\n> diff --git a/src/py/libcamera/pymain.cpp b/src/py/libcamera/pymain.cpp\n> index 96333ebc..ef3f157a 100644\n> --- a/src/py/libcamera/pymain.cpp\n> +++ b/src/py/libcamera/pymain.cpp\n> @@ -6,7 +6,6 @@\n> */\n> \n> /*\n> - * \\todo Add geometry classes (Point, Rectangle...)\n> * \\todo Add bindings for the ControlInfo class\n> */\n> \n> @@ -61,11 +60,11 @@ static py::object controlValueToPy(const ControlValue &cv)\n> return py::cast(cv.get());\n> case ControlTypeRectangle: {\n> const Rectangle *v = reinterpret_cast(cv.data().data());\n> - return py::make_tuple(v->x, v->y, v->width, v->height);\n> + return py::cast(v);\n> }\n> case ControlTypeSize: {\n> const Size *v = reinterpret_cast(cv.data().data());\n> - return py::make_tuple(v->width, v->height);\n> + return py::cast(v);\n> }\n> case ControlTypeNone:\n> default:\n> @@ -99,14 +98,10 @@ static ControlValue pyToControlValue(const py::object &ob, ControlType type)\n> return controlValueMaybeArray(ob);\n> case ControlTypeString:\n> return ControlValue(ob.cast());\n> - case ControlTypeRectangle: {\n> - auto array = ob.cast>();\n> - return ControlValue(Rectangle(array[0], array[1], array[2], array[3]));\n> - }\n> - case ControlTypeSize: {\n> - auto array = ob.cast>();\n> - return ControlValue(Size(array[0], array[1]));\n> - }\n> + case ControlTypeRectangle:\n> + return ControlValue(ob.cast());\n> + case ControlTypeSize:\n> + return ControlValue(ob.cast());\n> case ControlTypeNone:\n> default:\n> throw std::runtime_error(\"Control type not implemented\");\n> @@ -397,15 +392,7 @@ PYBIND11_MODULE(_libcamera, m)\n> .def(\"__str__\", &StreamConfiguration::toString)\n> .def_property_readonly(\"stream\", &StreamConfiguration::stream,\n> py::return_value_policy::reference_internal)\n> - .def_property(\n> - \"size\",\n> - [](StreamConfiguration &self) {\n> - return std::make_tuple(self.size.width, self.size.height);\n> - },\n> - [](StreamConfiguration &self, std::tuple size) {\n> - self.size.width = std::get<0>(size);\n> - self.size.height = std::get<1>(size);\n> - })\n> + .def_readwrite(\"size\", &StreamConfiguration::size)\n> .def_readwrite(\"pixel_format\", &StreamConfiguration::pixelFormat)\n> .def_readwrite(\"stride\", &StreamConfiguration::stride)\n> .def_readwrite(\"frame_size\", &StreamConfiguration::frameSize)\n> @@ -416,18 +403,8 @@ PYBIND11_MODULE(_libcamera, m)\n> \n> pyStreamFormats\n> .def_property_readonly(\"pixel_formats\", &StreamFormats::pixelformats)\n> - .def(\"sizes\", [](StreamFormats &self, const PixelFormat &pixelFormat) {\n> - std::vector> fmts;\n> - for (const auto &s : self.sizes(pixelFormat))\n> - fmts.push_back(std::make_tuple(s.width, s.height));\n> - return fmts;\n> - })\n> - .def(\"range\", [](StreamFormats &self, const PixelFormat &pixelFormat) {\n> - const auto &range = self.range(pixelFormat);\n> - return make_tuple(std::make_tuple(range.hStep, range.vStep),\n> - std::make_tuple(range.min.width, range.min.height),\n> - std::make_tuple(range.max.width, range.max.height));\n> - });\n> + .def(\"sizes\", &StreamFormats::sizes)\n> + .def(\"range\", &StreamFormats::range);\n> \n> pyFrameBufferAllocator\n> .def(py::init>(), py::keep_alive<1, 2>())\n> -- \n> 2.34.1\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 EEE46C0F2A\n\tfor ;\n\tTue, 17 May 2022 17:20:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5059765659;\n\tTue, 17 May 2022 19:20:59 +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 A3B006041D\n\tfor ;\n\tTue, 17 May 2022 19:20:58 +0200 (CEST)","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 F39614A8;\n\tTue, 17 May 2022 19:20:57 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1652808059;\n\tbh=wMcz0/iSM5I3IL2l1Z0kyYVbaePSZEMtBis/ky9xYII=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=Myqec9SyYwdMIionC3KvZffW92TuR/WPvNa1mH2LWJK5rN7IjS7mWO8TiKJSTtUGs\n\t0XY0qStl1ruuU5XUxCg4r8y1CAa41P5X0K1Qwe8pesimCvMsFocJz8ZPcoegvUxnLP\n\tYuTb8qV2OdqXhkKrjj4AUivYTJwVZq/H4Ly03wWUYINsrT96FMK8bdb6PpwyzAqZQ1\n\tkE0tgttkIOT+Q3V2uJ79IAR1jdcvgcPi2SN3ZN+idHjWbA/utW5y7DpvSDY92hcvRe\n\tKxdRBs6o3JS3mwlJQsU4AnnUWL+P+yDhvgPgrY5W87KMkI7L1NWV+y/X8YmimUngGq\n\t0BbJGU0V2PV/Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1652808058;\n\tbh=wMcz0/iSM5I3IL2l1Z0kyYVbaePSZEMtBis/ky9xYII=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=gzXw5ORV7XnzXkp2J6Wz+QOcdazJAevLx4Iks239bJiSumSg/96lYO3/Vt8gFGh3Q\n\tzrvF08erNM8EFHvEDwivI408B+nA5hzDwRh57AvzpYlIAOlvtpi4DdahhvqjuWM9fp\n\tKxgUrSkQP6C15BQiQ7Y6yscSWaWRzUO08LzuoOeo="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"gzXw5ORV\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220517143325.71784-14-tomi.valkeinen@ideasonboard.com>","References":"<20220517143325.71784-1-tomi.valkeinen@ideasonboard.com>\n\t<20220517143325.71784-14-tomi.valkeinen@ideasonboard.com>","To":"David Plowman ,\n\tJacopo Mondi ,\n\tLaurent Pinchart ,\n\tTomi Valkeinen ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 17 May 2022 18:20:55 +0100","Message-ID":"<165280805593.2416244.14811808292332482855@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 13/13] py: use geometry classes","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":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23030,"web_url":"https://patchwork.libcamera.org/comment/23030/","msgid":"<630b9229-ed49-2129-c941-99d6cdb1fba9@ideasonboard.com>","date":"2022-05-18T06:57:09","subject":"Re: [libcamera-devel] [PATCH v2 13/13] py: use geometry classes","submitter":{"id":109,"url":"https://patchwork.libcamera.org/api/people/109/","name":"Tomi Valkeinen","email":"tomi.valkeinen@ideasonboard.com"},"content":"On 17/05/2022 20:20, Kieran Bingham wrote:\n> Quoting Tomi Valkeinen (2022-05-17 15:33:25)\n>> Now that we have proper geometry classes in the Python bindings, change\n>> the existing bindings and the .py files accordingly.\n>>\n>> Signed-off-by: Tomi Valkeinen \n>> Reviewed-by: Laurent Pinchart \n>> ---\n>> src/py/cam/cam.py | 7 +++++--\n>> src/py/cam/cam_kms.py | 3 ++-\n>> src/py/cam/cam_qt.py | 7 ++++---\n>> src/py/cam/cam_qtgl.py | 3 ++-\n>> src/py/libcamera/pymain.cpp | 41 ++++++++-----------------------------\n>> 5 files changed, 22 insertions(+), 39 deletions(-)\n>>\n>> diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py\n>> index 001fb9de..2f0690b5 100755\n>> --- a/src/py/cam/cam.py\n>> +++ b/src/py/cam/cam.py\n>> @@ -160,8 +160,11 @@ def configure(ctx):\n>> for idx, stream_opts in enumerate(streams):\n>> stream_config = camconfig.at(idx)\n>> \n>> - if 'width' in stream_opts and 'height' in stream_opts:\n>> - stream_config.size = (stream_opts['width'], stream_opts['height'])\n>> + if 'width' in stream_opts:\n>> + stream_config.size.width = stream_opts['width']\n>> +\n>> + if 'height' in stream_opts:\n>> + stream_config.size.height = stream_opts['height']\n> \n> What happens here if only one of width, or height is set. Can it be\n> enforced to set both? Actually - I suspect the validation phase should\n> choose a (probably small, but more correct) configuration.\n\nYes, I think it picks up some default value for the other parameter. \nMaybe whatever is currently configured.\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 1E649C3256\n\tfor ;\n\tWed, 18 May 2022 06:57:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5474F65656;\n\tWed, 18 May 2022 08:57:14 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B6F4860422\n\tfor ;\n\tWed, 18 May 2022 08:57:12 +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 E88C248F;\n\tWed, 18 May 2022 08:57:11 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1652857034;\n\tbh=liq00H8abj13/dlSswZ5DRUvxOb9fdqatiLgMRE3pNs=;\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:\n\tFrom;\n\tb=EsieLU/BGYAapg9xcK9sOE6Fr3WdhHgIVHxL7ANMMiD0A1CUYjQnhYiVs46KN4E0j\n\tp66x9FaTuNzbRwdwPfrrlDHpVMW4drf7QtMeUm+CrlvVp+OzHj7gnIWzixStDOzA8w\n\tU2HJThAgvA+eNPFQDAJP3p1+ZmP/B5O234ila/IiUa8NX3nuX+wqpsWv/i3UedUG3b\n\tcUvEDvNXJl/0YrqR8fkX5lphhXPwu6qFLkLps3qovyz9KnCS/l7eh4m2vg/IvMTBJJ\n\tn2h9jHsNJSp+4A1BXhiJ7O4OmuN43Djh58xEQxJ3HeBNR5M125S1mhLnlaYOh64Hkd\n\tEICuu9DOCWCwA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1652857032;\n\tbh=liq00H8abj13/dlSswZ5DRUvxOb9fdqatiLgMRE3pNs=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=fg7SsSee1sGTmWPwvDcFMmlV8bAJ7tz05WF4KHY8qLRL+YhAC+dmxvUS0lZ8gwcNR\n\tflQzke17rcVOWKebhtkiZcfYl4Wx3pQeHaavl6oif6z8jzV3ZcvTH3UACeEX7s4G74\n\toad3HITJ3cifALzxUjHecQWIIHchVcDevbbGDA9c="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"fg7SsSee\"; dkim-atps=neutral","Message-ID":"<630b9229-ed49-2129-c941-99d6cdb1fba9@ideasonboard.com>","Date":"Wed, 18 May 2022 09:57:09 +0300","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.8.0","Content-Language":"en-US","To":"Kieran Bingham ,\n\tDavid Plowman ,\n\tJacopo Mondi ,\n\tLaurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220517143325.71784-1-tomi.valkeinen@ideasonboard.com>\n\t<20220517143325.71784-14-tomi.valkeinen@ideasonboard.com>\n\t<165280805593.2416244.14811808292332482855@Monstersaurus>","In-Reply-To":"<165280805593.2416244.14811808292332482855@Monstersaurus>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v2 13/13] py: use geometry classes","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]