[libcamera-devel,07/30] ipa: Switch to FrameBuffer interface

Message ID 20191126233620.1695316-8-niklas.soderlund@ragnatech.se
State Superseded
Headers show
Series
  • libcamera: Rework buffer API
Related show

Commit Message

Niklas Söderlund Nov. 26, 2019, 11:35 p.m. UTC
Switch the IPA interfaces and implementations to use the Framebuffer
interface.

- The IPA interface is switch to use the simpler FrameBuffer::Plane
  container when carrying dmabuf descriptions (fd and length) over the
  pipeline/IPA boundary.

- The RkISP1 IPA implementation takes advantage of the new simpler and
  safer (better control over file descriptors) FrameBuffer interface.

The biggest change can be observed in the test case where it is
demonstrated that FrameBuffer objects only carry the necessary number of
planes and not a fixed number. As a consequence of this the test needs
to verify the file descriptors it injects and not depend on the library
to handle uninitialized fields.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 include/ipa/ipa_interface.h              |  2 +-
 src/ipa/libipa/ipa_interface_wrapper.cpp |  7 ++---
 src/ipa/rkisp1/rkisp1.cpp                | 14 +++++----
 src/libcamera/ipa_context_wrapper.cpp    |  8 ++---
 src/libcamera/ipa_interface.cpp          |  7 ++---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 16 ++++++++--
 test/ipa/ipa_wrappers_test.cpp           | 38 ++++++++++++------------
 7 files changed, 52 insertions(+), 40 deletions(-)

Comments

Laurent Pinchart Dec. 10, 2019, 1:59 a.m. UTC | #1
Hi Niklas,

Thank you for the patch.

