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

[Tomi Valkeinen]

On 05/05/2022 20:59, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> 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.

>> +			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.

  Tomi