OSDN Git Service

Merge "Camera2: Correct error handling" into lmp-dev
authorEino-Ville Talvala <etalvala@google.com>
Wed, 27 Aug 2014 18:47:30 +0000 (18:47 +0000)
committerAndroid (Google) Code Review <android-gerrit@google.com>
Wed, 27 Aug 2014 18:47:30 +0000 (18:47 +0000)
core/java/android/hardware/camera2/ICameraDeviceCallbacks.aidl
core/java/android/hardware/camera2/impl/CameraCaptureSessionImpl.java
core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
core/java/android/hardware/camera2/legacy/CameraDeviceUserShim.java
core/java/android/hardware/camera2/legacy/LegacyCameraDevice.java
media/tests/MediaFrameworkTest/src/com/android/mediaframeworktest/integration/CameraBinderTest.java
media/tests/MediaFrameworkTest/src/com/android/mediaframeworktest/integration/CameraDeviceBinderTest.java

index caabed3..ca0935c 100644 (file)
@@ -26,8 +26,8 @@ interface ICameraDeviceCallbacks
      * Keep up-to-date with frameworks/av/include/camera/camera2/ICameraDeviceCallbacks.h
      */
 
-    oneway void onCameraError(int errorCode, in CaptureResultExtras resultExtras);
-    oneway void onCameraIdle();
+    oneway void onDeviceError(int errorCode, in CaptureResultExtras resultExtras);
+    oneway void onDeviceIdle();
     oneway void onCaptureStarted(in CaptureResultExtras resultExtras, long timestamp);
     oneway void onResultReceived(in CameraMetadataNative result,
                                  in CaptureResultExtras resultExtras);
index 621968b..9ca1fba 100644 (file)
@@ -74,7 +74,7 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
     private boolean mSkipUnconfigure = false;
 
     /** Is the session in the process of aborting? Pay attention to BUSY->IDLE transitions. */
-    private boolean mAborting;
+    private volatile boolean mAborting;
 
     /**
      * Create a new CameraCaptureSession.
@@ -346,6 +346,20 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
     }
 
     /**
+     * Whether currently in mid-abort.
+     *
+     * <p>This is used by the implementation to set the capture failure
+     * reason, in lieu of more accurate error codes from the camera service.
+     * Unsynchronized to avoid deadlocks between simultaneous session->device,
+     * device->session calls.</p>
+     *
+     * <p>Package-private.</p>
+     */
+    boolean isAborting() {
+        return mAborting;
+    }
+
+    /**
      * Post calls into a CameraCaptureSession.StateListener to the user-specified {@code handler}.
      */
     private StateListener createUserStateListenerProxy(Handler handler, StateListener listener) {
@@ -502,8 +516,8 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
 
                 // TODO: Queue captures during abort instead of failing them
                 // since the app won't be able to distinguish the two actives
+                // Don't signal the application since there's no clean mapping here
                 Log.w(TAG, "Device is now busy; do not submit new captures (TODO: allow this)");
-                mStateListener.onActive(session);
             }
 
             @Override
