From f67e23ef637d0b53a0d4bebb68c654234df3da94 Mon Sep 17 00:00:00 2001 From: Eino-Ville Talvala Date: Wed, 23 Jul 2014 17:17:59 -0700 Subject: [PATCH] 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 --- services/camera/libcameraservice/CameraService.cpp | 96 +++++++++++----------- services/camera/libcameraservice/CameraService.h | 13 +-- .../libcameraservice/common/Camera2ClientBase.cpp | 2 - .../device1/CameraHardwareInterface.h | 4 +- .../libcameraservice/device2/Camera2Device.cpp | 5 +- .../libcameraservice/device3/Camera3Device.cpp | 9 +- 6 files changed, 68 insertions(+), 61 deletions(-) diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp index 648e82cdbd..7766b904c0 100644 --- 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& 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& 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 client): diff --git a/services/camera/libcameraservice/CameraService.h b/services/camera/libcameraservice/CameraService.h index 28590eb289..cb98c9628b 100644 --- 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 asBinderWrapper() = 0; // Return the remote callback binder object (e.g. IProCameraCallbacks) - sp getRemote() { + sp getRemote() { return mRemoteBinder; } - virtual status_t dump(int fd, const Vector& args) = 0; + virtual status_t dump(int fd, const Vector& args) = 0; protected: BasicClient(const sp& cameraService, diff --git a/services/camera/libcameraservice/common/Camera2ClientBase.cpp b/services/camera/libcameraservice/common/Camera2ClientBase.cpp index 13c9f48271..24d173cfeb 100644 --- a/services/camera/libcameraservice/common/Camera2ClientBase.cpp +++ b/services/camera/libcameraservice/common/Camera2ClientBase.cpp @@ -112,8 +112,6 @@ Camera2ClientBase::~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 2746f6f401..63868384f0 100644 --- 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 89c6b10e8a..c1f77fa55f 100644 --- a/services/camera/libcameraservice/device2/Camera2Device.cpp +++ b/services/camera/libcameraservice/device2/Camera2Device.cpp @@ -30,6 +30,7 @@ #include #include #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(&device)); + res = CameraService::filterOpenErrorCode(module->common.methods->open( + &module->common, name, reinterpret_cast(&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 44e88224b4..a6214ccc9e 100644 --- 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(&device)); + res = CameraService::filterOpenErrorCode(module->common.methods->open( + &module->common, deviceName.string(), + reinterpret_cast(&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) { -- 2.11.0