[libcamera-devel] [PATCH v2] libcamera: pipeline: rkisp1: Use correct buffer count when importing buffers

[Laurent Pinchart]

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 at ragnatech.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart at 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());

-- 
Regards,

Laurent Pinchart