Camera: Make ProCamera connect take the same paths as Camera connect
* ProCamera uses the app ops manager
* Refactored connect calls to be as common as possible
* Removed some useless not implemented function calls in ProClient
Change-Id: I5dab30d20f0c202a494a07b2cfe4c1fa04a2a076
diff --git a/services/camera/libcameraservice/Camera2Client.cpp b/services/camera/libcameraservice/Camera2Client.cpp
index eb94d9f..056271d 100644
--- a/services/camera/libcameraservice/Camera2Client.cpp
+++ b/services/camera/libcameraservice/Camera2Client.cpp
@@ -81,27 +81,11 @@
ALOGV("%s: Initializing client for camera %d", __FUNCTION__, mCameraId);
status_t res;
- // Verify ops permissions
- res = startCameraOps();
+ res = Camera2ClientBase::initialize(module);
if (res != OK) {
return res;
}
- if (mDevice == NULL) {
- ALOGE("%s: Camera %d: No device connected",
- __FUNCTION__, mCameraId);
- return NO_INIT;
- }
-
- res = mDevice->initialize(module);
- if (res != OK) {
- ALOGE("%s: Camera %d: unable to initialize device: %s (%d)",
- __FUNCTION__, mCameraId, strerror(-res), res);
- return NO_INIT;
- }
-
- res = mDevice->setNotifyCallback(this);
-
SharedParameters::Lock l(mParameters);
res = l.mParameters.initialize(&(mDevice->info()));
diff --git a/services/camera/libcameraservice/Camera2ClientBase.cpp b/services/camera/libcameraservice/Camera2ClientBase.cpp
index e92ad1c..0623b89 100644
--- a/services/camera/libcameraservice/Camera2ClientBase.cpp
+++ b/services/camera/libcameraservice/Camera2ClientBase.cpp
@@ -76,6 +76,18 @@
TClientBase::mCameraId);
status_t res;
+ // Verify ops permissions
+ res = TClientBase::startCameraOps();
+ if (res != OK) {
+ return res;
+ }
+
+ if (mDevice == NULL) {
+ ALOGE("%s: Camera %d: No device connected",
+ __FUNCTION__, TClientBase::mCameraId);
+ return NO_INIT;
+ }
+
res = mDevice->initialize(module);
if (res != OK) {
ALOGE("%s: Camera %d: unable to initialize device: %s (%d)",
@@ -94,6 +106,8 @@
TClientBase::mDestructionStarted = true;
+ TClientBase::finishCameraOps();
+
disconnect();
ALOGI("Closed Camera %d", TClientBase::mCameraId);
@@ -157,7 +171,7 @@
detachDevice();
- TClientBase::disconnect();
+ CameraService::BasicClient::disconnect();
ALOGV("Camera %d: Shut down complete complete", TClientBase::mCameraId);
}
diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp
index d46ca88..7636143 100644
--- a/services/camera/libcameraservice/CameraService.cpp
+++ b/services/camera/libcameraservice/CameraService.cpp
@@ -176,18 +176,12 @@
return false;
}
-sp<ICamera> CameraService::connect(
- const sp<ICameraClient>& cameraClient,
- int cameraId,
- const String16& clientPackageName,
- int clientUid) {
+bool CameraService::validateConnect(int cameraId,
+ /*inout*/
+ int& clientUid) const {
- String8 clientName8(clientPackageName);
int callingPid = getCallingPid();
- LOG1("CameraService::connect E (pid %d \"%s\", id %d)", callingPid,
- clientName8.string(), cameraId);
-
if (clientUid == USE_CALLING_UID) {
clientUid = getCallingUid();
} else {
@@ -195,20 +189,19 @@
if (callingPid != getpid()) {
ALOGE("CameraService::connect X (pid %d) rejected (don't trust clientUid)",
callingPid);
- return NULL;
+ return false;
}
}
if (!mModule) {
ALOGE("Camera HAL module not loaded");
- return NULL;
+ return false;
}
- sp<Client> client;
if (cameraId < 0 || cameraId >= mNumberOfCameras) {
ALOGE("CameraService::connect X (pid %d) rejected (invalid cameraId %d).",
callingPid, cameraId);
- return NULL;
+ return false;
}
char value[PROPERTY_VALUE_MAX];
@@ -216,24 +209,32 @@
if (strcmp(value, "1") == 0) {
// Camera is disabled by DevicePolicyManager.
ALOGI("Camera is disabled. connect X (pid %d) rejected", callingPid);
- return NULL;
+ return false;
}
- Mutex::Autolock lock(mServiceLock);
+ return true;
+}
+
+bool CameraService::canConnectUnsafe(int cameraId,
+ const String16& clientPackageName,
+ const sp<IBinder>& remoteCallback,
+ sp<Client> &client) {
+ String8 clientName8(clientPackageName);
+ int callingPid = getCallingPid();
+
if (mClient[cameraId] != 0) {
client = mClient[cameraId].promote();
if (client != 0) {
- if (cameraClient->asBinder() ==
- client->getRemoteCallback()->asBinder()) {
-
+ if (remoteCallback == client->getRemoteCallback()->asBinder()) {
LOG1("CameraService::connect X (pid %d) (the same client)",
callingPid);
- return client;
+ return true;
} else {
- // TODOSC: need to support 1 regular client, multiple shared clients here
- ALOGW("CameraService::connect X (pid %d) rejected (existing client).",
- callingPid);
- return NULL;
+ // TODOSC: need to support 1 regular client,
+ // multiple shared clients here
+ ALOGW("CameraService::connect X (pid %d) rejected"
+ " (existing client).", callingPid);
+ return false;
}
}
mClient[cameraId].clear();
@@ -249,16 +250,47 @@
would be fine
*/
if (mBusy[cameraId]) {
-
ALOGW("CameraService::connect X (pid %d, \"%s\") rejected"
" (camera %d is still busy).", callingPid,
clientName8.string(), cameraId);
+ return false;
+ }
+
+ return true;
+}
+
+sp<ICamera> CameraService::connect(
+ const sp<ICameraClient>& cameraClient,
+ int cameraId,
+ const String16& clientPackageName,
+ int clientUid) {
+
+ String8 clientName8(clientPackageName);
+ int callingPid = getCallingPid();
+
+ LOG1("CameraService::connect E (pid %d \"%s\", id %d)", callingPid,
+ clientName8.string(), cameraId);
+
+ if (!validateConnect(cameraId, /*inout*/clientUid)) {
return NULL;
}
+ sp<Client> client;
+
+ Mutex::Autolock lock(mServiceLock);
+ if (!canConnectUnsafe(cameraId, clientPackageName,
+ cameraClient->asBinder(),
+ /*out*/client)) {
+ return NULL;
+ } else if (client.get() != NULL) {
+ return client;
+ }
+
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)) {
updateStatus(ICameraServiceListener::STATUS_NOT_AVAILABLE, cameraId);
}
@@ -285,21 +317,30 @@
return NULL;
}
- if (client->initialize(mModule) != OK) {
+ if (!connectFinishUnsafe(client, client->asBinder())) {
// this is probably not recoverable.. but maybe the client can try again
updateStatus(ICameraServiceListener::STATUS_AVAILABLE, cameraId);
return NULL;
}
- cameraClient->asBinder()->linkToDeath(this);
-
mClient[cameraId] = client;
LOG1("CameraService::connect X (id %d, this pid is %d)", cameraId, getpid());
return client;
}
+bool CameraService::connectFinishUnsafe(const sp<BasicClient>& client,
+ const sp<IBinder>& clientBinder) {
+ if (client->initialize(mModule) != OK) {
+ return false;
+ }
+
+ clientBinder->linkToDeath(this);
+
+ return true;
+}
+
sp<IProCameraUser> CameraService::connect(
const sp<IProCameraCallbacks>& cameraCb,
int cameraId,
@@ -309,38 +350,24 @@
String8 clientName8(clientPackageName);
int callingPid = getCallingPid();
- // TODO: use clientPackageName and clientUid with appOpsMangr
+ LOG1("CameraService::connectPro E (pid %d \"%s\", id %d)", callingPid,
+ clientName8.string(), cameraId);
- LOG1("CameraService::connectPro E (pid %d, id %d)", callingPid, cameraId);
-
- if (!mModule) {
- ALOGE("Camera HAL module not loaded");
+ if (!validateConnect(cameraId, /*inout*/clientUid)) {
return NULL;
}
+ Mutex::Autolock lock(mServiceLock);
+ {
+ sp<Client> client;
+ if (!canConnectUnsafe(cameraId, clientPackageName,
+ cameraCb->asBinder(),
+ /*out*/client)) {
+ return NULL;
+ }
+ }
+
sp<ProClient> client;
- if (cameraId < 0 || cameraId >= mNumberOfCameras) {
- ALOGE("CameraService::connectPro X (pid %d) rejected (invalid cameraId %d).",
- callingPid, cameraId);
- return NULL;
- }
-
- char value[PROPERTY_VALUE_MAX];
- property_get("sys.secpolicy.camera.disabled", value, "0");
- if (strcmp(value, "1") == 0) {
- // Camera is disabled by DevicePolicyManager.
- ALOGI("Camera is disabled. connect X (pid %d) rejected", callingPid);
- return NULL;
- }
-
- // TODO: allow concurrent connections with a ProCamera
- if (mBusy[cameraId]) {
-
- ALOGW("CameraService::connectPro X (pid %d, \"%s\") rejected"
- " (camera %d is still busy).", callingPid,
- clientName8.string(), cameraId);
- return NULL;
- }
int facing = -1;
int deviceVersion = getDeviceVersion(cameraId, &facing);
@@ -363,16 +390,15 @@
return NULL;
}
- if (client->initialize(mModule) != OK) {
+ if (!connectFinishUnsafe(client, client->asBinder())) {
return NULL;
}
mProClientList[cameraId].push(client);
- cameraCb->asBinder()->linkToDeath(this);
-
LOG1("CameraService::connectPro X (id %d, this pid is %d)", cameraId,
getpid());
+
return client;
}
@@ -654,7 +680,6 @@
mDestructionStarted = true;
mCameraService->releaseSound();
- finishCameraOps();
// unconditionally disconnect. function is idempotent
Client::disconnect();
}
@@ -691,6 +716,11 @@
mOpsCallback = new OpsCallback(this);
+ {
+ ALOGV("%s: Start camera ops, package name = %s, client UID = %d",
+ __FUNCTION__, String8(mClientPackageName).string(), mClientUid);
+ }
+
mAppOpsManager.startWatchingMode(AppOpsManager::OP_CAMERA,
mClientPackageName, mOpsCallback);
res = mAppOpsManager.startOp(AppOpsManager::OP_CAMERA,
@@ -812,79 +842,10 @@
}
CameraService::ProClient::~ProClient() {
- mDestructionStarted = true;
-
- ProClient::disconnect();
-}
-
-status_t CameraService::ProClient::connect(const sp<IProCameraCallbacks>& callbacks) {
- ALOGE("%s: not implemented yet", __FUNCTION__);
-
- return INVALID_OPERATION;
-}
-
-void CameraService::ProClient::disconnect() {
- BasicClient::disconnect();
-}
-
-status_t CameraService::ProClient::initialize(camera_module_t* module)
-{
- ALOGW("%s: not implemented yet", __FUNCTION__);
- return OK;
-}
-
-status_t CameraService::ProClient::exclusiveTryLock() {
- ALOGE("%s: not implemented yet", __FUNCTION__);
- return INVALID_OPERATION;
-}
-
-status_t CameraService::ProClient::exclusiveLock() {
- ALOGE("%s: not implemented yet", __FUNCTION__);
- return INVALID_OPERATION;
-}
-
-status_t CameraService::ProClient::exclusiveUnlock() {
- ALOGE("%s: not implemented yet", __FUNCTION__);
- return INVALID_OPERATION;
-}
-
-bool CameraService::ProClient::hasExclusiveLock() {
- ALOGE("%s: not implemented yet", __FUNCTION__);
- return false;
-}
-
-void CameraService::ProClient::onExclusiveLockStolen() {
- ALOGE("%s: not implemented yet", __FUNCTION__);
-}
-
-status_t CameraService::ProClient::submitRequest(camera_metadata_t* request, bool streaming) {
- ALOGE("%s: not implemented yet", __FUNCTION__);
-
- free_camera_metadata(request);
-
- return INVALID_OPERATION;
-}
-
-status_t CameraService::ProClient::cancelRequest(int requestId) {
- ALOGE("%s: not implemented yet", __FUNCTION__);
-
- return INVALID_OPERATION;
-}
-
-status_t CameraService::ProClient::requestStream(int streamId) {
- ALOGE("%s: not implemented yet", __FUNCTION__);
-
- return INVALID_OPERATION;
-}
-
-status_t CameraService::ProClient::cancelStream(int streamId) {
- ALOGE("%s: not implemented yet", __FUNCTION__);
-
- return INVALID_OPERATION;
}
void CameraService::ProClient::notifyError() {
- ALOGE("%s: not implemented yet", __FUNCTION__);
+ mRemoteCallback->notifyCallback(CAMERA_MSG_ERROR, CAMERA_ERROR_RELEASED, 0);
}
// ----------------------------------------------------------------------------
diff --git a/services/camera/libcameraservice/CameraService.h b/services/camera/libcameraservice/CameraService.h
index d7a336c..c5e495f 100644
--- a/services/camera/libcameraservice/CameraService.h
+++ b/services/camera/libcameraservice/CameraService.h
@@ -108,6 +108,7 @@
virtual void disconnect() = 0;
+ // Return the remote callback binder object (e.g. IProCameraCallbacks)
wp<IBinder> getRemote() {
return mRemoteBinder;
}
@@ -247,34 +248,24 @@
return mRemoteCallback;
}
- // BasicClient implementation
- virtual status_t initialize(camera_module_t *module);
-
/***
IProCamera implementation
***/
+ virtual status_t connect(const sp<IProCameraCallbacks>& callbacks)
+ = 0;
+ virtual status_t exclusiveTryLock() = 0;
+ virtual status_t exclusiveLock() = 0;
+ virtual status_t exclusiveUnlock() = 0;
-
- virtual status_t connect(
- const sp<IProCameraCallbacks>& callbacks);
- virtual void disconnect();
-
- virtual status_t exclusiveTryLock();
- virtual status_t exclusiveLock();
- virtual status_t exclusiveUnlock();
-
- virtual bool hasExclusiveLock();
+ virtual bool hasExclusiveLock() = 0;
// Note that the callee gets a copy of the metadata.
virtual int submitRequest(camera_metadata_t* metadata,
- bool streaming = false);
- virtual status_t cancelRequest(int requestId);
-
- virtual status_t requestStream(int streamId);
- virtual status_t cancelStream(int streamId);
+ bool streaming = false) = 0;
+ virtual status_t cancelRequest(int requestId) = 0;
// Callbacks from camera service
- virtual void onExclusiveLockStolen();
+ virtual void onExclusiveLockStolen() = 0;
protected:
virtual void notifyError();
@@ -287,6 +278,22 @@
// Delay-load the Camera HAL module
virtual void onFirstRef();
+ // Step 1. Check if we can connect, before we acquire the service lock.
+ bool validateConnect(int cameraId,
+ /*inout*/
+ int& clientUid) const;
+
+ // Step 2. Check if we can connect, after we acquire the service lock.
+ bool canConnectUnsafe(int cameraId,
+ const String16& clientPackageName,
+ const sp<IBinder>& remoteCallback,
+ /*out*/
+ sp<Client> &client);
+
+ // When connection is successful, initialize client and track its death
+ bool connectFinishUnsafe(const sp<BasicClient>& client,
+ const sp<IBinder>& clientBinder);
+
virtual sp<BasicClient> getClientByRemote(const wp<IBinder>& cameraClient);
Mutex mServiceLock;