Patch Detail
Show a patch.
GET /api/1.1/patches/13866/?format=api
{ "id": 13866, "url": "https://patchwork.libcamera.org/api/1.1/patches/13866/?format=api", "web_url": "https://patchwork.libcamera.org/patch/13866/", "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": "<20210920173752.1346190-3-umang.jain@ideasonboard.com>", "date": "2021-09-20T17:37:44", "name": "[libcamera-devel,v3,02/10] android: camera_device: Transform descriptors_ map to queue", "commit_ref": null, "pull_url": null, "state": "rfc", "archived": false, "hash": "c22560e7ed21d523776b7218d7a0fffe499620e3", "submitter": { "id": 86, "url": "https://patchwork.libcamera.org/api/1.1/people/86/?format=api", "name": "Umang Jain", "email": "umang.jain@ideasonboard.com" }, "delegate": { "id": 12, "url": "https://patchwork.libcamera.org/api/1.1/users/12/?format=api", "username": "uajain", "first_name": "Umang", "last_name": "Jain", "email": "umang.jain@ideasonboard.com" }, "mbox": "https://patchwork.libcamera.org/patch/13866/mbox/", "series": [ { "id": 2539, "url": "https://patchwork.libcamera.org/api/1.1/series/2539/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=2539", "date": "2021-09-20T17:37:42", "name": "Async post processor", "version": 3, "mbox": "https://patchwork.libcamera.org/series/2539/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/13866/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/13866/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 581F5BF01C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 20 Sep 2021 17:38:09 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 16BFB69191;\n\tMon, 20 Sep 2021 19:38:09 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1168A69189\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 20 Sep 2021 19:38:05 +0200 (CEST)", "from perceval.ideasonboard.com (unknown [103.251.226.144])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0A0713058;\n\tMon, 20 Sep 2021 19:38:03 +0200 (CEST)" ], "Authentication-Results": "lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"cvkzhpfs\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1632159484;\n\tbh=XVQt9uonzQXaBEPksRJEfGUehrZ3CroApZEdsp7mFxE=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=cvkzhpfshlVPot8rsHVfKQzqCk/UQgSYKjP661bTWBb2Gr3HSlyybo+AW+C+LozqP\n\tiQEXuUdUJOTqijpszzFHNHd5eFeDdDTQPL4uNX5DHx2Q9OeGFULU6gkzSUPuP/a4yR\n\t9Y2YsK8W0aMwif9kXRrnyYDJywZALFxrXI2H3XDw=", "From": "Umang Jain <umang.jain@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Mon, 20 Sep 2021 23:07:44 +0530", "Message-Id": "<20210920173752.1346190-3-umang.jain@ideasonboard.com>", "X-Mailer": "git-send-email 2.31.1", "In-Reply-To": "<20210920173752.1346190-1-umang.jain@ideasonboard.com>", "References": "<20210920173752.1346190-1-umang.jain@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v3 02/10] android: camera_device:\n\tTransform descriptors_ map to queue", "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 descriptors_ map hold Camera3RequestDescriptor(s) which are\nper-capture request placed by the framework to libcamera HAL.\nCameraDevice::requestComplete() looks for the descriptor for which the\ncamera request has been completed and removes it from the map.\nHowever, this approach has its limitations going forwards.\n\nIn subsequent commits, the post-processing operation which happens\nin requestComplete() synchronously, is going to be run asynchronously.\nTherefore, instead of a map for descriptors, a queue makes more sense\ngoing forwards and the framework expects capture results to be received\nin the same order as they were queued. When the async processing is\ncompleted, the descriptor queue is inspected to send back the capture\nresults and then de-queued. This helps to maintain the order of sending\nback the capture results whilst preventing unnecessary complexity of\nusing a map.\n\nSigned-off-by: Umang Jain <umang.jain@ideasonboard.com>\n---\n src/android/camera_device.cpp | 89 ++++++++++++++++++-----------------\n src/android/camera_device.h | 5 +-\n 2 files changed, 48 insertions(+), 46 deletions(-)", "diff": "diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\nindex f461e14c..0562c225 100644\n--- a/src/android/camera_device.cpp\n+++ b/src/android/camera_device.cpp\n@@ -926,7 +926,9 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n \t * The descriptor and the associated memory reserved here are freed\n \t * at request complete time.\n \t */\n-\tCamera3RequestDescriptor descriptor(camera_.get(), camera3Request);\n+\tstd::unique_ptr<Camera3RequestDescriptor> descriptor =\n+\t\tstd::make_unique<Camera3RequestDescriptor>(camera_.get(),\n+\t\t\t\t\t\t\t camera3Request);\n \n \t/*\n \t * \\todo The Android request model is incremental, settings passed in\n@@ -937,12 +939,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n \tif (camera3Request->settings)\n \t\tlastSettings_ = camera3Request->settings;\n \telse\n-\t\tdescriptor.settings_ = lastSettings_;\n+\t\tdescriptor->settings_ = lastSettings_;\n \n-\tLOG(HAL, Debug) << \"Queueing request \" << descriptor.request_->cookie()\n-\t\t\t<< \" with \" << descriptor.buffers_.size() << \" streams\";\n-\tfor (unsigned int i = 0; i < descriptor.buffers_.size(); ++i) {\n-\t\tconst camera3_stream_buffer_t &camera3Buffer = descriptor.buffers_[i];\n+\tLOG(HAL, Debug) << \"Queueing request \" << descriptor->request_->cookie()\n+\t\t\t<< \" with \" << descriptor->buffers_.size() << \" streams\";\n+\tfor (unsigned int i = 0; i < descriptor->buffers_.size(); ++i) {\n+\t\tconst camera3_stream_buffer_t &camera3Buffer = descriptor->buffers_[i];\n \t\tcamera3_stream *camera3Stream = camera3Buffer.stream;\n \t\tCameraStream *cameraStream = static_cast<CameraStream *>(camera3Stream->priv);\n \n@@ -977,7 +979,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n \t\t\tbuffer = createFrameBuffer(*camera3Buffer.buffer,\n \t\t\t\t\t\t cameraStream->configuration().pixelFormat,\n \t\t\t\t\t\t cameraStream->configuration().size);\n-\t\t\tdescriptor.frameBuffers_.emplace_back(buffer);\n+\t\t\tdescriptor->frameBuffers_.emplace_back(buffer);\n \t\t\tLOG(HAL, Debug) << ss.str() << \" (direct)\";\n \t\t\tbreak;\n \n@@ -999,7 +1001,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n \t\t\treturn -ENOMEM;\n \t\t}\n \n-\t\tdescriptor.request_->addBuffer(cameraStream->stream(), buffer,\n+\t\tdescriptor->request_->addBuffer(cameraStream->stream(), buffer,\n \t\t\t\t\t\tcamera3Buffer.acquire_fence);\n \t}\n \n@@ -1007,7 +1009,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n \t * Translate controls from Android to libcamera and queue the request\n \t * to the CameraWorker thread.\n \t */\n-\tint ret = processControls(&descriptor);\n+\tint ret = processControls(descriptor.get());\n \tif (ret)\n \t\treturn ret;\n \n@@ -1035,11 +1037,11 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n \t\tstate_ = State::Running;\n \t}\n \n-\tworker_.queueRequest(descriptor.request_.get());\n+\tworker_.queueRequest(descriptor->request_.get());\n \n \t{\n \t\tMutexLocker descriptorsLock(descriptorsMutex_);\n-\t\tdescriptors_[descriptor.request_->cookie()] = std::move(descriptor);\n+\t\tdescriptors_.push_back(std::move(descriptor));\n \t}\n \n \treturn 0;\n@@ -1047,26 +1049,22 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n \n void CameraDevice::requestComplete(Request *request)\n {\n-\tdecltype(descriptors_)::node_type node;\n-\t{\n-\t\tMutexLocker descriptorsLock(descriptorsMutex_);\n-\t\tauto it = descriptors_.find(request->cookie());\n-\t\tif (it == descriptors_.end()) {\n-\t\t\t/*\n-\t\t\t * \\todo Clarify if the Camera has to be closed on\n-\t\t\t * ERROR_DEVICE and possibly demote the Fatal to simple\n-\t\t\t * Error.\n-\t\t\t */\n-\t\t\tnotifyError(0, nullptr, CAMERA3_MSG_ERROR_DEVICE);\n-\t\t\tLOG(HAL, Fatal)\n-\t\t\t\t<< \"Unknown request: \" << request->cookie();\n+\tif (descriptors_.empty())\n+\t\treturn;\n \n-\t\t\treturn;\n-\t\t}\n+\tCamera3RequestDescriptor *descriptor = descriptors_.front().get();\n+\tif (descriptor->request_->cookie() != request->cookie()) {\n+\t\t/*\n+\t\t * \\todo Clarify if the Camera has to be closed on\n+\t\t * ERROR_DEVICE and possibly demote the Fatal to simple\n+\t\t * Error.\n+\t\t */\n+\t\tnotifyError(0, nullptr, CAMERA3_MSG_ERROR_DEVICE);\n+\t\tLOG(HAL, Fatal)\n+\t\t\t<< \"Unknown request: \" << request->cookie();\n \n-\t\tnode = descriptors_.extract(it);\n+\t\treturn;\n \t}\n-\tCamera3RequestDescriptor &descriptor = node.mapped();\n \n \t/*\n \t * Prepare the capture result for the Android camera stack.\n@@ -1075,14 +1073,14 @@ void CameraDevice::requestComplete(Request *request)\n \t * post-processing/compression fails.\n \t */\n \tcamera3_capture_result_t captureResult = {};\n-\tcaptureResult.frame_number = descriptor.frameNumber_;\n-\tcaptureResult.num_output_buffers = descriptor.buffers_.size();\n-\tfor (camera3_stream_buffer_t &buffer : descriptor.buffers_) {\n+\tcaptureResult.frame_number = descriptor->frameNumber_;\n+\tcaptureResult.num_output_buffers = descriptor->buffers_.size();\n+\tfor (camera3_stream_buffer_t &buffer : descriptor->buffers_) {\n \t\tbuffer.acquire_fence = -1;\n \t\tbuffer.release_fence = -1;\n \t\tbuffer.status = CAMERA3_BUFFER_STATUS_OK;\n \t}\n-\tcaptureResult.output_buffers = descriptor.buffers_.data();\n+\tcaptureResult.output_buffers = descriptor->buffers_.data();\n \tcaptureResult.partial_result = 1;\n \n \t/*\n@@ -1094,14 +1092,15 @@ void CameraDevice::requestComplete(Request *request)\n \t\t\t\t<< \" not successfully completed: \"\n \t\t\t\t<< request->status();\n \n-\t\tnotifyError(descriptor.frameNumber_, nullptr,\n+\t\tnotifyError(descriptor->frameNumber_, nullptr,\n \t\t\t CAMERA3_MSG_ERROR_REQUEST);\n \n \t\tcaptureResult.partial_result = 0;\n-\t\tfor (camera3_stream_buffer_t &buffer : descriptor.buffers_)\n+\t\tfor (camera3_stream_buffer_t &buffer : descriptor->buffers_)\n \t\t\tbuffer.status = CAMERA3_BUFFER_STATUS_ERROR;\n \t\tcallbacks_->process_capture_result(callbacks_, &captureResult);\n \n+\t\tdescriptors_.pop_front();\n \t\treturn;\n \t}\n \n@@ -1113,10 +1112,10 @@ void CameraDevice::requestComplete(Request *request)\n \t */\n \tuint64_t sensorTimestamp = static_cast<uint64_t>(request->metadata()\n \t\t\t\t\t\t\t .get(controls::SensorTimestamp));\n-\tnotifyShutter(descriptor.frameNumber_, sensorTimestamp);\n+\tnotifyShutter(descriptor->frameNumber_, sensorTimestamp);\n \n \tLOG(HAL, Debug) << \"Request \" << request->cookie() << \" completed with \"\n-\t\t\t<< descriptor.buffers_.size() << \" streams\";\n+\t\t\t<< descriptor->buffers_.size() << \" streams\";\n \n \t/*\n \t * Generate the metadata associated with the captured buffers.\n@@ -1126,14 +1125,14 @@ void CameraDevice::requestComplete(Request *request)\n \t */\n \tstd::unique_ptr<CameraMetadata> resultMetadata = getResultMetadata(descriptor);\n \tif (!resultMetadata) {\n-\t\tnotifyError(descriptor.frameNumber_, nullptr, CAMERA3_MSG_ERROR_RESULT);\n+\t\tnotifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_RESULT);\n \n \t\t/* The camera framework expects an empty metadata pack on error. */\n \t\tresultMetadata = std::make_unique<CameraMetadata>(0, 0);\n \t}\n \n \t/* Handle any JPEG compression. */\n-\tfor (camera3_stream_buffer_t &buffer : descriptor.buffers_) {\n+\tfor (camera3_stream_buffer_t &buffer : descriptor->buffers_) {\n \t\tCameraStream *cameraStream =\n \t\t\tstatic_cast<CameraStream *>(buffer.stream->priv);\n \n@@ -1144,13 +1143,13 @@ void CameraDevice::requestComplete(Request *request)\n \t\tif (!src) {\n \t\t\tLOG(HAL, Error) << \"Failed to find a source stream buffer\";\n \t\t\tbuffer.status = CAMERA3_BUFFER_STATUS_ERROR;\n-\t\t\tnotifyError(descriptor.frameNumber_, buffer.stream,\n+\t\t\tnotifyError(descriptor->frameNumber_, buffer.stream,\n \t\t\t\t CAMERA3_MSG_ERROR_BUFFER);\n \t\t\tcontinue;\n \t\t}\n \n \t\tint ret = cameraStream->process(*src, *buffer.buffer,\n-\t\t\t\t\t\tdescriptor.settings_,\n+\t\t\t\t\t\tdescriptor->settings_,\n \t\t\t\t\t\tresultMetadata.get());\n \t\t/*\n \t\t * Return the FrameBuffer to the CameraStream now that we're\n@@ -1161,13 +1160,15 @@ void CameraDevice::requestComplete(Request *request)\n \n \t\tif (ret) {\n \t\t\tbuffer.status = CAMERA3_BUFFER_STATUS_ERROR;\n-\t\t\tnotifyError(descriptor.frameNumber_, buffer.stream,\n+\t\t\tnotifyError(descriptor->frameNumber_, buffer.stream,\n \t\t\t\t CAMERA3_MSG_ERROR_BUFFER);\n \t\t}\n \t}\n \n \tcaptureResult.result = resultMetadata->get();\n \tcallbacks_->process_capture_result(callbacks_, &captureResult);\n+\n+\tdescriptors_.pop_front();\n }\n \n std::string CameraDevice::logPrefix() const\n@@ -1203,10 +1204,10 @@ void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream,\n * Produce a set of fixed result metadata.\n */\n std::unique_ptr<CameraMetadata>\n-CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) const\n+CameraDevice::getResultMetadata(const Camera3RequestDescriptor *descriptor) const\n {\n-\tconst ControlList &metadata = descriptor.request_->metadata();\n-\tconst CameraMetadata &settings = descriptor.settings_;\n+\tconst ControlList &metadata = descriptor->request_->metadata();\n+\tconst CameraMetadata &settings = descriptor->settings_;\n \tcamera_metadata_ro_entry_t entry;\n \tbool found;\n \ndiff --git a/src/android/camera_device.h b/src/android/camera_device.h\nindex a5576927..9c895b25 100644\n--- a/src/android/camera_device.h\n+++ b/src/android/camera_device.h\n@@ -7,6 +7,7 @@\n #ifndef __ANDROID_CAMERA_DEVICE_H__\n #define __ANDROID_CAMERA_DEVICE_H__\n \n+#include <deque>\n #include <map>\n #include <memory>\n #include <mutex>\n@@ -103,7 +104,7 @@ private:\n \t\t\t camera3_error_msg_code code);\n \tint processControls(Camera3RequestDescriptor *descriptor);\n \tstd::unique_ptr<CameraMetadata> getResultMetadata(\n-\t\tconst Camera3RequestDescriptor &descriptor) const;\n+\t\tconst Camera3RequestDescriptor *descriptor) const;\n \n \tunsigned int id_;\n \tcamera3_device_t camera3Device_;\n@@ -123,7 +124,7 @@ private:\n \tstd::vector<CameraStream> streams_;\n \n \tlibcamera::Mutex descriptorsMutex_; /* Protects descriptors_. */\n-\tstd::map<uint64_t, Camera3RequestDescriptor> descriptors_;\n+\tstd::deque<std::unique_ptr<Camera3RequestDescriptor>> descriptors_;\n \n \tstd::string maker_;\n \tstd::string model_;\n", "prefixes": [ "libcamera-devel", "v3", "02/10" ] }