[libcamera-devel] [PATCH v4.1 5/5] android: camera_hal_manager: Support camera hotplug
Umang Jain
email at uajain.com
Sun Aug 23 19:17:34 CEST 2020
Hi Laurent,
I have reviewed the changes and also tested on CrOS
with my setup. Things look good to me.
Thanks for the writing last final fixes of your review. :)
On 8/23/20 2:59 AM, Laurent Pinchart wrote:
> From: Umang Jain <email at uajain.com>
>
> Extend the support for camera hotplug from libcamera's CameraManager
> to CameraHalManager. Use camera module callbacks to let the framework
> know about the hotplug events and change the status of cameras being
> hotplugged or unplugged via camera_device_status_change().
>
> Introduce a map cameraIdsMap_ which book-keeps all cameras seen in the
> past by the CameraHalManager. If the camera is seen for the first time,
> a new id is assigned to it. If the camera has been seen before by the
> manager, its old id is reused. IDs for internal cameras start with
> '0' and for external cameras, they start with '1000'. Accesses to
> cameraIdsMap_ and cameras_ are protected by a mutex.
>
> Signed-off-by: Umang Jain <email at uajain.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> Changes since v4:
>
> - Rename cameraDeviceFromHALId() to cameraDeviceFromHalId()
> - Return a CameraDevice from cameraDeviceFromHalId()
> - Inline cameraDeviceFromCamera() in its only caller
> - Drop duplicate location check
> - Drop cameraDeviceIter type
> - Add firstExternalCameraId_
> - Make Mutex and MutexLocker private member types
> - Replace camerasMap_ with cameraIdsMap_ in commit message
> - Reflow some comments
>
> I've applied these changes locally during review of v4 to compile-test
> my comments, so I'm posting them to avoid duplicating work.
> ---
> src/android/camera_hal_manager.cpp | 168 ++++++++++++++++++++++++-----
> src/android/camera_hal_manager.h | 19 ++++
> 2 files changed, 163 insertions(+), 24 deletions(-)
>
> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
> index 3a744af5f231..a1805ebccf24 100644
> --- a/src/android/camera_hal_manager.cpp
> +++ b/src/android/camera_hal_manager.cpp
> @@ -8,6 +8,7 @@
> #include "camera_hal_manager.h"
>
> #include <libcamera/camera.h>
> +#include <libcamera/property_ids.h>
>
> #include "libcamera/internal/log.h"
>
> @@ -28,7 +29,8 @@ LOG_DECLARE_CATEGORY(HAL);
> */
>
> CameraHalManager::CameraHalManager()
> - : cameraManager_(nullptr)
> + : cameraManager_(nullptr), numInternalCameras_(0),
> + nextExternalCameraId_(firstExternalCameraId_)
> {
> }
>
> @@ -47,6 +49,10 @@ int CameraHalManager::init()
> {
> cameraManager_ = new CameraManager();
>
> + /* Support camera hotplug. */
> + cameraManager_->cameraAdded.connect(this, &CameraHalManager::cameraAdded);
> + cameraManager_->cameraRemoved.connect(this, &CameraHalManager::cameraRemoved);
> +
> int ret = cameraManager_->start();
> if (ret) {
> LOG(HAL, Error) << "Failed to start camera manager: "
> @@ -56,35 +62,20 @@ int CameraHalManager::init()
> return ret;
> }
>
> - /*
> - * For each Camera registered in the system, a CameraDevice
> - * gets created here to wraps a libcamera Camera instance.
> - *
> - * \todo Support camera hotplug.
> - */
> - unsigned int index = 0;
> - for (auto &cam : cameraManager_->cameras()) {
> - std::shared_ptr<CameraDevice> camera = CameraDevice::create(index, cam);
> - ret = camera->initialize();
> - if (ret)
> - continue;
> -
> - cameras_.emplace_back(std::move(camera));
> - ++index;
> - }
> -
> return 0;
> }
>
> CameraDevice *CameraHalManager::open(unsigned int id,
> const hw_module_t *hardwareModule)
> {
> - if (id >= numCameras()) {
> + MutexLocker locker(mutex_);
> +
> + CameraDevice *camera = cameraDeviceFromHalId(id);
> + if (!camera) {
> LOG(HAL, Error) << "Invalid camera id '" << id << "'";
> return nullptr;
> }
>
> - CameraDevice *camera = cameras_[id].get();
> if (camera->open(hardwareModule))
> return nullptr;
>
> @@ -93,9 +84,120 @@ CameraDevice *CameraHalManager::open(unsigned int id,
> return camera;
> }
>
> +void CameraHalManager::cameraAdded(std::shared_ptr<Camera> cam)
> +{
> + unsigned int id;
> + bool isCameraExternal = false;
> + bool isCameraNew = false;
> +
> + MutexLocker locker(mutex_);
> +
> + /*
> + * Each camera is assigned a unique integer ID when it is seen for the
> + * first time. If the camera has been seen before, the previous ID is
> + * re-used.
> + *
> + * IDs starts from '0' for internal cameras and '1000' for external
> + * cameras.
> + */
> + auto iter = cameraIdsMap_.find(cam->id());
> + if (iter != cameraIdsMap_.end()) {
> + id = iter->second;
> + } else {
> + isCameraNew = true;
> +
> + /*
> + * Now check if this is an external camera and assign
> + * its id accordingly.
> + */
> + if (cameraLocation(cam.get()) == properties::CameraLocationExternal) {
> + isCameraExternal = true;
> + id = nextExternalCameraId_;
> + } else {
> + id = numInternalCameras_;
> + }
> + }
> +
> + /* Create a CameraDevice instance to wrap the libcamera Camera. */
> + std::shared_ptr<CameraDevice> camera = CameraDevice::create(id, std::move(cam));
> + int ret = camera->initialize();
> + if (ret) {
> + LOG(HAL, Error) << "Failed to initialize camera: " << cam->id();
> + return;
> + }
> +
> + if (isCameraNew) {
> + cameraIdsMap_.emplace(cam->id(), id);
> +
> + if (isCameraExternal)
> + nextExternalCameraId_++;
> + else
> + numInternalCameras_++;
> + }
> +
> + cameras_.emplace_back(std::move(camera));
> +
> + if (callbacks_)
> + callbacks_->camera_device_status_change(callbacks_, id,
> + CAMERA_DEVICE_STATUS_PRESENT);
> +
> + LOG(HAL, Debug) << "Camera ID: " << id << " added successfully.";
> +}
> +
> +void CameraHalManager::cameraRemoved(std::shared_ptr<Camera> cam)
> +{
> + MutexLocker locker(mutex_);
> +
> + auto iter = std::find_if(cameras_.begin(), cameras_.end(),
> + [&cam](std::shared_ptr<CameraDevice> &camera) {
> + return cam.get() == camera->camera();
> + });
> + if (iter == cameras_.end())
> + return;
> +
> + /*
> + * CAMERA_DEVICE_STATUS_NOT_PRESENT should be set for external cameras
> + * only.
> + */
> + unsigned int id = (*iter)->id();
> + if (id >= firstExternalCameraId_)
> + callbacks_->camera_device_status_change(callbacks_, id,
> + CAMERA_DEVICE_STATUS_NOT_PRESENT);
> +
> + /*
> + * \todo Check if the camera is already open and running.
> + * Inform the framework about its absence before deleting its
> + * reference here.
> + */
> + cameras_.erase(iter);
> +
> + LOG(HAL, Debug) << "Camera ID: " << id << " removed successfully.";
> +}
> +
> +int32_t CameraHalManager::cameraLocation(const Camera *cam)
> +{
> + const ControlList &properties = cam->properties();
> + if (!properties.contains(properties::Location))
> + return -1;
> +
> + return properties.get(properties::Location);
> +}
> +
> +CameraDevice *CameraHalManager::cameraDeviceFromHalId(unsigned int id)
> +{
> + auto iter = std::find_if(cameras_.begin(), cameras_.end(),
> + [id](std::shared_ptr<CameraDevice> &camera) {
> + return camera->id() == id;
> + });
> + if (iter == cameras_.end())
> + return nullptr;
> +
> + return iter->get();
> +}
> +
> unsigned int CameraHalManager::numCameras() const
> {
> - return cameraManager_->cameras().size();
> + return numInternalCameras_;
> }
>
> int CameraHalManager::getCameraInfo(unsigned int id, struct camera_info *info)
> @@ -103,13 +205,14 @@ int CameraHalManager::getCameraInfo(unsigned int id, struct camera_info *info)
> if (!info)
> return -EINVAL;
>
> - if (id >= numCameras()) {
> + MutexLocker locker(mutex_);
> +
> + CameraDevice *camera = cameraDeviceFromHalId(id);
> + if (!camera) {
> LOG(HAL, Error) << "Invalid camera id '" << id << "'";
> return -EINVAL;
> }
>
> - CameraDevice *camera = cameras_[id].get();
> -
> info->facing = camera->facing();
> info->orientation = camera->orientation();
> info->device_version = CAMERA_DEVICE_API_VERSION_3_3;
> @@ -124,4 +227,21 @@ int CameraHalManager::getCameraInfo(unsigned int id, struct camera_info *info)
> void CameraHalManager::setCallbacks(const camera_module_callbacks_t *callbacks)
> {
> callbacks_ = callbacks;
> +
> + MutexLocker locker(mutex_);
> +
> + /*
> + * Some external cameras may have been identified before the callbacks_
> + * were set. Iterate all existing external cameras and mark them as
> + * CAMERA_DEVICE_STATUS_PRESENT explicitly.
> + *
> + * Internal cameras are already assumed to be present at module load
> + * time by the Android framework.
> + */
> + for (std::shared_ptr<CameraDevice> &camera : cameras_) {
> + unsigned int id = camera->id();
> + if (id >= firstExternalCameraId_)
> + callbacks_->camera_device_status_change(callbacks_, id,
> + CAMERA_DEVICE_STATUS_PRESENT);
> + }
> }
> diff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h
> index 3e34d63ff96c..a91decc7d8fe 100644
> --- a/src/android/camera_hal_manager.h
> +++ b/src/android/camera_hal_manager.h
> @@ -7,6 +7,8 @@
> #ifndef __ANDROID_CAMERA_MANAGER_H__
> #define __ANDROID_CAMERA_MANAGER_H__
>
> +#include <map>
> +#include <mutex>
> #include <stddef.h>
> #include <vector>
>
> @@ -33,10 +35,27 @@ public:
> void setCallbacks(const camera_module_callbacks_t *callbacks);
>
> private:
> + using Mutex = std::mutex;
> + using MutexLocker = std::unique_lock<std::mutex>;
> +
> + static constexpr unsigned int firstExternalCameraId_ = 1000;
> +
> + static int32_t cameraLocation(const libcamera::Camera *cam);
> +
> + void cameraAdded(std::shared_ptr<libcamera::Camera> cam);
> + void cameraRemoved(std::shared_ptr<libcamera::Camera> cam);
> +
> + CameraDevice *cameraDeviceFromHalId(unsigned int id);
> +
> libcamera::CameraManager *cameraManager_;
>
> const camera_module_callbacks_t *callbacks_;
> std::vector<std::shared_ptr<CameraDevice>> cameras_;
> + std::map<std::string, unsigned int> cameraIdsMap_;
> + Mutex mutex_;
> +
> + unsigned int numInternalCameras_;
> + unsigned int nextExternalCameraId_;
> };
>
> #endif /* __ANDROID_CAMERA_MANAGER_H__ */
More information about the libcamera-devel
mailing list