Message ID | 20200526142237.407557-4-jacopo@jmondi.org |
---|---|
State | Accepted |
Headers | show |
Series |
|
Related | show |
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@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@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();
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. + */ +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(); + */ 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();
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. Signed-off-by: Jacopo Mondi <jacopo@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(-)