Refactor CameraService to handle errors properly.

Bug: 10361136

- Connect calls now return status_t error flags.

Change-Id: Ibce9ab047348cfcade7e70a2ef03f5a833e13af8

diff --git a/core/java/android/hardware/ICameraService.aidl b/core/java/android/hardware/ICameraService.aidl
index 81e564e..fc54828 100644 (file)
--- 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 0089f26..e08d1e6 100644 (file)
--- 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 1370654..b8ec4da 100644 (file)
--- 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 (file)
index 0000000..f39d645
--- /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 (file)
index 0000000..9eea390
--- /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<BinderHolder> CREATOR =
+             new Parcelable.Creator<BinderHolder>() {
+         @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 586c759..fbe7ff4 100644 (file)
--- 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 9057f60..624bbaa 100644 (file)
--- 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 bdf14ff..5225e23 100644 (file)
--- 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();