[{"id":27241,"web_url":"https://patchwork.libcamera.org/comment/27241/","msgid":"<20230605064750.GF22604@pendragon.ideasonboard.com>","date":"2023-06-05T06:47:50","subject":"Re: [libcamera-devel] [PATCH v5 03/13] py: New event handling","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Tomi,\n\nThank you for the patch.\n\nOn Sat, Jun 03, 2023 at 10:56:05AM +0300, Tomi Valkeinen wrote:\n> At the moment the Python bindings only handle the requestCompleted\n> events. But we have others, bufferCompleted and disconnect from the\n> Camera, and cameraAdded and cameraRemoved from the CameraManager.\n> \n> This patch makes all those events available to the users.\n> \n> The get_ready_requests() method is now deprecated, but available to keep\n> backward compatibility.\n> \n> The new event handling works like get_ready_requests(), but instead of\n> returning only Requests from requestCompleted events, it returns all\n> event types using a new Event class.\n> \n> Additionally camera.stop() has been changed to return events for that\n> specific camera. This serves two purposes: first, it removes the\n> RequestCompleted and BufferCompleted events from the event queue, thus\n> preventing the old events being returned when the camera is started\n> again, and second, it allows the user to process those events if it so\n> wants.\n\nReturning events from stop() is a bit of a weird API, and I'm a bit\nworried it will confuse users, but I don't have any better option to\npropose. I think it's good enough for now.\n\n> The CameraManager events (CameraAdded and CameraRemoved) are always\n> enabled, but of the Camera events only RequestCompleted is enabled by\n> default. Other Camera events can be enabled\n> Camera.enable_camera_event().\n> \n> Signed-off-by: Tomi Valkeinen \n> ---\n\nNote for the future, per-patch changelogs would be useful.\n\n> Documentation/python-bindings.rst | 24 ++-\n> src/py/libcamera/py_camera_manager.cpp | 275 +++++++++++++++++++++++--\n> src/py/libcamera/py_camera_manager.h | 73 ++++++-\n> src/py/libcamera/py_main.cpp | 54 ++++-\n> 4 files changed, 386 insertions(+), 40 deletions(-)\n> \n> diff --git a/Documentation/python-bindings.rst b/Documentation/python-bindings.rst\n> index bac5cd34..8f2d76c3 100644\n> --- a/Documentation/python-bindings.rst\n> +++ b/Documentation/python-bindings.rst\n> @@ -43,17 +43,23 @@ CameraManager\n> The Python API provides a singleton CameraManager via ``CameraManager.singleton()``.\n> There is no need to start or stop the CameraManager.\n> \n> -Handling Completed Requests\n> ----------------------------\n> +Event Handling\n> +--------------\n> \n> -The Python bindings do not expose the ``Camera::requestCompleted`` signal\n> -directly as the signal is invoked from another thread and it has real-time\n> -constraints. Instead the bindings queue the completed requests internally and\n> -use an eventfd to inform the user that there are completed requests.\n> +The Python bindings do not expose the signals from the C++ side directly as the\n> +signals are invoked from another thread and they may have real-time\n> +constraints. Instead the bindings queue the received events internally and use\n> +an eventfd to inform the user that there are events to be handled.\n> \n> -The user can wait on the eventfd, and upon getting an event, use\n> -``CameraManager.get_ready_requests()`` to clear the eventfd event and to get\n> -the completed requests.\n> +The user can wait on the eventfd (e.g. by using Python Selector), and use\n> +``CameraManager.get_events()`` to reset the eventfd and get the events.\n> +\n> +The CameraManager events (CameraAdded and CameraRemoved) are always enabled, but\n> +of the Camera events only RequestCompleted is enabled by default. To enable\n> +Disconnect or BufferCompleted event, use ``Camera.enable_camera_event()``.\n> +\n> +The ``Camera.stop()`` method will return all events related to that Camera from\n> +the event queue.\n> \n> Controls & Properties\n> ---------------------\n> diff --git a/src/py/libcamera/py_camera_manager.cpp b/src/py/libcamera/py_camera_manager.cpp\n> index 9ccb7aad..83c2b063 100644\n> --- a/src/py/libcamera/py_camera_manager.cpp\n> +++ b/src/py/libcamera/py_camera_manager.cpp\n> @@ -5,6 +5,7 @@\n> \n> #include \"py_camera_manager.h\"\n> \n> +#include \n> #include \n> #include \n> #include \n> @@ -35,11 +36,24 @@ PyCameraManager::PyCameraManager()\n> \tif (ret)\n> \t\tthrow std::system_error(-ret, std::generic_category(),\n> \t\t\t\t\t\"Failed to start CameraManager\");\n> +\n> +\tcameraManager_->cameraAdded.connect(this, &PyCameraManager::handleCameraAdded);\n> +\tcameraManager_->cameraRemoved.connect(this, &PyCameraManager::handleCameraRemoved);\n> }\n> \n> PyCameraManager::~PyCameraManager()\n> {\n> \tLOG(Python, Debug) << \"~PyCameraManager()\";\n> +\n> +\tcameraManager_->cameraAdded.disconnect();\n> +\tcameraManager_->cameraRemoved.disconnect();\n> +}\n> +\n> +void PyCameraManager::init()\n> +{\n> +\t/* Always enable RequestCompleted for all cameras by default */\n> +\tfor (auto cam : cameraManager_->cameras())\n> +\t\tsetCameraEventFlag(cam, CameraEventType::RequestCompleted, true);\n\nYou need to also enable the event when a new camera is detected.\n\n> }\n> \n> py::list PyCameraManager::cameras()\n> @@ -60,6 +74,43 @@ py::list PyCameraManager::cameras()\n> \treturn l;\n> }\n> \n> +PyCameraEvent PyCameraManager::convertEvent(const CameraEvent &event)\n> +{\n> +\t/*\n> +\t * We need to set a keep-alive here so that the camera keeps the\n> +\t * camera manager alive.\n> +\t */\n> +\tpy::object py_cm = py::cast(this);\n> +\tpy::object py_cam = py::cast(event.camera_);\n> +\tpy::detail::keep_alive_impl(py_cam, py_cm);\n> +\n> +\tPyCameraEvent pyevent(event.type_, py_cam);\n> +\n> +\tswitch (event.type_) {\n> +\tcase CameraEventType::CameraAdded:\n> +\tcase CameraEventType::CameraRemoved:\n> +\tcase CameraEventType::Disconnect:\n> +\t\t/* No additional parameters to add */\n> +\t\tbreak;\n> +\n> +\tcase CameraEventType::BufferCompleted:\n> +\t\tpyevent.request_ = py::cast(event.request_);\n> +\t\tpyevent.fb_ = py::cast(event.fb_);\n> +\t\tbreak;\n> +\n> +\tcase CameraEventType::RequestCompleted:\n> +\t\tpyevent.request_ = py::cast(event.request_);\n> +\n> +\t\t/* Decrease the ref increased in Camera.queue_request() */\n> +\t\tpyevent.request_.dec_ref();\n> +\n> +\t\tbreak;\n> +\t}\n> +\n> +\treturn pyevent;\n> +}\n> +\n> +/* DEPRECATED */\n> std::vector PyCameraManager::getReadyRequests()\n> {\n> \tint ret = readFd();\n> @@ -72,21 +123,207 @@ std::vector PyCameraManager::getReadyRequests()\n> \n> \tstd::vector py_reqs;\n> \n> -\tfor (Request *request : getCompletedRequests()) {\n> -\t\tpy::object o = py::cast(request);\n> -\t\t/* Decrease the ref increased in Camera.queue_request() */\n> -\t\to.dec_ref();\n> -\t\tpy_reqs.push_back(o);\n> +\tfor (const auto &ev : getEvents()) {\n> +\t\tif (ev.type_ != CameraEventType::RequestCompleted)\n> +\t\t\tcontinue;\n> +\n> +\t\tPyCameraEvent pyev = convertEvent(ev);\n> +\t\tpy_reqs.push_back(pyev.request_);\n> \t}\n> \n> \treturn py_reqs;\n> }\n> \n> +std::vector PyCameraManager::getPyEvents()\n> +{\n> +\tint ret = readFd();\n> +\n> +\tif (ret == EAGAIN) {\n> +\t\tLOG(Python, Debug) << \"No events\";\n> +\t\treturn {};\n> +\t}\n> +\n> +\tif (ret != 0)\n> +\t\tthrow std::system_error(ret, std::generic_category());\n> +\n> +\tstd::vector events = getEvents();\n> +\n> +\tLOG(Python, Debug) << \"Got \" << events.size() << \" events\";\n> +\n> +\tstd::vector pyevents;\n> +\tpyevents.reserve(events.size());\n> +\n> +\tstd::transform(events.begin(), events.end(), std::back_inserter(pyevents),\n> +\t\t [this](const CameraEvent &ev) {\n> +\t\t\t return convertEvent(ev);\n> +\t\t });\n> +\n> +\treturn pyevents;\n> +}\n> +\n> +static bool isCameraSpecificEvent(const CameraEvent &event, std::shared_ptr &camera)\n> +{\n> +\treturn event.camera_ == camera &&\n> +\t (event.type_ == CameraEventType::RequestCompleted ||\n> +\t\tevent.type_ == CameraEventType::BufferCompleted ||\n> +\t\tevent.type_ == CameraEventType::Disconnect);\n> +}\n> +\n> +std::vector PyCameraManager::getPyCameraEvents(std::shared_ptr camera)\n> +{\n> +\tstd::vector events;\n> +\tsize_t unhandled_size;\n> +\n> +\t{\n> +\t\tMutexLocker guard(eventsMutex_);\n> +\n> +\t\t/*\n> +\t\t * Collect events related to the given camera and remove them\n> +\t\t * from the events_ vector.\n> +\t\t */\n> +\n> +\t\tauto it = events_.begin();\n> +\t\twhile (it != events_.end()) {\n> +\t\t\tif (isCameraSpecificEvent(*it, camera)) {\n> +\t\t\t\tevents.push_back(*it);\n> +\t\t\t\tit = events_.erase(it);\n> +\t\t\t} else {\n> +\t\t\t\tit++;\n> +\t\t\t}\n> +\t\t}\n> +\n> +\t\tunhandled_size = events_.size();\n> +\t}\n> +\n> +\t/* Convert events to Python events */\n> +\n> +\tstd::vector pyevents;\n> +\n> +\tfor (const auto &event : events) {\n> +\t\tPyCameraEvent pyev = convertEvent(event);\n> +\t\tpyevents.push_back(pyev);\n> +\t}\n> +\n> +\tLOG(Python, Debug) << \"Got \" << pyevents.size() << \" camera events, \"\n> +\t\t\t << unhandled_size << \" unhandled events left\";\n> +\n> +\treturn pyevents;\n> +}\n> +\n> /* Note: Called from another thread */\n> -void PyCameraManager::handleRequestCompleted(Request *req)\n> +void PyCameraManager::handleBufferCompleted(std::shared_ptr cam, Request *req, FrameBuffer *fb)\n> {\n> -\tpushRequest(req);\n> -\twriteFd();\n> +\tCameraEvent ev(CameraEventType::BufferCompleted, cam, req, fb);\n> +\n> +\tpushEvent(ev);\n> +}\n> +\n> +/* Note: Called from another thread */\n> +void PyCameraManager::handleRequestCompleted(std::shared_ptr cam, Request *req)\n> +{\n> +\tCameraEvent ev(CameraEventType::RequestCompleted, cam, req);\n> +\n> +\tpushEvent(ev);\n> +}\n> +\n> +/* Note: Called from another thread */\n> +void PyCameraManager::handleDisconnected(std::shared_ptr cam)\n> +{\n> +\tCameraEvent ev(CameraEventType::Disconnect, cam);\n> +\n> +\tpushEvent(ev);\n> +}\n> +\n> +/* Note: Called from another thread */\n> +void PyCameraManager::handleCameraAdded(std::shared_ptr cam)\n> +{\n> +\tCameraEvent ev(CameraEventType::CameraAdded, cam);\n> +\n> +\tpushEvent(ev);\n> +}\n> +\n> +/* Note: Called from another thread */\n> +void PyCameraManager::handleCameraRemoved(std::shared_ptr cam)\n> +{\n> +\tCameraEvent ev(CameraEventType::CameraRemoved, cam);\n> +\n> +\tpushEvent(ev);\n> +}\n> +\n> +bool PyCameraManager::getCameraEventFlag(std::shared_ptr camera, CameraEventType event_type)\n> +{\n> +\tconst uint32_t evbit = 1 << (uint32_t)event_type;\n> +\tuint32_t mask = 0;\n> +\n> +\tif (auto it = camera_event_masks_.find(camera); it != camera_event_masks_.end())\n\nThat's a weird style.\n\n\tauto it = camera_event_masks_.find(camera);\n\tif (it != camera_event_masks_.end())\n\n> +\t\tmask = it->second;\n> +\n> +\treturn !!(mask & evbit);\n> +}\n> +\n> +void PyCameraManager::setCameraEventFlag(std::shared_ptr camera, CameraEventType event_type, bool value)\n> +{\n> +\tswitch (event_type) {\n> +\tcase CameraEventType::RequestCompleted:\n> +\tcase CameraEventType::BufferCompleted:\n> +\tcase CameraEventType::Disconnect:\n> +\t\tbreak;\n> +\n> +\tdefault:\n> +\t\tthrow std::runtime_error(\"Bad camera event type\");\n> +\t}\n> +\n> +\tconst uint32_t evbit = 1 << (uint32_t)event_type;\n> +\tuint32_t mask = 0;\n> +\n> +\tif (auto it = camera_event_masks_.find(camera); it != camera_event_masks_.end())\n> +\t\tmask = it->second;\n\nSame here.\n\n> +\n> +\tbool old_val = !!(mask & evbit);\n> +\n> +\tif (old_val == value)\n> +\t\treturn;\n> +\n> +\tif (value)\n> +\t\tmask |= evbit;\n> +\telse\n> +\t\tmask &= ~evbit;\n> +\n> +\tcamera_event_masks_[camera] = mask;\n> +\n> +\tswitch (event_type) {\n> +\tcase CameraEventType::RequestCompleted:\n> +\t\tif (value) {\n> +\t\t\tcamera->requestCompleted.connect(camera.get(), [cm = this->shared_from_this(), camera](Request *req) {\n> +\t\t\t\tcm->handleRequestCompleted(camera, req);\n> +\t\t\t});\n> +\t\t} else {\n> +\t\t\tcamera->requestCompleted.disconnect();\n> +\t\t}\n> +\t\tbreak;\n\nThis is racy. PyCameraManager::handleRequestCompleted() could be called\nbefore you disconnect the signal, with the event then stored in the\nevents_ queue. The application would then receive the event after\ndisabling it.\n\nOne option would be to connect the signals unconditionally and instead\nfilter events out when retrieving them in getPyEvents().\n\n> +\n> +\tcase CameraEventType::BufferCompleted:\n> +\t\tif (value) {\n> +\t\t\tcamera->bufferCompleted.connect(camera.get(), [cm = this->shared_from_this(), camera](Request *req, FrameBuffer *fb) {\n> +\t\t\t\tcm->handleBufferCompleted(camera, req, fb);\n> +\t\t\t});\n> +\t\t} else {\n> +\t\t\tcamera->bufferCompleted.disconnect();\n> +\t\t}\n> +\t\tbreak;\n> +\n> +\tcase CameraEventType::Disconnect:\n> +\t\tif (value) {\n> +\t\t\tcamera->disconnected.connect(camera.get(), [cm = this->shared_from_this(), camera]() {\n> +\t\t\t\tcm->handleDisconnected(camera);\n> +\t\t\t});\n> +\t\t} else {\n> +\t\t\tcamera->disconnected.disconnect();\n> +\t\t}\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tASSERT(false);\n> +\t}\n> }\n> \n> void PyCameraManager::writeFd()\n> @@ -116,16 +353,24 @@ int PyCameraManager::readFd()\n> \t\treturn -EIO;\n> }\n> \n> -void PyCameraManager::pushRequest(Request *req)\n> +void PyCameraManager::pushEvent(const CameraEvent &ev)\n> {\n> -\tMutexLocker guard(completedRequestsMutex_);\n> -\tcompletedRequests_.push_back(req);\n> +\t{\n> +\t\tMutexLocker guard(eventsMutex_);\n> +\t\tevents_.push_back(ev);\n> +\t}\n> +\n> +\twriteFd();\n> +\n> +\tLOG(Python, Debug) << \"Queued events: \" << events_.size();\n> }\n> \n> -std::vector PyCameraManager::getCompletedRequests()\n> +std::vector PyCameraManager::getEvents()\n> {\n> -\tstd::vector v;\n> -\tMutexLocker guard(completedRequestsMutex_);\n> -\tswap(v, completedRequests_);\n> +\tstd::vector v;\n> +\n> +\tMutexLocker guard(eventsMutex_);\n> +\tswap(v, events_);\n> +\n> \treturn v;\n> }\n> diff --git a/src/py/libcamera/py_camera_manager.h b/src/py/libcamera/py_camera_manager.h\n> index 3574db23..31747547 100644\n> --- a/src/py/libcamera/py_camera_manager.h\n> +++ b/src/py/libcamera/py_camera_manager.h\n> @@ -13,12 +13,56 @@\n> \n> using namespace libcamera;\n> \n> -class PyCameraManager\n> +enum class CameraEventType {\n> +\tCameraAdded,\n> +\tCameraRemoved,\n> +\tDisconnect,\n> +\tRequestCompleted,\n> +\tBufferCompleted,\n> +};\n> +\n> +/*\n> + * This event struct is used internally to queue the events we receive from\n> + * other threads.\n> + */\n> +struct CameraEvent {\n> +\tCameraEvent(CameraEventType type, std::shared_ptr camera,\n> +\t\t Request *request = nullptr, FrameBuffer *fb = nullptr)\n> +\t\t: type_(type), camera_(camera), request_(request), fb_(fb)\n> +\t{\n> +\t}\n> +\n> +\tCameraEventType type_;\n> +\tstd::shared_ptr camera_;\n> +\tRequest *request_;\n> +\tFrameBuffer *fb_;\n> +};\n> +\n> +/*\n> + * This event struct is passed to Python. We need to use pybind11::object here\n> + * instead of a C++ pointer so that we keep a ref to the Request, and a\n> + * keep-alive from the camera to the camera manager.\n> + */\n> +struct PyCameraEvent {\n> +\tPyCameraEvent(CameraEventType type, pybind11::object camera)\n> +\t\t: type_(type), camera_(camera)\n> +\t{\n> +\t}\n> +\n> +\tCameraEventType type_;\n> +\tpybind11::object camera_;\n> +\tpybind11::object request_;\n> +\tpybind11::object fb_;\n> +};\n> +\n> +class PyCameraManager : public std::enable_shared_from_this\n> {\n> public:\n> \tPyCameraManager();\n> \t~PyCameraManager();\n> \n> +\tvoid init();\n> +\n> \tpybind11::list cameras();\n> \tstd::shared_ptr get(const std::string &name) { return cameraManager_->get(name); }\n> \n> @@ -26,20 +70,33 @@ public:\n> \n> \tint eventFd() const { return eventFd_.get(); }\n> \n> -\tstd::vector getReadyRequests();\n> +\tstd::vector getReadyRequests(); /* DEPRECATED */\n> +\tstd::vector getPyEvents();\n> +\tstd::vector getPyCameraEvents(std::shared_ptr camera);\n> +\n> +\tvoid handleBufferCompleted(std::shared_ptr cam, Request *req, FrameBuffer *fb);\n> +\tvoid handleRequestCompleted(std::shared_ptr cam, Request *req);\n> +\tvoid handleDisconnected(std::shared_ptr cam);\n> +\tvoid handleCameraAdded(std::shared_ptr cam);\n> +\tvoid handleCameraRemoved(std::shared_ptr cam);\n> \n> -\tvoid handleRequestCompleted(Request *req);\n> +\tbool getCameraEventFlag(std::shared_ptr camera, CameraEventType event_type);\n> +\tvoid setCameraEventFlag(std::shared_ptr camera, CameraEventType event_type, bool value);\n> \n> private:\n> \tstd::unique_ptr cameraManager_;\n> \n> \tUniqueFD eventFd_;\n> -\tlibcamera::Mutex completedRequestsMutex_;\n> -\tstd::vector completedRequests_\n> -\t\tLIBCAMERA_TSA_GUARDED_BY(completedRequestsMutex_);\n> +\tlibcamera::Mutex eventsMutex_;\n> +\tstd::vector events_\n> +\t\tLIBCAMERA_TSA_GUARDED_BY(eventsMutex_);\n> \n> \tvoid writeFd();\n> \tint readFd();\n> -\tvoid pushRequest(Request *req);\n> -\tstd::vector getCompletedRequests();\n> +\tvoid pushEvent(const CameraEvent &ev);\n> +\tstd::vector getEvents();\n> +\n> +\tPyCameraEvent convertEvent(const CameraEvent &event);\n> +\n> +\tstd::map, uint32_t> camera_event_masks_;\n> };\n> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n> index 5a5f1a37..981a3070 100644\n> --- a/src/py/libcamera/py_main.cpp\n> +++ b/src/py/libcamera/py_main.cpp\n> @@ -110,6 +110,7 @@ PYBIND11_MODULE(_libcamera, m)\n> \t * https://pybind11.readthedocs.io/en/latest/advanced/misc.html#avoiding-c-types-in-docstrings\n> \t */\n> \n> +\tauto pyEvent = py::class_(m, \"Event\");\n> \tauto pyCameraManager = py::class_>(m, \"CameraManager\");\n> \tauto pyCamera = py::class_>(m, \"Camera\");\n> \tauto pyCameraConfiguration = py::class_(m, \"CameraConfiguration\");\n> @@ -136,12 +137,27 @@ PYBIND11_MODULE(_libcamera, m)\n> \tm.def(\"log_set_level\", &logSetLevel);\n> \n> \t/* Classes */\n> +\n> +\tpy::enum_(pyEvent, \"Type\")\n> +\t\t.value(\"CameraAdded\", CameraEventType::CameraAdded)\n> +\t\t.value(\"CameraRemoved\", CameraEventType::CameraRemoved)\n> +\t\t.value(\"Disconnect\", CameraEventType::Disconnect)\n> +\t\t.value(\"RequestCompleted\", CameraEventType::RequestCompleted)\n> +\t\t.value(\"BufferCompleted\", CameraEventType::BufferCompleted);\n> +\n> +\tpyEvent\n> +\t\t.def_readonly(\"type\", &PyCameraEvent::type_)\n> +\t\t.def_readonly(\"camera\", &PyCameraEvent::camera_)\n> +\t\t.def_readonly(\"request\", &PyCameraEvent::request_)\n> +\t\t.def_readonly(\"fb\", &PyCameraEvent::fb_);\n> +\n> \tpyCameraManager\n> \t\t.def_static(\"singleton\", []() {\n> \t\t\tstd::shared_ptr cm = gCameraManager.lock();\n> \n> \t\t\tif (!cm) {\n> \t\t\t\tcm = std::make_shared();\n> +\t\t\t\tcm->init();\n> \t\t\t\tgCameraManager = cm;\n> \t\t\t}\n> \n> @@ -153,10 +169,33 @@ PYBIND11_MODULE(_libcamera, m)\n> \t\t.def_property_readonly(\"cameras\", &PyCameraManager::cameras)\n> \n> \t\t.def_property_readonly(\"event_fd\", &PyCameraManager::eventFd)\n> -\t\t.def(\"get_ready_requests\", &PyCameraManager::getReadyRequests);\n> +\n> +\t\t/* DEPRECATED */\n> +\t\t.def(\"get_ready_requests\", &PyCameraManager::getReadyRequests)\n> +\n> +\t\t.def(\"get_events\", &PyCameraManager::getPyEvents);\n> \n> \tpyCamera\n> \t\t.def_property_readonly(\"id\", &Camera::id)\n> +\n> +\t\t.def(\"get_camera_event_enabled\",\n> +\t\t\t[](Camera &self, CameraEventType type) {\n> +\t\t\t\tauto cm = gCameraManager.lock();\n> +\t\t\t\treturn cm->getCameraEventFlag(self.shared_from_this(), type);\n> +\t\t\t})\n> +\n> +\t\t.def(\"enable_camera_event\",\n> +\t\t\t[](Camera &self, CameraEventType type) {\n> +\t\t\t\tauto cm = gCameraManager.lock();\n> +\t\t\t\tcm->setCameraEventFlag(self.shared_from_this(), type, true);\n> +\t\t\t})\n> +\n> +\t\t.def(\"disable_camera_event\",\n> +\t\t\t[](Camera &self, CameraEventType type) {\n> +\t\t\t\tauto cm = gCameraManager.lock();\n> +\t\t\t\tcm->setCameraEventFlag(self.shared_from_this(), type, false);\n> +\t\t\t})\n> +\n> \t\t.def(\"acquire\", [](Camera &self) {\n> \t\t\tint ret = self.acquire();\n> \t\t\tif (ret)\n> @@ -173,11 +212,6 @@ PYBIND11_MODULE(_libcamera, m)\n> \t\t const std::unordered_map &controls) {\n> \t\t\t/* \\todo What happens if someone calls start() multiple times? */\n> \n> -\t\t\tauto cm = gCameraManager.lock();\n> -\t\t\tASSERT(cm);\n> -\n> -\t\t\tself.requestCompleted.connect(cm.get(), &PyCameraManager::handleRequestCompleted);\n> -\n> \t\t\tControlList controlList(self.controls());\n> \n> \t\t\tfor (const auto &[id, obj] : controls) {\n> @@ -187,7 +221,6 @@ PYBIND11_MODULE(_libcamera, m)\n> \n> \t\t\tint ret = self.start(&controlList);\n> \t\t\tif (ret) {\n> -\t\t\t\tself.requestCompleted.disconnect();\n> \t\t\t\tthrow std::system_error(-ret, std::generic_category(),\n> \t\t\t\t\t\t\t\"Failed to start camera\");\n> \t\t\t}\n\nYou can drop the curly braces.\n\n> @@ -196,11 +229,16 @@ PYBIND11_MODULE(_libcamera, m)\n> \t\t.def(\"stop\", [](Camera &self) {\n> \t\t\tint ret = self.stop();\n> \n> -\t\t\tself.requestCompleted.disconnect();\n> +\t\t\tauto cm = gCameraManager.lock();\n> +\t\t\tASSERT(cm);\n> +\n> +\t\t\tauto events = cm->getPyCameraEvents(self.shared_from_this());\n> \n> \t\t\tif (ret)\n> \t\t\t\tthrow std::system_error(-ret, std::generic_category(),\n> \t\t\t\t\t\t\t\"Failed to stop camera\");\n> +\n> +\t\t\treturn events;\n> \t\t})\n> \n> \t\t.def(\"__str__\", [](Camera &self) {","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A00A5C31E9\n\tfor ;\n\tMon, 5 Jun 2023 06:47:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 14D7562722;\n\tMon, 5 Jun 2023 08:47:54 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EA1BE61EA2\n\tfor ;\n\tMon, 5 Jun 2023 08:47:52 +0200 (CEST)","from pendragon.ideasonboard.com (om126156242094.26.openmobile.ne.jp\n\t[126.156.242.94])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0137E1BA;\n\tMon, 5 Jun 2023 08:47:26 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685947674;\n\tbh=n/wL4xEiqd2jz0yhwVVcpGiTgjMw8gvO0J5yW14SYDM=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=MWZrabZNVvGvFhF/m0tcCVAeBrGKMQDw4hWsMzvV1+n816rdAXjhfxQYvhQl67xMQ\n\tgOvv5nT3as9iElAPpMUIngp7TcwS8OFA0kSg/H541w08lqsZWbNToD5YS3pjk1WjZZ\n\tbJb8uBoVFUVhRhZYykYKEhGBUE84SgKmtLjkeYR3t+cSedPlnidWr/1XX43GGvxQXv\n\tRZ/jlbfRugmxg7tBp27roWAR9Ky908HmsEO2qXiX82fH2KBWQOD15fEr8anEVLWKDl\n\tbfrHtah3+mJflQ2xu/tuJtx5lc7GSJruWl30eLNQb0OvASIA9w0OBWNst5Ev2mzI4T\n\twDuCiYWqrQJcA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685947648;\n\tbh=n/wL4xEiqd2jz0yhwVVcpGiTgjMw8gvO0J5yW14SYDM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nV8UQu3COsSO9Mi2mSEOpg2pWMmHO7dsKroNdnLw7TsLbbq6McbcW8CwZB3bqPhlx\n\tLXju9EQCydqoD4whCOEUzwYc1OcHzI7FeK3XNAZwMOh6C1EiBsIJ09UhxSzrUo/PqB\n\t+qfGxZaH9hqjoeKyEHkI8smwEPnCqAzu9OcQek1E="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"nV8UQu3C\"; dkim-atps=neutral","Date":"Mon, 5 Jun 2023 09:47:50 +0300","To":"Tomi Valkeinen ","Message-ID":"<20230605064750.GF22604@pendragon.ideasonboard.com>","References":"<20230603075615.20663-1-tomi.valkeinen@ideasonboard.com>\n\t<20230603075615.20663-4-tomi.valkeinen@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230603075615.20663-4-tomi.valkeinen@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 03/13] py: New event handling","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27243,"web_url":"https://patchwork.libcamera.org/comment/27243/","msgid":"<51586616-f621-7fb8-1a53-86773aa969e4@ideasonboard.com>","date":"2023-06-05T07:31:00","subject":"Re: [libcamera-devel] [PATCH v5 03/13] py: New event handling","submitter":{"id":109,"url":"https://patchwork.libcamera.org/api/people/109/","name":"Tomi Valkeinen","email":"tomi.valkeinen@ideasonboard.com"},"content":"On 05/06/2023 09:47, Laurent Pinchart wrote:\n> Hi Tomi,\n> \n> Thank you for the patch.\n> \n> On Sat, Jun 03, 2023 at 10:56:05AM +0300, Tomi Valkeinen wrote:\n>> At the moment the Python bindings only handle the requestCompleted\n>> events. But we have others, bufferCompleted and disconnect from the\n>> Camera, and cameraAdded and cameraRemoved from the CameraManager.\n>>\n>> This patch makes all those events available to the users.\n>>\n>> The get_ready_requests() method is now deprecated, but available to keep\n>> backward compatibility.\n>>\n>> The new event handling works like get_ready_requests(), but instead of\n>> returning only Requests from requestCompleted events, it returns all\n>> event types using a new Event class.\n>>\n>> Additionally camera.stop() has been changed to return events for that\n>> specific camera. This serves two purposes: first, it removes the\n>> RequestCompleted and BufferCompleted events from the event queue, thus\n>> preventing the old events being returned when the camera is started\n>> again, and second, it allows the user to process those events if it so\n>> wants.\n> \n> Returning events from stop() is a bit of a weird API, and I'm a bit\n> worried it will confuse users, but I don't have any better option to\n> propose. I think it's good enough for now.\n\nIt was added to solve the reported user confusion. So... Confusing both \nways? =).\n\nIt's, of course, not a must. The user can get the events the normal way, \nthus flushing the event queue.\n\nBut I agree that it's a bit weird, and I kind of would like to drop it. \nMaybe just documenting it clearly that one needs to take care of the \nevents after calling stop would be enough?\n\n>> The CameraManager events (CameraAdded and CameraRemoved) are always\n>> enabled, but of the Camera events only RequestCompleted is enabled by\n>> default. Other Camera events can be enabled\n>> Camera.enable_camera_event().\n>>\n>> Signed-off-by: Tomi Valkeinen \n>> ---\n> \n> Note for the future, per-patch changelogs would be useful.\n\nI have never learned to use those... Maybe it's time.\n\n>> Documentation/python-bindings.rst | 24 ++-\n>> src/py/libcamera/py_camera_manager.cpp | 275 +++++++++++++++++++++++--\n>> src/py/libcamera/py_camera_manager.h | 73 ++++++-\n>> src/py/libcamera/py_main.cpp | 54 ++++-\n>> 4 files changed, 386 insertions(+), 40 deletions(-)\n>>\n>> diff --git a/Documentation/python-bindings.rst b/Documentation/python-bindings.rst\n>> index bac5cd34..8f2d76c3 100644\n>> --- a/Documentation/python-bindings.rst\n>> +++ b/Documentation/python-bindings.rst\n>> @@ -43,17 +43,23 @@ CameraManager\n>> The Python API provides a singleton CameraManager via ``CameraManager.singleton()``.\n>> There is no need to start or stop the CameraManager.\n>> \n>> -Handling Completed Requests\n>> ----------------------------\n>> +Event Handling\n>> +--------------\n>> \n>> -The Python bindings do not expose the ``Camera::requestCompleted`` signal\n>> -directly as the signal is invoked from another thread and it has real-time\n>> -constraints. Instead the bindings queue the completed requests internally and\n>> -use an eventfd to inform the user that there are completed requests.\n>> +The Python bindings do not expose the signals from the C++ side directly as the\n>> +signals are invoked from another thread and they may have real-time\n>> +constraints. Instead the bindings queue the received events internally and use\n>> +an eventfd to inform the user that there are events to be handled.\n>> \n>> -The user can wait on the eventfd, and upon getting an event, use\n>> -``CameraManager.get_ready_requests()`` to clear the eventfd event and to get\n>> -the completed requests.\n>> +The user can wait on the eventfd (e.g. by using Python Selector), and use\n>> +``CameraManager.get_events()`` to reset the eventfd and get the events.\n>> +\n>> +The CameraManager events (CameraAdded and CameraRemoved) are always enabled, but\n>> +of the Camera events only RequestCompleted is enabled by default. To enable\n>> +Disconnect or BufferCompleted event, use ``Camera.enable_camera_event()``.\n>> +\n>> +The ``Camera.stop()`` method will return all events related to that Camera from\n>> +the event queue.\n>> \n>> Controls & Properties\n>> ---------------------\n>> diff --git a/src/py/libcamera/py_camera_manager.cpp b/src/py/libcamera/py_camera_manager.cpp\n>> index 9ccb7aad..83c2b063 100644\n>> --- a/src/py/libcamera/py_camera_manager.cpp\n>> +++ b/src/py/libcamera/py_camera_manager.cpp\n>> @@ -5,6 +5,7 @@\n>> \n>> #include \"py_camera_manager.h\"\n>> \n>> +#include \n>> #include \n>> #include \n>> #include \n>> @@ -35,11 +36,24 @@ PyCameraManager::PyCameraManager()\n>> \tif (ret)\n>> \t\tthrow std::system_error(-ret, std::generic_category(),\n>> \t\t\t\t\t\"Failed to start CameraManager\");\n>> +\n>> +\tcameraManager_->cameraAdded.connect(this, &PyCameraManager::handleCameraAdded);\n>> +\tcameraManager_->cameraRemoved.connect(this, &PyCameraManager::handleCameraRemoved);\n>> }\n>> \n>> PyCameraManager::~PyCameraManager()\n>> {\n>> \tLOG(Python, Debug) << \"~PyCameraManager()\";\n>> +\n>> +\tcameraManager_->cameraAdded.disconnect();\n>> +\tcameraManager_->cameraRemoved.disconnect();\n>> +}\n>> +\n>> +void PyCameraManager::init()\n>> +{\n>> +\t/* Always enable RequestCompleted for all cameras by default */\n>> +\tfor (auto cam : cameraManager_->cameras())\n>> +\t\tsetCameraEventFlag(cam, CameraEventType::RequestCompleted, true);\n> \n> You need to also enable the event when a new camera is detected.\n\nAh, good point. But I also wonder, should this just be dropped too? I'm \nnot sure if the return-events-from-stop and this one are just hacks that \nseemingly make life easier for the users, but in the end just add \nconfusion for both the user and in the codebase. Or are they actually \ngood features...\n\n>> }\n>> \n>> py::list PyCameraManager::cameras()\n>> @@ -60,6 +74,43 @@ py::list PyCameraManager::cameras()\n>> \treturn l;\n>> }\n>> \n>> +PyCameraEvent PyCameraManager::convertEvent(const CameraEvent &event)\n>> +{\n>> +\t/*\n>> +\t * We need to set a keep-alive here so that the camera keeps the\n>> +\t * camera manager alive.\n>> +\t */\n>> +\tpy::object py_cm = py::cast(this);\n>> +\tpy::object py_cam = py::cast(event.camera_);\n>> +\tpy::detail::keep_alive_impl(py_cam, py_cm);\n>> +\n>> +\tPyCameraEvent pyevent(event.type_, py_cam);\n>> +\n>> +\tswitch (event.type_) {\n>> +\tcase CameraEventType::CameraAdded:\n>> +\tcase CameraEventType::CameraRemoved:\n>> +\tcase CameraEventType::Disconnect:\n>> +\t\t/* No additional parameters to add */\n>> +\t\tbreak;\n>> +\n>> +\tcase CameraEventType::BufferCompleted:\n>> +\t\tpyevent.request_ = py::cast(event.request_);\n>> +\t\tpyevent.fb_ = py::cast(event.fb_);\n>> +\t\tbreak;\n>> +\n>> +\tcase CameraEventType::RequestCompleted:\n>> +\t\tpyevent.request_ = py::cast(event.request_);\n>> +\n>> +\t\t/* Decrease the ref increased in Camera.queue_request() */\n>> +\t\tpyevent.request_.dec_ref();\n>> +\n>> +\t\tbreak;\n>> +\t}\n>> +\n>> +\treturn pyevent;\n>> +}\n>> +\n>> +/* DEPRECATED */\n>> std::vector PyCameraManager::getReadyRequests()\n>> {\n>> \tint ret = readFd();\n>> @@ -72,21 +123,207 @@ std::vector PyCameraManager::getReadyRequests()\n>> \n>> \tstd::vector py_reqs;\n>> \n>> -\tfor (Request *request : getCompletedRequests()) {\n>> -\t\tpy::object o = py::cast(request);\n>> -\t\t/* Decrease the ref increased in Camera.queue_request() */\n>> -\t\to.dec_ref();\n>> -\t\tpy_reqs.push_back(o);\n>> +\tfor (const auto &ev : getEvents()) {\n>> +\t\tif (ev.type_ != CameraEventType::RequestCompleted)\n>> +\t\t\tcontinue;\n>> +\n>> +\t\tPyCameraEvent pyev = convertEvent(ev);\n>> +\t\tpy_reqs.push_back(pyev.request_);\n>> \t}\n>> \n>> \treturn py_reqs;\n>> }\n>> \n>> +std::vector PyCameraManager::getPyEvents()\n>> +{\n>> +\tint ret = readFd();\n>> +\n>> +\tif (ret == EAGAIN) {\n>> +\t\tLOG(Python, Debug) << \"No events\";\n>> +\t\treturn {};\n>> +\t}\n>> +\n>> +\tif (ret != 0)\n>> +\t\tthrow std::system_error(ret, std::generic_category());\n>> +\n>> +\tstd::vector events = getEvents();\n>> +\n>> +\tLOG(Python, Debug) << \"Got \" << events.size() << \" events\";\n>> +\n>> +\tstd::vector pyevents;\n>> +\tpyevents.reserve(events.size());\n>> +\n>> +\tstd::transform(events.begin(), events.end(), std::back_inserter(pyevents),\n>> +\t\t [this](const CameraEvent &ev) {\n>> +\t\t\t return convertEvent(ev);\n>> +\t\t });\n>> +\n>> +\treturn pyevents;\n>> +}\n>> +\n>> +static bool isCameraSpecificEvent(const CameraEvent &event, std::shared_ptr &camera)\n>> +{\n>> +\treturn event.camera_ == camera &&\n>> +\t (event.type_ == CameraEventType::RequestCompleted ||\n>> +\t\tevent.type_ == CameraEventType::BufferCompleted ||\n>> +\t\tevent.type_ == CameraEventType::Disconnect);\n>> +}\n>> +\n>> +std::vector PyCameraManager::getPyCameraEvents(std::shared_ptr camera)\n>> +{\n>> +\tstd::vector events;\n>> +\tsize_t unhandled_size;\n>> +\n>> +\t{\n>> +\t\tMutexLocker guard(eventsMutex_);\n>> +\n>> +\t\t/*\n>> +\t\t * Collect events related to the given camera and remove them\n>> +\t\t * from the events_ vector.\n>> +\t\t */\n>> +\n>> +\t\tauto it = events_.begin();\n>> +\t\twhile (it != events_.end()) {\n>> +\t\t\tif (isCameraSpecificEvent(*it, camera)) {\n>> +\t\t\t\tevents.push_back(*it);\n>> +\t\t\t\tit = events_.erase(it);\n>> +\t\t\t} else {\n>> +\t\t\t\tit++;\n>> +\t\t\t}\n>> +\t\t}\n>> +\n>> +\t\tunhandled_size = events_.size();\n>> +\t}\n>> +\n>> +\t/* Convert events to Python events */\n>> +\n>> +\tstd::vector pyevents;\n>> +\n>> +\tfor (const auto &event : events) {\n>> +\t\tPyCameraEvent pyev = convertEvent(event);\n>> +\t\tpyevents.push_back(pyev);\n>> +\t}\n>> +\n>> +\tLOG(Python, Debug) << \"Got \" << pyevents.size() << \" camera events, \"\n>> +\t\t\t << unhandled_size << \" unhandled events left\";\n>> +\n>> +\treturn pyevents;\n>> +}\n>> +\n>> /* Note: Called from another thread */\n>> -void PyCameraManager::handleRequestCompleted(Request *req)\n>> +void PyCameraManager::handleBufferCompleted(std::shared_ptr cam, Request *req, FrameBuffer *fb)\n>> {\n>> -\tpushRequest(req);\n>> -\twriteFd();\n>> +\tCameraEvent ev(CameraEventType::BufferCompleted, cam, req, fb);\n>> +\n>> +\tpushEvent(ev);\n>> +}\n>> +\n>> +/* Note: Called from another thread */\n>> +void PyCameraManager::handleRequestCompleted(std::shared_ptr cam, Request *req)\n>> +{\n>> +\tCameraEvent ev(CameraEventType::RequestCompleted, cam, req);\n>> +\n>> +\tpushEvent(ev);\n>> +}\n>> +\n>> +/* Note: Called from another thread */\n>> +void PyCameraManager::handleDisconnected(std::shared_ptr cam)\n>> +{\n>> +\tCameraEvent ev(CameraEventType::Disconnect, cam);\n>> +\n>> +\tpushEvent(ev);\n>> +}\n>> +\n>> +/* Note: Called from another thread */\n>> +void PyCameraManager::handleCameraAdded(std::shared_ptr cam)\n>> +{\n>> +\tCameraEvent ev(CameraEventType::CameraAdded, cam);\n>> +\n>> +\tpushEvent(ev);\n>> +}\n>> +\n>> +/* Note: Called from another thread */\n>> +void PyCameraManager::handleCameraRemoved(std::shared_ptr cam)\n>> +{\n>> +\tCameraEvent ev(CameraEventType::CameraRemoved, cam);\n>> +\n>> +\tpushEvent(ev);\n>> +}\n>> +\n>> +bool PyCameraManager::getCameraEventFlag(std::shared_ptr camera, CameraEventType event_type)\n>> +{\n>> +\tconst uint32_t evbit = 1 << (uint32_t)event_type;\n>> +\tuint32_t mask = 0;\n>> +\n>> +\tif (auto it = camera_event_masks_.find(camera); it != camera_event_masks_.end())\n> \n> That's a weird style.\n\nWeird how? That's standard c++ style... It prevents the 'it' from \nleaking outside the if block.\n\n> \tauto it = camera_event_masks_.find(camera);\n> \tif (it != camera_event_masks_.end())\n> \n>> +\t\tmask = it->second;\n>> +\n>> +\treturn !!(mask & evbit);\n>> +}\n>> +\n>> +void PyCameraManager::setCameraEventFlag(std::shared_ptr camera, CameraEventType event_type, bool value)\n>> +{\n>> +\tswitch (event_type) {\n>> +\tcase CameraEventType::RequestCompleted:\n>> +\tcase CameraEventType::BufferCompleted:\n>> +\tcase CameraEventType::Disconnect:\n>> +\t\tbreak;\n>> +\n>> +\tdefault:\n>> +\t\tthrow std::runtime_error(\"Bad camera event type\");\n>> +\t}\n>> +\n>> +\tconst uint32_t evbit = 1 << (uint32_t)event_type;\n>> +\tuint32_t mask = 0;\n>> +\n>> +\tif (auto it = camera_event_masks_.find(camera); it != camera_event_masks_.end())\n>> +\t\tmask = it->second;\n> \n> Same here.\n> \n>> +\n>> +\tbool old_val = !!(mask & evbit);\n>> +\n>> +\tif (old_val == value)\n>> +\t\treturn;\n>> +\n>> +\tif (value)\n>> +\t\tmask |= evbit;\n>> +\telse\n>> +\t\tmask &= ~evbit;\n>> +\n>> +\tcamera_event_masks_[camera] = mask;\n>> +\n>> +\tswitch (event_type) {\n>> +\tcase CameraEventType::RequestCompleted:\n>> +\t\tif (value) {\n>> +\t\t\tcamera->requestCompleted.connect(camera.get(), [cm = this->shared_from_this(), camera](Request *req) {\n>> +\t\t\t\tcm->handleRequestCompleted(camera, req);\n>> +\t\t\t});\n>> +\t\t} else {\n>> +\t\t\tcamera->requestCompleted.disconnect();\n>> +\t\t}\n>> +\t\tbreak;\n> \n> This is racy. PyCameraManager::handleRequestCompleted() could be called\n> before you disconnect the signal, with the event then stored in the\n> events_ queue. The application would then receive the event after\n> disabling it.\n\nI don't think it's racy as such, that's \"normal\" for single threaded \napplication. The event could have happened at some point much before \nthis function is even called.\n\nWe could drop here all the events from the event queue for the specific \nevent being disabled. It's perhaps a question of how these \nenable/disable flags are defined.\n\nAre they enabling/disabling the low-level event as such (this is how I \nhave been thinking about it), but they don't affect the event queue \n(i.e. you can get now-disabled events from the event queue).\n\nOr are they enabling/disabling what the get_events() returns.\n\nTo me the former feels more natural, but I can see the logic in the \nlatter option too.\n\n> One option would be to connect the signals unconditionally and instead\n> filter events out when retrieving them in getPyEvents().\n\nThe whole point of these flags is to avoid all the extra allocations and \nstoring of the events that the user doesn't want.\n\n>> +\n>> +\tcase CameraEventType::BufferCompleted:\n>> +\t\tif (value) {\n>> +\t\t\tcamera->bufferCompleted.connect(camera.get(), [cm = this->shared_from_this(), camera](Request *req, FrameBuffer *fb) {\n>> +\t\t\t\tcm->handleBufferCompleted(camera, req, fb);\n>> +\t\t\t});\n>> +\t\t} else {\n>> +\t\t\tcamera->bufferCompleted.disconnect();\n>> +\t\t}\n>> +\t\tbreak;\n>> +\n>> +\tcase CameraEventType::Disconnect:\n>> +\t\tif (value) {\n>> +\t\t\tcamera->disconnected.connect(camera.get(), [cm = this->shared_from_this(), camera]() {\n>> +\t\t\t\tcm->handleDisconnected(camera);\n>> +\t\t\t});\n>> +\t\t} else {\n>> +\t\t\tcamera->disconnected.disconnect();\n>> +\t\t}\n>> +\t\tbreak;\n>> +\tdefault:\n>> +\t\tASSERT(false);\n>> +\t}\n>> }\n>> \n>> void PyCameraManager::writeFd()\n>> @@ -116,16 +353,24 @@ int PyCameraManager::readFd()\n>> \t\treturn -EIO;\n>> }\n>> \n>> -void PyCameraManager::pushRequest(Request *req)\n>> +void PyCameraManager::pushEvent(const CameraEvent &ev)\n>> {\n>> -\tMutexLocker guard(completedRequestsMutex_);\n>> -\tcompletedRequests_.push_back(req);\n>> +\t{\n>> +\t\tMutexLocker guard(eventsMutex_);\n>> +\t\tevents_.push_back(ev);\n>> +\t}\n>> +\n>> +\twriteFd();\n>> +\n>> +\tLOG(Python, Debug) << \"Queued events: \" << events_.size();\n>> }\n>> \n>> -std::vector PyCameraManager::getCompletedRequests()\n>> +std::vector PyCameraManager::getEvents()\n>> {\n>> -\tstd::vector v;\n>> -\tMutexLocker guard(completedRequestsMutex_);\n>> -\tswap(v, completedRequests_);\n>> +\tstd::vector v;\n>> +\n>> +\tMutexLocker guard(eventsMutex_);\n>> +\tswap(v, events_);\n>> +\n>> \treturn v;\n>> }\n>> diff --git a/src/py/libcamera/py_camera_manager.h b/src/py/libcamera/py_camera_manager.h\n>> index 3574db23..31747547 100644\n>> --- a/src/py/libcamera/py_camera_manager.h\n>> +++ b/src/py/libcamera/py_camera_manager.h\n>> @@ -13,12 +13,56 @@\n>> \n>> using namespace libcamera;\n>> \n>> -class PyCameraManager\n>> +enum class CameraEventType {\n>> +\tCameraAdded,\n>> +\tCameraRemoved,\n>> +\tDisconnect,\n>> +\tRequestCompleted,\n>> +\tBufferCompleted,\n>> +};\n>> +\n>> +/*\n>> + * This event struct is used internally to queue the events we receive from\n>> + * other threads.\n>> + */\n>> +struct CameraEvent {\n>> +\tCameraEvent(CameraEventType type, std::shared_ptr camera,\n>> +\t\t Request *request = nullptr, FrameBuffer *fb = nullptr)\n>> +\t\t: type_(type), camera_(camera), request_(request), fb_(fb)\n>> +\t{\n>> +\t}\n>> +\n>> +\tCameraEventType type_;\n>> +\tstd::shared_ptr camera_;\n>> +\tRequest *request_;\n>> +\tFrameBuffer *fb_;\n>> +};\n>> +\n>> +/*\n>> + * This event struct is passed to Python. We need to use pybind11::object here\n>> + * instead of a C++ pointer so that we keep a ref to the Request, and a\n>> + * keep-alive from the camera to the camera manager.\n>> + */\n>> +struct PyCameraEvent {\n>> +\tPyCameraEvent(CameraEventType type, pybind11::object camera)\n>> +\t\t: type_(type), camera_(camera)\n>> +\t{\n>> +\t}\n>> +\n>> +\tCameraEventType type_;\n>> +\tpybind11::object camera_;\n>> +\tpybind11::object request_;\n>> +\tpybind11::object fb_;\n>> +};\n>> +\n>> +class PyCameraManager : public std::enable_shared_from_this\n>> {\n>> public:\n>> \tPyCameraManager();\n>> \t~PyCameraManager();\n>> \n>> +\tvoid init();\n>> +\n>> \tpybind11::list cameras();\n>> \tstd::shared_ptr get(const std::string &name) { return cameraManager_->get(name); }\n>> \n>> @@ -26,20 +70,33 @@ public:\n>> \n>> \tint eventFd() const { return eventFd_.get(); }\n>> \n>> -\tstd::vector getReadyRequests();\n>> +\tstd::vector getReadyRequests(); /* DEPRECATED */\n>> +\tstd::vector getPyEvents();\n>> +\tstd::vector getPyCameraEvents(std::shared_ptr camera);\n>> +\n>> +\tvoid handleBufferCompleted(std::shared_ptr cam, Request *req, FrameBuffer *fb);\n>> +\tvoid handleRequestCompleted(std::shared_ptr cam, Request *req);\n>> +\tvoid handleDisconnected(std::shared_ptr cam);\n>> +\tvoid handleCameraAdded(std::shared_ptr cam);\n>> +\tvoid handleCameraRemoved(std::shared_ptr cam);\n>> \n>> -\tvoid handleRequestCompleted(Request *req);\n>> +\tbool getCameraEventFlag(std::shared_ptr camera, CameraEventType event_type);\n>> +\tvoid setCameraEventFlag(std::shared_ptr camera, CameraEventType event_type, bool value);\n>> \n>> private:\n>> \tstd::unique_ptr cameraManager_;\n>> \n>> \tUniqueFD eventFd_;\n>> -\tlibcamera::Mutex completedRequestsMutex_;\n>> -\tstd::vector completedRequests_\n>> -\t\tLIBCAMERA_TSA_GUARDED_BY(completedRequestsMutex_);\n>> +\tlibcamera::Mutex eventsMutex_;\n>> +\tstd::vector events_\n>> +\t\tLIBCAMERA_TSA_GUARDED_BY(eventsMutex_);\n>> \n>> \tvoid writeFd();\n>> \tint readFd();\n>> -\tvoid pushRequest(Request *req);\n>> -\tstd::vector getCompletedRequests();\n>> +\tvoid pushEvent(const CameraEvent &ev);\n>> +\tstd::vector getEvents();\n>> +\n>> +\tPyCameraEvent convertEvent(const CameraEvent &event);\n>> +\n>> +\tstd::map, uint32_t> camera_event_masks_;\n>> };\n>> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n>> index 5a5f1a37..981a3070 100644\n>> --- a/src/py/libcamera/py_main.cpp\n>> +++ b/src/py/libcamera/py_main.cpp\n>> @@ -110,6 +110,7 @@ PYBIND11_MODULE(_libcamera, m)\n>> \t * https://pybind11.readthedocs.io/en/latest/advanced/misc.html#avoiding-c-types-in-docstrings\n>> \t */\n>> \n>> +\tauto pyEvent = py::class_(m, \"Event\");\n>> \tauto pyCameraManager = py::class_>(m, \"CameraManager\");\n>> \tauto pyCamera = py::class_>(m, \"Camera\");\n>> \tauto pyCameraConfiguration = py::class_(m, \"CameraConfiguration\");\n>> @@ -136,12 +137,27 @@ PYBIND11_MODULE(_libcamera, m)\n>> \tm.def(\"log_set_level\", &logSetLevel);\n>> \n>> \t/* Classes */\n>> +\n>> +\tpy::enum_(pyEvent, \"Type\")\n>> +\t\t.value(\"CameraAdded\", CameraEventType::CameraAdded)\n>> +\t\t.value(\"CameraRemoved\", CameraEventType::CameraRemoved)\n>> +\t\t.value(\"Disconnect\", CameraEventType::Disconnect)\n>> +\t\t.value(\"RequestCompleted\", CameraEventType::RequestCompleted)\n>> +\t\t.value(\"BufferCompleted\", CameraEventType::BufferCompleted);\n>> +\n>> +\tpyEvent\n>> +\t\t.def_readonly(\"type\", &PyCameraEvent::type_)\n>> +\t\t.def_readonly(\"camera\", &PyCameraEvent::camera_)\n>> +\t\t.def_readonly(\"request\", &PyCameraEvent::request_)\n>> +\t\t.def_readonly(\"fb\", &PyCameraEvent::fb_);\n>> +\n>> \tpyCameraManager\n>> \t\t.def_static(\"singleton\", []() {\n>> \t\t\tstd::shared_ptr cm = gCameraManager.lock();\n>> \n>> \t\t\tif (!cm) {\n>> \t\t\t\tcm = std::make_shared();\n>> +\t\t\t\tcm->init();\n>> \t\t\t\tgCameraManager = cm;\n>> \t\t\t}\n>> \n>> @@ -153,10 +169,33 @@ PYBIND11_MODULE(_libcamera, m)\n>> \t\t.def_property_readonly(\"cameras\", &PyCameraManager::cameras)\n>> \n>> \t\t.def_property_readonly(\"event_fd\", &PyCameraManager::eventFd)\n>> -\t\t.def(\"get_ready_requests\", &PyCameraManager::getReadyRequests);\n>> +\n>> +\t\t/* DEPRECATED */\n>> +\t\t.def(\"get_ready_requests\", &PyCameraManager::getReadyRequests)\n>> +\n>> +\t\t.def(\"get_events\", &PyCameraManager::getPyEvents);\n>> \n>> \tpyCamera\n>> \t\t.def_property_readonly(\"id\", &Camera::id)\n>> +\n>> +\t\t.def(\"get_camera_event_enabled\",\n>> +\t\t\t[](Camera &self, CameraEventType type) {\n>> +\t\t\t\tauto cm = gCameraManager.lock();\n>> +\t\t\t\treturn cm->getCameraEventFlag(self.shared_from_this(), type);\n>> +\t\t\t})\n>> +\n>> +\t\t.def(\"enable_camera_event\",\n>> +\t\t\t[](Camera &self, CameraEventType type) {\n>> +\t\t\t\tauto cm = gCameraManager.lock();\n>> +\t\t\t\tcm->setCameraEventFlag(self.shared_from_this(), type, true);\n>> +\t\t\t})\n>> +\n>> +\t\t.def(\"disable_camera_event\",\n>> +\t\t\t[](Camera &self, CameraEventType type) {\n>> +\t\t\t\tauto cm = gCameraManager.lock();\n>> +\t\t\t\tcm->setCameraEventFlag(self.shared_from_this(), type, false);\n>> +\t\t\t})\n>> +\n>> \t\t.def(\"acquire\", [](Camera &self) {\n>> \t\t\tint ret = self.acquire();\n>> \t\t\tif (ret)\n>> @@ -173,11 +212,6 @@ PYBIND11_MODULE(_libcamera, m)\n>> \t\t const std::unordered_map &controls) {\n>> \t\t\t/* \\todo What happens if someone calls start() multiple times? */\n>> \n>> -\t\t\tauto cm = gCameraManager.lock();\n>> -\t\t\tASSERT(cm);\n>> -\n>> -\t\t\tself.requestCompleted.connect(cm.get(), &PyCameraManager::handleRequestCompleted);\n>> -\n>> \t\t\tControlList controlList(self.controls());\n>> \n>> \t\t\tfor (const auto &[id, obj] : controls) {\n>> @@ -187,7 +221,6 @@ PYBIND11_MODULE(_libcamera, m)\n>> \n>> \t\t\tint ret = self.start(&controlList);\n>> \t\t\tif (ret) {\n>> -\t\t\t\tself.requestCompleted.disconnect();\n>> \t\t\t\tthrow std::system_error(-ret, std::generic_category(),\n>> \t\t\t\t\t\t\t\"Failed to start camera\");\n>> \t\t\t}\n> \n> You can drop the curly braces.\n\nIndeed.\n\n>> @@ -196,11 +229,16 @@ PYBIND11_MODULE(_libcamera, m)\n>> \t\t.def(\"stop\", [](Camera &self) {\n>> \t\t\tint ret = self.stop();\n>> \n>> -\t\t\tself.requestCompleted.disconnect();\n>> +\t\t\tauto cm = gCameraManager.lock();\n>> +\t\t\tASSERT(cm);\n>> +\n>> +\t\t\tauto events = cm->getPyCameraEvents(self.shared_from_this());\n>> \n>> \t\t\tif (ret)\n>> \t\t\t\tthrow std::system_error(-ret, std::generic_category(),\n>> \t\t\t\t\t\t\t\"Failed to stop camera\");\n>> +\n>> +\t\t\treturn events;\n>> \t\t})\n>> \n>> \t\t.def(\"__str__\", [](Camera &self) {\n> \n\n Tomi","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 288A0C31E9\n\tfor ;\n\tMon, 5 Jun 2023 07:31:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5ED936287F;\n\tMon, 5 Jun 2023 09:31:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1579B61EA2\n\tfor ;\n\tMon, 5 Jun 2023 09:31:04 +0200 (CEST)","from [192.168.88.20] (91-154-35-171.elisa-laajakaista.fi\n\t[91.154.35.171])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 43D962BC;\n\tMon, 5 Jun 2023 09:30:39 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685950266;\n\tbh=rZeE3foX7L/GGSuYM8shWYNrrBN1npzoc63r99Gr80c=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=TUrd5TKL+3rsclJrkgQZWPoT37ViWCU4/yjvp0hDqtV9PeI0GDXZaaip+S61wj/br\n\tSRMwo9gDE7aoQrhPShFWGE+dhyIJ8nU1YcgXI7Qs2LPVT+yVnrRuUO3B9Ok2Caxf3J\n\tquhQLbnwJxgLdurXMfLdCld8s6om3SW7JLWfeN4JzTmh8PzLUylD1BqoLBm+Zep0x0\n\ttH9MIcFP+BNW215MxAPOkG5vw4Es9953L4oW2mgSplZ6jAUmFkTad8xw4tsL4w5Abe\n\tgawMLLz4WS3k0secYB0AJNygrnhPjIigTXAPZDQFJyjykxWFPEFSsKAXguoBff396Y\n\t3GoxzxrW1a6zA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685950239;\n\tbh=rZeE3foX7L/GGSuYM8shWYNrrBN1npzoc63r99Gr80c=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=vxGfz7ZTDVCGC1wf2pdqXhwGxJVU3h0SdYIVKwUUdUx/Z8QE4nHA0vhmvGICEF5wN\n\tibUy/u3gYT1fsCSNyeSseeqPMcG/bivbKcXszExR9RTTk0uxMcvM5c2XfggNA9Mpal\n\t3NmbrVbsb9JXcnZi9d/XROmGozmVIrZ0708Kw7L8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"vxGfz7ZT\"; dkim-atps=neutral","Message-ID":"<51586616-f621-7fb8-1a53-86773aa969e4@ideasonboard.com>","Date":"Mon, 5 Jun 2023 10:31:00 +0300","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.11.0","To":"Laurent Pinchart ","References":"<20230603075615.20663-1-tomi.valkeinen@ideasonboard.com>\n\t<20230603075615.20663-4-tomi.valkeinen@ideasonboard.com>\n\t<20230605064750.GF22604@pendragon.ideasonboard.com>","Content-Language":"en-US","In-Reply-To":"<20230605064750.GF22604@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v5 03/13] py: New event handling","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Tomi Valkeinen via libcamera-devel\n\t","Reply-To":"Tomi Valkeinen ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27246,"web_url":"https://patchwork.libcamera.org/comment/27246/","msgid":"<20230605075054.GA30841@pendragon.ideasonboard.com>","date":"2023-06-05T07:50:54","subject":"Re: [libcamera-devel] [PATCH v5 03/13] py: New event handling","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Tomi,\n\nOn Mon, Jun 05, 2023 at 10:31:00AM +0300, Tomi Valkeinen wrote:\n> On 05/06/2023 09:47, Laurent Pinchart wrote:\n> > On Sat, Jun 03, 2023 at 10:56:05AM +0300, Tomi Valkeinen wrote:\n> >> At the moment the Python bindings only handle the requestCompleted\n> >> events. But we have others, bufferCompleted and disconnect from the\n> >> Camera, and cameraAdded and cameraRemoved from the CameraManager.\n> >>\n> >> This patch makes all those events available to the users.\n> >>\n> >> The get_ready_requests() method is now deprecated, but available to keep\n> >> backward compatibility.\n> >>\n> >> The new event handling works like get_ready_requests(), but instead of\n> >> returning only Requests from requestCompleted events, it returns all\n> >> event types using a new Event class.\n> >>\n> >> Additionally camera.stop() has been changed to return events for that\n> >> specific camera. This serves two purposes: first, it removes the\n> >> RequestCompleted and BufferCompleted events from the event queue, thus\n> >> preventing the old events being returned when the camera is started\n> >> again, and second, it allows the user to process those events if it so\n> >> wants.\n> > \n> > Returning events from stop() is a bit of a weird API, and I'm a bit\n> > worried it will confuse users, but I don't have any better option to\n> > propose. I think it's good enough for now.\n> \n> It was added to solve the reported user confusion. So... Confusing both \n> ways? =).\n> \n> It's, of course, not a must. The user can get the events the normal way, \n> thus flushing the event queue.\n> \n> But I agree that it's a bit weird, and I kind of would like to drop it. \n> Maybe just documenting it clearly that one needs to take care of the \n> events after calling stop would be enough?\n\nIt would be great if people didn't ignore documentation :-)\n\n> >> The CameraManager events (CameraAdded and CameraRemoved) are always\n> >> enabled, but of the Camera events only RequestCompleted is enabled by\n> >> default. Other Camera events can be enabled\n> >> Camera.enable_camera_event().\n> >>\n> >> Signed-off-by: Tomi Valkeinen \n> >> ---\n> > \n> > Note for the future, per-patch changelogs would be useful.\n> \n> I have never learned to use those... Maybe it's time.\n\nYes please :-) I've meant to ask you for a long time already. It doesn't\nhave to be difficult, you can just record the changes every time you do\na `git commit --amend`.\n\n> >> Documentation/python-bindings.rst | 24 ++-\n> >> src/py/libcamera/py_camera_manager.cpp | 275 +++++++++++++++++++++++--\n> >> src/py/libcamera/py_camera_manager.h | 73 ++++++-\n> >> src/py/libcamera/py_main.cpp | 54 ++++-\n> >> 4 files changed, 386 insertions(+), 40 deletions(-)\n> >>\n> >> diff --git a/Documentation/python-bindings.rst b/Documentation/python-bindings.rst\n> >> index bac5cd34..8f2d76c3 100644\n> >> --- a/Documentation/python-bindings.rst\n> >> +++ b/Documentation/python-bindings.rst\n> >> @@ -43,17 +43,23 @@ CameraManager\n> >> The Python API provides a singleton CameraManager via ``CameraManager.singleton()``.\n> >> There is no need to start or stop the CameraManager.\n> >> \n> >> -Handling Completed Requests\n> >> ----------------------------\n> >> +Event Handling\n> >> +--------------\n> >> \n> >> -The Python bindings do not expose the ``Camera::requestCompleted`` signal\n> >> -directly as the signal is invoked from another thread and it has real-time\n> >> -constraints. Instead the bindings queue the completed requests internally and\n> >> -use an eventfd to inform the user that there are completed requests.\n> >> +The Python bindings do not expose the signals from the C++ side directly as the\n> >> +signals are invoked from another thread and they may have real-time\n> >> +constraints. Instead the bindings queue the received events internally and use\n> >> +an eventfd to inform the user that there are events to be handled.\n> >> \n> >> -The user can wait on the eventfd, and upon getting an event, use\n> >> -``CameraManager.get_ready_requests()`` to clear the eventfd event and to get\n> >> -the completed requests.\n> >> +The user can wait on the eventfd (e.g. by using Python Selector), and use\n> >> +``CameraManager.get_events()`` to reset the eventfd and get the events.\n> >> +\n> >> +The CameraManager events (CameraAdded and CameraRemoved) are always enabled, but\n> >> +of the Camera events only RequestCompleted is enabled by default. To enable\n> >> +Disconnect or BufferCompleted event, use ``Camera.enable_camera_event()``.\n> >> +\n> >> +The ``Camera.stop()`` method will return all events related to that Camera from\n> >> +the event queue.\n> >> \n> >> Controls & Properties\n> >> ---------------------\n> >> diff --git a/src/py/libcamera/py_camera_manager.cpp b/src/py/libcamera/py_camera_manager.cpp\n> >> index 9ccb7aad..83c2b063 100644\n> >> --- a/src/py/libcamera/py_camera_manager.cpp\n> >> +++ b/src/py/libcamera/py_camera_manager.cpp\n> >> @@ -5,6 +5,7 @@\n> >> \n> >> #include \"py_camera_manager.h\"\n> >> \n> >> +#include \n> >> #include \n> >> #include \n> >> #include \n> >> @@ -35,11 +36,24 @@ PyCameraManager::PyCameraManager()\n> >> \tif (ret)\n> >> \t\tthrow std::system_error(-ret, std::generic_category(),\n> >> \t\t\t\t\t\"Failed to start CameraManager\");\n> >> +\n> >> +\tcameraManager_->cameraAdded.connect(this, &PyCameraManager::handleCameraAdded);\n> >> +\tcameraManager_->cameraRemoved.connect(this, &PyCameraManager::handleCameraRemoved);\n> >> }\n> >> \n> >> PyCameraManager::~PyCameraManager()\n> >> {\n> >> \tLOG(Python, Debug) << \"~PyCameraManager()\";\n> >> +\n> >> +\tcameraManager_->cameraAdded.disconnect();\n> >> +\tcameraManager_->cameraRemoved.disconnect();\n> >> +}\n> >> +\n> >> +void PyCameraManager::init()\n> >> +{\n> >> +\t/* Always enable RequestCompleted for all cameras by default */\n> >> +\tfor (auto cam : cameraManager_->cameras())\n> >> +\t\tsetCameraEventFlag(cam, CameraEventType::RequestCompleted, true);\n> > \n> > You need to also enable the event when a new camera is detected.\n> \n> Ah, good point. But I also wonder, should this just be dropped too? I'm \n> not sure if the return-events-from-stop and this one are just hacks that \n> seemingly make life easier for the users, but in the end just add \n> confusion for both the user and in the codebase. Or are they actually \n> good features...\n\nWould enabling all events unconditionally have a big impact on\nperformance ?\n\n> >> }\n> >> \n> >> py::list PyCameraManager::cameras()\n> >> @@ -60,6 +74,43 @@ py::list PyCameraManager::cameras()\n> >> \treturn l;\n> >> }\n> >> \n> >> +PyCameraEvent PyCameraManager::convertEvent(const CameraEvent &event)\n> >> +{\n> >> +\t/*\n> >> +\t * We need to set a keep-alive here so that the camera keeps the\n> >> +\t * camera manager alive.\n> >> +\t */\n> >> +\tpy::object py_cm = py::cast(this);\n> >> +\tpy::object py_cam = py::cast(event.camera_);\n> >> +\tpy::detail::keep_alive_impl(py_cam, py_cm);\n> >> +\n> >> +\tPyCameraEvent pyevent(event.type_, py_cam);\n> >> +\n> >> +\tswitch (event.type_) {\n> >> +\tcase CameraEventType::CameraAdded:\n> >> +\tcase CameraEventType::CameraRemoved:\n> >> +\tcase CameraEventType::Disconnect:\n> >> +\t\t/* No additional parameters to add */\n> >> +\t\tbreak;\n> >> +\n> >> +\tcase CameraEventType::BufferCompleted:\n> >> +\t\tpyevent.request_ = py::cast(event.request_);\n> >> +\t\tpyevent.fb_ = py::cast(event.fb_);\n> >> +\t\tbreak;\n> >> +\n> >> +\tcase CameraEventType::RequestCompleted:\n> >> +\t\tpyevent.request_ = py::cast(event.request_);\n> >> +\n> >> +\t\t/* Decrease the ref increased in Camera.queue_request() */\n> >> +\t\tpyevent.request_.dec_ref();\n> >> +\n> >> +\t\tbreak;\n> >> +\t}\n> >> +\n> >> +\treturn pyevent;\n> >> +}\n> >> +\n> >> +/* DEPRECATED */\n> >> std::vector PyCameraManager::getReadyRequests()\n> >> {\n> >> \tint ret = readFd();\n> >> @@ -72,21 +123,207 @@ std::vector PyCameraManager::getReadyRequests()\n> >> \n> >> \tstd::vector py_reqs;\n> >> \n> >> -\tfor (Request *request : getCompletedRequests()) {\n> >> -\t\tpy::object o = py::cast(request);\n> >> -\t\t/* Decrease the ref increased in Camera.queue_request() */\n> >> -\t\to.dec_ref();\n> >> -\t\tpy_reqs.push_back(o);\n> >> +\tfor (const auto &ev : getEvents()) {\n> >> +\t\tif (ev.type_ != CameraEventType::RequestCompleted)\n> >> +\t\t\tcontinue;\n> >> +\n> >> +\t\tPyCameraEvent pyev = convertEvent(ev);\n> >> +\t\tpy_reqs.push_back(pyev.request_);\n> >> \t}\n> >> \n> >> \treturn py_reqs;\n> >> }\n> >> \n> >> +std::vector PyCameraManager::getPyEvents()\n> >> +{\n> >> +\tint ret = readFd();\n> >> +\n> >> +\tif (ret == EAGAIN) {\n> >> +\t\tLOG(Python, Debug) << \"No events\";\n> >> +\t\treturn {};\n> >> +\t}\n> >> +\n> >> +\tif (ret != 0)\n> >> +\t\tthrow std::system_error(ret, std::generic_category());\n> >> +\n> >> +\tstd::vector events = getEvents();\n> >> +\n> >> +\tLOG(Python, Debug) << \"Got \" << events.size() << \" events\";\n> >> +\n> >> +\tstd::vector pyevents;\n> >> +\tpyevents.reserve(events.size());\n> >> +\n> >> +\tstd::transform(events.begin(), events.end(), std::back_inserter(pyevents),\n> >> +\t\t [this](const CameraEvent &ev) {\n> >> +\t\t\t return convertEvent(ev);\n> >> +\t\t });\n> >> +\n> >> +\treturn pyevents;\n> >> +}\n> >> +\n> >> +static bool isCameraSpecificEvent(const CameraEvent &event, std::shared_ptr &camera)\n> >> +{\n> >> +\treturn event.camera_ == camera &&\n> >> +\t (event.type_ == CameraEventType::RequestCompleted ||\n> >> +\t\tevent.type_ == CameraEventType::BufferCompleted ||\n> >> +\t\tevent.type_ == CameraEventType::Disconnect);\n> >> +}\n> >> +\n> >> +std::vector PyCameraManager::getPyCameraEvents(std::shared_ptr camera)\n> >> +{\n> >> +\tstd::vector events;\n> >> +\tsize_t unhandled_size;\n> >> +\n> >> +\t{\n> >> +\t\tMutexLocker guard(eventsMutex_);\n> >> +\n> >> +\t\t/*\n> >> +\t\t * Collect events related to the given camera and remove them\n> >> +\t\t * from the events_ vector.\n> >> +\t\t */\n> >> +\n> >> +\t\tauto it = events_.begin();\n> >> +\t\twhile (it != events_.end()) {\n> >> +\t\t\tif (isCameraSpecificEvent(*it, camera)) {\n> >> +\t\t\t\tevents.push_back(*it);\n> >> +\t\t\t\tit = events_.erase(it);\n> >> +\t\t\t} else {\n> >> +\t\t\t\tit++;\n> >> +\t\t\t}\n> >> +\t\t}\n> >> +\n> >> +\t\tunhandled_size = events_.size();\n> >> +\t}\n> >> +\n> >> +\t/* Convert events to Python events */\n> >> +\n> >> +\tstd::vector pyevents;\n> >> +\n> >> +\tfor (const auto &event : events) {\n> >> +\t\tPyCameraEvent pyev = convertEvent(event);\n> >> +\t\tpyevents.push_back(pyev);\n> >> +\t}\n> >> +\n> >> +\tLOG(Python, Debug) << \"Got \" << pyevents.size() << \" camera events, \"\n> >> +\t\t\t << unhandled_size << \" unhandled events left\";\n> >> +\n> >> +\treturn pyevents;\n> >> +}\n> >> +\n> >> /* Note: Called from another thread */\n> >> -void PyCameraManager::handleRequestCompleted(Request *req)\n> >> +void PyCameraManager::handleBufferCompleted(std::shared_ptr cam, Request *req, FrameBuffer *fb)\n> >> {\n> >> -\tpushRequest(req);\n> >> -\twriteFd();\n> >> +\tCameraEvent ev(CameraEventType::BufferCompleted, cam, req, fb);\n> >> +\n> >> +\tpushEvent(ev);\n> >> +}\n> >> +\n> >> +/* Note: Called from another thread */\n> >> +void PyCameraManager::handleRequestCompleted(std::shared_ptr cam, Request *req)\n> >> +{\n> >> +\tCameraEvent ev(CameraEventType::RequestCompleted, cam, req);\n> >> +\n> >> +\tpushEvent(ev);\n> >> +}\n> >> +\n> >> +/* Note: Called from another thread */\n> >> +void PyCameraManager::handleDisconnected(std::shared_ptr cam)\n> >> +{\n> >> +\tCameraEvent ev(CameraEventType::Disconnect, cam);\n> >> +\n> >> +\tpushEvent(ev);\n> >> +}\n> >> +\n> >> +/* Note: Called from another thread */\n> >> +void PyCameraManager::handleCameraAdded(std::shared_ptr cam)\n> >> +{\n> >> +\tCameraEvent ev(CameraEventType::CameraAdded, cam);\n> >> +\n> >> +\tpushEvent(ev);\n> >> +}\n> >> +\n> >> +/* Note: Called from another thread */\n> >> +void PyCameraManager::handleCameraRemoved(std::shared_ptr cam)\n> >> +{\n> >> +\tCameraEvent ev(CameraEventType::CameraRemoved, cam);\n> >> +\n> >> +\tpushEvent(ev);\n> >> +}\n> >> +\n> >> +bool PyCameraManager::getCameraEventFlag(std::shared_ptr camera, CameraEventType event_type)\n> >> +{\n> >> +\tconst uint32_t evbit = 1 << (uint32_t)event_type;\n> >> +\tuint32_t mask = 0;\n> >> +\n> >> +\tif (auto it = camera_event_masks_.find(camera); it != camera_event_masks_.end())\n> > \n> > That's a weird style.\n> \n> Weird how? That's standard c++ style... It prevents the 'it' from \n> leaking outside the if block.\n\ns/weird/unusual in libcamera/\n\n> > \tauto it = camera_event_masks_.find(camera);\n> > \tif (it != camera_event_masks_.end())\n> > \n> >> +\t\tmask = it->second;\n> >> +\n> >> +\treturn !!(mask & evbit);\n> >> +}\n> >> +\n> >> +void PyCameraManager::setCameraEventFlag(std::shared_ptr camera, CameraEventType event_type, bool value)\n> >> +{\n> >> +\tswitch (event_type) {\n> >> +\tcase CameraEventType::RequestCompleted:\n> >> +\tcase CameraEventType::BufferCompleted:\n> >> +\tcase CameraEventType::Disconnect:\n> >> +\t\tbreak;\n> >> +\n> >> +\tdefault:\n> >> +\t\tthrow std::runtime_error(\"Bad camera event type\");\n> >> +\t}\n> >> +\n> >> +\tconst uint32_t evbit = 1 << (uint32_t)event_type;\n> >> +\tuint32_t mask = 0;\n> >> +\n> >> +\tif (auto it = camera_event_masks_.find(camera); it != camera_event_masks_.end())\n> >> +\t\tmask = it->second;\n> > \n> > Same here.\n> > \n> >> +\n> >> +\tbool old_val = !!(mask & evbit);\n> >> +\n> >> +\tif (old_val == value)\n> >> +\t\treturn;\n> >> +\n> >> +\tif (value)\n> >> +\t\tmask |= evbit;\n> >> +\telse\n> >> +\t\tmask &= ~evbit;\n> >> +\n> >> +\tcamera_event_masks_[camera] = mask;\n> >> +\n> >> +\tswitch (event_type) {\n> >> +\tcase CameraEventType::RequestCompleted:\n> >> +\t\tif (value) {\n> >> +\t\t\tcamera->requestCompleted.connect(camera.get(), [cm = this->shared_from_this(), camera](Request *req) {\n> >> +\t\t\t\tcm->handleRequestCompleted(camera, req);\n> >> +\t\t\t});\n> >> +\t\t} else {\n> >> +\t\t\tcamera->requestCompleted.disconnect();\n> >> +\t\t}\n> >> +\t\tbreak;\n> > \n> > This is racy. PyCameraManager::handleRequestCompleted() could be called\n> > before you disconnect the signal, with the event then stored in the\n> > events_ queue. The application would then receive the event after\n> > disabling it.\n> \n> I don't think it's racy as such, that's \"normal\" for single threaded \n> application. The event could have happened at some point much before \n> this function is even called.\n\nConsider the following code flow:\n\n\tcm.enable_camera_event(libcamera.Event.BufferCompleted)\n\n\tfor event in cm.get_events():\n\t\tif event.type == libcamera.Event.BufferCompleted:\n\t\t\traise RuntimeError(\"We have disabled this!\")\n\nShould a user really expect this to raise an exception ?\n\n> We could drop here all the events from the event queue for the specific \n> event being disabled. It's perhaps a question of how these \n> enable/disable flags are defined.\n> \n> Are they enabling/disabling the low-level event as such (this is how I \n> have been thinking about it), but they don't affect the event queue \n> (i.e. you can get now-disabled events from the event queue).\n> \n> Or are they enabling/disabling what the get_events() returns.\n\nWhichever option we pick, it has to be clearly documented.\n\n> To me the former feels more natural, but I can see the logic in the \n> latter option too.\n> \n> > One option would be to connect the signals unconditionally and instead\n> > filter events out when retrieving them in getPyEvents().\n> \n> The whole point of these flags is to avoid all the extra allocations and \n> storing of the events that the user doesn't want.\n\nWhat's the performance impact, are we over-engineering something here ?\n\n> >> +\n> >> +\tcase CameraEventType::BufferCompleted:\n> >> +\t\tif (value) {\n> >> +\t\t\tcamera->bufferCompleted.connect(camera.get(), [cm = this->shared_from_this(), camera](Request *req, FrameBuffer *fb) {\n> >> +\t\t\t\tcm->handleBufferCompleted(camera, req, fb);\n> >> +\t\t\t});\n> >> +\t\t} else {\n> >> +\t\t\tcamera->bufferCompleted.disconnect();\n> >> +\t\t}\n> >> +\t\tbreak;\n> >> +\n> >> +\tcase CameraEventType::Disconnect:\n> >> +\t\tif (value) {\n> >> +\t\t\tcamera->disconnected.connect(camera.get(), [cm = this->shared_from_this(), camera]() {\n> >> +\t\t\t\tcm->handleDisconnected(camera);\n> >> +\t\t\t});\n> >> +\t\t} else {\n> >> +\t\t\tcamera->disconnected.disconnect();\n> >> +\t\t}\n> >> +\t\tbreak;\n> >> +\tdefault:\n> >> +\t\tASSERT(false);\n> >> +\t}\n> >> }\n> >> \n> >> void PyCameraManager::writeFd()\n> >> @@ -116,16 +353,24 @@ int PyCameraManager::readFd()\n> >> \t\treturn -EIO;\n> >> }\n> >> \n> >> -void PyCameraManager::pushRequest(Request *req)\n> >> +void PyCameraManager::pushEvent(const CameraEvent &ev)\n> >> {\n> >> -\tMutexLocker guard(completedRequestsMutex_);\n> >> -\tcompletedRequests_.push_back(req);\n> >> +\t{\n> >> +\t\tMutexLocker guard(eventsMutex_);\n> >> +\t\tevents_.push_back(ev);\n> >> +\t}\n> >> +\n> >> +\twriteFd();\n> >> +\n> >> +\tLOG(Python, Debug) << \"Queued events: \" << events_.size();\n> >> }\n> >> \n> >> -std::vector PyCameraManager::getCompletedRequests()\n> >> +std::vector PyCameraManager::getEvents()\n> >> {\n> >> -\tstd::vector v;\n> >> -\tMutexLocker guard(completedRequestsMutex_);\n> >> -\tswap(v, completedRequests_);\n> >> +\tstd::vector v;\n> >> +\n> >> +\tMutexLocker guard(eventsMutex_);\n> >> +\tswap(v, events_);\n> >> +\n> >> \treturn v;\n> >> }\n> >> diff --git a/src/py/libcamera/py_camera_manager.h b/src/py/libcamera/py_camera_manager.h\n> >> index 3574db23..31747547 100644\n> >> --- a/src/py/libcamera/py_camera_manager.h\n> >> +++ b/src/py/libcamera/py_camera_manager.h\n> >> @@ -13,12 +13,56 @@\n> >> \n> >> using namespace libcamera;\n> >> \n> >> -class PyCameraManager\n> >> +enum class CameraEventType {\n> >> +\tCameraAdded,\n> >> +\tCameraRemoved,\n> >> +\tDisconnect,\n> >> +\tRequestCompleted,\n> >> +\tBufferCompleted,\n> >> +};\n> >> +\n> >> +/*\n> >> + * This event struct is used internally to queue the events we receive from\n> >> + * other threads.\n> >> + */\n> >> +struct CameraEvent {\n> >> +\tCameraEvent(CameraEventType type, std::shared_ptr camera,\n> >> +\t\t Request *request = nullptr, FrameBuffer *fb = nullptr)\n> >> +\t\t: type_(type), camera_(camera), request_(request), fb_(fb)\n> >> +\t{\n> >> +\t}\n> >> +\n> >> +\tCameraEventType type_;\n> >> +\tstd::shared_ptr camera_;\n> >> +\tRequest *request_;\n> >> +\tFrameBuffer *fb_;\n> >> +};\n> >> +\n> >> +/*\n> >> + * This event struct is passed to Python. We need to use pybind11::object here\n> >> + * instead of a C++ pointer so that we keep a ref to the Request, and a\n> >> + * keep-alive from the camera to the camera manager.\n> >> + */\n> >> +struct PyCameraEvent {\n> >> +\tPyCameraEvent(CameraEventType type, pybind11::object camera)\n> >> +\t\t: type_(type), camera_(camera)\n> >> +\t{\n> >> +\t}\n> >> +\n> >> +\tCameraEventType type_;\n> >> +\tpybind11::object camera_;\n> >> +\tpybind11::object request_;\n> >> +\tpybind11::object fb_;\n> >> +};\n> >> +\n> >> +class PyCameraManager : public std::enable_shared_from_this\n> >> {\n> >> public:\n> >> \tPyCameraManager();\n> >> \t~PyCameraManager();\n> >> \n> >> +\tvoid init();\n> >> +\n> >> \tpybind11::list cameras();\n> >> \tstd::shared_ptr get(const std::string &name) { return cameraManager_->get(name); }\n> >> \n> >> @@ -26,20 +70,33 @@ public:\n> >> \n> >> \tint eventFd() const { return eventFd_.get(); }\n> >> \n> >> -\tstd::vector getReadyRequests();\n> >> +\tstd::vector getReadyRequests(); /* DEPRECATED */\n> >> +\tstd::vector getPyEvents();\n> >> +\tstd::vector getPyCameraEvents(std::shared_ptr camera);\n> >> +\n> >> +\tvoid handleBufferCompleted(std::shared_ptr cam, Request *req, FrameBuffer *fb);\n> >> +\tvoid handleRequestCompleted(std::shared_ptr cam, Request *req);\n> >> +\tvoid handleDisconnected(std::shared_ptr cam);\n> >> +\tvoid handleCameraAdded(std::shared_ptr cam);\n> >> +\tvoid handleCameraRemoved(std::shared_ptr cam);\n> >> \n> >> -\tvoid handleRequestCompleted(Request *req);\n> >> +\tbool getCameraEventFlag(std::shared_ptr camera, CameraEventType event_type);\n> >> +\tvoid setCameraEventFlag(std::shared_ptr camera, CameraEventType event_type, bool value);\n> >> \n> >> private:\n> >> \tstd::unique_ptr cameraManager_;\n> >> \n> >> \tUniqueFD eventFd_;\n> >> -\tlibcamera::Mutex completedRequestsMutex_;\n> >> -\tstd::vector completedRequests_\n> >> -\t\tLIBCAMERA_TSA_GUARDED_BY(completedRequestsMutex_);\n> >> +\tlibcamera::Mutex eventsMutex_;\n> >> +\tstd::vector events_\n> >> +\t\tLIBCAMERA_TSA_GUARDED_BY(eventsMutex_);\n> >> \n> >> \tvoid writeFd();\n> >> \tint readFd();\n> >> -\tvoid pushRequest(Request *req);\n> >> -\tstd::vector getCompletedRequests();\n> >> +\tvoid pushEvent(const CameraEvent &ev);\n> >> +\tstd::vector getEvents();\n> >> +\n> >> +\tPyCameraEvent convertEvent(const CameraEvent &event);\n> >> +\n> >> +\tstd::map, uint32_t> camera_event_masks_;\n> >> };\n> >> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n> >> index 5a5f1a37..981a3070 100644\n> >> --- a/src/py/libcamera/py_main.cpp\n> >> +++ b/src/py/libcamera/py_main.cpp\n> >> @@ -110,6 +110,7 @@ PYBIND11_MODULE(_libcamera, m)\n> >> \t * https://pybind11.readthedocs.io/en/latest/advanced/misc.html#avoiding-c-types-in-docstrings\n> >> \t */\n> >> \n> >> +\tauto pyEvent = py::class_(m, \"Event\");\n> >> \tauto pyCameraManager = py::class_>(m, \"CameraManager\");\n> >> \tauto pyCamera = py::class_>(m, \"Camera\");\n> >> \tauto pyCameraConfiguration = py::class_(m, \"CameraConfiguration\");\n> >> @@ -136,12 +137,27 @@ PYBIND11_MODULE(_libcamera, m)\n> >> \tm.def(\"log_set_level\", &logSetLevel);\n> >> \n> >> \t/* Classes */\n> >> +\n> >> +\tpy::enum_(pyEvent, \"Type\")\n> >> +\t\t.value(\"CameraAdded\", CameraEventType::CameraAdded)\n> >> +\t\t.value(\"CameraRemoved\", CameraEventType::CameraRemoved)\n> >> +\t\t.value(\"Disconnect\", CameraEventType::Disconnect)\n> >> +\t\t.value(\"RequestCompleted\", CameraEventType::RequestCompleted)\n> >> +\t\t.value(\"BufferCompleted\", CameraEventType::BufferCompleted);\n> >> +\n> >> +\tpyEvent\n> >> +\t\t.def_readonly(\"type\", &PyCameraEvent::type_)\n> >> +\t\t.def_readonly(\"camera\", &PyCameraEvent::camera_)\n> >> +\t\t.def_readonly(\"request\", &PyCameraEvent::request_)\n> >> +\t\t.def_readonly(\"fb\", &PyCameraEvent::fb_);\n> >> +\n> >> \tpyCameraManager\n> >> \t\t.def_static(\"singleton\", []() {\n> >> \t\t\tstd::shared_ptr cm = gCameraManager.lock();\n> >> \n> >> \t\t\tif (!cm) {\n> >> \t\t\t\tcm = std::make_shared();\n> >> +\t\t\t\tcm->init();\n> >> \t\t\t\tgCameraManager = cm;\n> >> \t\t\t}\n> >> \n> >> @@ -153,10 +169,33 @@ PYBIND11_MODULE(_libcamera, m)\n> >> \t\t.def_property_readonly(\"cameras\", &PyCameraManager::cameras)\n> >> \n> >> \t\t.def_property_readonly(\"event_fd\", &PyCameraManager::eventFd)\n> >> -\t\t.def(\"get_ready_requests\", &PyCameraManager::getReadyRequests);\n> >> +\n> >> +\t\t/* DEPRECATED */\n> >> +\t\t.def(\"get_ready_requests\", &PyCameraManager::getReadyRequests)\n> >> +\n> >> +\t\t.def(\"get_events\", &PyCameraManager::getPyEvents);\n> >> \n> >> \tpyCamera\n> >> \t\t.def_property_readonly(\"id\", &Camera::id)\n> >> +\n> >> +\t\t.def(\"get_camera_event_enabled\",\n> >> +\t\t\t[](Camera &self, CameraEventType type) {\n> >> +\t\t\t\tauto cm = gCameraManager.lock();\n> >> +\t\t\t\treturn cm->getCameraEventFlag(self.shared_from_this(), type);\n> >> +\t\t\t})\n> >> +\n> >> +\t\t.def(\"enable_camera_event\",\n> >> +\t\t\t[](Camera &self, CameraEventType type) {\n> >> +\t\t\t\tauto cm = gCameraManager.lock();\n> >> +\t\t\t\tcm->setCameraEventFlag(self.shared_from_this(), type, true);\n> >> +\t\t\t})\n> >> +\n> >> +\t\t.def(\"disable_camera_event\",\n> >> +\t\t\t[](Camera &self, CameraEventType type) {\n> >> +\t\t\t\tauto cm = gCameraManager.lock();\n> >> +\t\t\t\tcm->setCameraEventFlag(self.shared_from_this(), type, false);\n> >> +\t\t\t})\n> >> +\n> >> \t\t.def(\"acquire\", [](Camera &self) {\n> >> \t\t\tint ret = self.acquire();\n> >> \t\t\tif (ret)\n> >> @@ -173,11 +212,6 @@ PYBIND11_MODULE(_libcamera, m)\n> >> \t\t const std::unordered_map &controls) {\n> >> \t\t\t/* \\todo What happens if someone calls start() multiple times? */\n> >> \n> >> -\t\t\tauto cm = gCameraManager.lock();\n> >> -\t\t\tASSERT(cm);\n> >> -\n> >> -\t\t\tself.requestCompleted.connect(cm.get(), &PyCameraManager::handleRequestCompleted);\n> >> -\n> >> \t\t\tControlList controlList(self.controls());\n> >> \n> >> \t\t\tfor (const auto &[id, obj] : controls) {\n> >> @@ -187,7 +221,6 @@ PYBIND11_MODULE(_libcamera, m)\n> >> \n> >> \t\t\tint ret = self.start(&controlList);\n> >> \t\t\tif (ret) {\n> >> -\t\t\t\tself.requestCompleted.disconnect();\n> >> \t\t\t\tthrow std::system_error(-ret, std::generic_category(),\n> >> \t\t\t\t\t\t\t\"Failed to start camera\");\n> >> \t\t\t}\n> > \n> > You can drop the curly braces.\n> \n> Indeed.\n> \n> >> @@ -196,11 +229,16 @@ PYBIND11_MODULE(_libcamera, m)\n> >> \t\t.def(\"stop\", [](Camera &self) {\n> >> \t\t\tint ret = self.stop();\n> >> \n> >> -\t\t\tself.requestCompleted.disconnect();\n> >> +\t\t\tauto cm = gCameraManager.lock();\n> >> +\t\t\tASSERT(cm);\n> >> +\n> >> +\t\t\tauto events = cm->getPyCameraEvents(self.shared_from_this());\n> >> \n> >> \t\t\tif (ret)\n> >> \t\t\t\tthrow std::system_error(-ret, std::generic_category(),\n> >> \t\t\t\t\t\t\t\"Failed to stop camera\");\n> >> +\n> >> +\t\t\treturn events;\n> >> \t\t})\n> >> \n> >> \t\t.def(\"__str__\", [](Camera &self) {","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 11A9EC31E9\n\tfor ;\n\tMon, 5 Jun 2023 07:51:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5B7016287D;\n\tMon, 5 Jun 2023 09:51:07 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4BDBE60579\n\tfor ;\n\tMon, 5 Jun 2023 09:51:06 +0200 (CEST)","from pendragon.ideasonboard.com (om126156242094.26.openmobile.ne.jp\n\t[126.156.242.94])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A166A2BC;\n\tMon, 5 Jun 2023 09:50:31 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685951467;\n\tbh=xAtFy5lJu1tIVqvZg59RkprMeyOq16tzKs3eQ1RQZPk=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=vJnvuRmgnuf4khQpxynTWjN/qj807Pn35VvkC9EiYg3NuMXD7a0bDiuMDhXxrBqcX\n\teLWTlrxfv4QbFJBkzRwnnKmgSI6ewJ5WSZRTBrjGdfUMzjmRTjM14lhpopQnjABZ7l\n\tGlKjMoTprfvY6bpZRmjWfA1VZ+wivzogptgsA2JDlTQHF+A+grLSTrRewpZV3r6NZf\n\t6h+7dsrL/UqiRPlIFwaufzyUqSggXICFZUyhtIddXz2EiOEa1zCuZHUYr5+CxRC4pq\n\trDlH63npofD7qH6kb9MT2B38fwl4FBsnNZDNKw0kapGMsl6EwoiCFS3C5WroskaPPD\n\t5S0kiOSiskHmQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685951441;\n\tbh=xAtFy5lJu1tIVqvZg59RkprMeyOq16tzKs3eQ1RQZPk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=HGu7rn2DNYxg3GIMBoHmFp4OSOQxzx8xjkkMhUG52MiENg4EfEnEUMrGPQMO3/xus\n\tukDjUinZsci0z1pYYJ4ujnIgWqThS3OVQCqRAMm14HyLB4xi2D4+ZR0kCbJtXL0v8F\n\tSbV4rsILXDl0+O1wj/X+JzoT3GXphGhZFuIL4fNU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"HGu7rn2D\"; dkim-atps=neutral","Date":"Mon, 5 Jun 2023 10:50:54 +0300","To":"Tomi Valkeinen ","Message-ID":"<20230605075054.GA30841@pendragon.ideasonboard.com>","References":"<20230603075615.20663-1-tomi.valkeinen@ideasonboard.com>\n\t<20230603075615.20663-4-tomi.valkeinen@ideasonboard.com>\n\t<20230605064750.GF22604@pendragon.ideasonboard.com>\n\t<51586616-f621-7fb8-1a53-86773aa969e4@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<51586616-f621-7fb8-1a53-86773aa969e4@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 03/13] py: New event handling","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]