[libcamera-devel] [PATCH v2 10/16] libcamera: stream: Shorten access to the bufferPool

[Niklas Söderlund]

Hi Jacopo,

Thanks for your patch.

On 2019-07-13 20:23:45 +0300, Laurent Pinchart wrote:
> From: Jacopo Mondi <jacopo at jmondi.org>
> 
> All interactions with the Stream's buffers currently go through the
> BufferPool. In order to shorten accessing the buffers array, and eventually
> restrict access to the Stream's internal buffer pool, provide operations to
> access, create and destroy buffers.
> 
> It is still possible to access the pool for pipeline handlers to
> populate it by exporting buffers from a video device to Stream's pool.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  include/libcamera/stream.h |  4 ++++
>  src/cam/capture.cpp        |  2 +-
>  src/libcamera/camera.cpp   |  4 ++--
>  src/libcamera/stream.cpp   | 36 ++++++++++++++++++++++++++++++++++--
>  src/qcam/main_window.cpp   |  2 +-
>  5 files changed, 42 insertions(+), 6 deletions(-)
> 
> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h
> index f595a630eb4a..bc14fb60480e 100644
> --- a/include/libcamera/stream.h
> +++ b/include/libcamera/stream.h
> @@ -71,11 +71,15 @@ public:
>  	std::unique_ptr<Buffer> createBuffer(unsigned int index);
>  
>  	BufferPool &bufferPool() { return bufferPool_; }
> +	std::vector<BufferMemory> &buffers() { return bufferPool_.buffers(); }
>  	const StreamConfiguration &configuration() const { return configuration_; }
>  
>  protected:
>  	friend class Camera;
>  
> +	void createBuffers(unsigned int count);
> +	void destroyBuffers();
> +
>  	BufferPool bufferPool_;
>  	StreamConfiguration configuration_;
>  };
> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> index 66ec1abaa5ac..5ffa4ae291da 100644
> --- a/src/cam/capture.cpp
> +++ b/src/cam/capture.cpp
> @@ -154,7 +154,7 @@ void Capture::requestComplete(Request *request, const std::map<Stream *, Buffer
>  	for (auto it = buffers.begin(); it != buffers.end(); ++it) {
>  		Stream *stream = it->first;
>  		Buffer *buffer = it->second;
> -		BufferMemory *mem = &stream->bufferPool().buffers()[buffer->index()];
> +		BufferMemory *mem = &stream->buffers()[buffer->index()];
>  		const std::string &name = streamName_[stream];
>  
>  		info << " " << name
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 1f307654ab01..61d3e821f48f 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -683,7 +683,7 @@ int Camera::configure(CameraConfiguration *config)
>  		 * Allocate buffer objects in the pool.
>  		 * Memory will be allocated and assigned later.
>  		 */
> -		stream->bufferPool().createBuffers(cfg.bufferCount);
> +		stream->createBuffers(cfg.bufferCount);
>  	}
>  
>  	state_ = CameraConfigured;
> @@ -744,7 +744,7 @@ int Camera::freeBuffers()
>  		 * All mappings must be destroyed before buffers can be freed
>  		 * by the V4L2 device that has allocated them.
>  		 */
> -		stream->bufferPool().destroyBuffers();
> +		stream->destroyBuffers();
>  	}
>  
>  	state_ = CameraConfigured;
> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp
> index 6d80646b55cd..884fbfebd52c 100644
> --- a/src/libcamera/stream.cpp
> +++ b/src/libcamera/stream.cpp
> @@ -435,19 +435,51 @@ std::unique_ptr<Buffer> Stream::createBuffer(unsigned int index)
>   * \fn Stream::bufferPool()
>   * \brief Retrieve the buffer pool for the stream
>   *
> - * The buffer pool handles the buffers used to capture frames at the output of
> - * the stream. It is initially created empty and shall be populated with
> + * The buffer pool handles the memory buffers used to store frames for the
> + * stream. It is initially created empty and shall be populated with
>   * buffers before being used.
>   *
>   * \return A reference to the buffer pool
>   */
>  
> +/**
> + * \fn Stream::buffers()
> + * \brief Retrieve the memory buffers in the Stream's buffer pool
> + * \return The list of stream's memory buffers
> + */
> +
>  /**
>   * \fn Stream::configuration()
>   * \brief Retrieve the active configuration of the stream
>   * \return The active configuration of the stream
>   */
>  
> +/**
> + * \brief Create buffers for the stream
> + * \param count The number of buffers to create

s/param/param[in]/

With this fixed,

Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>

> + *
> + * Create \a count empty buffers in the Stream's buffer pool.
> + */
> +void Stream::createBuffers(unsigned int count)
> +{
> +	destroyBuffers();
> +	if (count == 0)
> +		return;
> +
> +	bufferPool_.createBuffers(count);
> +}
> +
> +/**
> + * \brief Destroy buffers in the stream
> + *
> + * If no buffers have been created or if buffers have already been destroyed no
> + * operation is performed.
> + */
> +void Stream::destroyBuffers()
> +{
> +	bufferPool_.destroyBuffers();
> +}
> +
>  /**
>   * \var Stream::bufferPool_
>   * \brief The pool of buffers associated with the stream
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 7023e1158fb6..92f888323f0b 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -235,7 +235,7 @@ void MainWindow::requestComplete(Request *request,
>  		  << " fps: " << std::fixed << std::setprecision(2) << fps
>  		  << std::endl;
>  
> -	BufferMemory *mem = &stream->bufferPool().buffers()[buffer->index()];
> +	BufferMemory *mem = &stream->buffers()[buffer->index()];
>  	display(buffer, mem);
>  
>  	request = camera_->createRequest();
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund