From caf00087ba94b649d275686656a38e78e9fb438d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Thu, 9 Jan 2025 17:40:33 +0100 Subject: [PATCH] libcamera: pipeline: Avoid unnecessary indirection in frame info map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 manually calling new/delete, simplifies the code, and eliminates one memory allocation per frame. Signed-off-by: Barnabás Pőcze Reviewed-by: Paul Elder Acked-by: Laurent Pinchart Tested-by: Kieran Bingham Reviewed-by: Kieran Bingham --- src/libcamera/pipeline/ipu3/frames.cpp | 40 ++++++------- src/libcamera/pipeline/ipu3/frames.h | 2 +- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 75 +++++++++++------------- 3 files changed, 54 insertions(+), 63 deletions(-) diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp index bc0526a7..4232e976 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 &[id, info] : frameInfo_) { + for (const auto &[stream, buf] : info.request->buffers()) + if (buf == buffer) + return &info; - for (auto const itBuffers : info->request->buffers()) - if (itBuffers.second == buffer) - 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 a347b66f..1cabd0e6 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 8b434a56..81370f4c 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -85,7 +85,7 @@ public: private: PipelineHandlerRkISP1 *pipe_; - std::map frameInfo_; + std::map frameInfo_; }; class RkISP1CameraData : public Camera::Private @@ -289,49 +289,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(); @@ -342,7 +339,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"; @@ -351,14 +348,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"; @@ -368,11 +363,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";