From 6034bf5f21c57f66f3307d7934bc5c7616d2acf3 Mon Sep 17 00:00:00 2001
From: Guennadi Liakhovetski <guennadi.liakhovetski@intel.com>
Date: Thu, 7 Dec 2017 10:28:29 +0100
Subject: [PATCH] CameraService: fix camera removal paths

Currently the camera subsystem is trying to store all cameras, that
have ever been registered with it by camera HALs. This makes it
easier for the framework, but with hotpluggable cameras it makes
little sense, because for this HALs also have to store all cameras,
that have ever been plugged in and with every new plug in event
identify, whether this is a new camera or a known one. An easier and
cleaner approach is to remove cameras upon unplug. This patch
implements that.

Change-Id: Ie38cad59449386351518655e723e3f826a2ec826
Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@intel.com>
---
 camera/ndk/impl/ACameraManager.cpp                   |  3 +++
 services/camera/libcameraservice/CameraService.cpp   | 12 ++++++++++++
 services/camera/libcameraservice/CameraService.h     |  3 ++-
 .../common/CameraProviderManager.cpp                 | 11 +++++++++++
 .../libcameraservice/common/CameraProviderManager.h  |  2 ++
 5 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/camera/ndk/impl/ACameraManager.cpp b/camera/ndk/impl/ACameraManager.cpp
index 3f64bccdb1..a1a8cd6d1b 100644
--- a/camera/ndk/impl/ACameraManager.cpp
+++ b/camera/ndk/impl/ACameraManager.cpp
@@ -340,6 +340,9 @@ void CameraManagerGlobal::onStatusChangedLocked(
         msg->setString(kCameraIdKey, AString(cameraId));
         msg->post();
     }
+    if (status == hardware::ICameraServiceListener::STATUS_NOT_PRESENT) {
+        mDeviceStatusMap.erase(cameraId);
+    }
 }
 
 } // namespace android
diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp
index 6abfa81e73..514d37ab29 100644
--- a/services/camera/libcameraservice/CameraService.cpp
+++ b/services/camera/libcameraservice/CameraService.cpp
@@ -306,6 +306,17 @@ void CameraService::addStates(const String8 id) {
     logDeviceAdded(id, "Device added");
 }
 
+void CameraService::removeStates(const String8 id) {
+    if (mFlashlight->hasFlashUnit(id)) {
+        mTorchStatusMap.removeItem(id);
+    }
+
+    {
+        Mutex::Autolock lock(mCameraStatesLock);
+        mCameraStates.erase(id);
+    }
+}
+
 void CameraService::onDeviceStatusChanged(const String8& id,
         CameraDeviceStatus newHalStatus) {
     ALOGI("%s: Status changed for cameraId=%s, newStatus=%d", __FUNCTION__,
@@ -370,6 +381,7 @@ void CameraService::onDeviceStatusChanged(const String8& id,
             clientToDisconnect->disconnect();
         }
 
+        removeStates(id);
     } else {
         if (oldStatus == StatusInternal::NOT_PRESENT) {
             logDeviceAdded(id, String8::format("Device status changed from %d to %d", oldStatus,
diff --git a/services/camera/libcameraservice/CameraService.h b/services/camera/libcameraservice/CameraService.h
index e9373a6489..12ca372c4e 100644
--- a/services/camera/libcameraservice/CameraService.h
+++ b/services/camera/libcameraservice/CameraService.h
@@ -512,8 +512,9 @@ private:
     // Eumerate all camera providers in the system
     status_t enumerateProviders();
 
-    // Add a new camera to camera and torch state lists
+    // Add a new camera to camera and torch state lists or remove an unplugged one
     void addStates(const String8 id);
+    void removeStates(const String8 id);
 
     // Check if we can connect, before we acquire the service lock.
     // The returned originalClientPid is the PID of the original process that wants to connect to
diff --git a/services/camera/libcameraservice/common/CameraProviderManager.cpp b/services/camera/libcameraservice/common/CameraProviderManager.cpp
index ae3bbc15c5..b3d1132e8d 100644
--- a/services/camera/libcameraservice/common/CameraProviderManager.cpp
+++ b/services/camera/libcameraservice/common/CameraProviderManager.cpp
@@ -601,6 +601,15 @@ status_t CameraProviderManager::ProviderInfo::addDevice(const std::string& name,
     return OK;
 }
 
+void CameraProviderManager::ProviderInfo::removeDevice(std::string id) {
+    for (auto it = mDevices.begin(); it != mDevices.end(); it++) {
+        if ((*it)->mId == id) {
+            mDevices.erase(it);
+            break;
+        }
+    }
+}
+
 status_t CameraProviderManager::ProviderInfo::dump(int fd, const Vector<String16>&) const {
     dprintf(fd, "== Camera Provider HAL %s (v2.4, %s) static info: %zu devices: ==\n",
             mProviderName.c_str(), mInterface->isRemote() ? "remote" : "passthrough",
@@ -674,6 +683,8 @@ hardware::Return<void> CameraProviderManager::ProviderInfo::cameraDeviceStatusCh
                 return hardware::Void();
             }
             addDevice(cameraDeviceName, newStatus, &id);
+        } else if (newStatus == CameraDeviceStatus::NOT_PRESENT) {
+            removeDevice(id);
         }
         listener = mManager->getStatusListener();
     }
diff --git a/services/camera/libcameraservice/common/CameraProviderManager.h b/services/camera/libcameraservice/common/CameraProviderManager.h
index e82282fed0..b14a2c6faa 100644
--- a/services/camera/libcameraservice/common/CameraProviderManager.h
+++ b/services/camera/libcameraservice/common/CameraProviderManager.h
@@ -384,6 +384,8 @@ private:
 
         // Generate vendor tag id
         static metadata_vendor_id_t generateVendorTagId(const std::string &name);
+
+        void removeDevice(std::string id);
     };
 
     // Utility to find a DeviceInfo by ID; pointer is only valid while mInterfaceMutex is held