[v2,1/5] libcamera: software_isp: Track frames and requests
diff mbox series

Message ID 20241219211010.103310-2-mzamazal@redhat.com
State New
Headers show
Series
  • ipa: simple: Introduce metadata reporting
Related show

Commit Message

Milan Zamazal Dec. 19, 2024, 9:10 p.m. UTC
Hardware pipelines track requests and other information related to
particular frames.  This hasn't been needed in software ISP so far.  But
in order to be able to track metadata corresponding to a given frame,
frame-request tracking mechanism starts being useful.  This patch
introduces the basic tracking structure, actual metadata handling is
added in the following patch.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 src/libcamera/pipeline/simple/simple.cpp | 87 ++++++++++++++++++++++--
 1 file changed, 82 insertions(+), 5 deletions(-)

Comments

Barnabás Pőcze Dec. 19, 2024, 9:50 p.m. UTC | #1
Hi


2024. december 19., csütörtök 22:10 keltezéssel, Milan Zamazal <mzamazal@redhat.com> írta:

> Hardware pipelines track requests and other information related to
> particular frames.  This hasn't been needed in software ISP so far.  But
> in order to be able to track metadata corresponding to a given frame,
> frame-request tracking mechanism starts being useful.  This patch
> introduces the basic tracking structure, actual metadata handling is
> added in the following patch.
> 
> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
> ---
>  src/libcamera/pipeline/simple/simple.cpp | 87 ++++++++++++++++++++++--
>  1 file changed, 82 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 8ac24e6e..07c1cf1f 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -181,6 +181,70 @@ LOG_DEFINE_CATEGORY(SimplePipeline)
> 
>  class SimplePipelineHandler;
> 
> +struct SimpleFrameInfo {
> +	uint32_t frame;
> +	Request *request;
> +};
> +
> +class SimpleFrames
> +{
> +public:
> +	void create(Request *request);
> +	void destroy(uint32_t frame);
> +	void clear();
> +
> +	SimpleFrameInfo *find(uint32_t frame);
> +	SimpleFrameInfo *find(Request *request);
> +
> +private:
> +	std::map<uint32_t, SimpleFrameInfo *> frameInfo_;
> +};
> +
> +void SimpleFrames::create(Request *request)
> +{
> +	uint32_t frame = request->sequence();
> +	SimpleFrameInfo *info = new SimpleFrameInfo;

Any reason for dynamic allocation? I took a cursory look over the later changes
and didn't see anything. If there is indeed no reason, please do the following:

  std::map<uint32_t, SimpleFrameInfo> frameInfo_;
  ...
  auto [it, inserted] = frameInfo_.try_emplace(request->sequence());
  ASSERT(inserted); // or whatever is appropriate when the sequence number already exists
  
  it->second.... = ...;


Regards,
Barnabás Pőcze


> +	info->frame = frame;
> +	info->request = request;
> +	frameInfo_[frame] = info;
> +}
> +
> +void SimpleFrames::destroy(uint32_t frame)
> +{
> +	SimpleFrameInfo *info = find(frame);
> +	if (!info)
> +		return;
> +	delete info;
> +	frameInfo_.erase(frame);
> +}
> +
> +void SimpleFrames::clear()
> +{
> +	for (const auto &info : frameInfo_)
> +		delete info.second;
> +	frameInfo_.clear();
> +}
> +
> +SimpleFrameInfo *SimpleFrames::find(uint32_t frame)
> +{
> +	auto info = frameInfo_.find(frame);
> +	if (info == frameInfo_.end())
> +		return nullptr;
> +	return info->second;
> +}
> +
> +SimpleFrameInfo *SimpleFrames::find(Request *request)
> +{
> +	for (auto &itInfo : frameInfo_) {
> +		SimpleFrameInfo *info = itInfo.second;
> +
> +		if (info->request == request)
> +			return info;
> +	}
> +
> +	return nullptr;
> +}
> +
>  struct SimplePipelineInfo {
>  	const char *driver;
>  	/*
> @@ -293,11 +357,13 @@ public:
> 
>  	std::unique_ptr<Converter> converter_;
>  	std::unique_ptr<SoftwareIsp> swIsp_;
> +	SimpleFrames frameInfo_;
> 
>  private:
>  	void tryPipeline(unsigned int code, const Size &size);
>  	static std::vector<const MediaPad *> routedSourcePads(MediaPad *sink);
> 
> +	void completeRequest(Request *request);
>  	void conversionInputDone(FrameBuffer *buffer);
>  	void conversionOutputDone(FrameBuffer *buffer);
> 
> @@ -799,7 +865,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)
>  			/* No conversion, just complete the request. */
>  			Request *request = buffer->request();
>  			pipe->completeBuffer(request, buffer);
> -			pipe->completeRequest(request);
> +			completeRequest(request);
>  			return;
>  		}
> 
> @@ -817,7 +883,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)
>  		const RequestOutputs &outputs = conversionQueue_.front();
>  		for (auto &[stream, buf] : outputs.outputs)
>  			pipe->completeBuffer(outputs.request, buf);
> -		pipe->completeRequest(outputs.request);
> +		completeRequest(outputs.request);
>  		conversionQueue_.pop();
> 
>  		return;
> @@ -875,7 +941,7 @@ void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)
> 
>  	/* Otherwise simply complete the request. */
>  	pipe->completeBuffer(request, buffer);
> -	pipe->completeRequest(request);
> +	completeRequest(request);
>  }
> 
>  void SimpleCameraData::clearIncompleteRequests()
> @@ -886,6 +952,14 @@ void SimpleCameraData::clearIncompleteRequests()
>  	}
>  }
> 
> +void SimpleCameraData::completeRequest(Request *request)
> +{
> +	SimpleFrameInfo *info = frameInfo_.find(request);
> +	if (info)
> +		frameInfo_.destroy(info->frame);
> +	pipe()->completeRequest(request);
> +}
> +
>  void SimpleCameraData::conversionInputDone(FrameBuffer *buffer)
>  {
>  	/* Queue the input buffer back for capture. */
> @@ -899,7 +973,7 @@ void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)
>  	/* Complete the buffer and the request. */
>  	Request *request = buffer->request();
>  	if (pipe->completeBuffer(request, buffer))
> -		pipe->completeRequest(request);
> +		completeRequest(request);
>  }
> 
>  void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId)
> @@ -1412,6 +1486,7 @@ void SimplePipelineHandler::stopDevice(Camera *camera)
> 
>  	video->bufferReady.disconnect(data, &SimpleCameraData::imageBufferReady);
> 
> +	data->frameInfo_.clear();
>  	data->clearIncompleteRequests();
>  	data->conversionBuffers_.clear();
> 
> @@ -1442,8 +1517,10 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)
> 
>  	if (data->useConversion_) {
>  		data->conversionQueue_.push({ request, std::move(buffers) });
> -		if (data->swIsp_)
> +		if (data->swIsp_) {
> +			data->frameInfo_.create(request);
>  			data->swIsp_->queueRequest(request->sequence(), request->controls());
> +		}
>  	}
> 
>  	return 0;
> --
> 2.44.2
> 
>

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
index 8ac24e6e..07c1cf1f 100644
--- a/src/libcamera/pipeline/simple/simple.cpp
+++ b/src/libcamera/pipeline/simple/simple.cpp
@@ -181,6 +181,70 @@  LOG_DEFINE_CATEGORY(SimplePipeline)
 
 class SimplePipelineHandler;
 
