[libcamera-devel,2/5] libcamera: v4l2_device: importBuffers support
diff mbox series

Message ID 20190207212119.30299-3-kieran.bingham@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: Buffer Sharing
Related show

Commit Message

Kieran Bingham Feb. 7, 2019, 9:21 p.m. UTC 
Provide the ability to import a BufferPool into the V4L2Device allowing
external dmabuf backed buffers to be queued.

Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/libcamera/include/v4l2_device.h |  1 +
 src/libcamera/v4l2_device.cpp       | 45 ++++++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 1 deletion(-)

Comments

Jacopo Mondi Feb. 7, 2019, 10:16 p.m. UTC | #1 
Hi Kieran,

On Thu, Feb 07, 2019 at 09:21:16PM +0000, Kieran Bingham wrote:
> Provide the ability to import a BufferPool into the V4L2Device allowing
> external dmabuf backed buffers to be queued.
>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/libcamera/include/v4l2_device.h |  1 +
>  src/libcamera/v4l2_device.cpp       | 45 ++++++++++++++++++++++++++++-
>  2 files changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
> index 988e646c5de1..3acb5e466d64 100644
> --- a/src/libcamera/include/v4l2_device.h
> +++ b/src/libcamera/include/v4l2_device.h
> @@ -98,6 +98,7 @@ public:
>  	int setFormat(V4L2DeviceFormat *format);
>
>  	int exportBuffers(unsigned int count, BufferPool *pool);
> +	int importBuffers(BufferPool *pool);
>  	int releaseBuffers();
>
>  	int queueBuffer(Buffer *buffer);
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 64325ff9f5d9..c654e6dd7b8d 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -633,6 +633,36 @@ int V4L2Device::createPlane(Buffer *buffer, unsigned int planeIndex,
>  	return 0;
>  }
>
> +/**
> + * \brief Import the externally allocated \a pool of buffers

"an externally allocated" maybe?

> + * \param[in] pool BufferPool of buffers to import
> + * \return 0 on success or a negative error code otherwise

"A pool of buffer to import" ?

> + */
> +int V4L2Device::importBuffers(BufferPool *pool)
> +{
> +	unsigned int allocatedBuffers;
> +	int ret;
> +
> +	memoryType_ = V4L2_MEMORY_DMABUF;
> +
> +	ret = requestBuffers(pool->count());
> +	if (ret < 0)
> +		return ret;
> +
> +	allocatedBuffers = ret;

What is allocatedBuffers used for?
Could you drop it and compare with ret here below?

> +	if (allocatedBuffers < pool->count()) {
> +		LOG(V4L2, Error)
> +			<< "Not enough buffers provided by V4L2Device";
> +		requestBuffers(0);
> +		return -ENOMEM;
> +	}
> +
> +	LOG(V4L2, Debug) << "Device using an externally provided pool";
> +	bufferPool_ = pool;
> +
> +	return 0;
> +}
> +
>  /**
>   * \brief Release all internally allocated buffers
>   */
> @@ -670,7 +700,20 @@ int V4L2Device::queueBuffer(Buffer *buffer)
>  	buf.type = bufferType_;
>  	buf.memory = memoryType_;
>
> -	if (V4L2_TYPE_IS_MULTIPLANAR(buf.type)) {
> +	bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type);
> +
> +	if (buf.memory == V4L2_MEMORY_DMABUF) {
> +		if (multiPlanar) {
> +			for (unsigned int p = 0;
> +			     p < buffer->planes().size();
> +			     p++)
> +				planes[p].m.fd = buffer->planes()[p].dmabuf();
> +		} else {
> +			buf.m.fd = buffer->planes()[0].dmabuf();
> +		}
> +	}
> +
> +	if (multiPlanar) {

This change looks good to me!

Thanks
   j

>  		buf.length = buffer->planes().size();
>  		buf.m.planes = planes;
>  	}
> --
> 2.19.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Feb. 8, 2019, 3:50 p.m. UTC | #2 
Hi Kieran,

Thank you for the patch.

