CameraService: Clean up availability listeners and HAL error codes

- Refactor where availability listeners are called to centralize behavior,
  ensuring that all client creation/destruction invokes the listeners
- Clean up some of the client hierarchy
- Filter error codes from key HAL calls to ensure proper reporting

Bug: 16514157
Bug: 16483222
Change-Id: I59875a865b6a508b47423946c78862da8df34cd1

diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp
index 648e82c..7766b90 100644 (file)
--- a/services/camera/libcameraservice/CameraService.cpp
+++ b/services/camera/libcameraservice/CameraService.cpp
@@ -235,7 +235,8 @@ status_t CameraService::getCameraInfo(int cameraId,
     }
 
     struct camera_info info;
-    status_t rc = mModule->get_camera_info(cameraId, &info);
+    status_t rc = filterGetInfoErrorCode(
+        mModule->get_camera_info(cameraId, &info));
     cameraInfo->facing = info.facing;
     cameraInfo->orientation = info.orientation;
     return rc;
@@ -367,7 +368,7 @@ status_t CameraService::getCameraCharacteristics(int cameraId,
          * Normal HAL 2.1+ codepath.
          */
         struct camera_info info;
-        ret = mModule->get_camera_info(cameraId, &info);
+        ret = filterGetInfoErrorCode(mModule->get_camera_info(cameraId, &info));
         *cameraInfo = info.static_camera_characteristics;
     }
 
@@ -404,23 +405,28 @@ int CameraService::getDeviceVersion(int cameraId, int* facing) {
     return deviceVersion;
 }
 
