[libcamera-devel] [PATCH v3 2/4] android: Disallow external location in HAL config

[Laurent Pinchart]

Hi Umang,

Thank you for the patch.

On Fri, Jul 30, 2021 at 04:31:52PM +0530, Umang Jain wrote:
> Error out on any camera's location if set to "external",
> in the HAL configuration file. The HAL configuration file
> is only meant for the integrated cameras present on the system.

This makes sense, but I'd change the rationale a little bit. We may want
to use the HAL configuration file in the future to also specify
information for external cameras (not sure what the use cases would be,
but someone attaching an external camera to an Android device may want
to tweak properties through the file). However, in that case, libcamera
will report the location as external already, so there's no need for the
configuration file to override the location. Overriding the location to
external would only be needed for cameras that libcamera report as front
or back, and this wouldn't make sense.

You could write the commit message as

"Error out on any camera's location if set to "external", in the HAL
configuration file. The HAL configuration file is meant to override the
location property, and overriding an internal camera location to
external doesn't make sense."

> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>

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

> ---
>  src/android/camera_hal_config.cpp | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp
> index 833cf4ba..7126aba4 100644
> --- a/src/android/camera_hal_config.cpp
> +++ b/src/android/camera_hal_config.cpp
> @@ -127,8 +127,6 @@ int CameraHalConfig::Private::parseCameraLocation(CameraConfigData *cameraConfig
>  		cameraConfigData->facing = CAMERA_FACING_FRONT;
>  	else if (location == "back")
>  		cameraConfigData->facing = CAMERA_FACING_BACK;
> -	else if (location == "external")
> -		cameraConfigData->facing = CAMERA_FACING_EXTERNAL;
>  	else
>  		return -EINVAL;
>  

-- 
Regards,

Laurent Pinchart