From 66ef64514464a1bb9c7931993cc8db3d7539f39a Mon Sep 17 00:00:00 2001 From: Ruben Brunk Date: Thu, 8 Aug 2013 13:05:30 -0700 Subject: [PATCH] Refactor CameraService to handle errors properly. Bug: 10361136 - Connect calls now return status_t error flags. Change-Id: Ibce9ab047348cfcade7e70a2ef03f5a833e13af8 --- .../java/android/hardware/ICameraService.aidl | 19 +++-- .../camera2/CameraAccessException.java | 9 ++- .../hardware/camera2/CameraManager.java | 27 +++---- .../hardware/camera2/utils/BinderHolder.aidl | 20 +++++ .../hardware/camera2/utils/BinderHolder.java | 74 +++++++++++++++++++ .../camera2/utils/CameraBinderDecorator.java | 8 +- .../integration/CameraBinderTest.java | 26 ++++--- .../integration/CameraDeviceBinderTest.java | 11 ++- 8 files changed, 153 insertions(+), 41 deletions(-) create mode 100644 core/java/android/hardware/camera2/utils/BinderHolder.aidl create mode 100644 core/java/android/hardware/camera2/utils/BinderHolder.java diff --git a/core/java/android/hardware/ICameraService.aidl b/core/java/android/hardware/ICameraService.aidl index 81e564e16401c..fc54828c8c684 100644 --- a/core/java/android/hardware/ICameraService.aidl +++ b/core/java/android/hardware/ICameraService.aidl @@ -22,6 +22,7 @@ import android.hardware.IProCameraUser; import android.hardware.IProCameraCallbacks; import android.hardware.camera2.ICameraDeviceUser; import android.hardware.camera2.ICameraDeviceCallbacks; +import android.hardware.camera2.utils.BinderHolder; import android.hardware.ICameraServiceListener; import android.hardware.CameraInfo; @@ -37,17 +38,23 @@ interface ICameraService int getCameraInfo(int cameraId, out CameraInfo info); - ICamera connect(ICameraClient client, int cameraId, + int connect(ICameraClient client, int cameraId, String clientPackageName, - int clientUid); + int clientUid, + // Container for an ICamera object + out BinderHolder device); - IProCameraUser connectPro(IProCameraCallbacks callbacks, int cameraId, + int connectPro(IProCameraCallbacks callbacks, int cameraId, String clientPackageName, - int clientUid); + int clientUid, + // Container for an IProCameraUser object + out BinderHolder device); - ICameraDeviceUser connectDevice(ICameraDeviceCallbacks callbacks, int cameraId, + int connectDevice(ICameraDeviceCallbacks callbacks, int cameraId, String clientPackageName, - int clientUid); + int clientUid, + // Container for an ICameraDeviceUser object + out BinderHolder device); int addListener(ICameraServiceListener listener); int removeListener(ICameraServiceListener listener); diff --git a/core/java/android/hardware/camera2/CameraAccessException.java b/core/java/android/hardware/camera2/CameraAccessException.java index 0089f26af3252..e08d1e684eea6 100644 --- a/core/java/android/hardware/camera2/CameraAccessException.java +++ b/core/java/android/hardware/camera2/CameraAccessException.java @@ -48,11 +48,18 @@ public class CameraAccessException extends AndroidException { /** * The camera device is removable and has been disconnected from the Android - * device, or the camera service has shut down the connection due to a + * device, or the camera id used with {@link android.hardware.camera2.CameraManager#openCamera} + * is no longer valid, or the camera service has shut down the connection due to a * higher-priority access request for the camera device. */ public static final int CAMERA_DISCONNECTED = 4; + /** + * A deprecated HAL version is in use. + * @hide + */ + public static final int CAMERA_DEPRECATED_HAL = 1000; + // Make the eclipse warning about serializable exceptions go away private static final long serialVersionUID = 5630338637471475675L; // randomly generated diff --git a/core/java/android/hardware/camera2/CameraManager.java b/core/java/android/hardware/camera2/CameraManager.java index 13706541962b9..b8ec4da53747e 100644 --- a/core/java/android/hardware/camera2/CameraManager.java +++ b/core/java/android/hardware/camera2/CameraManager.java @@ -22,6 +22,7 @@ import android.hardware.ICameraServiceListener; import android.hardware.IProCameraUser; import android.hardware.camera2.utils.CameraBinderDecorator; import android.hardware.camera2.utils.CameraRuntimeException; +import android.hardware.camera2.utils.BinderHolder; import android.os.IBinder; import android.os.RemoteException; import android.os.ServiceManager; @@ -171,10 +172,10 @@ public final class CameraManager { * * @param cameraId The unique identifier of the camera device to open * - * @throws IllegalArgumentException if the cameraId does not match any - * currently connected camera device. * @throws CameraAccessException if the camera is disabled by device policy, - * or too many camera devices are already open. + * or too many camera devices are already open, or the cameraId does not match + * any currently available camera device. + * * @throws SecurityException if the application does not have permission to * access the camera * @@ -192,16 +193,11 @@ public final class CameraManager { android.hardware.camera2.impl.CameraDevice device = new android.hardware.camera2.impl.CameraDevice(cameraId); - cameraUser = mCameraService.connectDevice(device.getCallbacks(), + BinderHolder holder = new BinderHolder(); + mCameraService.connectDevice(device.getCallbacks(), Integer.parseInt(cameraId), - mContext.getPackageName(), USE_CALLING_UID); - - // TODO: change ICameraService#connectDevice to return status_t - if (cameraUser == null) { - // TEMPORARY CODE. - // catch-all exception since we aren't yet getting the actual error code - throw new IllegalStateException("Failed to open camera device"); - } + mContext.getPackageName(), USE_CALLING_UID, holder); + cameraUser = ICameraDeviceUser.Stub.asInterface(holder.getBinder()); // TODO: factor out listener to be non-nested, then move setter to constructor device.setRemoteDevice(cameraUser); @@ -214,12 +210,7 @@ public final class CameraManager { throw new IllegalArgumentException("Expected cameraId to be numeric, but it was: " + cameraId); } catch (CameraRuntimeException e) { - if (e.getReason() == CameraAccessException.CAMERA_DISCONNECTED) { - throw new IllegalArgumentException("Invalid camera ID specified -- " + - "perhaps the camera was physically disconnected", e); - } else { - throw e.asChecked(); - } + throw e.asChecked(); } catch (RemoteException e) { // impossible return null; diff --git a/core/java/android/hardware/camera2/utils/BinderHolder.aidl b/core/java/android/hardware/camera2/utils/BinderHolder.aidl new file mode 100644 index 0000000000000..f39d645a88572 --- /dev/null +++ b/core/java/android/hardware/camera2/utils/BinderHolder.aidl @@ -0,0 +1,20 @@ +/* + * Copyright (C) 2013 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.hardware.camera2.utils; + +/** @hide */ +parcelable BinderHolder; diff --git a/core/java/android/hardware/camera2/utils/BinderHolder.java b/core/java/android/hardware/camera2/utils/BinderHolder.java new file mode 100644 index 0000000000000..9eea390427c68 --- /dev/null +++ b/core/java/android/hardware/camera2/utils/BinderHolder.java @@ -0,0 +1,74 @@ +/* + * Copyright (C) 2013 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.hardware.camera2.utils; + +import android.os.Parcel; +import android.os.Parcelable; +import android.os.IBinder; + +/** + * @hide + */ +public class BinderHolder implements Parcelable { + private IBinder mBinder = null; + + @Override + public int describeContents() { + return 0; + } + + @Override + public void writeToParcel(Parcel dest, int flags) { + dest.writeStrongBinder(mBinder); + } + + public void readFromParcel(Parcel src) { + mBinder = src.readStrongBinder(); + } + + public static final Parcelable.Creator CREATOR = + new Parcelable.Creator() { + @Override + public BinderHolder createFromParcel(Parcel in) { + return new BinderHolder(in); + } + + @Override + public BinderHolder[] newArray(int size) { + return new BinderHolder[size]; + } + }; + + public IBinder getBinder() { + return mBinder; + } + + public void setBinder(IBinder binder) { + mBinder = binder; + } + + public BinderHolder() {} + + public BinderHolder(IBinder binder) { + mBinder = binder; + } + + private BinderHolder(Parcel in) { + mBinder = in.readStrongBinder(); + } +} + diff --git a/core/java/android/hardware/camera2/utils/CameraBinderDecorator.java b/core/java/android/hardware/camera2/utils/CameraBinderDecorator.java index 586c759a5e159..fbe7ff42bbbf5 100644 --- a/core/java/android/hardware/camera2/utils/CameraBinderDecorator.java +++ b/core/java/android/hardware/camera2/utils/CameraBinderDecorator.java @@ -19,6 +19,7 @@ package android.hardware.camera2.utils; import static android.hardware.camera2.CameraAccessException.CAMERA_DISABLED; import static android.hardware.camera2.CameraAccessException.CAMERA_DISCONNECTED; import static android.hardware.camera2.CameraAccessException.CAMERA_IN_USE; +import static android.hardware.camera2.CameraAccessException.CAMERA_DEPRECATED_HAL; import android.os.DeadObjectException; import android.os.RemoteException; @@ -48,6 +49,7 @@ public class CameraBinderDecorator { public static final int EACCES = -13; public static final int EBUSY = -16; public static final int ENODEV = -19; + public static final int ENOTSUP = -129; private static class CameraBinderDecoratorListener implements Decorator.DecoratorListener { @@ -75,9 +77,6 @@ public class CameraBinderDecorator { case DEAD_OBJECT: UncheckedThrow.throwAnyException(new CameraRuntimeException( CAMERA_DISCONNECTED)); - // TODO: Camera service (native side) should return - // EACCES error - // when there's a policy manager disabled causing this case EACCES: UncheckedThrow.throwAnyException(new CameraRuntimeException( CAMERA_DISABLED)); @@ -87,6 +86,9 @@ public class CameraBinderDecorator { case ENODEV: UncheckedThrow.throwAnyException(new CameraRuntimeException( CAMERA_DISCONNECTED)); + case ENOTSUP: + UncheckedThrow.throwAnyException(new CameraRuntimeException( + CAMERA_DEPRECATED_HAL)); } /** diff --git a/media/tests/MediaFrameworkTest/src/com/android/mediaframeworktest/integration/CameraBinderTest.java b/media/tests/MediaFrameworkTest/src/com/android/mediaframeworktest/integration/CameraBinderTest.java index 9057f60b66606..624bbaa25f173 100644 --- a/media/tests/MediaFrameworkTest/src/com/android/mediaframeworktest/integration/CameraBinderTest.java +++ b/media/tests/MediaFrameworkTest/src/com/android/mediaframeworktest/integration/CameraBinderTest.java @@ -25,6 +25,8 @@ import android.hardware.IProCameraUser; import android.hardware.camera2.CameraMetadata; import android.hardware.camera2.ICameraDeviceCallbacks; import android.hardware.camera2.ICameraDeviceUser; +import android.hardware.camera2.utils.BinderHolder; +import android.hardware.camera2.utils.CameraBinderDecorator; import android.os.Binder; import android.os.IBinder; import android.os.RemoteException; @@ -109,9 +111,11 @@ public class CameraBinderTest extends AndroidTestCase { String clientPackageName = getContext().getPackageName(); - ICamera cameraUser = mUtils.getCameraService().connect(dummyCallbacks, cameraId, - clientPackageName, - CameraBinderTestUtils.USE_CALLING_UID); + BinderHolder holder = new BinderHolder(); + CameraBinderDecorator.newInstance(mUtils.getCameraService()) + .connect(dummyCallbacks, cameraId, clientPackageName, + CameraBinderTestUtils.USE_CALLING_UID, holder); + ICamera cameraUser = ICamera.Stub.asInterface(holder.getBinder()); assertNotNull(String.format("Camera %s was null", cameraId), cameraUser); Log.v(TAG, String.format("Camera %s connected", cameraId)); @@ -131,9 +135,11 @@ public class CameraBinderTest extends AndroidTestCase { String clientPackageName = getContext().getPackageName(); - IProCameraUser cameraUser = mUtils.getCameraService().connectPro(dummyCallbacks, - cameraId, - clientPackageName, CameraBinderTestUtils.USE_CALLING_UID); + BinderHolder holder = new BinderHolder(); + CameraBinderDecorator.newInstance(mUtils.getCameraService()) + .connectPro(dummyCallbacks, cameraId, + clientPackageName, CameraBinderTestUtils.USE_CALLING_UID, holder); + IProCameraUser cameraUser = IProCameraUser.Stub.asInterface(holder.getBinder()); assertNotNull(String.format("Camera %s was null", cameraId), cameraUser); Log.v(TAG, String.format("Camera %s connected", cameraId)); @@ -161,9 +167,11 @@ public class CameraBinderTest extends AndroidTestCase { String clientPackageName = getContext().getPackageName(); - ICameraDeviceUser cameraUser = mUtils.getCameraService().connectDevice(dummyCallbacks, - cameraId, - clientPackageName, CameraBinderTestUtils.USE_CALLING_UID); + BinderHolder holder = new BinderHolder(); + CameraBinderDecorator.newInstance(mUtils.getCameraService()) + .connectDevice(dummyCallbacks, cameraId, + clientPackageName, CameraBinderTestUtils.USE_CALLING_UID, holder); + ICameraDeviceUser cameraUser = ICameraDeviceUser.Stub.asInterface(holder.getBinder()); assertNotNull(String.format("Camera %s was null", cameraId), cameraUser); Log.v(TAG, String.format("Camera %s connected", cameraId)); diff --git a/media/tests/MediaFrameworkTest/src/com/android/mediaframeworktest/integration/CameraDeviceBinderTest.java b/media/tests/MediaFrameworkTest/src/com/android/mediaframeworktest/integration/CameraDeviceBinderTest.java index bdf14ff84a296..5225e235442b5 100644 --- a/media/tests/MediaFrameworkTest/src/com/android/mediaframeworktest/integration/CameraDeviceBinderTest.java +++ b/media/tests/MediaFrameworkTest/src/com/android/mediaframeworktest/integration/CameraDeviceBinderTest.java @@ -22,6 +22,7 @@ import android.hardware.camera2.CameraPropertiesKeys; import android.hardware.camera2.CaptureRequest; import android.hardware.camera2.ICameraDeviceCallbacks; import android.hardware.camera2.ICameraDeviceUser; +import android.hardware.camera2.utils.BinderHolder; import android.os.RemoteException; import android.test.AndroidTestCase; import android.test.suitebuilder.annotation.SmallTest; @@ -39,8 +40,8 @@ public class CameraDeviceBinderTest extends AndroidTestCase { private static String TAG = "CameraDeviceBinderTest"; // Number of streaming callbacks need to check. private static int NUM_CALLBACKS_CHECKED = 10; - // Wait for capture result timeout value: 1000ms - private final static int WAIT_FOR_COMPLETE_TIMEOUT_MS = 1000; + // Wait for capture result timeout value: 1500ms + private final static int WAIT_FOR_COMPLETE_TIMEOUT_MS = 1500; private int mCameraId; private ICameraDeviceUser mCameraUser; @@ -129,8 +130,10 @@ public class CameraDeviceBinderTest extends AndroidTestCase { mMockCb = spy(dummyCallbacks); - mCameraUser = mUtils.getCameraService().connectDevice(mMockCb, mCameraId, - clientPackageName, CameraBinderTestUtils.USE_CALLING_UID); + BinderHolder holder = new BinderHolder(); + mUtils.getCameraService().connectDevice(mMockCb, mCameraId, + clientPackageName, CameraBinderTestUtils.USE_CALLING_UID, holder); + mCameraUser = ICameraDeviceUser.Stub.asInterface(holder.getBinder()); assertNotNull(String.format("Camera %s was null", mCameraId), mCameraUser); createDefaultSurface();