[libcamera-devel,v2] libcamera: pipeline: rkisp1: Use correct buffer count when importing buffers
diff mbox series

Message ID 20200320003224.3536634-1-niklas.soderlund@ragnatech.se
State Accepted
Commit a118b1a49110b43f4bff5616a5072ade41f8b684
Headers show
Series
  • [libcamera-devel,v2] libcamera: pipeline: rkisp1: Use correct buffer count when importing buffers
Related show

Commit Message

Niklas Söderlund March 20, 2020, 12:32 a.m. UTC 
When folding buffer management with start/stop the wrong variable was
passed to importBuffers() resulting in only one buffer being imported
for the video node making capture impossible. Fix this by first renaming
the confusingly named variable 'count' to 'ipaBufferId'. And then
reusing the 'count' name for the buffer count.

While at it remove the loop to find the maximum value of buffers from
the single stream used by the pipeline. Once we add more stream this
needs to be reworked anyhow so keep it simple for now.

Fixes: 33fedea818e2b6a9 ("libcamera: pipeline_handler: Fold buffer management with start/stop")
Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

Comments

Laurent Pinchart March 20, 2020, 12:40 a.m. UTC | #1 
Hi Niklas,

Thank you for the patch.

On Fri, Mar 20, 2020 at 01:32:24AM +0100, Niklas Söderlund wrote:
> When folding buffer management with start/stop the wrong variable was
> passed to importBuffers() resulting in only one buffer being imported
> for the video node making capture impossible. Fix this by first renaming
> the confusingly named variable 'count' to 'ipaBufferId'. And then
> reusing the 'count' name for the buffer count.
> 
> While at it remove the loop to find the maximum value of buffers from
> the single stream used by the pipeline. Once we add more stream this
> needs to be reworked anyhow so keep it simple for now.
> 
> Fixes: 33fedea818e2b6a9 ("libcamera: pipeline_handler: Fold buffer management with start/stop")
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 97bb4f72cde5423e..ec54291db416a669 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -668,34 +668,31 @@ int PipelineHandlerRkISP1::exportFrameBuffers(Camera *camera, Stream *stream,
>  int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
>  {
>  	RkISP1CameraData *data = cameraData(camera);
> -	unsigned int count = 1;
> +	unsigned int count = data->stream_.configuration().bufferCount;
> +	unsigned int ipaBufferId = 1;
>  	int ret;
>  
> -	unsigned int maxBuffers = 0;
> -	for (const Stream *s : camera->streams())
> -		maxBuffers = std::max(maxBuffers, s->configuration().bufferCount);
> -
>  	ret = video_->importBuffers(count);
>  	if (ret < 0)
>  		goto error;
>  
> -	ret = param_->allocateBuffers(maxBuffers, &paramBuffers_);
> +	ret = param_->allocateBuffers(count, &paramBuffers_);
>  	if (ret < 0)
>  		goto error;
>  
> -	ret = stat_->allocateBuffers(maxBuffers, &statBuffers_);
> +	ret = stat_->allocateBuffers(count, &statBuffers_);
>  	if (ret < 0)
>  		goto error;
>  
>  	for (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) {
> -		buffer->setCookie(count++);
> +		buffer->setCookie(ipaBufferId++);
>  		data->ipaBuffers_.push_back({ .id = buffer->cookie(),
>  					      .planes = buffer->planes() });
>  		availableParamBuffers_.push(buffer.get());
>  	}
>  
>  	for (std::unique_ptr<FrameBuffer> &buffer : statBuffers_) {
> -		buffer->setCookie(count++);
> +		buffer->setCookie(ipaBufferId++);
>  		data->ipaBuffers_.push_back({ .id = buffer->cookie(),
>  					      .planes = buffer->planes() });
>  		availableStatBuffers_.push(buffer.get());
Niklas Söderlund March 20, 2020, 12:53 a.m. UTC | #2 
Pushed to master.

On 2020-03-20 01:32:24 +0100, Niklas Söderlund wrote:
> When folding buffer management with start/stop the wrong variable was
> passed to importBuffers() resulting in only one buffer being imported
> for the video node making capture impossible. Fix this by first renaming
> the confusingly named variable 'count' to 'ipaBufferId'. And then
> reusing the 'count' name for the buffer count.
> 
> While at it remove the loop to find the maximum value of buffers from
> the single stream used by the pipeline. Once we add more stream this
> needs to be reworked anyhow so keep it simple for now.
> 
> Fixes: 33fedea818e2b6a9 ("libcamera: pipeline_handler: Fold buffer management with start/stop")
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 97bb4f72cde5423e..ec54291db416a669 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -668,34 +668,31 @@ int PipelineHandlerRkISP1::exportFrameBuffers(Camera *camera, Stream *stream,
>  int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
>  {
>  	RkISP1CameraData *data = cameraData(camera);
> -	unsigned int count = 1;
> +	unsigned int count = data->stream_.configuration().bufferCount;
> +	unsigned int ipaBufferId = 1;
>  	int ret;
>  
> -	unsigned int maxBuffers = 0;
> -	for (const Stream *s : camera->streams())
> -		maxBuffers = std::max(maxBuffers, s->configuration().bufferCount);
> -
>  	ret = video_->importBuffers(count);
>  	if (ret < 0)
>  		goto error;
>  
> -	ret = param_->allocateBuffers(maxBuffers, &paramBuffers_);
> +	ret = param_->allocateBuffers(count, &paramBuffers_);
>  	if (ret < 0)
>  		goto error;
>  
> -	ret = stat_->allocateBuffers(maxBuffers, &statBuffers_);
> +	ret = stat_->allocateBuffers(count, &statBuffers_);
>  	if (ret < 0)
>  		goto error;
>  
>  	for (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) {
> -		buffer->setCookie(count++);
> +		buffer->setCookie(ipaBufferId++);
>  		data->ipaBuffers_.push_back({ .id = buffer->cookie(),
>  					      .planes = buffer->planes() });
>  		availableParamBuffers_.push(buffer.get());
>  	}
>  
>  	for (std::unique_ptr<FrameBuffer> &buffer : statBuffers_) {
> -		buffer->setCookie(count++);
> +		buffer->setCookie(ipaBufferId++);
>  		data->ipaBuffers_.push_back({ .id = buffer->cookie(),
>  					      .planes = buffer->planes() });
>  		availableStatBuffers_.push(buffer.get());
> -- 
> 2.25.1
>

Patch
diff mbox series 

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 97bb4f72cde5423e..ec54291db416a669 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -668,34 +668,31 @@  int PipelineHandlerRkISP1::exportFrameBuffers(Camera *camera, Stream *stream,
 int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
 {
 	RkISP1CameraData *data = cameraData(camera);
-	unsigned int count = 1;
+	unsigned int count = data->stream_.configuration().bufferCount;
+	unsigned int ipaBufferId = 1;
 	int ret;
 
-	unsigned int maxBuffers = 0;
-	for (const Stream *s : camera->streams())
-		maxBuffers = std::max(maxBuffers, s->configuration().bufferCount);
-
 	ret = video_->importBuffers(count);
 	if (ret < 0)
 		goto error;
 
-	ret = param_->allocateBuffers(maxBuffers, &paramBuffers_);
+	ret = param_->allocateBuffers(count, &paramBuffers_);
 	if (ret < 0)
 		goto error;
 
-	ret = stat_->allocateBuffers(maxBuffers, &statBuffers_);
+	ret = stat_->allocateBuffers(count, &statBuffers_);
 	if (ret < 0)
 		goto error;
 
 	for (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) {
-		buffer->setCookie(count++);
+		buffer->setCookie(ipaBufferId++);
 		data->ipaBuffers_.push_back({ .id = buffer->cookie(),
 					      .planes = buffer->planes() });
 		availableParamBuffers_.push(buffer.get());
 	}
 
 	for (std::unique_ptr<FrameBuffer> &buffer : statBuffers_) {
-		buffer->setCookie(count++);
+		buffer->setCookie(ipaBufferId++);
 		data->ipaBuffers_.push_back({ .id = buffer->cookie(),
 					      .planes = buffer->planes() });
 		availableStatBuffers_.push(buffer.get());