Patch Detail
Show a patch.
GET /api/patches/13632/?format=api
{ "id": 13632, "url": "https://patchwork.libcamera.org/api/patches/13632/?format=api", "web_url": "https://patchwork.libcamera.org/patch/13632/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20210906140152.636883-5-jacopo@jmondi.org>", "date": "2021-09-06T14:01:51", "name": "[libcamera-devel,4/5] android: camera_worker: Notify fence wait failures", "commit_ref": null, "pull_url": null, "state": "not-applicable", "archived": false, "hash": "ed4fc89a03bf9ffc56b15bbe31c2d0f47ea45fc9", "submitter": { "id": 3, "url": "https://patchwork.libcamera.org/api/people/3/?format=api", "name": "Jacopo Mondi", "email": "jacopo@jmondi.org" }, "delegate": { "id": 15, "url": "https://patchwork.libcamera.org/api/users/15/?format=api", "username": "jmondi", "first_name": "Jacopo", "last_name": "Mondi", "email": "jacopo@jmondi.org" }, "mbox": "https://patchwork.libcamera.org/patch/13632/mbox/", "series": [ { "id": 2448, "url": "https://patchwork.libcamera.org/api/series/2448/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=2448", "date": "2021-09-06T14:01:47", "name": "android: Fix descriptors_ clean up", "version": 1, "mbox": "https://patchwork.libcamera.org/series/2448/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/13632/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/13632/checks/", "tags": {}, "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 918F0BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 6 Sep 2021 14:01:15 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4EB436916A;\n\tMon, 6 Sep 2021 16:01:14 +0200 (CEST)", "from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CBCE36916F\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 6 Sep 2021 16:01:10 +0200 (CEST)", "(Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 6454A40011;\n\tMon, 6 Sep 2021 14:01:10 +0000 (UTC)" ], "From": "Jacopo Mondi <jacopo@jmondi.org>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Mon, 6 Sep 2021 16:01:51 +0200", "Message-Id": "<20210906140152.636883-5-jacopo@jmondi.org>", "X-Mailer": "git-send-email 2.32.0", "In-Reply-To": "<20210906140152.636883-1-jacopo@jmondi.org>", "References": "<20210906140152.636883-1-jacopo@jmondi.org>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH 4/5] android: camera_worker: Notify fence\n\twait failures", "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>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "The CameraDevice class stores one CaptureRequestDescriptor instance\nfor each capture request queued to the Camera. Such descriptors are\ncleared when the Camera completes the Request in the\nCameraDevice::requestComplete() slot.\n\nThe CameraDevice class relies on an internal worker to off-load\nwaiting on synchronization fences, and does unconditionally store\nthe descriptor in the descriptors_ class member map to handle its\nlifetime.\n\nIf waiting on a fence fails, the created descriptor remains in the\ndescriptors_ map until the next call to stop() or close(), as\nrequestComplete() will never be called, and the camera\nservice is never notified about the queueing failure.\n\nFix that by allowing the CameraWorker to notify to the CameraDevice if\nwaiting on a fence has failed, by installing a new requestQueueFailed\nSignal.\n\nThe Signal is emitted by the CameraWorker internal worker if waiting on\na fence has failed. The CameraDevice is augmented with a slot connected\nto the Signal that removes the descriptor from the map and notifies the\ncamera framework about the failure.\n\nSigned-off-by: Jacopo Mondi <jacopo@jmondi.org>\n---\n src/android/camera_device.cpp | 22 +++++++++++++++++++++-\n src/android/camera_device.h | 1 +\n src/android/camera_worker.cpp | 10 ++++++----\n src/android/camera_worker.h | 9 +++++++--\n 4 files changed, 35 insertions(+), 7 deletions(-)", "diff": "diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\nindex a0ea138d9499..0ce9b699bfae 100644\n--- a/src/android/camera_device.cpp\n+++ b/src/android/camera_device.cpp\n@@ -244,7 +244,7 @@ CameraDevice::Camera3RequestDescriptor::Camera3RequestDescriptor(\n \t * Create the CaptureRequest, stored as a unique_ptr<> to tie its\n \t * lifetime to the descriptor.\n \t */\n-\trequest_ = std::make_unique<CaptureRequest>(camera);\n+\trequest_ = std::make_unique<CaptureRequest>(camera, camera3Request);\n }\n \n /*\n@@ -264,6 +264,7 @@ CameraDevice::CameraDevice(unsigned int id, std::shared_ptr<Camera> camera)\n \t: id_(id), state_(State::Stopped), camera_(std::move(camera)),\n \t facing_(CAMERA_FACING_FRONT), orientation_(0)\n {\n+\tworker_.requestQueueFailed.connect(this, &CameraDevice::requestQueueFailed);\n \tcamera_->requestCompleted.connect(this, &CameraDevice::requestComplete);\n \n \tmaker_ = \"libcamera\";\n@@ -1060,6 +1061,25 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n \treturn 0;\n }\n \n+void CameraDevice::requestQueueFailed(CaptureRequest *request)\n+{\n+\t/*\n+\t * Remove the descriptor from the map if the worker has failed\n+\t * to process it and notify the camera service about the failure.\n+\t */\n+\tMutexLocker descriptorsLock(descriptorsMutex_);\n+\tauto it = descriptors_.find(request->cookie());\n+\tif (it == descriptors_.end()) {\n+\t\tLOG(HAL, Fatal)\n+\t\t\t<< \"Unknown request: \" << request->cookie();\n+\t\treturn;\n+\t}\n+\n+\tdescriptors_.extract(it);\n+\n+\tabortRequest(request->camera3Request());\n+}\n+\n void CameraDevice::requestComplete(Request *request)\n {\n \tdecltype(descriptors_)::node_type node;\ndiff --git a/src/android/camera_device.h b/src/android/camera_device.h\nindex 54c4cb9ab499..65456ecca7e3 100644\n--- a/src/android/camera_device.h\n+++ b/src/android/camera_device.h\n@@ -83,6 +83,7 @@ private:\n \t\tstd::vector<std::unique_ptr<libcamera::FrameBuffer>> frameBuffers_;\n \t\tCameraMetadata settings_;\n \t\tstd::unique_ptr<CaptureRequest> request_;\n+\t\tconst camera3_capture_request_t *camera3Request_;\n \t};\n \n \tenum class State {\ndiff --git a/src/android/camera_worker.cpp b/src/android/camera_worker.cpp\nindex 98dddd9eb13b..7f39fab01638 100644\n--- a/src/android/camera_worker.cpp\n+++ b/src/android/camera_worker.cpp\n@@ -27,8 +27,9 @@ LOG_DECLARE_CATEGORY(HAL)\n * by the CameraWorker which queues it to the libcamera::Camera after handling\n * fences.\n */\n-CaptureRequest::CaptureRequest(libcamera::Camera *camera)\n-\t: camera_(camera)\n+CaptureRequest::CaptureRequest(libcamera::Camera *camera,\n+\t\t\t const camera3_capture_request_t *camera3Request)\n+\t: camera_(camera), camera3Request_(camera3Request)\n {\n \trequest_ = camera_->createRequest(reinterpret_cast<uint64_t>(this));\n }\n@@ -77,7 +78,7 @@ void CameraWorker::queueRequest(CaptureRequest *request)\n {\n \t/* Async process the request on the worker which runs its own thread. */\n \tworker_.invokeMethod(&Worker::processRequest, ConnectionTypeQueued,\n-\t\t\t request);\n+\t\t\t this, request);\n }\n \n /*\n@@ -109,7 +110,7 @@ int CameraWorker::Worker::waitFence(int fence)\n \treturn -errno;\n }\n \n-void CameraWorker::Worker::processRequest(CaptureRequest *request)\n+void CameraWorker::Worker::processRequest(CameraWorker *context, CaptureRequest *request)\n {\n \t/* Wait on all fences before queuing the Request. */\n \tfor (int fence : request->fences()) {\n@@ -121,6 +122,7 @@ void CameraWorker::Worker::processRequest(CaptureRequest *request)\n \t\tif (ret < 0) {\n \t\t\tLOG(HAL, Error) << \"Failed waiting for fence: \"\n \t\t\t\t\t<< fence << \": \" << strerror(-ret);\n+\t\t\tcontext->requestQueueFailed.emit(request);\n \t\t\treturn;\n \t\t}\n \t}\ndiff --git a/src/android/camera_worker.h b/src/android/camera_worker.h\nindex 67ae50bd9288..86f20f741e54 100644\n--- a/src/android/camera_worker.h\n+++ b/src/android/camera_worker.h\n@@ -10,6 +10,7 @@\n #include <memory>\n \n #include <libcamera/base/object.h>\n+#include <libcamera/base/signal.h>\n #include <libcamera/base/thread.h>\n \n #include <libcamera/camera.h>\n@@ -22,7 +23,8 @@ class CameraDevice;\n class CaptureRequest\n {\n public:\n-\tCaptureRequest(libcamera::Camera *camera);\n+\tCaptureRequest(libcamera::Camera *camera,\n+\t\t const camera3_capture_request_t *camera3Request);\n \n \tconst std::vector<int> &fences() const { return acquireFences_; }\n \tlibcamera::ControlList &controls() { return request_->controls(); }\n@@ -31,6 +33,7 @@ public:\n \t\treturn request_->metadata();\n \t}\n \tunsigned long cookie() const { return request_->cookie(); }\n+\tconst camera3_capture_request_t *camera3Request() const { return camera3Request_; }\n \n \tvoid addBuffer(libcamera::Stream *stream,\n \t\t libcamera::FrameBuffer *buffer, int fence);\n@@ -40,6 +43,7 @@ private:\n \tlibcamera::Camera *camera_;\n \tstd::vector<int> acquireFences_;\n \tstd::unique_ptr<libcamera::Request> request_;\n+\tconst camera3_capture_request_t *camera3Request_;\n };\n \n class CameraWorker : private libcamera::Thread\n@@ -51,6 +55,7 @@ public:\n \tvoid stop();\n \n \tvoid queueRequest(CaptureRequest *request);\n+\tlibcamera::Signal<CaptureRequest *> requestQueueFailed;\n \n protected:\n \tvoid run() override;\n@@ -59,7 +64,7 @@ private:\n \tclass Worker : public libcamera::Object\n \t{\n \tpublic:\n-\t\tvoid processRequest(CaptureRequest *request);\n+\t\tvoid processRequest(CameraWorker *context, CaptureRequest *request);\n \n \tprivate:\n \t\tint waitFence(int fence);\n", "prefixes": [ "libcamera-devel", "4/5" ] }