Patch Detail
Show a patch.
GET /api/patches/19481/?format=api
{ "id": 19481, "url": "https://patchwork.libcamera.org/api/patches/19481/?format=api", "web_url": "https://patchwork.libcamera.org/patch/19481/", "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": "<20240212153532.179283-5-dan.scally@ideasonboard.com>", "date": "2024-02-12T15:35:31", "name": "[4/5] libcamera: rkisp1: Switch tryCompleteRequest() to use Request *", "commit_ref": null, "pull_url": null, "state": "rejected", "archived": true, "hash": "b2ba22020cec2167ac2ac4fbd908f13a7c9d2e57", "submitter": { "id": 156, "url": "https://patchwork.libcamera.org/api/people/156/?format=api", "name": "Dan Scally", "email": "dan.scally@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/19481/mbox/", "series": [ { "id": 4166, "url": "https://patchwork.libcamera.org/api/series/4166/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4166", "date": "2024-02-12T15:35:27", "name": "Remove RkISP1FrameInfo class", "version": 1, "mbox": "https://patchwork.libcamera.org/series/4166/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/19481/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/19481/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 72A0FC32C4\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 12 Feb 2024 15:35:49 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 71FF062817;\n\tMon, 12 Feb 2024 16:35:47 +0100 (CET)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6F60362807\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 12 Feb 2024 16:35:44 +0100 (CET)", "from mail.ideasonboard.com\n\t(cpc141996-chfd3-2-0-cust928.12-3.cable.virginm.net [86.13.91.161])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B8B8D2D8;\n\tMon, 12 Feb 2024 16:35:42 +0100 (CET)" ], "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=\"k+EreKkh\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1707752142;\n\tbh=xruQDwl4uQuAn9+wiqUFgjgX5aj25sHp22fgs/3FZfw=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=k+EreKkhZd6PkrMqJmgs12vtfssUqMFsvCR8buXq5WPPMxo1Dn2b5gC7fKBI+Ulri\n\t/goBCXT8ZgrXdmMazjk/pXJm32+0b+jl2O8cy/j9T0EUIGNKSpu7EQQkzWLn/D00Oo\n\t4xKdBeUZdTo3tOlP0ShvoCm34iFuDpUqCd2q2CoU=", "From": "Daniel Scally <dan.scally@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Subject": "[PATCH 4/5] libcamera: rkisp1: Switch tryCompleteRequest() to use\n\tRequest *", "Date": "Mon, 12 Feb 2024 15:35:31 +0000", "Message-Id": "<20240212153532.179283-5-dan.scally@ideasonboard.com>", "X-Mailer": "git-send-email 2.34.1", "In-Reply-To": "<20240212153532.179283-1-dan.scally@ideasonboard.com>", "References": "<20240212153532.179283-1-dan.scally@ideasonboard.com>", "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": "tryCompleteRequest() is used to delay completeRequest() until all the\nimage buffers have been returned to userspace and the Request's\nmetadata has been filled with control values by the IPA. To do that\nthe pipeline handler has a struct which holds various pieces of info\nthat allow the tryCompleteRequest() function to decide whether it's\nok to complete the request yet or not. This is quite a heavy way of\ndoing things - most of the information is tracked internally in the\nRequest already (status of the image buffers), some doesn't actually\nfactor into the decision (whether the next param buffer is dequeued)\nand the final piece (the status of the statistics buffer) can be\ntracked more simply with a map.\n\nRe-factor the tryCompleteRequest() function to take a Request * and\nsimply check hasPendingBuffers(request) and a new map flagging stats\nbuffers as in-flight to decide whether or not to complete.\n\nSigned-off-by: Daniel Scally <dan.scally@ideasonboard.com>\n---\nAs an alternative to the inFlightStatBuffers_ map we could instead extend\nRequest with a concept of internal-only buffers, probably in Request::Private,\nthat wouldn't be exposed to the application but which could be added with an\nanalogue of addBuffer() and checked with an analogue of hasPendingBuffers().\n\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 25 +++++++++++++++++-------\n 1 file changed, 18 insertions(+), 7 deletions(-)", "diff": "diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex d4ed38a4..d8f27e96 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -8,6 +8,7 @@\n #include <algorithm>\n #include <array>\n #include <iomanip>\n+#include <map>\n #include <memory>\n #include <numeric>\n #include <queue>\n@@ -173,7 +174,7 @@ private:\n \tint initLinks(Camera *camera, const CameraSensor *sensor,\n \t\t const RkISP1CameraConfiguration &config);\n \tint createCamera(MediaEntity *sensor);\n-\tvoid tryCompleteRequest(RkISP1FrameInfo *info);\n+\tvoid tryCompleteRequest(Request *request);\n \tvoid bufferReady(FrameBuffer *buffer);\n \tvoid statReady(FrameBuffer *buffer);\n \tvoid frameStart(uint32_t sequence);\n@@ -197,6 +198,7 @@ private:\n \tstd::vector<std::unique_ptr<FrameBuffer>> statBuffers_;\n \tstd::queue<FrameBuffer *> availableParamBuffers_;\n \tstd::queue<FrameBuffer *> availableStatBuffers_;\n+\tstd::map<int, FrameBuffer *> inFlightStatBuffers_;\n \n \tCamera *activeCamera_;\n \n@@ -229,6 +231,7 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req\n \n \t\tparamBuffer = pipe_->availableParamBuffers_.front();\n \t\tpipe_->availableParamBuffers_.pop();\n+\t\tparamBuffer->_d()->setRequest(request);\n \n \t\tstatBuffer = pipe_->availableStatBuffers_.front();\n \t\tpipe_->availableStatBuffers_.pop();\n@@ -425,8 +428,9 @@ void RkISP1CameraData::metadataReady(unsigned int bufferId,\n \n \t\tinfo->request->metadata().merge(metadata);\n \t\tinfo->metadataProcessed = true;\n+\t\tpipe()->inFlightStatBuffers_.erase(info->request->sequence());\n \n-\t\tpipe()->tryCompleteRequest(info);\n+\t\tpipe()->tryCompleteRequest(statBuffer->request());\n \t\treturn;\n \t}\n \n@@ -1044,6 +1048,8 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n \tif (!info)\n \t\treturn -ENOENT;\n \n+\tinFlightStatBuffers_[request->sequence()] = info->statBuffer;\n+\n \tdata->ipa_->queueRequest(request->sequence(), request->controls());\n \tif (isRaw_) {\n \t\tif (info->mainPathBuffer)\n@@ -1245,15 +1251,19 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n * Buffer Handling\n */\n \n-void PipelineHandlerRkISP1::tryCompleteRequest(RkISP1FrameInfo *info)\n+void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)\n {\n \tRkISP1CameraData *data = cameraData(activeCamera_);\n-\tRequest *request = info->request;\n+\n+\tRkISP1FrameInfo *info = data->frameInfo_.find(request);\n+\tif (!info)\n+\t\treturn;\n \n \tif (request->hasPendingBuffers())\n \t\treturn;\n \n-\tif (!info->metadataProcessed)\n+\tif (inFlightStatBuffers_.find(request->sequence()) !=\n+\t inFlightStatBuffers_.end())\n \t\treturn;\n \n \tdata->frameInfo_.destroy(info->frame);\n@@ -1295,7 +1305,7 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n \t}\n \n \tcompleteBuffer(request, buffer);\n-\ttryCompleteRequest(info);\n+\ttryCompleteRequest(request);\n }\n \n void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n@@ -1310,7 +1320,8 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n \n \tif (buffer->metadata().status == FrameMetadata::FrameCancelled) {\n \t\tinfo->metadataProcessed = true;\n-\t\ttryCompleteRequest(info);\n+\t\tinFlightStatBuffers_.erase(request->sequence());\n+\t\ttryCompleteRequest(request);\n \t\treturn;\n \t}\n \n", "prefixes": [ "4/5" ] }