[libcamera-devel] py: Add the SensorConfiguration class
diff mbox series

Message ID 20230928094526.21999-1-david.plowman@raspberrypi.com
State Accepted
Headers show
Series
  • [libcamera-devel] py: Add the SensorConfiguration class
Related show

Commit Message

David Plowman Sept. 28, 2023, 9:45 a.m. UTC 
We provide access to the bitDepth and outputSize fields of the new
SensorConfiguration class. The class also needs a constructor so that
Python applications can make one and put it into the
CameraConfiguration.

Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
---
 src/py/libcamera/py_main.cpp | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Naushir Patuck Sept. 28, 2023, 9:52 a.m. UTC | #1 
Hi David,

Thank you for your patch.

On Thu, 28 Sept 2023 at 10:45, David Plowman via libcamera-devel
<libcamera-devel@lists.libcamera.org> wrote:
>
> We provide access to the bitDepth and outputSize fields of the new
> SensorConfiguration class. The class also needs a constructor so that
> Python applications can make one and put it into the
> CameraConfiguration.
>
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>

Reviewed-by: Naushir Patuck <naush@raspberrypi.com>

> ---
>  src/py/libcamera/py_main.cpp | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
> index 01fb15a9..8df04520 100644
> --- a/src/py/libcamera/py_main.cpp
> +++ b/src/py/libcamera/py_main.cpp
> @@ -112,6 +112,7 @@ PYBIND11_MODULE(_libcamera, m)
>
>         auto pyCameraManager = py::class_<PyCameraManager, std::shared_ptr<PyCameraManager>>(m, "CameraManager");
>         auto pyCamera = py::class_<Camera, PyCameraSmartPtr<Camera>>(m, "Camera");
> +       auto pySensorConfiguration = py::class_<SensorConfiguration>(m, "SensorConfiguration");
>         auto pyCameraConfiguration = py::class_<CameraConfiguration>(m, "CameraConfiguration");
>         auto pyCameraConfigurationStatus = py::enum_<CameraConfiguration::Status>(pyCameraConfiguration, "Status");
>         auto pyStreamConfiguration = py::class_<StreamConfiguration>(m, "StreamConfiguration");
> @@ -281,6 +282,11 @@ PYBIND11_MODULE(_libcamera, m)
>                         return ret;
>                 });
>
> +       pySensorConfiguration
> +               .def(py::init<>())
> +               .def_readwrite("bit_depth", &SensorConfiguration::bitDepth)
> +               .def_readwrite("output_size", &SensorConfiguration::outputSize);
> +
>         pyCameraConfiguration
>                 .def("__iter__", [](CameraConfiguration &self) {
>                         return py::make_iterator<py::return_value_policy::reference_internal>(self);
> @@ -293,6 +299,7 @@ PYBIND11_MODULE(_libcamera, m)
>                      py::return_value_policy::reference_internal)
>                 .def_property_readonly("size", &CameraConfiguration::size)
>                 .def_property_readonly("empty", &CameraConfiguration::empty)
> +               .def_readwrite("sensor_config", &CameraConfiguration::sensorConfig)
>                 .def_readwrite("transform", &CameraConfiguration::transform);
>
>         pyCameraConfigurationStatus
> --
> 2.39.2
>
Tomi Valkeinen Oct. 5, 2023, 10:03 a.m. UTC | #2 
On 28/09/2023 12:45, David Plowman via libcamera-devel wrote:
> We provide access to the bitDepth and outputSize fields of the new
> SensorConfiguration class. The class also needs a constructor so that
> Python applications can make one and put it into the
> CameraConfiguration.
> 
> Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> ---
>   src/py/libcamera/py_main.cpp | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
> index 01fb15a9..8df04520 100644
> --- a/src/py/libcamera/py_main.cpp
> +++ b/src/py/libcamera/py_main.cpp
> @@ -112,6 +112,7 @@ PYBIND11_MODULE(_libcamera, m)
>   
>   	auto pyCameraManager = py::class_<PyCameraManager, std::shared_ptr<PyCameraManager>>(m, "CameraManager");
>   	auto pyCamera = py::class_<Camera, PyCameraSmartPtr<Camera>>(m, "Camera");
> +	auto pySensorConfiguration = py::class_<SensorConfiguration>(m, "SensorConfiguration");
>   	auto pyCameraConfiguration = py::class_<CameraConfiguration>(m, "CameraConfiguration");
>   	auto pyCameraConfigurationStatus = py::enum_<CameraConfiguration::Status>(pyCameraConfiguration, "Status");
>   	auto pyStreamConfiguration = py::class_<StreamConfiguration>(m, "StreamConfiguration");
> @@ -281,6 +282,11 @@ PYBIND11_MODULE(_libcamera, m)
>   			return ret;
>   		});
>   
> +	pySensorConfiguration
> +		.def(py::init<>())
> +		.def_readwrite("bit_depth", &SensorConfiguration::bitDepth)
> +		.def_readwrite("output_size", &SensorConfiguration::outputSize);
> +

