Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5fa15be4 authored by Eino-Ville Talvala's avatar Eino-Ville Talvala Committed by Android (Google) Code Review
Browse files

Merge "Camera2: Clean up corner case error handling" into lmp-dev

parents b6eaa864 95037853
Loading
Loading
Loading
Loading
+56 −25
Original line number Original line Diff line number Diff line
@@ -42,6 +42,10 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
    private static final String TAG = "CameraCaptureSession";
    private static final String TAG = "CameraCaptureSession";
    private static final boolean VERBOSE = Log.isLoggable(TAG, Log.VERBOSE);
    private static final boolean VERBOSE = Log.isLoggable(TAG, Log.VERBOSE);


    /** Simple integer ID for session for debugging */
    private final int mId;
    private final String mIdString;

    /** User-specified set of surfaces used as the configuration outputs */
    /** User-specified set of surfaces used as the configuration outputs */
    private final List<Surface> mOutputs;
    private final List<Surface> mOutputs;
    /**
    /**
@@ -83,7 +87,7 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
     * There must be no pending actions
     * There must be no pending actions
     * (e.g. no pending captures, no repeating requests, no flush).</p>
     * (e.g. no pending captures, no repeating requests, no flush).</p>
     */
     */
    CameraCaptureSessionImpl(List<Surface> outputs,
    CameraCaptureSessionImpl(int id, List<Surface> outputs,
            CameraCaptureSession.StateCallback callback, Handler stateHandler,
            CameraCaptureSession.StateCallback callback, Handler stateHandler,
            android.hardware.camera2.impl.CameraDeviceImpl deviceImpl,
            android.hardware.camera2.impl.CameraDeviceImpl deviceImpl,
            Handler deviceStateHandler, boolean configureSuccess) {
            Handler deviceStateHandler, boolean configureSuccess) {
@@ -93,6 +97,9 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
            throw new IllegalArgumentException("callback must not be null");
            throw new IllegalArgumentException("callback must not be null");
        }
        }


        mId = id;
        mIdString = String.format("Session %d: ", mId);

        // TODO: extra verification of outputs
        // TODO: extra verification of outputs
        mOutputs = outputs;
        mOutputs = outputs;
        mStateHandler = checkHandler(stateHandler);
        mStateHandler = checkHandler(stateHandler);
@@ -120,12 +127,12 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {


        if (configureSuccess) {
        if (configureSuccess) {
            mStateCallback.onConfigured(this);
            mStateCallback.onConfigured(this);
            if (VERBOSE) Log.v(TAG, "ctor - Created session successfully");
            if (VERBOSE) Log.v(TAG, mIdString + "Created session successfully");
            mConfigureSuccess = true;
            mConfigureSuccess = true;
        } else {
        } else {
            mStateCallback.onConfigureFailed(this);
            mStateCallback.onConfigureFailed(this);
            mClosed = true; // do not fire any other callbacks, do not allow any other work
            mClosed = true; // do not fire any other callbacks, do not allow any other work
            Log.e(TAG, "Failed to create capture session; configuration failed");
            Log.e(TAG, mIdString + "Failed to create capture session; configuration failed");
            mConfigureSuccess = false;
            mConfigureSuccess = false;
        }
        }
    }
    }
