Patch Detail
Show a patch.
GET /api/1.1/patches/14179/?format=api
{ "id": 14179, "url": "https://patchwork.libcamera.org/api/1.1/patches/14179/?format=api", "web_url": "https://patchwork.libcamera.org/patch/14179/", "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": "<20211019114802.665980-6-umang.jain@ideasonboard.com>", "date": "2021-10-19T11:47:55", "name": "[libcamera-devel,v2,05/12] android: camera_device: Create struct to track per stream buffer", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "f3af08931ea7b02ba3012fda16438ffc988f9366", "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/14179/mbox/", "series": [ { "id": 2638, "url": "https://patchwork.libcamera.org/api/1.1/series/2638/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=2638", "date": "2021-10-19T11:47:50", "name": "android: Overhaul request handling", "version": 2, "mbox": "https://patchwork.libcamera.org/series/2638/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/14179/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/14179/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 742A6C324C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 19 Oct 2021 11:48:22 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3743768F59;\n\tTue, 19 Oct 2021 13:48:22 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 893BA604FE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 19 Oct 2021 13:48:21 +0200 (CEST)", "from perceval.ideasonboard.com (unknown [103.251.226.98])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D2C1812A;\n\tTue, 19 Oct 2021 13:48:19 +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=\"tYcudbQX\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1634644101;\n\tbh=ZdCFvEFMpdvApvGOrr3vA2fbqvGFEVLxWkdJHVqMk38=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=tYcudbQXWCFiUQdX3cPwc9F/GXecgto2xqVDjOrpPbmR4FG6MgoccfbkSMC98gV54\n\tQVdph+nC7Lpl6pfoBXkP4YAWs1A4WKPRlQY8C7fXQwkj3ZpGSyvDg4mI5OaUWlkHCt\n\tmGIkeQ3bIoN8IqYFgWqbcX0wFEnhqoQ8MXo2WJ+s=", "From": "Umang Jain <umang.jain@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Tue, 19 Oct 2021 17:17:55 +0530", "Message-Id": "<20211019114802.665980-6-umang.jain@ideasonboard.com>", "X-Mailer": "git-send-email 2.31.1", "In-Reply-To": "<20211019114802.665980-1-umang.jain@ideasonboard.com>", "References": "<20211019114802.665980-1-umang.jain@ideasonboard.com>", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH v2 05/12] android: camera_device: Create\n\tstruct to track per stream buffer", "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 Camera3RequestDescriptor structure stores, for each stream, the\ncamera3_stream_buffer_t and the libcamera FrameBuffer in two separate\nvectors. This complicates buffer handling, as the code needs to keep\nboth vectors in sync. Create a new structure to group all data about\nper-stream buffers to simplify this.\n\nAs a side effect, we need to create a local vector of\ncamera3_stream_buffer_t in CameraDevice::sendCaptureResults() as the\ncamera3_stream_buffer_t instances stored in the new structure in\nCamera3RequestDescriptor are not contiguous anymore. This is a small\nprice to pay for easier handling of buffers, and will be refactored in\nsubsequent commits anyway.\n\nSigned-off-by: Umang Jain <umang.jain@ideasonboard.com>\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\nReviewed-by: Hirokazu Honda <hiroh@chromium.org>\n---\n src/android/camera_device.cpp | 75 ++++++++++++++++++----------------\n src/android/camera_request.cpp | 9 +---\n src/android/camera_request.h | 15 ++++++-\n 3 files changed, 55 insertions(+), 44 deletions(-)", "diff": "diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\nindex a8c66a79..74d9b952 100644\n--- a/src/android/camera_device.cpp\n+++ b/src/android/camera_device.cpp\n@@ -802,9 +802,9 @@ void CameraDevice::abortRequest(Camera3RequestDescriptor *descriptor) const\n \tnotifyError(descriptor->frameNumber_, nullptr, CAMERA3_MSG_ERROR_REQUEST);\n \n \tfor (auto &buffer : descriptor->buffers_) {\n-\t\tbuffer.release_fence = buffer.acquire_fence;\n-\t\tbuffer.acquire_fence = -1;\n-\t\tbuffer.status = CAMERA3_BUFFER_STATUS_ERROR;\n+\t\tbuffer.buffer.release_fence = buffer.buffer.acquire_fence;\n+\t\tbuffer.buffer.acquire_fence = -1;\n+\t\tbuffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR;\n \t}\n \n \tdescriptor->status_ = Camera3RequestDescriptor::Status::Error;\n@@ -902,8 +902,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n \tLOG(HAL, Debug) << \"Queueing request \" << descriptor->request_->cookie()\n \t\t\t<< \" with \" << descriptor->buffers_.size() << \" streams\";\n \n-\tfor (const auto &[i, camera3Buffer] : utils::enumerate(descriptor->buffers_)) {\n-\t\tcamera3_stream *camera3Stream = camera3Buffer.stream;\n+\tfor (const auto &[i, buffer] : utils::enumerate(descriptor->buffers_)) {\n+\t\tcamera3_stream *camera3Stream = buffer.buffer.stream;\n \t\tCameraStream *cameraStream = static_cast<CameraStream *>(camera3Stream->priv);\n \n \t\tstd::stringstream ss;\n@@ -920,7 +920,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n \t\t * while fences for streams of type Internal and Mapped are\n \t\t * handled at post-processing time.\n \t\t */\n-\t\tFrameBuffer *buffer = nullptr;\n+\t\tFrameBuffer *frameBuffer = nullptr;\n \t\tint acquireFence = -1;\n \t\tswitch (cameraStream->type()) {\n \t\tcase CameraStream::Type::Mapped:\n@@ -938,13 +938,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\t\tcreateFrameBuffer(*camera3Buffer.buffer,\n+\t\t\tbuffer.frameBuffer =\n+\t\t\t\tcreateFrameBuffer(*buffer.buffer.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\tacquireFence = camera3Buffer.acquire_fence;\n+\t\t\t\t\t\t cameraStream->configuration().size);\n+\t\t\tframeBuffer = buffer.frameBuffer.get();\n+\t\t\tacquireFence = buffer.buffer.acquire_fence;\n \t\t\tLOG(HAL, Debug) << ss.str() << \" (direct)\";\n \t\t\tbreak;\n \n@@ -956,18 +955,18 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques\n \t\t\t * The buffer has to be returned to the CameraStream\n \t\t\t * once it has been processed.\n \t\t\t */\n-\t\t\tbuffer = cameraStream->getBuffer();\n+\t\t\tframeBuffer = cameraStream->getBuffer();\n \t\t\tLOG(HAL, Debug) << ss.str() << \" (internal)\";\n \t\t\tbreak;\n \t\t}\n \n-\t\tif (!buffer) {\n-\t\t\tLOG(HAL, Error) << \"Failed to create buffer\";\n+\t\tif (!frameBuffer) {\n+\t\t\tLOG(HAL, Error) << \"Failed to create frame buffer\";\n \t\t\treturn -ENOMEM;\n \t\t}\n \n-\t\tdescriptor->request_->addBuffer(cameraStream->stream(), buffer,\n-\t\t\t\t\t acquireFence);\n+\t\tdescriptor->request_->addBuffer(cameraStream->stream(),\n+\t\t\t\t\t\tframeBuffer, acquireFence);\n \t}\n \n \t/*\n@@ -1054,9 +1053,9 @@ void CameraDevice::requestComplete(Request *request)\n \t * The buffer status is set to OK and later changed to ERROR if\n \t * post-processing/compression fails.\n \t */\n-\tfor (camera3_stream_buffer_t &buffer : descriptor->buffers_) {\n+\tfor (auto &buffer : descriptor->buffers_) {\n \t\tCameraStream *cameraStream =\n-\t\t\tstatic_cast<CameraStream *>(buffer.stream->priv);\n+\t\t\tstatic_cast<CameraStream *>(buffer.buffer.stream->priv);\n \n \t\t/*\n \t\t * Streams of type Direct have been queued to the\n@@ -1070,9 +1069,9 @@ void CameraDevice::requestComplete(Request *request)\n \t\t * fence to -1 once it has handled it and remove this check.\n \t\t */\n \t\tif (cameraStream->type() == CameraStream::Type::Direct)\n-\t\t\tbuffer.acquire_fence = -1;\n-\t\tbuffer.release_fence = -1;\n-\t\tbuffer.status = CAMERA3_BUFFER_STATUS_OK;\n+\t\t\tbuffer.buffer.acquire_fence = -1;\n+\t\tbuffer.buffer.release_fence = -1;\n+\t\tbuffer.buffer.status = CAMERA3_BUFFER_STATUS_OK;\n \t}\n \n \t/*\n@@ -1087,15 +1086,15 @@ void CameraDevice::requestComplete(Request *request)\n \t\tnotifyError(descriptor->frameNumber_, nullptr,\n \t\t\t CAMERA3_MSG_ERROR_REQUEST);\n \n-\t\tfor (camera3_stream_buffer_t &buffer : descriptor->buffers_) {\n+\t\tfor (auto &buffer : descriptor->buffers_) {\n \t\t\t/*\n \t\t\t * Signal to the framework it has to handle fences that\n \t\t\t * have not been waited on by setting the release fence\n \t\t\t * to the acquire fence value.\n \t\t\t */\n-\t\t\tbuffer.release_fence = buffer.acquire_fence;\n-\t\t\tbuffer.acquire_fence = -1;\n-\t\t\tbuffer.status = CAMERA3_BUFFER_STATUS_ERROR;\n+\t\t\tbuffer.buffer.release_fence = buffer.buffer.acquire_fence;\n+\t\t\tbuffer.buffer.acquire_fence = -1;\n+\t\t\tbuffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR;\n \t\t}\n \n \t\tdescriptor->status_ = Camera3RequestDescriptor::Status::Error;\n@@ -1137,9 +1136,9 @@ void CameraDevice::requestComplete(Request *request)\n \t}\n \n \t/* Handle post-processing. */\n-\tfor (camera3_stream_buffer_t &buffer : descriptor->buffers_) {\n+\tfor (auto &buffer : descriptor->buffers_) {\n \t\tCameraStream *cameraStream =\n-\t\t\tstatic_cast<CameraStream *>(buffer.stream->priv);\n+\t\t\tstatic_cast<CameraStream *>(buffer.buffer.stream->priv);\n \n \t\tif (cameraStream->type() == CameraStream::Type::Direct)\n \t\t\tcontinue;\n@@ -1147,13 +1146,13 @@ void CameraDevice::requestComplete(Request *request)\n \t\tFrameBuffer *src = request->findBuffer(cameraStream->stream());\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\tbuffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR;\n+\t\t\tnotifyError(descriptor->frameNumber_, buffer.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, descriptor);\n+\t\tint ret = cameraStream->process(*src, buffer.buffer, descriptor);\n \n \t\t/*\n \t\t * Return the FrameBuffer to the CameraStream now that we're\n@@ -1163,8 +1162,8 @@ void CameraDevice::requestComplete(Request *request)\n \t\t\tcameraStream->putBuffer(src);\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\tbuffer.buffer.status = CAMERA3_BUFFER_STATUS_ERROR;\n+\t\t\tnotifyError(descriptor->frameNumber_, buffer.buffer.stream,\n \t\t\t\t CAMERA3_MSG_ERROR_BUFFER);\n \t\t}\n \t}\n@@ -1190,10 +1189,16 @@ void CameraDevice::sendCaptureResults()\n \t\tcamera3_capture_result_t captureResult = {};\n \n \t\tcaptureResult.frame_number = descriptor->frameNumber_;\n+\n \t\tif (descriptor->resultMetadata_)\n \t\t\tcaptureResult.result = descriptor->resultMetadata_->get();\n-\t\tcaptureResult.num_output_buffers = descriptor->buffers_.size();\n-\t\tcaptureResult.output_buffers = descriptor->buffers_.data();\n+\n+\t\tstd::vector<camera3_stream_buffer_t> resultBuffers;\n+\t\tfor (const auto &buffer : descriptor->buffers_)\n+\t\t\tresultBuffers.emplace_back(buffer.buffer);\n+\n+\t\tcaptureResult.num_output_buffers = resultBuffers.size();\n+\t\tcaptureResult.output_buffers = resultBuffers.data();\n \n \t\tif (descriptor->status_ == Camera3RequestDescriptor::Status::Success)\n \t\t\tcaptureResult.partial_result = 1;\ndiff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp\nindex 16a632b3..614baed4 100644\n--- a/src/android/camera_request.cpp\n+++ b/src/android/camera_request.cpp\n@@ -23,15 +23,10 @@ Camera3RequestDescriptor::Camera3RequestDescriptor(\n \n \t/* Copy the camera3 request stream information for later access. */\n \tconst uint32_t numBuffers = camera3Request->num_output_buffers;\n+\n \tbuffers_.resize(numBuffers);\n \tfor (uint32_t i = 0; i < numBuffers; i++)\n-\t\tbuffers_[i] = camera3Request->output_buffers[i];\n-\n-\t/*\n-\t * FrameBuffer instances created by wrapping a camera3 provided dmabuf\n-\t * are emplaced in this vector of unique_ptr<> for lifetime management.\n-\t */\n-\tframeBuffers_.reserve(numBuffers);\n+\t\tbuffers_[i].buffer = camera3Request->output_buffers[i];\n \n \t/* Clone the controls associated with the camera3 request. */\n \tsettings_ = CameraMetadata(camera3Request->settings);\ndiff --git a/src/android/camera_request.h b/src/android/camera_request.h\nindex db13f624..a030febf 100644\n--- a/src/android/camera_request.h\n+++ b/src/android/camera_request.h\n@@ -29,6 +29,16 @@ public:\n \t\tError,\n \t};\n \n+\tstruct StreamBuffer {\n+\t\tcamera3_stream_buffer_t buffer;\n+\t\t/*\n+\t\t * FrameBuffer instances created by wrapping a camera3 provided\n+\t\t * dmabuf are emplaced in this vector of unique_ptr<> for\n+\t\t * lifetime management.\n+\t\t */\n+\t\tstd::unique_ptr<libcamera::FrameBuffer> frameBuffer;\n+\t};\n+\n \tCamera3RequestDescriptor(libcamera::Camera *camera,\n \t\t\t\t const camera3_capture_request_t *camera3Request);\n \t~Camera3RequestDescriptor();\n@@ -36,8 +46,9 @@ public:\n \tbool isPending() const { return status_ == Status::Pending; }\n \n \tuint32_t frameNumber_ = 0;\n-\tstd::vector<camera3_stream_buffer_t> buffers_;\n-\tstd::vector<std::unique_ptr<libcamera::FrameBuffer>> frameBuffers_;\n+\n+\tstd::vector<StreamBuffer> buffers_;\n+\n \tCameraMetadata settings_;\n \tstd::unique_ptr<CaptureRequest> request_;\n \tstd::unique_ptr<CameraMetadata> resultMetadata_;\n", "prefixes": [ "libcamera-devel", "v2", "05/12" ] }