{"id":18661,"url":"https://patchwork.libcamera.org/api/patches/18661/?format=json","web_url":"https://patchwork.libcamera.org/patch/18661/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20230530092016.34953-6-tomi.valkeinen@ideasonboard.com>","date":"2023-05-30T09:20:16","name":"[libcamera-devel,5/5] py: Move to mainline pybind11 version","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"728ed8312aa85972ddfc7d75fd4147c974ffcce2","submitter":{"id":109,"url":"https://patchwork.libcamera.org/api/people/109/?format=json","name":"Tomi Valkeinen","email":"tomi.valkeinen@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/18661/mbox/","series":[{"id":3893,"url":"https://patchwork.libcamera.org/api/series/3893/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=3893","date":"2023-05-30T09:20:11","name":"py: Misc changes & move to mainline pybind11","version":1,"mbox":"https://patchwork.libcamera.org/series/3893/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/18661/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/18661/checks/","tags":{},"headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 B7B8BC3213\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 30 May 2023 09:21:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 787886287F;\n\tTue, 30 May 2023 11:21:03 +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 B831A6287D\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 30 May 2023 11:20:58 +0200 (CEST)","from desky.lan (91-154-35-171.elisa-laajakaista.fi [91.154.35.171])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AC3A07F3;\n\tTue, 30 May 2023 11:20:37 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685438463;\n\tbh=PltyImOLae7tYoV0O4X3/qeWYKQH+YvzD+u+TI6Fqls=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=b6+HObCkXMbJFyKePIIuOhJr59bT7mZoQM+EvQ5iWvmIL0vDn1StKtAxUnB9h7D2x\n\tK3jtvKmhi5jSP6ACKfdlantEwbGMgRlcJuWEvinVDn1+7n6ksodsJ1omFVUOd6wGbd\n\tqUmlziU3EQNLYnzIFvFWX/LdLtbtbj38rRBCulQIP6aWragNzh4fpBrs1reoDNDMA1\n\txTYsb3FxDwptqyrCI+Fugt6fjNlZ6IUwSfiXtV9dA0qBFDUmvIoD0OFvNnlTqpFh25\n\tQsunj+dFKLD7HMVdfkstOjEUvsbtreX5DViczuyOI5fZe8YCOog4ULNrRrprNurt7x\n\tyhe0AkaIzQgkA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685438438;\n\tbh=PltyImOLae7tYoV0O4X3/qeWYKQH+YvzD+u+TI6Fqls=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=EFvzvG+Kz9T9Hlb1Pm2/NCnmuHIwLFyymBJGkBGQvoUVLu4JQmlUoSZiJGqR4Jdwg\n\tK8/lccl/5K+xu7MratR5Ma2hb53BZchoqjE5QpUkWIOHw6CeJov5MgQZgSn6MSo5yp\n\t6CO6ZQ6MsYkpsJ3KXp1FQOBHlkkf3c5nN2nrcZp0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"EFvzvG+K\"; dkim-atps=neutral","To":"libcamera-devel@lists.libcamera.org","Date":"Tue, 30 May 2023 12:20:16 +0300","Message-Id":"<20230530092016.34953-6-tomi.valkeinen@ideasonboard.com>","X-Mailer":"git-send-email 2.34.1","In-Reply-To":"<20230530092016.34953-1-tomi.valkeinen@ideasonboard.com>","References":"<20230530092016.34953-1-tomi.valkeinen@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH 5/5] py: Move to mainline pybind11 version","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>","From":"Tomi Valkeinen via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"},"content":"We are using pybind11 'smart_holder' branch to solve the Camera\ndestructor issue (see the comment in this patch, or the commit\nthat originally added Python bindings support).\n\nAs it would be very nice to use the mainline pybind11 (which is packaged\nin distributions), this patch adds a workaround allowing us to move to\nthe mainline pybind11 version.\n\nThe workaround is simply creating a custom holder class, used only for\nthe Camera, which wraps around the shared_ptr. This makes the compiler\nhappy.\n\nMoving to mainline pybdin11 is achieved with:\n\n- Change the pybind11 wrap to point to the mainline pybdind11 version\n\n- Change the meson.build file to use a system-installed pybind11 if\n  available, and use the pybind11 subproject as a fallback. The fallback\n  is there as a convenience, as pybind11 may not be available in\n  pre-packaged form in all environments.\n\nSigned-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>\n---\n src/py/libcamera/meson.build                  | 11 ++--\n src/py/libcamera/py_camera_manager.h          |  2 +-\n src/py/libcamera/py_color_space.cpp           |  2 +-\n src/py/libcamera/py_controls_generated.cpp.in |  2 +-\n src/py/libcamera/py_enums.cpp                 |  2 +-\n src/py/libcamera/py_formats_generated.cpp.in  |  2 +-\n src/py/libcamera/py_geometry.cpp              |  2 +-\n src/py/libcamera/py_helpers.h                 |  2 +-\n src/py/libcamera/py_main.cpp                  | 52 +++++++++++++++++--\n .../libcamera/py_properties_generated.cpp.in  |  2 +-\n src/py/libcamera/py_transform.cpp             |  2 +-\n subprojects/.gitignore                        |  2 +-\n subprojects/pybind11.wrap                     | 18 ++++---\n 13 files changed, 76 insertions(+), 25 deletions(-)","diff":"diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build\nindex f87b1b4d..9d86d8bf 100644\n--- a/src/py/libcamera/meson.build\n+++ b/src/py/libcamera/meson.build\n@@ -7,10 +7,15 @@ if not py3_dep.found()\n     subdir_done()\n endif\n \n-pycamera_enabled = true\n+pybind11_dep = dependency('pybind11', required : get_option('pycamera'),\n+                          fallback : ['pybind11', 'pybind11_dep'])\n+\n+if not pybind11_dep.found()\n+    pycamera_enabled = false\n+    subdir_done()\n+endif\n \n-pybind11_proj = subproject('pybind11')\n-pybind11_dep = pybind11_proj.get_variable('pybind11_dep')\n+pycamera_enabled = true\n \n pycamera_sources = files([\n     'py_camera_manager.cpp',\ndiff --git a/src/py/libcamera/py_camera_manager.h b/src/py/libcamera/py_camera_manager.h\nindex 3525057d..3574db23 100644\n--- a/src/py/libcamera/py_camera_manager.h\n+++ b/src/py/libcamera/py_camera_manager.h\n@@ -9,7 +9,7 @@\n \n #include <libcamera/libcamera.h>\n \n-#include <pybind11/smart_holder.h>\n+#include <pybind11/pybind11.h>\n \n using namespace libcamera;\n \ndiff --git a/src/py/libcamera/py_color_space.cpp b/src/py/libcamera/py_color_space.cpp\nindex a8301e3d..5201121a 100644\n--- a/src/py/libcamera/py_color_space.cpp\n+++ b/src/py/libcamera/py_color_space.cpp\n@@ -9,7 +9,7 @@\n #include <libcamera/libcamera.h>\n \n #include <pybind11/operators.h>\n-#include <pybind11/smart_holder.h>\n+#include <pybind11/pybind11.h>\n #include <pybind11/stl.h>\n \n namespace py = pybind11;\ndiff --git a/src/py/libcamera/py_controls_generated.cpp.in b/src/py/libcamera/py_controls_generated.cpp.in\nindex cb8442ba..18fa57d9 100644\n--- a/src/py/libcamera/py_controls_generated.cpp.in\n+++ b/src/py/libcamera/py_controls_generated.cpp.in\n@@ -9,7 +9,7 @@\n \n #include <libcamera/control_ids.h>\n \n-#include <pybind11/smart_holder.h>\n+#include <pybind11/pybind11.h>\n \n namespace py = pybind11;\n \ndiff --git a/src/py/libcamera/py_enums.cpp b/src/py/libcamera/py_enums.cpp\nindex 96d4beef..803c4e7e 100644\n--- a/src/py/libcamera/py_enums.cpp\n+++ b/src/py/libcamera/py_enums.cpp\n@@ -7,7 +7,7 @@\n \n #include <libcamera/libcamera.h>\n \n-#include <pybind11/smart_holder.h>\n+#include <pybind11/pybind11.h>\n \n namespace py = pybind11;\n \ndiff --git a/src/py/libcamera/py_formats_generated.cpp.in b/src/py/libcamera/py_formats_generated.cpp.in\nindex b88807f3..a3f7f94d 100644\n--- a/src/py/libcamera/py_formats_generated.cpp.in\n+++ b/src/py/libcamera/py_formats_generated.cpp.in\n@@ -9,7 +9,7 @@\n \n #include <libcamera/formats.h>\n \n-#include <pybind11/smart_holder.h>\n+#include <pybind11/pybind11.h>\n \n namespace py = pybind11;\n \ndiff --git a/src/py/libcamera/py_geometry.cpp b/src/py/libcamera/py_geometry.cpp\nindex 84b0cb08..5c2aeac4 100644\n--- a/src/py/libcamera/py_geometry.cpp\n+++ b/src/py/libcamera/py_geometry.cpp\n@@ -11,7 +11,7 @@\n #include <libcamera/libcamera.h>\n \n #include <pybind11/operators.h>\n-#include <pybind11/smart_holder.h>\n+#include <pybind11/pybind11.h>\n #include <pybind11/stl.h>\n \n namespace py = pybind11;\ndiff --git a/src/py/libcamera/py_helpers.h b/src/py/libcamera/py_helpers.h\nindex cd31e2cc..983969df 100644\n--- a/src/py/libcamera/py_helpers.h\n+++ b/src/py/libcamera/py_helpers.h\n@@ -7,7 +7,7 @@\n \n #include <libcamera/libcamera.h>\n \n-#include <pybind11/smart_holder.h>\n+#include <pybind11/pybind11.h>\n \n pybind11::object controlValueToPy(const libcamera::ControlValue &cv);\n libcamera::ControlValue pyToControlValue(const pybind11::object &ob, libcamera::ControlType type);\ndiff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\nindex c66b90cd..bbae7a99 100644\n--- a/src/py/libcamera/py_main.cpp\n+++ b/src/py/libcamera/py_main.cpp\n@@ -17,7 +17,7 @@\n #include <libcamera/libcamera.h>\n \n #include <pybind11/functional.h>\n-#include <pybind11/smart_holder.h>\n+#include <pybind11/pybind11.h>\n #include <pybind11/stl.h>\n #include <pybind11/stl_bind.h>\n \n@@ -33,6 +33,50 @@ namespace libcamera {\n LOG_DEFINE_CATEGORY(Python)\n \n }\n+/*\n+ * This is a holder class used only for the Camera class, for the sole purpose\n+ * of avoiding the compilation issue with Camera's private destructor.\n+ *\n+ * pybind11 requires a public destructor for classes held with shared_ptrs, even\n+ * in cases where the public destructor is not strictly needed. The current\n+ * understanding is that there are the following options to solve the problem:\n+ *\n+ * - Use pybind11 'smart_holder' branch. The downside is that 'smart_holder'\n+ *   is not the mainline branch, and not available in distributions.\n+ * - https://github.com/pybind/pybind11/pull/2067\n+ * - Make the Camera destructor public\n+ * - Something like the PyCameraSmartPtr here, which adds a layer, hiding the\n+ *   issue.\n+ */\n+template<typename T>\n+class PyCameraSmartPtr\n+{\n+public:\n+\tusing element_type = T;\n+\n+\tPyCameraSmartPtr()\n+\t{\n+\t}\n+\n+\texplicit PyCameraSmartPtr(T *)\n+\t{\n+\t\tthrow std::runtime_error(\"invalid SmartPtr constructor call\");\n+\t}\n+\n+\texplicit PyCameraSmartPtr(std::shared_ptr<T> p)\n+\t\t: ptr_(p)\n+\t{\n+\t}\n+\n+\tT *get() const { return ptr_.get(); }\n+\n+\toperator std::shared_ptr<T>() const { return ptr_; }\n+\n+private:\n+\tstd::shared_ptr<T> ptr_;\n+};\n+\n+PYBIND11_DECLARE_HOLDER_TYPE(T, PyCameraSmartPtr<T>);\n \n /*\n  * Note: global C++ destructors can be ran on this before the py module is\n@@ -65,8 +109,8 @@ PYBIND11_MODULE(_libcamera, m)\n \t * https://pybind11.readthedocs.io/en/latest/advanced/misc.html#avoiding-c-types-in-docstrings\n \t */\n \n-\tauto pyCameraManager = py::class_<PyCameraManager>(m, \"CameraManager\");\n-\tauto pyCamera = py::class_<Camera>(m, \"Camera\");\n+\tauto pyCameraManager = py::class_<PyCameraManager, std::shared_ptr<PyCameraManager>>(m, \"CameraManager\");\n+\tauto pyCamera = py::class_<Camera, PyCameraSmartPtr<Camera>>(m, \"Camera\");\n \tauto pyCameraConfiguration = py::class_<CameraConfiguration>(m, \"CameraConfiguration\");\n \tauto pyCameraConfigurationStatus = py::enum_<CameraConfiguration::Status>(pyCameraConfiguration, \"Status\");\n \tauto pyStreamConfiguration = py::class_<StreamConfiguration>(m, \"StreamConfiguration\");\n@@ -271,7 +315,7 @@ PYBIND11_MODULE(_libcamera, m)\n \t\t.def(\"range\", &StreamFormats::range);\n \n \tpyFrameBufferAllocator\n-\t\t.def(py::init<std::shared_ptr<Camera>>(), py::keep_alive<1, 2>())\n+\t\t.def(py::init<PyCameraSmartPtr<Camera>>(), py::keep_alive<1, 2>())\n \t\t.def(\"allocate\", [](FrameBufferAllocator &self, Stream *stream) {\n \t\t\tint ret = self.allocate(stream);\n \t\t\tif (ret < 0)\ndiff --git a/src/py/libcamera/py_properties_generated.cpp.in b/src/py/libcamera/py_properties_generated.cpp.in\nindex 044b2b2a..e49b6e91 100644\n--- a/src/py/libcamera/py_properties_generated.cpp.in\n+++ b/src/py/libcamera/py_properties_generated.cpp.in\n@@ -9,7 +9,7 @@\n \n #include <libcamera/property_ids.h>\n \n-#include <pybind11/smart_holder.h>\n+#include <pybind11/pybind11.h>\n \n namespace py = pybind11;\n \ndiff --git a/src/py/libcamera/py_transform.cpp b/src/py/libcamera/py_transform.cpp\nindex 08783e29..f3a0bfaf 100644\n--- a/src/py/libcamera/py_transform.cpp\n+++ b/src/py/libcamera/py_transform.cpp\n@@ -9,7 +9,7 @@\n #include <libcamera/libcamera.h>\n \n #include <pybind11/operators.h>\n-#include <pybind11/smart_holder.h>\n+#include <pybind11/pybind11.h>\n #include <pybind11/stl.h>\n \n namespace py = pybind11;\ndiff --git a/subprojects/.gitignore b/subprojects/.gitignore\nindex 0a8fd3a6..ebe59479 100644\n--- a/subprojects/.gitignore\n+++ b/subprojects/.gitignore\n@@ -4,4 +4,4 @@\n /libyaml\n /libyuv\n /packagecache\n-/pybind11\n+/pybind11-*\ndiff --git a/subprojects/pybind11.wrap b/subprojects/pybind11.wrap\nindex dd02687b..96ec57a1 100644\n--- a/subprojects/pybind11.wrap\n+++ b/subprojects/pybind11.wrap\n@@ -1,11 +1,13 @@\n-# SPDX-License-Identifier: CC0-1.0\n-\n-[wrap-git]\n-url = https://github.com/pybind/pybind11.git\n-# This is the head of 'smart_holder' branch\n-revision = aebdf00cd060b871c5a1e0c2cf4a333503dd0431\n-depth = 1\n-patch_directory = pybind11\n+[wrap-file]\n+directory = pybind11-2.10.4\n+source_url = https://github.com/pybind/pybind11/archive/refs/tags/v2.10.4.tar.gz\n+source_filename = pybind11-2.10.4.tar.gz\n+source_hash = 832e2f309c57da9c1e6d4542dedd34b24e4192ecb4d62f6f4866a737454c9970\n+patch_filename = pybind11_2.10.4-1_patch.zip\n+patch_url = https://wrapdb.mesonbuild.com/v2/pybind11_2.10.4-1/get_patch\n+patch_hash = 9489d0cdc1244078a3108c52b4591a6f07f3dc30ca7299d3a3c42b84fa763396\n+source_fallback_url = https://github.com/mesonbuild/wrapdb/releases/download/pybind11_2.10.4-1/pybind11-2.10.4.tar.gz\n+wrapdb_version = 2.10.4-1\n \n [provide]\n pybind11 = pybind11_dep\n","prefixes":["libcamera-devel","5/5"]}