Any reason not to expose all the fields in the class? In any case, looks 
fine to me:

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


>   	pyCameraConfiguration
>   		.def("__iter__", [](CameraConfiguration &self) {
>   			return py::make_iterator<py::return_value_policy::reference_internal>(self);
> @@ -293,6 +299,7 @@ PYBIND11_MODULE(_libcamera, m)
>   		     py::return_value_policy::reference_internal)
>   		.def_property_readonly("size", &CameraConfiguration::size)
>   		.def_property_readonly("empty", &CameraConfiguration::empty)
> +		.def_readwrite("sensor_config", &CameraConfiguration::sensorConfig)
>   		.def_readwrite("transform", &CameraConfiguration::transform);
>   
>   	pyCameraConfigurationStatus
David Plowman Oct. 11, 2023, 9:37 a.m. UTC | #3 
Hi Tomi

On Thu, 5 Oct 2023 at 11:03, Tomi Valkeinen
<tomi.valkeinen@ideasonboard.com> wrote:
>
> On 28/09/2023 12:45, David Plowman via libcamera-devel wrote:
> > We provide access to the bitDepth and outputSize fields of the new
> > SensorConfiguration class. The class also needs a constructor so that
> > Python applications can make one and put it into the
> > CameraConfiguration.
> >
> > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > ---
> >   src/py/libcamera/py_main.cpp | 7 +++++++
> >   1 file changed, 7 insertions(+)
> >
> > diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
> > index 01fb15a9..8df04520 100644
> > --- a/src/py/libcamera/py_main.cpp
> > +++ b/src/py/libcamera/py_main.cpp
> > @@ -112,6 +112,7 @@ PYBIND11_MODULE(_libcamera, m)
> >
> >       auto pyCameraManager = py::class_<PyCameraManager, std::shared_ptr<PyCameraManager>>(m, "CameraManager");
> >       auto pyCamera = py::class_<Camera, PyCameraSmartPtr<Camera>>(m, "Camera");
> > +     auto pySensorConfiguration = py::class_<SensorConfiguration>(m, "SensorConfiguration");
> >       auto pyCameraConfiguration = py::class_<CameraConfiguration>(m, "CameraConfiguration");
> >       auto pyCameraConfigurationStatus = py::enum_<CameraConfiguration::Status>(pyCameraConfiguration, "Status");
> >       auto pyStreamConfiguration = py::class_<StreamConfiguration>(m, "StreamConfiguration");
> > @@ -281,6 +282,11 @@ PYBIND11_MODULE(_libcamera, m)
> >                       return ret;
> >               });
> >
> > +     pySensorConfiguration
> > +             .def(py::init<>())
> > +             .def_readwrite("bit_depth", &SensorConfiguration::bitDepth)
> > +             .def_readwrite("output_size", &SensorConfiguration::outputSize);
> > +
>
> Any reason not to expose all the fields in the class? In any case, looks
> fine to me:

Yes, that's a fair point. I guess the only reason was that I don't use
those other fields, in large part because there's currently no way to
discover what values you'd put into them, so you'd struggle to do
anything meaningful with them!

But I could add simple accessors to these other bits of data where
that's straightforward. Do we think that would be useful?

Thanks!
David

