Patch Detail
Show a patch.
GET /api/patches/22275/?format=api
{ "id": 22275, "url": "https://patchwork.libcamera.org/api/patches/22275/?format=api", "web_url": "https://patchwork.libcamera.org/patch/22275/", "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": "<20241210142557.2886315-8-chenghaoyang@chromium.org>", "date": "2024-12-10T14:24:00", "name": "[v4,7/7] android: Remove Camera3RequestDescriptor::streamsProcessMutex_", "commit_ref": null, "pull_url": null, "state": "new", "archived": false, "hash": "def3b6730460df91e3643b0a506a5490e0b45bdc", "submitter": { "id": 117, "url": "https://patchwork.libcamera.org/api/people/117/?format=api", "name": "Cheng-Hao Yang", "email": "chenghaoyang@chromium.org" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/22275/mbox/", "series": [ { "id": 4870, "url": "https://patchwork.libcamera.org/api/series/4870/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4870", "date": "2024-12-10T14:23:53", "name": "Refactor Android HAL before supporting partial result", "version": 4, "mbox": "https://patchwork.libcamera.org/series/4870/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/22275/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/22275/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 DD16FC32DC\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 10 Dec 2024 14:26:18 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 81B4A67E96;\n\tTue, 10 Dec 2024 15:26:18 +0100 (CET)", "from mail-pj1-x1036.google.com (mail-pj1-x1036.google.com\n\t[IPv6:2607:f8b0:4864:20::1036])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4618C67E88\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 10 Dec 2024 15:26:14 +0100 (CET)", "by mail-pj1-x1036.google.com with SMTP id\n\t98e67ed59e1d1-2ee51f8c47dso4136362a91.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 10 Dec 2024 06:26:14 -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\t98e67ed59e1d1-2ef26ff97f2sm11812053a91.11.2024.12.10.06.26.11\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 10 Dec 2024 06:26:12 -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=\"QemRdkVS\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1733840772; x=1734445572;\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=QsaKE+pnPxyxFfHOwhaGtd2pcOwdYEqO6rU+o+44mQk=;\n\tb=QemRdkVSxr8G9kcjQQ7KWIeHiUlwAuEinwBmJfJ/ItRzGsUmF9NADlljaquNjwF1g7\n\tv5bWk0Yckfad3NYUDx+x5TqaIwWYCA7qcnXXNbtgtB8UT/S+PnOBUc4iCdLiRKqwuGAd\n\tIoHjAPxC+LaEGuViKrziyoEWMaxzM2E6HelGw=", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1733840772; x=1734445572;\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=QsaKE+pnPxyxFfHOwhaGtd2pcOwdYEqO6rU+o+44mQk=;\n\tb=awI1UodWgNSUifOEAuLwgcFx+keEKUi9cP9cKzCkzgz0162+jxp04flYIikzl+mie8\n\tQjsP6FetK41aathUOV9HDk1bLb58Ncve6u7YGZpNUgPkkZPqSGf5B4ou/1FsPPVNTn9Z\n\twS46wZu0ZxGFTmSdN+UvbbNaJNCX+mMTZyDVResbhzEKw3nKs3bukV1kDFzbIt0tW5o+\n\t/Fc0c/raYeYMZ5sprdiIcMnmHKa8l6M2ekxswJL592K6JlkFufg6hoMz2RGgs4bt46oI\n\t9/0i2Ghytho8XNmRIEZEoom3zZcxxu7j1jW231SA0yJiyT3HjTc2s3O3dMi5HWNOUXRN\n\tpJkg==", "X-Gm-Message-State": "AOJu0YzcTttrTDPLj3E7dWYQjVsw7+HTp0SKCzosQ9sgNqH91DyeHwWk\n\t2KFhZtEfULs6l0CR9HXXsyQculZKFYtv4p0thjbWMZI4+3tuxp8Mc0uaPa0SNFN9rqzhNbz7s4k\n\t=", "X-Gm-Gg": "ASbGncv+NDrjZ8OMiyH+7RvK5JE7aWIWI/PFMoMMPiAtV/j52C04Pc+Bs65XxsL/miS\n\tnH4N101vI+cm39lyw+a2MghXEEI2qtk4zEWU6y2eP+3LkNAzQ3Yvok0A591rdr0KO7JuLtzJ8Wa\n\tPEgAHVVJWvI8rCy3M1wsorVYRnUZzv0pdNfBUCdxsTf1WpTctXPmvPW3eFgW2k085efxdLEJztT\n\tbZLzd3Ka/92ImvL2IzOAAUtfEC26Oqt0wG1n7Ny0B7DP1i600yG9o+jZ+Mf+OCMDGi+h4Hq9XH/\n\tmgnquPBefsn+0Ldtfm6h0NLaaiuC9Z8AVCbXCLFfuh8HYDnuTyMVQXWeuBuS", "X-Google-Smtp-Source": "AGHT+IE3YA0vbFyY+16obTdgKt7fZmBsAyX0Dut7K7I8LkoLktIsyBQnza30udAzvhms3Juelm7GCw==", "X-Received": "by 2002:a17:90b:35d2:b0:2ee:f80c:6889 with SMTP id\n\t98e67ed59e1d1-2efcf26e386mr7414361a91.33.1733840772468; \n\tTue, 10 Dec 2024 06:26:12 -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 v4 7/7] android: Remove\n\tCamera3RequestDescriptor::streamsProcessMutex_", "Date": "Tue, 10 Dec 2024 14:24:00 +0000", "Message-ID": "<20241210142557.2886315-8-chenghaoyang@chromium.org>", "X-Mailer": "git-send-email 2.47.0.338.g60cca15819-goog", "In-Reply-To": "<20241210142557.2886315-1-chenghaoyang@chromium.org>", "References": "<20241210142557.2886315-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 a95114c8d..cb819938a 100644\n--- a/src/android/camera_device.cpp\n+++ b/src/android/camera_device.cpp\n@@ -989,8 +989,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@@ -1066,7 +1064,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@@ -1252,9 +1249,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@@ -1288,10 +1282,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@@ -1398,6 +1390,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@@ -1418,13 +1423,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": [ "v4", "7/7" ] }