[libcamera-devel,v4,1/2] android: cameraDevice: Factorize the code of validating camera3_capture_request
diff mbox series

Message ID 20210403133741.1595440-1-hiroh@chromium.org
State Accepted
Headers show
Series
  • [libcamera-devel,v4,1/2] android: cameraDevice: Factorize the code of validating camera3_capture_request
Related show

Commit Message

Hirokazu Honda April 3, 2021, 1:37 p.m. UTC 
CameraDevice::processCaptureRequest() checks the validness of a
provided camera3_capture_request. This factorizes the code in
order to add more validation to the request later.

Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/android/camera_device.cpp | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

Comments

Laurent Pinchart April 3, 2021, 10:47 p.m. UTC | #1 
Hi Hiro,

On Sat, Apr 03, 2021 at 10:37:40PM +0900, Hirokazu Honda wrote:
> CameraDevice::processCaptureRequest() checks the validness of a

Just a small comment, it's "validity", not "validness". I'll fix when
applying.

> provided camera3_capture_request. This factorizes the code in
> order to add more validation to the request later.
> 
> Signed-off-by: Hirokazu Honda <hiroh@chromium.org>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/android/camera_device.cpp | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index eb327978..d8ce43be 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -256,6 +256,21 @@ void sortCamera3StreamConfigs(std::vector<Camera3StreamConfig> &unsortedConfigs,
>  	unsortedConfigs = sortedConfigs;
>  }
>  
> +bool isValidRequest(camera3_capture_request_t *camera3Request)
> +{
> +	if (!camera3Request) {
> +		LOG(HAL, Error) << "No capture request provided";
> +		return false;
> +	}
> +
> +	if (!camera3Request->num_output_buffers) {
> +		LOG(HAL, Error) << "No output buffers provided";
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
>  } /* namespace */
>  
>  /*
> @@ -1785,15 +1800,8 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
>  
>  int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Request)
>  {
> -	if (!camera3Request) {
> -		LOG(HAL, Error) << "No capture request provided";
> +	if (!isValidRequest(camera3Request))
>  		return -EINVAL;
> -	}
> -
> -	if (!camera3Request->num_output_buffers) {
> -		LOG(HAL, Error) << "No output buffers provided";
> -		return -EINVAL;
> -	}
>  
>  	/* Start the camera if that's the first request we handle. */
>  	if (!running_) {

Patch
diff mbox series 

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index eb327978..d8ce43be 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -256,6 +256,21 @@  void sortCamera3StreamConfigs(std::vector<Camera3StreamConfig> &unsortedConfigs,
 	unsortedConfigs = sortedConfigs;
 }
 
+bool isValidRequest(camera3_capture_request_t *camera3Request)
+{
+	if (!camera3Request) {
+		LOG(HAL, Error) << "No capture request provided";
+		return false;
+	}
+
+	if (!camera3Request->num_output_buffers) {
+		LOG(HAL, Error) << "No output buffers provided";
+		return false;
+	}
+
+	return true;
+}
+
 } /* namespace */
 
 /*
@@ -1785,15 +1800,8 @@  int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
 
 int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Request)
 {
-	if (!camera3Request) {
-		LOG(HAL, Error) << "No capture request provided";
+	if (!isValidRequest(camera3Request))
 		return -EINVAL;
-	}
-
-	if (!camera3Request->num_output_buffers) {
-		LOG(HAL, Error) << "No output buffers provided";
-		return -EINVAL;
-	}
 
 	/* Start the camera if that's the first request we handle. */
 	if (!running_) {