| Message ID | 20220524114610.41848-17-tomi.valkeinen@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Tomi, Thank you for the patch. On Tue, May 24, 2022 at 02:46:07PM +0300, Tomi Valkeinen wrote: > Add ControlInfo class and change the controls related methods to > resemble the C++ API (e.g. no more string based control methods). > > We don't implement ControlList or ControlInfoMap but just expose the > same data via standard Python dict. > > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> > --- > src/py/cam/cam.py | 8 +-- > src/py/cam/cam_qt.py | 9 ++-- > src/py/libcamera/py_main.cpp | 100 ++++++++++++++++++----------------- > 3 files changed, 61 insertions(+), 56 deletions(-) > > diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py > index f6e8232c..f464bd01 100755 > --- a/src/py/cam/cam.py > +++ b/src/py/cam/cam.py > @@ -48,16 +48,16 @@ class CameraContext: > > print('Properties for', self.id) > > - for name, prop in camera.properties.items(): > - print('\t{}: {}'.format(name, prop)) > + for cid, val in camera.properties.items(): > + print('\t{}: {}'.format(cid, val)) > > def do_cmd_list_controls(self): > camera = self.camera > > print('Controls for', self.id) > > - for name, prop in camera.controls.items(): > - print('\t{}: {}'.format(name, prop)) > + for cid, info in camera.controls.items(): > + print('\t{}: {}'.format(cid, info)) > > def do_cmd_info(self): > camera = self.camera > diff --git a/src/py/cam/cam_qt.py b/src/py/cam/cam_qt.py > index d638e9cc..739e9749 100644 > --- a/src/py/cam/cam_qt.py > +++ b/src/py/cam/cam_qt.py > @@ -267,9 +267,9 @@ class MainWindow(QtWidgets.QWidget): > > camera = ctx.camera > > - for k, v in camera.properties.items(): > + for cid, cv in camera.properties.items(): > lab = QtWidgets.QLabel() > - lab.setText(k + ' = ' + str(v)) > + lab.setText("{} = {}".format(cid, cv)) s/"/'/g > groupLayout.addWidget(lab) > > group = QtWidgets.QGroupBox('Controls') > @@ -277,9 +277,10 @@ class MainWindow(QtWidgets.QWidget): > group.setLayout(groupLayout) > controlsLayout.addWidget(group) > > - for k, (min, max, default) in camera.controls.items(): > + for cid, cinfo in camera.controls.items(): > lab = QtWidgets.QLabel() > - lab.setText('{} = {}/{}/{}'.format(k, min, max, default)) > + lab.setText('{} = {}/{}/{}' > + .format(cid, cinfo.min, cinfo.max, cinfo.default)) > groupLayout.addWidget(lab) > > controlsLayout.addStretch() > diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp > index 33ecc1cd..80f26c12 100644 > --- a/src/py/libcamera/py_main.cpp > +++ b/src/py/libcamera/py_main.cpp > @@ -159,6 +159,7 @@ PYBIND11_MODULE(_libcamera, m) > auto pyFrameBuffer = py::class_<FrameBuffer>(m, "FrameBuffer"); > auto pyStream = py::class_<Stream>(m, "Stream"); > auto pyControlId = py::class_<ControlId>(m, "ControlId"); > + auto pyControlInfo = py::class_<ControlInfo>(m, "ControlInfo"); > auto pyRequest = py::class_<Request>(m, "Request"); > auto pyRequestStatus = py::enum_<Request::Status>(pyRequest, "Status"); > auto pyRequestReuse = py::enum_<Request::ReuseFlag>(pyRequest, "Reuse"); > @@ -260,28 +261,17 @@ PYBIND11_MODULE(_libcamera, m) > .def_property_readonly("id", &Camera::id) > .def("acquire", &Camera::acquire) > .def("release", &Camera::release) > - .def("start", [](Camera &self, py::dict controls) { > + .def("start", [](Camera &self, > + const std::unordered_map<const ControlId *, py::object> &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>(); > + ControlList controlList(self.controls()); > > - 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())); > + for (const auto& [id, obj]: controls) { > + auto val = pyToControlValue(obj, id->type()); > + controlList.set(id->id(), val); > } > > int ret = self.start(&controlList); > @@ -291,7 +281,7 @@ PYBIND11_MODULE(_libcamera, m) > } > > return 0; > - }, py::arg("controls") = py::dict()) > + }, py::arg("controls") = std::unordered_map<const ControlId *, py::object>()) > > .def("stop", [](Camera &self) { > int ret = self.stop(); > @@ -341,40 +331,26 @@ PYBIND11_MODULE(_libcamera, m) > return set; > }) > > - .def("find_control", [](Camera &self, const std::string &name) { > - const auto &controls = self.controls(); > - > - auto it = std::find_if(controls.begin(), controls.end(), > - [&name](const auto &kvp) { > - return kvp.first->name() == name; > - }); > - > - if (it == controls.end()) > - throw std::runtime_error("Control '" + name + "' not found"); > - > - return it->first; > - }, py::return_value_policy::reference_internal) > - > .def_property_readonly("controls", [](Camera &self) { > - py::dict ret; > + /* Convert ControlInfoMap to std container */ > > - 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())); > - } > + std::unordered_map<const ControlId *, ControlInfo> ret; > + > + for (const auto &[k, cv] : self.controls()) > + ret[k] = cv; > > return ret; > }) > > .def_property_readonly("properties", [](Camera &self) { > - py::dict ret; > + /* Convert ControlList to std container */ > > - for (const auto &[key, cv] : self.properties()) { > - const ControlId *id = properties::properties.at(key); > - py::object ob = controlValueToPy(cv); > + std::unordered_map<const ControlId *, py::object> ret; > > - ret[id->name().c_str()] = ob; > + for (const auto &[k, cv] : self.properties()) { > + const ControlId *id = properties::properties.at(k); > + py::object ob = controlValueToPy(cv); > + ret[id] = ob; > } > > return ret; > @@ -464,7 +440,34 @@ PYBIND11_MODULE(_libcamera, m) > pyControlId > .def_property_readonly("id", &ControlId::id) > .def_property_readonly("name", &ControlId::name) > - .def_property_readonly("type", &ControlId::type); > + .def_property_readonly("type", &ControlId::type) > + .def("__str__", [](const ControlId &self) { return self.name(); }) > + .def("__repr__", [](const ControlId &self) { > + return py::str("libcamera.ControlId({}, {}, {})") > + .format(self.id(), self.name(), self.type()); Should this be py::str("libcamera.controls.{}").format(self.name()) ? That may not work for draft controls though. > + }); > + > + pyControlInfo > + .def_property_readonly("min", [](const ControlInfo& self) { s/& self/ &self/ Same below. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + return controlValueToPy(self.min()); > + }) > + .def_property_readonly("max", [](const ControlInfo& self) { > + return controlValueToPy(self.max()); > + }) > + .def_property_readonly("default", [](const ControlInfo& self) { > + return controlValueToPy(self.def()); > + }) > + .def_property_readonly("values", [](const ControlInfo& self) { > + py::list l; > + for (const auto &v : self.values()) > + l.append(controlValueToPy(v)); > + return l; > + }) > + .def("__str__", &ControlInfo::toString) > + .def("__repr__", [](const ControlInfo& self) { > + return py::str("libcamera.ControlInfo({})") > + .format(self.toString()); > + }); > > pyRequest > /* \todo Fence is not supported, so we cannot expose addBuffer() directly */ > @@ -475,17 +478,18 @@ PYBIND11_MODULE(_libcamera, m) > .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) { > + .def("set_control", [](Request &self, const ControlId &id, py::object value) { > self.controls().set(id.id(), pyToControlValue(value, id.type())); > }) > .def_property_readonly("metadata", [](Request &self) { > - py::dict ret; > + /* Convert ControlList to std container */ > + > + std::unordered_map<const ControlId *, py::object> 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; > + ret[id] = ob; > } > > return ret;
On 27/05/2022 13:52, Laurent Pinchart wrote: >> @@ -464,7 +440,34 @@ PYBIND11_MODULE(_libcamera, m) >> pyControlId >> .def_property_readonly("id", &ControlId::id) >> .def_property_readonly("name", &ControlId::name) >> - .def_property_readonly("type", &ControlId::type); >> + .def_property_readonly("type", &ControlId::type) >> + .def("__str__", [](const ControlId &self) { return self.name(); }) >> + .def("__repr__", [](const ControlId &self) { >> + return py::str("libcamera.ControlId({}, {}, {})") >> + .format(self.id(), self.name(), self.type()); > > Should this be py::str("libcamera.controls.{}").format(self.name()) ? > That may not work for draft controls though. Doesn't work for properties either. Generally speaking, I'm not quite sure what to do with __repr__ in cases where you can't create the object yourself. Tomi
diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py index f6e8232c..f464bd01 100755 --- a/src/py/cam/cam.py +++ b/src/py/cam/cam.py @@ -48,16 +48,16 @@ class CameraContext: print('Properties for', self.id) - for name, prop in camera.properties.items(): - print('\t{}: {}'.format(name, prop)) + for cid, val in camera.properties.items(): + print('\t{}: {}'.format(cid, val)) def do_cmd_list_controls(self): camera = self.camera print('Controls for', self.id) - for name, prop in camera.controls.items(): - print('\t{}: {}'.format(name, prop)) + for cid, info in camera.controls.items(): + print('\t{}: {}'.format(cid, info)) def do_cmd_info(self): camera = self.camera diff --git a/src/py/cam/cam_qt.py b/src/py/cam/cam_qt.py index d638e9cc..739e9749 100644 --- a/src/py/cam/cam_qt.py +++ b/src/py/cam/cam_qt.py @@ -267,9 +267,9 @@ class MainWindow(QtWidgets.QWidget): camera = ctx.camera - for k, v in camera.properties.items(): + for cid, cv in camera.properties.items(): lab = QtWidgets.QLabel() - lab.setText(k + ' = ' + str(v)) + lab.setText("{} = {}".format(cid, cv)) groupLayout.addWidget(lab) group = QtWidgets.QGroupBox('Controls') @@ -277,9 +277,10 @@ class MainWindow(QtWidgets.QWidget): group.setLayout(groupLayout) controlsLayout.addWidget(group) - for k, (min, max, default) in camera.controls.items(): + for cid, cinfo in camera.controls.items(): lab = QtWidgets.QLabel() - lab.setText('{} = {}/{}/{}'.format(k, min, max, default)) + lab.setText('{} = {}/{}/{}' + .format(cid, cinfo.min, cinfo.max, cinfo.default)) groupLayout.addWidget(lab) controlsLayout.addStretch() diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp index 33ecc1cd..80f26c12 100644 --- a/src/py/libcamera/py_main.cpp +++ b/src/py/libcamera/py_main.cpp @@ -159,6 +159,7 @@ PYBIND11_MODULE(_libcamera, m) auto pyFrameBuffer = py::class_<FrameBuffer>(m, "FrameBuffer"); auto pyStream = py::class_<Stream>(m, "Stream"); auto pyControlId = py::class_<ControlId>(m, "ControlId"); + auto pyControlInfo = py::class_<ControlInfo>(m, "ControlInfo"); auto pyRequest = py::class_<Request>(m, "Request"); auto pyRequestStatus = py::enum_<Request::Status>(pyRequest, "Status"); auto pyRequestReuse = py::enum_<Request::ReuseFlag>(pyRequest, "Reuse"); @@ -260,28 +261,17 @@ PYBIND11_MODULE(_libcamera, m) .def_property_readonly("id", &Camera::id) .def("acquire", &Camera::acquire) .def("release", &Camera::release) - .def("start", [](Camera &self, py::dict controls) { + .def("start", [](Camera &self, + const std::unordered_map<const ControlId *, py::object> &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>(); + ControlList controlList(self.controls()); - 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())); + for (const auto& [id, obj]: controls) { + auto val = pyToControlValue(obj, id->type()); + controlList.set(id->id(), val); } int ret = self.start(&controlList); @@ -291,7 +281,7 @@ PYBIND11_MODULE(_libcamera, m) } return 0; - }, py::arg("controls") = py::dict()) + }, py::arg("controls") = std::unordered_map<const ControlId *, py::object>()) .def("stop", [](Camera &self) { int ret = self.stop(); @@ -341,40 +331,26 @@ PYBIND11_MODULE(_libcamera, m) return set; }) - .def("find_control", [](Camera &self, const std::string &name) { - const auto &controls = self.controls(); - - auto it = std::find_if(controls.begin(), controls.end(), - [&name](const auto &kvp) { - return kvp.first->name() == name; - }); - - if (it == controls.end()) - throw std::runtime_error("Control '" + name + "' not found"); - - return it->first; - }, py::return_value_policy::reference_internal) - .def_property_readonly("controls", [](Camera &self) { - py::dict ret; + /* Convert ControlInfoMap to std container */ - 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())); - } + std::unordered_map<const ControlId *, ControlInfo> ret; + + for (const auto &[k, cv] : self.controls()) + ret[k] = cv; return ret; }) .def_property_readonly("properties", [](Camera &self) { - py::dict ret; + /* Convert ControlList to std container */ - for (const auto &[key, cv] : self.properties()) { - const ControlId *id = properties::properties.at(key); - py::object ob = controlValueToPy(cv); + std::unordered_map<const ControlId *, py::object> ret; - ret[id->name().c_str()] = ob; + for (const auto &[k, cv] : self.properties()) { + const ControlId *id = properties::properties.at(k); + py::object ob = controlValueToPy(cv); + ret[id] = ob; } return ret; @@ -464,7 +440,34 @@ PYBIND11_MODULE(_libcamera, m) pyControlId .def_property_readonly("id", &ControlId::id) .def_property_readonly("name", &ControlId::name) - .def_property_readonly("type", &ControlId::type); + .def_property_readonly("type", &ControlId::type) + .def("__str__", [](const ControlId &self) { return self.name(); }) + .def("__repr__", [](const ControlId &self) { + return py::str("libcamera.ControlId({}, {}, {})") + .format(self.id(), self.name(), self.type()); + }); + + pyControlInfo + .def_property_readonly("min", [](const ControlInfo& self) { + return controlValueToPy(self.min()); + }) + .def_property_readonly("max", [](const ControlInfo& self) { + return controlValueToPy(self.max()); + }) + .def_property_readonly("default", [](const ControlInfo& self) { + return controlValueToPy(self.def()); + }) + .def_property_readonly("values", [](const ControlInfo& self) { + py::list l; + for (const auto &v : self.values()) + l.append(controlValueToPy(v)); + return l; + }) + .def("__str__", &ControlInfo::toString) + .def("__repr__", [](const ControlInfo& self) { + return py::str("libcamera.ControlInfo({})") + .format(self.toString()); + }); pyRequest /* \todo Fence is not supported, so we cannot expose addBuffer() directly */ @@ -475,17 +478,18 @@ PYBIND11_MODULE(_libcamera, m) .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) { + .def("set_control", [](Request &self, const ControlId &id, py::object value) { self.controls().set(id.id(), pyToControlValue(value, id.type())); }) .def_property_readonly("metadata", [](Request &self) { - py::dict ret; + /* Convert ControlList to std container */ + + std::unordered_map<const ControlId *, py::object> 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; + ret[id] = ob; } return ret;
Add ControlInfo class and change the controls related methods to resemble the C++ API (e.g. no more string based control methods). We don't implement ControlList or ControlInfoMap but just expose the same data via standard Python dict. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> --- src/py/cam/cam.py | 8 +-- src/py/cam/cam_qt.py | 9 ++-- src/py/libcamera/py_main.cpp | 100 ++++++++++++++++++----------------- 3 files changed, 61 insertions(+), 56 deletions(-)