On Wed, Nov 27, 2019 at 12:35:57AM +0100, Niklas Söderlund wrote:
> Switch the IPA interfaces and implementations to use the Framebuffer
> interface.
> 
> - The IPA interface is switch to use the simpler FrameBuffer::Plane
>   container when carrying dmabuf descriptions (fd and length) over the
>   pipeline/IPA boundary.
> 
> - The RkISP1 IPA implementation takes advantage of the new simpler and
>   safer (better control over file descriptors) FrameBuffer interface.
> 
> The biggest change can be observed in the test case where it is
> demonstrated that FrameBuffer objects only carry the necessary number of
> planes and not a fixed number. As a consequence of this the test needs
> to verify the file descriptors it injects and not depend on the library
> to handle uninitialized fields.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  include/ipa/ipa_interface.h              |  2 +-
>  src/ipa/libipa/ipa_interface_wrapper.cpp |  7 ++---
>  src/ipa/rkisp1/rkisp1.cpp                | 14 +++++----
>  src/libcamera/ipa_context_wrapper.cpp    |  8 ++---
>  src/libcamera/ipa_interface.cpp          |  7 ++---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 16 ++++++++--
>  test/ipa/ipa_wrappers_test.cpp           | 38 ++++++++++++------------
>  7 files changed, 52 insertions(+), 40 deletions(-)
> 
> diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h
> index 0ea53d120fe10acf..229d1124b19ec1c9 100644
> --- a/include/ipa/ipa_interface.h
> +++ b/include/ipa/ipa_interface.h
> @@ -105,7 +105,7 @@ struct IPAStream {
>  
>  struct IPABuffer {
>  	unsigned int id;
> -	BufferMemory memory;
> +	std::vector<FrameBuffer::Plane> planes;

Depending on how the discussion on FrameBuffer::Plane and Dmabuf turns
out, you may need to move FrameBuffer::Plane to the IPA interface
definition, to an IPABufferPlane class probably.

>  };
>  
>  struct IPAOperationData {
> diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp
> index 6a389dfa714ab535..a1aa88979d73446f 100644
> --- a/src/ipa/libipa/ipa_interface_wrapper.cpp
> +++ b/src/ipa/libipa/ipa_interface_wrapper.cpp
> @@ -144,15 +144,14 @@ void IPAInterfaceWrapper::map_buffers(struct ipa_context *_ctx,
>  	for (unsigned int i = 0; i < num_buffers; ++i) {
>  		const struct ipa_buffer &_buffer = _buffers[i];
>  		IPABuffer &buffer = buffers[i];
> -		std::vector<Plane> &planes = buffer.memory.planes();
> +		std::vector<FrameBuffer::Plane> &planes = buffer.planes;
>  
>  		buffer.id = _buffer.id;
>  
>  		planes.resize(_buffer.num_planes);
>  		for (unsigned int j = 0; j < _buffer.num_planes; ++j) {
> -			if (_buffer.planes[j].dmabuf != -1)
> -				planes[j].setDmabuf(_buffer.planes[j].dmabuf,
> -						    _buffer.planes[j].length);
> +			planes[j].fd = _buffer.planes[j].dmabuf;

You're assigning the numerical dmabuf fd here.

> +			planes[j].length = _buffer.planes[j].length;
>  			/** \todo Create a Dmabuf class to implement RAII. */
>  			::close(_buffer.planes[j].dmabuf);

And you then close it. Looks like a test case is missing.

We have a Dmabuf class now to address the above \todo. If you give it an
int &&fd move constructor, and if you store a Dmabuf in
FrameBuffer::Plane instead of a plain integer, I think you could get rid
of the ::close().

In any case storing a plain integer in FrameBuffer::Plane seems to be
looking for trouble.

>  		}
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 744a16ae5b9aa420..7f29d0351ba58e6d 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -48,7 +48,7 @@ private:
>  	void setControls(unsigned int frame);
>  	void metadataReady(unsigned int frame, unsigned int aeState);
>  
> -	std::map<unsigned int, BufferMemory> bufferInfo_;
> +	std::map<unsigned int, FrameBuffer *> bufferInfo_;

It's not info anymore, let's rename the variable.

Why do you store pointers instead of instances ? I fear memory leaks.

>  
>  	ControlInfoMap ctrls_;
>  
> @@ -102,15 +102,17 @@ void IPARkISP1::configure(const std::map<unsigned int, IPAStream> &streamConfig,
>  void IPARkISP1::mapBuffers(const std::vector<IPABuffer> &buffers)
>  {
>  	for (const IPABuffer &buffer : buffers) {
> -		bufferInfo_[buffer.id] = buffer.memory;
> -		bufferInfo_[buffer.id].planes()[0].mem();
> +		bufferInfo_[buffer.id] = new FrameBuffer(buffer.planes);
> +		bufferInfo_[buffer.id]->dmabufs()[0]->mem();

Interestingly enough, we're now using FrameBuffer for buffers that store
metadata. Not necessarily a big issue, but we need to explain this in
the FrameBuffer documentation. The alternative would be to change the
level of abstraction of the IPA API to handle dmabufs instead of
multi-planar buffers. I'm not sure that would be desirable though.

>  	}
>  }
>  
>  void IPARkISP1::unmapBuffers(const std::vector<unsigned int> &ids)
>  {
> -	for (unsigned int id : ids)
> +	for (unsigned int id : ids) {
> +		delete bufferInfo_[id];
>  		bufferInfo_.erase(id);
> +	}
>  }
>  
>  void IPARkISP1::processEvent(const IPAOperationData &event)
> @@ -121,7 +123,7 @@ void IPARkISP1::processEvent(const IPAOperationData &event)
>  		unsigned int bufferId = event.data[1];
>  
>  		const rkisp1_stat_buffer *stats =
> -			static_cast<rkisp1_stat_buffer *>(bufferInfo_[bufferId].planes()[0].mem());
> +			static_cast<rkisp1_stat_buffer *>(bufferInfo_[bufferId]->dmabufs()[0]->mem());
>  
>  		updateStatistics(frame, stats);
>  		break;
> @@ -131,7 +133,7 @@ void IPARkISP1::processEvent(const IPAOperationData &event)
>  		unsigned int bufferId = event.data[1];
>  
>  		rkisp1_isp_params_cfg *params =
> -			static_cast<rkisp1_isp_params_cfg *>(bufferInfo_[bufferId].planes()[0].mem());
> +			static_cast<rkisp1_isp_params_cfg *>(bufferInfo_[bufferId]->dmabufs()[0]->mem());
>  
>  		queueRequest(frame, params, event.controls[0]);
>  		break;
> diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp
> index 9603fdac87150aa0..978ead927d196f02 100644
> --- a/src/libcamera/ipa_context_wrapper.cpp
> +++ b/src/libcamera/ipa_context_wrapper.cpp
> @@ -149,15 +149,15 @@ void IPAContextWrapper::mapBuffers(const std::vector<IPABuffer> &buffers)
>  	for (unsigned int i = 0; i < buffers.size(); ++i) {
>  		struct ipa_buffer &c_buffer = c_buffers[i];
>  		const IPABuffer &buffer = buffers[i];
> -		const std::vector<Plane> &planes = buffer.memory.planes();
> +		const std::vector<FrameBuffer::Plane> &planes = buffer.planes;
>  
>  		c_buffer.id = buffer.id;
>  		c_buffer.num_planes = planes.size();
>  
>  		for (unsigned int j = 0; j < planes.size(); ++j) {
> -			const Plane &plane = planes[j];
> -			c_buffer.planes[j].dmabuf = plane.dmabuf();
> -			c_buffer.planes[j].length = plane.length();
> +			const FrameBuffer::Plane &plane = planes[j];
> +			c_buffer.planes[j].dmabuf = plane.fd;
> +			c_buffer.planes[j].length = plane.length;
>  		}
>  	}
>  
> diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp
> index ee3e3622f39ae85f..2f86087ee4aa414f 100644
> --- a/src/libcamera/ipa_interface.cpp
> +++ b/src/libcamera/ipa_interface.cpp
> @@ -334,11 +334,10 @@ namespace libcamera {
>   */
>  
>  /**
> - * \var IPABuffer::memory
> - * \brief The buffer memory description
> + * \var IPABuffer::planes
> + * \brief The buffer planes description
>   *
> - * The memory field stores the dmabuf handle and size for each plane of the
> - * buffer.
> + * Stores the dmabuf handle and length for each plane of the buffer.
>   */
>  
>  /**
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index f75b25c6787e40df..00fa4d4f19cb4aa4 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -687,14 +687,26 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,
>  	}
>  
>  	for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) {
> +		std::vector<FrameBuffer::Plane> planes;
> +		planes.push_back({
> +			.fd = paramPool_.buffers()[i].planes()[0].dmabuf(),
> +			.length = paramPool_.buffers()[i].planes()[0].length(),
> +		});
> +
>  		data->ipaBuffers_.push_back({ .id = RKISP1_PARAM_BASE | i,
> -					      .memory = paramPool_.buffers()[i] });
> +					      .planes = planes });
>  		paramBuffers_.push(new Buffer(i));
>  	}
>  
>  	for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) {
> +		std::vector<FrameBuffer::Plane> planes;
> +		planes.push_back({
> +			.fd = statPool_.buffers()[i].planes()[0].dmabuf(),
> +			.length = statPool_.buffers()[i].planes()[0].length(),
> +		});
> +
>  		data->ipaBuffers_.push_back({ .id = RKISP1_STAT_BASE | i,
> -					      .memory = statPool_.buffers()[i] });
> +					      .planes = planes });
>  		statBuffers_.push(new Buffer(i));
>  	}
>  
> diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp
> index a1e34ad52317f47b..022db05172fcd548 100644
> --- a/test/ipa/ipa_wrappers_test.cpp
> +++ b/test/ipa/ipa_wrappers_test.cpp
> @@ -115,28 +115,28 @@ public:
>  			return report(Op_mapBuffers, TestFail);
>  		}
>  
> -		if (buffers[0].memory.planes().size() != 3 ||
> -		    buffers[1].memory.planes().size() != 3) {
> +		if (buffers[0].planes.size() != 3 ||
> +		    buffers[1].planes.size() != 3) {
>  			cerr << "mapBuffers(): Invalid number of planes" << endl;
>  			return report(Op_mapBuffers, TestFail);
>  		}
>  
> -		if (buffers[0].memory.planes()[0].length() != 4096 ||
> -		    buffers[0].memory.planes()[1].length() != 0 ||
> -		    buffers[0].memory.planes()[2].length() != 0 ||
> -		    buffers[0].memory.planes()[0].length() != 4096 ||
> -		    buffers[1].memory.planes()[1].length() != 4096 ||
> -		    buffers[1].memory.planes()[2].length() != 0) {
> +		if (buffers[0].planes[0].length != 4096 ||
> +		    buffers[0].planes[1].length != 0 ||
> +		    buffers[0].planes[2].length != 0 ||
> +		    buffers[0].planes[0].length != 4096 ||
> +		    buffers[1].planes[1].length != 4096 ||
> +		    buffers[1].planes[2].length != 0) {
>  			cerr << "mapBuffers(): Invalid length" << endl;
>  			return report(Op_mapBuffers, TestFail);
>  		}
>  
> -		if (buffers[0].memory.planes()[0].dmabuf() == -1 ||
> -		    buffers[0].memory.planes()[1].dmabuf() != -1 ||
> -		    buffers[0].memory.planes()[2].dmabuf() != -1 ||
> -		    buffers[0].memory.planes()[0].dmabuf() == -1 ||
> -		    buffers[1].memory.planes()[1].dmabuf() == -1 ||
> -		    buffers[1].memory.planes()[2].dmabuf() != -1) {
> +		if (buffers[0].planes[0].fd == 0 ||
> +		    buffers[0].planes[1].fd != 0 ||
> +		    buffers[0].planes[2].fd != 0 ||
> +		    buffers[0].planes[0].fd == 0 ||
> +		    buffers[1].planes[1].fd == 0 ||
> +		    buffers[1].planes[2].fd != 0) {
>  			cerr << "mapBuffers(): Invalid dmabuf" << endl;
>  			return report(Op_mapBuffers, TestFail);
>  		}
> @@ -287,13 +287,13 @@ protected:
>  
>  		/* Test mapBuffers(). */
>  		std::vector<IPABuffer> buffers(2);
> -		buffers[0].memory.planes().resize(3);
> +		buffers[0].planes.resize(3);
>  		buffers[0].id = 10;
> -		buffers[0].memory.planes()[0].setDmabuf(fd_, 4096);
> +		buffers[0].planes[0] = { .fd = fd_, .length = 4096 };
>  		buffers[1].id = 11;
> -		buffers[1].memory.planes().resize(3);
> -		buffers[1].memory.planes()[0].setDmabuf(fd_, 4096);
> -		buffers[1].memory.planes()[1].setDmabuf(fd_, 4096);
> +		buffers[1].planes.resize(3);
> +		buffers[1].planes[0] = { .fd = fd_, .length = 4096 };
> +		buffers[1].planes[1] = { .fd = fd_, .length = 4096 };
>  
>  		ret = INVOKE(mapBuffers, buffers);
>  		if (ret == TestFail)

Patch

diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h
index 0ea53d120fe10acf..229d1124b19ec1c9 100644
--- a/include/ipa/ipa_interface.h
+++ b/include/ipa/ipa_interface.h
@@ -105,7 +105,7 @@  struct IPAStream {
 
 struct IPABuffer {
 	unsigned int id;
-	BufferMemory memory;
+	std::vector<FrameBuffer::Plane> planes;
 };
 
 struct IPAOperationData {
diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp
index 6a389dfa714ab535..a1aa88979d73446f 100644
--- a/src/ipa/libipa/ipa_interface_wrapper.cpp
+++ b/src/ipa/libipa/ipa_interface_wrapper.cpp
@@ -144,15 +144,14 @@  void IPAInterfaceWrapper::map_buffers(struct ipa_context *_ctx,
 	for (unsigned int i = 0; i < num_buffers; ++i) {
 		const struct ipa_buffer &_buffer = _buffers[i];
 		IPABuffer &buffer = buffers[i];
-		std::vector<Plane> &planes = buffer.memory.planes();
+		std::vector<FrameBuffer::Plane> &planes = buffer.planes;
 
 		buffer.id = _buffer.id;
 
 		planes.resize(_buffer.num_planes);
 		for (unsigned int j = 0; j < _buffer.num_planes; ++j) {
-			if (_buffer.planes[j].dmabuf != -1)
-				planes[j].setDmabuf(_buffer.planes[j].dmabuf,
-						    _buffer.planes[j].length);
+			planes[j].fd = _buffer.planes[j].dmabuf;
+			planes[j].length = _buffer.planes[j].length;
 			/** \todo Create a Dmabuf class to implement RAII. */
 			::close(_buffer.planes[j].dmabuf);
 		}
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index 744a16ae5b9aa420..7f29d0351ba58e6d 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -48,7 +48,7 @@  private:
 	void setControls(unsigned int frame);
 	void metadataReady(unsigned int frame, unsigned int aeState);
 
-	std::map<unsigned int, BufferMemory> bufferInfo_;
+	std::map<unsigned int, FrameBuffer *> bufferInfo_;
 
 	ControlInfoMap ctrls_;
 
@@ -102,15 +102,17 @@  void IPARkISP1::configure(const std::map<unsigned int, IPAStream> &streamConfig,
 void IPARkISP1::mapBuffers(const std::vector<IPABuffer> &buffers)
 {
 	for (const IPABuffer &buffer : buffers) {
-		bufferInfo_[buffer.id] = buffer.memory;
-		bufferInfo_[buffer.id].planes()[0].mem();
+		bufferInfo_[buffer.id] = new FrameBuffer(buffer.planes);
+		bufferInfo_[buffer.id]->dmabufs()[0]->mem();
 	}
 }
 
 void IPARkISP1::unmapBuffers(const std::vector<unsigned int> &ids)
 {
-	for (unsigned int id : ids)
+	for (unsigned int id : ids) {
+		delete bufferInfo_[id];
 		bufferInfo_.erase(id);
+	}
 }
 
 void IPARkISP1::processEvent(const IPAOperationData &event)
@@ -121,7 +123,7 @@  void IPARkISP1::processEvent(const IPAOperationData &event)
 		unsigned int bufferId = event.data[1];
 
 		const rkisp1_stat_buffer *stats =
-			static_cast<rkisp1_stat_buffer *>(bufferInfo_[bufferId].planes()[0].mem());
+			static_cast<rkisp1_stat_buffer *>(bufferInfo_[bufferId]->dmabufs()[0]->mem());
 
 		updateStatistics(frame, stats);
 		break;
@@ -131,7 +133,7 @@  void IPARkISP1::processEvent(const IPAOperationData &event)
 		unsigned int bufferId = event.data[1];
 
 		rkisp1_isp_params_cfg *params =
-			static_cast<rkisp1_isp_params_cfg *>(bufferInfo_[bufferId].planes()[0].mem());
+			static_cast<rkisp1_isp_params_cfg *>(bufferInfo_[bufferId]->dmabufs()[0]->mem());
 
 		queueRequest(frame, params, event.controls[0]);
 		break;
diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp
index 9603fdac87150aa0..978ead927d196f02 100644
--- a/src/libcamera/ipa_context_wrapper.cpp
+++ b/src/libcamera/ipa_context_wrapper.cpp
@@ -149,15 +149,15 @@  void IPAContextWrapper::mapBuffers(const std::vector<IPABuffer> &buffers)
 	for (unsigned int i = 0; i < buffers.size(); ++i) {
 		struct ipa_buffer &c_buffer = c_buffers[i];
 		const IPABuffer &buffer = buffers[i];
-		const std::vector<Plane> &planes = buffer.memory.planes();
+		const std::vector<FrameBuffer::Plane> &planes = buffer.planes;
 
 		c_buffer.id = buffer.id;
 		c_buffer.num_planes = planes.size();
 
 		for (unsigned int j = 0; j < planes.size(); ++j) {
-			const Plane &plane = planes[j];
-			c_buffer.planes[j].dmabuf = plane.dmabuf();
-			c_buffer.planes[j].length = plane.length();
+			const FrameBuffer::Plane &plane = planes[j];
+			c_buffer.planes[j].dmabuf = plane.fd;
+			c_buffer.planes[j].length = plane.length;
 		}
 	}
 
diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp
index ee3e3622f39ae85f..2f86087ee4aa414f 100644
--- a/src/libcamera/ipa_interface.cpp
+++ b/src/libcamera/ipa_interface.cpp
@@ -334,11 +334,10 @@  namespace libcamera {
  */
 
 /**
- * \var IPABuffer::memory
- * \brief The buffer memory description
+ * \var IPABuffer::planes
+ * \brief The buffer planes description
  *
- * The memory field stores the dmabuf handle and size for each plane of the
- * buffer.
+ * Stores the dmabuf handle and length for each plane of the buffer.
  */
 
 /**
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index f75b25c6787e40df..00fa4d4f19cb4aa4 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -687,14 +687,26 @@  int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,
 	}
 
 	for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) {
+		std::vector<FrameBuffer::Plane> planes;
+		planes.push_back({
+			.fd = paramPool_.buffers()[i].planes()[0].dmabuf(),
+			.length = paramPool_.buffers()[i].planes()[0].length(),
+		});
+
 		data->ipaBuffers_.push_back({ .id = RKISP1_PARAM_BASE | i,
-					      .memory = paramPool_.buffers()[i] });
+					      .planes = planes });
 		paramBuffers_.push(new Buffer(i));
 	}
 
 	for (unsigned int i = 0; i < stream->configuration().bufferCount + 1; i++) {
+		std::vector<FrameBuffer::Plane> planes;
+		planes.push_back({
+			.fd = statPool_.buffers()[i].planes()[0].dmabuf(),
+			.length = statPool_.buffers()[i].planes()[0].length(),
+		});
+
 		data->ipaBuffers_.push_back({ .id = RKISP1_STAT_BASE | i,
-					      .memory = statPool_.buffers()[i] });
+					      .planes = planes });
 		statBuffers_.push(new Buffer(i));
 	}
 
diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp
index a1e34ad52317f47b..022db05172fcd548 100644
--- a/test/ipa/ipa_wrappers_test.cpp
+++ b/test/ipa/ipa_wrappers_test.cpp
@@ -115,28 +115,28 @@  public:
 			return report(Op_mapBuffers, TestFail);
 		}
 
-		if (buffers[0].memory.planes().size() != 3 ||
-		    buffers[1].memory.planes().size() != 3) {
+		if (buffers[0].planes.size() != 3 ||
+		    buffers[1].planes.size() != 3) {
 			cerr << "mapBuffers(): Invalid number of planes" << endl;
 			return report(Op_mapBuffers, TestFail);
 		}
 
-		if (buffers[0].memory.planes()[0].length() != 4096 ||
-		    buffers[0].memory.planes()[1].length() != 0 ||
-		    buffers[0].memory.planes()[2].length() != 0 ||
-		    buffers[0].memory.planes()[0].length() != 4096 ||
-		    buffers[1].memory.planes()[1].length() != 4096 ||
-		    buffers[1].memory.planes()[2].length() != 0) {
+		if (buffers[0].planes[0].length != 4096 ||
+		    buffers[0].planes[1].length != 0 ||
+		    buffers[0].planes[2].length != 0 ||
+		    buffers[0].planes[0].length != 4096 ||
+		    buffers[1].planes[1].length != 4096 ||
+		    buffers[1].planes[2].length != 0) {
 			cerr << "mapBuffers(): Invalid length" << endl;
 			return report(Op_mapBuffers, TestFail);
 		}
 
-		if (buffers[0].memory.planes()[0].dmabuf() == -1 ||
-		    buffers[0].memory.planes()[1].dmabuf() != -1 ||
-		    buffers[0].memory.planes()[2].dmabuf() != -1 ||
-		    buffers[0].memory.planes()[0].dmabuf() == -1 ||
-		    buffers[1].memory.planes()[1].dmabuf() == -1 ||
-		    buffers[1].memory.planes()[2].dmabuf() != -1) {
+		if (buffers[0].planes[0].fd == 0 ||
+		    buffers[0].planes[1].fd != 0 ||
+		    buffers[0].planes[2].fd != 0 ||
+		    buffers[0].planes[0].fd == 0 ||
+		    buffers[1].planes[1].fd == 0 ||
+		    buffers[1].planes[2].fd != 0) {
 			cerr << "mapBuffers(): Invalid dmabuf" << endl;
 			return report(Op_mapBuffers, TestFail);
 		}
@@ -287,13 +287,13 @@  protected:
 
 		/* Test mapBuffers(). */
 		std::vector<IPABuffer> buffers(2);
-		buffers[0].memory.planes().resize(3);
+		buffers[0].planes.resize(3);
 		buffers[0].id = 10;
-		buffers[0].memory.planes()[0].setDmabuf(fd_, 4096);
+		buffers[0].planes[0] = { .fd = fd_, .length = 4096 };
 		buffers[1].id = 11;
-		buffers[1].memory.planes().resize(3);
-		buffers[1].memory.planes()[0].setDmabuf(fd_, 4096);
-		buffers[1].memory.planes()[1].setDmabuf(fd_, 4096);
+		buffers[1].planes.resize(3);
+		buffers[1].planes[0] = { .fd = fd_, .length = 4096 };
+		buffers[1].planes[1] = { .fd = fd_, .length = 4096 };
 
 		ret = INVOKE(mapBuffers, buffers);
 		if (ret == TestFail)