[libcamera-devel,v8,4/7] Add Python bindings

15815 diff mbox series

Add libcamera Python bindings. pybind11 is used to generate the C++ <->
Python layer.

We use pybind11 'smart_holder' version to avoid issues with private
destructors and shared_ptr. There is also an alternative solution here:

https://github.com/pybind/pybind11/pull/2067

Only a subset of libcamera classes are exposed. Implementing and testing
the wrapper classes is challenging, and as such only classes that I have
needed have been added so far.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 meson.build                                   |   1 +
 meson_options.txt                             |   5 +
 src/meson.build                               |   1 +
 src/py/libcamera/__init__.py                  |  84 +++
 src/py/libcamera/meson.build                  |  51 ++
 src/py/libcamera/pyenums.cpp                  |  34 +
 src/py/libcamera/pymain.cpp                   | 640 ++++++++++++++++++
 src/py/meson.build                            |   1 +
 subprojects/.gitignore                        |   3 +-
 subprojects/packagefiles/pybind11/meson.build |   7 +
 subprojects/pybind11.wrap                     |   9 +
 11 files changed, 835 insertions(+), 1 deletion(-)
 create mode 100644 src/py/libcamera/__init__.py
 create mode 100644 src/py/libcamera/meson.build
 create mode 100644 src/py/libcamera/pyenums.cpp
 create mode 100644 src/py/libcamera/pymain.cpp
 create mode 100644 src/py/meson.build
 create mode 100644 subprojects/packagefiles/pybind11/meson.build
 create mode 100644 subprojects/pybind11.wrap

Hi Tomi,

Thank you for the patch.