+struct SimpleFrameInfo {
+	uint32_t frame;
+	Request *request;
+};
+
+class SimpleFrames
+{
+public:
+	void create(Request *request);
+	void destroy(uint32_t frame);
+	void clear();
+
+	SimpleFrameInfo *find(uint32_t frame);
+	SimpleFrameInfo *find(Request *request);
+
+private:
+	std::map<uint32_t, SimpleFrameInfo *> frameInfo_;
+};
+
+void SimpleFrames::create(Request *request)
+{
+	uint32_t frame = request->sequence();
+	SimpleFrameInfo *info = new SimpleFrameInfo;
+	info->frame = frame;
+	info->request = request;
+	frameInfo_[frame] = info;
+}
+
+void SimpleFrames::destroy(uint32_t frame)
+{
+	SimpleFrameInfo *info = find(frame);
+	if (!info)
+		return;
+	delete info;
+	frameInfo_.erase(frame);
+}
+
+void SimpleFrames::clear()
+{
+	for (const auto &info : frameInfo_)
+		delete info.second;
+	frameInfo_.clear();
+}
+
+SimpleFrameInfo *SimpleFrames::find(uint32_t frame)
+{
+	auto info = frameInfo_.find(frame);
+	if (info == frameInfo_.end())
+		return nullptr;
+	return info->second;
+}
+
+SimpleFrameInfo *SimpleFrames::find(Request *request)
+{
+	for (auto &itInfo : frameInfo_) {
+		SimpleFrameInfo *info = itInfo.second;
+
+		if (info->request == request)
+			return info;
+	}
+
+	return nullptr;
+}
+
 struct SimplePipelineInfo {
 	const char *driver;
 	/*
@@ -293,11 +357,13 @@  public:
 
 	std::unique_ptr<Converter> converter_;
 	std::unique_ptr<SoftwareIsp> swIsp_;
+	SimpleFrames frameInfo_;
 
 private:
 	void tryPipeline(unsigned int code, const Size &size);
 	static std::vector<const MediaPad *> routedSourcePads(MediaPad *sink);
 
+	void completeRequest(Request *request);
 	void conversionInputDone(FrameBuffer *buffer);
 	void conversionOutputDone(FrameBuffer *buffer);
 
@@ -799,7 +865,7 @@  void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)
 			/* No conversion, just complete the request. */
 			Request *request = buffer->request();
 			pipe->completeBuffer(request, buffer);
-			pipe->completeRequest(request);
+			completeRequest(request);
 			return;
 		}
 
