[RFC,v1] py: Use smart holder for `Camera` if available
diff mbox series

Message ID 20260519072152.346204-1-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • [RFC,v1] py: Use smart holder for `Camera` if available
Related show

Commit Message

Barnabás Pőcze May 19, 2026, 7:21 a.m. UTC
The `Camera` class has a private destructor, so `std::shared_ptr<>` cannot
be used as a holder type. In recent versions of pybind11, the custom holder
type that was used no longer works:

  RuntimeError: Unable to convert std::shared_ptr<T> to Python when the bound
  type does not use std::shared_ptr or py::smart_holder as its holder type

This appears to be intentional and the fact that it worked is accidental.

Fix that by using a "smart holder" since pybind11 3.x, and falling back to
the custom one in previous versions.

Link: https://github.com/pybind/pybind11/issues/6064
Closes: https://gitlab.freedesktop.org/camera/libcamera/-/work_items/334
---
 src/py/libcamera/py_main.cpp | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

Barnabás Pőcze May 19, 2026, 8:41 a.m. UTC | #1
2026. 05. 19. 9:21 keltezéssel, Barnabás Pőcze írta:
> The `Camera` class has a private destructor, so `std::shared_ptr<>` cannot
> be used as a holder type. In recent versions of pybind11, the custom holder
> type that was used no longer works:
> 
>    RuntimeError: Unable to convert std::shared_ptr<T> to Python when the bound
>    type does not use std::shared_ptr or py::smart_holder as its holder type
> 
> This appears to be intentional and the fact that it worked is accidental.
> 
> Fix that by using a "smart holder" since pybind11 3.x, and falling back to
> the custom one in previous versions.
> 
> Link: https://github.com/pybind/pybind11/issues/6064
> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/work_items/334
> ---

More context:

   * https://github.com/pybind/pybind11/issues/6064
   * https://github.com/pybind/pybind11/pull/6065
   * https://github.com/pybind/pybind11/pull/6066
   * https://github.com/pybind/pybind11/pull/6068
   * https://github.com/pybind/pybind11/pull/6069

and I have only tested this with pybind11 3.0.4.

