Patch Detail
Show a patch.
GET /api/patches/22610/?format=api
{ "id": 22610, "url": "https://patchwork.libcamera.org/api/patches/22610/?format=api", "web_url": "https://patchwork.libcamera.org/patch/22610/", "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": "<20250121190547.304107-1-pobrn@protonmail.com>", "date": "2025-01-21T19:05:50", "name": "[RFC,v2] libcamera: pipeline: Avoid unnecessary indirection in frame info map", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "910e703876593d5c4c39012b1f53919873dd0be4", "submitter": { "id": 133, "url": "https://patchwork.libcamera.org/api/people/133/?format=api", "name": "Pőcze Barnabás", "email": "pobrn@protonmail.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/22610/mbox/", "series": [ { "id": 4962, "url": "https://patchwork.libcamera.org/api/series/4962/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4962", "date": "2025-01-21T19:05:50", "name": "[RFC,v2] libcamera: pipeline: Avoid unnecessary indirection in frame info map", "version": 2, "mbox": "https://patchwork.libcamera.org/series/4962/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/22610/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/22610/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 C90BDBD160\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 21 Jan 2025 19:06:00 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 046B968558;\n\tTue, 21 Jan 2025 20:06:00 +0100 (CET)", "from mail-40134.protonmail.ch (mail-40134.protonmail.ch\n\t[185.70.40.134])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DF39F60380\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 21 Jan 2025 20:05:58 +0100 (CET)" ], "Authentication-Results": "lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"hvybMrra\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1737486354; x=1737745554;\n\tbh=xaUXeIPMR16NAY4NbIAomGCF0lBUKxglAlVSsAtnaGY=;\n\th=Date:To:From:Subject:Message-ID:Feedback-ID:From:To:Cc:Date:\n\tSubject:Reply-To:Feedback-ID:Message-ID:BIMI-Selector:\n\tList-Unsubscribe:List-Unsubscribe-Post;\n\tb=hvybMrraLUBsWaBtK+2Fm+y6W4UAcwULbPoSZhMCMhvNGYREqwRg878rPOmTNnDE4\n\thaGOVKwMoGjkWRkFGvbtMpB0IsGfNaxfw7eF7v5v5NccWC9/RHZM4mVQeXH+VE4Oy+\n\tnLo8GOkWkJLKwUnY8r+WKHDNUTPJE5FJO4bABAARs7ENVgQhb+2ZfBCWOwTdDRHRwM\n\t4OiSGbPMR+LRgQKM0JeHxb7rmE3dWhdmPOLoFODMQ3On2JUrGZBZ+xP2H62omEm0BZ\n\t8uO/9nlbRRFredj6n2P8O857NWBwjvDi4UE0L3FMf7kUNzY/WqIrFHbnIE3EvVILNy\n\tUAJ0+qPgIwyCw==", "Date": "Tue, 21 Jan 2025 19:05:50 +0000", "To": "libcamera-devel@lists.libcamera.org", "From": "=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>", "Subject": "[RFC PATCH v2] libcamera: pipeline: Avoid unnecessary indirection in\n\tframe info map", "Message-ID": "<20250121190547.304107-1-pobrn@protonmail.com>", "Feedback-ID": "20568564:user:proton", "X-Pm-Message-ID": "a6b54e7b1f07895a2557a09758f086835f88d0a4", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=utf-8", "Content-Transfer-Encoding": "quoted-printable", "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": "There is no reason to allocate the frame info objects dynamically,\nand then store raw pointers in the `std::map` in the rkisp1\nand ipu3 pipeline handler.\n\nInstead, store the objects directly in the map. This removes\nthe need for manully calling new/delete, simplifies the code,\nand eliminates one memory allocation per frame.\n\nSigned-off-by: Barnabás Pőcze <pobrn@protonmail.com>\n---\n src/libcamera/pipeline/ipu3/frames.cpp | 38 ++++++------\n src/libcamera/pipeline/ipu3/frames.h | 2 +-\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 75 +++++++++++-------------\n 3 files changed, 53 insertions(+), 62 deletions(-)", "diff": "diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp\nindex bc0526a77..f3ea78797 100644\n--- a/src/libcamera/pipeline/ipu3/frames.cpp\n+++ b/src/libcamera/pipeline/ipu3/frames.cpp\n@@ -64,20 +64,20 @@ IPU3Frames::Info *IPU3Frames::create(Request *request)\n \tavailableParamBuffers_.pop();\n \tavailableStatBuffers_.pop();\n \n-\t/* \\todo Remove the dynamic allocation of Info */\n-\tstd::unique_ptr<Info> info = std::make_unique<Info>();\n+\tauto [it, inserted] = frameInfo_.try_emplace(id);\n+\tASSERT(inserted);\n \n-\tinfo->id = id;\n-\tinfo->request = request;\n-\tinfo->rawBuffer = nullptr;\n-\tinfo->paramBuffer = paramBuffer;\n-\tinfo->statBuffer = statBuffer;\n-\tinfo->paramDequeued = false;\n-\tinfo->metadataProcessed = false;\n+\tauto &info = it->second;\n \n-\tframeInfo_[id] = std::move(info);\n+\tinfo.id = id;\n+\tinfo.request = request;\n+\tinfo.rawBuffer = nullptr;\n+\tinfo.paramBuffer = paramBuffer;\n+\tinfo.statBuffer = statBuffer;\n+\tinfo.paramDequeued = false;\n+\tinfo.metadataProcessed = false;\n \n-\treturn frameInfo_[id].get();\n+\treturn &info;\n }\n \n void IPU3Frames::remove(IPU3Frames::Info *info)\n@@ -115,7 +115,7 @@ IPU3Frames::Info *IPU3Frames::find(unsigned int id)\n \tconst auto &itInfo = frameInfo_.find(id);\n \n \tif (itInfo != frameInfo_.end())\n-\t\treturn itInfo->second.get();\n+\t\treturn &itInfo->second;\n \n \tLOG(IPU3, Fatal) << \"Can't find tracking information for frame \" << id;\n \n@@ -124,16 +124,14 @@ IPU3Frames::Info *IPU3Frames::find(unsigned int id)\n \n IPU3Frames::Info *IPU3Frames::find(FrameBuffer *buffer)\n {\n-\tfor (auto const &itInfo : frameInfo_) {\n-\t\tInfo *info = itInfo.second.get();\n-\n-\t\tfor (auto const itBuffers : info->request->buffers())\n+\tfor (auto &[id, info] : frameInfo_) {\n+\t\tfor (auto const itBuffers : info.request->buffers())\n \t\t\tif (itBuffers.second == buffer)\n-\t\t\t\treturn info;\n+\t\t\t\treturn &info;\n \n-\t\tif (info->rawBuffer == buffer || info->paramBuffer == buffer ||\n-\t\t info->statBuffer == buffer)\n-\t\t\treturn info;\n+\t\tif (info.rawBuffer == buffer || info.paramBuffer == buffer ||\n+\t\t info.statBuffer == buffer)\n+\t\t\treturn &info;\n \t}\n \n \tLOG(IPU3, Fatal) << \"Can't find tracking information from buffer\";\ndiff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h\nindex a347b66f3..1cabd0e64 100644\n--- a/src/libcamera/pipeline/ipu3/frames.h\n+++ b/src/libcamera/pipeline/ipu3/frames.h\n@@ -61,7 +61,7 @@ private:\n \tstd::queue<FrameBuffer *> availableParamBuffers_;\n \tstd::queue<FrameBuffer *> availableStatBuffers_;\n \n-\tstd::map<unsigned int, std::unique_ptr<Info>> frameInfo_;\n+\tstd::map<unsigned int, Info> frameInfo_;\n };\n \n } /* namespace libcamera */\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex ca8bbaaf1..953138305 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -84,7 +84,7 @@ public:\n \n private:\n \tPipelineHandlerRkISP1 *pipe_;\n-\tstd::map<unsigned int, RkISP1FrameInfo *> frameInfo_;\n+\tstd::map<unsigned int, RkISP1FrameInfo> frameInfo_;\n };\n \n class RkISP1CameraData : public Camera::Private\n@@ -269,49 +269,46 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req\n \t\tmainPathBuffer = request->findBuffer(&data->mainPathStream_);\n \tselfPathBuffer = request->findBuffer(&data->selfPathStream_);\n \n-\tRkISP1FrameInfo *info = new RkISP1FrameInfo;\n+\tauto [it, inserted] = frameInfo_.try_emplace(frame);\n+\tASSERT(inserted);\n \n-\tinfo->frame = frame;\n-\tinfo->request = request;\n-\tinfo->paramBuffer = paramBuffer;\n-\tinfo->mainPathBuffer = mainPathBuffer;\n-\tinfo->selfPathBuffer = selfPathBuffer;\n-\tinfo->statBuffer = statBuffer;\n-\tinfo->paramDequeued = false;\n-\tinfo->metadataProcessed = false;\n+\tauto &info = it->second;\n \n-\tframeInfo_[frame] = info;\n+\tinfo.frame = frame;\n+\tinfo.request = request;\n+\tinfo.paramBuffer = paramBuffer;\n+\tinfo.mainPathBuffer = mainPathBuffer;\n+\tinfo.selfPathBuffer = selfPathBuffer;\n+\tinfo.statBuffer = statBuffer;\n+\tinfo.paramDequeued = false;\n+\tinfo.metadataProcessed = false;\n \n-\treturn info;\n+\treturn &info;\n }\n \n int RkISP1Frames::destroy(unsigned int frame)\n {\n-\tRkISP1FrameInfo *info = find(frame);\n-\tif (!info)\n+\tauto it = frameInfo_.find(frame);\n+\tif (it == frameInfo_.end())\n \t\treturn -ENOENT;\n \n-\tpipe_->availableParamBuffers_.push(info->paramBuffer);\n-\tpipe_->availableStatBuffers_.push(info->statBuffer);\n-\tpipe_->availableMainPathBuffers_.push(info->mainPathBuffer);\n+\tauto &info = it->second;\n \n-\tframeInfo_.erase(info->frame);\n+\tpipe_->availableParamBuffers_.push(info.paramBuffer);\n+\tpipe_->availableStatBuffers_.push(info.statBuffer);\n+\tpipe_->availableMainPathBuffers_.push(info.mainPathBuffer);\n \n-\tdelete info;\n+\tframeInfo_.erase(it);\n \n \treturn 0;\n }\n \n void RkISP1Frames::clear()\n {\n-\tfor (const auto &entry : frameInfo_) {\n-\t\tRkISP1FrameInfo *info = entry.second;\n-\n-\t\tpipe_->availableParamBuffers_.push(info->paramBuffer);\n-\t\tpipe_->availableStatBuffers_.push(info->statBuffer);\n-\t\tpipe_->availableMainPathBuffers_.push(info->mainPathBuffer);\n-\n-\t\tdelete info;\n+\tfor (const auto &[frame, info] : frameInfo_) {\n+\t\tpipe_->availableParamBuffers_.push(info.paramBuffer);\n+\t\tpipe_->availableStatBuffers_.push(info.statBuffer);\n+\t\tpipe_->availableMainPathBuffers_.push(info.mainPathBuffer);\n \t}\n \n \tframeInfo_.clear();\n@@ -322,7 +319,7 @@ RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame)\n \tauto itInfo = frameInfo_.find(frame);\n \n \tif (itInfo != frameInfo_.end())\n-\t\treturn itInfo->second;\n+\t\treturn &itInfo->second;\n \n \tLOG(RkISP1, Fatal) << \"Can't locate info from frame\";\n \n@@ -331,14 +328,12 @@ RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame)\n \n RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer)\n {\n-\tfor (auto &itInfo : frameInfo_) {\n-\t\tRkISP1FrameInfo *info = itInfo.second;\n-\n-\t\tif (info->paramBuffer == buffer ||\n-\t\t info->statBuffer == buffer ||\n-\t\t info->mainPathBuffer == buffer ||\n-\t\t info->selfPathBuffer == buffer)\n-\t\t\treturn info;\n+\tfor (auto &[frame, info] : frameInfo_) {\n+\t\tif (info.paramBuffer == buffer ||\n+\t\t info.statBuffer == buffer ||\n+\t\t info.mainPathBuffer == buffer ||\n+\t\t info.selfPathBuffer == buffer)\n+\t\t\treturn &info;\n \t}\n \n \tLOG(RkISP1, Fatal) << \"Can't locate info from buffer\";\n@@ -348,11 +343,9 @@ RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer)\n \n RkISP1FrameInfo *RkISP1Frames::find(Request *request)\n {\n-\tfor (auto &itInfo : frameInfo_) {\n-\t\tRkISP1FrameInfo *info = itInfo.second;\n-\n-\t\tif (info->request == request)\n-\t\t\treturn info;\n+\tfor (auto &[frame, info] : frameInfo_) {\n+\t\tif (info.request == request)\n+\t\t\treturn &info;\n \t}\n \n \tLOG(RkISP1, Fatal) << \"Can't locate info from request\";\n", "prefixes": [ "RFC", "v2" ] }