>
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>
>
> >       pyCameraConfiguration
> >               .def("__iter__", [](CameraConfiguration &self) {
> >                       return py::make_iterator<py::return_value_policy::reference_internal>(self);
> > @@ -293,6 +299,7 @@ PYBIND11_MODULE(_libcamera, m)
> >                    py::return_value_policy::reference_internal)
> >               .def_property_readonly("size", &CameraConfiguration::size)
> >               .def_property_readonly("empty", &CameraConfiguration::empty)
> > +             .def_readwrite("sensor_config", &CameraConfiguration::sensorConfig)
> >               .def_readwrite("transform", &CameraConfiguration::transform);
> >
> >       pyCameraConfigurationStatus
>
Kieran Bingham Oct. 11, 2023, 10:28 a.m. UTC | #4 
Quoting David Plowman via libcamera-devel (2023-10-11 10:37:49)
> Hi Tomi
> 
> On Thu, 5 Oct 2023 at 11:03, Tomi Valkeinen
> <tomi.valkeinen@ideasonboard.com> wrote:
> >
> > On 28/09/2023 12:45, David Plowman via libcamera-devel wrote:
> > > We provide access to the bitDepth and outputSize fields of the new
> > > SensorConfiguration class. The class also needs a constructor so that
> > > Python applications can make one and put it into the
> > > CameraConfiguration.
> > >
> > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > > ---
> > >   src/py/libcamera/py_main.cpp | 7 +++++++
> > >   1 file changed, 7 insertions(+)
> > >
> > > diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
> > > index 01fb15a9..8df04520 100644
> > > --- a/src/py/libcamera/py_main.cpp
> > > +++ b/src/py/libcamera/py_main.cpp
> > > @@ -112,6 +112,7 @@ PYBIND11_MODULE(_libcamera, m)
> > >
> > >       auto pyCameraManager = py::class_<PyCameraManager, std::shared_ptr<PyCameraManager>>(m, "CameraManager");
> > >       auto pyCamera = py::class_<Camera, PyCameraSmartPtr<Camera>>(m, "Camera");
> > > +     auto pySensorConfiguration = py::class_<SensorConfiguration>(m, "SensorConfiguration");
> > >       auto pyCameraConfiguration = py::class_<CameraConfiguration>(m, "CameraConfiguration");
> > >       auto pyCameraConfigurationStatus = py::enum_<CameraConfiguration::Status>(pyCameraConfiguration, "Status");
> > >       auto pyStreamConfiguration = py::class_<StreamConfiguration>(m, "StreamConfiguration");
> > > @@ -281,6 +282,11 @@ PYBIND11_MODULE(_libcamera, m)
> > >                       return ret;
> > >               });
> > >
> > > +     pySensorConfiguration
> > > +             .def(py::init<>())
> > > +             .def_readwrite("bit_depth", &SensorConfiguration::bitDepth)
> > > +             .def_readwrite("output_size", &SensorConfiguration::outputSize);
> > > +
> >
> > Any reason not to expose all the fields in the class? In any case, looks
> > fine to me:
> 
> Yes, that's a fair point. I guess the only reason was that I don't use
> those other fields, in large part because there's currently no way to
> discover what values you'd put into them, so you'd struggle to do
> anything meaningful with them!
> 
> But I could add simple accessors to these other bits of data where
> that's straightforward. Do we think that would be useful?

I would say yes please. Otherwise this just counts as technical debt
that will get left behind.

--
Kieran


