[libcamera-devel] [PATCH 3/8] android: hal_manager: Do not hardcode properties

Laurent Pinchart laurent.pinchart at ideasonboard.com
Thu Jun 4 02:48:07 CEST 2020


Hi Jacopo,

Thank you for the patch.

On Tue, May 26, 2020 at 04:22:32PM +0200, Jacopo Mondi wrote:
> The CameraHalManager::getCameraInfo() method hardcodes the camera facing
> side and orientation (which corresponds, confusingly, to libcamera's
> location and rotation properties).
> 
> Instead of hard-coding the values based on the camera id, cache the
> libcamera Camera properties in a new initialize() method, and use them
> both to report camera info and to populate the static metadata buffer.

Technically speaking, you're not caching the libcamera properties, but
the facing and orientation values.

> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/android/camera_device.cpp      | 80 ++++++++++++++++++++----------
>  src/android/camera_device.h        |  8 +++
>  src/android/camera_hal_manager.cpp | 10 ++--
>  3 files changed, 68 insertions(+), 30 deletions(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index ad277cb059ca..69b25ed2f11f 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -53,7 +53,8 @@ CameraDevice::Camera3RequestDescriptor::~Camera3RequestDescriptor()
>   */
>  
>  CameraDevice::CameraDevice(unsigned int id, const std::shared_ptr<Camera> &camera)
> -	: running_(false), camera_(camera), staticMetadata_(nullptr)
> +	: running_(false), camera_(camera), staticMetadata_(nullptr),
> +	  facing_(CAMERA_FACING_FRONT), orientation_(0)
>  {
>  	camera_->requestCompleted.connect(this, &CameraDevice::requestComplete);
>  }
> @@ -67,6 +68,45 @@ CameraDevice::~CameraDevice()
>  		delete it.second;
>  }
>  
> +/*
> + * Initialize the camera static information.
> + * This method is called before the camera device is open.

s/open/opened/ ?

> + */
> +int CameraDevice::initialize()
> +{
> +	/* Initialize orientation and facing side of the camera. */
> +	const ControlList &properties = camera_->properties();
> +
> +	if (properties.contains(properties::Location)) {
> +		int32_t location = properties.get(properties::Location);
> +		switch (location) {
> +		case properties::CameraLocationFront:
> +			facing_ = CAMERA_FACING_FRONT;
> +			break;
> +		case properties::CameraLocationBack:
> +			facing_ = CAMERA_FACING_BACK;
> +			break;
> +		case properties::CameraLocationExternal:
> +			facing_ = CAMERA_FACING_EXTERNAL;
> +			break;
> +		}
> +	}
> +
> +	/*
> +	 * The Android orientation metadata and libcamera rotation property are
> +	 * defined differently but have identical numerical values for Android
> +	 * devices such as phones and tablets.
> +	 */
> +	if (properties.contains(properties::Rotation))
> +		orientation_ = properties.get(properties::Rotation);
> +
> +	return 0;
> +}
> +
> +/*
> + * Open a camera device. The static information on the camera shall have been
> + * initialized with a call to CameraDevice::init();

s/init();/initialize()./

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

> + */
>  int CameraDevice::open(const hw_module_t *hardwareModule)
>  {
>  	int ret = camera_->acquire();
> @@ -112,8 +152,6 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  	if (staticMetadata_)
>  		return staticMetadata_->get();
>  
> -	const ControlList &properties = camera_->properties();
> -
>  	/*
>  	 * The here reported metadata are enough to implement a basic capture
>  	 * example application, but a real camera implementation will require
> @@ -278,15 +316,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  	staticMetadata_->addEntry(ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE,
>  				  &exposureTimeRange, 2);
>  
> -	/*
> -	 * The Android orientation metadata and libcamera rotation property are
> -	 * defined differently but have identical numerical values for Android
> -	 * devices such as phones and tablets.
> -	 */
> -	int32_t orientation = 0;
> -	if (properties.contains(properties::Rotation))
> -		orientation = properties.get(properties::Rotation);
> -	staticMetadata_->addEntry(ANDROID_SENSOR_ORIENTATION, &orientation, 1);
> +	staticMetadata_->addEntry(ANDROID_SENSOR_ORIENTATION, &orientation_, 1);
>  
>  	std::vector<int32_t> testPatterModes = {
>  		ANDROID_SENSOR_TEST_PATTERN_MODE_OFF,
> @@ -332,20 +362,18 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  				  lensApertures.data(),
>  				  lensApertures.size());
>  
> -	uint8_t lensFacing = ANDROID_LENS_FACING_FRONT;
> -	if (properties.contains(properties::Location)) {
> -		int32_t location = properties.get(properties::Location);
> -		switch (location) {
> -		case properties::CameraLocationFront:
> -			lensFacing = ANDROID_LENS_FACING_FRONT;
> -			break;
> -		case properties::CameraLocationBack:
> -			lensFacing = ANDROID_LENS_FACING_BACK;
> -			break;
> -		case properties::CameraLocationExternal:
> -			lensFacing = ANDROID_LENS_FACING_EXTERNAL;
> -			break;
> -		}
> +	uint8_t lensFacing;
> +	switch (facing_) {
> +	default:
> +	case CAMERA_FACING_FRONT:
> +		lensFacing = ANDROID_LENS_FACING_FRONT;
> +		break;
> +	case CAMERA_FACING_BACK:
> +		lensFacing = ANDROID_LENS_FACING_BACK;
> +		break;
> +	case CAMERA_FACING_EXTERNAL:
> +		lensFacing = ANDROID_LENS_FACING_EXTERNAL;
> +		break;
>  	}
>  	staticMetadata_->addEntry(ANDROID_LENS_FACING, &lensFacing, 1);
>  
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 350408c1a3e4..ace9c1b7c929 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -27,12 +27,17 @@ public:
>  	CameraDevice(unsigned int id, const std::shared_ptr<libcamera::Camera> &camera);
>  	~CameraDevice();
>  
> +	int initialize();
> +
>  	int open(const hw_module_t *hardwareModule);
>  	void close();
>  
>  	unsigned int id() const { return id_; }
>  	camera3_device_t *camera3Device() { return &camera3Device_; }
>  
> +	int facing() const { return facing_; }
> +	int orientation() const { return orientation_; }
> +
>  	void setCallbacks(const camera3_callback_ops_t *callbacks);
>  	const camera_metadata_t *getStaticMetadata();
>  	const camera_metadata_t *constructDefaultRequestSettings(int type);
> @@ -69,6 +74,9 @@ private:
>  	CameraMetadata *staticMetadata_;
>  	std::map<unsigned int, CameraMetadata *> requestTemplates_;
>  	const camera3_callback_ops_t *callbacks_;
> +
> +	int facing_;
> +	int orientation_;
>  };
>  
>  #endif /* __ANDROID_CAMERA_DEVICE_H__ */
> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
> index b02d8d1a8362..02b6418fb36d 100644
> --- a/src/android/camera_hal_manager.cpp
> +++ b/src/android/camera_hal_manager.cpp
> @@ -65,8 +65,11 @@ int CameraHalManager::init()
>  	unsigned int index = 0;
>  	for (auto &cam : cameraManager_->cameras()) {
>  		CameraDevice *camera = new CameraDevice(index, cam);
> -		cameras_.emplace_back(camera);
> +		ret = camera->initialize();
> +		if (ret)
> +			continue;
>  
> +		cameras_.emplace_back(camera);
>  		++index;
>  	}
>  
> @@ -107,9 +110,8 @@ int CameraHalManager::getCameraInfo(unsigned int id, struct camera_info *info)
>  
>  	CameraDevice *camera = cameras_[id].get();
>  
> -	/* \todo Get these info dynamically inspecting the camera module. */
> -	info->facing = id ? CAMERA_FACING_FRONT : CAMERA_FACING_BACK;
> -	info->orientation = 0;
> +	info->facing = camera->facing();
> +	info->orientation = camera->orientation();
>  	info->device_version = CAMERA_DEVICE_API_VERSION_3_3;
>  	info->resource_cost = 0;
>  	info->static_camera_characteristics = camera->getStaticMetadata();

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list