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

Commit 2f75e7d7 authored by Eino-Ville Talvala's avatar Eino-Ville Talvala
Browse files

Camera2: Fix callback operation 

- Remove CloseableLock use; looks to be incompatible with invocations during callbacks
- Replace with basic interface lock to be thread-safe
- Add intermediate callback thread to legacy mode to match cross-process one-way Binder
  semantics

Change-Id: Iecd4ff6cf260c5a13bd11b850177ccea93e25933
parent 0b633fc1

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

+77 −81
Original line number Diff line number Diff line
@@ -28,8 +28,6 @@ import android.hardware.camera2.ICameraDeviceUser; 
import android.hardware.camera2.TotalCaptureResult;

import android.hardware.camera2.utils.CameraBinderDecorator;

import android.hardware.camera2.utils.CameraRuntimeException;

import android.hardware.camera2.utils.CloseableLock;

import android.hardware.camera2.utils.CloseableLock.ScopedLock;

import android.hardware.camera2.utils.LongParcelable;

import android.os.Handler;

import android.os.IBinder;
@@ -59,7 +57,8 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice { 
    // TODO: guard every function with if (!mRemoteDevice) check (if it was closed)

    private ICameraDeviceUser mRemoteDevice;



    private final CloseableLock mCloseLock;

    // Lock to synchronize cross-thread access to device public interface

    private final Object mInterfaceLock = new Object();

    private final CameraDeviceCallbacks mCallbacks = new CameraDeviceCallbacks();



    private final StateListener mDeviceListener;
@@ -104,61 +103,65 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice { 
    private final Runnable mCallOnOpened = new Runnable() {

        @Override

        public void run() {

            try (ScopedLock scopedLock = mCloseLock.acquireLock()) {

                if (scopedLock == null) return; // Camera already closed

            StateListener sessionListener = null;

            synchronized(mInterfaceLock) {

                if (mRemoteDevice == null) return; // Camera already closed



                StateListener sessionListener = mSessionStateListener;

                sessionListener = mSessionStateListener;

            }

            if (sessionListener != null) {

                sessionListener.onOpened(CameraDeviceImpl.this);

            }

            mDeviceListener.onOpened(CameraDeviceImpl.this);

        }

        }

    };



    private final Runnable mCallOnUnconfigured = new Runnable() {

        @Override

        public void run() {

            try (ScopedLock scopedLock = mCloseLock.acquireLock()) {

                if (scopedLock == null) return; // Camera already closed

            StateListener sessionListener = null;

            synchronized(mInterfaceLock) {

                if (mRemoteDevice == null) return; // Camera already closed



                StateListener sessionListener = mSessionStateListener;

                sessionListener = mSessionStateListener;

            }

            if (sessionListener != null) {

                sessionListener.onUnconfigured(CameraDeviceImpl.this);

            }

            mDeviceListener.onUnconfigured(CameraDeviceImpl.this);

        }

        }

    };



    private final Runnable mCallOnActive = new Runnable() {

        @Override

        public void run() {

            try (ScopedLock scopedLock = mCloseLock.acquireLock()) {

                if (scopedLock == null) return; // Camera already closed

            StateListener sessionListener = null;

            synchronized(mInterfaceLock) {

                if (mRemoteDevice == null) return; // Camera already closed



                StateListener sessionListener = mSessionStateListener;

                sessionListener = mSessionStateListener;

            }

            if (sessionListener != null) {

                sessionListener.onActive(CameraDeviceImpl.this);

            }

            mDeviceListener.onActive(CameraDeviceImpl.this);

        }

        }

    };



    private final Runnable mCallOnBusy = new Runnable() {

        @Override

        public void run() {

            try (ScopedLock scopedLock = mCloseLock.acquireLock()) {

                if (scopedLock == null) return; // Camera already closed

            StateListener sessionListener = null;

            synchronized(mInterfaceLock) {

                if (mRemoteDevice == null) return; // Camera already closed



                StateListener sessionListener = mSessionStateListener;

                sessionListener = mSessionStateListener;

            }

            if (sessionListener != null) {

                sessionListener.onBusy(CameraDeviceImpl.this);

            }

            mDeviceListener.onBusy(CameraDeviceImpl.this);

        }

        }

    };



    private final Runnable mCallOnClosed = new Runnable() {
@@ -169,8 +172,10 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice { 
            if (mClosedOnce) {

                throw new AssertionError("Don't post #onClosed more than once");

            }



            StateListener sessionListener = mSessionStateListener;

            StateListener sessionListener = null;

            synchronized(mInterfaceLock) {

                sessionListener = mSessionStateListener;

            }

            if (sessionListener != null) {

                sessionListener.onClosed(CameraDeviceImpl.this);

            }
@@ -182,31 +187,33 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice { 
    private final Runnable mCallOnIdle = new Runnable() {

        @Override

        public void run() {

            try (ScopedLock scopedLock = mCloseLock.acquireLock()) {

                if (scopedLock == null) return; // Camera already closed

            StateListener sessionListener = null;

            synchronized(mInterfaceLock) {

                if (mRemoteDevice == null) return; // Camera already closed



                StateListener sessionListener = mSessionStateListener;

                sessionListener = mSessionStateListener;

            }

            if (sessionListener != null) {

                sessionListener.onIdle(CameraDeviceImpl.this);

            }

            mDeviceListener.onIdle(CameraDeviceImpl.this);

        }

        }

    };



    private final Runnable mCallOnDisconnected = new Runnable() {

        @Override

        public void run() {

            try (ScopedLock scopedLock = mCloseLock.acquireLock()) {

                if (scopedLock == null) return; // Camera already closed

            StateListener sessionListener = null;

            synchronized(mInterfaceLock) {

                if (mRemoteDevice == null) return; // Camera already closed



                StateListener sessionListener = mSessionStateListener;

                sessionListener = mSessionStateListener;

            }

            if (sessionListener != null) {

                sessionListener.onDisconnected(CameraDeviceImpl.this);

            }

            mDeviceListener.onDisconnected(CameraDeviceImpl.this);

        }

        }

    };



    public CameraDeviceImpl(String cameraId, StateListener listener, Handler handler,
@@ -218,7 +225,6 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice { 
        mDeviceListener = listener;

        mDeviceHandler = handler;

        mCharacteristics = characteristics;

        mCloseLock = new CloseableLock(/*name*/"CD-" + mCameraId);



        final int MAX_TAG_LEN = 23;

        String tag = String.format("CameraDevice-JV-%s", mCameraId);
@@ -243,7 +249,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice { 
    }



    public void setRemoteDevice(ICameraDeviceUser remoteDevice) {

        try (ScopedLock scopedLock = mCloseLock.acquireLock()) {

        synchronized(mInterfaceLock) {

            // TODO: Move from decorator to direct binder-mediated exceptions

            // If setRemoteFailure already called, do nothing

            if (mInError) return;
@@ -287,8 +293,8 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice { 
        }

        final int code = failureCode;

        final boolean isError = failureIsError;

        try (ScopedLock scopedLock = mCloseLock.acquireLock()) {

            if (scopedLock == null) return; // Camera already closed, can't go to error state

        synchronized(mInterfaceLock) {

            if (mRemoteDevice == null) return; // Camera already closed, can't go to error state



            mInError = true;

            mDeviceHandler.post(new Runnable() {
@@ -314,7 +320,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice { 
        if (outputs == null) {

            outputs = new ArrayList<Surface>();

        }

        try (ScopedLock scopedLock = mCloseLock.acquireExclusiveLock()) {

        synchronized(mInterfaceLock) {

            checkIfCameraClosedOrInError();



            HashSet<Surface> addSet = new HashSet<Surface>(outputs);    // Streams to create
@@ -378,7 +384,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice { 
    public void createCaptureSession(List<Surface> outputs,

            CameraCaptureSession.StateListener listener, Handler handler)

            throws CameraAccessException {

        try (ScopedLock scopedLock = mCloseLock.acquireExclusiveLock()) {

        synchronized(mInterfaceLock) {

            if (DEBUG) {

                Log.d(TAG, "createCaptureSession");

            }
@@ -423,7 +429,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice { 
    @Override

    public CaptureRequest.Builder createCaptureRequest(int templateType)

            throws CameraAccessException {

        try (ScopedLock scopedLock = mCloseLock.acquireExclusiveLock()) {

        synchronized(mInterfaceLock) {

            checkIfCameraClosedOrInError();



            CameraMetadataNative templatedRequest = new CameraMetadataNative();
@@ -554,7 +560,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice { 
            }

        }



        try (ScopedLock scopedLock = mCloseLock.acquireExclusiveLock()) {

        synchronized(mInterfaceLock) {

            checkIfCameraClosedOrInError();

            int requestId;
@@ -623,7 +629,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice { 


    public void stopRepeating() throws CameraAccessException {



        try (ScopedLock scopedLock = mCloseLock.acquireExclusiveLock()) {

        synchronized(mInterfaceLock) {

            checkIfCameraClosedOrInError();

            if (mRepeatingRequestId != REQUEST_ID_NONE) {
@@ -654,12 +660,12 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice { 


    private void waitUntilIdle() throws CameraAccessException {



        try (ScopedLock scopedLock = mCloseLock.acquireExclusiveLock()) {

        synchronized(mInterfaceLock) {

            checkIfCameraClosedOrInError();



            if (mRepeatingRequestId != REQUEST_ID_NONE) {

                throw new IllegalStateException("Active repeating request ongoing");

            }



            try {

                mRemoteDevice.waitUntilIdle();

            } catch (CameraRuntimeException e) {
@@ -668,13 +674,11 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice { 
                // impossible

                return;

            }



            mRepeatingRequestId = REQUEST_ID_NONE;

        }

    }



    public void flush() throws CameraAccessException {

        try (ScopedLock scopedLock = mCloseLock.acquireExclusiveLock()) {

        synchronized(mInterfaceLock) {

            checkIfCameraClosedOrInError();



            mDeviceHandler.post(mCallOnBusy);
@@ -697,15 +701,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice { 


    @Override

    public void close() {

        mClosing = true;

        // Acquire exclusive lock, close, release (idempotent)

        mCloseLock.close();



        /*

         *  The rest of this is safe, since no other methods will be able to execute

         *  (they will throw ISE instead; the callbacks will get dropped)

         */

        {

        synchronized (mInterfaceLock) {

            try {

                if (mRemoteDevice != null) {

                    mRemoteDevice.disconnect();
@@ -853,8 +849,8 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice { 
                // remove request from mCaptureListenerMap

                final int requestId = frameNumberRequestPair.getValue();

                final CaptureListenerHolder holder;

                try (ScopedLock scopedLock = mCloseLock.acquireLock()) {

                    if (scopedLock == null) {

                synchronized(mInterfaceLock) {

                    if (mRemoteDevice == null) {

                        Log.w(TAG, "Camera closed while checking sequences");

                        return;

                    }
@@ -944,8 +940,8 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice { 
        public void onCameraError(final int errorCode, CaptureResultExtras resultExtras) {

            Runnable r = null;



            try (ScopedLock scopedLock = mCloseLock.acquireLock()) {

                if (scopedLock == null) {

            synchronized(mInterfaceLock) {

                if (mRemoteDevice == null) {

                    return; // Camera already closed

                }
@@ -985,8 +981,8 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice { 
            if (DEBUG) {

                Log.d(TAG, "Camera now idle");

            }

            try (ScopedLock scopedLock = mCloseLock.acquireLock()) {

                if (scopedLock == null) return; // Camera already closed

            synchronized(mInterfaceLock) {

                if (mRemoteDevice == null) return; // Camera already closed



                if (!CameraDeviceImpl.this.mIdle) {

                    CameraDeviceImpl.this.mDeviceHandler.post(mCallOnIdle);
@@ -1003,8 +999,8 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice { 
            }

            final CaptureListenerHolder holder;



            try (ScopedLock scopedLock = mCloseLock.acquireLock()) {

                if (scopedLock == null) return; // Camera already closed

            synchronized(mInterfaceLock) {

                if (mRemoteDevice == null) return; // Camera already closed



                // Get the listener for this frame ID, if there is one

                holder = CameraDeviceImpl.this.mCaptureListenerMap.get(requestId);
@@ -1042,8 +1038,8 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice { 
                        + requestId);

            }



            try (ScopedLock scopedLock = mCloseLock.acquireLock()) {

                if (scopedLock == null) return; // Camera already closed

            synchronized(mInterfaceLock) {

                if (mRemoteDevice == null) return; // Camera already closed



                // TODO: Handle CameraCharacteristics access from CaptureResult correctly.

                result.set(CameraCharacteristics.LENS_INFO_SHADING_MAP_SIZE,

core/java/android/hardware/camera2/legacy/CameraDeviceUserShim.java

+130 −3
Original line number Diff line number Diff line
@@ -25,11 +25,15 @@ import android.hardware.camera2.ICameraDeviceCallbacks; 
import android.hardware.camera2.ICameraDeviceUser;

import android.hardware.camera2.utils.LongParcelable;

import android.hardware.camera2.impl.CameraMetadataNative;

import android.hardware.camera2.impl.CaptureResultExtras;

import android.hardware.camera2.utils.CameraBinderDecorator;

import android.hardware.camera2.utils.CameraRuntimeException;

import android.os.ConditionVariable;

import android.os.IBinder;

import android.os.Looper;

import android.os.Handler;

import android.os.HandlerThread;

import android.os.Message;

import android.os.RemoteException;

import android.util.Log;

import android.util.SparseArray;
@@ -66,14 +70,18 @@ public class CameraDeviceUserShim implements ICameraDeviceUser { 
    private final SparseArray<Surface> mSurfaces;

    private final CameraCharacteristics mCameraCharacteristics;

    private final CameraLooper mCameraInit;

    private final CameraCallbackThread mCameraCallbacks;





    protected CameraDeviceUserShim(int cameraId, LegacyCameraDevice legacyCamera,

            CameraCharacteristics characteristics, CameraLooper cameraInit) {

            CameraCharacteristics characteristics, CameraLooper cameraInit,

            CameraCallbackThread cameraCallbacks) {

        mLegacyDevice = legacyCamera;

        mConfiguring = false;

        mSurfaces = new SparseArray<Surface>();

        mCameraCharacteristics = characteristics;

        mCameraInit = cameraInit;

        mCameraCallbacks = cameraCallbacks;



        mSurfaceIdCounter = 0;

    }
@@ -173,6 +181,122 @@ public class CameraDeviceUserShim implements ICameraDeviceUser { 
        }

    }



    /**

     * A thread to process callbacks to send back to the camera client.

     *

     * <p>This effectively emulates one-way binder semantics when in the same process as the

     * callee.</p>

     */

    private static class CameraCallbackThread implements ICameraDeviceCallbacks {

        private static final int CAMERA_ERROR = 0;

        private static final int CAMERA_IDLE = 1;

        private static final int CAPTURE_STARTED = 2;

        private static final int RESULT_RECEIVED = 3;



        private final HandlerThread mHandlerThread;

        private Handler mHandler;



        private final ICameraDeviceCallbacks mCallbacks;



        public CameraCallbackThread(ICameraDeviceCallbacks callbacks) {

            mCallbacks = callbacks;



            mHandlerThread = new HandlerThread("LegacyCameraCallback");

            mHandlerThread.start();

        }



        public void close() {

            mHandlerThread.quitSafely();

        }



        @Override

        public void onCameraError(final int errorCode, final CaptureResultExtras resultExtras) {

            Message msg = getHandler().obtainMessage(CAMERA_ERROR,

                /*arg1*/ errorCode, /*arg2*/ 0,

                /*obj*/ resultExtras);

            getHandler().sendMessage(msg);

        }



        @Override

        public void onCameraIdle() {

            Message msg = getHandler().obtainMessage(CAMERA_IDLE);

            getHandler().sendMessage(msg);

        }



        @Override

        public void onCaptureStarted(final CaptureResultExtras resultExtras, final long timestamp) {

            Message msg = getHandler().obtainMessage(CAPTURE_STARTED,

                    /*arg1*/ (int) (timestamp & 0xFFFFFFFFL),

                    /*arg2*/ (int) ( (timestamp >> 32) & 0xFFFFFFFFL),

                    /*obj*/ resultExtras);

            getHandler().sendMessage(msg);

        }



        @Override

        public void onResultReceived(final CameraMetadataNative result,

                final CaptureResultExtras resultExtras) {

            Object[] resultArray = new Object[] { result, resultExtras };

            Message msg = getHandler().obtainMessage(RESULT_RECEIVED,

                    /*obj*/ resultArray);

            getHandler().sendMessage(msg);

        }



        @Override

        public IBinder asBinder() {

            // This is solely intended to be used for in-process binding.

            return null;

        }



        private Handler getHandler() {

            if (mHandler == null) {

                mHandler = new CallbackHandler(mHandlerThread.getLooper());

            }

            return mHandler;

        }



        private class CallbackHandler extends Handler {

            public CallbackHandler(Looper l) {

                super(l);

            }



            public void handleMessage(Message msg) {

                try {

                    switch (msg.what) {

                        case CAMERA_ERROR: {

                            int errorCode = msg.arg1;

                            CaptureResultExtras resultExtras = (CaptureResultExtras) msg.obj;

                            mCallbacks.onCameraError(errorCode, resultExtras);

                            break;

                        }

                        case CAMERA_IDLE:

                            mCallbacks.onCameraIdle();

                            break;

                        case CAPTURE_STARTED: {

                            long timestamp = msg.arg2 & 0xFFFFFFFFL;

                            timestamp = (timestamp << 32) | (msg.arg1 & 0xFFFFFFFFL);

                            CaptureResultExtras resultExtras = (CaptureResultExtras) msg.obj;

                            mCallbacks.onCaptureStarted(resultExtras, timestamp);

                            break;

                        }

                        case RESULT_RECEIVED: {

                            Object[] resultArray = (Object[]) msg.obj;

                            CameraMetadataNative result = (CameraMetadataNative) resultArray[0];

                            CaptureResultExtras resultExtras = (CaptureResultExtras) resultArray[1];

                            mCallbacks.onResultReceived(result, resultExtras);

                            break;

                        }

                        default:

                            throw new IllegalArgumentException(

                                "Unknown callback message " + msg.what);

                    }

                } catch (RemoteException e) {

                    throw new IllegalStateException(

                        "Received remote exception during camera callback " + msg.what, e);

                }

            }

        }

    }



    public static CameraDeviceUserShim connectBinderShim(ICameraDeviceCallbacks callbacks,

                                                         int cameraId) {

        if (DEBUG) {
@@ -187,6 +311,8 @@ public class CameraDeviceUserShim implements ICameraDeviceUser { 


        CameraLooper init = new CameraLooper(cameraId);



        CameraCallbackThread threadCallbacks = new CameraCallbackThread(callbacks);



        // TODO: Make this async instead of blocking

        int initErrors = init.waitForOpen(OPEN_CAMERA_TIMEOUT_MS);

        Camera legacyCamera = init.getCamera();
@@ -200,8 +326,8 @@ public class CameraDeviceUserShim implements ICameraDeviceUser { 
        CameraCharacteristics characteristics =

                LegacyMetadataMapper.createCharacteristics(legacyCamera.getParameters(), info);

        LegacyCameraDevice device = new LegacyCameraDevice(

                cameraId, legacyCamera, characteristics, callbacks);

        return new CameraDeviceUserShim(cameraId, device, characteristics, init);

                cameraId, legacyCamera, characteristics, threadCallbacks);

        return new CameraDeviceUserShim(cameraId, device, characteristics, init, threadCallbacks);

    }



    @Override
@@ -214,6 +340,7 @@ public class CameraDeviceUserShim implements ICameraDeviceUser { 
            mLegacyDevice.close();

        } finally {

            mCameraInit.close();

            mCameraCallbacks.close();

        }

    }