[libcamera-devel,v4,21/31] RFC: libcamera: pipeline_handlers: Add preAllocateBuffers
diff mbox series

Message ID 20190320163055.22056-22-jacopo@jmondi.org
State Superseded
Headers show
Series
  • libcamera: ipu3: Add ImgU support + multiple streams
Related show

Commit Message

Jacopo Mondi March 20, 2019, 4:30 p.m. UTC 
Add a preAllocateBuffers virtual method to the PipelineHandler base
class and call it before performing per-stream memory allocation.

Implement the method in the IPU3 pipeline handler to perform memory
allocation on the CIO2 unit and the ImgU input and stat video devices.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/camera.cpp                 | 10 ++++-
 src/libcamera/include/pipeline_handler.h |  5 +++
 src/libcamera/pipeline/ipu3/ipu3.cpp     | 54 ++++++++++++++----------
 src/libcamera/pipeline_handler.cpp       | 21 ++++++++-
 4 files changed, 66 insertions(+), 24 deletions(-)

Comments

Laurent Pinchart March 23, 2019, 1:57 p.m. UTC | #1 
Hi Jacopo,

Thank you for the patch.

On Wed, Mar 20, 2019 at 05:30:45PM +0100, Jacopo Mondi wrote:
> Add a preAllocateBuffers virtual method to the PipelineHandler base
> class and call it before performing per-stream memory allocation.
> 
> Implement the method in the IPU3 pipeline handler to perform memory
> allocation on the CIO2 unit and the ImgU input and stat video devices.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/libcamera/camera.cpp                 | 10 ++++-
>  src/libcamera/include/pipeline_handler.h |  5 +++
>  src/libcamera/pipeline/ipu3/ipu3.cpp     | 54 ++++++++++++++----------
>  src/libcamera/pipeline_handler.cpp       | 21 ++++++++-
>  4 files changed, 66 insertions(+), 24 deletions(-)
> 
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 8ee9cc086616..8020dff8f8ea 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -455,6 +455,8 @@ int Camera::configureStreams(std::map<Stream *, StreamConfiguration> &config)
>   */
>  int Camera::allocateBuffers()
>  {
> +	int ret;
> +
>  	if (disconnected_)
>  		return -ENODEV;
>  
> @@ -467,8 +469,14 @@ int Camera::allocateBuffers()
>  		return -EINVAL;
>  	}
>  
> +	ret = pipe_->preAllocateBuffers(this, activeStreams_);
> +	if (ret) {
> +		LOG(Camera, Error) << "Buffers pre-allocation failed";
> +		return ret;
> +	}
> +

I'm not a big fan of pre/post hooks. I think this should the
PipelineHandler::allocateBuffers() and PipelineHandler::freeBuffers()
methods are not correctly designed. It may be better to give them the
list of active streams, moving the loop from Camera to the pipeline
handlers. That way pipeline handlers could perform any extra step they
require, and even handle dependencies between allocations for different
streams if the need arises.

>  	for (Stream *stream : activeStreams_) {
> -		int ret = pipe_->allocateBuffers(this, stream);
> +		ret = pipe_->allocateBuffers(this, stream);
>  		if (ret) {
>  			LOG(Camera, Error) << "Failed to allocate buffers";
>  			freeBuffers();
> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> index acb376e07030..7ce5b67cc7fc 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -57,6 +57,11 @@ public:
>  	virtual int configureStreams(Camera *camera,
>  				     std::map<Stream *, StreamConfiguration> &config) = 0;
>  
> +	virtual int preAllocateBuffers(Camera *camera,
> +				       const std::set<Stream *> &activeStreams)
> +	{
> +		return 0;
> +	}
>  	virtual int allocateBuffers(Camera *camera, Stream *stream) = 0;
>  	virtual int freeBuffers(Camera *camera, Stream *stream) = 0;
>  
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 0507fc380e68..5f323888d84f 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -105,6 +105,8 @@ public:
>  	int configureStreams(Camera *camera,
>  			     std::map<Stream *, StreamConfiguration> &config) override;
>  
> +	int preAllocateBuffers(Camera *camera,
> +			       const std::set<Stream *> &activeStreams) override;
>  	int allocateBuffers(Camera *camera, Stream *stream) override;
>  	int freeBuffers(Camera *camera, Stream *stream) override;
>  
> @@ -451,40 +453,48 @@ int PipelineHandlerIPU3::configureStreams(Camera *camera,
>  	return 0;
>  }
>  
> -int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream)
> +int PipelineHandlerIPU3::preAllocateBuffers(Camera *camera,
> +					    const std::set<Stream *> &activeStreams)
>  {
>  	IPU3CameraData *data = cameraData(camera);
> -	V4L2Device *viewfinder = data->imgu->viewfinder;
> -	V4L2Device *output = data->imgu->output;
>  	V4L2Device *input = data->imgu->input;
>  	V4L2Device *cio2 = data->cio2.output;
>  	V4L2Device *stat = data->imgu->stat;
>  	ImgUDevice *imgu = data->imgu;
>  	int ret;
>  
> -	if (data->cio2.pool.count() == 0) {
> -		/* Share buffers between CIO2 output and ImgU input. */
> -		data->cio2.pool.createBuffers(IPU3_CIO2_BUFFER_COUNT);
> -		ret = cio2->exportBuffers(&data->cio2.pool);
> -		if (ret) {
> -			LOG(IPU3, Error) << "Failed to reserve CIO2 memory";
> -			return ret;
> -		}
> +	/* Share buffers between CIO2 output and ImgU input. */
> +	data->cio2.pool.createBuffers(IPU3_CIO2_BUFFER_COUNT);
> +	ret = cio2->exportBuffers(&data->cio2.pool);
> +	if (ret) {
> +		LOG(IPU3, Error) << "Failed to reserve CIO2 memory";
> +		return ret;
> +	}
>  
> -		ret = input->importBuffers(&data->cio2.pool);
> -		if (ret) {
> -			LOG(IPU3, Error) << "Failed to import ImgU memory";
> -			return ret;
> -		}
> +	ret = input->importBuffers(&data->cio2.pool);
> +	if (ret) {
> +		LOG(IPU3, Error) << "Failed to import ImgU memory";
> +		return ret;
> +	}
>  
> -		imgu->statPool.createBuffers(IPU3_IMGU_BUFFER_COUNT);
> -		ret = stat->exportBuffers(&imgu->statPool);
> -		if (ret) {
> -			LOG(IPU3, Error) << "Failed to reserve ImgU stat memory";
> -			return ret;
> -		}
> +	imgu->statPool.createBuffers(IPU3_IMGU_BUFFER_COUNT);
> +	ret = stat->exportBuffers(&imgu->statPool);
> +	if (ret) {
> +		LOG(IPU3, Error) << "Failed to reserve ImgU stat memory";
> +		return ret;
>  	}
>  
> +	return 0;
> +}
> +
> +int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream)
> +{
> +	IPU3CameraData *data = cameraData(camera);
> +	V4L2Device *viewfinder = data->imgu->viewfinder;
> +	V4L2Device *output = data->imgu->output;
> +	ImgUDevice *imgu = data->imgu;
> +	int ret;
> +
>  	if (isOutput(data, stream)) {
>  		/* Export ImgU output buffers to the stream's pool. */
>  		ret = output->exportBuffers(&stream->bufferPool());
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 1a858f2638ce..1f556ed789b6 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -189,6 +189,24 @@ PipelineHandler::~PipelineHandler()
>   * \return 0 on success or a negative error code otherwise
>   */
>  
> +/**
> + * \fn PipelineHandler::preAllocateBuffers()
> + * \brief Perform operations to prepare for memory allocation. Optional for
> + * pipeline handlers to implement
> + * \param[in] camera The camera the streams belongs to
> + * \param[in] activeStreams The set of active streams
> + *
> + * If operations to prepare for the per-stream memory allocation are required,
> + * this virtual method provides an entry point for pipeline handlers to do so.
> + *
> + * This method is optional to implement for pipeline handlers, and its intended
> + * caller is the Camera class, which calls it once before performing per-stream
> + * memory allocation by calling the PipelineHandler::allocateBuffers() method
> + * once per each active stream.
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +
>  /**
>   * \fn PipelineHandler::allocateBuffers()
>   * \brief Allocate buffers for a stream
> @@ -198,7 +216,8 @@ PipelineHandler::~PipelineHandler()
>   * This method allocates buffers internally in the pipeline handler and
>   * associates them with the stream's buffer pool.
>   *
> - * The intended caller of this method is the Camera class.
> + * The intended caller of this method is the Camera class which calls it
> + * once per each active stream.
>   *
>   * \return 0 on success or a negative error code otherwise
>   */
Jacopo Mondi March 23, 2019, 2 p.m. UTC | #2 
Hi Laurent,

On Sat, Mar 23, 2019 at 03:57:45PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Wed, Mar 20, 2019 at 05:30:45PM +0100, Jacopo Mondi wrote:
> > Add a preAllocateBuffers virtual method to the PipelineHandler base
> > class and call it before performing per-stream memory allocation.
> >
> > Implement the method in the IPU3 pipeline handler to perform memory
> > allocation on the CIO2 unit and the ImgU input and stat video devices.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  src/libcamera/camera.cpp                 | 10 ++++-
> >  src/libcamera/include/pipeline_handler.h |  5 +++
> >  src/libcamera/pipeline/ipu3/ipu3.cpp     | 54 ++++++++++++++----------
> >  src/libcamera/pipeline_handler.cpp       | 21 ++++++++-
> >  4 files changed, 66 insertions(+), 24 deletions(-)
> >
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index 8ee9cc086616..8020dff8f8ea 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -455,6 +455,8 @@ int Camera::configureStreams(std::map<Stream *, StreamConfiguration> &config)
> >   */
> >  int Camera::allocateBuffers()
> >  {
> > +	int ret;
> > +
> >  	if (disconnected_)
> >  		return -ENODEV;
> >
> > @@ -467,8 +469,14 @@ int Camera::allocateBuffers()
> >  		return -EINVAL;
> >  	}
> >
> > +	ret = pipe_->preAllocateBuffers(this, activeStreams_);
> > +	if (ret) {
> > +		LOG(Camera, Error) << "Buffers pre-allocation failed";
> > +		return ret;
> > +	}
> > +
>
> I'm not a big fan of pre/post hooks. I think this should the
> PipelineHandler::allocateBuffers() and PipelineHandler::freeBuffers()
> methods are not correctly designed. It may be better to give them the
> list of active streams, moving the loop from Camera to the pipeline
> handlers. That way pipeline handlers could perform any extra step they
> require, and even handle dependencies between allocations for different
> streams if the need arises.
>

I agree, I'll do this in v5.

Thanks
  j

> >  	for (Stream *stream : activeStreams_) {
> > -		int ret = pipe_->allocateBuffers(this, stream);
> > +		ret = pipe_->allocateBuffers(this, stream);
> >  		if (ret) {
> >  			LOG(Camera, Error) << "Failed to allocate buffers";
> >  			freeBuffers();
> > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> > index acb376e07030..7ce5b67cc7fc 100644
> > --- a/src/libcamera/include/pipeline_handler.h
> > +++ b/src/libcamera/include/pipeline_handler.h
> > @@ -57,6 +57,11 @@ public:
> >  	virtual int configureStreams(Camera *camera,
> >  				     std::map<Stream *, StreamConfiguration> &config) = 0;
> >
> > +	virtual int preAllocateBuffers(Camera *camera,
> > +				       const std::set<Stream *> &activeStreams)
> > +	{
> > +		return 0;
> > +	}
> >  	virtual int allocateBuffers(Camera *camera, Stream *stream) = 0;
> >  	virtual int freeBuffers(Camera *camera, Stream *stream) = 0;
> >
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 0507fc380e68..5f323888d84f 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -105,6 +105,8 @@ public:
> >  	int configureStreams(Camera *camera,
> >  			     std::map<Stream *, StreamConfiguration> &config) override;
> >
> > +	int preAllocateBuffers(Camera *camera,
> > +			       const std::set<Stream *> &activeStreams) override;
> >  	int allocateBuffers(Camera *camera, Stream *stream) override;
> >  	int freeBuffers(Camera *camera, Stream *stream) override;
> >
> > @@ -451,40 +453,48 @@ int PipelineHandlerIPU3::configureStreams(Camera *camera,
> >  	return 0;
> >  }
> >
> > -int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream)
> > +int PipelineHandlerIPU3::preAllocateBuffers(Camera *camera,
> > +					    const std::set<Stream *> &activeStreams)
> >  {
> >  	IPU3CameraData *data = cameraData(camera);
> > -	V4L2Device *viewfinder = data->imgu->viewfinder;
> > -	V4L2Device *output = data->imgu->output;
> >  	V4L2Device *input = data->imgu->input;
> >  	V4L2Device *cio2 = data->cio2.output;
> >  	V4L2Device *stat = data->imgu->stat;
> >  	ImgUDevice *imgu = data->imgu;
> >  	int ret;
> >
> > -	if (data->cio2.pool.count() == 0) {
> > -		/* Share buffers between CIO2 output and ImgU input. */
> > -		data->cio2.pool.createBuffers(IPU3_CIO2_BUFFER_COUNT);
> > -		ret = cio2->exportBuffers(&data->cio2.pool);
> > -		if (ret) {
> > -			LOG(IPU3, Error) << "Failed to reserve CIO2 memory";
> > -			return ret;
> > -		}
> > +	/* Share buffers between CIO2 output and ImgU input. */
> > +	data->cio2.pool.createBuffers(IPU3_CIO2_BUFFER_COUNT);
> > +	ret = cio2->exportBuffers(&data->cio2.pool);
> > +	if (ret) {
> > +		LOG(IPU3, Error) << "Failed to reserve CIO2 memory";
> > +		return ret;
> > +	}
> >
> > -		ret = input->importBuffers(&data->cio2.pool);
> > -		if (ret) {
> > -			LOG(IPU3, Error) << "Failed to import ImgU memory";
> > -			return ret;
> > -		}
> > +	ret = input->importBuffers(&data->cio2.pool);
> > +	if (ret) {
> > +		LOG(IPU3, Error) << "Failed to import ImgU memory";
> > +		return ret;
> > +	}
> >
> > -		imgu->statPool.createBuffers(IPU3_IMGU_BUFFER_COUNT);
> > -		ret = stat->exportBuffers(&imgu->statPool);
> > -		if (ret) {
> > -			LOG(IPU3, Error) << "Failed to reserve ImgU stat memory";
> > -			return ret;
> > -		}
> > +	imgu->statPool.createBuffers(IPU3_IMGU_BUFFER_COUNT);
> > +	ret = stat->exportBuffers(&imgu->statPool);
> > +	if (ret) {
> > +		LOG(IPU3, Error) << "Failed to reserve ImgU stat memory";
> > +		return ret;
> >  	}
> >
> > +	return 0;
> > +}
> > +
> > +int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream)
> > +{
> > +	IPU3CameraData *data = cameraData(camera);
> > +	V4L2Device *viewfinder = data->imgu->viewfinder;
> > +	V4L2Device *output = data->imgu->output;
> > +	ImgUDevice *imgu = data->imgu;
> > +	int ret;
> > +
> >  	if (isOutput(data, stream)) {
> >  		/* Export ImgU output buffers to the stream's pool. */
> >  		ret = output->exportBuffers(&stream->bufferPool());
> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> > index 1a858f2638ce..1f556ed789b6 100644
> > --- a/src/libcamera/pipeline_handler.cpp
> > +++ b/src/libcamera/pipeline_handler.cpp
> > @@ -189,6 +189,24 @@ PipelineHandler::~PipelineHandler()
> >   * \return 0 on success or a negative error code otherwise
> >   */
> >
> > +/**
> > + * \fn PipelineHandler::preAllocateBuffers()
> > + * \brief Perform operations to prepare for memory allocation. Optional for
> > + * pipeline handlers to implement
> > + * \param[in] camera The camera the streams belongs to
> > + * \param[in] activeStreams The set of active streams
> > + *
> > + * If operations to prepare for the per-stream memory allocation are required,
> > + * this virtual method provides an entry point for pipeline handlers to do so.
> > + *
> > + * This method is optional to implement for pipeline handlers, and its intended
> > + * caller is the Camera class, which calls it once before performing per-stream
> > + * memory allocation by calling the PipelineHandler::allocateBuffers() method
> > + * once per each active stream.
> > + *
> > + * \return 0 on success or a negative error code otherwise
> > + */
> > +
> >  /**
> >   * \fn PipelineHandler::allocateBuffers()
> >   * \brief Allocate buffers for a stream
> > @@ -198,7 +216,8 @@ PipelineHandler::~PipelineHandler()
> >   * This method allocates buffers internally in the pipeline handler and
> >   * associates them with the stream's buffer pool.
> >   *
> > - * The intended caller of this method is the Camera class.
> > + * The intended caller of this method is the Camera class which calls it
> > + * once per each active stream.
> >   *
> >   * \return 0 on success or a negative error code otherwise
> >   */
>
> --
> Regards,
>
> Laurent Pinchart

Patch
diff mbox series 

diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index 8ee9cc086616..8020dff8f8ea 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -455,6 +455,8 @@  int Camera::configureStreams(std::map<Stream *, StreamConfiguration> &config)
  */
 int Camera::allocateBuffers()
 {
+	int ret;
+
 	if (disconnected_)
 		return -ENODEV;
 
@@ -467,8 +469,14 @@  int Camera::allocateBuffers()
 		return -EINVAL;
 	}
 
+	ret = pipe_->preAllocateBuffers(this, activeStreams_);
+	if (ret) {
+		LOG(Camera, Error) << "Buffers pre-allocation failed";
+		return ret;
+	}
+
 	for (Stream *stream : activeStreams_) {
-		int ret = pipe_->allocateBuffers(this, stream);
+		ret = pipe_->allocateBuffers(this, stream);
 		if (ret) {
 			LOG(Camera, Error) << "Failed to allocate buffers";
 			freeBuffers();
diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
index acb376e07030..7ce5b67cc7fc 100644
--- a/src/libcamera/include/pipeline_handler.h
+++ b/src/libcamera/include/pipeline_handler.h
@@ -57,6 +57,11 @@  public:
 	virtual int configureStreams(Camera *camera,
 				     std::map<Stream *, StreamConfiguration> &config) = 0;
 
+	virtual int preAllocateBuffers(Camera *camera,
+				       const std::set<Stream *> &activeStreams)
+	{
+		return 0;
+	}
 	virtual int allocateBuffers(Camera *camera, Stream *stream) = 0;
 	virtual int freeBuffers(Camera *camera, Stream *stream) = 0;
 
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 0507fc380e68..5f323888d84f 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -105,6 +105,8 @@  public:
 	int configureStreams(Camera *camera,
 			     std::map<Stream *, StreamConfiguration> &config) override;
 
+	int preAllocateBuffers(Camera *camera,
+			       const std::set<Stream *> &activeStreams) override;
 	int allocateBuffers(Camera *camera, Stream *stream) override;
 	int freeBuffers(Camera *camera, Stream *stream) override;
 
@@ -451,40 +453,48 @@  int PipelineHandlerIPU3::configureStreams(Camera *camera,
 	return 0;
 }
 
-int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream)
+int PipelineHandlerIPU3::preAllocateBuffers(Camera *camera,
+					    const std::set<Stream *> &activeStreams)
 {
 	IPU3CameraData *data = cameraData(camera);
-	V4L2Device *viewfinder = data->imgu->viewfinder;
-	V4L2Device *output = data->imgu->output;
 	V4L2Device *input = data->imgu->input;
 	V4L2Device *cio2 = data->cio2.output;
 	V4L2Device *stat = data->imgu->stat;
 	ImgUDevice *imgu = data->imgu;
 	int ret;
 
-	if (data->cio2.pool.count() == 0) {
-		/* Share buffers between CIO2 output and ImgU input. */
-		data->cio2.pool.createBuffers(IPU3_CIO2_BUFFER_COUNT);
-		ret = cio2->exportBuffers(&data->cio2.pool);
-		if (ret) {
-			LOG(IPU3, Error) << "Failed to reserve CIO2 memory";
-			return ret;
-		}
+	/* Share buffers between CIO2 output and ImgU input. */
+	data->cio2.pool.createBuffers(IPU3_CIO2_BUFFER_COUNT);
+	ret = cio2->exportBuffers(&data->cio2.pool);
+	if (ret) {
+		LOG(IPU3, Error) << "Failed to reserve CIO2 memory";
+		return ret;
+	}
 
-		ret = input->importBuffers(&data->cio2.pool);
-		if (ret) {
-			LOG(IPU3, Error) << "Failed to import ImgU memory";
-			return ret;
-		}
+	ret = input->importBuffers(&data->cio2.pool);
+	if (ret) {
+		LOG(IPU3, Error) << "Failed to import ImgU memory";
+		return ret;
+	}
 
-		imgu->statPool.createBuffers(IPU3_IMGU_BUFFER_COUNT);
-		ret = stat->exportBuffers(&imgu->statPool);
-		if (ret) {
-			LOG(IPU3, Error) << "Failed to reserve ImgU stat memory";
-			return ret;
-		}
+	imgu->statPool.createBuffers(IPU3_IMGU_BUFFER_COUNT);
+	ret = stat->exportBuffers(&imgu->statPool);
+	if (ret) {
+		LOG(IPU3, Error) << "Failed to reserve ImgU stat memory";
+		return ret;
 	}
 
+	return 0;
+}
+
+int PipelineHandlerIPU3::allocateBuffers(Camera *camera, Stream *stream)
+{
+	IPU3CameraData *data = cameraData(camera);
+	V4L2Device *viewfinder = data->imgu->viewfinder;
+	V4L2Device *output = data->imgu->output;
+	ImgUDevice *imgu = data->imgu;
+	int ret;
+
 	if (isOutput(data, stream)) {
 		/* Export ImgU output buffers to the stream's pool. */
 		ret = output->exportBuffers(&stream->bufferPool());
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index 1a858f2638ce..1f556ed789b6 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -189,6 +189,24 @@  PipelineHandler::~PipelineHandler()
  * \return 0 on success or a negative error code otherwise
  */
 
+/**
+ * \fn PipelineHandler::preAllocateBuffers()
+ * \brief Perform operations to prepare for memory allocation. Optional for
+ * pipeline handlers to implement
+ * \param[in] camera The camera the streams belongs to
+ * \param[in] activeStreams The set of active streams
+ *
+ * If operations to prepare for the per-stream memory allocation are required,
+ * this virtual method provides an entry point for pipeline handlers to do so.
+ *
+ * This method is optional to implement for pipeline handlers, and its intended
+ * caller is the Camera class, which calls it once before performing per-stream
+ * memory allocation by calling the PipelineHandler::allocateBuffers() method
+ * once per each active stream.
+ *
+ * \return 0 on success or a negative error code otherwise
+ */
+
 /**
  * \fn PipelineHandler::allocateBuffers()
  * \brief Allocate buffers for a stream
@@ -198,7 +216,8 @@  PipelineHandler::~PipelineHandler()
  * This method allocates buffers internally in the pipeline handler and
  * associates them with the stream's buffer pool.
  *
- * The intended caller of this method is the Camera class.
+ * The intended caller of this method is the Camera class which calls it
+ * once per each active stream.
  *
  * \return 0 on success or a negative error code otherwise
  */