camera2: Enable configuration failure to call #onConfigureFailed

Bug: 16629195
Change-Id: I0c365bc8f760466916dcc089217a43c43f9f4c9d

core/java/android/hardware/camera2/impl/CameraCaptureSessionImpl.java

@@ -68,6 +68,8 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
 
     /** This session is closed; all further calls will throw ISE */
     private boolean mClosed = false;
+    /** This session failed to be configured successfully */
+    private final boolean mConfigureSuccess;
     /** Do not unconfigure if this is set; another session will overwrite configuration */
     private boolean mSkipUnconfigure = false;
 
@@ -119,10 +121,12 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
         if (configureSuccess) {
             mStateListener.onConfigured(this);
             if (VERBOSE) Log.v(TAG, "ctor - Created session successfully");
+            mConfigureSuccess = true;
         } else {
             mStateListener.onConfigureFailed(this);
             mClosed = true; // do not fire any other callbacks, do not allow any other work
             Log.e(TAG, "Failed to create capture session; configuration failed");
+            mConfigureSuccess = false;
         }
     }
 
@@ -285,9 +289,9 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
         // - This session is active, so close() below starts the shutdown drain
         // - This session is mid-shutdown drain, and hasn't yet reached the idle drain listener.
         // - This session is already closed and has executed the idle drain listener, and
-        //   configureOutputs(null) has already been called.
+        //   configureOutputsChecked(null) has already been called.
         //
-        // Do not call configureOutputs(null) going forward, since it would race with the
+        // Do not call configureOutputsChecked(null) going forward, since it would race with the
         // configuration for the new session. If it was already called, then we don't care, since it
         // won't get called again.
         mSkipUnconfigure = true;
@@ -506,7 +510,7 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
             public void onUnconfigured(CameraDevice camera) {
                 synchronized (session) {
                     // Ignore #onUnconfigured before #close is called
-                    if (mClosed) {
+                    if (mClosed && mConfigureSuccess) {
                         mUnconfigureDrainer.taskFinished();
                     }
                 }
@@ -619,7 +623,7 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
 
                 try {
                     mUnconfigureDrainer.taskStarted();
-                    mDeviceImpl.configureOutputs(null); // begin transition to unconfigured state
+                    mDeviceImpl.configureOutputsChecked(null); // begin transition to unconfigured
                 } catch (CameraAccessException e) {
                     // OK: do not throw checked exceptions.
                     Log.e(TAG, "Exception while configuring outputs: ", e);

core/java/android/hardware/camera2/impl/CameraDeviceImpl.java

@@ -312,10 +312,33 @@ public class CameraDeviceImpl extends CameraDevice {
     }
 
     public void configureOutputs(List<Surface> outputs) throws CameraAccessException {
+        // Leave this here for backwards compatibility with older code using this directly
+        configureOutputsChecked(outputs);
+    }
+
+    /**
+     * Attempt to configure the outputs; the device goes to idle and then configures the
+     * new outputs if possible.
+     *
+     * <p>The configuration may gracefully fail, if there are too many outputs, if the formats
+     * are not supported, or if the sizes for that format is not supported. In this case this
+     * function will return {@code false} and the unconfigured callback will be fired.</p>
+     *
+     * <p>If the configuration succeeds (with 1 or more outputs), then the idle callback is fired.
+     * Unconfiguring the device always fires the idle callback.</p>
+     *
+     * @param outputs a list of one or more surfaces, or {@code null} to unconfigure
+     * @return whether or not the configuration was successful
+     *
+     * @throws CameraAccessException if there were any unexpected problems during configuration
+     */
+    public boolean configureOutputsChecked(List<Surface> outputs) throws CameraAccessException {
         // Treat a null input the same an empty list
         if (outputs == null) {
             outputs = new ArrayList<Surface>();
         }
+        boolean success = false;
+
         synchronized(mInterfaceLock) {
             checkIfCameraClosedOrInError();
 
@@ -355,7 +378,17 @@ public class CameraDeviceImpl extends CameraDevice {
                     mConfiguredOutputs.put(streamId, s);
                 }
 
-                mRemoteDevice.endConfigure();
+                try {
+                    mRemoteDevice.endConfigure();
+                }
+                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);
+                    return false;
+                }
+
+                success = true;
             } catch (CameraRuntimeException e) {
                 if (e.getReason() == CAMERA_IN_USE) {
                     throw new IllegalStateException("The camera is currently busy." +
@@ -365,15 +398,18 @@ public class CameraDeviceImpl extends CameraDevice {
                 throw e.asChecked();
             } catch (RemoteException e) {
                 // impossible
-                return;
-            }
-
-            if (outputs.size() > 0) {
-                mDeviceHandler.post(mCallOnIdle);
-            } else {
-                mDeviceHandler.post(mCallOnUnconfigured);
+                return false;
+            } finally {
+                if (success && outputs.size() > 0) {
+                    mDeviceHandler.post(mCallOnIdle);
+                } else {
+                    // Always return to the 'unconfigured' state if we didn't hit a fatal error
+                    mDeviceHandler.post(mCallOnUnconfigured);
+                }
             }
         }
+
+        return success;
     }
 
     @Override
@@ -397,7 +433,7 @@ public class CameraDeviceImpl extends CameraDevice {
             boolean configureSuccess = true;
             CameraAccessException pendingException = null;
             try {
-                configureOutputs(outputs); // and then block until IDLE
+                configureSuccess = configureOutputsChecked(outputs); // and then block until IDLE
             } catch (CameraAccessException e) {
                 configureSuccess = false;
                 pendingException = e;