On Fri, May 06, 2022 at 05:54:11PM +0300, Tomi Valkeinen wrote:
> Add libcamera Python bindings. pybind11 is used to generate the C++ <->
> Python layer.
> 
> We use pybind11 'smart_holder' version to avoid issues with private
> destructors and shared_ptr. There is also an alternative solution here:
> 
> https://github.com/pybind/pybind11/pull/2067
> 
> Only a subset of libcamera classes are exposed. Implementing and testing
> the wrapper classes is challenging, and as such only classes that I have
> needed have been added so far.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  meson.build                                   |   1 +
>  meson_options.txt                             |   5 +
>  src/meson.build                               |   1 +
>  src/py/libcamera/__init__.py                  |  84 +++
>  src/py/libcamera/meson.build                  |  51 ++
>  src/py/libcamera/pyenums.cpp                  |  34 +
>  src/py/libcamera/pymain.cpp                   | 640 ++++++++++++++++++
>  src/py/meson.build                            |   1 +
>  subprojects/.gitignore                        |   3 +-
>  subprojects/packagefiles/pybind11/meson.build |   7 +
>  subprojects/pybind11.wrap                     |   9 +
>  11 files changed, 835 insertions(+), 1 deletion(-)
>  create mode 100644 src/py/libcamera/__init__.py
>  create mode 100644 src/py/libcamera/meson.build
>  create mode 100644 src/py/libcamera/pyenums.cpp
>  create mode 100644 src/py/libcamera/pymain.cpp
>  create mode 100644 src/py/meson.build
>  create mode 100644 subprojects/packagefiles/pybind11/meson.build
>  create mode 100644 subprojects/pybind11.wrap
> 
> diff --git a/meson.build b/meson.build
> index 0124e7d3..60a911e0 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -177,6 +177,7 @@ summary({
>              'Tracing support': tracing_enabled,
>              'Android support': android_enabled,
>              'GStreamer support': gst_enabled,
> +            'Python bindings': pycamera_enabled,
>              'V4L2 emulation support': v4l2_enabled,
>              'cam application': cam_enabled,
>              'qcam application': qcam_enabled,
> diff --git a/meson_options.txt b/meson_options.txt
> index 2c80ad8b..ca00c78e 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -58,3 +58,8 @@ option('v4l2',
>          type : 'boolean',
>          value : false,
>          description : 'Compile the V4L2 compatibility layer')
> +
> +option('pycamera',
> +        type : 'feature',
> +        value : 'auto',
> +        description : 'Enable libcamera Python bindings (experimental)')
> diff --git a/src/meson.build b/src/meson.build
> index e0ea9c35..34663a6f 100644
> --- a/src/meson.build
> +++ b/src/meson.build
> @@ -37,4 +37,5 @@ subdir('cam')
>  subdir('qcam')
>  
>  subdir('gstreamer')
> +subdir('py')
>  subdir('v4l2')
> diff --git a/src/py/libcamera/__init__.py b/src/py/libcamera/__init__.py
> new file mode 100644
> index 00000000..6b330890
> --- /dev/null
> +++ b/src/py/libcamera/__init__.py
> @@ -0,0 +1,84 @@
> +# SPDX-License-Identifier: LGPL-2.1-or-later
> +# Copyright (C) 2022, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> +
> +from ._libcamera import *
> +
> +
> +class MappedFrameBuffer:
> +    def __init__(self, fb):
> +        self.__fb = fb
> +
> +    def __enter__(self):
> +        from os import lseek, SEEK_END

As it's local to the function it doesn't matter much, but I would have
just imported os and used os.lseek and os.SEEK_END.

> +        import mmap
> +
> +        fb = self.__fb
> +
> +        # Collect information about the buffers
> +
> +        bufinfos = {}
> +
> +        for i in range(fb.num_planes):
> +            fd = fb.fd(i)
> +
> +            if fd not in bufinfos:
> +                buflen = lseek(fd, 0, SEEK_END)
> +                bufinfos[fd] = {'maplen': 0, 'buflen': buflen}
> +            else:
> +                buflen = bufinfos[fd]['buflen']
> +
> +            if fb.offset(i) > buflen or fb.offset(i) + fb.length(i) > buflen:
> +                raise RuntimeError(f'plane is out of buffer: buffer length={buflen}, ' +
> +                                   f'plane offset={fb.offset(i)}, plane length={fb.length(i)}')
> +
> +            bufinfos[fd]['maplen'] = max(bufinfos[fd]['maplen'], fb.offset(i) + fb.length(i))
> +
> +        # mmap the buffers
> +
> +        maps = []
> +
> +        for fd, info in bufinfos.items():
> +            map = mmap.mmap(fd, info['maplen'], mmap.MAP_SHARED, mmap.PROT_READ | mmap.PROT_WRITE)
> +            info['map'] = map
> +            maps.append(map)
> +
> +        self.__maps = tuple(maps)
> +
> +        # Create memoryviews for the planes
> +
> +        planes = []
> +
> +        for i in range(fb.num_planes):
> +            fd = fb.fd(i)
> +            info = bufinfos[fd]
> +
> +            mv = memoryview(info['map'])
> +
> +            start = fb.offset(i)
> +            end = fb.offset(i) + fb.length(i)
> +
> +            mv = mv[start:end]
> +
> +            planes.append(mv)
> +
> +        self.__planes = tuple(planes)
> +
> +        return self
> +
> +    def __exit__(self, exc_type, exc_value, exc_traceback):
> +        for p in self.__planes:
> +            p.release()
> +
> +        for mm in self.__maps:
> +            mm.close()
> +
> +    @property
> +    def planes(self):
> +        return self.__planes
> +
> +
> +def __FrameBuffer__mmap(self):
> +    return MappedFrameBuffer(self)
> +
> +
> +FrameBuffer.mmap = __FrameBuffer__mmap
> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build
> new file mode 100644
> index 00000000..e4abc34a
> --- /dev/null
> +++ b/src/py/libcamera/meson.build
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: CC0-1.0
> +
> +py3_dep = dependency('python3', required : get_option('pycamera'))
> +
> +if not py3_dep.found()
> +    pycamera_enabled = false
> +    subdir_done()
> +endif
> +
> +pycamera_enabled = true
> +
> +pybind11_proj = subproject('pybind11')
> +pybind11_dep = pybind11_proj.get_variable('pybind11_dep')
> +
> +pycamera_sources = files([
> +    'pyenums.cpp',
> +    'pymain.cpp',
> +])
> +
> +pycamera_deps = [
> +    libcamera_public,
> +    py3_dep,
> +    pybind11_dep,
> +]
> +
> +pycamera_args = [
> +    '-fvisibility=hidden',
> +    '-Wno-shadow',
> +    '-DPYBIND11_USE_SMART_HOLDER_AS_DEFAULT',
> +]
> +
> +destdir = get_option('libdir') / ('python' + py3_dep.version()) / 'site-packages' / 'libcamera'
> +
> +pycamera = shared_module('_libcamera',
> +                         pycamera_sources,
> +                         install : true,
> +                         install_dir : destdir,
> +                         name_prefix : '',
> +                         dependencies : pycamera_deps,
> +                         cpp_args : pycamera_args)
> +
> +run_command('ln', '-fsT', '../../../../src/py/libcamera/__init__.py',
> +            meson.current_build_dir() / '__init__.py',
> +            check: true)
> +
> +install_data(['__init__.py'], install_dir : destdir)
> +
> +# \todo: Generate stubs when building. Depends on pybind11-stubgen. Sometimes

s/todo:/todo/

I'm still not sure what this is for :-) Do we need to generate stubs
later ? What are they for ?

> +# this works, sometimes doesn't... To generate pylibcamera stubs.
> +# $ PYTHONPATH=build/src/py pybind11-stubgen --no-setup-py -o build/src/py libcamera
> +# $ mv build/src/py/libcamera-stubs/* build/src/py/libcamera/
> diff --git a/src/py/libcamera/pyenums.cpp b/src/py/libcamera/pyenums.cpp
> new file mode 100644
> index 00000000..b655e622
> --- /dev/null
> +++ b/src/py/libcamera/pyenums.cpp
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2022, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> + *
> + * Python bindings - Enumerations
> + */
> +
> +#include <libcamera/libcamera.h>
> +
> +#include <pybind11/smart_holder.h>
> +
> +namespace py = pybind11;
> +
> +using namespace libcamera;
> +
> +void init_pyenums(py::module &m)
> +{
> +	py::enum_<StreamRole>(m, "StreamRole")
> +		.value("StillCapture", StreamRole::StillCapture)
> +		.value("Raw", StreamRole::Raw)
> +		.value("VideoRecording", StreamRole::VideoRecording)
> +		.value("Viewfinder", StreamRole::Viewfinder);
> +
> +	py::enum_<ControlType>(m, "ControlType")
> +		.value("None", ControlType::ControlTypeNone)
> +		.value("Bool", ControlType::ControlTypeBool)
> +		.value("Byte", ControlType::ControlTypeByte)
> +		.value("Integer32", ControlType::ControlTypeInteger32)
> +		.value("Integer64", ControlType::ControlTypeInteger64)
> +		.value("Float", ControlType::ControlTypeFloat)
> +		.value("String", ControlType::ControlTypeString)
> +		.value("Rectangle", ControlType::ControlTypeRectangle)
> +		.value("Size", ControlType::ControlTypeSize);
> +}
> diff --git a/src/py/libcamera/pymain.cpp b/src/py/libcamera/pymain.cpp
> new file mode 100644
> index 00000000..8c3be8f4
> --- /dev/null
> +++ b/src/py/libcamera/pymain.cpp
> @@ -0,0 +1,640 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2022, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> + *
> + * Python bindings
> + */
> +
> +/*
> + * \todo Add geometry classes (Point, Rectangle...)
> + * \todo Add bindings for the ControlInfo class
> + */
> +
> +#include <chrono>
> +#include <fcntl.h>
> +#include <mutex>
> +#include <sys/eventfd.h>
> +#include <sys/mman.h>
> +#include <thread>
> +#include <unistd.h>
> +
> +#include <libcamera/libcamera.h>
> +
> +#include <pybind11/functional.h>
> +#include <pybind11/smart_holder.h>
> +#include <pybind11/stl.h>
> +#include <pybind11/stl_bind.h>
> +
> +namespace py = pybind11;
> +
> +using namespace libcamera;
> +
> +template<typename T>
> +static py::object valueOrTuple(const ControlValue &cv)
> +{
> +	if (cv.isArray()) {
> +		const T *v = reinterpret_cast<const T *>(cv.data().data());
> +		auto t = py::tuple(cv.numElements());
> +
> +		for (size_t i = 0; i < cv.numElements(); ++i)
> +			t[i] = v[i];
> +
> +		return t;
> +	}
> +
> +	return py::cast(cv.get<T>());
> +}
> +
> +static py::object controlValueToPy(const ControlValue &cv)
> +{
> +	switch (cv.type()) {
> +	case ControlTypeBool:
> +		return valueOrTuple<bool>(cv);
> +	case ControlTypeByte:
> +		return valueOrTuple<uint8_t>(cv);
> +	case ControlTypeInteger32:
> +		return valueOrTuple<int32_t>(cv);
> +	case ControlTypeInteger64:
> +		return valueOrTuple<int64_t>(cv);
> +	case ControlTypeFloat:
> +		return valueOrTuple<float>(cv);
> +	case ControlTypeString:
> +		return py::cast(cv.get<std::string>());
> +	case ControlTypeRectangle: {
> +		const Rectangle *v = reinterpret_cast<const Rectangle *>(cv.data().data());
> +		return py::make_tuple(v->x, v->y, v->width, v->height);
> +	}
> +	case ControlTypeSize: {
> +		const Size *v = reinterpret_cast<const Size *>(cv.data().data());
> +		return py::make_tuple(v->width, v->height);
> +	}
> +	case ControlTypeNone:
> +	default:
> +		throw std::runtime_error("Unsupported ControlValue type");
> +	}
> +}
> +
> +template<typename T>
> +static ControlValue controlValueMaybeArray(const py::object &ob)
> +{
> +	if (py::isinstance<py::list>(ob) || py::isinstance<py::tuple>(ob)) {
> +		std::vector<T> vec = ob.cast<std::vector<T>>();
> +		return ControlValue(Span<const T>(vec));
> +	}
> +
> +	return ControlValue(ob.cast<T>());
> +}
> +
> +static ControlValue pyToControlValue(const py::object &ob, ControlType type)
> +{
> +	switch (type) {
> +	case ControlTypeBool:
> +		return ControlValue(ob.cast<bool>());
> +	case ControlTypeByte:
> +		return controlValueMaybeArray<uint8_t>(ob);
> +	case ControlTypeInteger32:
> +		return controlValueMaybeArray<int32_t>(ob);
> +	case ControlTypeInteger64:
> +		return controlValueMaybeArray<int64_t>(ob);
> +	case ControlTypeFloat:
> +		return controlValueMaybeArray<float>(ob);
> +	case ControlTypeString:
> +		return ControlValue(ob.cast<std::string>());
> +	case ControlTypeRectangle: {
> +		auto array = ob.cast<std::array<int32_t, 4>>();
> +		return ControlValue(Rectangle(array[0], array[1], array[2], array[3]));
> +	}
> +	case ControlTypeSize: {
> +		auto array = ob.cast<std::array<int32_t, 2>>();
> +		return ControlValue(Size(array[0], array[1]));
> +	}
> +	case ControlTypeNone:
> +	default:
> +		throw std::runtime_error("Control type not implemented");
> +	}
> +}
> +
> +static std::weak_ptr<CameraManager> gCameraManager;
> +static int gEventfd;
> +static std::mutex gReqlistMutex;
> +static std::vector<Request *> gReqList;
> +
> +static void handleRequestCompleted(Request *req)
> +{
> +	{
> +		std::lock_guard guard(gReqlistMutex);
> +		gReqList.push_back(req);
> +	}
> +
> +	uint64_t v = 1;
> +	write(gEventfd, &v, 8);
> +}
> +
> +void init_pyenums(py::module &m);
> +
> +PYBIND11_MODULE(_libcamera, m)
> +{
> +	init_pyenums(m);
> +
> +	/* Forward declarations */
> +
> +	/*
> +	 * We need to declare all the classes here so that Python docstrings
> +	 * can be generated correctly.
> +	 * https://pybind11.readthedocs.io/en/latest/advanced/misc.html#avoiding-c-types-in-docstrings
> +	 */
> +
> +	auto pyCameraManager = py::class_<CameraManager>(m, "CameraManager");
> +	auto pyCamera = py::class_<Camera>(m, "Camera");
> +	auto pyCameraConfiguration = py::class_<CameraConfiguration>(m, "CameraConfiguration");
> +	auto pyCameraConfigurationStatus = py::enum_<CameraConfiguration::Status>(pyCameraConfiguration, "Status");
> +	auto pyStreamConfiguration = py::class_<StreamConfiguration>(m, "StreamConfiguration");
> +	auto pyStreamFormats = py::class_<StreamFormats>(m, "StreamFormats");
> +	auto pyFrameBufferAllocator = py::class_<FrameBufferAllocator>(m, "FrameBufferAllocator");
> +	auto pyFrameBuffer = py::class_<FrameBuffer>(m, "FrameBuffer");
> +	auto pyStream = py::class_<Stream>(m, "Stream");
> +	auto pyControlId = py::class_<ControlId>(m, "ControlId");
> +	auto pyRequest = py::class_<Request>(m, "Request");
> +	auto pyRequestStatus = py::enum_<Request::Status>(pyRequest, "Status");
> +	auto pyRequestReuse = py::enum_<Request::ReuseFlag>(pyRequest, "Reuse");
> +	auto pyFrameMetadata = py::class_<FrameMetadata>(m, "FrameMetadata");
> +	auto pyFrameMetadataStatus = py::enum_<FrameMetadata::Status>(pyFrameMetadata, "Status");
> +	auto pyTransform = py::class_<Transform>(m, "Transform");
> +	auto pyColorSpace = py::class_<ColorSpace>(m, "ColorSpace");
> +	auto pyColorSpacePrimaries = py::enum_<ColorSpace::Primaries>(pyColorSpace, "Primaries");
> +	auto pyColorSpaceTransferFunction = py::enum_<ColorSpace::TransferFunction>(pyColorSpace, "TransferFunction");
> +	auto pyColorSpaceYcbcrEncoding = py::enum_<ColorSpace::YcbcrEncoding>(pyColorSpace, "YcbcrEncoding");
> +	auto pyColorSpaceRange = py::enum_<ColorSpace::Range>(pyColorSpace, "Range");
> +
> +	/* Global functions */
> +	m.def("log_set_level", &logSetLevel);
> +
> +	/* Classes */
> +	pyCameraManager
> +		.def_static("singleton", []() {
> +			std::shared_ptr<CameraManager> cm = gCameraManager.lock();
> +			if (cm)
> +				return cm;
> +
> +			int fd = eventfd(0, 0);
> +			if (fd == -1)
> +				throw std::system_error(errno, std::generic_category(),
> +				                   "Failed to create eventfd");

There should be tabs instead of spaces, and the '"' should be aligned
under errno. Same below.

> +
> +			cm = std::shared_ptr<CameraManager>(new CameraManager, [](auto p) {
> +				close(gEventfd);
> +				gEventfd = -1;
> +				delete p;
> +			});
> +
> +			gEventfd = fd;
> +			gCameraManager = cm;
> +
> +			int ret = cm->start();
> +			if (ret)
> +				throw std::system_error(-ret, std::generic_category(),
> +				                   "Failed to start CameraManager");
> +
> +			return cm;
> +		})
> +
> +		.def_property_readonly("version", &CameraManager::version)
> +
> +		.def_property_readonly("efd", [](CameraManager &) {
> +			return gEventfd;
> +		})
> +
> +		.def("get_ready_requests", [](CameraManager &) {
> +			std::vector<Request *> v;
> +
> +			{
> +				std::lock_guard guard(gReqlistMutex);
> +				swap(v, gReqList);
> +			}
> +
> +			std::vector<py::object> ret;
> +
> +			for (Request *req : v) {
> +				py::object o = py::cast(req);
> +				/* Decrease the ref increased in Camera.queue_request() */
> +				o.dec_ref();
> +				ret.push_back(o);
> +			}
> +
> +			return ret;
> +		})
> +
> +		.def("get", py::overload_cast<const std::string &>(&CameraManager::get), py::keep_alive<0, 1>())
> +
> +		/* Create a list of Cameras, where each camera has a keep-alive to CameraManager */
> +		.def_property_readonly("cameras", [](CameraManager &self) {
> +			py::list l;
> +
> +			for (auto &c : self.cameras()) {
> +				py::object py_cm = py::cast(self);
> +				py::object py_cam = py::cast(c);
> +				py::detail::keep_alive_impl(py_cam, py_cm);
> +				l.append(py_cam);
> +			}
> +
> +			return l;
> +		});
> +
> +	pyCamera
> +		.def_property_readonly("id", &Camera::id)
> +		.def("acquire", &Camera::acquire)
> +		.def("release", &Camera::release)
> +		.def("start", [](Camera &self, py::dict controls) {
> +			/* \todo What happens if someone calls start() multiple times? */
> +
> +			self.requestCompleted.connect(handleRequestCompleted);
> +
> +			const ControlInfoMap &controlMap = self.controls();
> +			ControlList controlList(controlMap);
> +			for (const auto& [hkey, hval]: controls) {
> +				auto key = hkey.cast<std::string>();
> +
> +				auto it = find_if(controlMap.begin(), controlMap.end(),
> +						  [&key](const auto &kvp) {
> +							  return kvp.first->name() == key; });

				auto it = std::find_if(controlMap.begin(), controlMap.end(),
						       [&key](const auto &kvp) {
							       return kvp.first->name() == key;
						       });

> +
> +				if (it == controlMap.end())
> +					throw std::runtime_error("Control " + key + " not found");
> +
> +				const auto &id = it->first;
> +				auto obj = py::cast<py::object>(hval);
> +
> +				controlList.set(id->id(), pyToControlValue(obj, id->type()));
> +			}
> +
> +			int ret = self.start(&controlList);
> +			if (ret) {
> +				self.requestCompleted.disconnect(handleRequestCompleted);
> +				return ret;
> +			}
> +
> +			return 0;
> +		}, py::arg("controls") = py::dict())
> +
> +		.def("stop", [](Camera &self) {
> +			int ret = self.stop();
> +			if (ret)
> +				return ret;
> +
> +			self.requestCompleted.disconnect(handleRequestCompleted);
> +
> +			return 0;
> +		})
> +
> +		.def("__repr__", [](Camera &self) {
> +			return "<libcamera.Camera '" + self.id() + "'>";
> +		})
> +
> +		/* Keep the camera alive, as StreamConfiguration contains a Stream* */
> +		.def("generate_configuration", &Camera::generateConfiguration, py::keep_alive<0, 1>())
> +		.def("configure", &Camera::configure)
> +
> +		.def("create_request", &Camera::createRequest, py::arg("cookie") = 0)
> +
> +		.def("queue_request", [](Camera &self, Request *req) {
> +			py::object py_req = py::cast(req);
> +
> +			/*
> +			 * Increase the reference count, will be dropped in
> +			 * CameraManager.get_ready_requests().
> +			 */
> +
> +			py_req.inc_ref();
> +
> +			int ret = self.queueRequest(req);
> +			if (ret)
> +				py_req.dec_ref();
> +
> +			return ret;
> +		})
> +
> +		.def_property_readonly("streams", [](Camera &self) {
> +			py::set set;
> +			for (auto &s : self.streams()) {
> +				py::object py_self = py::cast(self);
> +				py::object py_s = py::cast(s);
> +				py::detail::keep_alive_impl(py_s, py_self);
> +				set.add(py_s);
> +			}
> +			return set;
> +		})
> +
> +		.def("find_control", [](Camera &self, const std::string &name) {
> +			const auto &controls = self.controls();
> +
> +			auto it = find_if(controls.begin(), controls.end(),
> +					  [&name](const auto &kvp) { return kvp.first->name() == name; });

Missing std:: here too (and I would also wrap the line).

> +
> +			if (it == controls.end())
> +				throw std::runtime_error("Control not found");

				throw std::runtime_error("Control '" + name + "' not found");

could be nicer to debug issues.

> +
> +			return it->first;
> +		}, py::return_value_policy::reference_internal)
> +
> +		.def_property_readonly("controls", [](Camera &self) {
> +			py::dict ret;
> +
> +			for (const auto &[id, ci] : self.controls()) {
> +				ret[id->name().c_str()] = std::make_tuple<py::object>(controlValueToPy(ci.min()),
> +										      controlValueToPy(ci.max()),
> +										      controlValueToPy(ci.def()));
> +			}
> +
> +			return ret;
> +		})
> +
> +		.def_property_readonly("properties", [](Camera &self) {
> +			py::dict ret;
> +
> +			for (const auto &[key, cv] : self.properties()) {
> +				const ControlId *id = properties::properties.at(key);
> +				py::object ob = controlValueToPy(cv);
> +
> +				ret[id->name().c_str()] = ob;
> +			}
> +
> +			return ret;
> +		});
> +
> +	pyCameraConfiguration
> +		.def("__iter__", [](CameraConfiguration &self) {
> +			return py::make_iterator<py::return_value_policy::reference_internal>(self);
> +		}, py::keep_alive<0, 1>())
> +		.def("__len__", [](CameraConfiguration &self) {
> +			return self.size();
> +		})
> +		.def("validate", &CameraConfiguration::validate)
> +		.def("at", py::overload_cast<unsigned int>(&CameraConfiguration::at),
> +		     py::return_value_policy::reference_internal)
> +		.def_property_readonly("size", &CameraConfiguration::size)
> +		.def_property_readonly("empty", &CameraConfiguration::empty)
> +		.def_readwrite("transform", &CameraConfiguration::transform);
> +
> +	pyCameraConfigurationStatus
> +		.value("Valid", CameraConfiguration::Valid)
> +		.value("Adjusted", CameraConfiguration::Adjusted)
> +		.value("Invalid", CameraConfiguration::Invalid);
> +
> +	pyStreamConfiguration
> +		.def("to_string", &StreamConfiguration::toString)

Should this be __str__ ?

> +		.def_property_readonly("stream", &StreamConfiguration::stream,
> +		                       py::return_value_policy::reference_internal)
> +		.def_property(
> +			"size",
> +			[](StreamConfiguration &self) {
> +				return std::make_tuple(self.size.width, self.size.height);
> +			},
> +			[](StreamConfiguration &self, std::tuple<uint32_t, uint32_t> size) {
> +				self.size.width = std::get<0>(size);
> +				self.size.height = std::get<1>(size);
> +			})
> +		.def_property(
> +			"pixel_format",
> +			[](StreamConfiguration &self) {
> +				return self.pixelFormat.toString();
> +			},
> +			[](StreamConfiguration &self, std::string fmt) {
> +				self.pixelFormat = PixelFormat::fromString(fmt);
> +			})
> +		.def_readwrite("stride", &StreamConfiguration::stride)
> +		.def_readwrite("frame_size", &StreamConfiguration::frameSize)
> +		.def_readwrite("buffer_count", &StreamConfiguration::bufferCount)
> +		.def_property_readonly("formats", &StreamConfiguration::formats,
> +		                       py::return_value_policy::reference_internal)
> +		.def_readwrite("colorSpace", &StreamConfiguration::colorSpace);

color_space

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

> +
> +	pyStreamFormats
> +		.def_property_readonly("pixel_formats", [](StreamFormats &self) {
> +			std::vector<std::string> fmts;
> +			for (auto &fmt : self.pixelformats())
> +				fmts.push_back(fmt.toString());
> +			return fmts;
> +		})
> +		.def("sizes", [](StreamFormats &self, const std::string &pixelFormat) {
> +			auto fmt = PixelFormat::fromString(pixelFormat);
> +			std::vector<std::tuple<uint32_t, uint32_t>> fmts;
> +			for (const auto &s : self.sizes(fmt))
> +				fmts.push_back(std::make_tuple(s.width, s.height));
> +			return fmts;
> +		})
> +		.def("range", [](StreamFormats &self, const std::string &pixelFormat) {
> +			auto fmt = PixelFormat::fromString(pixelFormat);
> +			const auto &range = self.range(fmt);
> +			return make_tuple(std::make_tuple(range.hStep, range.vStep),
> +					  std::make_tuple(range.min.width, range.min.height),
> +					  std::make_tuple(range.max.width, range.max.height));
> +		});
> +
> +	pyFrameBufferAllocator
> +		.def(py::init<std::shared_ptr<Camera>>(), py::keep_alive<1, 2>())
> +		.def("allocate", &FrameBufferAllocator::allocate)
> +		.def_property_readonly("allocated", &FrameBufferAllocator::allocated)
> +		/* Create a list of FrameBuffers, where each FrameBuffer has a keep-alive to FrameBufferAllocator */
> +		.def("buffers", [](FrameBufferAllocator &self, Stream *stream) {
> +			py::object py_self = py::cast(self);
> +			py::list l;
> +			for (auto &ub : self.buffers(stream)) {
> +				py::object py_buf = py::cast(ub.get(), py::return_value_policy::reference_internal, py_self);
> +				l.append(py_buf);
> +			}
> +			return l;
> +		});
> +
> +	pyFrameBuffer
> +		/* \todo implement FrameBuffer::Plane properly */
> +		.def(py::init([](std::vector<std::tuple<int, unsigned int>> planes, unsigned int cookie) {
> +			std::vector<FrameBuffer::Plane> v;
> +			for (const auto &t : planes)
> +				v.push_back({ SharedFD(std::get<0>(t)), FrameBuffer::Plane::kInvalidOffset, std::get<1>(t) });
> +			return new FrameBuffer(v, cookie);
> +		}))
> +		.def_property_readonly("metadata", &FrameBuffer::metadata, py::return_value_policy::reference_internal)
> +		.def_property_readonly("num_planes", [](const FrameBuffer &self) {
> +			return self.planes().size();
> +		})
> +		.def("length", [](FrameBuffer &self, uint32_t idx) {
> +			const FrameBuffer::Plane &plane = self.planes()[idx];
> +			return plane.length;
> +		})
> +		.def("fd", [](FrameBuffer &self, uint32_t idx) {
> +			const FrameBuffer::Plane &plane = self.planes()[idx];
> +			return plane.fd.get();
> +		})
> +		.def("offset", [](FrameBuffer &self, uint32_t idx) {
> +			const FrameBuffer::Plane &plane = self.planes()[idx];
> +			return plane.offset;
> +		})
> +		.def_property("cookie", &FrameBuffer::cookie, &FrameBuffer::setCookie);
> +
> +	pyStream
> +		.def_property_readonly("configuration", &Stream::configuration);
> +
> +	pyControlId
> +		.def_property_readonly("id", &ControlId::id)
> +		.def_property_readonly("name", &ControlId::name)
> +		.def_property_readonly("type", &ControlId::type);
> +
> +	pyRequest
> +		/* \todo Fence is not supported, so we cannot expose addBuffer() directly */
> +		.def("add_buffer", [](Request &self, const Stream *stream, FrameBuffer *buffer) {
> +			return self.addBuffer(stream, buffer);
> +		}, py::keep_alive<1, 3>()) /* Request keeps Framebuffer alive */
> +		.def_property_readonly("status", &Request::status)
> +		.def_property_readonly("buffers", &Request::buffers)
> +		.def_property_readonly("cookie", &Request::cookie)
> +		.def_property_readonly("has_pending_buffers", &Request::hasPendingBuffers)
> +		.def("set_control", [](Request &self, ControlId &id, py::object value) {
> +			self.controls().set(id.id(), pyToControlValue(value, id.type()));
> +		})
> +		.def_property_readonly("metadata", [](Request &self) {
> +			py::dict ret;
> +
> +			for (const auto &[key, cv] : self.metadata()) {
> +				const ControlId *id = controls::controls.at(key);
> +				py::object ob = controlValueToPy(cv);
> +
> +				ret[id->name().c_str()] = ob;
> +			}
> +
> +			return ret;
> +		})
> +		/*
> +		 * \todo As we add a keep_alive to the fb in addBuffers(), we
> +		 * can only allow reuse with ReuseBuffers.
> +		 */
> +		.def("reuse", [](Request &self) { self.reuse(Request::ReuseFlag::ReuseBuffers); });
> +
> +	pyRequestStatus
> +		.value("Pending", Request::RequestPending)
> +		.value("Complete", Request::RequestComplete)
> +		.value("Cancelled", Request::RequestCancelled);
> +
> +	pyRequestReuse
> +		.value("Default", Request::ReuseFlag::Default)
> +		.value("ReuseBuffers", Request::ReuseFlag::ReuseBuffers);
> +
> +	pyFrameMetadata
> +		.def_readonly("status", &FrameMetadata::status)
> +		.def_readonly("sequence", &FrameMetadata::sequence)
> +		.def_readonly("timestamp", &FrameMetadata::timestamp)
> +		/* \todo Implement FrameMetadata::Plane properly */
> +		.def_property_readonly("bytesused", [](FrameMetadata &self) {
> +			std::vector<unsigned int> v;
> +			v.resize(self.planes().size());
> +			transform(self.planes().begin(), self.planes().end(), v.begin(), [](const auto &p) { return p.bytesused; });
> +			return v;
> +		});
> +
> +	pyFrameMetadataStatus
> +		.value("Success", FrameMetadata::FrameSuccess)
> +		.value("Error", FrameMetadata::FrameError)
> +		.value("Cancelled", FrameMetadata::FrameCancelled);
> +
> +	pyTransform
> +		.def(py::init([](int rotation, bool hflip, bool vflip, bool transpose) {
> +			bool ok;
> +
> +			Transform t = transformFromRotation(rotation, &ok);
> +			if (!ok)
> +				throw std::invalid_argument("Invalid rotation");
> +
> +			if (hflip)
> +				t ^= Transform::HFlip;
> +			if (vflip)
> +				t ^= Transform::VFlip;
> +			if (transpose)
> +				t ^= Transform::Transpose;
> +			return t;
> +		}), py::arg("rotation") = 0, py::arg("hflip") = false,
> +		    py::arg("vflip") = false, py::arg("transpose") = false)
> +		.def(py::init([](Transform &other) { return other; }))
> +		.def("__repr__", [](Transform &self) {
> +			return "<libcamera.Transform '" + std::string(transformToString(self)) + "'>";
> +		})
> +		.def_property("hflip",
> +			      [](Transform &self) {
> +				      return !!(self & Transform::HFlip);
> +			      },
> +			      [](Transform &self, bool hflip) {
> +				      if (hflip)
> +					      self |= Transform::HFlip;
> +				      else
> +					      self &= ~Transform::HFlip;
> +			      })
> +		.def_property("vflip",
> +			      [](Transform &self) {
> +				      return !!(self & Transform::VFlip);
> +			      },
> +			      [](Transform &self, bool vflip) {
> +				      if (vflip)
> +					      self |= Transform::VFlip;
> +				      else
> +					      self &= ~Transform::VFlip;
> +			      })
> +		.def_property("transpose",
> +			      [](Transform &self) {
> +				      return !!(self & Transform::Transpose);
> +			      },
> +			      [](Transform &self, bool transpose) {
> +				      if (transpose)
> +					      self |= Transform::Transpose;
> +				      else
> +					      self &= ~Transform::Transpose;
> +			      })
> +		.def("inverse", [](Transform &self) { return -self; })
> +		.def("invert", [](Transform &self) {
> +			self = -self;
> +		})
> +		.def("compose", [](Transform &self, Transform &other) {
> +			self = self * other;
> +		});
> +
> +	pyColorSpace
> +		.def(py::init([](ColorSpace::Primaries primaries,
> +				 ColorSpace::TransferFunction transferFunction,
> +				 ColorSpace::YcbcrEncoding ycbcrEncoding,
> +				 ColorSpace::Range range) {
> +			return ColorSpace(primaries, transferFunction, ycbcrEncoding, range);
> +		}), py::arg("primaries"), py::arg("transferFunction"),
> +		    py::arg("ycbcrEncoding"), py::arg("range"))
> +		.def(py::init([](ColorSpace &other) { return other; }))
> +		.def("__repr__", [](ColorSpace &self) {
> +			return "<libcamera.ColorSpace '" + self.toString() + "'>";
> +		})
> +		.def_readwrite("primaries", &ColorSpace::primaries)
> +		.def_readwrite("transferFunction", &ColorSpace::transferFunction)
> +		.def_readwrite("ycbcrEncoding", &ColorSpace::ycbcrEncoding)
> +		.def_readwrite("range", &ColorSpace::range)
> +		.def_static("Raw", []() { return ColorSpace::Raw; })
> +		.def_static("Jpeg", []() { return ColorSpace::Jpeg; })
> +		.def_static("Srgb", []() { return ColorSpace::Srgb; })
> +		.def_static("Smpte170m", []() { return ColorSpace::Smpte170m; })
> +		.def_static("Rec709", []() { return ColorSpace::Rec709; })
> +		.def_static("Rec2020", []() { return ColorSpace::Rec2020; });
> +
> +	pyColorSpacePrimaries
> +		.value("Raw", ColorSpace::Primaries::Raw)
> +		.value("Smpte170m", ColorSpace::Primaries::Smpte170m)
> +		.value("Rec709", ColorSpace::Primaries::Rec709)
> +		.value("Rec2020", ColorSpace::Primaries::Rec2020);
> +
> +	pyColorSpaceTransferFunction
> +		.value("Linear", ColorSpace::TransferFunction::Linear)
> +		.value("Srgb", ColorSpace::TransferFunction::Srgb)
> +		.value("Rec709", ColorSpace::TransferFunction::Rec709);
> +
> +	pyColorSpaceYcbcrEncoding
> +		.value("Null", ColorSpace::YcbcrEncoding::None)
> +		.value("Rec601", ColorSpace::YcbcrEncoding::Rec601)
> +		.value("Rec709", ColorSpace::YcbcrEncoding::Rec709)
> +		.value("Rec2020", ColorSpace::YcbcrEncoding::Rec2020);
> +
> +	pyColorSpaceRange
> +		.value("Full", ColorSpace::Range::Full)
> +		.value("Limited", ColorSpace::Range::Limited);
> +}
> diff --git a/src/py/meson.build b/src/py/meson.build
> new file mode 100644
> index 00000000..4ce9668c
> --- /dev/null
> +++ b/src/py/meson.build
> @@ -0,0 +1 @@
> +subdir('libcamera')
> diff --git a/subprojects/.gitignore b/subprojects/.gitignore
> index 391fde2c..0e194289 100644
> --- a/subprojects/.gitignore
> +++ b/subprojects/.gitignore
> @@ -1,3 +1,4 @@
>  /googletest-release*
>  /libyuv
> -/packagecache
> \ No newline at end of file
> +/packagecache
> +/pybind11
> diff --git a/subprojects/packagefiles/pybind11/meson.build b/subprojects/packagefiles/pybind11/meson.build
> new file mode 100644
> index 00000000..1be47ca4
> --- /dev/null
> +++ b/subprojects/packagefiles/pybind11/meson.build
> @@ -0,0 +1,7 @@
> +project('pybind11', 'cpp',
> +    version : '2.9.1',
> +    license : 'BSD-3-Clause')
> +
> +pybind11_incdir = include_directories('include')
> +
> +pybind11_dep = declare_dependency(include_directories : pybind11_incdir)
> diff --git a/subprojects/pybind11.wrap b/subprojects/pybind11.wrap
> new file mode 100644
> index 00000000..43c0608d
> --- /dev/null
> +++ b/subprojects/pybind11.wrap
> @@ -0,0 +1,9 @@
> +[wrap-git]
> +url = https://github.com/pybind/pybind11.git
> +# This is the head of 'smart_holder' branch
> +revision = 82734801f23314b4c34d70a79509e060a2648e04
> +depth = 1
> +patch_directory = pybind11
> +
> +[provide]
> +pybind11 = pybind11_dep

On 06/05/2022 20:21, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Fri, May 06, 2022 at 05:54:11PM +0300, Tomi Valkeinen wrote:
>> Add libcamera Python bindings. pybind11 is used to generate the C++ <->
>> Python layer.
>>
>> We use pybind11 'smart_holder' version to avoid issues with private
>> destructors and shared_ptr. There is also an alternative solution here:
>>
>> https://github.com/pybind/pybind11/pull/2067
>>
>> Only a subset of libcamera classes are exposed. Implementing and testing
>> the wrapper classes is challenging, and as such only classes that I have
>> needed have been added so far.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>>   meson.build                                   |   1 +
>>   meson_options.txt                             |   5 +
>>   src/meson.build                               |   1 +
>>   src/py/libcamera/__init__.py                  |  84 +++
>>   src/py/libcamera/meson.build                  |  51 ++
>>   src/py/libcamera/pyenums.cpp                  |  34 +
>>   src/py/libcamera/pymain.cpp                   | 640 ++++++++++++++++++
>>   src/py/meson.build                            |   1 +
>>   subprojects/.gitignore                        |   3 +-
>>   subprojects/packagefiles/pybind11/meson.build |   7 +
>>   subprojects/pybind11.wrap                     |   9 +
>>   11 files changed, 835 insertions(+), 1 deletion(-)
>>   create mode 100644 src/py/libcamera/__init__.py
>>   create mode 100644 src/py/libcamera/meson.build
>>   create mode 100644 src/py/libcamera/pyenums.cpp
>>   create mode 100644 src/py/libcamera/pymain.cpp
>>   create mode 100644 src/py/meson.build
>>   create mode 100644 subprojects/packagefiles/pybind11/meson.build
>>   create mode 100644 subprojects/pybind11.wrap
>>
>> diff --git a/meson.build b/meson.build
>> index 0124e7d3..60a911e0 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -177,6 +177,7 @@ summary({
>>               'Tracing support': tracing_enabled,
>>               'Android support': android_enabled,
>>               'GStreamer support': gst_enabled,
>> +            'Python bindings': pycamera_enabled,
>>               'V4L2 emulation support': v4l2_enabled,
>>               'cam application': cam_enabled,
>>               'qcam application': qcam_enabled,
>> diff --git a/meson_options.txt b/meson_options.txt
>> index 2c80ad8b..ca00c78e 100644
>> --- a/meson_options.txt
>> +++ b/meson_options.txt
>> @@ -58,3 +58,8 @@ option('v4l2',
>>           type : 'boolean',
>>           value : false,
>>           description : 'Compile the V4L2 compatibility layer')
>> +
>> +option('pycamera',
>> +        type : 'feature',
>> +        value : 'auto',
>> +        description : 'Enable libcamera Python bindings (experimental)')
>> diff --git a/src/meson.build b/src/meson.build
>> index e0ea9c35..34663a6f 100644
>> --- a/src/meson.build
>> +++ b/src/meson.build
>> @@ -37,4 +37,5 @@ subdir('cam')
>>   subdir('qcam')
>>   
>>   subdir('gstreamer')
>> +subdir('py')
>>   subdir('v4l2')
>> diff --git a/src/py/libcamera/__init__.py b/src/py/libcamera/__init__.py
>> new file mode 100644
>> index 00000000..6b330890
>> --- /dev/null
>> +++ b/src/py/libcamera/__init__.py
>> @@ -0,0 +1,84 @@
>> +# SPDX-License-Identifier: LGPL-2.1-or-later
>> +# Copyright (C) 2022, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> +
>> +from ._libcamera import *
>> +
>> +
>> +class MappedFrameBuffer:
>> +    def __init__(self, fb):
>> +        self.__fb = fb
>> +
>> +    def __enter__(self):
>> +        from os import lseek, SEEK_END
> 
> As it's local to the function it doesn't matter much, but I would have
> just imported os and used os.lseek and os.SEEK_END.

Yep, makes sense.

>> +        import mmap
>> +
>> +        fb = self.__fb
>> +
>> +        # Collect information about the buffers
>> +
>> +        bufinfos = {}
>> +
>> +        for i in range(fb.num_planes):
>> +            fd = fb.fd(i)
>> +
>> +            if fd not in bufinfos:
>> +                buflen = lseek(fd, 0, SEEK_END)
>> +                bufinfos[fd] = {'maplen': 0, 'buflen': buflen}
>> +            else:
>> +                buflen = bufinfos[fd]['buflen']
>> +
>> +            if fb.offset(i) > buflen or fb.offset(i) + fb.length(i) > buflen:
>> +                raise RuntimeError(f'plane is out of buffer: buffer length={buflen}, ' +
>> +                                   f'plane offset={fb.offset(i)}, plane length={fb.length(i)}')
>> +
>> +            bufinfos[fd]['maplen'] = max(bufinfos[fd]['maplen'], fb.offset(i) + fb.length(i))
>> +
>> +        # mmap the buffers
>> +
>> +        maps = []
>> +
>> +        for fd, info in bufinfos.items():
>> +            map = mmap.mmap(fd, info['maplen'], mmap.MAP_SHARED, mmap.PROT_READ | mmap.PROT_WRITE)
>> +            info['map'] = map
>> +            maps.append(map)
>> +
>> +        self.__maps = tuple(maps)
>> +
>> +        # Create memoryviews for the planes
>> +
>> +        planes = []
>> +
>> +        for i in range(fb.num_planes):
>> +            fd = fb.fd(i)
>> +            info = bufinfos[fd]
>> +
>> +            mv = memoryview(info['map'])
>> +
>> +            start = fb.offset(i)
>> +            end = fb.offset(i) + fb.length(i)
>> +
>> +            mv = mv[start:end]
>> +
>> +            planes.append(mv)
>> +
>> +        self.__planes = tuple(planes)
>> +
>> +        return self
>> +
>> +    def __exit__(self, exc_type, exc_value, exc_traceback):
>> +        for p in self.__planes:
>> +            p.release()
>> +
>> +        for mm in self.__maps:
>> +            mm.close()
>> +
>> +    @property
>> +    def planes(self):
>> +        return self.__planes
>> +
>> +
>> +def __FrameBuffer__mmap(self):
>> +    return MappedFrameBuffer(self)
>> +
>> +
>> +FrameBuffer.mmap = __FrameBuffer__mmap
>> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build
>> new file mode 100644
>> index 00000000..e4abc34a
>> --- /dev/null
>> +++ b/src/py/libcamera/meson.build
>> @@ -0,0 +1,51 @@
>> +# SPDX-License-Identifier: CC0-1.0
>> +
>> +py3_dep = dependency('python3', required : get_option('pycamera'))
>> +
>> +if not py3_dep.found()
>> +    pycamera_enabled = false
>> +    subdir_done()
>> +endif
>> +
>> +pycamera_enabled = true
>> +
>> +pybind11_proj = subproject('pybind11')
>> +pybind11_dep = pybind11_proj.get_variable('pybind11_dep')
>> +
>> +pycamera_sources = files([
>> +    'pyenums.cpp',
>> +    'pymain.cpp',
>> +])
>> +
>> +pycamera_deps = [
>> +    libcamera_public,
>> +    py3_dep,
>> +    pybind11_dep,
>> +]
>> +
>> +pycamera_args = [
>> +    '-fvisibility=hidden',
>> +    '-Wno-shadow',
>> +    '-DPYBIND11_USE_SMART_HOLDER_AS_DEFAULT',
>> +]
>> +
>> +destdir = get_option('libdir') / ('python' + py3_dep.version()) / 'site-packages' / 'libcamera'
>> +
>> +pycamera = shared_module('_libcamera',
>> +                         pycamera_sources,
>> +                         install : true,
>> +                         install_dir : destdir,
>> +                         name_prefix : '',
>> +                         dependencies : pycamera_deps,
>> +                         cpp_args : pycamera_args)
>> +
>> +run_command('ln', '-fsT', '../../../../src/py/libcamera/__init__.py',
>> +            meson.current_build_dir() / '__init__.py',
>> +            check: true)
>> +
>> +install_data(['__init__.py'], install_dir : destdir)
>> +
>> +# \todo: Generate stubs when building. Depends on pybind11-stubgen. Sometimes
> 
> s/todo:/todo/
> 
> I'm still not sure what this is for :-) Do we need to generate stubs
> later ? What are they for ?

Oh, I see. https://peps.python.org/pep-0484/#stub-files

I'm not very familiar with them, but my editor is able to introspect 
pure python code, but not the pybind11 module. A stub file can provide 
the pure-python view to the module's API.

I haven't gotten them to work too well, thought. Earlier today it 
worked, then later it didn't. I haven't figured out the exact method on 
how the stub files are searched, etc...

>> +# this works, sometimes doesn't... To generate pylibcamera stubs.
>> +# $ PYTHONPATH=build/src/py pybind11-stubgen --no-setup-py -o build/src/py libcamera
>> +# $ mv build/src/py/libcamera-stubs/* build/src/py/libcamera/
>> diff --git a/src/py/libcamera/pyenums.cpp b/src/py/libcamera/pyenums.cpp
>> new file mode 100644
>> index 00000000..b655e622
>> --- /dev/null
>> +++ b/src/py/libcamera/pyenums.cpp
>> @@ -0,0 +1,34 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2022, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> + *
>> + * Python bindings - Enumerations
>> + */
>> +
>> +#include <libcamera/libcamera.h>
>> +
>> +#include <pybind11/smart_holder.h>
>> +
>> +namespace py = pybind11;
>> +
>> +using namespace libcamera;
>> +
>> +void init_pyenums(py::module &m)
>> +{
>> +	py::enum_<StreamRole>(m, "StreamRole")
>> +		.value("StillCapture", StreamRole::StillCapture)
>> +		.value("Raw", StreamRole::Raw)
>> +		.value("VideoRecording", StreamRole::VideoRecording)
>> +		.value("Viewfinder", StreamRole::Viewfinder);
>> +
>> +	py::enum_<ControlType>(m, "ControlType")
>> +		.value("None", ControlType::ControlTypeNone)
>> +		.value("Bool", ControlType::ControlTypeBool)
>> +		.value("Byte", ControlType::ControlTypeByte)
>> +		.value("Integer32", ControlType::ControlTypeInteger32)
>> +		.value("Integer64", ControlType::ControlTypeInteger64)
>> +		.value("Float", ControlType::ControlTypeFloat)
>> +		.value("String", ControlType::ControlTypeString)
>> +		.value("Rectangle", ControlType::ControlTypeRectangle)
>> +		.value("Size", ControlType::ControlTypeSize);
>> +}
>> diff --git a/src/py/libcamera/pymain.cpp b/src/py/libcamera/pymain.cpp
>> new file mode 100644
>> index 00000000..8c3be8f4
>> --- /dev/null
>> +++ b/src/py/libcamera/pymain.cpp
>> @@ -0,0 +1,640 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2022, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> + *
>> + * Python bindings
>> + */
>> +
>> +/*
>> + * \todo Add geometry classes (Point, Rectangle...)
>> + * \todo Add bindings for the ControlInfo class
>> + */
>> +
>> +#include <chrono>
>> +#include <fcntl.h>
>> +#include <mutex>
>> +#include <sys/eventfd.h>
>> +#include <sys/mman.h>
>> +#include <thread>
>> +#include <unistd.h>
>> +
>> +#include <libcamera/libcamera.h>
>> +
>> +#include <pybind11/functional.h>
>> +#include <pybind11/smart_holder.h>
>> +#include <pybind11/stl.h>
>> +#include <pybind11/stl_bind.h>
>> +
>> +namespace py = pybind11;
>> +
>> +using namespace libcamera;
>> +
>> +template<typename T>
>> +static py::object valueOrTuple(const ControlValue &cv)
>> +{
>> +	if (cv.isArray()) {
>> +		const T *v = reinterpret_cast<const T *>(cv.data().data());
>> +		auto t = py::tuple(cv.numElements());
>> +
>> +		for (size_t i = 0; i < cv.numElements(); ++i)
>> +			t[i] = v[i];
>> +
>> +		return t;
>> +	}
>> +
>> +	return py::cast(cv.get<T>());
>> +}
>> +
>> +static py::object controlValueToPy(const ControlValue &cv)
>> +{
>> +	switch (cv.type()) {
>> +	case ControlTypeBool:
>> +		return valueOrTuple<bool>(cv);
>> +	case ControlTypeByte:
>> +		return valueOrTuple<uint8_t>(cv);
>> +	case ControlTypeInteger32:
>> +		return valueOrTuple<int32_t>(cv);
>> +	case ControlTypeInteger64:
>> +		return valueOrTuple<int64_t>(cv);
>> +	case ControlTypeFloat:
>> +		return valueOrTuple<float>(cv);
>> +	case ControlTypeString:
>> +		return py::cast(cv.get<std::string>());
>> +	case ControlTypeRectangle: {
>> +		const Rectangle *v = reinterpret_cast<const Rectangle *>(cv.data().data());
>> +		return py::make_tuple(v->x, v->y, v->width, v->height);
>> +	}
>> +	case ControlTypeSize: {
>> +		const Size *v = reinterpret_cast<const Size *>(cv.data().data());
>> +		return py::make_tuple(v->width, v->height);
>> +	}
>> +	case ControlTypeNone:
>> +	default:
>> +		throw std::runtime_error("Unsupported ControlValue type");
>> +	}
>> +}
>> +
>> +template<typename T>
>> +static ControlValue controlValueMaybeArray(const py::object &ob)
>> +{
>> +	if (py::isinstance<py::list>(ob) || py::isinstance<py::tuple>(ob)) {
>> +		std::vector<T> vec = ob.cast<std::vector<T>>();
>> +		return ControlValue(Span<const T>(vec));
>> +	}
>> +
>> +	return ControlValue(ob.cast<T>());
>> +}
>> +
>> +static ControlValue pyToControlValue(const py::object &ob, ControlType type)
>> +{
>> +	switch (type) {
>> +	case ControlTypeBool:
>> +		return ControlValue(ob.cast<bool>());
>> +	case ControlTypeByte:
>> +		return controlValueMaybeArray<uint8_t>(ob);
>> +	case ControlTypeInteger32:
>> +		return controlValueMaybeArray<int32_t>(ob);
>> +	case ControlTypeInteger64:
>> +		return controlValueMaybeArray<int64_t>(ob);
>> +	case ControlTypeFloat:
>> +		return controlValueMaybeArray<float>(ob);
>> +	case ControlTypeString:
>> +		return ControlValue(ob.cast<std::string>());
>> +	case ControlTypeRectangle: {
>> +		auto array = ob.cast<std::array<int32_t, 4>>();
>> +		return ControlValue(Rectangle(array[0], array[1], array[2], array[3]));
>> +	}
>> +	case ControlTypeSize: {
>> +		auto array = ob.cast<std::array<int32_t, 2>>();
>> +		return ControlValue(Size(array[0], array[1]));
>> +	}
>> +	case ControlTypeNone:
>> +	default:
>> +		throw std::runtime_error("Control type not implemented");
>> +	}
>> +}
>> +
>> +static std::weak_ptr<CameraManager> gCameraManager;
>> +static int gEventfd;
>> +static std::mutex gReqlistMutex;
>> +static std::vector<Request *> gReqList;
>> +
>> +static void handleRequestCompleted(Request *req)
>> +{
>> +	{
>> +		std::lock_guard guard(gReqlistMutex);
>> +		gReqList.push_back(req);
>> +	}
>> +
>> +	uint64_t v = 1;
>> +	write(gEventfd, &v, 8);
>> +}
>> +
>> +void init_pyenums(py::module &m);
>> +
>> +PYBIND11_MODULE(_libcamera, m)
>> +{
>> +	init_pyenums(m);
>> +
>> +	/* Forward declarations */
>> +
>> +	/*
>> +	 * We need to declare all the classes here so that Python docstrings
>> +	 * can be generated correctly.
>> +	 * https://pybind11.readthedocs.io/en/latest/advanced/misc.html#avoiding-c-types-in-docstrings
>> +	 */
>> +
>> +	auto pyCameraManager = py::class_<CameraManager>(m, "CameraManager");
>> +	auto pyCamera = py::class_<Camera>(m, "Camera");
>> +	auto pyCameraConfiguration = py::class_<CameraConfiguration>(m, "CameraConfiguration");
>> +	auto pyCameraConfigurationStatus = py::enum_<CameraConfiguration::Status>(pyCameraConfiguration, "Status");
>> +	auto pyStreamConfiguration = py::class_<StreamConfiguration>(m, "StreamConfiguration");
>> +	auto pyStreamFormats = py::class_<StreamFormats>(m, "StreamFormats");
>> +	auto pyFrameBufferAllocator = py::class_<FrameBufferAllocator>(m, "FrameBufferAllocator");
>> +	auto pyFrameBuffer = py::class_<FrameBuffer>(m, "FrameBuffer");
>> +	auto pyStream = py::class_<Stream>(m, "Stream");
>> +	auto pyControlId = py::class_<ControlId>(m, "ControlId");
>> +	auto pyRequest = py::class_<Request>(m, "Request");
>> +	auto pyRequestStatus = py::enum_<Request::Status>(pyRequest, "Status");
>> +	auto pyRequestReuse = py::enum_<Request::ReuseFlag>(pyRequest, "Reuse");
>> +	auto pyFrameMetadata = py::class_<FrameMetadata>(m, "FrameMetadata");
>> +	auto pyFrameMetadataStatus = py::enum_<FrameMetadata::Status>(pyFrameMetadata, "Status");
>> +	auto pyTransform = py::class_<Transform>(m, "Transform");
>> +	auto pyColorSpace = py::class_<ColorSpace>(m, "ColorSpace");
>> +	auto pyColorSpacePrimaries = py::enum_<ColorSpace::Primaries>(pyColorSpace, "Primaries");
>> +	auto pyColorSpaceTransferFunction = py::enum_<ColorSpace::TransferFunction>(pyColorSpace, "TransferFunction");
>> +	auto pyColorSpaceYcbcrEncoding = py::enum_<ColorSpace::YcbcrEncoding>(pyColorSpace, "YcbcrEncoding");
>> +	auto pyColorSpaceRange = py::enum_<ColorSpace::Range>(pyColorSpace, "Range");
>> +
>> +	/* Global functions */
>> +	m.def("log_set_level", &logSetLevel);
>> +
>> +	/* Classes */
>> +	pyCameraManager
>> +		.def_static("singleton", []() {
>> +			std::shared_ptr<CameraManager> cm = gCameraManager.lock();
>> +			if (cm)
>> +				return cm;
>> +
>> +			int fd = eventfd(0, 0);
>> +			if (fd == -1)
>> +				throw std::system_error(errno, std::generic_category(),
>> +				                   "Failed to create eventfd");
> 
> There should be tabs instead of spaces, and the '"' should be aligned
> under errno. Same below.

Ok.

>> +
>> +			cm = std::shared_ptr<CameraManager>(new CameraManager, [](auto p) {
>> +				close(gEventfd);
>> +				gEventfd = -1;
>> +				delete p;
>> +			});
>> +
>> +			gEventfd = fd;
>> +			gCameraManager = cm;
>> +
>> +			int ret = cm->start();
>> +			if (ret)
>> +				throw std::system_error(-ret, std::generic_category(),
>> +				                   "Failed to start CameraManager");
>> +
>> +			return cm;
>> +		})
>> +
>> +		.def_property_readonly("version", &CameraManager::version)
>> +
>> +		.def_property_readonly("efd", [](CameraManager &) {
>> +			return gEventfd;
>> +		})
>> +
>> +		.def("get_ready_requests", [](CameraManager &) {
>> +			std::vector<Request *> v;
>> +
>> +			{
>> +				std::lock_guard guard(gReqlistMutex);
>> +				swap(v, gReqList);
>> +			}
>> +
>> +			std::vector<py::object> ret;
>> +
>> +			for (Request *req : v) {
>> +				py::object o = py::cast(req);
>> +				/* Decrease the ref increased in Camera.queue_request() */
>> +				o.dec_ref();
>> +				ret.push_back(o);
>> +			}
>> +
>> +			return ret;
>> +		})
>> +
>> +		.def("get", py::overload_cast<const std::string &>(&CameraManager::get), py::keep_alive<0, 1>())
>> +
>> +		/* Create a list of Cameras, where each camera has a keep-alive to CameraManager */
>> +		.def_property_readonly("cameras", [](CameraManager &self) {
>> +			py::list l;
>> +
>> +			for (auto &c : self.cameras()) {
>> +				py::object py_cm = py::cast(self);
>> +				py::object py_cam = py::cast(c);
>> +				py::detail::keep_alive_impl(py_cam, py_cm);
>> +				l.append(py_cam);
>> +			}
>> +
>> +			return l;
>> +		});
>> +
>> +	pyCamera
>> +		.def_property_readonly("id", &Camera::id)
>> +		.def("acquire", &Camera::acquire)
>> +		.def("release", &Camera::release)
>> +		.def("start", [](Camera &self, py::dict controls) {
>> +			/* \todo What happens if someone calls start() multiple times? */
>> +
>> +			self.requestCompleted.connect(handleRequestCompleted);
>> +
>> +			const ControlInfoMap &controlMap = self.controls();
>> +			ControlList controlList(controlMap);
>> +			for (const auto& [hkey, hval]: controls) {
>> +				auto key = hkey.cast<std::string>();
>> +
>> +				auto it = find_if(controlMap.begin(), controlMap.end(),
>> +						  [&key](const auto &kvp) {
>> +							  return kvp.first->name() == key; });
> 
> 				auto it = std::find_if(controlMap.begin(), controlMap.end(),
> 						       [&key](const auto &kvp) {
> 							       return kvp.first->name() == key;
> 						       });

Ok.

>> +
>> +				if (it == controlMap.end())
>> +					throw std::runtime_error("Control " + key + " not found");
>> +
>> +				const auto &id = it->first;
>> +				auto obj = py::cast<py::object>(hval);
>> +
>> +				controlList.set(id->id(), pyToControlValue(obj, id->type()));
>> +			}
>> +
>> +			int ret = self.start(&controlList);
>> +			if (ret) {
>> +				self.requestCompleted.disconnect(handleRequestCompleted);
>> +				return ret;
>> +			}
>> +
>> +			return 0;
>> +		}, py::arg("controls") = py::dict())
>> +
>> +		.def("stop", [](Camera &self) {
>> +			int ret = self.stop();
>> +			if (ret)
>> +				return ret;
>> +
>> +			self.requestCompleted.disconnect(handleRequestCompleted);
>> +
>> +			return 0;
>> +		})
>> +
>> +		.def("__repr__", [](Camera &self) {
>> +			return "<libcamera.Camera '" + self.id() + "'>";
>> +		})
>> +
>> +		/* Keep the camera alive, as StreamConfiguration contains a Stream* */
>> +		.def("generate_configuration", &Camera::generateConfiguration, py::keep_alive<0, 1>())
>> +		.def("configure", &Camera::configure)
>> +
>> +		.def("create_request", &Camera::createRequest, py::arg("cookie") = 0)
>> +
>> +		.def("queue_request", [](Camera &self, Request *req) {
>> +			py::object py_req = py::cast(req);
>> +
>> +			/*
>> +			 * Increase the reference count, will be dropped in
>> +			 * CameraManager.get_ready_requests().
>> +			 */
>> +
>> +			py_req.inc_ref();
>> +
>> +			int ret = self.queueRequest(req);
>> +			if (ret)
>> +				py_req.dec_ref();
>> +
>> +			return ret;
>> +		})
>> +
>> +		.def_property_readonly("streams", [](Camera &self) {
>> +			py::set set;
>> +			for (auto &s : self.streams()) {
>> +				py::object py_self = py::cast(self);
>> +				py::object py_s = py::cast(s);
>> +				py::detail::keep_alive_impl(py_s, py_self);
>> +				set.add(py_s);
>> +			}
>> +			return set;
>> +		})
>> +
>> +		.def("find_control", [](Camera &self, const std::string &name) {
>> +			const auto &controls = self.controls();
>> +
>> +			auto it = find_if(controls.begin(), controls.end(),
>> +					  [&name](const auto &kvp) { return kvp.first->name() == name; });
> 
> Missing std:: here too (and I would also wrap the line).

Interesting... Why does it compile...

>> +
>> +			if (it == controls.end())
>> +				throw std::runtime_error("Control not found");
> 
> 				throw std::runtime_error("Control '" + name + "' not found");
> 
> could be nicer to debug issues.

Ok.

>> +
>> +			return it->first;
>> +		}, py::return_value_policy::reference_internal)
>> +
>> +		.def_property_readonly("controls", [](Camera &self) {
>> +			py::dict ret;
>> +
>> +			for (const auto &[id, ci] : self.controls()) {
>> +				ret[id->name().c_str()] = std::make_tuple<py::object>(controlValueToPy(ci.min()),
>> +										      controlValueToPy(ci.max()),
>> +										      controlValueToPy(ci.def()));
>> +			}
>> +
>> +			return ret;
>> +		})
>> +
>> +		.def_property_readonly("properties", [](Camera &self) {
>> +			py::dict ret;
>> +
>> +			for (const auto &[key, cv] : self.properties()) {
>> +				const ControlId *id = properties::properties.at(key);
>> +				py::object ob = controlValueToPy(cv);
>> +
>> +				ret[id->name().c_str()] = ob;
>> +			}
>> +
>> +			return ret;
>> +		});
>> +
>> +	pyCameraConfiguration
>> +		.def("__iter__", [](CameraConfiguration &self) {
>> +			return py::make_iterator<py::return_value_policy::reference_internal>(self);
>> +		}, py::keep_alive<0, 1>())
>> +		.def("__len__", [](CameraConfiguration &self) {
>> +			return self.size();
>> +		})
>> +		.def("validate", &CameraConfiguration::validate)
>> +		.def("at", py::overload_cast<unsigned int>(&CameraConfiguration::at),
>> +		     py::return_value_policy::reference_internal)
>> +		.def_property_readonly("size", &CameraConfiguration::size)
>> +		.def_property_readonly("empty", &CameraConfiguration::empty)
>> +		.def_readwrite("transform", &CameraConfiguration::transform);
>> +
>> +	pyCameraConfigurationStatus
>> +		.value("Valid", CameraConfiguration::Valid)
>> +		.value("Adjusted", CameraConfiguration::Adjusted)
>> +		.value("Invalid", CameraConfiguration::Invalid);
>> +
>> +	pyStreamConfiguration
>> +		.def("to_string", &StreamConfiguration::toString)
> 
> Should this be __str__ ?

Yes. And we seem to have a few __repr__, which should be __str__. I'll 
change those too.

>> +		.def_property_readonly("stream", &StreamConfiguration::stream,
>> +		                       py::return_value_policy::reference_internal)
>> +		.def_property(
>> +			"size",
>> +			[](StreamConfiguration &self) {
>> +				return std::make_tuple(self.size.width, self.size.height);
>> +			},
>> +			[](StreamConfiguration &self, std::tuple<uint32_t, uint32_t> size) {
>> +				self.size.width = std::get<0>(size);
>> +				self.size.height = std::get<1>(size);
>> +			})
>> +		.def_property(
>> +			"pixel_format",
>> +			[](StreamConfiguration &self) {
>> +				return self.pixelFormat.toString();
>> +			},
>> +			[](StreamConfiguration &self, std::string fmt) {
>> +				self.pixelFormat = PixelFormat::fromString(fmt);
>> +			})
>> +		.def_readwrite("stride", &StreamConfiguration::stride)
>> +		.def_readwrite("frame_size", &StreamConfiguration::frameSize)
>> +		.def_readwrite("buffer_count", &StreamConfiguration::bufferCount)
>> +		.def_property_readonly("formats", &StreamConfiguration::formats,
>> +		                       py::return_value_policy::reference_internal)
>> +		.def_readwrite("colorSpace", &StreamConfiguration::colorSpace);
> 
> color_space

Ok.

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

  Tomi

Hi Tomi,

On Fri, May 06, 2022 at 08:50:20PM +0300, Tomi Valkeinen wrote:
> On 06/05/2022 20:21, Laurent Pinchart wrote:
> > On Fri, May 06, 2022 at 05:54:11PM +0300, Tomi Valkeinen wrote:
> >> Add libcamera Python bindings. pybind11 is used to generate the C++ <->
> >> Python layer.
> >>
> >> We use pybind11 'smart_holder' version to avoid issues with private
> >> destructors and shared_ptr. There is also an alternative solution here:
> >>
> >> https://github.com/pybind/pybind11/pull/2067
> >>
> >> Only a subset of libcamera classes are exposed. Implementing and testing
> >> the wrapper classes is challenging, and as such only classes that I have
> >> needed have been added so far.

There's another issue I'm afraid :-( Compiling with clang-13, I get

In file included from ../../src/py/libcamera/pyenums.cpp:10:
In file included from ../../subprojects/pybind11/include/pybind11/smart_holder.h:7:
In file included from ../../subprojects/pybind11/include/pybind11/pybind11.h:13:
In file included from ../../subprojects/pybind11/include/pybind11/detail/class.h:12:
In file included from ../../subprojects/pybind11/include/pybind11/detail/../attr.h:14:
In file included from ../../subprojects/pybind11/include/pybind11/cast.h:33:
In file included from ../../subprojects/pybind11/include/pybind11/detail/smart_holder_type_casters.h:9:
In file included from ../../subprojects/pybind11/include/pybind11/detail/../trampoline_self_life_support.h:8:
../../subprojects/pybind11/include/pybind11/detail/smart_holder_poc.h:109:2: error: extra ';' outside of a function is incompatible with C++98 [-Werror,-Wc++98-compat-extra-semi]
};
 ^
1 error generated.

With clang-9, there's additionally

In file included from ../../src/py/libcamera/pymain.cpp:23:
In file included from ../../subprojects/pybind11/include/pybind11/functional.h:12:
In file included from ../../subprojects/pybind11/include/pybind11/pybind11.h:13:
In file included from ../../subprojects/pybind11/include/pybind11/detail/class.h:12:
In file included from ../../subprojects/pybind11/include/pybind11/detail/../attr.h:14:
In file included from ../../subprojects/pybind11/include/pybind11/cast.h:33:
In file included from ../../subprojects/pybind11/include/pybind11/detail/smart_holder_type_casters.h:9:
In file included from ../../subprojects/pybind11/include/pybind11/detail/../trampoline_self_life_support.h:8:
../../subprojects/pybind11/include/pybind11/detail/smart_holder_poc.h:109:2: error: extra ';' outside of a function is incompatible with C++98 [-Werror,-Wc++98-compat-extra-semi]
};
 ^
../../src/py/libcamera/pymain.cpp:42:10: error: local variable 't' will be copied despite being returned by name [-Werror,-Wreturn-std-move]
                return t;
                       ^
../../src/py/libcamera/pymain.cpp:42:10: note: call 'std::move' explicitly to avoid copying
                return t;
                       ^
                       std::move(t)
../../src/py/libcamera/pymain.cpp:42:10: error: local variable 't' will be copied despite being returned by name [-Werror,-Wreturn-std-move]
                return t;
                       ^
../../src/py/libcamera/pymain.cpp:52:10: note: in instantiation of function template specialization 'valueOrTuple<bool>' requested here
                return valueOrTuple<bool>(cv);
                       ^
../../src/py/libcamera/pymain.cpp:42:10: note: call 'std::move' explicitly to avoid copying
                return t;
                       ^
                       std::move(t)
../../src/py/libcamera/pymain.cpp:42:10: error: local variable 't' will be copied despite being returned by name [-Werror,-Wreturn-std-move]
                return t;
                       ^
../../src/py/libcamera/pymain.cpp:54:10: note: in instantiation of function template specialization 'valueOrTuple<unsigned char>' requested here
                return valueOrTuple<uint8_t>(cv);
                       ^
../../src/py/libcamera/pymain.cpp:42:10: note: call 'std::move' explicitly to avoid copying
                return t;
                       ^
                       std::move(t)
../../src/py/libcamera/pymain.cpp:42:10: error: local variable 't' will be copied despite being returned by name [-Werror,-Wreturn-std-move]
                return t;
                       ^
../../src/py/libcamera/pymain.cpp:56:10: note: in instantiation of function template specialization 'valueOrTuple<int>' requested here
                return valueOrTuple<int32_t>(cv);
                       ^
../../src/py/libcamera/pymain.cpp:42:10: note: call 'std::move' explicitly to avoid copying
                return t;
                       ^
                       std::move(t)
../../src/py/libcamera/pymain.cpp:42:10: error: local variable 't' will be copied despite being returned by name [-Werror,-Wreturn-std-move]
                return t;
                       ^
../../src/py/libcamera/pymain.cpp:58:10: note: in instantiation of function template specialization 'valueOrTuple<long>' requested here
                return valueOrTuple<int64_t>(cv);
                       ^
../../src/py/libcamera/pymain.cpp:42:10: note: call 'std::move' explicitly to avoid copying
                return t;
                       ^
                       std::move(t)
../../src/py/libcamera/pymain.cpp:42:10: error: local variable 't' will be copied despite being returned by name [-Werror,-Wreturn-std-move]
                return t;
                       ^
../../src/py/libcamera/pymain.cpp:60:10: note: in instantiation of function template specialization 'valueOrTuple<float>' requested here
                return valueOrTuple<float>(cv);
                       ^
../../src/py/libcamera/pymain.cpp:42:10: note: call 'std::move' explicitly to avoid copying
                return t;
                       ^
                       std::move(t)
7 errors generated.

With gcc-10 and gcc-11, errors are different:

../../src/py/libcamera/pymain.cpp: In function ‘void handleRequestCompleted(libcamera::Request*)’:
../../src/py/libcamera/pymain.cpp:130:14: error: ignoring return value of ‘ssize_t write(int, const void*, size_t)’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
  130 |         write(gEventfd, &v, 8);
      |         ~~~~~^~~~~~~~~~~~~~~~~

> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> >> ---
> >>   meson.build                                   |   1 +
> >>   meson_options.txt                             |   5 +
> >>   src/meson.build                               |   1 +
> >>   src/py/libcamera/__init__.py                  |  84 +++
> >>   src/py/libcamera/meson.build                  |  51 ++
> >>   src/py/libcamera/pyenums.cpp                  |  34 +
> >>   src/py/libcamera/pymain.cpp                   | 640 ++++++++++++++++++
> >>   src/py/meson.build                            |   1 +
> >>   subprojects/.gitignore                        |   3 +-
> >>   subprojects/packagefiles/pybind11/meson.build |   7 +
> >>   subprojects/pybind11.wrap                     |   9 +
> >>   11 files changed, 835 insertions(+), 1 deletion(-)
> >>   create mode 100644 src/py/libcamera/__init__.py
> >>   create mode 100644 src/py/libcamera/meson.build
> >>   create mode 100644 src/py/libcamera/pyenums.cpp
> >>   create mode 100644 src/py/libcamera/pymain.cpp
> >>   create mode 100644 src/py/meson.build
> >>   create mode 100644 subprojects/packagefiles/pybind11/meson.build
> >>   create mode 100644 subprojects/pybind11.wrap
> >>
> >> diff --git a/meson.build b/meson.build
> >> index 0124e7d3..60a911e0 100644
> >> --- a/meson.build
> >> +++ b/meson.build
> >> @@ -177,6 +177,7 @@ summary({
> >>               'Tracing support': tracing_enabled,
> >>               'Android support': android_enabled,
> >>               'GStreamer support': gst_enabled,
> >> +            'Python bindings': pycamera_enabled,
> >>               'V4L2 emulation support': v4l2_enabled,
> >>               'cam application': cam_enabled,
> >>               'qcam application': qcam_enabled,
> >> diff --git a/meson_options.txt b/meson_options.txt
> >> index 2c80ad8b..ca00c78e 100644
> >> --- a/meson_options.txt
> >> +++ b/meson_options.txt
> >> @@ -58,3 +58,8 @@ option('v4l2',
> >>           type : 'boolean',
> >>           value : false,
> >>           description : 'Compile the V4L2 compatibility layer')
> >> +
> >> +option('pycamera',
> >> +        type : 'feature',
> >> +        value : 'auto',
> >> +        description : 'Enable libcamera Python bindings (experimental)')
> >> diff --git a/src/meson.build b/src/meson.build
> >> index e0ea9c35..34663a6f 100644
> >> --- a/src/meson.build
> >> +++ b/src/meson.build
> >> @@ -37,4 +37,5 @@ subdir('cam')
> >>   subdir('qcam')
> >>   
> >>   subdir('gstreamer')
> >> +subdir('py')
> >>   subdir('v4l2')
> >> diff --git a/src/py/libcamera/__init__.py b/src/py/libcamera/__init__.py
> >> new file mode 100644
> >> index 00000000..6b330890
> >> --- /dev/null
> >> +++ b/src/py/libcamera/__init__.py
> >> @@ -0,0 +1,84 @@
> >> +# SPDX-License-Identifier: LGPL-2.1-or-later
> >> +# Copyright (C) 2022, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> >> +
> >> +from ._libcamera import *
> >> +
> >> +
> >> +class MappedFrameBuffer:
> >> +    def __init__(self, fb):
> >> +        self.__fb = fb
> >> +
> >> +    def __enter__(self):
> >> +        from os import lseek, SEEK_END
> > 
> > As it's local to the function it doesn't matter much, but I would have
> > just imported os and used os.lseek and os.SEEK_END.
> 
> Yep, makes sense.
> 
> >> +        import mmap
> >> +
> >> +        fb = self.__fb
> >> +
> >> +        # Collect information about the buffers
> >> +
> >> +        bufinfos = {}
> >> +
> >> +        for i in range(fb.num_planes):
> >> +            fd = fb.fd(i)
> >> +
> >> +            if fd not in bufinfos:
> >> +                buflen = lseek(fd, 0, SEEK_END)
> >> +                bufinfos[fd] = {'maplen': 0, 'buflen': buflen}
> >> +            else:
> >> +                buflen = bufinfos[fd]['buflen']
> >> +
> >> +            if fb.offset(i) > buflen or fb.offset(i) + fb.length(i) > buflen:
> >> +                raise RuntimeError(f'plane is out of buffer: buffer length={buflen}, ' +
> >> +                                   f'plane offset={fb.offset(i)}, plane length={fb.length(i)}')
> >> +
> >> +            bufinfos[fd]['maplen'] = max(bufinfos[fd]['maplen'], fb.offset(i) + fb.length(i))
> >> +
> >> +        # mmap the buffers
> >> +
> >> +        maps = []
> >> +
> >> +        for fd, info in bufinfos.items():
> >> +            map = mmap.mmap(fd, info['maplen'], mmap.MAP_SHARED, mmap.PROT_READ | mmap.PROT_WRITE)
> >> +            info['map'] = map
> >> +            maps.append(map)
> >> +
> >> +        self.__maps = tuple(maps)
> >> +
> >> +        # Create memoryviews for the planes
> >> +
> >> +        planes = []
> >> +
> >> +        for i in range(fb.num_planes):
> >> +            fd = fb.fd(i)
> >> +            info = bufinfos[fd]
> >> +
> >> +            mv = memoryview(info['map'])
> >> +
> >> +            start = fb.offset(i)
> >> +            end = fb.offset(i) + fb.length(i)
> >> +
> >> +            mv = mv[start:end]
> >> +
> >> +            planes.append(mv)
> >> +
> >> +        self.__planes = tuple(planes)
> >> +
> >> +        return self
> >> +
> >> +    def __exit__(self, exc_type, exc_value, exc_traceback):
> >> +        for p in self.__planes:
> >> +            p.release()
> >> +
> >> +        for mm in self.__maps:
> >> +            mm.close()
> >> +
> >> +    @property
> >> +    def planes(self):
> >> +        return self.__planes
> >> +
> >> +
> >> +def __FrameBuffer__mmap(self):
> >> +    return MappedFrameBuffer(self)
> >> +
> >> +
> >> +FrameBuffer.mmap = __FrameBuffer__mmap
> >> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build
> >> new file mode 100644
> >> index 00000000..e4abc34a
> >> --- /dev/null
> >> +++ b/src/py/libcamera/meson.build
> >> @@ -0,0 +1,51 @@
> >> +# SPDX-License-Identifier: CC0-1.0
> >> +
> >> +py3_dep = dependency('python3', required : get_option('pycamera'))
> >> +
> >> +if not py3_dep.found()
> >> +    pycamera_enabled = false
> >> +    subdir_done()
> >> +endif
> >> +
> >> +pycamera_enabled = true
> >> +
> >> +pybind11_proj = subproject('pybind11')
> >> +pybind11_dep = pybind11_proj.get_variable('pybind11_dep')
> >> +
> >> +pycamera_sources = files([
> >> +    'pyenums.cpp',
> >> +    'pymain.cpp',
> >> +])
> >> +
> >> +pycamera_deps = [
> >> +    libcamera_public,
> >> +    py3_dep,
> >> +    pybind11_dep,
> >> +]
> >> +
> >> +pycamera_args = [
> >> +    '-fvisibility=hidden',
> >> +    '-Wno-shadow',
> >> +    '-DPYBIND11_USE_SMART_HOLDER_AS_DEFAULT',
> >> +]
> >> +
> >> +destdir = get_option('libdir') / ('python' + py3_dep.version()) / 'site-packages' / 'libcamera'
> >> +
> >> +pycamera = shared_module('_libcamera',
> >> +                         pycamera_sources,
> >> +                         install : true,
> >> +                         install_dir : destdir,
> >> +                         name_prefix : '',
> >> +                         dependencies : pycamera_deps,
> >> +                         cpp_args : pycamera_args)
> >> +
> >> +run_command('ln', '-fsT', '../../../../src/py/libcamera/__init__.py',
> >> +            meson.current_build_dir() / '__init__.py',
> >> +            check: true)
> >> +
> >> +install_data(['__init__.py'], install_dir : destdir)
> >> +
> >> +# \todo: Generate stubs when building. Depends on pybind11-stubgen. Sometimes
> > 
> > s/todo:/todo/
> > 
> > I'm still not sure what this is for :-) Do we need to generate stubs
> > later ? What are they for ?
> 
> Oh, I see. https://peps.python.org/pep-0484/#stub-files
> 
> I'm not very familiar with them, but my editor is able to introspect 
> pure python code, but not the pybind11 module. A stub file can provide 
> the pure-python view to the module's API.
> 
> I haven't gotten them to work too well, thought. Earlier today it 
> worked, then later it didn't. I haven't figured out the exact method on 
> how the stub files are searched, etc...

Thanks for the explanation. Adding the above link to the comment would
be enough for me.

> >> +# this works, sometimes doesn't... To generate pylibcamera stubs.
> >> +# $ PYTHONPATH=build/src/py pybind11-stubgen --no-setup-py -o build/src/py libcamera
> >> +# $ mv build/src/py/libcamera-stubs/* build/src/py/libcamera/
> >> diff --git a/src/py/libcamera/pyenums.cpp b/src/py/libcamera/pyenums.cpp
> >> new file mode 100644
> >> index 00000000..b655e622
> >> --- /dev/null
> >> +++ b/src/py/libcamera/pyenums.cpp
> >> @@ -0,0 +1,34 @@
> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> >> +/*
> >> + * Copyright (C) 2022, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> >> + *
> >> + * Python bindings - Enumerations
> >> + */
> >> +
> >> +#include <libcamera/libcamera.h>
> >> +
> >> +#include <pybind11/smart_holder.h>
> >> +
> >> +namespace py = pybind11;
> >> +
> >> +using namespace libcamera;
> >> +
> >> +void init_pyenums(py::module &m)
> >> +{
> >> +	py::enum_<StreamRole>(m, "StreamRole")
> >> +		.value("StillCapture", StreamRole::StillCapture)
> >> +		.value("Raw", StreamRole::Raw)
> >> +		.value("VideoRecording", StreamRole::VideoRecording)
> >> +		.value("Viewfinder", StreamRole::Viewfinder);
> >> +
> >> +	py::enum_<ControlType>(m, "ControlType")
> >> +		.value("None", ControlType::ControlTypeNone)
> >> +		.value("Bool", ControlType::ControlTypeBool)
> >> +		.value("Byte", ControlType::ControlTypeByte)
> >> +		.value("Integer32", ControlType::ControlTypeInteger32)
> >> +		.value("Integer64", ControlType::ControlTypeInteger64)
> >> +		.value("Float", ControlType::ControlTypeFloat)
> >> +		.value("String", ControlType::ControlTypeString)
> >> +		.value("Rectangle", ControlType::ControlTypeRectangle)
> >> +		.value("Size", ControlType::ControlTypeSize);
> >> +}
> >> diff --git a/src/py/libcamera/pymain.cpp b/src/py/libcamera/pymain.cpp
> >> new file mode 100644
> >> index 00000000..8c3be8f4
> >> --- /dev/null
> >> +++ b/src/py/libcamera/pymain.cpp
> >> @@ -0,0 +1,640 @@
> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> >> +/*
> >> + * Copyright (C) 2022, Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> >> + *
> >> + * Python bindings
> >> + */
> >> +
> >> +/*
> >> + * \todo Add geometry classes (Point, Rectangle...)
> >> + * \todo Add bindings for the ControlInfo class
> >> + */
> >> +
> >> +#include <chrono>
> >> +#include <fcntl.h>
> >> +#include <mutex>
> >> +#include <sys/eventfd.h>
> >> +#include <sys/mman.h>
> >> +#include <thread>
> >> +#include <unistd.h>
> >> +
> >> +#include <libcamera/libcamera.h>
> >> +
> >> +#include <pybind11/functional.h>
> >> +#include <pybind11/smart_holder.h>
> >> +#include <pybind11/stl.h>
> >> +#include <pybind11/stl_bind.h>
> >> +
> >> +namespace py = pybind11;
> >> +
> >> +using namespace libcamera;
> >> +
> >> +template<typename T>
> >> +static py::object valueOrTuple(const ControlValue &cv)
> >> +{
> >> +	if (cv.isArray()) {
> >> +		const T *v = reinterpret_cast<const T *>(cv.data().data());
> >> +		auto t = py::tuple(cv.numElements());
> >> +
> >> +		for (size_t i = 0; i < cv.numElements(); ++i)
> >> +			t[i] = v[i];
> >> +
> >> +		return t;
> >> +	}
> >> +
> >> +	return py::cast(cv.get<T>());
> >> +}
> >> +
> >> +static py::object controlValueToPy(const ControlValue &cv)
> >> +{
> >> +	switch (cv.type()) {
> >> +	case ControlTypeBool:
> >> +		return valueOrTuple<bool>(cv);
> >> +	case ControlTypeByte:
> >> +		return valueOrTuple<uint8_t>(cv);
> >> +	case ControlTypeInteger32:
> >> +		return valueOrTuple<int32_t>(cv);
> >> +	case ControlTypeInteger64:
> >> +		return valueOrTuple<int64_t>(cv);
> >> +	case ControlTypeFloat:
> >> +		return valueOrTuple<float>(cv);
> >> +	case ControlTypeString:
> >> +		return py::cast(cv.get<std::string>());
> >> +	case ControlTypeRectangle: {
> >> +		const Rectangle *v = reinterpret_cast<const Rectangle *>(cv.data().data());
> >> +		return py::make_tuple(v->x, v->y, v->width, v->height);
> >> +	}
> >> +	case ControlTypeSize: {
> >> +		const Size *v = reinterpret_cast<const Size *>(cv.data().data());
> >> +		return py::make_tuple(v->width, v->height);
> >> +	}
> >> +	case ControlTypeNone:
> >> +	default:
> >> +		throw std::runtime_error("Unsupported ControlValue type");
> >> +	}
> >> +}
> >> +
> >> +template<typename T>
> >> +static ControlValue controlValueMaybeArray(const py::object &ob)
> >> +{
> >> +	if (py::isinstance<py::list>(ob) || py::isinstance<py::tuple>(ob)) {
> >> +		std::vector<T> vec = ob.cast<std::vector<T>>();
> >> +		return ControlValue(Span<const T>(vec));
> >> +	}
> >> +
> >> +	return ControlValue(ob.cast<T>());
> >> +}
> >> +
> >> +static ControlValue pyToControlValue(const py::object &ob, ControlType type)
> >> +{
> >> +	switch (type) {
> >> +	case ControlTypeBool:
> >> +		return ControlValue(ob.cast<bool>());
> >> +	case ControlTypeByte:
> >> +		return controlValueMaybeArray<uint8_t>(ob);
> >> +	case ControlTypeInteger32:
> >> +		return controlValueMaybeArray<int32_t>(ob);
> >> +	case ControlTypeInteger64:
> >> +		return controlValueMaybeArray<int64_t>(ob);
> >> +	case ControlTypeFloat:
> >> +		return controlValueMaybeArray<float>(ob);
> >> +	case ControlTypeString:
> >> +		return ControlValue(ob.cast<std::string>());
> >> +	case ControlTypeRectangle: {
> >> +		auto array = ob.cast<std::array<int32_t, 4>>();
> >> +		return ControlValue(Rectangle(array[0], array[1], array[2], array[3]));
> >> +	}
> >> +	case ControlTypeSize: {
> >> +		auto array = ob.cast<std::array<int32_t, 2>>();
> >> +		return ControlValue(Size(array[0], array[1]));
> >> +	}
> >> +	case ControlTypeNone:
> >> +	default:
> >> +		throw std::runtime_error("Control type not implemented");
> >> +	}
> >> +}
> >> +
> >> +static std::weak_ptr<CameraManager> gCameraManager;
> >> +static int gEventfd;
> >> +static std::mutex gReqlistMutex;
> >> +static std::vector<Request *> gReqList;
> >> +
> >> +static void handleRequestCompleted(Request *req)
> >> +{
> >> +	{
> >> +		std::lock_guard guard(gReqlistMutex);
> >> +		gReqList.push_back(req);
> >> +	}
> >> +
> >> +	uint64_t v = 1;
> >> +	write(gEventfd, &v, 8);
> >> +}
> >> +
> >> +void init_pyenums(py::module &m);
> >> +
> >> +PYBIND11_MODULE(_libcamera, m)
> >> +{
> >> +	init_pyenums(m);
> >> +
> >> +	/* Forward declarations */
> >> +
> >> +	/*
> >> +	 * We need to declare all the classes here so that Python docstrings
> >> +	 * can be generated correctly.
> >> +	 * https://pybind11.readthedocs.io/en/latest/advanced/misc.html#avoiding-c-types-in-docstrings
> >> +	 */
> >> +
> >> +	auto pyCameraManager = py::class_<CameraManager>(m, "CameraManager");
> >> +	auto pyCamera = py::class_<Camera>(m, "Camera");
> >> +	auto pyCameraConfiguration = py::class_<CameraConfiguration>(m, "CameraConfiguration");
> >> +	auto pyCameraConfigurationStatus = py::enum_<CameraConfiguration::Status>(pyCameraConfiguration, "Status");
> >> +	auto pyStreamConfiguration = py::class_<StreamConfiguration>(m, "StreamConfiguration");
> >> +	auto pyStreamFormats = py::class_<StreamFormats>(m, "StreamFormats");
> >> +	auto pyFrameBufferAllocator = py::class_<FrameBufferAllocator>(m, "FrameBufferAllocator");
> >> +	auto pyFrameBuffer = py::class_<FrameBuffer>(m, "FrameBuffer");
> >> +	auto pyStream = py::class_<Stream>(m, "Stream");
> >> +	auto pyControlId = py::class_<ControlId>(m, "ControlId");
> >> +	auto pyRequest = py::class_<Request>(m, "Request");
> >> +	auto pyRequestStatus = py::enum_<Request::Status>(pyRequest, "Status");
> >> +	auto pyRequestReuse = py::enum_<Request::ReuseFlag>(pyRequest, "Reuse");
> >> +	auto pyFrameMetadata = py::class_<FrameMetadata>(m, "FrameMetadata");
> >> +	auto pyFrameMetadataStatus = py::enum_<FrameMetadata::Status>(pyFrameMetadata, "Status");
> >> +	auto pyTransform = py::class_<Transform>(m, "Transform");
> >> +	auto pyColorSpace = py::class_<ColorSpace>(m, "ColorSpace");
> >> +	auto pyColorSpacePrimaries = py::enum_<ColorSpace::Primaries>(pyColorSpace, "Primaries");
> >> +	auto pyColorSpaceTransferFunction = py::enum_<ColorSpace::TransferFunction>(pyColorSpace, "TransferFunction");
> >> +	auto pyColorSpaceYcbcrEncoding = py::enum_<ColorSpace::YcbcrEncoding>(pyColorSpace, "YcbcrEncoding");
> >> +	auto pyColorSpaceRange = py::enum_<ColorSpace::Range>(pyColorSpace, "Range");
> >> +
> >> +	/* Global functions */
> >> +	m.def("log_set_level", &logSetLevel);
> >> +
> >> +	/* Classes */
> >> +	pyCameraManager
> >> +		.def_static("singleton", []() {
> >> +			std::shared_ptr<CameraManager> cm = gCameraManager.lock();
> >> +			if (cm)
> >> +				return cm;
> >> +
> >> +			int fd = eventfd(0, 0);
> >> +			if (fd == -1)
> >> +				throw std::system_error(errno, std::generic_category(),
> >> +				                   "Failed to create eventfd");
> > 
> > There should be tabs instead of spaces, and the '"' should be aligned
> > under errno. Same below.
> 
> Ok.
> 
> >> +
> >> +			cm = std::shared_ptr<CameraManager>(new CameraManager, [](auto p) {
> >> +				close(gEventfd);
> >> +				gEventfd = -1;
> >> +				delete p;
> >> +			});
> >> +
> >> +			gEventfd = fd;
> >> +			gCameraManager = cm;
> >> +
> >> +			int ret = cm->start();
> >> +			if (ret)
> >> +				throw std::system_error(-ret, std::generic_category(),
> >> +				                   "Failed to start CameraManager");
> >> +
> >> +			return cm;
> >> +		})
> >> +
> >> +		.def_property_readonly("version", &CameraManager::version)
> >> +
> >> +		.def_property_readonly("efd", [](CameraManager &) {
> >> +			return gEventfd;
> >> +		})
> >> +
> >> +		.def("get_ready_requests", [](CameraManager &) {
> >> +			std::vector<Request *> v;
> >> +
> >> +			{
> >> +				std::lock_guard guard(gReqlistMutex);
> >> +				swap(v, gReqList);
> >> +			}
> >> +
> >> +			std::vector<py::object> ret;
> >> +
> >> +			for (Request *req : v) {
> >> +				py::object o = py::cast(req);
> >> +				/* Decrease the ref increased in Camera.queue_request() */
> >> +				o.dec_ref();
> >> +				ret.push_back(o);
> >> +			}
> >> +
> >> +			return ret;
> >> +		})
> >> +
> >> +		.def("get", py::overload_cast<const std::string &>(&CameraManager::get), py::keep_alive<0, 1>())
> >> +
> >> +		/* Create a list of Cameras, where each camera has a keep-alive to CameraManager */
> >> +		.def_property_readonly("cameras", [](CameraManager &self) {
> >> +			py::list l;
> >> +
> >> +			for (auto &c : self.cameras()) {
> >> +				py::object py_cm = py::cast(self);
> >> +				py::object py_cam = py::cast(c);
> >> +				py::detail::keep_alive_impl(py_cam, py_cm);
> >> +				l.append(py_cam);
> >> +			}
> >> +
> >> +			return l;
> >> +		});
> >> +
> >> +	pyCamera
> >> +		.def_property_readonly("id", &Camera::id)
> >> +		.def("acquire", &Camera::acquire)
> >> +		.def("release", &Camera::release)
> >> +		.def("start", [](Camera &self, py::dict controls) {
> >> +			/* \todo What happens if someone calls start() multiple times? */
> >> +
> >> +			self.requestCompleted.connect(handleRequestCompleted);
> >> +
> >> +			const ControlInfoMap &controlMap = self.controls();
> >> +			ControlList controlList(controlMap);
> >> +			for (const auto& [hkey, hval]: controls) {
> >> +				auto key = hkey.cast<std::string>();
> >> +
> >> +				auto it = find_if(controlMap.begin(), controlMap.end(),
> >> +						  [&key](const auto &kvp) {
> >> +							  return kvp.first->name() == key; });
> > 
> > 				auto it = std::find_if(controlMap.begin(), controlMap.end(),
> > 						       [&key](const auto &kvp) {
> > 							       return kvp.first->name() == key;
> > 						       });
> 
> Ok.
> 
> >> +
> >> +				if (it == controlMap.end())
> >> +					throw std::runtime_error("Control " + key + " not found");
> >> +
> >> +				const auto &id = it->first;
> >> +				auto obj = py::cast<py::object>(hval);
> >> +
> >> +				controlList.set(id->id(), pyToControlValue(obj, id->type()));
> >> +			}
> >> +
> >> +			int ret = self.start(&controlList);
> >> +			if (ret) {
> >> +				self.requestCompleted.disconnect(handleRequestCompleted);
> >> +				return ret;
> >> +			}
> >> +
> >> +			return 0;
> >> +		}, py::arg("controls") = py::dict())
> >> +
> >> +		.def("stop", [](Camera &self) {
> >> +			int ret = self.stop();
> >> +			if (ret)
> >> +				return ret;
> >> +
> >> +			self.requestCompleted.disconnect(handleRequestCompleted);
> >> +
> >> +			return 0;
> >> +		})
> >> +
> >> +		.def("__repr__", [](Camera &self) {
> >> +			return "<libcamera.Camera '" + self.id() + "'>";
> >> +		})
> >> +
> >> +		/* Keep the camera alive, as StreamConfiguration contains a Stream* */
> >> +		.def("generate_configuration", &Camera::generateConfiguration, py::keep_alive<0, 1>())
> >> +		.def("configure", &Camera::configure)
> >> +
> >> +		.def("create_request", &Camera::createRequest, py::arg("cookie") = 0)
> >> +
> >> +		.def("queue_request", [](Camera &self, Request *req) {
> >> +			py::object py_req = py::cast(req);
> >> +
> >> +			/*
> >> +			 * Increase the reference count, will be dropped in
> >> +			 * CameraManager.get_ready_requests().
> >> +			 */
> >> +
> >> +			py_req.inc_ref();
> >> +
> >> +			int ret = self.queueRequest(req);
> >> +			if (ret)
> >> +				py_req.dec_ref();
> >> +
> >> +			return ret;
> >> +		})
> >> +
> >> +		.def_property_readonly("streams", [](Camera &self) {
> >> +			py::set set;
> >> +			for (auto &s : self.streams()) {
> >> +				py::object py_self = py::cast(self);
> >> +				py::object py_s = py::cast(s);
> >> +				py::detail::keep_alive_impl(py_s, py_self);
> >> +				set.add(py_s);
> >> +			}
> >> +			return set;
> >> +		})
> >> +
> >> +		.def("find_control", [](Camera &self, const std::string &name) {
> >> +			const auto &controls = self.controls();
> >> +
> >> +			auto it = find_if(controls.begin(), controls.end(),
> >> +					  [&name](const auto &kvp) { return kvp.first->name() == name; });
> > 
> > Missing std:: here too (and I would also wrap the line).
> 
> Interesting... Why does it compile...

If you figure it out, I'm interested in knowing :-)

> >> +
> >> +			if (it == controls.end())
> >> +				throw std::runtime_error("Control not found");
> > 
> > 				throw std::runtime_error("Control '" + name + "' not found");
> > 
> > could be nicer to debug issues.
> 
> Ok.
> 
> >> +
> >> +			return it->first;
> >> +		}, py::return_value_policy::reference_internal)
> >> +
> >> +		.def_property_readonly("controls", [](Camera &self) {
> >> +			py::dict ret;
> >> +
> >> +			for (const auto &[id, ci] : self.controls()) {
> >> +				ret[id->name().c_str()] = std::make_tuple<py::object>(controlValueToPy(ci.min()),
> >> +										      controlValueToPy(ci.max()),
> >> +										      controlValueToPy(ci.def()));
> >> +			}
> >> +
> >> +			return ret;
> >> +		})
> >> +
> >> +		.def_property_readonly("properties", [](Camera &self) {
> >> +			py::dict ret;
> >> +
> >> +			for (const auto &[key, cv] : self.properties()) {
> >> +				const ControlId *id = properties::properties.at(key);
> >> +				py::object ob = controlValueToPy(cv);
> >> +
> >> +				ret[id->name().c_str()] = ob;
> >> +			}
> >> +
> >> +			return ret;
> >> +		});
> >> +
> >> +	pyCameraConfiguration
> >> +		.def("__iter__", [](CameraConfiguration &self) {
> >> +			return py::make_iterator<py::return_value_policy::reference_internal>(self);
> >> +		}, py::keep_alive<0, 1>())
> >> +		.def("__len__", [](CameraConfiguration &self) {
> >> +			return self.size();
> >> +		})
> >> +		.def("validate", &CameraConfiguration::validate)
> >> +		.def("at", py::overload_cast<unsigned int>(&CameraConfiguration::at),
> >> +		     py::return_value_policy::reference_internal)
> >> +		.def_property_readonly("size", &CameraConfiguration::size)
> >> +		.def_property_readonly("empty", &CameraConfiguration::empty)
> >> +		.def_readwrite("transform", &CameraConfiguration::transform);
> >> +
> >> +	pyCameraConfigurationStatus
> >> +		.value("Valid", CameraConfiguration::Valid)
> >> +		.value("Adjusted", CameraConfiguration::Adjusted)
> >> +		.value("Invalid", CameraConfiguration::Invalid);
> >> +
> >> +	pyStreamConfiguration
> >> +		.def("to_string", &StreamConfiguration::toString)
> > 
> > Should this be __str__ ?
> 
> Yes. And we seem to have a few __repr__, which should be __str__. I'll 
> change those too.

I see three __repr__ implementations. I'm not familiar enough with the
difference between __str__ and __repr__ to tell which one would be best
here.

> >> +		.def_property_readonly("stream", &StreamConfiguration::stream,
> >> +		                       py::return_value_policy::reference_internal)
> >> +		.def_property(
> >> +			"size",
> >> +			[](StreamConfiguration &self) {
> >> +				return std::make_tuple(self.size.width, self.size.height);
> >> +			},
> >> +			[](StreamConfiguration &self, std::tuple<uint32_t, uint32_t> size) {
> >> +				self.size.width = std::get<0>(size);
> >> +				self.size.height = std::get<1>(size);
> >> +			})
> >> +		.def_property(
> >> +			"pixel_format",
> >> +			[](StreamConfiguration &self) {
> >> +				return self.pixelFormat.toString();
> >> +			},
> >> +			[](StreamConfiguration &self, std::string fmt) {
> >> +				self.pixelFormat = PixelFormat::fromString(fmt);
> >> +			})
> >> +		.def_readwrite("stride", &StreamConfiguration::stride)
> >> +		.def_readwrite("frame_size", &StreamConfiguration::frameSize)
> >> +		.def_readwrite("buffer_count", &StreamConfiguration::bufferCount)
> >> +		.def_property_readonly("formats", &StreamConfiguration::formats,
> >> +		                       py::return_value_policy::reference_internal)
> >> +		.def_readwrite("colorSpace", &StreamConfiguration::colorSpace);
> > 
> > color_space
> 
> Ok.
> 
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>