[libcamera-devel] [PATCH 6/7] android: camera_device: Get properties from config

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2021-03-24 12:25:26 +0100, Jacopo Mondi wrote:
> Create the CameraDevice with a reference to the HAL configuration
> file and use it to retrieve device and camera properties.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/android/camera_device.cpp      | 71 ++++++++++++++++--------------
>  src/android/camera_device.h        |  8 +++-
>  src/android/camera_hal_manager.cpp |  3 +-
>  3 files changed, 47 insertions(+), 35 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 72a89258386d..403d149e4f68 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -312,33 +312,22 @@ CameraDevice::Camera3RequestDescriptor::~Camera3RequestDescriptor()
>   * back to the framework using the designated callbacks.
>   */
>  
> -CameraDevice::CameraDevice(unsigned int id, const std::shared_ptr<Camera> &camera)
> -	: id_(id), running_(false), camera_(camera), staticMetadata_(nullptr),
> -	  facing_(CAMERA_FACING_FRONT), orientation_(0)
> +CameraDevice::CameraDevice(unsigned int id, const std::shared_ptr<Camera> &camera,
> +			   CameraHalConfig &halConfig)
> +	: id_(id), running_(false), camera_(camera), halConfig_(&halConfig),
> +	  staticMetadata_(nullptr), facing_(CAMERA_FACING_FRONT), orientation_(0)
>  {
> -	camera_->requestCompleted.connect(this, &CameraDevice::requestComplete);
> -
> -	maker_ = "libcamera";
> -	model_ = "cameraModel";
> -
> -	/* \todo Support getting properties on Android */
> -	std::ifstream fstream("/var/cache/camera/camera.prop");
> -	if (!fstream.is_open())
> -		return;
> -
> -	std::string line;
> -	while (std::getline(fstream, line)) {
> -		std::string::size_type delimPos = line.find("=");
> -		if (delimPos == std::string::npos)
> -			continue;
> -		std::string key = line.substr(0, delimPos);
> -		std::string val = line.substr(delimPos + 1);
> -
> -		if (!key.compare("ro.product.model"))
> -			model_ = val;
> -		else if (!key.compare("ro.product.manufacturer"))
> -			maker_ = val;
> +	maker_ = halConfig_->deviceMaker();
> +	model_ = halConfig_->deviceModel();
> +	if (maker_.empty() || model_.empty()) {
> +		maker_ = "libcamera";
> +		model_ = "cameraModel";
> +		LOG(HAL, Warning)
> +			<< "Cannot find manufacturer information. "
> +			<< "Default it to '" << maker_ << " - " << model_;
>  	}
> +
> +	camera_->requestCompleted.connect(this, &CameraDevice::requestComplete);
>  }
>  
>  CameraDevice::~CameraDevice()
> @@ -351,9 +340,10 @@ CameraDevice::~CameraDevice()
>  }
>  
>  std::shared_ptr<CameraDevice> CameraDevice::create(unsigned int id,
> -						   const std::shared_ptr<Camera> &cam)
> +						   const std::shared_ptr<Camera> &cam,
> +						   CameraHalConfig &halConfig)
>  {
> -	CameraDevice *camera = new CameraDevice(id, cam);
> +	CameraDevice *camera = new CameraDevice(id, cam, halConfig);
>  	return std::shared_ptr<CameraDevice>(camera);
>  }
>  
> @@ -380,11 +370,28 @@ int CameraDevice::initialize()
>  			break;
>  		}
>  	} else {
> -		/*
> -		 * \todo Retrieve the camera location from configuration file
> -		 * if not available from the library.
> -		 */
> -		facing_ = CAMERA_FACING_FRONT;
> +		std::string location = halConfig_->cameraLocation(camera_->id());
> +		if (location.empty()) {
> +			LOG(HAL, Error) << "Location for camera "
> +					<< camera_->id() << " not reported";
> +			return -EINVAL;
> +		}

Would it make sens to cameraLocation() to translate from the string to 
the enum value?

> +
> +		if (location == "front") {
> +			facing_ = CAMERA_FACING_FRONT;
> +		} else if (location == "back") {
> +			facing_ = CAMERA_FACING_BACK;
> +		} else if (location == "external") {
> +			facing_ = CAMERA_FACING_EXTERNAL;
> +		} else {
> +			LOG(HAL, Error) << "Unsupported camera location "
> +					<< location;
> +			return -EINVAL;
> +		}
> +
> +		LOG(HAL, Debug)
> +			<< "Camera location retrieved from configration file: "
> +			<< location;
>  	}
>  
>  	/*
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 823d561cc295..33bfd115a703 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -24,6 +24,7 @@
>  #include "libcamera/internal/log.h"
>  #include "libcamera/internal/message.h"
>  
> +#include "camera_hal_config.h"
>  #include "camera_metadata.h"
>  #include "camera_stream.h"
>  #include "camera_worker.h"
> @@ -33,7 +34,8 @@ class CameraDevice : protected libcamera::Loggable
>  {
>  public:
>  	static std::shared_ptr<CameraDevice> create(unsigned int id,
> -						    const std::shared_ptr<libcamera::Camera> &cam);
> +						    const std::shared_ptr<libcamera::Camera> &cam,
> +						    CameraHalConfig &halConfig);
>  	~CameraDevice();
>  
>  	int initialize();
> @@ -66,7 +68,8 @@ protected:
>  	std::string logPrefix() const override;
>  
>  private:
> -	CameraDevice(unsigned int id, const std::shared_ptr<libcamera::Camera> &camera);
> +	CameraDevice(unsigned int id, const std::shared_ptr<libcamera::Camera> &camera,
> +		     CameraHalConfig &halConfig);
>  
>  	struct Camera3RequestDescriptor {
>  		Camera3RequestDescriptor(libcamera::Camera *camera,
> @@ -113,6 +116,7 @@ private:
>  	bool running_;
>  	std::shared_ptr<libcamera::Camera> camera_;
>  	std::unique_ptr<libcamera::CameraConfiguration> config_;
> +	CameraHalConfig *halConfig_;
>  
>  	CameraMetadata *staticMetadata_;
>  	std::map<unsigned int, const CameraMetadata *> requestTemplates_;
> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
> index a19f80edede8..4fb5c87e2a68 100644
> --- a/src/android/camera_hal_manager.cpp
> +++ b/src/android/camera_hal_manager.cpp
> @@ -129,7 +129,8 @@ void CameraHalManager::cameraAdded(std::shared_ptr<Camera> cam)
>  	}
>  
>  	/* Create a CameraDevice instance to wrap the libcamera Camera. */
> -	std::shared_ptr<CameraDevice> camera = CameraDevice::create(id, std::move(cam));
> +	std::shared_ptr<CameraDevice> camera = CameraDevice::create(id, std::move(cam),
> +								    halConfig_);
>  	int ret = camera->initialize();
>  	if (ret) {
>  		LOG(HAL, Error) << "Failed to initialize camera: " << cam->id();
> -- 
> 2.30.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund