Patch Detail
Show a patch.
GET /api/patches/8589/?format=api
{ "id": 8589, "url": "https://patchwork.libcamera.org/api/patches/8589/?format=api", "web_url": "https://patchwork.libcamera.org/patch/8589/", "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": "<20200703145404.2246129-1-kieran.bingham@ideasonboard.com>", "date": "2020-07-03T14:54:04", "name": "[libcamera-devel,v3.1,7/8] android: camera_device: Add buffers for each stream to Requests", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "523e7c73eea769d7a05b5fe402e6293c398ee343", "submitter": { "id": 4, "url": "https://patchwork.libcamera.org/api/people/4/?format=api", "name": "Kieran Bingham", "email": "kieran.bingham@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/8589/mbox/", "series": [ { "id": 1078, "url": "https://patchwork.libcamera.org/api/series/1078/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1078", "date": "2020-07-03T14:54:04", "name": null, "version": 1, "mbox": "https://patchwork.libcamera.org/series/1078/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/8589/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/8589/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 5AF7ABE905\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 3 Jul 2020 14:54:16 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CB0F860C57;\n\tFri, 3 Jul 2020 16:54:15 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 217CF603AE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 3 Jul 2020 16:54:15 +0200 (CEST)", "from Q.local (cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net\n\t[86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7BA7029E;\n\tFri, 3 Jul 2020 16:54:14 +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=\"Ven/Dpnd\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593788054;\n\tbh=6SSTFk9rhZ9i+iI5UOPrSzWaCHz0gAYr9gjEat/a1Qg=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=Ven/DpndHftOQ3JrkMKlkKQamLtTbsmQiPwTY178v8N3kXpeWm/mXmFOBWbOZ9JPn\n\t71BlIAOWCsTAuL8Hwgq6auQx3PY/tTtl+01Tp3fRvQoiw2/aGKAs8Flu+TL0D+kBb4\n\tg94B1uj1lXhQwIa8k6oxMEygqFXin+BHMAJJJBIE=", "From": "Kieran Bingham <kieran.bingham@ideasonboard.com>", "To": "libcamera devel <libcamera-devel@lists.libcamera.org>", "Date": "Fri, 3 Jul 2020 15:54:04 +0100", "Message-Id": "<20200703145404.2246129-1-kieran.bingham@ideasonboard.com>", "X-Mailer": "git-send-email 2.25.1", "In-Reply-To": "<20200703123919.2223048-8-kieran.bingham@ideasonboard.com>", "References": "<20200703123919.2223048-8-kieran.bingham@ideasonboard.com>", "MIME-Version": "1.0", "Subject": "[libcamera-devel] [PATCH v3.1 7/8] android: camera_device: Add\n\tbuffers for each stream to Requests", "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>", "Content-Type": "text/plain; charset=\"utf-8\"", "Content-Transfer-Encoding": "base64", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Construct a FrameBuffer for every buffer given in the camera3Request\nand add it to the libcamera Request on the appropriate stream.\n\nThe correct stream is obtained from the private data of the camera3_stream\nassociated with the camera3_buffer.\n\nComments regarding supporting only one buffer are now removed.\n\nSigned-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n src/android/camera_device.cpp | 58 ++++++++++++++++++++---------------\n 1 file changed, 33 insertions(+), 25 deletions(-)\n\nRather than resend a v4 just for the change in this patch - sending a\nv3.1 for direct review of this one.\n\nWithin requestComplete(), the 'first' buffer pointer, which is needed to\nextract the timestamp is moved to directly before it is used, and a\ncomment added to highlight that this is using a single buffer purely for\nthe timestamp.\n\nThen, after calling the process_capture_result callback to the android\nhal, we iterate all buffers in the request and delete them to prevent\nleaks.", "diff": "diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\nindex 28334751a26e..72be4ab86e50 100644\n--- a/src/android/camera_device.cpp\n+++ b/src/android/camera_device.cpp\n@@ -991,6 +991,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)\n \n \t\t/* Maintain internal state of all stream mappings. */\n \t\tstreams_[i].index = streamIndex++;\n+\t\tstream->priv = static_cast<void *>(&streams_[i]);\n \t}\n \n \tswitch (config_->validate()) {\n@@ -1049,9 +1050,6 @@ FrameBuffer *CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer\n \n int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Request)\n {\n-\tStreamConfiguration *streamConfiguration = &config_->at(0);\n-\tStream *stream = streamConfiguration->stream();\n-\n \tif (camera3Request->num_output_buffers != 1) {\n \t\tLOG(HAL, Error) << \"Invalid number of output buffers: \"\n \t\t\t\t<< camera3Request->num_output_buffers;\n@@ -1089,30 +1087,32 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n \t\tcamera_->createRequest(reinterpret_cast<uint64_t>(descriptor));\n \n \tfor (unsigned int i = 0; i < descriptor->numBuffers; ++i) {\n+\t\tCameraStream *cameraStream = static_cast<CameraStream *>(camera3Buffers[i].stream->priv);\n+\n \t\t/*\n \t\t * Keep track of which stream the request belongs to and store\n \t\t * the native buffer handles.\n-\t\t *\n-\t\t * \\todo Currently we only support one capture buffer. Copy\n-\t\t * all of them to be ready once we'll support more.\n \t\t */\n \t\tdescriptor->buffers[i].stream = camera3Buffers[i].stream;\n \t\tdescriptor->buffers[i].buffer = camera3Buffers[i].buffer;\n-\t}\n \n-\t/*\n-\t * Create a libcamera buffer using the dmabuf descriptors of the first\n-\t * and (currently) only supported request buffer.\n-\t */\n-\tFrameBuffer *buffer = createFrameBuffer(*camera3Buffers[0].buffer);\n-\tif (!buffer) {\n-\t\tLOG(HAL, Error) << \"Failed to create buffer\";\n-\t\tdelete request;\n-\t\tdelete descriptor;\n-\t\treturn -ENOMEM;\n-\t}\n+\t\t/*\n+\t\t* Create a libcamera buffer using the dmabuf descriptors of the\n+\t\t* first and (currently) only supported request buffer.\n+\t\t*/\n+\t\tFrameBuffer *buffer = createFrameBuffer(*camera3Buffers[0].buffer);\n+\t\tif (!buffer) {\n+\t\t\tLOG(HAL, Error) << \"Failed to create buffer\";\n+\t\t\tdelete request;\n+\t\t\tdelete descriptor;\n+\t\t\treturn -ENOMEM;\n+\t\t}\n \n-\trequest->addBuffer(stream, buffer);\n+\t\tStreamConfiguration *streamConfiguration = &config_->at(cameraStream->index);\n+\t\tStream *stream = streamConfiguration->stream();\n+\n+\t\trequest->addBuffer(stream, buffer);\n+\t}\n \n \tint ret = camera_->queueRequest(request);\n \tif (ret) {\n@@ -1128,7 +1128,6 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n void CameraDevice::requestComplete(Request *request)\n {\n \tconst std::map<Stream *, FrameBuffer *> &buffers = request->buffers();\n-\tFrameBuffer *buffer = buffers.begin()->second;\n \tcamera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK;\n \tstd::unique_ptr<CameraMetadata> resultMetadata;\n \n@@ -1146,10 +1145,6 @@ void CameraDevice::requestComplete(Request *request)\n \tcaptureResult.frame_number = descriptor->frameNumber;\n \tcaptureResult.num_output_buffers = descriptor->numBuffers;\n \tfor (unsigned int i = 0; i < descriptor->numBuffers; ++i) {\n-\t\t/*\n-\t\t * \\todo Currently we only support one capture buffer. Prepare\n-\t\t * all of them to be ready once we'll support more.\n-\t\t */\n \t\tdescriptor->buffers[i].acquire_fence = -1;\n \t\tdescriptor->buffers[i].release_fence = -1;\n \t\tdescriptor->buffers[i].status = status;\n@@ -1157,6 +1152,14 @@ void CameraDevice::requestComplete(Request *request)\n \tcaptureResult.output_buffers =\n \t\tconst_cast<const camera3_stream_buffer_t *>(descriptor->buffers);\n \n+\t/*\n+\t * \\todo The timestamp used for the metadata is currently always taken\n+\t * from the first buffer (which may be the first stream) in the Request.\n+\t * It might be appropriate to return a 'correct' (as determined by\n+\t * pipeline handlers) timestamp in the Request itself.\n+\t */\n+\tFrameBuffer *buffer = buffers.begin()->second;\n+\n \tif (status == CAMERA3_BUFFER_STATUS_OK) {\n \t\tnotifyShutter(descriptor->frameNumber,\n \t\t\t buffer->metadata().timestamp);\n@@ -1180,8 +1183,13 @@ void CameraDevice::requestComplete(Request *request)\n \n \tcallbacks_->process_capture_result(callbacks_, &captureResult);\n \n+\t/* Release all buffers created with createFrameBuffer(). */\n+\tfor (auto it : buffers) {\n+\t\tFrameBuffer *buffer = it.second;\n+\t\tdelete buffer;\n+\t}\n+\n \tdelete descriptor;\n-\tdelete buffer;\n }\n \n std::string CameraDevice::logPrefix() const\n", "prefixes": [ "libcamera-devel", "v3.1", "7/8" ] }