[RFC PATCH v1] pipeline: rkisp1: Avoid unnecessary indirection in `std::map`

Barnabás Pőcze pobrn at protonmail.com
Sat Jan 11 12:47:41 CET 2025


There is no reason to allocate the `RkISP1FrameInfo` objects
dynamically, and then store raw pointers in the `std::map`.

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 at protonmail.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 76 +++++++++++-------------
 1 file changed, 34 insertions(+), 42 deletions(-)

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 35c793da9..111acc35b 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,45 @@ 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);
-
-	frameInfo_.erase(info->frame);
+	auto &info = it->second;
+	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 +318,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 +327,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 +342,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";
-- 
2.47.1




More information about the libcamera-devel mailing list