Patch Detail
Show a patch.
GET /api/1.1/patches/22169/?format=api
{ "id": 22169, "url": "https://patchwork.libcamera.org/api/1.1/patches/22169/?format=api", "web_url": "https://patchwork.libcamera.org/patch/22169/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/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": "<20241204164137.3938891-8-chenghaoyang@chromium.org>", "date": "2024-12-04T16:36:32", "name": "[v3,7/7] android: Remove Camera3RequestDescriptor::streamsProcessMutex_", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "def3b6730460df91e3643b0a506a5490e0b45bdc", "submitter": { "id": 117, "url": "https://patchwork.libcamera.org/api/1.1/people/117/?format=api", "name": "Cheng-Hao Yang", "email": "chenghaoyang@chromium.org" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/22169/mbox/", "series": [ { "id": 4847, "url": "https://patchwork.libcamera.org/api/1.1/series/4847/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4847", "date": "2024-12-04T16:36:25", "name": "Refactor Android HAL before supporting partial result", "version": 3, "mbox": "https://patchwork.libcamera.org/series/4847/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/22169/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/22169/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 B8A08BDB1C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 4 Dec 2024 16:42:02 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 053BE660ED;\n\tWed, 4 Dec 2024 17:42:01 +0100 (CET)", "from mail-pf1-x430.google.com (mail-pf1-x430.google.com\n\t[IPv6:2607:f8b0:4864:20::430])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DD043660EC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 4 Dec 2024 17:41:56 +0100 (CET)", "by mail-pf1-x430.google.com with SMTP id\n\td2e1a72fcca58-7242f559a9fso30537b3a.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 04 Dec 2024 08:41:56 -0800 (PST)", "from chenghaoyang-low.c.googlers.com.com\n\t(112.212.229.35.bc.googleusercontent.com. [35.229.212.112])\n\tby smtp.gmail.com with ESMTPSA id\n\td2e1a72fcca58-725417725e9sm12551779b3a.80.2024.12.04.08.41.51\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 04 Dec 2024 08:41:52 -0800 (PST)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"kBtH2XNC\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1733330513; x=1733935313;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:mime-version:references:in-reply-to\n\t:message-id:date:subject:cc:to:from:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=LcsRfFPzzANl9rpdLlsmIw3KAWzqxumyatnPESv6yy0=;\n\tb=kBtH2XNCLyNCaqY2wmoLZwNKFtp8K/slmgxxQkziJdvgrH1OqccdVGcUTiCY0uVaOQ\n\tLx5Jhbn2uoPjGUf7wv7jyRP61J43J6YyFqVlvK2ANQV8Kj30yWGKbkX9UXeTfPIkjKdY\n\tyrWr2oLU5v3uyMDegQw+Pht99WBFmgWxtJFeE=", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1733330513; x=1733935313;\n\th=content-transfer-encoding:mime-version:references:in-reply-to\n\t:message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=LcsRfFPzzANl9rpdLlsmIw3KAWzqxumyatnPESv6yy0=;\n\tb=BlicmEQJ24fAW7vl1ZmPbrtISjj7fhIH5zp1gs5827YJ5d2ICf+WQmOYZsQcJT5UP7\n\tKcFzBM90Gw+2v2UDGa8dVr3oE6kwUdxwWxRf1Ld91qbJk+6R1KkZA9/QJUfzADf2BIQl\n\tFcr4hcDJhNgOj0N6iWQ5qhLWO3dTZjiOOds9KKuZRndzNPP5jQ4Xyyrfrovc7y0pf4D6\n\tPKDiFGKomeZ/uEaA8KjFwVYOPHZarHHLi33Wk0Pg1nevUgXpKerfDZK/llusC5aEKwyH\n\tbyScxgbRSglCfZJjq+NKLOIAxlrP4QhzwDWm80zS83ePh6MosvYLhr6/O3lNgNc2+CWw\n\te1tg==", "X-Gm-Message-State": "AOJu0YxdiK6Gbs/8kzEhcaLb7N2ATBCoVh2U9dHoxFsAjyqBml+4yF1R\n\tRp/7gAiX9E32oAjymtTMj6SuoGslGYazuIgxgm2NSUsATSZKsK5DBHCX1GGOQZJy4yt9bHfnWjs\n\t=", "X-Gm-Gg": "ASbGncv5OtQ6w2sXqGIqcYGishjgDY/QcZesQ+BbaGOX2oyqQJSkK7KRFKPQMe0oNbt\n\tWpPA39XXQ4Fc9MRrPlxE/C7jrVs3V0a+xG0oxqJS8oDFoSSVQ3ugc32bFDJrb3JgH6b8HlzNsBZ\n\tp1qxemIdAuXcl8SHi8vnF7D/IyXGCr+MAUiQVasACIvVq5ZpOq2PyE331oSp2UyL99+JjJHJm+j\n\t/YpgoNOlNUX1zJ5YpoZfai6RjkH6mlPgIVdSdxPRi70J1XJPmZy2rP8RnEI4GZEAby1ouEjdDIs\n\t1X19YK9xt1p13sGY6a9LSQzsPyh/QDYLFcOx+K6n7E4vJppb+gD43f9a", "X-Google-Smtp-Source": "AGHT+IHC4shiNXtPjF0zbg6RzFr3W5vWujFZzmYbm4Dm+KerLCezwoM9+4UWYWaj7y1k5fCXq1bDpA==", "X-Received": "by 2002:a05:6a00:3a1b:b0:71e:2a0:b0d0 with SMTP id\n\td2e1a72fcca58-7257fa7167emr9211118b3a.13.1733330513509; \n\tWed, 04 Dec 2024 08:41:53 -0800 (PST)", "From": "Harvey Yang <chenghaoyang@chromium.org>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Harvey Yang <chenghaoyang@chromium.org>,\n\tHan-Lin Chen <hanlinchen@chromium.org>", "Subject": "[PATCH v3 7/7] android: Remove\n\tCamera3RequestDescriptor::streamsProcessMutex_", "Date": "Wed, 4 Dec 2024 16:36:32 +0000", "Message-ID": "<20241204164137.3938891-8-chenghaoyang@chromium.org>", "X-Mailer": "git-send-email 2.47.0.338.g60cca15819-goog", "In-Reply-To": "<20241204164137.3938891-1-chenghaoyang@chromium.org>", "References": "<20241204164137.3938891-1-chenghaoyang@chromium.org>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "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": "This mutex was needed when CameraStream's worker thread posts a result\nback to CameraDevice. We can simplify it by calling CameraDevice's\nfunction on libcamera::Camera's owner thread. With this delegation,\n`Camera3RequestDescriptor::pendingStreamsToProcess_` will be firstly\nsetup in the application's thread in processCaptureRequest(), and the\nrest accesses will be done in libcamera::CameraManager's thread.\n\nSigned-off-by: Han-Lin Chen <hanlinchen@chromium.org>\nCo-developed-by: Harvey Yang <chenghaoyang@chromium.org>\nSigned-off-by: Harvey Yang <chenghaoyang@chromium.org>\n---\n src/android/camera_device.cpp | 33 +++++++++++++++++----------------\n src/android/camera_device.h | 6 ++++--\n src/android/camera_request.h | 4 +---\n src/android/camera_stream.cpp | 4 ++--\n 4 files changed, 24 insertions(+), 23 deletions(-)", "diff": "diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\nindex 18e974f56..0e71a9f0c 100644\n--- a/src/android/camera_device.cpp\n+++ b/src/android/camera_device.cpp\n@@ -990,8 +990,6 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n \t\tFrameBuffer *frameBuffer = nullptr;\n \t\tUniqueFD acquireFence;\n \n-\t\tMutexLocker lock(descriptor->streamsProcessMutex_);\n-\n \t\tswitch (cameraStream->type()) {\n \t\tcase CameraStream::Type::Mapped:\n \t\t\t/* Mapped streams will be handled in the next loop. */\n@@ -1067,7 +1065,6 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n \t\t\t\t<< cameraStream->configuration().pixelFormat << \"]\"\n \t\t\t\t<< \" (mapped)\";\n \n-\t\tMutexLocker lock(descriptor->streamsProcessMutex_);\n \t\tdescriptor->pendingStreamsToProcess_.insert({ cameraStream, &buffer });\n \n \t\t/*\n@@ -1242,9 +1239,6 @@ void CameraDevice::requestComplete(Request *request)\n \t\tdescriptor->resultMetadata_ = std::make_unique<CameraMetadata>(0, 0);\n \t}\n \n-\t/* Handle post-processing. */\n-\tMutexLocker locker(descriptor->streamsProcessMutex_);\n-\n \t/*\n \t * Queue all the post-processing streams request at once. The completion\n \t * slot streamProcessingComplete() can only execute when we are out\n@@ -1272,10 +1266,8 @@ void CameraDevice::requestComplete(Request *request)\n \t\t}\n \t}\n \n-\tif (descriptor->pendingStreamsToProcess_.empty()) {\n-\t\tlocker.unlock();\n+\tif (descriptor->pendingStreamsToProcess_.empty())\n \t\tcompleteDescriptor(descriptor);\n-\t}\n }\n \n /**\n@@ -1382,6 +1374,19 @@ void CameraDevice::setBufferStatus(StreamBuffer &streamBuffer,\n \t}\n }\n \n+void CameraDevice::streamProcessingCompleteDelegate(StreamBuffer *streamBuffer,\n+\t\t\t\t\t\t StreamBuffer::Status status)\n+{\n+\t/*\n+\t * Delegate the callback to the camera manager thread to simplify race condition.\n+\t */\n+\tauto *method = new BoundMethodMember{\n+\t\tthis, camera_.get(), &CameraDevice::streamProcessingComplete, ConnectionTypeQueued\n+\t};\n+\n+\tmethod->activate(streamBuffer, status);\n+}\n+\n /**\n * \\brief Handle post-processing completion of a stream in a capture request\n * \\param[in] streamBuffer The StreamBuffer for which processing is complete\n@@ -1402,13 +1407,9 @@ void CameraDevice::streamProcessingComplete(StreamBuffer *streamBuffer,\n \n \tCamera3RequestDescriptor *request = streamBuffer->request;\n \n-\t{\n-\t\tMutexLocker locker(request->streamsProcessMutex_);\n-\n-\t\trequest->pendingStreamsToProcess_.erase(streamBuffer->stream);\n-\t\tif (!request->pendingStreamsToProcess_.empty())\n-\t\t\treturn;\n-\t}\n+\trequest->pendingStreamsToProcess_.erase(streamBuffer->stream);\n+\tif (!request->pendingStreamsToProcess_.empty())\n+\t\treturn;\n \n \tcompleteDescriptor(streamBuffer->request);\n }\ndiff --git a/src/android/camera_device.h b/src/android/camera_device.h\nindex 699aa8f17..69d163d76 100644\n--- a/src/android/camera_device.h\n+++ b/src/android/camera_device.h\n@@ -65,8 +65,8 @@ public:\n \tint configureStreams(camera3_stream_configuration_t *stream_list);\n \tint processCaptureRequest(camera3_capture_request_t *request);\n \tvoid requestComplete(libcamera::Request *request);\n-\tvoid streamProcessingComplete(StreamBuffer *bufferStream,\n-\t\t\t\t StreamBuffer::Status status);\n+\tvoid streamProcessingCompleteDelegate(StreamBuffer *bufferStream,\n+\t\t\t\t\t StreamBuffer::Status status);\n \n protected:\n \tstd::string logPrefix() const override;\n@@ -96,6 +96,8 @@ private:\n \tint processControls(Camera3RequestDescriptor *descriptor);\n \tvoid completeDescriptor(Camera3RequestDescriptor *descriptor)\n \t\tLIBCAMERA_TSA_EXCLUDES(descriptorsMutex_);\n+\tvoid streamProcessingComplete(StreamBuffer *bufferStream,\n+\t\t\t\t StreamBuffer::Status status);\n \tvoid sendCaptureResults() LIBCAMERA_TSA_REQUIRES(descriptorsMutex_);\n \tvoid sendCaptureResult(Camera3RequestDescriptor *request) const;\n \tvoid setBufferStatus(StreamBuffer &buffer,\ndiff --git a/src/android/camera_request.h b/src/android/camera_request.h\nindex 6b2a00795..bd75d4595 100644\n--- a/src/android/camera_request.h\n+++ b/src/android/camera_request.h\n@@ -66,9 +66,7 @@ public:\n \t};\n \n \t/* Keeps track of streams requiring post-processing. */\n-\tstd::map<CameraStream *, StreamBuffer *> pendingStreamsToProcess_\n-\t\tLIBCAMERA_TSA_GUARDED_BY(streamsProcessMutex_);\n-\tlibcamera::Mutex streamsProcessMutex_;\n+\tstd::map<CameraStream *, StreamBuffer *> pendingStreamsToProcess_;\n \n \tCamera3RequestDescriptor(libcamera::Camera *camera,\n \t\t\t\t const camera3_capture_request_t *camera3Request);\ndiff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\nindex 53f292d4b..7837fd7aa 100644\n--- a/src/android/camera_stream.cpp\n+++ b/src/android/camera_stream.cpp\n@@ -121,8 +121,8 @@ int CameraStream::configure()\n \t\t\t\telse\n \t\t\t\t\tbufferStatus = StreamBuffer::Status::Error;\n \n-\t\t\t\tcameraDevice_->streamProcessingComplete(streamBuffer,\n-\t\t\t\t\t\t\t\t\tbufferStatus);\n+\t\t\t\tcameraDevice_->streamProcessingCompleteDelegate(streamBuffer,\n+\t\t\t\t\t\t\t\t\t\tbufferStatus);\n \t\t\t});\n \n \t\tworker_->start();\n", "prefixes": [ "v3", "7/7" ] }