[libcamera-devel] [PATCH 17/20] libcamera: pipeline: simple: Hardcode the number of internal buffers

[Kieran Bingham]

On 31/01/2021 22:46, Laurent Pinchart wrote:
> The number of internal buffers, used between the capture device and the
> converter, doesn't need to depend on the number of buffers allocated for
> the output stream of the pipeline. Hardcode it to a fixed value.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/libcamera/pipeline/simple/simple.cpp | 21 +++++++++++++++------
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index 9dffe64ee870..c987e1a0d9cb 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -228,6 +228,8 @@ protected:
>  	int queueRequestDevice(Camera *camera, Request *request) override;
>  
>  private:
> +	static constexpr unsigned int kNumInternalBuffers = 3;

Will it be obvious that these are 'converter' buffers?

> +
>  	SimpleCameraData *cameraData(const Camera *camera)
>  	{
>  		return static_cast<SimpleCameraData *>(
> @@ -699,7 +701,7 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)
>  		inputCfg.pixelFormat = pipeConfig->captureFormat;
>  		inputCfg.size = pipeConfig->captureSize;
>  		inputCfg.stride = captureFormat.planes[0].bpl;
> -		inputCfg.bufferCount = cfg.bufferCount;
> +		inputCfg.bufferCount = kNumInternalBuffers;
>  
>  		ret = converter_->configure(inputCfg, { cfg });
>  		if (ret < 0) {
> @@ -736,13 +738,20 @@ int SimplePipelineHandler::start(Camera *camera, [[maybe_unused]] ControlList *c
>  {
>  	SimpleCameraData *data = cameraData(camera);
>  	V4L2VideoDevice *video = data->video_;
> -	unsigned int count = data->streams_[0].configuration().bufferCount;
>  	int ret;
>  
> -	if (data->useConverter_)
> -		ret = video->allocateBuffers(count, &data->converterBuffers_);
> -	else
> -		ret = video->importBuffers(count);
> +	if (data->useConverter_) {
> +		/*
> +		 * When using the converter allocate a fixed number of internal
> +		 * buffers.
> +		 */
> +		ret = video->allocateBuffers(kNumInternalBuffers,
> +					     &data->converterBuffers_);

I think that means probably yes ;-)

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> +	} else {
> +		/* Otherwise, prepare for using buffers from the only stream. */
> +		Stream *stream = &data->streams_[0];
> +		ret = video->importBuffers(stream->configuration().bufferCount);
> +	}
>  	if (ret < 0)
>  		return ret;
>  
> 

-- 
Regards
--
Kieran