@@ -147,8 +154,8 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
        handler = checkHandler(handler, callback);
        handler = checkHandler(handler, callback);


        if (VERBOSE) {
        if (VERBOSE) {
            Log.v(TAG, "capture - request " + request + ", callback " + callback + " handler" +
            Log.v(TAG, mIdString + "capture - request " + request + ", callback " + callback +
                    " " + handler);
                    " handler " + handler);
        }
        }


        return addPendingSequence(mDeviceImpl.capture(request,
        return addPendingSequence(mDeviceImpl.capture(request,
@@ -170,8 +177,8 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {


        if (VERBOSE) {
        if (VERBOSE) {
            CaptureRequest[] requestArray = requests.toArray(new CaptureRequest[0]);
            CaptureRequest[] requestArray = requests.toArray(new CaptureRequest[0]);
            Log.v(TAG, "captureBurst - requests " + Arrays.toString(requestArray) + ", callback " +
            Log.v(TAG, mIdString + "captureBurst - requests " + Arrays.toString(requestArray) +
                    callback + " handler" + "" + handler);
                    ", callback " + callback + " handler " + handler);
        }
        }


        return addPendingSequence(mDeviceImpl.captureBurst(requests,
        return addPendingSequence(mDeviceImpl.captureBurst(requests,
@@ -190,8 +197,8 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
        handler = checkHandler(handler, callback);
        handler = checkHandler(handler, callback);


        if (VERBOSE) {
        if (VERBOSE) {
            Log.v(TAG, "setRepeatingRequest - request " + request + ", callback " + callback +
            Log.v(TAG, mIdString + "setRepeatingRequest - request " + request + ", callback " +
                    " handler" + " " + handler);
                    callback + " handler" + " " + handler);
        }
        }


        return addPendingSequence(mDeviceImpl.setRepeatingRequest(request,
        return addPendingSequence(mDeviceImpl.setRepeatingRequest(request,
@@ -213,7 +220,7 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {


        if (VERBOSE) {
        if (VERBOSE) {
            CaptureRequest[] requestArray = requests.toArray(new CaptureRequest[0]);
            CaptureRequest[] requestArray = requests.toArray(new CaptureRequest[0]);
            Log.v(TAG, "setRepeatingBurst - requests " + Arrays.toString(requestArray) +
            Log.v(TAG, mIdString + "setRepeatingBurst - requests " + Arrays.toString(requestArray) +
                    ", callback " + callback + " handler" + "" + handler);
                    ", callback " + callback + " handler" + "" + handler);
        }
        }


@@ -226,7 +233,7 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
        checkNotClosed();
        checkNotClosed();


        if (VERBOSE) {
        if (VERBOSE) {
            Log.v(TAG, "stopRepeating");
            Log.v(TAG, mIdString + "stopRepeating");
        }
        }


        mDeviceImpl.stopRepeating();
        mDeviceImpl.stopRepeating();
@@ -237,11 +244,11 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
        checkNotClosed();
        checkNotClosed();


        if (VERBOSE) {
        if (VERBOSE) {
            Log.v(TAG, "abortCaptures");
            Log.v(TAG, mIdString + "abortCaptures");
        }
        }


        if (mAborting) {
        if (mAborting) {
            Log.w(TAG, "abortCaptures - Session is already aborting; doing nothing");
            Log.w(TAG, mIdString + "abortCaptures - Session is already aborting; doing nothing");
            return;
            return;
        }
        }


@@ -279,7 +286,7 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
         * but this would introduce nondeterministic behavior.
         * but this would introduce nondeterministic behavior.
         */
         */


        if (VERBOSE) Log.v(TAG, "replaceSessionClose");
        if (VERBOSE) Log.v(TAG, mIdString + "replaceSessionClose");


        // Set up fast shutdown. Possible alternative paths:
        // Set up fast shutdown. Possible alternative paths:
        // - This session is active, so close() below starts the shutdown drain
        // - This session is active, so close() below starts the shutdown drain
@@ -299,11 +306,11 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
    public synchronized void close() {
    public synchronized void close() {


        if (mClosed) {
        if (mClosed) {
            if (VERBOSE) Log.v(TAG, "close - reentering");
            if (VERBOSE) Log.v(TAG, mIdString + "close - reentering");
            return;
            return;
        }
        }


        if (VERBOSE) Log.v(TAG, "close - first time");
        if (VERBOSE) Log.v(TAG, mIdString + "close - first time");


        mClosed = true;
        mClosed = true;


@@ -321,7 +328,7 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
            mDeviceImpl.stopRepeating();
            mDeviceImpl.stopRepeating();
        } catch (IllegalStateException e) {
        } catch (IllegalStateException e) {
            // OK: Camera device may already be closed, nothing else to do
            // OK: Camera device may already be closed, nothing else to do
            Log.w(TAG, "The camera device was already closed: ", e);
            Log.w(TAG, mIdString + "The camera device was already closed: ", e);


            // TODO: Fire onClosed anytime we get the device onClosed or the ISE?
            // TODO: Fire onClosed anytime we get the device onClosed or the ISE?
            // or just suppress the ISE only and rely onClosed.
            // or just suppress the ISE only and rely onClosed.
@@ -332,7 +339,7 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
            return;
            return;
        } catch (CameraAccessException e) {
        } catch (CameraAccessException e) {
            // OK: close does not throw checked exceptions.
            // OK: close does not throw checked exceptions.
            Log.e(TAG, "Exception while stopping repeating: ", e);
            Log.e(TAG, mIdString + "Exception while stopping repeating: ", e);


            // TODO: call onError instead of onClosed if this happens
            // TODO: call onError instead of onClosed if this happens
        }
        }
@@ -453,13 +460,14 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {


            @Override
            @Override
            public void onDisconnected(CameraDevice camera) {
            public void onDisconnected(CameraDevice camera) {
                if (VERBOSE) Log.v(TAG, mIdString + "onDisconnected");
                close();
                close();
            }
            }


            @Override
            @Override
            public void onError(CameraDevice camera, int error) {
            public void onError(CameraDevice camera, int error) {
                // TODO: Handle errors somehow.
                // Should not be reached, handled by device code
                Log.wtf(TAG, "Got device error " + error);
                Log.wtf(TAG, mIdString + "Got device error " + error);
            }
            }


            @Override
            @Override
@@ -467,12 +475,15 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
                mIdleDrainer.taskStarted();
                mIdleDrainer.taskStarted();
                mActive = true;
                mActive = true;


                if (VERBOSE) Log.v(TAG, mIdString + "onActive");
                mStateCallback.onActive(session);
                mStateCallback.onActive(session);
            }
            }


            @Override
            @Override
            public void onIdle(CameraDevice camera) {
            public void onIdle(CameraDevice camera) {
                boolean isAborting;
                boolean isAborting;
                if (VERBOSE) Log.v(TAG, mIdString + "onIdle");

                synchronized (session) {
                synchronized (session) {
                    isAborting = mAborting;
                    isAborting = mAborting;
                }
                }
@@ -513,14 +524,29 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
                // TODO: Queue captures during abort instead of failing them
                // TODO: Queue captures during abort instead of failing them
                // since the app won't be able to distinguish the two actives
                // since the app won't be able to distinguish the two actives
                // Don't signal the application since there's no clean mapping here
                // 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)");
                if (VERBOSE) Log.v(TAG, mIdString + "onBusy");
            }
            }


            @Override
            @Override
            public void onUnconfigured(CameraDevice camera) {
            public void onUnconfigured(CameraDevice camera) {
                if (VERBOSE) Log.v(TAG, mIdString + "onUnconfigured");
                synchronized (session) {
                synchronized (session) {
                    // Ignore #onUnconfigured before #close is called
                    // Ignore #onUnconfigured before #close is called.
                    if (mClosed && mConfigureSuccess) {
                    //
                    // Normally, this is reached when this session is closed and no immediate other
                    // activity happens for the camera, in which case the camera is configured to
                    // null streams by this session and the UnconfigureDrainer task is started.
                    // However, we can also end up here if
                    //
                    // 1) Session is closed
                    // 2) New session is created before this session finishes closing, setting
                    //    mSkipUnconfigure and therefore this session does not configure null or
                    //    start the UnconfigureDrainer task.
                    // 3) And then the new session fails to be created, so onUnconfigured fires
                    //    _anyway_.
                    // In this second case, need to not finish a task that was never started, so
                    // guard with mSkipUnconfigure
                    if (mClosed && mConfigureSuccess && !mSkipUnconfigure) {
                        mUnconfigureDrainer.taskFinished();
                        mUnconfigureDrainer.taskFinished();
                    }
                    }
                }
                }
@@ -580,6 +606,7 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
             * If the camera is already "IDLE" and no aborts are pending,
             * If the camera is already "IDLE" and no aborts are pending,
             * then the drain immediately finishes.
             * then the drain immediately finishes.
             */
             */
            if (VERBOSE) Log.v(TAG, mIdString + "onSequenceDrained");
            mAbortDrainer.beginDrain();
            mAbortDrainer.beginDrain();
        }
        }
    }
    }
@@ -587,6 +614,7 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
    private class AbortDrainListener implements TaskDrainer.DrainListener {
    private class AbortDrainListener implements TaskDrainer.DrainListener {
        @Override
        @Override
        public void onDrained() {
        public void onDrained() {
            if (VERBOSE) Log.v(TAG, mIdString + "onAbortDrained");
            synchronized (CameraCaptureSessionImpl.this) {
            synchronized (CameraCaptureSessionImpl.this) {
                /*
                /*
                 * Any queued aborts have now completed.
                 * Any queued aborts have now completed.
@@ -604,6 +632,7 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
    private class IdleDrainListener implements TaskDrainer.DrainListener {
    private class IdleDrainListener implements TaskDrainer.DrainListener {
        @Override
        @Override
        public void onDrained() {
        public void onDrained() {
            if (VERBOSE) Log.v(TAG, mIdString + "onIdleDrained");
            synchronized (CameraCaptureSessionImpl.this) {
            synchronized (CameraCaptureSessionImpl.this) {
                /*
                /*
                 * The device is now IDLE, and has settled. It will not transition to
                 * The device is now IDLE, and has settled. It will not transition to
@@ -613,7 +642,7 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
                 *
                 *
                 * This operation is idempotent; a session will not be closed twice.
                 * This operation is idempotent; a session will not be closed twice.
                 */
                 */
                if (VERBOSE) Log.v(TAG, "Session drain complete, skip unconfigure: " +
                if (VERBOSE) Log.v(TAG, mIdString + "Session drain complete, skip unconfigure: " +
                        mSkipUnconfigure);
                        mSkipUnconfigure);


                // Fast path: A new capture session has replaced this one; don't unconfigure.
                // Fast path: A new capture session has replaced this one; don't unconfigure.
@@ -629,7 +658,7 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {
                    mDeviceImpl.configureOutputsChecked(null); // begin transition to unconfigured
                    mDeviceImpl.configureOutputsChecked(null); // begin transition to unconfigured
                } catch (CameraAccessException e) {
                } catch (CameraAccessException e) {
                    // OK: do not throw checked exceptions.
                    // OK: do not throw checked exceptions.
                    Log.e(TAG, "Exception while configuring outputs: ", e);
                    Log.e(TAG, mIdString + "Exception while configuring outputs: ", e);


                    // TODO: call onError instead of onClosed if this happens
                    // TODO: call onError instead of onClosed if this happens
                }
                }
@@ -641,7 +670,9 @@ public class CameraCaptureSessionImpl extends CameraCaptureSession {


    private class UnconfigureDrainListener implements TaskDrainer.DrainListener {
    private class UnconfigureDrainListener implements TaskDrainer.DrainListener {
        @Override
        @Override

        public void onDrained() {
        public void onDrained() {
            if (VERBOSE) Log.v(TAG, mIdString + "onUnconfigureDrained");
            synchronized (CameraCaptureSessionImpl.this) {
            synchronized (CameraCaptureSessionImpl.this) {
                // The device has finished unconfiguring. It's now fully closed.
                // The device has finished unconfiguring. It's now fully closed.
                mStateCallback.onClosed(CameraCaptureSessionImpl.this);
                mStateCallback.onClosed(CameraCaptureSessionImpl.this);
+6 −2
Original line number Original line Diff line number Diff line
@@ -99,6 +99,7 @@ public class CameraDeviceImpl extends CameraDevice {
    private final FrameNumberTracker mFrameNumberTracker = new FrameNumberTracker();
    private final FrameNumberTracker mFrameNumberTracker = new FrameNumberTracker();


    private CameraCaptureSessionImpl mCurrentSession;
    private CameraCaptureSessionImpl mCurrentSession;
    private int mNextSessionId = 0;


    // Runnables for all state transitions, except error, which needs the
    // Runnables for all state transitions, except error, which needs the
    // error code argument
    // error code argument
@@ -445,7 +446,8 @@ public class CameraDeviceImpl extends CameraDevice {


            // Fire onConfigured if configureOutputs succeeded, fire onConfigureFailed otherwise.
            // Fire onConfigured if configureOutputs succeeded, fire onConfigureFailed otherwise.
            CameraCaptureSessionImpl newSession =
            CameraCaptureSessionImpl newSession =
                    new CameraCaptureSessionImpl(outputs, callback, handler, this, mDeviceHandler,
                    new CameraCaptureSessionImpl(mNextSessionId++,
                            outputs, callback, handler, this, mDeviceHandler,
                            configureSuccess);
                            configureSuccess);


            // TODO: wait until current session closes, then create the new session
            // TODO: wait until current session closes, then create the new session
@@ -1003,8 +1005,10 @@ public class CameraDeviceImpl extends CameraDevice {
                    Log.e(TAG, String.format(
                    Log.e(TAG, String.format(
                            "result frame number %d comes out of order, should be %d + 1",
                            "result frame number %d comes out of order, should be %d + 1",
                            frameNumber, mCompletedFrameNumber));
                            frameNumber, mCompletedFrameNumber));
                    // Continue on to set the completed frame number to this frame anyway,
                    // to be robust to lower-level errors and allow for clean shutdowns.
                }
                }
                mCompletedFrameNumber++;
                mCompletedFrameNumber = frameNumber;
            }
            }
            update();
            update();
        }
        }
+6 −1
Original line number Original line Diff line number Diff line
@@ -19,6 +19,7 @@ package android.hardware.camera2.utils;
import static android.hardware.camera2.CameraAccessException.CAMERA_DISABLED;
import static android.hardware.camera2.CameraAccessException.CAMERA_DISABLED;
import static android.hardware.camera2.CameraAccessException.CAMERA_DISCONNECTED;
import static android.hardware.camera2.CameraAccessException.CAMERA_DISCONNECTED;
import static android.hardware.camera2.CameraAccessException.CAMERA_IN_USE;
import static android.hardware.camera2.CameraAccessException.CAMERA_IN_USE;
import static android.hardware.camera2.CameraAccessException.CAMERA_ERROR;
import static android.hardware.camera2.CameraAccessException.MAX_CAMERAS_IN_USE;
import static android.hardware.camera2.CameraAccessException.MAX_CAMERAS_IN_USE;
import static android.hardware.camera2.CameraAccessException.CAMERA_DEPRECATED_HAL;
import static android.hardware.camera2.CameraAccessException.CAMERA_DEPRECATED_HAL;


@@ -41,6 +42,7 @@ public class CameraBinderDecorator {
    public static final int BAD_VALUE = -22;
    public static final int BAD_VALUE = -22;
    public static final int DEAD_OBJECT = -32;
    public static final int DEAD_OBJECT = -32;
    public static final int INVALID_OPERATION = -38;
    public static final int INVALID_OPERATION = -38;
    public static final int TIMED_OUT = -110;


    /**
    /**
     * TODO: add as error codes in Errors.h
     * TODO: add as error codes in Errors.h
@@ -112,6 +114,9 @@ public class CameraBinderDecorator {
                throw new IllegalArgumentException("Bad argument passed to camera service");
                throw new IllegalArgumentException("Bad argument passed to camera service");
            case DEAD_OBJECT:
            case DEAD_OBJECT:
                throw new CameraRuntimeException(CAMERA_DISCONNECTED);
                throw new CameraRuntimeException(CAMERA_DISCONNECTED);
            case TIMED_OUT:
                throw new CameraRuntimeException(CAMERA_ERROR,
                        "Operation timed out in camera service");
            case EACCES:
            case EACCES:
                throw new CameraRuntimeException(CAMERA_DISABLED);
                throw new CameraRuntimeException(CAMERA_DISABLED);
            case EBUSY:
            case EBUSY:
@@ -123,7 +128,7 @@ public class CameraBinderDecorator {
            case EOPNOTSUPP:
            case EOPNOTSUPP:
                throw new CameraRuntimeException(CAMERA_DEPRECATED_HAL);
                throw new CameraRuntimeException(CAMERA_DEPRECATED_HAL);
            case INVALID_OPERATION:
            case INVALID_OPERATION:
                throw new IllegalStateException(
                throw new CameraRuntimeException(CAMERA_ERROR,
                        "Illegal state encountered in camera service.");
                        "Illegal state encountered in camera service.");
        }
        }