[libcamera-devel] [PATCH 11/14] py: add geometry classes

Tomi Valkeinen tomi.valkeinen at ideasonboard.com
Tue May 17 11:02:13 CEST 2022


On 17/05/2022 11:37, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Mon, May 16, 2022 at 05:10:19PM +0300, Tomi Valkeinen wrote:
>> Add libcamera's geometry classes to the Python bindings.
>>
>> Note that this commit only adds the classes, but they are not used
>> anywhere yet.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
>> ---
>>   src/py/libcamera/meson.build    |   1 +
>>   src/py/libcamera/pygeometry.cpp | 104 ++++++++++++++++++++++++++++++++
>>   src/py/libcamera/pymain.cpp     |   2 +
>>   3 files changed, 107 insertions(+)
>>   create mode 100644 src/py/libcamera/pygeometry.cpp
>>
>> diff --git a/src/py/libcamera/meson.build b/src/py/libcamera/meson.build
>> index af8ba6a5..12e006e7 100644
>> --- a/src/py/libcamera/meson.build
>> +++ b/src/py/libcamera/meson.build
>> @@ -14,6 +14,7 @@ pybind11_dep = pybind11_proj.get_variable('pybind11_dep')
>>   
>>   pycamera_sources = files([
>>       'pyenums.cpp',
>> +    'pygeometry.cpp',
>>       'pymain.cpp',
>>   ])
>>   
>> diff --git a/src/py/libcamera/pygeometry.cpp b/src/py/libcamera/pygeometry.cpp
>> new file mode 100644
>> index 00000000..2cd1432e
>> --- /dev/null
>> +++ b/src/py/libcamera/pygeometry.cpp
>> @@ -0,0 +1,104 @@
>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>> +/*
>> + * Copyright (C) 2022, Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
>> + *
>> + * Python bindings - Geometry classes
>> + */
>> +
>> +#include <array>
>> +
>> +#include <libcamera/geometry.h>
>> +#include <libcamera/libcamera.h>
>> +
>> +#include <pybind11/operators.h>
>> +#include <pybind11/smart_holder.h>
>> +#include <pybind11/stl.h>
>> +
>> +namespace py = pybind11;
>> +
>> +using namespace libcamera;
>> +
>> +void init_py_geometry(py::module &m)
>> +{
>> +	py::class_<Point>(m, "Point")
>> +		.def(py::init<>())
>> +		.def(py::init<int, int>())
>> +		.def_readwrite("x", &Point::x)
>> +		.def_readwrite("y", &Point::y)
>> +		.def(py::self == py::self)
>> +		.def(-py::self)
>> +		.def("__str__", &Point::toString)
>> +		.def("__repr__", [](const Point &self) {
>> +			return "libcamera.Point(" + std::to_string(self.x) + ", " + std::to_string(self.y) + ")";
>> +		});
>> +
>> +	py::class_<Size>(m, "Size")
>> +		.def(py::init<>())
>> +		.def(py::init<unsigned int, unsigned int>())
>> +		.def_readwrite("width", &Size::width)
>> +		.def_readwrite("height", &Size::height)
>> +		.def_property_readonly("is_null", &Size::isNull)
>> +		.def(py::self == py::self)
>> +		.def(py::self * float())
>> +		.def(py::self / float())
>> +		.def(py::self *= float())
>> +		.def(py::self /= float())
>> +		.def("__str__", &Size::toString)
>> +		.def("__repr__", [](const Size &self) {
>> +			return "libcamera.Size(" + std::to_string(self.width) + ", " + std::to_string(self.height) + ")";
>> +		});
>> +
>> +	py::class_<SizeRange>(m, "SizeRange")
>> +		.def(py::init<>())
>> +		.def(py::init<Size>())
>> +		.def(py::init<Size, Size>())
>> +		.def(py::init<Size, Size, unsigned int, unsigned int>())
>> +		.def(py::init<>([](const std::array<unsigned int, 2> &s) {
>> +			return SizeRange(Size(std::get<0>(s), std::get<1>(s)));
>> +		}))
>> +		.def(py::init<>([](const std::array<unsigned int, 2> &min, const std::array<unsigned int, 2> &max) {
>> +			return SizeRange(Size(std::get<0>(min), std::get<1>(min)),
>> +					 Size(std::get<0>(max), std::get<1>(max)));
>> +		}))
>> +		.def(py::init<>([](const std::array<unsigned int, 2> &min, const std::array<unsigned int, 2> &max,
>> +				   unsigned int hStep, unsigned int vStep) {
>> +			return SizeRange(Size(std::get<0>(min), std::get<1>(min)),
>> +					 Size(std::get<0>(max), std::get<1>(max)),
>> +					 hStep, vStep);
>> +		}))
>> +		.def_readwrite("min", &SizeRange::min)
>> +		.def_readwrite("max", &SizeRange::max)
>> +		.def_readwrite("hStep", &SizeRange::hStep)
>> +		.def_readwrite("vStep", &SizeRange::vStep)
>> +		.def(py::self == py::self)
>> +		.def("__str__", &SizeRange::toString)
>> +		.def("__repr__", [](const SizeRange &self) {
>> +			return py::str("libcamera.SizeRange(({}, {}), ({}, {}), {}, {})")
>> +				.format(self.min.width, self.min.height,
>> +					self.max.width, self.max.height,
>> +					self.hStep, self.vStep);
>> +		});
>> +
>> +	py::class_<Rectangle>(m, "Rectangle")
>> +		.def(py::init<>())
>> +		.def(py::init<int, int, Size>())
>> +		.def(py::init<>([](int x, int y, const std::array<unsigned int, 2> &s) {
>> +			return Rectangle(x, y, std::get<0>(s), std::get<1>(s));
>> +		}))
> 
> I'm puzzled a bit by this constructor. Where do you use it, and could
> then next constructor be used instead ? If it's meant to cover a common
> use case, should the C++ API also implement this ?

It allows:

libcam.Rectangle(1, 2, (3, 4))

It's more obvious with SizeRange:

libcam.SizeRange((1, 2), (3, 4), 5, 6)

instead of

libcam.SizeRange(libcam.Size(1, 2), libcam.Size(3, 4), 5, 6)

I really like tuples in python. I'd also like to do unpacking, like:

w, h = libcam.Size(1, 2)

I think I figured out how to do that, but after sending the series.

These kind of features are perhaps something we need to do a decision 
on: include them, or keep the API as bare-bones as possible.

>> +		.def(py::init<int, int, unsigned int, unsigned int>())
>> +		.def(py::init<Size>())
>> +		.def_readwrite("x", &Rectangle::x)
>> +		.def_readwrite("y", &Rectangle::y)
>> +		.def_readwrite("width", &Rectangle::width)
>> +		.def_readwrite("height", &Rectangle::height)
>> +		.def_property_readonly("is_null", &Rectangle::isNull)
>> +		.def_property_readonly("center", &Rectangle::center)
>> +		.def_property_readonly("size", &Rectangle::size)
>> +		.def_property_readonly("topLeft", &Rectangle::topLeft)
>> +		.def(py::self == py::self)
>> +		.def("__str__", &Rectangle::toString)
>> +		.def("__repr__", [](const Rectangle &self) {
>> +			return py::str("libcamera.Rectangle({}, {}, {}, {})")
>> +				.format(self.x, self.y, self.width, self.height);
>> +		});
> 
> Could you add todo comments to remind that the other members of the
> geometry classes should be implemented too ?

Yes.

>> +}
>> diff --git a/src/py/libcamera/pymain.cpp b/src/py/libcamera/pymain.cpp
>> index cc2ddee5..1dd70d9c 100644
>> --- a/src/py/libcamera/pymain.cpp
>> +++ b/src/py/libcamera/pymain.cpp
>> @@ -137,11 +137,13 @@ static void handleRequestCompleted(Request *req)
>>   
>>   void init_pyenums(py::module &m);
>>   void init_pyenums_generated(py::module &m);
>> +void init_py_geometry(py::module &m);
>>   
>>   PYBIND11_MODULE(_libcamera, m)
>>   {
>>   	init_pyenums(m);
>>   	init_pyenums_generated(m);
>> +	init_py_geometry(m);
> 
> My OCD kicks in :-)

Now that I look at it... Yes...

  Tomi


More information about the libcamera-devel mailing list