> 
> Thanks!
> David
> 
> >
> > Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> >
> >
> > >       pyCameraConfiguration
> > >               .def("__iter__", [](CameraConfiguration &self) {
> > >                       return py::make_iterator<py::return_value_policy::reference_internal>(self);
> > > @@ -293,6 +299,7 @@ PYBIND11_MODULE(_libcamera, m)
> > >                    py::return_value_policy::reference_internal)
> > >               .def_property_readonly("size", &CameraConfiguration::size)
> > >               .def_property_readonly("empty", &CameraConfiguration::empty)
> > > +             .def_readwrite("sensor_config", &CameraConfiguration::sensorConfig)
> > >               .def_readwrite("transform", &CameraConfiguration::transform);
> > >
> > >       pyCameraConfigurationStatus
> >
David Plowman Oct. 11, 2023, 3:39 p.m. UTC | #5 
Will do!

I've submitted a v2 that adds the extra bits. Not sure what to do with
those anonymous structures, so I've made Python turn them into tuples.

David

On Wed, 11 Oct 2023 at 11:29, Kieran Bingham
<kieran.bingham@ideasonboard.com> wrote:
>
> Quoting David Plowman via libcamera-devel (2023-10-11 10:37:49)
> > Hi Tomi
> >
> > On Thu, 5 Oct 2023 at 11:03, Tomi Valkeinen
> > <tomi.valkeinen@ideasonboard.com> wrote:
> > >
> > > On 28/09/2023 12:45, David Plowman via libcamera-devel wrote:
> > > > We provide access to the bitDepth and outputSize fields of the new
> > > > SensorConfiguration class. The class also needs a constructor so that
> > > > Python applications can make one and put it into the
> > > > CameraConfiguration.
> > > >
> > > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
> > > > ---
> > > >   src/py/libcamera/py_main.cpp | 7 +++++++
> > > >   1 file changed, 7 insertions(+)
> > > >
> > > > diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
> > > > index 01fb15a9..8df04520 100644
> > > > --- a/src/py/libcamera/py_main.cpp
> > > > +++ b/src/py/libcamera/py_main.cpp
> > > > @@ -112,6 +112,7 @@ PYBIND11_MODULE(_libcamera, m)
> > > >
> > > >       auto pyCameraManager = py::class_<PyCameraManager, std::shared_ptr<PyCameraManager>>(m, "CameraManager");
> > > >       auto pyCamera = py::class_<Camera, PyCameraSmartPtr<Camera>>(m, "Camera");
> > > > +     auto pySensorConfiguration = py::class_<SensorConfiguration>(m, "SensorConfiguration");
> > > >       auto pyCameraConfiguration = py::class_<CameraConfiguration>(m, "CameraConfiguration");
> > > >       auto pyCameraConfigurationStatus = py::enum_<CameraConfiguration::Status>(pyCameraConfiguration, "Status");
> > > >       auto pyStreamConfiguration = py::class_<StreamConfiguration>(m, "StreamConfiguration");
> > > > @@ -281,6 +282,11 @@ PYBIND11_MODULE(_libcamera, m)
> > > >                       return ret;
> > > >               });
> > > >
> > > > +     pySensorConfiguration
> > > > +             .def(py::init<>())
> > > > +             .def_readwrite("bit_depth", &SensorConfiguration::bitDepth)
> > > > +             .def_readwrite("output_size", &SensorConfiguration::outputSize);
> > > > +
> > >
> > > Any reason not to expose all the fields in the class? In any case, looks
> > > fine to me:
> >
> > Yes, that's a fair point. I guess the only reason was that I don't use
> > those other fields, in large part because there's currently no way to
> > discover what values you'd put into them, so you'd struggle to do
> > anything meaningful with them!
> >
> > But I could add simple accessors to these other bits of data where
> > that's straightforward. Do we think that would be useful?
>
> I would say yes please. Otherwise this just counts as technical debt
> that will get left behind.
>
> --
> Kieran
>
>
> >
> > Thanks!
> > David
> >
> > >
> > > Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> > >
> > >
> > > >       pyCameraConfiguration
> > > >               .def("__iter__", [](CameraConfiguration &self) {
> > > >                       return py::make_iterator<py::return_value_policy::reference_internal>(self);
> > > > @@ -293,6 +299,7 @@ PYBIND11_MODULE(_libcamera, m)
> > > >                    py::return_value_policy::reference_internal)
> > > >               .def_property_readonly("size", &CameraConfiguration::size)
> > > >               .def_property_readonly("empty", &CameraConfiguration::empty)
> > > > +             .def_readwrite("sensor_config", &CameraConfiguration::sensorConfig)
> > > >               .def_readwrite("transform", &CameraConfiguration::transform);
> > > >
> > > >       pyCameraConfigurationStatus
> > >

Patch
diff mbox series 

diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
index 01fb15a9..8df04520 100644
--- a/src/py/libcamera/py_main.cpp
+++ b/src/py/libcamera/py_main.cpp
@@ -112,6 +112,7 @@  PYBIND11_MODULE(_libcamera, m)
 
 	auto pyCameraManager = py::class_<PyCameraManager, std::shared_ptr<PyCameraManager>>(m, "CameraManager");
 	auto pyCamera = py::class_<Camera, PyCameraSmartPtr<Camera>>(m, "Camera");
+	auto pySensorConfiguration = py::class_<SensorConfiguration>(m, "SensorConfiguration");
 	auto pyCameraConfiguration = py::class_<CameraConfiguration>(m, "CameraConfiguration");
 	auto pyCameraConfigurationStatus = py::enum_<CameraConfiguration::Status>(pyCameraConfiguration, "Status");
 	auto pyStreamConfiguration = py::class_<StreamConfiguration>(m, "StreamConfiguration");
@@ -281,6 +282,11 @@  PYBIND11_MODULE(_libcamera, m)
 			return ret;
 		});
 
+	pySensorConfiguration
+		.def(py::init<>())
+		.def_readwrite("bit_depth", &SensorConfiguration::bitDepth)
+		.def_readwrite("output_size", &SensorConfiguration::outputSize);
+
 	pyCameraConfiguration
 		.def("__iter__", [](CameraConfiguration &self) {
 			return py::make_iterator<py::return_value_policy::reference_internal>(self);
@@ -293,6 +299,7 @@  PYBIND11_MODULE(_libcamera, m)
 		     py::return_value_policy::reference_internal)
 		.def_property_readonly("size", &CameraConfiguration::size)
 		.def_property_readonly("empty", &CameraConfiguration::empty)
+		.def_readwrite("sensor_config", &CameraConfiguration::sensorConfig)
 		.def_readwrite("transform", &CameraConfiguration::transform);
 
 	pyCameraConfigurationStatus