[libcamera-devel] [PATCH 3/9] libcamera: pipeline: Support external buffers

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2019-07-05 00:53:28 +0200, Jacopo Mondi wrote:
> Add support for use external buffers to libcamera pipeline handlers.
> 
> Use the memory type flag in pipeline handlers (todo: change all pipeline
> handlers) during buffer setup operations, to decide if the Stream's
> internal memory has to be exported to applications, or the Stream
> should prepare to use buffers whose memory is allocated elsewhere.

I think you can drop the TODO now as it seems all pipeline handlers are 
now supprted, right?

> 
> To support the last use case, a translation mechanism between the external
> buffers provided by applications and internal ones used by pipeline
> handlers to feed the video device will be implemented on top of this
> change.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp     | 31 +++++++++++++++++++++++-
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp |  9 ++++++-
>  src/libcamera/pipeline/uvcvideo.cpp      |  8 +++++-
>  src/libcamera/pipeline/vimc.cpp          |  8 +++++-
>  4 files changed, 52 insertions(+), 4 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 2de0892138a8..6f14da1f8c70 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -71,6 +71,7 @@ public:
>  
>  	int importBuffers(BufferPool *pool);
>  	int exportBuffers(ImgUOutput *output, BufferPool *pool);
> +	int reserveDmabufBuffers(ImgUOutput *output, BufferPool *pool);
>  	void freeBuffers();
>  
>  	int start();
> @@ -624,7 +625,11 @@ int PipelineHandlerIPU3::allocateBuffers(Camera *camera,
>  		IPU3Stream *stream = static_cast<IPU3Stream *>(s);
>  		ImgUDevice::ImgUOutput *dev = stream->device_;
>  
> -		ret = imgu->exportBuffers(dev, &stream->bufferPool());
> +		if (stream->memoryType() == ExternalMemory)
> +			ret = imgu->reserveDmabufBuffers(dev,
> +							 &stream->bufferPool());

I would s/reserveDmabufBuffers/importBuffers/ to match the logic in 
other pipeline handlers.

> +		else
> +			ret = imgu->exportBuffers(dev, &stream->bufferPool());
>  		if (ret)
>  			goto error;
>  	}
> @@ -1153,6 +1158,30 @@ int ImgUDevice::exportBuffers(ImgUOutput *output, BufferPool *pool)
>  	return 0;
>  }
>  
> +/**
> + * \brief Reserve buffers in \a output from the provided \a pool
> + * \param[in] output The ImgU output device
> + * \param[in] pool The buffer pool used to reserve buffers in \a output
> + *
> + * Reserve a number of buffers equal to the number of buffers in \a pool
> + * in the \a output device to prepare for streaming operations using buffers
> + * whose memory has been reserved elsewhere identified with DMABUF file
> + * descriptors.
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int ImgUDevice::reserveDmabufBuffers(ImgUOutput *output, BufferPool *pool)
> +{
> +	int ret = output->dev->importBuffers(pool);
> +	if (ret) {
> +		LOG(IPU3, Error) << "Failed to import buffer in "
> +				 << output->name << " ImgU device";
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  /**
>   * \brief Release buffers for all the ImgU video devices
>   */
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 4a5898d25f91..dbb61a21354d 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -319,7 +319,14 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,
>  					   const std::set<Stream *> &streams)
>  {
>  	Stream *stream = *streams.begin();
> -	return video_->exportBuffers(&stream->bufferPool());
> +	int ret;
> +
> +	if (stream->memoryType() == InternalMemory)
> +		ret = video_->exportBuffers(&stream->bufferPool());
> +	else
> +		ret = video_->importBuffers(&stream->bufferPool());
> +
> +	return ret;
>  }
>  
>  int PipelineHandlerRkISP1::freeBuffers(Camera *camera,
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index 72313cfd246f..5b3c79dda6dd 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -197,10 +197,16 @@ int PipelineHandlerUVC::allocateBuffers(Camera *camera,
>  	UVCCameraData *data = cameraData(camera);
>  	Stream *stream = *streams.begin();
>  	const StreamConfiguration &cfg = stream->configuration();
> +	int ret;
>  
>  	LOG(UVC, Debug) << "Requesting " << cfg.bufferCount << " buffers";
>  
> -	return data->video_->exportBuffers(&stream->bufferPool());
> +	if (stream->memoryType() == InternalMemory)
> +		ret = data->video_->exportBuffers(&stream->bufferPool());
> +	else
> +		ret = data->video_->importBuffers(&stream->bufferPool());
> +
> +	return ret;
>  }
>  
>  int PipelineHandlerUVC::freeBuffers(Camera *camera,
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index f8a58be060bb..c67bdb0ecbff 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -199,10 +199,16 @@ int PipelineHandlerVimc::allocateBuffers(Camera *camera,
>  	VimcCameraData *data = cameraData(camera);
>  	Stream *stream = *streams.begin();
>  	const StreamConfiguration &cfg = stream->configuration();
> +	int ret;
>  
>  	LOG(VIMC, Debug) << "Requesting " << cfg.bufferCount << " buffers";
>  
> -	return data->video_->exportBuffers(&stream->bufferPool());
> +	if (stream->memoryType() == InternalMemory)
> +		ret = data->video_->exportBuffers(&stream->bufferPool());
> +	else
> +		ret = data->video_->importBuffers(&stream->bufferPool());
> +
> +	return ret;
>  }
>  
>  int PipelineHandlerVimc::freeBuffers(Camera *camera,
> -- 
> 2.21.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund