From patchwork Tue Jan 21 19:05:50 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 22610 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id C90BDBD160 for ; Tue, 21 Jan 2025 19:06:00 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 046B968558; Tue, 21 Jan 2025 20:06:00 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=protonmail.com header.i=@protonmail.com header.b="hvybMrra"; dkim-atps=neutral Received: from mail-40134.protonmail.ch (mail-40134.protonmail.ch [185.70.40.134]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id DF39F60380 for ; Tue, 21 Jan 2025 20:05:58 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1737486354; x=1737745554; bh=xaUXeIPMR16NAY4NbIAomGCF0lBUKxglAlVSsAtnaGY=; h=Date:To:From:Subject:Message-ID:Feedback-ID:From:To:Cc:Date: Subject:Reply-To:Feedback-ID:Message-ID:BIMI-Selector: List-Unsubscribe:List-Unsubscribe-Post; b=hvybMrraLUBsWaBtK+2Fm+y6W4UAcwULbPoSZhMCMhvNGYREqwRg878rPOmTNnDE4 haGOVKwMoGjkWRkFGvbtMpB0IsGfNaxfw7eF7v5v5NccWC9/RHZM4mVQeXH+VE4Oy+ nLo8GOkWkJLKwUnY8r+WKHDNUTPJE5FJO4bABAARs7ENVgQhb+2ZfBCWOwTdDRHRwM 4OiSGbPMR+LRgQKM0JeHxb7rmE3dWhdmPOLoFODMQ3On2JUrGZBZ+xP2H62omEm0BZ 8uO/9nlbRRFredj6n2P8O857NWBwjvDi4UE0L3FMf7kUNzY/WqIrFHbnIE3EvVILNy UAJ0+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?= Subject: [RFC PATCH v2] libcamera: pipeline: Avoid unnecessary indirection in frame info map Message-ID: <20250121190547.304107-1-pobrn@protonmail.com> Feedback-ID: 20568564:user:proton X-Pm-Message-ID: a6b54e7b1f07895a2557a09758f086835f88d0a4 MIME-Version: 1.0 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" There is no reason to allocate the frame info objects dynamically, and then store raw pointers in the `std::map` in the rkisp1 and ipu3 pipeline handler. Instead, store the objects directly in the map. This removes the need for manully calling new/delete, simplifies the code, and eliminates one memory allocation per frame. Signed-off-by: Barnabás Pőcze --- src/libcamera/pipeline/ipu3/frames.cpp | 38 ++++++------ src/libcamera/pipeline/ipu3/frames.h | 2 +- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 75 +++++++++++------------- 3 files changed, 53 insertions(+), 62 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp index bc0526a77..f3ea78797 100644 --- a/src/libcamera/pipeline/ipu3/frames.cpp +++ b/src/libcamera/pipeline/ipu3/frames.cpp @@ -64,20 +64,20 @@ IPU3Frames::Info *IPU3Frames::create(Request *request) availableParamBuffers_.pop(); availableStatBuffers_.pop(); - /* \todo Remove the dynamic allocation of Info */ - std::unique_ptr info = std::make_unique(); + auto [it, inserted] = frameInfo_.try_emplace(id); + ASSERT(inserted); - info->id = id; - info->request = request; - info->rawBuffer = nullptr; - info->paramBuffer = paramBuffer; - info->statBuffer = statBuffer; - info->paramDequeued = false; - info->metadataProcessed = false; + auto &info = it->second; - frameInfo_[id] = std::move(info); + info.id = id; + info.request = request; + info.rawBuffer = nullptr; + info.paramBuffer = paramBuffer; + info.statBuffer = statBuffer; + info.paramDequeued = false; + info.metadataProcessed = false; - return frameInfo_[id].get(); + return &info; } void IPU3Frames::remove(IPU3Frames::Info *info) @@ -115,7 +115,7 @@ IPU3Frames::Info *IPU3Frames::find(unsigned int id) const auto &itInfo = frameInfo_.find(id); if (itInfo != frameInfo_.end()) - return itInfo->second.get(); + return &itInfo->second; LOG(IPU3, Fatal) << "Can't find tracking information for frame " << id; @@ -124,16 +124,14 @@ IPU3Frames::Info *IPU3Frames::find(unsigned int id) IPU3Frames::Info *IPU3Frames::find(FrameBuffer *buffer) { - for (auto const &itInfo : frameInfo_) { - Info *info = itInfo.second.get(); - - for (auto const itBuffers : info->request->buffers()) + for (auto &[id, info] : frameInfo_) { + for (auto const itBuffers : info.request->buffers()) if (itBuffers.second == buffer) - return info; + return &info; - if (info->rawBuffer == buffer || info->paramBuffer == buffer || - info->statBuffer == buffer) - return info; + if (info.rawBuffer == buffer || info.paramBuffer == buffer || + info.statBuffer == buffer) + return &info; } LOG(IPU3, Fatal) << "Can't find tracking information from buffer"; diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h index a347b66f3..1cabd0e64 100644 --- a/src/libcamera/pipeline/ipu3/frames.h +++ b/src/libcamera/pipeline/ipu3/frames.h @@ -61,7 +61,7 @@ private: std::queue availableParamBuffers_; std::queue availableStatBuffers_; - std::map> frameInfo_; + std::map frameInfo_; }; } /* namespace libcamera */ diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index ca8bbaaf1..953138305 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -84,7 +84,7 @@ public: private: PipelineHandlerRkISP1 *pipe_; - std::map frameInfo_; + std::map frameInfo_; }; class RkISP1CameraData : public Camera::Private @@ -269,49 +269,46 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req mainPathBuffer = request->findBuffer(&data->mainPathStream_); selfPathBuffer = request->findBuffer(&data->selfPathStream_); - RkISP1FrameInfo *info = new RkISP1FrameInfo; + auto [it, inserted] = frameInfo_.try_emplace(frame); + ASSERT(inserted); - info->frame = frame; - info->request = request; - info->paramBuffer = paramBuffer; - info->mainPathBuffer = mainPathBuffer; - info->selfPathBuffer = selfPathBuffer; - info->statBuffer = statBuffer; - info->paramDequeued = false; - info->metadataProcessed = false; + auto &info = it->second; - frameInfo_[frame] = info; + info.frame = frame; + info.request = request; + info.paramBuffer = paramBuffer; + info.mainPathBuffer = mainPathBuffer; + info.selfPathBuffer = selfPathBuffer; + info.statBuffer = statBuffer; + info.paramDequeued = false; + info.metadataProcessed = false; - return info; + return &info; } int RkISP1Frames::destroy(unsigned int frame) { - RkISP1FrameInfo *info = find(frame); - if (!info) + auto it = frameInfo_.find(frame); + if (it == frameInfo_.end()) return -ENOENT; - pipe_->availableParamBuffers_.push(info->paramBuffer); - pipe_->availableStatBuffers_.push(info->statBuffer); - pipe_->availableMainPathBuffers_.push(info->mainPathBuffer); + auto &info = it->second; - frameInfo_.erase(info->frame); + pipe_->availableParamBuffers_.push(info.paramBuffer); + pipe_->availableStatBuffers_.push(info.statBuffer); + pipe_->availableMainPathBuffers_.push(info.mainPathBuffer); - delete info; + frameInfo_.erase(it); return 0; } void RkISP1Frames::clear() { - for (const auto &entry : frameInfo_) { - RkISP1FrameInfo *info = entry.second; - - pipe_->availableParamBuffers_.push(info->paramBuffer); - pipe_->availableStatBuffers_.push(info->statBuffer); - pipe_->availableMainPathBuffers_.push(info->mainPathBuffer); - - delete info; + for (const auto &[frame, info] : frameInfo_) { + pipe_->availableParamBuffers_.push(info.paramBuffer); + pipe_->availableStatBuffers_.push(info.statBuffer); + pipe_->availableMainPathBuffers_.push(info.mainPathBuffer); } frameInfo_.clear(); @@ -322,7 +319,7 @@ RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame) auto itInfo = frameInfo_.find(frame); if (itInfo != frameInfo_.end()) - return itInfo->second; + return &itInfo->second; LOG(RkISP1, Fatal) << "Can't locate info from frame"; @@ -331,14 +328,12 @@ RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame) RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer) { - for (auto &itInfo : frameInfo_) { - RkISP1FrameInfo *info = itInfo.second; - - if (info->paramBuffer == buffer || - info->statBuffer == buffer || - info->mainPathBuffer == buffer || - info->selfPathBuffer == buffer) - return info; + for (auto &[frame, info] : frameInfo_) { + if (info.paramBuffer == buffer || + info.statBuffer == buffer || + info.mainPathBuffer == buffer || + info.selfPathBuffer == buffer) + return &info; } LOG(RkISP1, Fatal) << "Can't locate info from buffer"; @@ -348,11 +343,9 @@ RkISP1FrameInfo *RkISP1Frames::find(FrameBuffer *buffer) RkISP1FrameInfo *RkISP1Frames::find(Request *request) { - for (auto &itInfo : frameInfo_) { - RkISP1FrameInfo *info = itInfo.second; - - if (info->request == request) - return info; + for (auto &[frame, info] : frameInfo_) { + if (info.request == request) + return &info; } LOG(RkISP1, Fatal) << "Can't locate info from request";