[libcamera-devel] [PATCH 13/30] libcamera: v4l2_videodevice: Extract exportDmaBuffer() to export DMA buffer

[Jacopo Mondi]

Hi Niklas,

On Wed, Nov 27, 2019 at 12:36:03AM +0100, Niklas Söderlund wrote:
> The part in createPlane() that exports a dma buffer from a video device
> will be used directly by the FrameBuffer interface. Break it out to a
> own function.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/include/v4l2_videodevice.h |  1 +
>  src/libcamera/v4l2_videodevice.cpp       | 41 +++++++++++++++---------
>  2 files changed, 27 insertions(+), 15 deletions(-)
>
> diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
> index fdf11b3a6ec9b702..34bbff41760753bd 100644
> --- a/src/libcamera/include/v4l2_videodevice.h
> +++ b/src/libcamera/include/v4l2_videodevice.h
> @@ -179,6 +179,7 @@ private:
>  	int requestBuffers(unsigned int count);
>  	int createPlane(BufferMemory *buffer, unsigned int index,
>  			unsigned int plane, unsigned int length);
> +	int exportDmaBuffer(unsigned int index, unsigned int plane);

I would have named this exportBuffer()

>
>  	Buffer *dequeueBuffer();
>  	void bufferAvailable(EventNotifier *notifier);
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 644e4545a2f33b2e..cc0a1c9382a2b1ed 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -901,28 +901,19 @@ int V4L2VideoDevice::exportBuffers(BufferPool *pool)
>  int V4L2VideoDevice::createPlane(BufferMemory *buffer, unsigned int index,
>  				 unsigned int planeIndex, unsigned int length)
>  {
> -	struct v4l2_exportbuffer expbuf = {};
> -	int ret;
> +	int fd;
>
>  	LOG(V4L2, Debug)
>  		<< "Buffer " << index
>  		<< " plane " << planeIndex
>  		<< ": length=" << length;
>
> -	expbuf.type = bufferType_;
> -	expbuf.index = index;
> -	expbuf.plane = planeIndex;
> -	expbuf.flags = O_RDWR;
> -
> -	ret = ioctl(VIDIOC_EXPBUF, &expbuf);
> -	if (ret < 0) {
> -		LOG(V4L2, Error)
> -			<< "Failed to export buffer: " << strerror(-ret);
> -		return ret;
> -	}
> +	fd = exportDmaBuffer(index, planeIndex);
> +	if (fd < 0)
> +		return fd;
>
> -	buffer->planes().emplace_back(expbuf.fd, length);
> -	::close(expbuf.fd);
> +	buffer->planes().emplace_back(fd, length);
> +	::close(fd);
>
>  	return 0;
>  }
> @@ -948,6 +939,26 @@ int V4L2VideoDevice::importBuffers(BufferPool *pool)
>  	return 0;
>  }
>
> +int V4L2VideoDevice::exportDmaBuffer(unsigned int index, unsigned int plane)
> +{
> +	struct v4l2_exportbuffer expbuf = {};
> +	int ret;
> +
> +	expbuf.type = bufferType_;
> +	expbuf.index = index;
> +	expbuf.plane = plane;
> +	expbuf.flags = O_RDWR;
> +
> +	ret = ioctl(VIDIOC_EXPBUF, &expbuf);
> +	if (ret < 0) {

Do you need the usual ret = -errno dance to avoid errno being
overwritten by LOG() ?

> +		LOG(V4L2, Error)
> +			<< "Failed to export buffer: " << strerror(-ret);

Ah, you really need to assign ret before this one :)
> +		return ret;
> +	}
> +
> +	return expbuf.fd;
> +}
> +

With this addressed

Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

>  /**
>   * \brief Release all internally allocated buffers
>   */
> --
> 2.24.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20191127/0aeceff8/attachment.sig>