{"id":9373,"url":"https://patchwork.libcamera.org/api/patches/9373/?format=json","web_url":"https://patchwork.libcamera.org/patch/9373/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20200822212913.28635-1-laurent.pinchart@ideasonboard.com>","date":"2020-08-22T21:29:13","name":"[libcamera-devel,v4.1,5/5] android: camera_hal_manager: Support camera hotplug","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"4acf6a7f9369013a60dc212474cf34a2e3129b6e","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/9373/mbox/","series":[{"id":1237,"url":"https://patchwork.libcamera.org/api/series/1237/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=1237","date":"2020-08-22T21:29:13","name":null,"version":1,"mbox":"https://patchwork.libcamera.org/series/1237/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/9373/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/9373/checks/","tags":{},"headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id AE2A8BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 22 Aug 2020 21:29:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2A33062710;\n\tSat, 22 Aug 2020 23:29:41 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9864F61EA0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 22 Aug 2020 23:29:39 +0200 (CEST)","from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 13E0629E;\n\tSat, 22 Aug 2020 23:29:39 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"dw0owEnn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1598131779;\n\tbh=4310SqbfNGV+9nWEHOQUYctzp/ZvCZ1C7FF5jpVhkno=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=dw0owEnnKHSsuZF4kmvzZWhw1Qa0awzRbdyswsVRGEKnOGuCo+7Oqu7qbvPrNiGOt\n\tTYCYW6FmdIWmub8tLSXn6mskVkHDWTRgaAFDMZbCN4HA0A6afIMsukjZ2o9wrPacQg\n\ttg5uHZGbfST01gikrpOQ7q4lP/3+qYZYss9xNRtI=","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Date":"Sun, 23 Aug 2020 00:29:13 +0300","Message-Id":"<20200822212913.28635-1-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.27.0","In-Reply-To":"<20200821144601.67860-6-email@uajain.com>","References":"<20200821144601.67860-6-email@uajain.com>","MIME-Version":"1.0","Subject":"[libcamera-devel] [PATCH v4.1 5/5] android: camera_hal_manager:\n\tSupport camera hotplug","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"},"content":"From: Umang Jain <email@uajain.com>\n\nExtend the support for camera hotplug from libcamera's CameraManager\nto CameraHalManager. Use camera module callbacks to let the framework\nknow about the hotplug events and change the status of cameras being\nhotplugged or unplugged via camera_device_status_change().\n\nIntroduce a map cameraIdsMap_ which book-keeps all cameras seen in the\npast by the CameraHalManager. If the camera is seen for the first time,\na new id is assigned to it. If the camera has been seen before by the\nmanager, its old id is reused. IDs for internal cameras start with\n'0' and for external cameras, they start with '1000'. Accesses to\ncameraIdsMap_ and cameras_ are protected by a mutex.\n\nSigned-off-by: Umang Jain <email@uajain.com>\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\nChanges since v4:\n\n- Rename cameraDeviceFromHALId() to cameraDeviceFromHalId()\n- Return a CameraDevice from cameraDeviceFromHalId()\n- Inline cameraDeviceFromCamera() in its only caller\n- Drop duplicate location check\n- Drop cameraDeviceIter type\n- Add firstExternalCameraId_\n- Make Mutex and MutexLocker private member types\n- Replace camerasMap_ with cameraIdsMap_ in commit message\n- Reflow some comments\n\nI've applied these changes locally during review of v4 to compile-test\nmy comments, so I'm posting them to avoid duplicating work.\n---\n src/android/camera_hal_manager.cpp | 168 ++++++++++++++++++++++++-----\n src/android/camera_hal_manager.h   |  19 ++++\n 2 files changed, 163 insertions(+), 24 deletions(-)","diff":"diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp\nindex 3a744af5f231..a1805ebccf24 100644\n--- a/src/android/camera_hal_manager.cpp\n+++ b/src/android/camera_hal_manager.cpp\n@@ -8,6 +8,7 @@\n #include \"camera_hal_manager.h\"\n \n #include <libcamera/camera.h>\n+#include <libcamera/property_ids.h>\n \n #include \"libcamera/internal/log.h\"\n \n@@ -28,7 +29,8 @@ LOG_DECLARE_CATEGORY(HAL);\n  */\n \n CameraHalManager::CameraHalManager()\n-\t: cameraManager_(nullptr)\n+\t: cameraManager_(nullptr), numInternalCameras_(0),\n+\t  nextExternalCameraId_(firstExternalCameraId_)\n {\n }\n \n@@ -47,6 +49,10 @@ int CameraHalManager::init()\n {\n \tcameraManager_ = new CameraManager();\n \n+\t/* Support camera hotplug. */\n+\tcameraManager_->cameraAdded.connect(this, &CameraHalManager::cameraAdded);\n+\tcameraManager_->cameraRemoved.connect(this, &CameraHalManager::cameraRemoved);\n+\n \tint ret = cameraManager_->start();\n \tif (ret) {\n \t\tLOG(HAL, Error) << \"Failed to start camera manager: \"\n@@ -56,35 +62,20 @@ int CameraHalManager::init()\n \t\treturn ret;\n \t}\n \n-\t/*\n-\t * For each Camera registered in the system, a CameraDevice\n-\t * gets created here to wraps a libcamera Camera instance.\n-\t *\n-\t * \\todo Support camera hotplug.\n-\t */\n-\tunsigned int index = 0;\n-\tfor (auto &cam : cameraManager_->cameras()) {\n-\t\tstd::shared_ptr<CameraDevice> camera = CameraDevice::create(index, cam);\n-\t\tret = camera->initialize();\n-\t\tif (ret)\n-\t\t\tcontinue;\n-\n-\t\tcameras_.emplace_back(std::move(camera));\n-\t\t++index;\n-\t}\n-\n \treturn 0;\n }\n \n CameraDevice *CameraHalManager::open(unsigned int id,\n \t\t\t\t     const hw_module_t *hardwareModule)\n {\n-\tif (id >= numCameras()) {\n+\tMutexLocker locker(mutex_);\n+\n+\tCameraDevice *camera = cameraDeviceFromHalId(id);\n+\tif (!camera) {\n \t\tLOG(HAL, Error) << \"Invalid camera id '\" << id << \"'\";\n \t\treturn nullptr;\n \t}\n \n-\tCameraDevice *camera = cameras_[id].get();\n \tif (camera->open(hardwareModule))\n \t\treturn nullptr;\n \n@@ -93,9 +84,120 @@ CameraDevice *CameraHalManager::open(unsigned int id,\n \treturn camera;\n }\n \n+void CameraHalManager::cameraAdded(std::shared_ptr<Camera> cam)\n+{\n+\tunsigned int id;\n+\tbool isCameraExternal = false;\n+\tbool isCameraNew = false;\n+\n+\tMutexLocker locker(mutex_);\n+\n+\t/*\n+\t * Each camera is assigned a unique integer ID when it is seen for the\n+\t * first time. If the camera has been seen before, the previous ID is\n+\t * re-used.\n+\t *\n+\t * IDs starts from '0' for internal cameras and '1000' for external\n+\t * cameras.\n+\t */\n+\tauto iter = cameraIdsMap_.find(cam->id());\n+\tif (iter != cameraIdsMap_.end()) {\n+\t\tid = iter->second;\n+\t} else {\n+\t\tisCameraNew = true;\n+\n+\t\t/*\n+\t\t * Now check if this is an external camera and assign\n+\t\t * its id accordingly.\n+\t\t */\n+\t\tif (cameraLocation(cam.get()) == properties::CameraLocationExternal) {\n+\t\t\tisCameraExternal = true;\n+\t\t\tid = nextExternalCameraId_;\n+\t\t} else {\n+\t\t\tid = numInternalCameras_;\n+\t\t}\n+\t}\n+\n+\t/* Create a CameraDevice instance to wrap the libcamera Camera. */\n+\tstd::shared_ptr<CameraDevice> camera = CameraDevice::create(id, std::move(cam));\n+\tint ret = camera->initialize();\n+\tif (ret) {\n+\t\tLOG(HAL, Error) << \"Failed to initialize camera: \" << cam->id();\n+\t\treturn;\n+\t}\n+\n+\tif (isCameraNew) {\n+\t\tcameraIdsMap_.emplace(cam->id(), id);\n+\n+\t\tif (isCameraExternal)\n+\t\t\tnextExternalCameraId_++;\n+\t\telse\n+\t\t\tnumInternalCameras_++;\n+\t}\n+\n+\tcameras_.emplace_back(std::move(camera));\n+\n+\tif (callbacks_)\n+\t\tcallbacks_->camera_device_status_change(callbacks_, id,\n+\t\t\t\t\t\t\tCAMERA_DEVICE_STATUS_PRESENT);\n+\n+\tLOG(HAL, Debug) << \"Camera ID: \" << id << \" added successfully.\";\n+}\n+\n+void CameraHalManager::cameraRemoved(std::shared_ptr<Camera> cam)\n+{\n+\tMutexLocker locker(mutex_);\n+\n+\tauto iter = std::find_if(cameras_.begin(), cameras_.end(),\n+\t\t\t\t [&cam](std::shared_ptr<CameraDevice> &camera) {\n+\t\t\t\t\t return cam.get() == camera->camera();\n+\t\t\t\t });\n+\tif (iter == cameras_.end())\n+\t\treturn;\n+\n+\t/*\n+\t * CAMERA_DEVICE_STATUS_NOT_PRESENT should be set for external cameras\n+\t * only.\n+\t */\n+\tunsigned int id = (*iter)->id();\n+\tif (id >= firstExternalCameraId_)\n+\t\tcallbacks_->camera_device_status_change(callbacks_, id,\n+\t\t\t\t\t\t\tCAMERA_DEVICE_STATUS_NOT_PRESENT);\n+\n+\t/*\n+\t * \\todo Check if the camera is already open and running.\n+\t * Inform the framework about its absence before deleting its\n+\t * reference here.\n+\t */\n+\tcameras_.erase(iter);\n+\n+\tLOG(HAL, Debug) << \"Camera ID: \" << id << \" removed successfully.\";\n+}\n+\n+int32_t CameraHalManager::cameraLocation(const Camera *cam)\n+{\n+\tconst ControlList &properties = cam->properties();\n+\tif (!properties.contains(properties::Location))\n+\t\treturn -1;\n+\n+\treturn properties.get(properties::Location);\n+}\n+\n+CameraDevice *CameraHalManager::cameraDeviceFromHalId(unsigned int id)\n+{\n+\tauto iter = std::find_if(cameras_.begin(), cameras_.end(),\n+\t\t\t\t [id](std::shared_ptr<CameraDevice> &camera) {\n+\t\t\t\t\t return camera->id() == id;\n+\t\t\t\t });\n+\tif (iter == cameras_.end())\n+\t\treturn nullptr;\n+\n+\treturn iter->get();\n+}\n+\n unsigned int CameraHalManager::numCameras() const\n {\n-\treturn cameraManager_->cameras().size();\n+\treturn numInternalCameras_;\n }\n \n int CameraHalManager::getCameraInfo(unsigned int id, struct camera_info *info)\n@@ -103,13 +205,14 @@ int CameraHalManager::getCameraInfo(unsigned int id, struct camera_info *info)\n \tif (!info)\n \t\treturn -EINVAL;\n \n-\tif (id >= numCameras()) {\n+\tMutexLocker locker(mutex_);\n+\n+\tCameraDevice *camera = cameraDeviceFromHalId(id);\n+\tif (!camera) {\n \t\tLOG(HAL, Error) << \"Invalid camera id '\" << id << \"'\";\n \t\treturn -EINVAL;\n \t}\n \n-\tCameraDevice *camera = cameras_[id].get();\n-\n \tinfo->facing = camera->facing();\n \tinfo->orientation = camera->orientation();\n \tinfo->device_version = CAMERA_DEVICE_API_VERSION_3_3;\n@@ -124,4 +227,21 @@ int CameraHalManager::getCameraInfo(unsigned int id, struct camera_info *info)\n void CameraHalManager::setCallbacks(const camera_module_callbacks_t *callbacks)\n {\n \tcallbacks_ = callbacks;\n+\n+\tMutexLocker locker(mutex_);\n+\n+\t/*\n+\t * Some external cameras may have been identified before the callbacks_\n+\t * were set. Iterate all existing external cameras and mark them as\n+\t * CAMERA_DEVICE_STATUS_PRESENT explicitly.\n+\t *\n+\t * Internal cameras are already assumed to be present at module load\n+\t * time by the Android framework.\n+\t */\n+\tfor (std::shared_ptr<CameraDevice> &camera : cameras_) {\n+\t\tunsigned int id = camera->id();\n+\t\tif (id >= firstExternalCameraId_)\n+\t\t\tcallbacks_->camera_device_status_change(callbacks_, id,\n+\t\t\t\t\t\t\t\tCAMERA_DEVICE_STATUS_PRESENT);\n+\t}\n }\ndiff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h\nindex 3e34d63ff96c..a91decc7d8fe 100644\n--- a/src/android/camera_hal_manager.h\n+++ b/src/android/camera_hal_manager.h\n@@ -7,6 +7,8 @@\n #ifndef __ANDROID_CAMERA_MANAGER_H__\n #define __ANDROID_CAMERA_MANAGER_H__\n \n+#include <map>\n+#include <mutex>\n #include <stddef.h>\n #include <vector>\n \n@@ -33,10 +35,27 @@ public:\n \tvoid setCallbacks(const camera_module_callbacks_t *callbacks);\n \n private:\n+\tusing Mutex = std::mutex;\n+\tusing MutexLocker = std::unique_lock<std::mutex>;\n+\n+\tstatic constexpr unsigned int firstExternalCameraId_ = 1000;\n+\n+\tstatic int32_t cameraLocation(const libcamera::Camera *cam);\n+\n+\tvoid cameraAdded(std::shared_ptr<libcamera::Camera> cam);\n+\tvoid cameraRemoved(std::shared_ptr<libcamera::Camera> cam);\n+\n+\tCameraDevice *cameraDeviceFromHalId(unsigned int id);\n+\n \tlibcamera::CameraManager *cameraManager_;\n \n \tconst camera_module_callbacks_t *callbacks_;\n \tstd::vector<std::shared_ptr<CameraDevice>> cameras_;\n+\tstd::map<std::string, unsigned int> cameraIdsMap_;\n+\tMutex mutex_;\n+\n+\tunsigned int numInternalCameras_;\n+\tunsigned int nextExternalCameraId_;\n };\n \n #endif /* __ANDROID_CAMERA_MANAGER_H__ */\n","prefixes":["libcamera-devel","v4.1","5/5"]}