{"id":13948,"url":"https://patchwork.libcamera.org/api/1.1/patches/13948/?format=json","web_url":"https://patchwork.libcamera.org/patch/13948/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20210927111149.692004-3-umang.jain@ideasonboard.com>","date":"2021-09-27T11:11:48","name":"[libcamera-devel,v1,2/3] android: camera_device: Transform descriptors_ map to queue","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"74dfd8c05f05e3fca14d6350bec57d04570bfde6","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/1.1/people/86/?format=json","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"delegate":{"id":12,"url":"https://patchwork.libcamera.org/api/1.1/users/12/?format=json","username":"uajain","first_name":"Umang","last_name":"Jain","email":"umang.jain@ideasonboard.com"},"mbox":"https://patchwork.libcamera.org/patch/13948/mbox/","series":[{"id":2573,"url":"https://patchwork.libcamera.org/api/1.1/series/2573/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=2573","date":"2021-09-27T11:11:46","name":"Camera3RequestDescriptors std::map => deque","version":1,"mbox":"https://patchwork.libcamera.org/series/2573/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/13948/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/13948/checks/","tags":{},"headers":{"Return-Path":"","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 0DBB6BDC71\n\tfor ;\n\tMon, 27 Sep 2021 11:12:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CF95969192;\n\tMon, 27 Sep 2021 13:12:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E461469195\n\tfor ;\n\tMon, 27 Sep 2021 13:12:04 +0200 (CEST)","from perceval.ideasonboard.com (unknown [103.251.226.181])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A4E41B91;\n\tMon, 27 Sep 2021 13:12: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=\"FnWNh9c7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1632741124;\n\tbh=kvHlwP8soFdj4qGG3oiIfv9lQ9BqT5Zd3RSOMU450l0=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=FnWNh9c7bT6260vy2VwDvVRt2E20PBl89606NNkWDfQf1z3w/A9X7YmJocQL9Aj50\n\tz+lILTtlpTJyEW0BUrQtd589vMVcEfAbhnHI+WBhjWjBvJd2uEi3in+aPLBpo36QJ1\n\tC/Y6n0lTdm3F0xFY3WzrqcG8y7tUzZBv7dIxTNPo=","From":"Umang Jain ","To":"libcamera-devel@lists.libcamera.org","Date":"Mon, 27 Sep 2021 16:41:48 +0530","Message-Id":"<20210927111149.692004-3-umang.jain@ideasonboard.com>","X-Mailer":"git-send-email 2.31.1","In-Reply-To":"<20210927111149.692004-1-umang.jain@ideasonboard.com>","References":"<20210927111149.692004-1-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v1 2/3] android: camera_device: Transform\n\tdescriptors_ map to queue","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"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.\nSince, the request is placed in form of FIFO and the framework expects\nthe order of completion to be FIFO as well, this calls for a need of\na queue rather than a std::map.\n\nThis patch still keeps the same lifetime of Camera3RequestDescriptor as\nbefore i.e. in the requestComplete(). Previously, a descriptor is\nextracted from the map and its lifetime is bound to requestComplete().\nThe lifetime is kept same by manually calling .pop_front() on the\nqueue. In the subsequent commit, this is likely to change with a\ncentralized location of dropping descriptors from the queue for request\ncompletion.\n\nSigned-off-by: Umang Jain \n---\n src/android/camera_device.cpp | 91 ++++++++++++++++++-----------------\n src/android/camera_device.h | 5 +-\n 2 files changed, 49 insertions(+), 47 deletions(-)","diff":"diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\nindex 8b5dd7a1..b0b7f4fd 100644\n--- a/src/android/camera_device.cpp\n+++ b/src/android/camera_device.cpp\n@@ -955,7 +955,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 descriptor =\n+\t\tstd::make_unique(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@@ -966,12 +968,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(camera3Stream->priv);\n \n@@ -1003,12 +1005,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n \t\t\t * associate it with the Camera3RequestDescriptor for\n \t\t\t * lifetime management only.\n \t\t\t */\n-\t\t\tdescriptor.frameBuffers_.push_back(\n+\t\t\tdescriptor->frameBuffers_.push_back(\n \t\t\t\tcreateFrameBuffer(*camera3Buffer.buffer,\n \t\t\t\t\t\t cameraStream->configuration().pixelFormat,\n \t\t\t\t\t\t cameraStream->configuration().size));\n \n-\t\t\tbuffer = descriptor.frameBuffers_.back().get();\n+\t\t\tbuffer = descriptor->frameBuffers_.back().get();\n \t\t\tLOG(HAL, Debug) << ss.str() << \" (direct)\";\n \t\t\tbreak;\n \n@@ -1030,7 +1032,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@@ -1038,7 +1040,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@@ -1066,11 +1068,11 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n \t\tstate_ = State::Running;\n \t}\n \n-\tCaptureRequest *request = descriptor.request_.get();\n+\tCaptureRequest *request = 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 \tworker_.queueRequest(request);\n@@ -1080,31 +1082,27 @@ 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 \tcamera3_capture_result_t captureResult = {};\n-\tcaptureResult.frame_number = descriptor.frameNumber_;\n-\tcaptureResult.num_output_buffers = descriptor.buffers_.size();\n-\tcaptureResult.output_buffers = descriptor.buffers_.data();\n+\tcaptureResult.frame_number = descriptor->frameNumber_;\n+\tcaptureResult.num_output_buffers = descriptor->buffers_.size();\n+\tcaptureResult.output_buffers = descriptor->buffers_.data();\n \n \t/*\n \t * If the Request has failed, abort the request by notifying the error\n@@ -1115,11 +1113,11 @@ 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\tCameraStream *cameraStream =\n \t\t\t\tstatic_cast(buffer.stream->priv);\n \n@@ -1142,6 +1140,7 @@ void CameraDevice::requestComplete(Request *request)\n \t\t}\n \t\tcallbacks_->process_capture_result(callbacks_, &captureResult);\n \n+\t\tdescriptors_.pop_front();\n \t\treturn;\n \t}\n \n@@ -1153,10 +1152,10 @@ void CameraDevice::requestComplete(Request *request)\n \t */\n \tuint64_t sensorTimestamp = static_cast(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@@ -1166,14 +1165,14 @@ void CameraDevice::requestComplete(Request *request)\n \t */\n \tstd::unique_ptr 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(0, 0);\n \t}\n \n \t/* Handle post-processing. */\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(buffer.stream->priv);\n \n@@ -1184,13 +1183,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,\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@@ -1201,7 +1200,7 @@ 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@@ -1210,7 +1209,7 @@ void CameraDevice::requestComplete(Request *request)\n \t * Finalize the capture result by setting fences and buffer status\n \t * before executing the callback.\n \t */\n-\tfor (camera3_stream_buffer_t &buffer : descriptor.buffers_) {\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@@ -1219,6 +1218,8 @@ void CameraDevice::requestComplete(Request *request)\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@@ -1254,10 +1255,10 @@ void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream,\n * Produce a set of fixed result metadata.\n */\n std::unique_ptr\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 43eb5895..5889a0e7 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 \n #include \n #include \n #include \n@@ -105,7 +106,7 @@ private:\n \t\t\t camera3_error_msg_code code);\n \tint processControls(Camera3RequestDescriptor *descriptor);\n \tstd::unique_ptr getResultMetadata(\n-\t\tconst Camera3RequestDescriptor &descriptor) const;\n+\t\tconst Camera3RequestDescriptor *descriptor) const;\n \n \tunsigned int id_;\n \tcamera3_device_t camera3Device_;\n@@ -125,7 +126,7 @@ private:\n \tstd::vector streams_;\n \n \tlibcamera::Mutex descriptorsMutex_; /* Protects descriptors_. */\n-\tstd::map descriptors_;\n+\tstd::deque> descriptors_;\n \n \tstd::string maker_;\n \tstd::string model_;\n","prefixes":["libcamera-devel","v1","2/3"]}