[RFC,v2] libcamera: pipeline: Avoid unnecessary indirection in frame info map
diff mbox series

Message ID 20250121190547.304107-1-pobrn@protonmail.com
State New
Headers show
Series
  • [RFC,v2] libcamera: pipeline: Avoid unnecessary indirection in frame info map
Related show

Commit Message

Barnabás Pőcze Jan. 21, 2025, 7:05 p.m. UTC
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 <pobrn@protonmail.com>
---
 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(-)

Patch
diff mbox series

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> info = std::make_unique<Info>();
+	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<FrameBuffer *> availableParamBuffers_;
 	std::queue<FrameBuffer *> availableStatBuffers_;
 
-	std::map<unsigned int, std::unique_ptr<Info>> frameInfo_;
+	std::map<unsigned int, Info> 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<unsigned int, RkISP1FrameInfo *> frameInfo_;
+	std::map<unsigned int, RkISP1FrameInfo> 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";