On Thu, Feb 07, 2019 at 09:21:16PM +0000, Kieran Bingham wrote:
> Provide the ability to import a BufferPool into the V4L2Device allowing
> external dmabuf backed buffers to be queued.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/libcamera/include/v4l2_device.h |  1 +
>  src/libcamera/v4l2_device.cpp       | 45 ++++++++++++++++++++++++++++-
>  2 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
> index 988e646c5de1..3acb5e466d64 100644
> --- a/src/libcamera/include/v4l2_device.h
> +++ b/src/libcamera/include/v4l2_device.h
> @@ -98,6 +98,7 @@ public:
>  	int setFormat(V4L2DeviceFormat *format);
>  
>  	int exportBuffers(unsigned int count, BufferPool *pool);
> +	int importBuffers(BufferPool *pool);
>  	int releaseBuffers();
>  
>  	int queueBuffer(Buffer *buffer);
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 64325ff9f5d9..c654e6dd7b8d 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -633,6 +633,36 @@ int V4L2Device::createPlane(Buffer *buffer, unsigned int planeIndex,
>  	return 0;
>  }
>  
> +/**
> + * \brief Import the externally allocated \a pool of buffers
> + * \param[in] pool BufferPool of buffers to import
> + * \return 0 on success or a negative error code otherwise
> + */
> +int V4L2Device::importBuffers(BufferPool *pool)
> +{
> +	unsigned int allocatedBuffers;
> +	int ret;
> +
> +	memoryType_ = V4L2_MEMORY_DMABUF;
> +
> +	ret = requestBuffers(pool->count());
> +	if (ret < 0)
> +		return ret;
> +
> +	allocatedBuffers = ret;
> +	if (allocatedBuffers < pool->count()) {
> +		LOG(V4L2, Error)
> +			<< "Not enough buffers provided by V4L2Device";
> +		requestBuffers(0);
> +		return -ENOMEM;
> +	}
> +

Should we use VIDIOC_PREPARE_BUF to already associate the dmabuf fds
with V4L2 buffers ?

> +	LOG(V4L2, Debug) << "Device using an externally provided pool";
> +	bufferPool_ = pool;
> +
> +	return 0;
> +}
> +
>  /**
>   * \brief Release all internally allocated buffers
>   */
> @@ -670,7 +700,20 @@ int V4L2Device::queueBuffer(Buffer *buffer)
>  	buf.type = bufferType_;
>  	buf.memory = memoryType_;
>  
> -	if (V4L2_TYPE_IS_MULTIPLANAR(buf.type)) {
> +	bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type);
> +
> +	if (buf.memory == V4L2_MEMORY_DMABUF) {
> +		if (multiPlanar) {
> +			for (unsigned int p = 0;
> +			     p < buffer->planes().size();
> +			     p++)
> +				planes[p].m.fd = buffer->planes()[p].dmabuf();
> +		} else {
> +			buf.m.fd = buffer->planes()[0].dmabuf();
> +		}
> +	}
> +
> +	if (multiPlanar) {
>  		buf.length = buffer->planes().size();
>  		buf.m.planes = planes;
>  	}

Patch
diff mbox series 

diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h
index 988e646c5de1..3acb5e466d64 100644
--- a/src/libcamera/include/v4l2_device.h
+++ b/src/libcamera/include/v4l2_device.h
@@ -98,6 +98,7 @@  public:
 	int setFormat(V4L2DeviceFormat *format);
 
 	int exportBuffers(unsigned int count, BufferPool *pool);
+	int importBuffers(BufferPool *pool);
 	int releaseBuffers();
 
 	int queueBuffer(Buffer *buffer);
diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
index 64325ff9f5d9..c654e6dd7b8d 100644
--- a/src/libcamera/v4l2_device.cpp
+++ b/src/libcamera/v4l2_device.cpp
@@ -633,6 +633,36 @@  int V4L2Device::createPlane(Buffer *buffer, unsigned int planeIndex,
 	return 0;
 }
 
+/**
+ * \brief Import the externally allocated \a pool of buffers
+ * \param[in] pool BufferPool of buffers to import
+ * \return 0 on success or a negative error code otherwise
+ */
+int V4L2Device::importBuffers(BufferPool *pool)
+{
+	unsigned int allocatedBuffers;
+	int ret;
+
+	memoryType_ = V4L2_MEMORY_DMABUF;
+
+	ret = requestBuffers(pool->count());
+	if (ret < 0)
+		return ret;
+
+	allocatedBuffers = ret;
+	if (allocatedBuffers < pool->count()) {
+		LOG(V4L2, Error)
+			<< "Not enough buffers provided by V4L2Device";
+		requestBuffers(0);
+		return -ENOMEM;
+	}
+
+	LOG(V4L2, Debug) << "Device using an externally provided pool";
+	bufferPool_ = pool;
+
+	return 0;
+}
+
 /**
  * \brief Release all internally allocated buffers
  */
@@ -670,7 +700,20 @@  int V4L2Device::queueBuffer(Buffer *buffer)
 	buf.type = bufferType_;
 	buf.memory = memoryType_;
 
-	if (V4L2_TYPE_IS_MULTIPLANAR(buf.type)) {
+	bool multiPlanar = V4L2_TYPE_IS_MULTIPLANAR(buf.type);
+
+	if (buf.memory == V4L2_MEMORY_DMABUF) {
+		if (multiPlanar) {
+			for (unsigned int p = 0;
+			     p < buffer->planes().size();
+			     p++)
+				planes[p].m.fd = buffer->planes()[p].dmabuf();
+		} else {
+			buf.m.fd = buffer->planes()[0].dmabuf();
+		}
+	}
+
+	if (multiPlanar) {
 		buf.length = buffer->planes().size();
 		buf.m.planes = planes;
 	}