@@ -817,7 +883,7 @@  void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)
 		const RequestOutputs &outputs = conversionQueue_.front();
 		for (auto &[stream, buf] : outputs.outputs)
 			pipe->completeBuffer(outputs.request, buf);
-		pipe->completeRequest(outputs.request);
+		completeRequest(outputs.request);
 		conversionQueue_.pop();
 
 		return;
@@ -875,7 +941,7 @@  void SimpleCameraData::imageBufferReady(FrameBuffer *buffer)
 
 	/* Otherwise simply complete the request. */
 	pipe->completeBuffer(request, buffer);
-	pipe->completeRequest(request);
+	completeRequest(request);
 }
 
 void SimpleCameraData::clearIncompleteRequests()
@@ -886,6 +952,14 @@  void SimpleCameraData::clearIncompleteRequests()
 	}
 }
 
+void SimpleCameraData::completeRequest(Request *request)
+{
+	SimpleFrameInfo *info = frameInfo_.find(request);
+	if (info)
+		frameInfo_.destroy(info->frame);
+	pipe()->completeRequest(request);
+}
+
 void SimpleCameraData::conversionInputDone(FrameBuffer *buffer)
 {
 	/* Queue the input buffer back for capture. */
@@ -899,7 +973,7 @@  void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)
 	/* Complete the buffer and the request. */
 	Request *request = buffer->request();
 	if (pipe->completeBuffer(request, buffer))
-		pipe->completeRequest(request);
+		completeRequest(request);
 }
 
 void SimpleCameraData::ispStatsReady(uint32_t frame, uint32_t bufferId)
@@ -1412,6 +1486,7 @@  void SimplePipelineHandler::stopDevice(Camera *camera)
 
 	video->bufferReady.disconnect(data, &SimpleCameraData::imageBufferReady);
 
+	data->frameInfo_.clear();
 	data->clearIncompleteRequests();
 	data->conversionBuffers_.clear();
 
@@ -1442,8 +1517,10 @@  int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request)
 
 	if (data->useConversion_) {
 		data->conversionQueue_.push({ request, std::move(buffers) });
-		if (data->swIsp_)
+		if (data->swIsp_) {
+			data->frameInfo_.create(request);
 			data->swIsp_->queueRequest(request->sequence(), request->controls());
+		}
 	}
 
 	return 0;