>   src/py/libcamera/py_main.cpp | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
> index d0ef6915b..3b101f996 100644
> --- a/src/py/libcamera/py_main.cpp
> +++ b/src/py/libcamera/py_main.cpp
> @@ -34,6 +34,7 @@ LOG_DEFINE_CATEGORY(Python)
>   
>   }
>   
> +#if PYBIND11_VERSION_MAJOR < 3
>   /*
>    * This is a holder class used only for the Camera class, for the sole purpose
>    * of avoiding the compilation issue with Camera's private destructor.
> @@ -79,6 +80,13 @@ private:
>   
>   PYBIND11_DECLARE_HOLDER_TYPE(T, PyCameraSmartPtr<T>)
>   
> +using PyCameraHolder = PyCameraSmartPtr<Camera>;
> +using PyCameraArg = PyCameraSmartPtr<Camera>;
> +#else
> +using PyCameraHolder = py::smart_holder;
> +using PyCameraArg = std::shared_ptr<Camera>;
> +#endif
> +
>   /*
>    * Note: global C++ destructors can be ran on this before the py module is
>    * destructed.
> @@ -103,7 +111,7 @@ PYBIND11_MODULE(_libcamera, m)
>   	 */
>   
>   	auto pyCameraManager = py::class_<PyCameraManager, std::shared_ptr<PyCameraManager>>(m, "CameraManager");
> -	auto pyCamera = py::class_<Camera, PyCameraSmartPtr<Camera>>(m, "Camera");
> +	auto pyCamera = py::class_<Camera, PyCameraHolder>(m, "Camera");
>   	auto pySensorConfiguration = py::class_<SensorConfiguration>(m, "SensorConfiguration");
>   	auto pyCameraConfiguration = py::class_<CameraConfiguration>(m, "CameraConfiguration");
>   	auto pyCameraConfigurationStatus = py::enum_<CameraConfiguration::Status>(pyCameraConfiguration, "Status");
> @@ -347,7 +355,7 @@ PYBIND11_MODULE(_libcamera, m)
>   		.def("range", &StreamFormats::range);
>   
>   	pyFrameBufferAllocator
> -		.def(py::init<PyCameraSmartPtr<Camera>>(), py::keep_alive<1, 2>())
> +		.def(py::init<PyCameraArg>(), py::keep_alive<1, 2>())
>   		.def("allocate", [](FrameBufferAllocator &self, Stream *stream) {
>   			int ret = self.allocate(stream);
>   			if (ret < 0)
Tomi Valkeinen May 19, 2026, 8:55 a.m. UTC | #2
Hi,

On 5/19/26 09:21, Barnabás Pőcze wrote:
> The `Camera` class has a private destructor, so `std::shared_ptr<>` cannot
> be used as a holder type. In recent versions of pybind11, the custom holder
> type that was used no longer works:
> 
>    RuntimeError: Unable to convert std::shared_ptr<T> to Python when the bound
>    type does not use std::shared_ptr or py::smart_holder as its holder type
> 
> This appears to be intentional and the fact that it worked is accidental.
> 
> Fix that by using a "smart holder" since pybind11 3.x, and falling back to
> the custom one in previous versions.
> 
> Link:https://github.com/pybind/pybind11/issues/6064
> Closes:https://gitlab.freedesktop.org/camera/libcamera/-/work_items/334
> ---
>   src/py/libcamera/py_main.cpp | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)

Looks sensible to me. I tested on pybind11 2.11.1-2. At some point we 
can perhaps require pybind11 3.x, but probably not yet. Or switch to 
nanobind. Or if we get the C API, then the py side will be much simpler.

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

  Tomi
Laurent Pinchart May 20, 2026, 11:02 p.m. UTC | #3
Hi Barnabás,

Thank you for the patch.

On Tue, May 19, 2026 at 09:21:52AM +0200, Barnabás Pőcze wrote:
> The `Camera` class has a private destructor, so `std::shared_ptr<>` cannot
> be used as a holder type. In recent versions of pybind11, the custom holder
> type that was used no longer works:
> 
>   RuntimeError: Unable to convert std::shared_ptr<T> to Python when the bound
>   type does not use std::shared_ptr or py::smart_holder as its holder type
> 
> This appears to be intentional and the fact that it worked is accidental.
> 
> Fix that by using a "smart holder" since pybind11 3.x, and falling back to
> the custom one in previous versions.
> 
> Link: https://github.com/pybind/pybind11/issues/6064
> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/work_items/334

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/py/libcamera/py_main.cpp | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
> index d0ef6915b..3b101f996 100644
> --- a/src/py/libcamera/py_main.cpp
> +++ b/src/py/libcamera/py_main.cpp
> @@ -34,6 +34,7 @@ LOG_DEFINE_CATEGORY(Python)
>  
>  }
>  
> +#if PYBIND11_VERSION_MAJOR < 3
>  /*
>   * This is a holder class used only for the Camera class, for the sole purpose
>   * of avoiding the compilation issue with Camera's private destructor.
> @@ -79,6 +80,13 @@ private:
>  
>  PYBIND11_DECLARE_HOLDER_TYPE(T, PyCameraSmartPtr<T>)
>  
> +using PyCameraHolder = PyCameraSmartPtr<Camera>;
> +using PyCameraArg = PyCameraSmartPtr<Camera>;
> +#else
> +using PyCameraHolder = py::smart_holder;
> +using PyCameraArg = std::shared_ptr<Camera>;
> +#endif
> +
>  /*
>   * Note: global C++ destructors can be ran on this before the py module is
>   * destructed.
> @@ -103,7 +111,7 @@ PYBIND11_MODULE(_libcamera, m)
>  	 */
>  
>  	auto pyCameraManager = py::class_<PyCameraManager, std::shared_ptr<PyCameraManager>>(m, "CameraManager");
> -	auto pyCamera = py::class_<Camera, PyCameraSmartPtr<Camera>>(m, "Camera");
> +	auto pyCamera = py::class_<Camera, PyCameraHolder>(m, "Camera");
>  	auto pySensorConfiguration = py::class_<SensorConfiguration>(m, "SensorConfiguration");
>  	auto pyCameraConfiguration = py::class_<CameraConfiguration>(m, "CameraConfiguration");
>  	auto pyCameraConfigurationStatus = py::enum_<CameraConfiguration::Status>(pyCameraConfiguration, "Status");
> @@ -347,7 +355,7 @@ PYBIND11_MODULE(_libcamera, m)
>  		.def("range", &StreamFormats::range);
>  
>  	pyFrameBufferAllocator
> -		.def(py::init<PyCameraSmartPtr<Camera>>(), py::keep_alive<1, 2>())
> +		.def(py::init<PyCameraArg>(), py::keep_alive<1, 2>())
>  		.def("allocate", [](FrameBufferAllocator &self, Stream *stream) {
>  			int ret = self.allocate(stream);
>  			if (ret < 0)

Patch
diff mbox series

diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
index d0ef6915b..3b101f996 100644
--- a/src/py/libcamera/py_main.cpp
+++ b/src/py/libcamera/py_main.cpp
@@ -34,6 +34,7 @@  LOG_DEFINE_CATEGORY(Python)
 
 }
 
+#if PYBIND11_VERSION_MAJOR < 3
 /*
  * This is a holder class used only for the Camera class, for the sole purpose
  * of avoiding the compilation issue with Camera's private destructor.
@@ -79,6 +80,13 @@  private:
 
 PYBIND11_DECLARE_HOLDER_TYPE(T, PyCameraSmartPtr<T>)
 
+using PyCameraHolder = PyCameraSmartPtr<Camera>;
+using PyCameraArg = PyCameraSmartPtr<Camera>;
+#else
+using PyCameraHolder = py::smart_holder;
+using PyCameraArg = std::shared_ptr<Camera>;
+#endif
+
 /*
  * Note: global C++ destructors can be ran on this before the py module is
  * destructed.
@@ -103,7 +111,7 @@  PYBIND11_MODULE(_libcamera, m)
 	 */
 
 	auto pyCameraManager = py::class_<PyCameraManager, std::shared_ptr<PyCameraManager>>(m, "CameraManager");
-	auto pyCamera = py::class_<Camera, PyCameraSmartPtr<Camera>>(m, "Camera");
+	auto pyCamera = py::class_<Camera, PyCameraHolder>(m, "Camera");
 	auto pySensorConfiguration = py::class_<SensorConfiguration>(m, "SensorConfiguration");
 	auto pyCameraConfiguration = py::class_<CameraConfiguration>(m, "CameraConfiguration");
 	auto pyCameraConfigurationStatus = py::enum_<CameraConfiguration::Status>(pyCameraConfiguration, "Status");
@@ -347,7 +355,7 @@  PYBIND11_MODULE(_libcamera, m)
 		.def("range", &StreamFormats::range);
 
 	pyFrameBufferAllocator
-		.def(py::init<PyCameraSmartPtr<Camera>>(), py::keep_alive<1, 2>())
+		.def(py::init<PyCameraArg>(), py::keep_alive<1, 2>())
 		.def("allocate", [](FrameBufferAllocator &self, Stream *stream) {
 			int ret = self.allocate(stream);
 			if (ret < 0)