[PATCH 5/5] libcamera: rkisp1: Remove RkISP1FrameInfo

Daniel Scally dan.scally at ideasonboard.com
Mon Feb 12 16:35:32 CET 2024


The RkISP1FrameInfo class existed to hold varying bits of info to
help guarantee that a Request is not completed until all of the
image buffers are returned to userspace and the metadata has been
filled by the IPA module. Now that we're no longer using it for
that function it can be removed.

Remove the class and refactor its code out to the rest of the
Pipeline Handler.

Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 252 +++++------------------
 1 file changed, 46 insertions(+), 206 deletions(-)

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index d8f27e96..f6dfd1cb 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -52,44 +52,13 @@ LOG_DEFINE_CATEGORY(RkISP1)
 class PipelineHandlerRkISP1;
 class RkISP1CameraData;
 
-struct RkISP1FrameInfo {
-	unsigned int frame;
-	Request *request;
-
-	FrameBuffer *paramBuffer;
-	FrameBuffer *statBuffer;
-	FrameBuffer *mainPathBuffer;
-	FrameBuffer *selfPathBuffer;
-
-	bool metadataProcessed;
-};
-
-class RkISP1Frames
-{
-public:
-	RkISP1Frames(PipelineHandler *pipe);
-
-	RkISP1FrameInfo *create(const RkISP1CameraData *data, Request *request,
-				bool isRaw);
-	int destroy(unsigned int frame);
-	void clear();
-
-	RkISP1FrameInfo *find(unsigned int frame);
-	RkISP1FrameInfo *find(FrameBuffer *buffer);
-	RkISP1FrameInfo *find(Request *request);
-
-private:
-	PipelineHandlerRkISP1 *pipe_;
-	std::map<unsigned int, RkISP1FrameInfo *> frameInfo_;
-};
-
 class RkISP1CameraData : public Camera::Private
 {
 public:
 	RkISP1CameraData(PipelineHandler *pipe, RkISP1MainPath *mainPath,
 			 RkISP1SelfPath *selfPath)
-		: Camera::Private(pipe), frameInfo_(pipe),
-		  mainPath_(mainPath), selfPath_(selfPath)
+		: Camera::Private(pipe), mainPath_(mainPath),
+		  selfPath_(selfPath)
 	{
 	}
 
@@ -101,7 +70,6 @@ public:
 	std::unique_ptr<CameraSensor> sensor_;
 	std::unique_ptr<DelayedControls> delayedCtrls_;
 	std::vector<IPABuffer> ipaBuffers_;
-	RkISP1Frames frameInfo_;
 
 	RkISP1MainPath *mainPath_;
 	RkISP1SelfPath *selfPath_;
@@ -169,7 +137,6 @@ private:
 	}
 
 	friend RkISP1CameraData;
-	friend RkISP1Frames;
 
 	int initLinks(Camera *camera, const CameraSensor *sensor,
 		      const RkISP1CameraConfiguration &config);
@@ -205,130 +172,6 @@ private:
 	const MediaPad *ispSink_;
 };
 
-RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)
-	: pipe_(static_cast<PipelineHandlerRkISP1 *>(pipe))
-{
-}
-
-RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *request,
-				      bool isRaw)
-{
-	unsigned int frame = request->sequence();
-
-	FrameBuffer *paramBuffer = nullptr;
-	FrameBuffer *statBuffer = nullptr;
-
-	if (!isRaw) {
-		if (pipe_->availableParamBuffers_.empty()) {
-			LOG(RkISP1, Error) << "Parameters buffer underrun";
-			return nullptr;
-		}
-
-		if (pipe_->availableStatBuffers_.empty()) {
-			LOG(RkISP1, Error) << "Statistic buffer underrun";
-			return nullptr;
-		}
-
-		paramBuffer = pipe_->availableParamBuffers_.front();
-		pipe_->availableParamBuffers_.pop();
-		paramBuffer->_d()->setRequest(request);
-
-		statBuffer = pipe_->availableStatBuffers_.front();
-		pipe_->availableStatBuffers_.pop();
-		statBuffer->_d()->setRequest(request);
-	}
-
-	FrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);
-	FrameBuffer *selfPathBuffer = request->findBuffer(&data->selfPathStream_);
-
-	RkISP1FrameInfo *info = new RkISP1FrameInfo;
-
-	info->frame = frame;
-	info->request = request;
-	info->paramBuffer = paramBuffer;
-	info->mainPathBuffer = mainPathBuffer;
-	info->selfPathBuffer = selfPathBuffer;
-	info->statBuffer = statBuffer;
-	info->metadataProcessed = false;
-
-	frameInfo_[frame] = info;
-
-	return info;
-}
-
-int RkISP1Frames::destroy(unsigned int frame)
-{
-	RkISP1FrameInfo *info = find(frame);
-	if (!info)
-		return -ENOENT;
-
-	pipe_->availableParamBuffers_.push(info->paramBuffer);
-	pipe_->availableStatBuffers_.push(info->statBuffer);
-
-	frameInfo_.erase(info->frame);
-
-	delete info;
-
-	return 0;
-}
-
-void RkISP1Frames::clear()
-{
-	for (const auto &entry : frameInfo_) {
-		RkISP1FrameInfo *info = entry.second;
-
-		pipe_->availableParamBuffers_.push(info->paramBuffer);
-		pipe_->availableStatBuffers_.push(info->statBuffer);
-
-		delete info;
-	}
-
-	frameInfo_.clear();
-}
-
-RkISP1FrameInfo *RkISP1Frames::find(unsigned int frame)
-{
-	auto itInfo = frameInfo_.find(frame);
-
-	if (itInfo != frameInfo_.end())
-		return itInfo->second;
-
-	LOG(RkISP1, Fatal) << "Can't locate info from frame";
-
-	return nullptr;
-}
-
-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;
-	}
-
-	LOG(RkISP1, Fatal) << "Can't locate info from buffer";
-
-	return nullptr;
-}
-
-RkISP1FrameInfo *RkISP1Frames::find(Request *request)
-{
-	for (auto &itInfo : frameInfo_) {
-		RkISP1FrameInfo *info = itInfo.second;
-
-		if (info->request == request)
-			return info;
-	}
-
-	LOG(RkISP1, Fatal) << "Can't locate info from request";
-
-	return nullptr;
-}
-
 PipelineHandlerRkISP1 *RkISP1CameraData::pipe()
 {
 	return static_cast<PipelineHandlerRkISP1 *>(Camera::Private::pipe());
@@ -387,20 +230,30 @@ void RkISP1CameraData::paramFilled(unsigned int bufferId)
 		if (paramBuffer->cookie() != bufferId)
 			continue;
 
-		RkISP1FrameInfo *info = frameInfo_.find(paramBuffer.get());
-		if (!info)
+		Request *request = paramBuffer->request();
+		FrameBuffer *mainPathBuffer = request->findBuffer(&mainPathStream_);
+		FrameBuffer *selfPathBuffer = request->findBuffer(&selfPathStream_);
+
+		if (pipe->availableStatBuffers_.empty()) {
+			LOG(RkISP1, Fatal) << "Parameters buffer underrun";
 			return;
+		}
+
+		FrameBuffer *statBuffer = pipe->availableStatBuffers_.front();
+		pipe->availableStatBuffers_.pop();
+		pipe->inFlightStatBuffers_[request->sequence()] = statBuffer;
+		statBuffer->_d()->setRequest(request);
 
-		info->paramBuffer->_d()->metadata().planes()[0].bytesused =
+		paramBuffer->_d()->metadata().planes()[0].bytesused =
 			sizeof(struct rkisp1_params_cfg);
-		pipe->param_->queueBuffer(info->paramBuffer);
-		pipe->stat_->queueBuffer(info->statBuffer);
+		pipe->param_->queueBuffer(paramBuffer.get());
+		pipe->stat_->queueBuffer(statBuffer);
 
-		if (info->mainPathBuffer)
-			mainPath_->queueBuffer(info->mainPathBuffer);
+		if (mainPathBuffer)
+			mainPath_->queueBuffer(mainPathBuffer);
 
-		if (selfPath_ && info->selfPathBuffer)
-			selfPath_->queueBuffer(info->selfPathBuffer);
+		if (selfPath_ && selfPathBuffer)
+			selfPath_->queueBuffer(selfPathBuffer);
 
 		return;
 	}
