[libcamera-devel,v3,24/33] libcamera: pipeline: Add FrameBuffer handlers

2571 diff mbox series

Extend the pipeline handlers to support the FrameBuffer API with three
new methods to handle allocation, importing and freeing of buffers. The
new methods will replace allocateBuffers() and freeBuffers().

The FrameBuffer API will use the methods on a stream level and either
allocate or import buffers for each active stream controlled from the
Camera class and an upcoming FrameBufferAllocator helper. With this new
API the implementation in pipeline handlers can be made simpler as all
streams don't need to be handled in allocateBuffers().

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
* Changes since v2
- In the subject line, s/pipelines/pipeline/
- Rename allocateFrameBuffers() to exportFrameBuffers()
- Rewrite of documentation, thanks Laurent!
- Remove count argument from exportFrameBuffers() and
  importFrameBuffers() and instead get the buffer count from the stream
  configuration.
---
 src/libcamera/include/pipeline_handler.h |  5 ++
 src/libcamera/pipeline/ipu3/ipu3.cpp     | 32 +++++++++++
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 23 ++++++++
 src/libcamera/pipeline/uvcvideo.cpp      | 29 ++++++++++
 src/libcamera/pipeline/vimc.cpp          | 29 ++++++++++
 src/libcamera/pipeline_handler.cpp       | 67 ++++++++++++++++++++++++
 6 files changed, 185 insertions(+)

Hi Niklas,

Thank you for the patch.

