Camera2: Correct error handling
- Report capture failures from service to application
- Only go to error state for device-level errors
- Adjust binder interface method names to match the service side names
- Reduce failed session creation logging
- Don't fire CaptureSession.onActive for CameraDevice.onBusy
- Check with session to determine capture failure reason
Bug: 17160301
Bug: 15524101
Bug: 14448494
Bug: 11272459
Change-Id: I9dd606004fd7845910dc865738fbe17f1640f07d
diff --git a/core/java/android/hardware/camera2/ICameraDeviceCallbacks.aidl b/core/java/android/hardware/camera2/ICameraDeviceCallbacks.aidl
index caabed3..ca0935c 100644
--- a/core/java/android/hardware/camera2/ICameraDeviceCallbacks.aidl
+++ b/core/java/android/hardware/camera2/ICameraDeviceCallbacks.aidl
@@ -26,8 +26,8 @@
* 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);
diff --git a/core/java/android/hardware/camera2/impl/CameraCaptureSessionImpl.java b/core/java/android/hardware/camera2/impl/CameraCaptureSessionImpl.java
index 621968b..9ca1fba 100644
--- a/core/java/android/hardware/camera2/impl/CameraCaptureSessionImpl.java
+++ b/core/java/android/hardware/camera2/impl/CameraCaptureSessionImpl.java
@@ -74,7 +74,7 @@
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 @@
}
/**
+ * 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 @@
// 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
diff --git a/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java b/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
index 79ce9df..513d222 100644
--- a/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
+++ b/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java
@@ -384,7 +384,7 @@
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 @@
*/
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 @@
}
}
};
+ 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 @@
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 @@
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.
diff --git a/core/java/android/hardware/camera2/legacy/CameraDeviceUserShim.java b/core/java/android/hardware/camera2/legacy/CameraDeviceUserShim.java
index 5cbf109..410934e 100644
--- a/core/java/android/hardware/camera2/legacy/CameraDeviceUserShim.java
+++ b/core/java/android/hardware/camera2/legacy/CameraDeviceUserShim.java
@@ -211,7 +211,7 @@
}
@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 @@
}
@Override
- public void onCameraIdle() {
+ public void onDeviceIdle() {
Message msg = getHandler().obtainMessage(CAMERA_IDLE);
getHandler().sendMessage(msg);
}
@@ -267,11 +267,11 @@
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;
diff --git a/core/java/android/hardware/camera2/legacy/LegacyCameraDevice.java b/core/java/android/hardware/camera2/legacy/LegacyCameraDevice.java
index 1cf7797..ffc55f1 100644
--- a/core/java/android/hardware/camera2/legacy/LegacyCameraDevice.java
+++ b/core/java/android/hardware/camera2/legacy/LegacyCameraDevice.java
@@ -97,7 +97,7 @@
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 @@
Log.d(TAG, "doing onIdle callback.");
}
try {
- mDeviceCallbacks.onCameraIdle();
+ mDeviceCallbacks.onDeviceIdle();
} catch (RemoteException e) {
throw new IllegalStateException(
"Received remote exception during onCameraIdle callback: ", e);
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 b6bb578..cc50c43 100644
--- a/media/tests/MediaFrameworkTest/src/com/android/mediaframeworktest/integration/CameraBinderTest.java
+++ b/media/tests/MediaFrameworkTest/src/com/android/mediaframeworktest/integration/CameraBinderTest.java
@@ -245,7 +245,7 @@
* 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 @@
* @see android.hardware.camera2.ICameraDeviceCallbacks#onCameraIdle()
*/
@Override
- public void onCameraIdle() throws RemoteException {
+ public void onDeviceIdle() throws RemoteException {
// TODO Auto-generated method stub
}
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 7b2e7dd..3cae19d 100644
--- a/media/tests/MediaFrameworkTest/src/com/android/mediaframeworktest/integration/CameraDeviceBinderTest.java
+++ b/media/tests/MediaFrameworkTest/src/com/android/mediaframeworktest/integration/CameraDeviceBinderTest.java
@@ -88,10 +88,10 @@
/*
* (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 @@
/*
* (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 @@
// 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 @@
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 @@
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 @@
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.