@@ -422,15 +275,13 @@ void RkISP1CameraData::metadataReady(unsigned int bufferId,
 		if (statBuffer->cookie() != bufferId)
 			continue;
 
-		RkISP1FrameInfo *info = frameInfo_.find(statBuffer.get());
-		if (!info)
-			return;
+		Request *request = statBuffer->request();
 
-		info->request->metadata().merge(metadata);
-		info->metadataProcessed = true;
-		pipe()->inFlightStatBuffers_.erase(info->request->sequence());
+		request->metadata().merge(metadata);
+		pipe()->inFlightStatBuffers_.erase(request->sequence());
+		pipe()->availableStatBuffers_.push(statBuffer.get());
+		pipe()->tryCompleteRequest(request);
 
-		pipe()->tryCompleteRequest(statBuffer->request());
 		return;
 	}
 
@@ -1033,7 +884,6 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera)
 	}
 
 	ASSERT(data->queuedRequests_.empty());
-	data->frameInfo_.clear();
 
 	freeBuffers(camera);
 
@@ -1043,23 +893,32 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera)
 int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)
 {
 	RkISP1CameraData *data = cameraData(camera);
+	FrameBuffer *paramBuffer = nullptr;
 
-	RkISP1FrameInfo *info = data->frameInfo_.create(data, request, isRaw_);
-	if (!info)
-		return -ENOENT;
+	if (!isRaw_) {
+		if (availableParamBuffers_.empty()) {
+			LOG(RkISP1, Error) << "Parameters buffer underrun";
+			return -ENOENT;
+		}
 
-	inFlightStatBuffers_[request->sequence()] = info->statBuffer;
+		paramBuffer = availableParamBuffers_.front();
+		availableParamBuffers_.pop();
+		paramBuffer->_d()->setRequest(request);
+	}
 
 	data->ipa_->queueRequest(request->sequence(), request->controls());
 	if (isRaw_) {
-		if (info->mainPathBuffer)
-			data->mainPath_->queueBuffer(info->mainPathBuffer);
+		FrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);
+		FrameBuffer *selfPathBuffer = request->findBuffer(&data->selfPathStream_);
+
+		if (mainPathBuffer)
+			data->mainPath_->queueBuffer(mainPathBuffer);
 
-		if (data->selfPath_ && info->selfPathBuffer)
-			data->selfPath_->queueBuffer(info->selfPathBuffer);
+		if (data->selfPath_ && selfPathBuffer)
+			data->selfPath_->queueBuffer(selfPathBuffer);
 	} else {
 		data->ipa_->fillParamsBuffer(request->sequence(),
-					     info->paramBuffer->cookie());
+					     paramBuffer->cookie());
 	}
 
 	return 0;
@@ -1253,12 +1112,6 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)
 
 void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)
 {
-	RkISP1CameraData *data = cameraData(activeCamera_);
-
-	RkISP1FrameInfo *info = data->frameInfo_.find(request);
-	if (!info)
-		return;
-
 	if (request->hasPendingBuffers())
 		return;
 
@@ -1266,8 +1119,6 @@ void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)
 	    inFlightStatBuffers_.end())
 		return;
 
-	data->frameInfo_.destroy(info->frame);
-
 	completeRequest(request);
 }
 
@@ -1275,11 +1126,6 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
 {
 	ASSERT(activeCamera_);
 	RkISP1CameraData *data = cameraData(activeCamera_);
-
-	RkISP1FrameInfo *info = data->frameInfo_.find(buffer);
-	if (!info)
-		return;
-
 	const FrameMetadata &metadata = buffer->metadata();
 	Request *request = buffer->request();
 
@@ -1299,9 +1145,6 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
 			data->ipa_->processStatsBuffer(request->sequence(),
 						       0, ctrls);
 		}
-	} else {
-		if (isRaw_)
-			info->metadataProcessed = true;
 	}
 
 	completeBuffer(request, buffer);
@@ -1314,18 +1157,15 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
 	RkISP1CameraData *data = cameraData(activeCamera_);
 	Request *request = buffer->request();
 
-	RkISP1FrameInfo *info = data->frameInfo_.find(buffer);
-	if (!info)
-		return;
 
 	if (buffer->metadata().status == FrameMetadata::FrameCancelled) {
-		info->metadataProcessed = true;
 		inFlightStatBuffers_.erase(request->sequence());
+		availableStatBuffers_.push(buffer);
 		tryCompleteRequest(request);
 		return;
 	}
 
-	data->ipa_->processStatsBuffer(request->sequence(), info->statBuffer->cookie(),
+	data->ipa_->processStatsBuffer(request->sequence(), buffer->cookie(),
 				       data->delayedCtrls_->get(request->sequence()));
 }
 
-- 
2.34.1



More information about the libcamera-devel mailing list