On Fri, Jan 10, 2020 at 08:37:59PM +0100, Niklas Söderlund wrote:
> Extend the pipeline handlers to support the FrameBuffer API with three
> new methods to handle allocation, importing and freeing of buffers. The
> new methods will replace allocateBuffers() and freeBuffers().
> 
> The FrameBuffer API will use the methods on a stream level and either
> allocate or import buffers for each active stream controlled from the
> Camera class and an upcoming FrameBufferAllocator helper. With this new
> API the implementation in pipeline handlers can be made simpler as all
> streams don't need to be handled in allocateBuffers().
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> * Changes since v2
> - In the subject line, s/pipelines/pipeline/
> - Rename allocateFrameBuffers() to exportFrameBuffers()
> - Rewrite of documentation, thanks Laurent!
> - Remove count argument from exportFrameBuffers() and
>   importFrameBuffers() and instead get the buffer count from the stream
>   configuration.
> ---
>  src/libcamera/include/pipeline_handler.h |  5 ++
>  src/libcamera/pipeline/ipu3/ipu3.cpp     | 32 +++++++++++
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 23 ++++++++
>  src/libcamera/pipeline/uvcvideo.cpp      | 29 ++++++++++
>  src/libcamera/pipeline/vimc.cpp          | 29 ++++++++++
>  src/libcamera/pipeline_handler.cpp       | 67 ++++++++++++++++++++++++
>  6 files changed, 185 insertions(+)
> 
> diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> index 067baef56278b577..560be3bad8d59b20 100644
> --- a/src/libcamera/include/pipeline_handler.h
> +++ b/src/libcamera/include/pipeline_handler.h
> @@ -70,6 +70,11 @@ public:
>  		const StreamRoles &roles) = 0;
>  	virtual int configure(Camera *camera, CameraConfiguration *config) = 0;
>  
> +	virtual int exportFrameBuffers(Camera *camera, Stream *stream,
> +				       std::vector<std::unique_ptr<FrameBuffer>> *buffers) = 0;
> +	virtual int importFrameBuffers(Camera *camera, Stream *stream) = 0;
> +	virtual void freeFrameBuffers(Camera *camera, Stream *stream) = 0;
> +
>  	virtual int allocateBuffers(Camera *camera,
>  				    const std::set<Stream *> &streams) = 0;
>  	virtual int freeBuffers(Camera *camera,
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 7c4539d488bd2d26..b73f0f0725ee2613 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -210,6 +210,11 @@ public:
>  		const StreamRoles &roles) override;
>  	int configure(Camera *camera, CameraConfiguration *config) override;
>  
> +	int exportFrameBuffers(Camera *camera, Stream *stream,
> +			       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> +	int importFrameBuffers(Camera *camera, Stream *stream) override;
> +	void freeFrameBuffers(Camera *camera, Stream *stream) override;
> +
>  	int allocateBuffers(Camera *camera,
>  			    const std::set<Stream *> &streams) override;
>  	int freeBuffers(Camera *camera,
> @@ -616,6 +621,33 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
>  	return 0;
>  }
>  
> +int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,
> +					    std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +{
> +	IPU3Stream *ipu3stream = static_cast<IPU3Stream *>(stream);
> +	V4L2VideoDevice *video = ipu3stream->device_->dev;
> +	unsigned int count = stream->configuration().bufferCount;
> +
> +	return video->exportBuffers(count, buffers);
> +}
> +
> +int PipelineHandlerIPU3::importFrameBuffers(Camera *camera, Stream *stream)
> +{
> +	IPU3Stream *ipu3stream = static_cast<IPU3Stream *>(stream);
> +	V4L2VideoDevice *video = ipu3stream->device_->dev;
> +	unsigned int count = stream->configuration().bufferCount;
> +
> +	return video->importBuffers(count);
> +}
> +
> +void PipelineHandlerIPU3::freeFrameBuffers(Camera *camera, Stream *stream)
> +{
> +	IPU3Stream *ipu3stream = static_cast<IPU3Stream *>(stream);
> +	V4L2VideoDevice *video = ipu3stream->device_->dev;
> +
> +	video->releaseBuffers();
> +}
> +
>  /**
>   * \todo Clarify if 'viewfinder' and 'stat' nodes have to be set up and
>   * started even if not in use. As of now, if not properly configured and
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index dbc5df801f30e76b..ba8f93e8584c97a9 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -174,6 +174,11 @@ public:
>  		const StreamRoles &roles) override;
>  	int configure(Camera *camera, CameraConfiguration *config) override;
>  
> +	int exportFrameBuffers(Camera *camera, Stream *stream,
> +			       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> +	int importFrameBuffers(Camera *camera, Stream *stream) override;
> +	void freeFrameBuffers(Camera *camera, Stream *stream) override;
> +
>  	int allocateBuffers(Camera *camera,
>  		const std::set<Stream *> &streams) override;
>  	int freeBuffers(Camera *camera,
> @@ -665,6 +670,24 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  	return 0;
>  }
>  
> +int PipelineHandlerRkISP1::exportFrameBuffers(Camera *camera, Stream *stream,
> +					      std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +{
> +	unsigned int count = stream->configuration().bufferCount;
> +	return video_->exportBuffers(count, buffers);
> +}
> +
> +int PipelineHandlerRkISP1::importFrameBuffers(Camera *camera, Stream *stream)
> +{
> +	unsigned int count = stream->configuration().bufferCount;
> +	return video_->importBuffers(count);
> +}
> +
> +void PipelineHandlerRkISP1::freeFrameBuffers(Camera *camera, Stream *stream)
> +{
> +	video_->releaseBuffers();
> +}
> +
>  int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,
>  					   const std::set<Stream *> &streams)
>  {
> diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> index ce38445d2a48ca82..b72841edee572f99 100644
> --- a/src/libcamera/pipeline/uvcvideo.cpp
> +++ b/src/libcamera/pipeline/uvcvideo.cpp
> @@ -65,6 +65,11 @@ public:
>  		const StreamRoles &roles) override;
>  	int configure(Camera *camera, CameraConfiguration *config) override;
>  
> +	int exportFrameBuffers(Camera *camera, Stream *stream,
> +			       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> +	int importFrameBuffers(Camera *camera, Stream *stream) override;
> +	void freeFrameBuffers(Camera *camera, Stream *stream) override;
> +
>  	int allocateBuffers(Camera *camera,
>  			    const std::set<Stream *> &streams) override;
>  	int freeBuffers(Camera *camera,
> @@ -193,6 +198,30 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)
>  	return 0;
>  }
>  
> +int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream,
> +					   std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +{
> +	UVCCameraData *data = cameraData(camera);
> +	unsigned int count = stream->configuration().bufferCount;
> +
> +	return data->video_->exportBuffers(count, buffers);
> +}
> +
> +int PipelineHandlerUVC::importFrameBuffers(Camera *camera, Stream *stream)
> +{
> +	UVCCameraData *data = cameraData(camera);
> +	unsigned int count = stream->configuration().bufferCount;
> +
> +	return data->video_->importBuffers(count);
> +}
> +
> +void PipelineHandlerUVC::freeFrameBuffers(Camera *camera, Stream *stream)
> +{
> +	UVCCameraData *data = cameraData(camera);
> +
> +	data->video_->releaseBuffers();
> +}
> +
>  int PipelineHandlerUVC::allocateBuffers(Camera *camera,
>  					const std::set<Stream *> &streams)
>  {
> diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> index 292900bcf8d1b359..3fb5cacde0dab5b6 100644
> --- a/src/libcamera/pipeline/vimc.cpp
> +++ b/src/libcamera/pipeline/vimc.cpp
> @@ -82,6 +82,11 @@ public:
>  		const StreamRoles &roles) override;
>  	int configure(Camera *camera, CameraConfiguration *config) override;
>  
> +	int exportFrameBuffers(Camera *camera, Stream *stream,
> +			       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> +	int importFrameBuffers(Camera *camera, Stream *stream) override;
> +	void freeFrameBuffers(Camera *camera, Stream *stream) override;
> +
>  	int allocateBuffers(Camera *camera,
>  			    const std::set<Stream *> &streams) override;
>  	int freeBuffers(Camera *camera,
> @@ -259,6 +264,30 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)
>  	return 0;
>  }
>  
> +int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream,
> +					    std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +{
> +	VimcCameraData *data = cameraData(camera);
> +	unsigned int count = stream->configuration().bufferCount;
> +
> +	return data->video_->exportBuffers(count, buffers);
> +}
> +
> +int PipelineHandlerVimc::importFrameBuffers(Camera *camera, Stream *stream)
> +{
> +	VimcCameraData *data = cameraData(camera);
> +	unsigned int count = stream->configuration().bufferCount;
> +
> +	return data->video_->importBuffers(count);
> +}
> +
> +void PipelineHandlerVimc::freeFrameBuffers(Camera *camera, Stream *stream)
> +{
> +	VimcCameraData *data = cameraData(camera);
> +
> +	data->video_->releaseBuffers();
> +}
> +
>  int PipelineHandlerVimc::allocateBuffers(Camera *camera,
>  					 const std::set<Stream *> &streams)
>  {
> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> index 698dd52560797d7c..572f751b5217eb5f 100644
> --- a/src/libcamera/pipeline_handler.cpp
> +++ b/src/libcamera/pipeline_handler.cpp
> @@ -289,6 +289,73 @@ const ControlInfoMap &PipelineHandler::controls(Camera *camera)
>   * \return 0 on success or a negative error code otherwise
>   */
>  
> +/**
> + * \fn PipelineHandler::exportFrameBuffers()
> + * \brief Allocate buffers for \a stream
> + * \param[in] camera The camera
> + * \param[in] stream The stream to allocate buffers for
> + * \param[out] buffers Array of buffers successfully allocated
> + *
> + * This method allocates buffers for the \a stream from the devices associated
> + * with the stream in the corresponding pipeline handler. Those buffers shall be
> + * suitable to be added to a Request for the stream, and shall be mappable to
> + * the CPU through their associated dmabufs with mmap().
> + *

Is there a reason why you haven't included the following paragraph,
proposed in the review of v2 ?

 * The method may only be called after the Camera has been configured and before
 * it gets started, or after it gets stopped. It shall be called only for
 * streams that are part of the active camera configuration, and at most once
 * per stream until buffers for the stream are freed with freeFrameBuffers().

> + * exportFrameBuffers() shall also allocate all other resources required by
> + * the pipeline handler for the stream to prepare for starting the Camera. This
> + * responsibility is shared with importFrameBuffers(), and one and only one of
> + * those two methods shall be called for each stream until the buffers are
> + * freed. The pipeline handler shall support all combinations of
> + * exportFrameBuffers() and importFrameBuffers() for the streams contained in
> + * any camera configuration.
> + *
> + * The only intended caller is the FrameBufferAllocator helper.
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +
> +/**
> + * \fn PipelineHandler::importFrameBuffers()
> + * \brief Prepare \a stream to use external buffers
> + * \param[in] camera The camera
> + * \param[in] stream The stream to prepare for import
> + *
> + * This method prepares the pipeline handler to use buffers provided by the
> + * application for the \a stream.
> + *
> + * The method may only be called after the Camera has been configured and before
> + * it gets started, or after it gets stopped. It shall be called only for
> + * streams that are part of the active camera configuration, and at most once
> + * per stream until buffers for the stream are freed with freeFrameBuffers().
> + *
> + * importFrameBuffers() shall also allocate all other resources required by the
> + * pipeline handler for the stream to prepare for starting the Camera. This
> + * responsibility is shared with exportFrameBuffers(), and one and only one of
> + * those two methods shall be called for each stream until the buffers are
> + * freed. The pipeline handler shall support all combinations of
> + * exportFrameBuffers() and importFrameBuffers() for the streams contained in
> + * any camera configuration.
> + *
> + * The only intended caller is Camera::start().
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +
> +/**
> + * \fn PipelineHandler::freeFrameBuffers()
> + * \brief Free buffers allocated from the stream
> + * \param[in] camera The camera
> + * \param[in] stream The stream to free buffers for
> + *
> + * This method shall free all buffers and all other resources allocated for the
> + * \a stream by exportFrameBuffers() or importFrameBuffers(). It shall be
> + * called only after a successful call to either of these two methods, and only
> + * once per stream.
> + *
> + * The only intended callers are Camera::stop() and the FrameBufferAllocator
> + * helper.
> + */
> +
>  /**
>   * \fn PipelineHandler::allocateBuffers()
>   * \brief Allocate buffers for a stream

Hi Laurent,

Thanks for your feedback.

On 2020-01-11 03:07:39 +0200, Laurent Pinchart wrote:
> Hi Niklas,
> 
> Thank you for the patch.
> 
> On Fri, Jan 10, 2020 at 08:37:59PM +0100, Niklas Söderlund wrote:
> > Extend the pipeline handlers to support the FrameBuffer API with three
> > new methods to handle allocation, importing and freeing of buffers. The
> > new methods will replace allocateBuffers() and freeBuffers().
> > 
> > The FrameBuffer API will use the methods on a stream level and either
> > allocate or import buffers for each active stream controlled from the
> > Camera class and an upcoming FrameBufferAllocator helper. With this new
> > API the implementation in pipeline handlers can be made simpler as all
> > streams don't need to be handled in allocateBuffers().
> > 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > * Changes since v2
> > - In the subject line, s/pipelines/pipeline/
> > - Rename allocateFrameBuffers() to exportFrameBuffers()
> > - Rewrite of documentation, thanks Laurent!
> > - Remove count argument from exportFrameBuffers() and
> >   importFrameBuffers() and instead get the buffer count from the stream
> >   configuration.
> > ---
> >  src/libcamera/include/pipeline_handler.h |  5 ++
> >  src/libcamera/pipeline/ipu3/ipu3.cpp     | 32 +++++++++++
> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 23 ++++++++
> >  src/libcamera/pipeline/uvcvideo.cpp      | 29 ++++++++++
> >  src/libcamera/pipeline/vimc.cpp          | 29 ++++++++++
> >  src/libcamera/pipeline_handler.cpp       | 67 ++++++++++++++++++++++++
> >  6 files changed, 185 insertions(+)
> > 
> > diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
> > index 067baef56278b577..560be3bad8d59b20 100644
> > --- a/src/libcamera/include/pipeline_handler.h
> > +++ b/src/libcamera/include/pipeline_handler.h
> > @@ -70,6 +70,11 @@ public:
> >  		const StreamRoles &roles) = 0;
> >  	virtual int configure(Camera *camera, CameraConfiguration *config) = 0;
> >  
> > +	virtual int exportFrameBuffers(Camera *camera, Stream *stream,
> > +				       std::vector<std::unique_ptr<FrameBuffer>> *buffers) = 0;
> > +	virtual int importFrameBuffers(Camera *camera, Stream *stream) = 0;
> > +	virtual void freeFrameBuffers(Camera *camera, Stream *stream) = 0;
> > +
> >  	virtual int allocateBuffers(Camera *camera,
> >  				    const std::set<Stream *> &streams) = 0;
> >  	virtual int freeBuffers(Camera *camera,
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index 7c4539d488bd2d26..b73f0f0725ee2613 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -210,6 +210,11 @@ public:
> >  		const StreamRoles &roles) override;
> >  	int configure(Camera *camera, CameraConfiguration *config) override;
> >  
> > +	int exportFrameBuffers(Camera *camera, Stream *stream,
> > +			       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > +	int importFrameBuffers(Camera *camera, Stream *stream) override;
> > +	void freeFrameBuffers(Camera *camera, Stream *stream) override;
> > +
> >  	int allocateBuffers(Camera *camera,
> >  			    const std::set<Stream *> &streams) override;
> >  	int freeBuffers(Camera *camera,
> > @@ -616,6 +621,33 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)
> >  	return 0;
> >  }
> >  
> > +int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,
> > +					    std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > +{
> > +	IPU3Stream *ipu3stream = static_cast<IPU3Stream *>(stream);
> > +	V4L2VideoDevice *video = ipu3stream->device_->dev;
> > +	unsigned int count = stream->configuration().bufferCount;
> > +
> > +	return video->exportBuffers(count, buffers);
> > +}
> > +
> > +int PipelineHandlerIPU3::importFrameBuffers(Camera *camera, Stream *stream)
> > +{
> > +	IPU3Stream *ipu3stream = static_cast<IPU3Stream *>(stream);
> > +	V4L2VideoDevice *video = ipu3stream->device_->dev;
> > +	unsigned int count = stream->configuration().bufferCount;
> > +
> > +	return video->importBuffers(count);
> > +}
> > +
> > +void PipelineHandlerIPU3::freeFrameBuffers(Camera *camera, Stream *stream)
> > +{
> > +	IPU3Stream *ipu3stream = static_cast<IPU3Stream *>(stream);
> > +	V4L2VideoDevice *video = ipu3stream->device_->dev;
> > +
> > +	video->releaseBuffers();
> > +}
> > +
> >  /**
> >   * \todo Clarify if 'viewfinder' and 'stat' nodes have to be set up and
> >   * started even if not in use. As of now, if not properly configured and
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index dbc5df801f30e76b..ba8f93e8584c97a9 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -174,6 +174,11 @@ public:
> >  		const StreamRoles &roles) override;
> >  	int configure(Camera *camera, CameraConfiguration *config) override;
> >  
> > +	int exportFrameBuffers(Camera *camera, Stream *stream,
> > +			       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > +	int importFrameBuffers(Camera *camera, Stream *stream) override;
> > +	void freeFrameBuffers(Camera *camera, Stream *stream) override;
> > +
> >  	int allocateBuffers(Camera *camera,
> >  		const std::set<Stream *> &streams) override;
> >  	int freeBuffers(Camera *camera,
> > @@ -665,6 +670,24 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> >  	return 0;
> >  }
> >  
> > +int PipelineHandlerRkISP1::exportFrameBuffers(Camera *camera, Stream *stream,
> > +					      std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > +{
> > +	unsigned int count = stream->configuration().bufferCount;
> > +	return video_->exportBuffers(count, buffers);
> > +}
> > +
> > +int PipelineHandlerRkISP1::importFrameBuffers(Camera *camera, Stream *stream)
> > +{
> > +	unsigned int count = stream->configuration().bufferCount;
> > +	return video_->importBuffers(count);
> > +}
> > +
> > +void PipelineHandlerRkISP1::freeFrameBuffers(Camera *camera, Stream *stream)
> > +{
> > +	video_->releaseBuffers();
> > +}
> > +
> >  int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,
> >  					   const std::set<Stream *> &streams)
> >  {
> > diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
> > index ce38445d2a48ca82..b72841edee572f99 100644
> > --- a/src/libcamera/pipeline/uvcvideo.cpp
> > +++ b/src/libcamera/pipeline/uvcvideo.cpp
> > @@ -65,6 +65,11 @@ public:
> >  		const StreamRoles &roles) override;
> >  	int configure(Camera *camera, CameraConfiguration *config) override;
> >  
> > +	int exportFrameBuffers(Camera *camera, Stream *stream,
> > +			       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > +	int importFrameBuffers(Camera *camera, Stream *stream) override;
> > +	void freeFrameBuffers(Camera *camera, Stream *stream) override;
> > +
> >  	int allocateBuffers(Camera *camera,
> >  			    const std::set<Stream *> &streams) override;
> >  	int freeBuffers(Camera *camera,
> > @@ -193,6 +198,30 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)
> >  	return 0;
> >  }
> >  
> > +int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream,
> > +					   std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > +{
> > +	UVCCameraData *data = cameraData(camera);
> > +	unsigned int count = stream->configuration().bufferCount;
> > +
> > +	return data->video_->exportBuffers(count, buffers);
> > +}
> > +
> > +int PipelineHandlerUVC::importFrameBuffers(Camera *camera, Stream *stream)
> > +{
> > +	UVCCameraData *data = cameraData(camera);
> > +	unsigned int count = stream->configuration().bufferCount;
> > +
> > +	return data->video_->importBuffers(count);
> > +}
> > +
> > +void PipelineHandlerUVC::freeFrameBuffers(Camera *camera, Stream *stream)
> > +{
> > +	UVCCameraData *data = cameraData(camera);
> > +
> > +	data->video_->releaseBuffers();
> > +}
> > +
> >  int PipelineHandlerUVC::allocateBuffers(Camera *camera,
> >  					const std::set<Stream *> &streams)
> >  {
> > diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
> > index 292900bcf8d1b359..3fb5cacde0dab5b6 100644
> > --- a/src/libcamera/pipeline/vimc.cpp
> > +++ b/src/libcamera/pipeline/vimc.cpp
> > @@ -82,6 +82,11 @@ public:
> >  		const StreamRoles &roles) override;
> >  	int configure(Camera *camera, CameraConfiguration *config) override;
> >  
> > +	int exportFrameBuffers(Camera *camera, Stream *stream,
> > +			       std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> > +	int importFrameBuffers(Camera *camera, Stream *stream) override;
> > +	void freeFrameBuffers(Camera *camera, Stream *stream) override;
> > +
> >  	int allocateBuffers(Camera *camera,
> >  			    const std::set<Stream *> &streams) override;
> >  	int freeBuffers(Camera *camera,
> > @@ -259,6 +264,30 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)
> >  	return 0;
> >  }
> >  
> > +int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream,
> > +					    std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > +{
> > +	VimcCameraData *data = cameraData(camera);
> > +	unsigned int count = stream->configuration().bufferCount;
> > +
> > +	return data->video_->exportBuffers(count, buffers);
> > +}
> > +
> > +int PipelineHandlerVimc::importFrameBuffers(Camera *camera, Stream *stream)
> > +{
> > +	VimcCameraData *data = cameraData(camera);
> > +	unsigned int count = stream->configuration().bufferCount;
> > +
> > +	return data->video_->importBuffers(count);
> > +}
> > +
> > +void PipelineHandlerVimc::freeFrameBuffers(Camera *camera, Stream *stream)
> > +{
> > +	VimcCameraData *data = cameraData(camera);
> > +
> > +	data->video_->releaseBuffers();
> > +}
> > +
> >  int PipelineHandlerVimc::allocateBuffers(Camera *camera,
> >  					 const std::set<Stream *> &streams)
> >  {
> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
> > index 698dd52560797d7c..572f751b5217eb5f 100644
> > --- a/src/libcamera/pipeline_handler.cpp
> > +++ b/src/libcamera/pipeline_handler.cpp
> > @@ -289,6 +289,73 @@ const ControlInfoMap &PipelineHandler::controls(Camera *camera)
> >   * \return 0 on success or a negative error code otherwise
> >   */
> >  
> > +/**
> > + * \fn PipelineHandler::exportFrameBuffers()
> > + * \brief Allocate buffers for \a stream
> > + * \param[in] camera The camera
> > + * \param[in] stream The stream to allocate buffers for
> > + * \param[out] buffers Array of buffers successfully allocated
> > + *
> > + * This method allocates buffers for the \a stream from the devices associated
> > + * with the stream in the corresponding pipeline handler. Those buffers shall be
> > + * suitable to be added to a Request for the stream, and shall be mappable to
> > + * the CPU through their associated dmabufs with mmap().
> > + *
> 
> Is there a reason why you haven't included the following paragraph,
> proposed in the review of v2 ?

I must have missed it :-( Sorry about that, will add for v4.

> 
>  * The method may only be called after the Camera has been configured and before
>  * it gets started, or after it gets stopped. It shall be called only for
>  * streams that are part of the active camera configuration, and at most once
>  * per stream until buffers for the stream are freed with freeFrameBuffers().
> 
> > + * exportFrameBuffers() shall also allocate all other resources required by
> > + * the pipeline handler for the stream to prepare for starting the Camera. This
> > + * responsibility is shared with importFrameBuffers(), and one and only one of
> > + * those two methods shall be called for each stream until the buffers are
> > + * freed. The pipeline handler shall support all combinations of
> > + * exportFrameBuffers() and importFrameBuffers() for the streams contained in
> > + * any camera configuration.
> > + *
> > + * The only intended caller is the FrameBufferAllocator helper.
> > + *
> > + * \return 0 on success or a negative error code otherwise
> > + */
> > +
> > +/**
> > + * \fn PipelineHandler::importFrameBuffers()
> > + * \brief Prepare \a stream to use external buffers
> > + * \param[in] camera The camera
> > + * \param[in] stream The stream to prepare for import
> > + *
> > + * This method prepares the pipeline handler to use buffers provided by the
> > + * application for the \a stream.
> > + *
> > + * The method may only be called after the Camera has been configured and before
> > + * it gets started, or after it gets stopped. It shall be called only for
> > + * streams that are part of the active camera configuration, and at most once
> > + * per stream until buffers for the stream are freed with freeFrameBuffers().
> > + *
> > + * importFrameBuffers() shall also allocate all other resources required by the
> > + * pipeline handler for the stream to prepare for starting the Camera. This
> > + * responsibility is shared with exportFrameBuffers(), and one and only one of
> > + * those two methods shall be called for each stream until the buffers are
> > + * freed. The pipeline handler shall support all combinations of
> > + * exportFrameBuffers() and importFrameBuffers() for the streams contained in
> > + * any camera configuration.
> > + *
> > + * The only intended caller is Camera::start().
> > + *
> > + * \return 0 on success or a negative error code otherwise
> > + */
> > +
> > +/**
> > + * \fn PipelineHandler::freeFrameBuffers()
> > + * \brief Free buffers allocated from the stream
> > + * \param[in] camera The camera
> > + * \param[in] stream The stream to free buffers for
> > + *
> > + * This method shall free all buffers and all other resources allocated for the
> > + * \a stream by exportFrameBuffers() or importFrameBuffers(). It shall be
> > + * called only after a successful call to either of these two methods, and only
> > + * once per stream.
> > + *
> > + * The only intended callers are Camera::stop() and the FrameBufferAllocator
> > + * helper.
> > + */
> > +
> >  /**
> >   * \fn PipelineHandler::allocateBuffers()
> >   * \brief Allocate buffers for a stream
> 
> -- 
> Regards,
> 
> Laurent Pinchart