[{"id":26636,"web_url":"https://patchwork.libcamera.org/comment/26636/","msgid":"<20230312154430.GV2545@pendragon.ideasonboard.com>","date":"2023-03-12T15:44:30","subject":"Re: [libcamera-devel] [PATCH v4 02/15] 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 Thu, Mar 09, 2023 at 04:25:48PM +0200, Tomi Valkeinen via libcamera-devel 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> camera. This serves two purposes: first, it removes the requestCompleted\n> events from the event queue, thus preventing the old events being\n> returned when the camera is started again, and second, it allows the\n> user to process those events if it so wants.\n> \n> All other event types are always collected and returned, except\n> bufferCompleted which needs to be enabled by setting the\n> CameraManager.buffer_completed_active to True. This is to reduce the\n> overhead a bit. It is not clear if this is a necessary optimization or\n> not.\n> \n> TODO: Documentation.\n\nLooks like we're converging on the API, so it's time to write\ndocumentation :-)\n\n> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>\n> ---\n>  src/py/libcamera/py_camera_manager.cpp | 195 +++++++++++++++++++++++--\n>  src/py/libcamera/py_camera_manager.h   |  66 ++++++++-\n>  src/py/libcamera/py_main.cpp           |  44 +++++-\n>  3 files changed, 281 insertions(+), 24 deletions(-)\n> \n> diff --git a/src/py/libcamera/py_camera_manager.cpp b/src/py/libcamera/py_camera_manager.cpp\n> index 9ccb7aad..7d6dded4 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 <algorithm>\n>  #include <errno.h>\n>  #include <memory>\n>  #include <sys/eventfd.h>\n> @@ -35,11 +36,17 @@ 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>  py::list PyCameraManager::cameras()\n> @@ -60,6 +67,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<py::object> PyCameraManager::getReadyRequests()\n>  {\n>  \tint ret = readFd();\n> @@ -72,21 +116,134 @@ std::vector<py::object> PyCameraManager::getReadyRequests()\n>  \n>  \tstd::vector<py::object> 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<PyCameraEvent> 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<CameraEvent> events = getEvents();\n> +\n> +\tLOG(Python, Debug) << \"Got \" << events.size() << \" events\";\n> +\n> +\tstd::vector<PyCameraEvent> 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> &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\nCould you check my latest reply to the v3 mail thread for this function\n(message ID Yw5ki0Tmycz9Uk6A@pendragon.ideasonboard.com) ? I think we\nhaven't finished the discussion.\n\n> +}\n> +\n> +std::vector<PyCameraEvent> PyCameraManager::getPyCameraEvents(std::shared_ptr<Camera> camera)\n> +{\n> +\tstd::vector<CameraEvent> 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\nErasing elements in the middle of a vactor is quite inefficient. We've\ndiscussed this in the review of v4. Could you confirm you have deviced\nto not switch to a std::list, and not just forgotten about it ?\n\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<PyCameraEvent> 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<Camera> cam, Request *req, FrameBuffer *fb)\n>  {\n> -\tpushRequest(req);\n> -\twriteFd();\n> +\tif (!bufferCompletedEventActive_)\n> +\t\treturn;\n> +\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<Camera> 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<Camera> 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<Camera> 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<Camera> cam)\n> +{\n> +\tCameraEvent ev(CameraEventType::CameraRemoved, cam);\n> +\n> +\tpushEvent(ev);\n>  }\n>  \n>  void PyCameraManager::writeFd()\n> @@ -116,16 +273,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<Request *> PyCameraManager::getCompletedRequests()\n> +std::vector<CameraEvent> PyCameraManager::getEvents()\n>  {\n> -\tstd::vector<Request *> v;\n> -\tMutexLocker guard(completedRequestsMutex_);\n> -\tswap(v, completedRequests_);\n> +\tstd::vector<CameraEvent> 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 3525057d..757f6d8e 100644\n> --- a/src/py/libcamera/py_camera_manager.h\n> +++ b/src/py/libcamera/py_camera_manager.h\n> @@ -13,6 +13,48 @@\n>  \n>  using namespace libcamera;\n>  \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> 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> 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\n>  {\n>  public:\n> @@ -26,20 +68,30 @@ public:\n>  \n>  \tint eventFd() const { return eventFd_.get(); }\n>  \n> -\tstd::vector<pybind11::object> getReadyRequests();\n> +\tstd::vector<pybind11::object> getReadyRequests(); /* DEPRECATED */\n> +\tstd::vector<PyCameraEvent> getPyEvents();\n> +\tstd::vector<PyCameraEvent> getPyCameraEvents(std::shared_ptr<Camera> camera);\n> +\n> +\tvoid handleBufferCompleted(std::shared_ptr<Camera> cam, Request *req, FrameBuffer *fb);\n> +\tvoid handleRequestCompleted(std::shared_ptr<Camera> cam, Request *req);\n> +\tvoid handleDisconnected(std::shared_ptr<Camera> cam);\n> +\tvoid handleCameraAdded(std::shared_ptr<Camera> cam);\n> +\tvoid handleCameraRemoved(std::shared_ptr<Camera> cam);\n>  \n> -\tvoid handleRequestCompleted(Request *req);\n> +\tbool bufferCompletedEventActive_ = false;\n>  \n>  private:\n>  \tstd::unique_ptr<CameraManager> cameraManager_;\n>  \n>  \tUniqueFD eventFd_;\n> -\tlibcamera::Mutex completedRequestsMutex_;\n> -\tstd::vector<Request *> completedRequests_\n> -\t\tLIBCAMERA_TSA_GUARDED_BY(completedRequestsMutex_);\n> +\tlibcamera::Mutex eventsMutex_;\n> +\tstd::vector<CameraEvent> events_\n> +\t\tLIBCAMERA_TSA_GUARDED_BY(eventsMutex_);\n>  \n>  \tvoid writeFd();\n>  \tint readFd();\n> -\tvoid pushRequest(Request *req);\n> -\tstd::vector<Request *> getCompletedRequests();\n> +\tvoid pushEvent(const CameraEvent &ev);\n> +\tstd::vector<CameraEvent> getEvents();\n> +\n> +\tPyCameraEvent convertEvent(const CameraEvent &event);\n>  };\n> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n> index 1d4c23b3..0fffc030 100644\n> --- a/src/py/libcamera/py_main.cpp\n> +++ b/src/py/libcamera/py_main.cpp\n> @@ -61,6 +61,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_<PyCameraEvent>(m, \"Event\");\n\nAlphabetical order ?\n\n>  \tauto pyCameraManager = py::class_<PyCameraManager>(m, \"CameraManager\");\n>  \tauto pyCamera = py::class_<Camera>(m, \"Camera\");\n>  \tauto pyCameraConfiguration = py::class_<CameraConfiguration>(m, \"CameraConfiguration\");\n> @@ -93,6 +94,20 @@ PYBIND11_MODULE(_libcamera, m)\n>  \tm.def(\"log_set_level\", &logSetLevel);\n>  \n>  \t/* Classes */\n> +\n\nExtra blank line ?\n\n> +\tpy::enum_<CameraEventType>(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<PyCameraManager> cm = gCameraManager.lock();\n> @@ -110,7 +125,13 @@ 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> +\t\t.def_readwrite(\"buffer_completed_active\", &PyCameraManager::bufferCompletedEventActive_);\n>  \n>  \tpyCamera\n>  \t\t.def_property_readonly(\"id\", &Camera::id)\n> @@ -133,7 +154,17 @@ PYBIND11_MODULE(_libcamera, m)\n>  \t\t\tauto cm = gCameraManager.lock();\n>  \t\t\tASSERT(cm);\n>  \n> -\t\t\tself.requestCompleted.connect(cm.get(), &PyCameraManager::handleRequestCompleted);\n> +\t\t\tself.requestCompleted.connect(&self, [cm, camera=self.shared_from_this()](Request *req) {\n> +\t\t\t\tcm->handleRequestCompleted(camera, req);\n> +\t\t\t});\n> +\n> +\t\t\tself.bufferCompleted.connect(&self, [cm, camera=self.shared_from_this()](Request *req, FrameBuffer *fb) {\n> +\t\t\t\tcm->handleBufferCompleted(camera, req, fb);\n> +\t\t\t});\n> +\n> +\t\t\tself.disconnected.connect(&self, [cm, camera=self.shared_from_this()]() {\n> +\t\t\t\tcm->handleDisconnected(camera);\n> +\t\t\t});\n>  \n>  \t\t\tControlList controlList(self.controls());\n>  \n> @@ -154,10 +185,19 @@ PYBIND11_MODULE(_libcamera, m)\n>  \t\t\tint ret = self.stop();\n>  \n>  \t\t\tself.requestCompleted.disconnect();\n> +\t\t\tself.bufferCompleted.disconnect();\n> +\t\t\tself.disconnected.disconnect();\n> +\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":"<libcamera-devel-bounces@lists.libcamera.org>","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 76027BE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 12 Mar 2023 15:44:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CA7A862709;\n\tSun, 12 Mar 2023 16:44:31 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 14E9162705\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 12 Mar 2023 16:44:30 +0100 (CET)","from pendragon.ideasonboard.com (85-76-21-162-nat.elisa-mobile.fi\n\t[85.76.21.162])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A6676814;\n\tSun, 12 Mar 2023 16:44:28 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1678635871;\n\tbh=ebwpHmFPuXTc7eQd81UhBSDrBz9RBOKriQ0eFQQlL48=;\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=tcW6FMd+/cgAgnFQSnsxuLmSE6mGl9qeZy4uzCxq5MvVZ1jBqJmRFG6cvATrllDVc\n\tSByWjPZBOswHe+WnSKELmYucUerxAfTG43reHcuSgbVsOiL2JxDaJEUZYm5RJGZBcZ\n\t3+3MosFFRcCIlvdjYWJr3lmvMKGHhVSmg7aNW4ji8fIO+KX0iMRGvHU+CJbtO0OIG8\n\tF0gqkBPRmGcUQ4FSwIDi1tpgh9D2X2gGotUTjF4gve77I7KaCXmU3gh68yPdvvTNyU\n\tEV9kEn0prFGcpegS0hFGxrbAn7DsdFGzBo1QC6fMewMTTeNaeVO4Fh2SMAV65R8cfY\n\txvKBJjr/UdxPg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1678635869;\n\tbh=ebwpHmFPuXTc7eQd81UhBSDrBz9RBOKriQ0eFQQlL48=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=b1+VrsC0dYeohggP6FFE5gEU/0bofAEVh6dl23vhxh0u5eZSaCibK66lReGS4nPYQ\n\trdJ587IfxQwHCvEqOEdasjnzcjH85UGd1h0xmYFhAk7E9jQBJUstVh4oU5NU9ix5Pp\n\tzDY60zOD7wr8DFgJn10CMcgotYDTmmsYW8HAn42s="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"b1+VrsC0\"; dkim-atps=neutral","Date":"Sun, 12 Mar 2023 17:44:30 +0200","To":"Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>","Message-ID":"<20230312154430.GV2545@pendragon.ideasonboard.com>","References":"<20230309142601.70556-1-tomi.valkeinen@ideasonboard.com>\n\t<20230309142601.70556-3-tomi.valkeinen@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230309142601.70556-3-tomi.valkeinen@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 02/15] py: New event handling","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":26644,"web_url":"https://patchwork.libcamera.org/comment/26644/","msgid":"<6fb2a7d2-5787-d172-f637-5706c95f7d91@ideasonboard.com>","date":"2023-03-13T06:49:33","subject":"Re: [libcamera-devel] [PATCH v4 02/15] py: New event handling","submitter":{"id":109,"url":"https://patchwork.libcamera.org/api/people/109/","name":"Tomi Valkeinen","email":"tomi.valkeinen@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 12/03/2023 17:44, Laurent Pinchart wrote:\n> Hi Tomi,\n> \n> Thank you for the patch.\n> \n> On Thu, Mar 09, 2023 at 04:25:48PM +0200, Tomi Valkeinen via libcamera-devel 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>> camera. This serves two purposes: first, it removes the requestCompleted\n>> events from the event queue, thus preventing the old events being\n>> returned when the camera is started again, and second, it allows the\n>> user to process those events if it so wants.\n>>\n>> All other event types are always collected and returned, except\n>> bufferCompleted which needs to be enabled by setting the\n>> CameraManager.buffer_completed_active to True. This is to reduce the\n>> overhead a bit. It is not clear if this is a necessary optimization or\n>> not.\n>>\n>> TODO: Documentation.\n> \n> Looks like we're converging on the API, so it's time to write\n> documentation :-)\n\nWell, I don't know about that. I haven't really gotten any comments \nabout the API. And the RFC hack patch at the end of the series \nintroduces another style. But probably I just have to pick an approach, \nwrite the docs, and send a pull req, as otherwise we're still here the \nnext year =).\n\n>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>\n>> ---\n>>   src/py/libcamera/py_camera_manager.cpp | 195 +++++++++++++++++++++++--\n>>   src/py/libcamera/py_camera_manager.h   |  66 ++++++++-\n>>   src/py/libcamera/py_main.cpp           |  44 +++++-\n>>   3 files changed, 281 insertions(+), 24 deletions(-)\n>>\n>> diff --git a/src/py/libcamera/py_camera_manager.cpp b/src/py/libcamera/py_camera_manager.cpp\n>> index 9ccb7aad..7d6dded4 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 <algorithm>\n>>   #include <errno.h>\n>>   #include <memory>\n>>   #include <sys/eventfd.h>\n>> @@ -35,11 +36,17 @@ 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>>   py::list PyCameraManager::cameras()\n>> @@ -60,6 +67,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<py::object> PyCameraManager::getReadyRequests()\n>>   {\n>>   \tint ret = readFd();\n>> @@ -72,21 +116,134 @@ std::vector<py::object> PyCameraManager::getReadyRequests()\n>>   \n>>   \tstd::vector<py::object> 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<PyCameraEvent> 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<CameraEvent> events = getEvents();\n>> +\n>> +\tLOG(Python, Debug) << \"Got \" << events.size() << \" events\";\n>> +\n>> +\tstd::vector<PyCameraEvent> 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> &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> Could you check my latest reply to the v3 mail thread for this function\n> (message ID Yw5ki0Tmycz9Uk6A@pendragon.ideasonboard.com) ? I think we\n> haven't finished the discussion.\n\nYes, I mentioned this in the intro letter.\n\nIf we use the style used in this patch, I still think it makes sense to \nhandle all three event types here. cam.start() connects those events, \ncam.stop() disconnects, so it makes sense that cam.stop() also returns \nall those events.\n\nIf we go with something like in the last patch, where the event \n\"subscription\" is outside the start and stop methods, it's not so clear. \nMaybe stop() shouldn't return any events in that case, although then we \nmight want to have another method to get and remove the events for that \ncamera, so that the user can flush the events.\n\nIf I remember right, I have never managed to get a Disconnect event, so \nI don't quite know when they come or how one would use them. So I don't \nhave very strong feelings here.\n\n>> +}\n>> +\n>> +std::vector<PyCameraEvent> PyCameraManager::getPyCameraEvents(std::shared_ptr<Camera> camera)\n>> +{\n>> +\tstd::vector<CameraEvent> 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> \n> Erasing elements in the middle of a vactor is quite inefficient. We've\n> discussed this in the review of v4. Could you confirm you have deviced\n> to not switch to a std::list, and not just forgotten about it ?\n\nI did move the conversion to py events to be outside the lock.\n\nAs for the vector, yes, erasing is inefficient, but so is allocating and \nfreeing memory. The number of events is small (I get 4 even if using \nmultiple cameras). Afaics, we have to options:\n\n- Use vector. There is some overhead when camera.stop() is called.\n- Use list. There is some overhead while capturing frames.\n\nIn both cases I think the overhead is unmeasurable in practice, but \nchoosing the overhead for .stop() sounds better to me.\n\nPossibly the code could be faster if we just create a new vector for the \nunhandled events, instead of using erase(). This would be the case at \nleast in the normal case (at least for me) where all the events are for \nthe camera being stopped, as there would be no unhandled events. But I \nthink the difference is again unmeasurable.\n\nIn any case, this is something we can easily tune later.\n\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<PyCameraEvent> 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<Camera> cam, Request *req, FrameBuffer *fb)\n>>   {\n>> -\tpushRequest(req);\n>> -\twriteFd();\n>> +\tif (!bufferCompletedEventActive_)\n>> +\t\treturn;\n>> +\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<Camera> 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<Camera> 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<Camera> 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<Camera> cam)\n>> +{\n>> +\tCameraEvent ev(CameraEventType::CameraRemoved, cam);\n>> +\n>> +\tpushEvent(ev);\n>>   }\n>>   \n>>   void PyCameraManager::writeFd()\n>> @@ -116,16 +273,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<Request *> PyCameraManager::getCompletedRequests()\n>> +std::vector<CameraEvent> PyCameraManager::getEvents()\n>>   {\n>> -\tstd::vector<Request *> v;\n>> -\tMutexLocker guard(completedRequestsMutex_);\n>> -\tswap(v, completedRequests_);\n>> +\tstd::vector<CameraEvent> 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 3525057d..757f6d8e 100644\n>> --- a/src/py/libcamera/py_camera_manager.h\n>> +++ b/src/py/libcamera/py_camera_manager.h\n>> @@ -13,6 +13,48 @@\n>>   \n>>   using namespace libcamera;\n>>   \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> 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> 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\n>>   {\n>>   public:\n>> @@ -26,20 +68,30 @@ public:\n>>   \n>>   \tint eventFd() const { return eventFd_.get(); }\n>>   \n>> -\tstd::vector<pybind11::object> getReadyRequests();\n>> +\tstd::vector<pybind11::object> getReadyRequests(); /* DEPRECATED */\n>> +\tstd::vector<PyCameraEvent> getPyEvents();\n>> +\tstd::vector<PyCameraEvent> getPyCameraEvents(std::shared_ptr<Camera> camera);\n>> +\n>> +\tvoid handleBufferCompleted(std::shared_ptr<Camera> cam, Request *req, FrameBuffer *fb);\n>> +\tvoid handleRequestCompleted(std::shared_ptr<Camera> cam, Request *req);\n>> +\tvoid handleDisconnected(std::shared_ptr<Camera> cam);\n>> +\tvoid handleCameraAdded(std::shared_ptr<Camera> cam);\n>> +\tvoid handleCameraRemoved(std::shared_ptr<Camera> cam);\n>>   \n>> -\tvoid handleRequestCompleted(Request *req);\n>> +\tbool bufferCompletedEventActive_ = false;\n>>   \n>>   private:\n>>   \tstd::unique_ptr<CameraManager> cameraManager_;\n>>   \n>>   \tUniqueFD eventFd_;\n>> -\tlibcamera::Mutex completedRequestsMutex_;\n>> -\tstd::vector<Request *> completedRequests_\n>> -\t\tLIBCAMERA_TSA_GUARDED_BY(completedRequestsMutex_);\n>> +\tlibcamera::Mutex eventsMutex_;\n>> +\tstd::vector<CameraEvent> events_\n>> +\t\tLIBCAMERA_TSA_GUARDED_BY(eventsMutex_);\n>>   \n>>   \tvoid writeFd();\n>>   \tint readFd();\n>> -\tvoid pushRequest(Request *req);\n>> -\tstd::vector<Request *> getCompletedRequests();\n>> +\tvoid pushEvent(const CameraEvent &ev);\n>> +\tstd::vector<CameraEvent> getEvents();\n>> +\n>> +\tPyCameraEvent convertEvent(const CameraEvent &event);\n>>   };\n>> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n>> index 1d4c23b3..0fffc030 100644\n>> --- a/src/py/libcamera/py_main.cpp\n>> +++ b/src/py/libcamera/py_main.cpp\n>> @@ -61,6 +61,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_<PyCameraEvent>(m, \"Event\");\n> \n> Alphabetical order ?\n\nThey're not in alphabetical order. Now that we use the style where we \ndeclare the types before we define them we might order everything \nalphabetically. But that will be a big change, causing big conflicts. So \nI'd rather do that at some other time.\n\n>>   \tauto pyCameraManager = py::class_<PyCameraManager>(m, \"CameraManager\");\n>>   \tauto pyCamera = py::class_<Camera>(m, \"Camera\");\n>>   \tauto pyCameraConfiguration = py::class_<CameraConfiguration>(m, \"CameraConfiguration\");\n>> @@ -93,6 +94,20 @@ PYBIND11_MODULE(_libcamera, m)\n>>   \tm.def(\"log_set_level\", &logSetLevel);\n>>   \n>>   \t/* Classes */\n>> +\n> \n> Extra blank line ?\n\nI think it looks better with a blank line.\n\n>> +\tpy::enum_<CameraEventType>(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<PyCameraManager> cm = gCameraManager.lock();\n>> @@ -110,7 +125,13 @@ 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>> +\t\t.def_readwrite(\"buffer_completed_active\", &PyCameraManager::bufferCompletedEventActive_);\n>>   \n>>   \tpyCamera\n>>   \t\t.def_property_readonly(\"id\", &Camera::id)\n>> @@ -133,7 +154,17 @@ PYBIND11_MODULE(_libcamera, m)\n>>   \t\t\tauto cm = gCameraManager.lock();\n>>   \t\t\tASSERT(cm);\n>>   \n>> -\t\t\tself.requestCompleted.connect(cm.get(), &PyCameraManager::handleRequestCompleted);\n>> +\t\t\tself.requestCompleted.connect(&self, [cm, camera=self.shared_from_this()](Request *req) {\n>> +\t\t\t\tcm->handleRequestCompleted(camera, req);\n>> +\t\t\t});\n>> +\n>> +\t\t\tself.bufferCompleted.connect(&self, [cm, camera=self.shared_from_this()](Request *req, FrameBuffer *fb) {\n>> +\t\t\t\tcm->handleBufferCompleted(camera, req, fb);\n>> +\t\t\t});\n>> +\n>> +\t\t\tself.disconnected.connect(&self, [cm, camera=self.shared_from_this()]() {\n>> +\t\t\t\tcm->handleDisconnected(camera);\n>> +\t\t\t});\n>>   \n>>   \t\t\tControlList controlList(self.controls());\n>>   \n>> @@ -154,10 +185,19 @@ PYBIND11_MODULE(_libcamera, m)\n>>   \t\t\tint ret = self.stop();\n>>   \n>>   \t\t\tself.requestCompleted.disconnect();\n>> +\t\t\tself.bufferCompleted.disconnect();\n>> +\t\t\tself.disconnected.disconnect();\n>> +\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>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 78628BE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 13 Mar 2023 06:49:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AFDE762709;\n\tMon, 13 Mar 2023 07:49:40 +0100 (CET)","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 876E761ED5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 13 Mar 2023 07:49:37 +0100 (CET)","from [192.168.1.15] (91-154-32-225.elisa-laajakaista.fi\n\t[91.154.32.225])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id CDF64563;\n\tMon, 13 Mar 2023 07:49:36 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1678690180;\n\tbh=yFpT9NhCLhPthFAPFrz5vrzD3dGhP3vZ35flVKDo+Qc=;\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=Ymnfr1gSnIcMW4hGzMI1VXGsX4dVPClaAHHZPvfeBhvCz9k7AbEUXVt4L6dQdVRcK\n\tfgZPwcuxklNTQb0OGbw8zF7mAu/vcTLhp9YLjP159jCsy7dBEg90hj5zi594NJAd2J\n\ttiJti5E8khdzbSZEsPnOIWF1jQFdOdEGlxMiOuGSzJofEh3x0dAdJRlZ3X4JQAg+aB\n\tACKgd8xY3dP7R/TrfwbwiwnxxVISgEf6sesE8koHhQBhbduXqPN3MYDskKc8dK6rbT\n\tciIRnBSje+n8P4fnHuo41vEg0ajI15xGgXMKSBlBN5xJR1WnoSsnGVlSvLIXfBRCQp\n\tH/l56lJ+eeJng==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1678690177;\n\tbh=yFpT9NhCLhPthFAPFrz5vrzD3dGhP3vZ35flVKDo+Qc=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=Olj+ExAixS/xUUKNwNkB6Nm3rEBCk7TLKRBXfDf89Bduu/Lo9F+i3YK9I0zO2aiL1\n\tKykrYBd2i2G3qKP+/1spahPGBeRNRsuDt3+cTFjGa4eBI2sK4kGusQR6MPQ/7iPGpQ\n\tSeblxFg7Zz0WoEfsnc74LrcKvXWJKq7e+Az2DVM8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Olj+ExAi\"; dkim-atps=neutral","Message-ID":"<6fb2a7d2-5787-d172-f637-5706c95f7d91@ideasonboard.com>","Date":"Mon, 13 Mar 2023 08:49:33 +0200","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.7.1","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","References":"<20230309142601.70556-1-tomi.valkeinen@ideasonboard.com>\n\t<20230309142601.70556-3-tomi.valkeinen@ideasonboard.com>\n\t<20230312154430.GV2545@pendragon.ideasonboard.com>","Content-Language":"en-US","In-Reply-To":"<20230312154430.GV2545@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v4 02/15] py: New event handling","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Tomi Valkeinen via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]