[libcamera-devel,v4,21/32] libcamera: pipeline: rkisp1: Destroy frame information before completing request

Message ID 20200112010212.2609025-22-niklas.soderlund@ragnatech.se
State Accepted
Headers show
Series
  • libcamera: Rework buffer API
Related show

Commit Message

Niklas Söderlund Jan. 12, 2020, 1:02 a.m. UTC
It's common for applications to create and queue a new request in a
previous request completion handler. When the new request gets queued to
the RkISP1 pipeline handler it tries to find a parameters and statistic
buffer to be used with the request. The problem is if the pipeline depth
is already filled there are no internal buffers free to be used by the
new request.

This was solved by allocation one more parameters and statistic buffer
then the pipeline depth, this is waste full. Instead free the resources
of the request that has completed before it is signaled to the
application, this way if the pipeline depth is full it can reuse the
internal resources and the wasteful allocation can be removed.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
* Changes since v2
- Rewrote commit message.
- Actually remove the wasteful allocation of extra buffers in this
  commit instead of depending on the switch to FrameBuffer to do it for
  us.
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Patch

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 979b670e4cb75512..607ff85539a22324 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -671,14 +671,14 @@  int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,
 	if (ret)
 		return ret;
 
-	paramPool_.createBuffers(stream->configuration().bufferCount + 1);
+	paramPool_.createBuffers(stream->configuration().bufferCount);
 	ret = param_->exportBuffers(&paramPool_);
 	if (ret) {
 		video_->releaseBuffers();
 		return ret;
 	}
 
-	statPool_.createBuffers(stream->configuration().bufferCount + 1);
+	statPool_.createBuffers(stream->configuration().bufferCount);
 	ret = stat_->exportBuffers(&statPool_);
 	if (ret) {
 		param_->releaseBuffers();
@@ -686,13 +686,13 @@  int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,
 		return ret;
 	}
 
-	for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) {
+	for (unsigned int i = 0; i < stream->configuration().bufferCount; i++) {
 		data->ipaBuffers_.push_back({ .id = RKISP1_PARAM_BASE | i,
 					      .planes = paramPool_.buffers()[i].planes() });
 		paramBuffers_.push(new Buffer(i));
 	}
 
-	for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) {
+	for (unsigned int i = 0; i < stream->configuration().bufferCount; i++) {
 		data->ipaBuffers_.push_back({ .id = RKISP1_STAT_BASE | i,
 					      .planes = statPool_.buffers()[i].planes() });
 		statBuffers_.push(new Buffer(i));
@@ -981,9 +981,9 @@  void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)
 	if (!info->paramDequeued)
 		return;
 
-	completeRequest(activeCamera_, request);
-
 	data->frameInfo_.destroy(info->frame);
+
+	completeRequest(activeCamera_, request);
 }
 
 void PipelineHandlerRkISP1::bufferReady(Buffer *buffer)