[libcamera-devel] [PATCH v4 4/5] android: camera_device: Get properties from configuration

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Tue, Apr 06, 2021 at 05:45:56PM +0200, Jacopo Mondi wrote:
> Open the HAL configuration file in the Camera HAL manager and get
> the camera properties for each created CameraDevice and initialize it
> with them.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/android/camera_device.cpp      | 29 ++++++++++++++++++++++-------
>  src/android/camera_device.h        |  3 ++-
>  src/android/camera_hal_manager.cpp | 21 +++++++++++++++++++--
>  src/android/camera_hal_manager.h   |  3 +++
>  4 files changed, 46 insertions(+), 10 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 89044efa7ebe..66030c012db0 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -6,6 +6,7 @@
>   */
>  
>  #include "camera_device.h"
> +#include "camera_hal_config.h"
>  #include "camera_ops.h"
>  #include "post_processor.h"
>  
> @@ -446,9 +447,15 @@ std::unique_ptr<CameraDevice> CameraDevice::create(unsigned int id,
>   * Initialize the camera static information.
>   * This method is called before the camera device is opened.
>   */
> -int CameraDevice::initialize()
> +int CameraDevice::initialize(const CameraProps &cameraProps)
>  {
> -	/* Initialize orientation and facing side of the camera. */
> +	/*
> +	 * Initialize orientation and facing side of the camera.
> +	 *
> +	 * If the libcamera::Camera provides those information as retrieved
> +	 * from firmware use that, otherwise fallback to values parsed from
> +	 * the configuration file.
> +	 */
>  	const ControlList &properties = camera_->properties();
>  
>  	if (properties.contains(properties::Location)) {
> @@ -464,12 +471,14 @@ int CameraDevice::initialize()
>  			facing_ = CAMERA_FACING_EXTERNAL;
>  			break;
>  		}
> +
> +		if (facing_ != cameraProps.facing)
> +			LOG(HAL, Warning)
> +				<< "Camera location does not match"
> +				<< " configuration file. Use " << facing_;

s/Use/Using/

> +
>  	} else {
> -		/*
> -		 * \todo Retrieve the camera location from configuration file
> -		 * if not available from the library.
> -		 */
> -		facing_ = CAMERA_FACING_FRONT;
> +		facing_ = cameraProps.facing;
>  	}
>  
>  	/*
> @@ -483,6 +492,12 @@ int CameraDevice::initialize()
>  	if (properties.contains(properties::Rotation)) {
>  		int rotation = properties.get(properties::Rotation);
>  		orientation_ = (360 - rotation) % 360;
> +		if (orientation_ != cameraProps.rotation)
> +			LOG(HAL, Warning)
> +				<< "Camera orientation does not match"
> +				<< " configuration file. Use " << orientation_;

Same here.

> +	} else {
> +		orientation_ = cameraProps.rotation;
>  	}
>  
>  	int ret = camera_->acquire();
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 11bdfec8d587..ba3ec8770e11 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -29,6 +29,7 @@
>  #include "camera_worker.h"
>  #include "jpeg/encoder.h"
>  
> +class CameraProps;
>  class CameraDevice : protected libcamera::Loggable
>  {
>  public:
> @@ -36,7 +37,7 @@ public:
>  						    std::shared_ptr<libcamera::Camera> cam);
>  	~CameraDevice();
>  
> -	int initialize();
> +	int initialize(const CameraProps &cameraProps);
>  
>  	int open(const hw_module_t *hardwareModule);
>  	void close();
> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
> index bf3fcda75237..a517727ea0b8 100644
> --- a/src/android/camera_hal_manager.cpp
> +++ b/src/android/camera_hal_manager.cpp
> @@ -39,13 +39,17 @@ CameraHalManager::~CameraHalManager() = default;
>  
>  int CameraHalManager::init()
>  {
> +	int ret = halConfig_.open();
> +	if (ret)
> +		return ret;
> +
>  	cameraManager_ = std::make_unique<CameraManager>();
>  
>  	/* Support camera hotplug. */
>  	cameraManager_->cameraAdded.connect(this, &CameraHalManager::cameraAdded);
>  	cameraManager_->cameraRemoved.connect(this, &CameraHalManager::cameraRemoved);
>  
> -	int ret = cameraManager_->start();
> +	ret = cameraManager_->start();
>  	if (ret) {
>  		LOG(HAL, Error) << "Failed to start camera manager: "
>  				<< strerror(-ret);
> @@ -115,9 +119,22 @@ void CameraHalManager::cameraAdded(std::shared_ptr<Camera> cam)
>  		}
>  	}
>  
> +	/*
> +	 * Get camera properties from the configuration file.
> +	 *
> +	 * The camera properties as recorded in the configuration file
> +	 * supplement information that cannot be retrieved from the
> +	 * libcamera::Camera at run time.
> +	 */
> +	const CameraProps &cameraProps = halConfig_.cameraProps(cam->id());
> +	if (!cameraProps.valid) {
> +		LOG(HAL, Error) << "Failed to register camera: " << cam->id();
> +		return;
> +	}

Could you please reply to the comment in v4 regarding UVC cameras ?

> +
>  	/* Create a CameraDevice instance to wrap the libcamera Camera. */
>  	std::unique_ptr<CameraDevice> camera = CameraDevice::create(id, cam);
> -	int ret = camera->initialize();
> +	int ret = camera->initialize(cameraProps);
>  	if (ret) {
>  		LOG(HAL, Error) << "Failed to initialize camera: " << cam->id();
>  		return;
> diff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h
> index d9bf27989965..af1581da6579 100644
> --- a/src/android/camera_hal_manager.h
> +++ b/src/android/camera_hal_manager.h
> @@ -19,6 +19,8 @@
>  
>  #include <libcamera/camera_manager.h>
>  
> +#include "camera_hal_config.h"
> +
>  class CameraDevice;
>  
>  class CameraHalManager
> @@ -50,6 +52,7 @@ private:
>  	CameraDevice *cameraDeviceFromHalId(unsigned int id);
>  
>  	std::unique_ptr<libcamera::CameraManager> cameraManager_;
> +	CameraHalConfig halConfig_;
>  
>  	const camera_module_callbacks_t *callbacks_;
>  	std::vector<std::unique_ptr<CameraDevice>> cameras_;

-- 
Regards,

Laurent Pinchart