index 79ce9df..513d222 100644 (file)
@@ -384,7 +384,7 @@ public class CameraDeviceImpl extends CameraDevice {
                 catch (IllegalArgumentException e) {
                     // OK. camera service can reject stream config if it's not supported by HAL
                     // This is only the result of a programmer misusing the camera2 api.
-                    Log.e(TAG, "Stream configuration failed", e);
+                    Log.w(TAG, "Stream configuration failed");
                     return false;
                 }
 
@@ -1097,31 +1097,51 @@ public class CameraDeviceImpl extends CameraDevice {
          */
         static final int ERROR_CAMERA_SERVICE = 2;
 
+        /**
+         * Camera has encountered an error processing a single request.
+         */
+        static final int ERROR_CAMERA_REQUEST = 3;
+
+        /**
+         * Camera has encountered an error producing metadata for a single capture
+         */
+        static final int ERROR_CAMERA_RESULT = 4;
+
+        /**
+         * Camera has encountered an error producing an image buffer for a single capture
+         */
+        static final int ERROR_CAMERA_BUFFER = 5;
+
         @Override
         public IBinder asBinder() {
             return this;
         }
 
         @Override
-        public void onCameraError(final int errorCode, CaptureResultExtras resultExtras) {
-            Runnable r = null;
+        public void onDeviceError(final int errorCode, CaptureResultExtras resultExtras) {
+            if (DEBUG) {
+                Log.d(TAG, String.format(
+                    "Device error received, code %d, frame number %d, request ID %d, subseq ID %d",
+                    errorCode, resultExtras.getFrameNumber(), resultExtras.getRequestId(),
+                    resultExtras.getSubsequenceId()));
+            }
 
             synchronized(mInterfaceLock) {
                 if (mRemoteDevice == null) {
                     return; // Camera already closed
                 }
 
-                mInError = true;
                 switch (errorCode) {
                     case ERROR_CAMERA_DISCONNECTED:
-                        r = mCallOnDisconnected;
+                        CameraDeviceImpl.this.mDeviceHandler.post(mCallOnDisconnected);
                         break;
                     default:
                         Log.e(TAG, "Unknown error from camera device: " + errorCode);
                         // no break
                     case ERROR_CAMERA_DEVICE:
                     case ERROR_CAMERA_SERVICE:
-                        r = new Runnable() {
+                        mInError = true;
+                        Runnable r = new Runnable() {
                             @Override
                             public void run() {
                                 if (!CameraDeviceImpl.this.isClosed()) {
@@ -1129,21 +1149,19 @@ public class CameraDeviceImpl extends CameraDevice {
                                 }
                             }
                         };
+                        CameraDeviceImpl.this.mDeviceHandler.post(r);
+                        break;
+                    case ERROR_CAMERA_REQUEST:
+                    case ERROR_CAMERA_RESULT:
+                    case ERROR_CAMERA_BUFFER:
+                        onCaptureErrorLocked(errorCode, resultExtras);
                         break;
                 }
-                CameraDeviceImpl.this.mDeviceHandler.post(r);
-
-                // Fire onCaptureSequenceCompleted
-                if (DEBUG) {
-                    Log.v(TAG, String.format("got error frame %d", resultExtras.getFrameNumber()));
-                }
-                mFrameNumberTracker.updateTracker(resultExtras.getFrameNumber(), /*error*/true);
-                checkAndFireSequenceComplete();
             }
         }
 
         @Override
-        public void onCameraIdle() {
+        public void onDeviceIdle() {
             if (DEBUG) {
                 Log.d(TAG, "Camera now idle");
             }
@@ -1246,7 +1264,6 @@ public class CameraDeviceImpl extends CameraDevice {
 
                 final CaptureRequest request = holder.getRequest(resultExtras.getSubsequenceId());
 
-
                 Runnable resultDispatch = null;
 
                 // Either send a partial result or the final capture completed result
@@ -1290,11 +1307,67 @@ public class CameraDeviceImpl extends CameraDevice {
                 if (!isPartialResult) {
                     checkAndFireSequenceComplete();
                 }
+            }
+        }
 
+        /**
+         * Called by onDeviceError for handling single-capture failures.
+         */
+        private void onCaptureErrorLocked(int errorCode, CaptureResultExtras resultExtras) {
+
+            final int requestId = resultExtras.getRequestId();
+            final int subsequenceId = resultExtras.getSubsequenceId();
+            final long frameNumber = resultExtras.getFrameNumber();
+            final CaptureListenerHolder holder =
+                    CameraDeviceImpl.this.mCaptureListenerMap.get(requestId);
+
+            final CaptureRequest request = holder.getRequest(subsequenceId);
+
+            // No way to report buffer errors right now
+            if (errorCode == ERROR_CAMERA_BUFFER) {
+                Log.e(TAG, String.format("Lost output buffer reported for frame %d", frameNumber));
+                return;
+            }
+
+            boolean mayHaveBuffers = (errorCode == ERROR_CAMERA_RESULT);
+
+            // This is only approximate - exact handling needs the camera service and HAL to
+            // disambiguate between request failures to due abort and due to real errors.
+            // For now, assume that if the session believes we're mid-abort, then the error
+            // is due to abort.
+            int reason = (mCurrentSession != null && mCurrentSession.isAborting()) ?
+                    CaptureFailure.REASON_FLUSHED :
+                    CaptureFailure.REASON_ERROR;
+
+            final CaptureFailure failure = new CaptureFailure(
+                request,
+                reason,
+                /*dropped*/ mayHaveBuffers,
+                requestId,
+                frameNumber);
+
+            Runnable failureDispatch = new Runnable() {
+                @Override
+                public void run() {
+                    if (!CameraDeviceImpl.this.isClosed()){
+                        holder.getListener().onCaptureFailed(
+                            CameraDeviceImpl.this,
+                            request,
+                            failure);
+                    }
+                }
+            };
+            holder.getHandler().post(failureDispatch);
+
+            // Fire onCaptureSequenceCompleted if appropriate
+            if (DEBUG) {
+                Log.v(TAG, String.format("got error frame %d", frameNumber));
             }
+            mFrameNumberTracker.updateTracker(frameNumber, /*error*/true);
+            checkAndFireSequenceComplete();
         }
 
-    }
+    } // public class CameraDeviceCallbacks
 
     /**
      * Default handler management.
index 5cbf109..410934e 100644 (file)
@@ -211,7 +211,7 @@ public class CameraDeviceUserShim implements ICameraDeviceUser {
         }
 
         @Override
-        public void onCameraError(final int errorCode, final CaptureResultExtras resultExtras) {
+        public void onDeviceError(final int errorCode, final CaptureResultExtras resultExtras) {
             Message msg = getHandler().obtainMessage(CAMERA_ERROR,
                 /*arg1*/ errorCode, /*arg2*/ 0,
                 /*obj*/ resultExtras);
@@ -219,7 +219,7 @@ public class CameraDeviceUserShim implements ICameraDeviceUser {
         }
 
         @Override
-        public void onCameraIdle() {
+        public void onDeviceIdle() {
             Message msg = getHandler().obtainMessage(CAMERA_IDLE);
             getHandler().sendMessage(msg);
         }
@@ -267,11 +267,11 @@ public class CameraDeviceUserShim implements ICameraDeviceUser {
                         case CAMERA_ERROR: {
                             int errorCode = msg.arg1;
                             CaptureResultExtras resultExtras = (CaptureResultExtras) msg.obj;
-                            mCallbacks.onCameraError(errorCode, resultExtras);
+                            mCallbacks.onDeviceError(errorCode, resultExtras);
                             break;
                         }
                         case CAMERA_IDLE:
-                            mCallbacks.onCameraIdle();
+                            mCallbacks.onDeviceIdle();
                             break;
                         case CAPTURE_STARTED: {
                             long timestamp = msg.arg2 & 0xFFFFFFFFL;
index 1cf7797..ffc55f1 100644 (file)
@@ -97,7 +97,7 @@ public class LegacyCameraDevice implements AutoCloseable {
                         Log.d(TAG, "doing onError callback.");
                     }
                     try {
-                        mDeviceCallbacks.onCameraError(errorCode, extras);
+                        mDeviceCallbacks.onDeviceError(errorCode, extras);
                     } catch (RemoteException e) {
                         throw new IllegalStateException(
                                 "Received remote exception during onCameraError callback: ", e);
@@ -125,7 +125,7 @@ public class LegacyCameraDevice implements AutoCloseable {
                         Log.d(TAG, "doing onIdle callback.");
                     }
                     try {
-                        mDeviceCallbacks.onCameraIdle();
+                        mDeviceCallbacks.onDeviceIdle();
                     } catch (RemoteException e) {
                         throw new IllegalStateException(
                                 "Received remote exception during onCameraIdle callback: ", e);
index b6bb578..cc50c43 100644 (file)
@@ -245,7 +245,7 @@ public class CameraBinderTest extends AndroidTestCase {
          * android.hardware.camera2.CaptureResultExtras)
          */
         @Override
-        public void onCameraError(int errorCode, CaptureResultExtras resultExtras)
+        public void onDeviceError(int errorCode, CaptureResultExtras resultExtras)
                 throws RemoteException {
             // TODO Auto-generated method stub
 
@@ -283,7 +283,7 @@ public class CameraBinderTest extends AndroidTestCase {
          * @see android.hardware.camera2.ICameraDeviceCallbacks#onCameraIdle()
          */
         @Override
-        public void onCameraIdle() throws RemoteException {
+        public void onDeviceIdle() throws RemoteException {
             // TODO Auto-generated method stub
 
         }
index 7b2e7dd..3cae19d 100644 (file)
@@ -88,10 +88,10 @@ public class CameraDeviceBinderTest extends AndroidTestCase {
         /*
          * (non-Javadoc)
          * @see
-         * android.hardware.camera2.ICameraDeviceCallbacks#onCameraError(int,
+         * android.hardware.camera2.ICameraDeviceCallbacks#onDeviceError(int,
          * android.hardware.camera2.CaptureResultExtras)
          */
-        public void onCameraError(int errorCode, CaptureResultExtras resultExtras)
+        public void onDeviceError(int errorCode, CaptureResultExtras resultExtras)
                 throws RemoteException {
             // TODO Auto-generated method stub
 
@@ -99,9 +99,9 @@ public class CameraDeviceBinderTest extends AndroidTestCase {
 
         /*
          * (non-Javadoc)
-         * @see android.hardware.camera2.ICameraDeviceCallbacks#onCameraIdle()
+         * @see android.hardware.camera2.ICameraDeviceCallbacks#onDeviceIdle()
          */
-        public void onCameraIdle() throws RemoteException {
+        public void onDeviceIdle() throws RemoteException {
             // TODO Auto-generated method stub
 
         }
@@ -432,7 +432,7 @@ public class CameraDeviceBinderTest extends AndroidTestCase {
         // Cancel and make sure we eventually quiesce
         status = mCameraUser.cancelRequest(streamingId, null);
 
-        verify(mMockCb, timeout(WAIT_FOR_IDLE_TIMEOUT_MS).times(1)).onCameraIdle();
+        verify(mMockCb, timeout(WAIT_FOR_IDLE_TIMEOUT_MS).times(1)).onDeviceIdle();
 
         // Submit a few capture requests
         int requestId1 = submitCameraRequest(request, /* streaming */false);
@@ -442,7 +442,7 @@ public class CameraDeviceBinderTest extends AndroidTestCase {
         int requestId5 = submitCameraRequest(request, /* streaming */false);
 
         // And wait for more idle
-        verify(mMockCb, timeout(WAIT_FOR_IDLE_TIMEOUT_MS).times(2)).onCameraIdle();
+        verify(mMockCb, timeout(WAIT_FOR_IDLE_TIMEOUT_MS).times(2)).onDeviceIdle();
 
     }
 
@@ -472,7 +472,7 @@ public class CameraDeviceBinderTest extends AndroidTestCase {
         status = mCameraUser.flush(null);
         assertEquals(CameraBinderTestUtils.NO_ERROR, status);
 
-        verify(mMockCb, timeout(WAIT_FOR_FLUSH_TIMEOUT_MS).times(1)).onCameraIdle();
+        verify(mMockCb, timeout(WAIT_FOR_FLUSH_TIMEOUT_MS).times(1)).onDeviceIdle();
 
         // Now a streaming request
         int streamingId = submitCameraRequest(request, /* streaming */true);
@@ -484,7 +484,7 @@ public class CameraDeviceBinderTest extends AndroidTestCase {
         status = mCameraUser.flush(null);
         assertEquals(CameraBinderTestUtils.NO_ERROR, status);
 
-        verify(mMockCb, timeout(WAIT_FOR_FLUSH_TIMEOUT_MS).times(2)).onCameraIdle();
+        verify(mMockCb, timeout(WAIT_FOR_FLUSH_TIMEOUT_MS).times(2)).onDeviceIdle();
 
         // TODO: When errors are hooked up, count that errors + successful
         // requests equal to 5.