-bool CameraService::isValidCameraId(int cameraId) {
-    int facing;
-    int deviceVersion = getDeviceVersion(cameraId, &facing);
-
-    switch(deviceVersion) {
-      case CAMERA_DEVICE_API_VERSION_1_0:
-      case CAMERA_DEVICE_API_VERSION_2_0:
-      case CAMERA_DEVICE_API_VERSION_2_1:
-      case CAMERA_DEVICE_API_VERSION_3_0:
-      case CAMERA_DEVICE_API_VERSION_3_1:
-      case CAMERA_DEVICE_API_VERSION_3_2:
-        return true;
-      default:
-        return false;
+status_t CameraService::filterOpenErrorCode(status_t err) {
+    switch(err) {
+        case NO_ERROR:
+        case -EBUSY:
+        case -EINVAL:
+        case -EUSERS:
+            return err;
+        default:
+            break;
     }
+    return -ENODEV;
+}
 
-    return false;
+status_t CameraService::filterGetInfoErrorCode(status_t err) {
+    switch(err) {
+        case NO_ERROR:
+        case -EINVAL:
+            return err;
+        default:
+            break;
+    }
+    return -ENODEV;
 }
 
 bool CameraService::setUpVendorTags() {
@@ -665,14 +671,6 @@ status_t CameraService::connectHelperLocked(const sp<ICameraClient>& cameraClien
     int facing = -1;
     int deviceVersion = getDeviceVersion(cameraId, &facing);
 
-    // If there are other non-exclusive users of the camera,
-    //  this will tear them down before we can reuse the camera
-    if (isValidCameraId(cameraId)) {
-        // transition from PRESENT -> NOT_AVAILABLE
-        updateStatus(ICameraServiceListener::STATUS_NOT_AVAILABLE,
-                     cameraId);
-    }
-
     if (halVersion < 0 || halVersion == deviceVersion) {
         // Default path: HAL version is unspecified by caller, create CameraClient
         // based on device version reported by the HAL.
@@ -719,8 +717,6 @@ status_t CameraService::connectHelperLocked(const sp<ICameraClient>& cameraClien
     status_t status = connectFinishUnsafe(client, client->getRemote());
     if (status != OK) {
         // this is probably not recoverable.. maybe the client can try again
-        // OK: we can only get here if we were originally in PRESENT state
-        updateStatus(ICameraServiceListener::STATUS_PRESENT, cameraId);
         return status;
     }
 
@@ -970,14 +966,6 @@ status_t CameraService::connectDevice(
         int facing = -1;
         int deviceVersion = getDeviceVersion(cameraId, &facing);
 
-        // If there are other non-exclusive users of the camera,
-        //  this will tear them down before we can reuse the camera
-        if (isValidCameraId(cameraId)) {
-            // transition from PRESENT -> NOT_AVAILABLE
-            updateStatus(ICameraServiceListener::STATUS_NOT_AVAILABLE,
-                         cameraId);
-        }
-
         switch(deviceVersion) {
           case CAMERA_DEVICE_API_VERSION_1_0:
             ALOGW("Camera using old HAL version: %d", deviceVersion);
@@ -1002,8 +990,6 @@ status_t CameraService::connectDevice(
         status_t status = connectFinishUnsafe(client, client->getRemote());
         if (status != OK) {
             // this is probably not recoverable.. maybe the client can try again
-            // OK: we can only get here if we were originally in PRESENT state
-            updateStatus(ICameraServiceListener::STATUS_PRESENT, cameraId);
             return status;
         }
 
@@ -1427,13 +1413,15 @@ CameraService::BasicClient::~BasicClient() {
 void CameraService::BasicClient::disconnect() {
     ALOGV("BasicClient::disconnect");
     mCameraService->removeClientByRemote(mRemoteBinder);
+
+    finishCameraOps();
     // client shouldn't be able to call into us anymore
     mClientPid = 0;
 }
 
 status_t CameraService::BasicClient::startCameraOps() {
     int32_t res;
-
+    // Notify app ops that the camera is not available
     mOpsCallback = new OpsCallback(this);
 
     {
@@ -1451,16 +1439,39 @@ status_t CameraService::BasicClient::startCameraOps() {
                 mCameraId, String8(mClientPackageName).string());
         return PERMISSION_DENIED;
     }
+
     mOpsActive = true;
+
+    // Transition device availability listeners from PRESENT -> NOT_AVAILABLE
+    mCameraService->updateStatus(ICameraServiceListener::STATUS_NOT_AVAILABLE,
+            mCameraId);
+
     return OK;
 }
 
 status_t CameraService::BasicClient::finishCameraOps() {
+    // Check if startCameraOps succeeded, and if so, finish the camera op
     if (mOpsActive) {
+        // Notify app ops that the camera is available again
         mAppOpsManager.finishOp(AppOpsManager::OP_CAMERA, mClientUid,
                 mClientPackageName);
         mOpsActive = false;
+
+        // Notify device availability listeners that this camera is available
+        // again
+
+        StatusVector rejectSourceStates;
+        rejectSourceStates.push_back(ICameraServiceListener::STATUS_NOT_PRESENT);
+        rejectSourceStates.push_back(ICameraServiceListener::STATUS_ENUMERATING);
+
+        // Transition to PRESENT if the camera is not in either of above 2
+        // states
+        mCameraService->updateStatus(ICameraServiceListener::STATUS_PRESENT,
+                mCameraId,
+                &rejectSourceStates);
+
     }
+    // Always stop watching, even if no camera op is active
     mAppOpsManager.stopWatchingMode(mOpsCallback);
     mOpsCallback.clear();
 
@@ -1531,15 +1542,6 @@ void CameraService::Client::disconnect() {
     ALOGV("Client::disconnect");
     BasicClient::disconnect();
     mCameraService->setCameraFree(mCameraId);
-
-    StatusVector rejectSourceStates;
-    rejectSourceStates.push_back(ICameraServiceListener::STATUS_NOT_PRESENT);
-    rejectSourceStates.push_back(ICameraServiceListener::STATUS_ENUMERATING);
-
-    // Transition to PRESENT if the camera is not in either of above 2 states
-    mCameraService->updateStatus(ICameraServiceListener::STATUS_PRESENT,
-                                 mCameraId,
-                                 &rejectSourceStates);
 }
 
 CameraService::Client::OpsCallback::OpsCallback(wp<BasicClient> client):

diff --git a/services/camera/libcameraservice/CameraService.h b/services/camera/libcameraservice/CameraService.h
index 28590eb..cb98c96 100644 (file)
--- a/services/camera/libcameraservice/CameraService.h
+++ b/services/camera/libcameraservice/CameraService.h
@@ -138,6 +138,10 @@ public:
     // CameraDeviceFactory functionality
     int                 getDeviceVersion(int cameraId, int* facing = NULL);
 
+    /////////////////////////////////////////////////////////////////////
+    // Shared utilities
+    static status_t     filterOpenErrorCode(status_t err);
+    static status_t     filterGetInfoErrorCode(status_t err);
 
     /////////////////////////////////////////////////////////////////////
     // CameraClient functionality
@@ -149,20 +153,19 @@ public:
 
     class BasicClient : public virtual RefBase {
     public:
-        virtual status_t initialize(camera_module_t *module) = 0;
-
-        virtual void          disconnect() = 0;
+        virtual status_t    initialize(camera_module_t *module) = 0;
+        virtual void        disconnect();
 
         // because we can't virtually inherit IInterface, which breaks
         // virtual inheritance
         virtual sp<IBinder> asBinderWrapper() = 0;
 
         // Return the remote callback binder object (e.g. IProCameraCallbacks)
-        sp<IBinder>     getRemote() {
+        sp<IBinder>         getRemote() {
             return mRemoteBinder;
         }
 
-        virtual status_t      dump(int fd, const Vector<String16>& args) = 0;
+        virtual status_t    dump(int fd, const Vector<String16>& args) = 0;
 
     protected:
         BasicClient(const sp<CameraService>& cameraService,

diff --git a/services/camera/libcameraservice/common/Camera2ClientBase.cpp b/services/camera/libcameraservice/common/Camera2ClientBase.cpp
index 13c9f48..24d173c 100644 (file)
--- a/services/camera/libcameraservice/common/Camera2ClientBase.cpp
+++ b/services/camera/libcameraservice/common/Camera2ClientBase.cpp
@@ -112,8 +112,6 @@ Camera2ClientBase<TClientBase>::~Camera2ClientBase() {
 
     TClientBase::mDestructionStarted = true;
 
-    TClientBase::finishCameraOps();
-
     disconnect();
 
     ALOGI("Closed Camera %d", TClientBase::mCameraId);

diff --git a/services/camera/libcameraservice/device1/CameraHardwareInterface.h b/services/camera/libcameraservice/device1/CameraHardwareInterface.h
index 2746f6f..6386838 100644 (file)
--- a/services/camera/libcameraservice/device1/CameraHardwareInterface.h
+++ b/services/camera/libcameraservice/device1/CameraHardwareInterface.h
@@ -105,8 +105,8 @@ public:
                                                CAMERA_DEVICE_API_VERSION_1_0,
                                                (hw_device_t **)&mDevice);
         } else {
-            rc = module->methods->open(module, mName.string(),
-                                           (hw_device_t **)&mDevice);
+            rc = CameraService::filterOpenErrorCode(module->methods->open(
+                module, mName.string(), (hw_device_t **)&mDevice));
         }
         if (rc != OK) {
             ALOGE("Could not open camera %s: %d", mName.string(), rc);

diff --git a/services/camera/libcameraservice/device2/Camera2Device.cpp b/services/camera/libcameraservice/device2/Camera2Device.cpp
index 89c6b10..c1f77fa 100644 (file)
--- a/services/camera/libcameraservice/device2/Camera2Device.cpp
+++ b/services/camera/libcameraservice/device2/Camera2Device.cpp
@@ -30,6 +30,7 @@
 #include <utils/Trace.h>
 #include <utils/Timers.h>
 #include "Camera2Device.h"
+#include "CameraService.h"
 
 namespace android {
 
@@ -67,8 +68,8 @@ status_t Camera2Device::initialize(camera_module_t *module)
 
     camera2_device_t *device;
 
-    res = module->common.methods->open(&module->common, name,
-            reinterpret_cast<hw_device_t**>(&device));
+    res = CameraService::filterOpenErrorCode(module->common.methods->open(
+        &module->common, name, reinterpret_cast<hw_device_t**>(&device)));
 
     if (res != OK) {
         ALOGE("%s: Could not open camera %d: %s (%d)", __FUNCTION__,

diff --git a/services/camera/libcameraservice/device3/Camera3Device.cpp b/services/camera/libcameraservice/device3/Camera3Device.cpp
index 44e8822..a6214cc 100644 (file)
--- a/services/camera/libcameraservice/device3/Camera3Device.cpp
+++ b/services/camera/libcameraservice/device3/Camera3Device.cpp
@@ -48,6 +48,7 @@
 #include "device3/Camera3OutputStream.h"
 #include "device3/Camera3InputStream.h"
 #include "device3/Camera3ZslStream.h"
+#include "CameraService.h"
 
 using namespace android::camera3;
 
@@ -104,8 +105,9 @@ status_t Camera3Device::initialize(camera_module_t *module)
     camera3_device_t *device;
 
     ATRACE_BEGIN("camera3->open");
-    res = module->common.methods->open(&module->common, deviceName.string(),
-            reinterpret_cast<hw_device_t**>(&device));
+    res = CameraService::filterOpenErrorCode(module->common.methods->open(
+        &module->common, deviceName.string(),
+        reinterpret_cast<hw_device_t**>(&device)));
     ATRACE_END();
 
     if (res != OK) {
@@ -124,7 +126,8 @@ status_t Camera3Device::initialize(camera_module_t *module)
     }
 
     camera_info info;
-    res = module->get_camera_info(mId, &info);
+    res = CameraService::filterGetInfoErrorCode(module->get_camera_info(
+        mId, &info));
     if (res != OK) return res;
 
     if (info.device_version != device->common.version) {