[libcamera-devel,06/10] libcamera: v4l2_videodevice: Simplify error checking for requestBuffers()

Message ID 20191028022224.795355-7-niklas.soderlund@ragnatech.se
State Accepted
Headers show
Series
  • libcamera: Fixes found while working on new buffer API
Related show

Commit Message

Niklas Söderlund Oct. 28, 2019, 2:22 a.m. UTC
There is no point in explicitly checking the same error in the only call
sites for the internal function, centralize the check and simplify the
code.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/libcamera/v4l2_videodevice.cpp | 31 ++++++++++--------------------
 1 file changed, 10 insertions(+), 21 deletions(-)

Comments

Laurent Pinchart Nov. 18, 2019, 11:27 a.m. UTC | #1
Hi Niklas,

Thank you for the patch.

On Mon, Oct 28, 2019 at 03:22:20AM +0100, Niklas Söderlund wrote:
> There is no point in explicitly checking the same error in the only call
> sites for the internal function, centralize the check and simplify the
> code.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/libcamera/v4l2_videodevice.cpp | 31 ++++++++++--------------------
>  1 file changed, 10 insertions(+), 21 deletions(-)
> 
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 208ab54199b141c3..a2a9eab2bcc0d7e8 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -819,9 +819,16 @@ int V4L2VideoDevice::requestBuffers(unsigned int count)
>  		return ret;
>  	}
>  
> +	if (rb.count < count) {
> +		LOG(V4L2, Error)
> +			<< "Not enough buffers provided by V4L2VideoDevice";
> +		requestBuffers(0);
> +		return -ENOMEM;
> +	}
> +
>  	LOG(V4L2, Debug) << rb.count << " buffers requested.";
>  
> -	return rb.count;
> +	return 0;
>  }
>  
>  /**
> @@ -832,24 +839,15 @@ int V4L2VideoDevice::requestBuffers(unsigned int count)
>   */
>  int V4L2VideoDevice::exportBuffers(BufferPool *pool)
>  {
> -	unsigned int allocatedBuffers;
>  	unsigned int i;
>  	int ret;
>  
>  	memoryType_ = V4L2_MEMORY_MMAP;
>  
>  	ret = requestBuffers(pool->count());
> -	if (ret < 0)
> +	if (ret)
>  		return ret;
>  
> -	allocatedBuffers = ret;
> -	if (allocatedBuffers < pool->count()) {
> -		LOG(V4L2, Error)
> -			<< "Not enough buffers provided by V4L2VideoDevice";
> -		requestBuffers(0);
> -		return -ENOMEM;
> -	}
> -
>  	/* Map the buffers. */
>  	for (i = 0; i < pool->count(); ++i) {
>  		struct v4l2_plane planes[VIDEO_MAX_PLANES] = {};
> @@ -936,23 +934,14 @@ int V4L2VideoDevice::createPlane(BufferMemory *buffer, unsigned int index,
>   */
>  int V4L2VideoDevice::importBuffers(BufferPool *pool)
>  {
> -	unsigned int allocatedBuffers;
>  	int ret;
>  
>  	memoryType_ = V4L2_MEMORY_DMABUF;
>  
>  	ret = requestBuffers(pool->count());
> -	if (ret < 0)
> +	if (ret)
>  		return ret;
>  
> -	allocatedBuffers = ret;
> -	if (allocatedBuffers < pool->count()) {
> -		LOG(V4L2, Error)
> -			<< "Not enough buffers provided by V4L2VideoDevice";
> -		requestBuffers(0);
> -		return -ENOMEM;
> -	}
> -
>  	LOG(V4L2, Debug) << "provided pool of " << pool->count() << " buffers";
>  	bufferPool_ = pool;
>

Patch

diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
index 208ab54199b141c3..a2a9eab2bcc0d7e8 100644
--- a/src/libcamera/v4l2_videodevice.cpp
+++ b/src/libcamera/v4l2_videodevice.cpp
@@ -819,9 +819,16 @@  int V4L2VideoDevice::requestBuffers(unsigned int count)
 		return ret;
 	}
 
+	if (rb.count < count) {
+		LOG(V4L2, Error)
+			<< "Not enough buffers provided by V4L2VideoDevice";
+		requestBuffers(0);
+		return -ENOMEM;
+	}
+
 	LOG(V4L2, Debug) << rb.count << " buffers requested.";
 
-	return rb.count;
+	return 0;
 }
 
 /**
@@ -832,24 +839,15 @@  int V4L2VideoDevice::requestBuffers(unsigned int count)
  */
 int V4L2VideoDevice::exportBuffers(BufferPool *pool)
 {
-	unsigned int allocatedBuffers;
 	unsigned int i;
 	int ret;
 
 	memoryType_ = V4L2_MEMORY_MMAP;
 
 	ret = requestBuffers(pool->count());
-	if (ret < 0)
+	if (ret)
 		return ret;
 
-	allocatedBuffers = ret;
-	if (allocatedBuffers < pool->count()) {
-		LOG(V4L2, Error)
-			<< "Not enough buffers provided by V4L2VideoDevice";
-		requestBuffers(0);
-		return -ENOMEM;
-	}
-
 	/* Map the buffers. */
 	for (i = 0; i < pool->count(); ++i) {
 		struct v4l2_plane planes[VIDEO_MAX_PLANES] = {};
@@ -936,23 +934,14 @@  int V4L2VideoDevice::createPlane(BufferMemory *buffer, unsigned int index,
  */
 int V4L2VideoDevice::importBuffers(BufferPool *pool)
 {
-	unsigned int allocatedBuffers;
 	int ret;
 
 	memoryType_ = V4L2_MEMORY_DMABUF;
 
 	ret = requestBuffers(pool->count());
-	if (ret < 0)
+	if (ret)
 		return ret;
 
-	allocatedBuffers = ret;
-	if (allocatedBuffers < pool->count()) {
-		LOG(V4L2, Error)
-			<< "Not enough buffers provided by V4L2VideoDevice";
-		requestBuffers(0);
-		return -ENOMEM;
-	}
-
 	LOG(V4L2, Debug) << "provided pool of " << pool->count() << " buffers";
 	bufferPool_ = pool;