[libcamera-devel] [PATCH v7 09/13] py: add Transform

[Laurent Pinchart]

Hi Tomi,

On Fri, May 06, 2022 at 02:25:50PM +0300, Tomi Valkeinen wrote:
> On 05/05/2022 20:59, Laurent Pinchart wrote:
> > On Thu, May 05, 2022 at 01:41:00PM +0300, Tomi Valkeinen wrote:
> >> Add binding for Transform.
> >>
> >> C++'s Transform is an enum class, but we expose it as a Python class so
> >> that it can be easily manipulated.
> >>
> >> Original version from David Plowman <david.plowman at raspberrypi.com>.
> >>
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
> >> ---
> >>   src/py/libcamera/pymain.cpp | 63 ++++++++++++++++++++++++++++++++++++-
> >>   1 file changed, 62 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/src/py/libcamera/pymain.cpp b/src/py/libcamera/pymain.cpp
> >> index db9d90ab..3d2393ab 100644
> >> --- a/src/py/libcamera/pymain.cpp
> >> +++ b/src/py/libcamera/pymain.cpp
> >> @@ -152,6 +152,7 @@ PYBIND11_MODULE(_libcamera, m)
> >>   	auto pyControlId = py::class_<ControlId>(m, "ControlId");
> >>   	auto pyRequest = py::class_<Request>(m, "Request");
> >>   	auto pyFrameMetadata = py::class_<FrameMetadata>(m, "FrameMetadata");
> >> +	auto pyTransform = py::class_<Transform>(m, "Transform");
> >>   
> >>   	/* Global functions */
> >>   	m.def("logSetLevel", &logSetLevel);
> >> @@ -342,7 +343,8 @@ PYBIND11_MODULE(_libcamera, m)
> >>   		.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_property_readonly("empty", &CameraConfiguration::empty)
> >> +		.def_readwrite("transform", &CameraConfiguration::transform);
> >>   
> >>   	pyStreamConfiguration
> >>   		.def("toString", &StreamConfiguration::toString)
> >> @@ -462,4 +464,63 @@ PYBIND11_MODULE(_libcamera, m)
> >>   			transform(self.planes().begin(), self.planes().end(), v.begin(), [](const auto &p) { return p.bytesused; });
> >>   			return v;
> >>   		});
> >> +
> >> +	pyTransform
> >> +		.def(py::init([](int rotation, bool hflip, bool vflip, bool transpose) {
> > 
> > I'm not fond of the constructor, three bool arguments in a row are quite
> > error-prone. I'd rather expose the enum.
> 
> It's not that bad with python, e.g.:
> 
> Transform(transpose=True, vflip=True)
> 
> I think that's more pythonic than or'ing enums.

One could also write

    Transform(0, true, false, true)

which isn't nice. We can address that out later if needed, either
merging this patch already or postponing it. David, do you have any
opinion ?

> >> +			bool ok;
> >> +
> >> +			Transform t = transformFromRotation(rotation, &ok);
> >> +			if (!ok)
> >> +				throw std::runtime_error("Unable to create the transform");
> > 
> > "Invalid rotation" would be a better message I think.
> 
> And std::invalid_argument.

Good point.

-- 
Regards,

Laurent Pinchart