Merge "camera2: Fix deadlocks in shim #close and make #testInvalidCapture pass" (e76e8bce) · Commits · e / os / android_frameworks_base

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

+160 −116

Original line number	Diff line number	Diff line
		@@ -28,6 +28,8 @@ 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;
		@@ -57,13 +59,14 @@ 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 Object mLock = new Object();
		private final CloseableLock mCloseLock;
		private final CameraDeviceCallbacks mCallbacks = new CameraDeviceCallbacks();

		private final StateListener mDeviceListener;
		private volatile StateListener mSessionStateListener;
		private final Handler mDeviceHandler;

		private volatile boolean mClosing = false;
		private boolean mInError = false;
		private boolean mIdle = true;

		@@ -100,7 +103,9 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		private final Runnable mCallOnOpened = new Runnable() {
		@Override
		public void run() {
		if (!CameraDeviceImpl.this.isClosed()) {
		try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
		if (scopedLock == null) return; // Camera already closed

		StateListener sessionListener = mSessionStateListener;
		if (sessionListener != null) {
		sessionListener.onOpened(CameraDeviceImpl.this);
		@@ -113,7 +118,9 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		private final Runnable mCallOnUnconfigured = new Runnable() {
		@Override
		public void run() {
		if (!CameraDeviceImpl.this.isClosed()) {
		try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
		if (scopedLock == null) return; // Camera already closed

		StateListener sessionListener = mSessionStateListener;
		if (sessionListener != null) {
		sessionListener.onUnconfigured(CameraDeviceImpl.this);
		@@ -126,7 +133,9 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		private final Runnable mCallOnActive = new Runnable() {
		@Override
		public void run() {
		if (!CameraDeviceImpl.this.isClosed()) {
		try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
		if (scopedLock == null) return; // Camera already closed

		StateListener sessionListener = mSessionStateListener;
		if (sessionListener != null) {
		sessionListener.onActive(CameraDeviceImpl.this);
		@@ -139,7 +148,9 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		private final Runnable mCallOnBusy = new Runnable() {
		@Override
		public void run() {
		if (!CameraDeviceImpl.this.isClosed()) {
		try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
		if (scopedLock == null) return; // Camera already closed

		StateListener sessionListener = mSessionStateListener;
		if (sessionListener != null) {
		sessionListener.onBusy(CameraDeviceImpl.this);
		@@ -150,20 +161,29 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		};

		private final Runnable mCallOnClosed = new Runnable() {
		private boolean mClosedOnce = false;

		@Override
		public void run() {
		if (mClosedOnce) {
		throw new AssertionError("Don't post #onClosed more than once");
		}

		StateListener sessionListener = mSessionStateListener;
		if (sessionListener != null) {
		sessionListener.onClosed(CameraDeviceImpl.this);
		}
		mDeviceListener.onClosed(CameraDeviceImpl.this);
		mClosedOnce = true;
		}
		};

		private final Runnable mCallOnIdle = new Runnable() {
		@Override
		public void run() {
		if (!CameraDeviceImpl.this.isClosed()) {
		try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
		if (scopedLock == null) return; // Camera already closed

		StateListener sessionListener = mSessionStateListener;
		if (sessionListener != null) {
		sessionListener.onIdle(CameraDeviceImpl.this);
		@@ -176,7 +196,9 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		private final Runnable mCallOnDisconnected = new Runnable() {
		@Override
		public void run() {
		if (!CameraDeviceImpl.this.isClosed()) {
		try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
		if (scopedLock == null) return; // Camera already closed

		StateListener sessionListener = mSessionStateListener;
		if (sessionListener != null) {
		sessionListener.onDisconnected(CameraDeviceImpl.this);
		@@ -195,6 +217,7 @@ 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);
		@@ -210,8 +233,8 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		}

		public void setRemoteDevice(ICameraDeviceUser remoteDevice) {
		try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
		// TODO: Move from decorator to direct binder-mediated exceptions
		synchronized(mLock) {
		// If setRemoteFailure already called, do nothing
		if (mInError) return;

		@@ -254,7 +277,9 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		}
		final int code = failureCode;
		final boolean isError = failureIsError;
		synchronized (mLock) {
		try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
		if (scopedLock == null) return; // Camera already closed, can't go to error state

		mInError = true;
		mDeviceHandler.post(new Runnable() {
		@Override
		@@ -280,7 +305,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		if (outputs == null) {
		outputs = new ArrayList<Surface>();
		}
		synchronized (mLock) {
		try (ScopedLock scopedLock = mCloseLock.acquireExclusiveLock()) {
		checkIfCameraClosedOrInError();

		HashSet<Surface> addSet = new HashSet<Surface>(outputs); // Streams to create
		@@ -344,7 +369,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		public void createCaptureSession(List<Surface> outputs,
		CameraCaptureSession.StateListener listener, Handler handler)
		throws CameraAccessException {
		synchronized (mLock) {
		try (ScopedLock scopedLock = mCloseLock.acquireExclusiveLock()) {
		if (DEBUG) {
		Log.d(TAG, "createCaptureSession");
		}
		@@ -389,7 +414,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		@Override
		public CaptureRequest.Builder createCaptureRequest(int templateType)
		throws CameraAccessException {
		synchronized (mLock) {
		try (ScopedLock scopedLock = mCloseLock.acquireExclusiveLock()) {
		checkIfCameraClosedOrInError();

		CameraMetadataNative templatedRequest = new CameraMetadataNative();
		@@ -509,7 +534,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		handler = checkHandler(handler);
		}

		synchronized (mLock) {
		try (ScopedLock scopedLock = mCloseLock.acquireExclusiveLock()) {
		checkIfCameraClosedOrInError();
		int requestId;

		@@ -581,7 +606,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		@Override
		public void stopRepeating() throws CameraAccessException {

		synchronized (mLock) {
		try (ScopedLock scopedLock = mCloseLock.acquireExclusiveLock()) {
		checkIfCameraClosedOrInError();
		if (mRepeatingRequestId != REQUEST_ID_NONE) {

		@@ -612,7 +637,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {

		private void waitUntilIdle() throws CameraAccessException {

		synchronized (mLock) {
		try (ScopedLock scopedLock = mCloseLock.acquireExclusiveLock()) {
		checkIfCameraClosedOrInError();
		if (mRepeatingRequestId != REQUEST_ID_NONE) {
		throw new IllegalStateException("Active repeating request ongoing");
		@@ -633,7 +658,7 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {

		@Override
		public void flush() throws CameraAccessException {
		synchronized (mLock) {
		try (ScopedLock scopedLock = mCloseLock.acquireExclusiveLock()) {
		checkIfCameraClosedOrInError();

		mDeviceHandler.post(mCallOnBusy);
		@@ -656,8 +681,15 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {

		@Override
		public void close() {
		synchronized (mLock) {
		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)
		*/
		{
		try {
		if (mRemoteDevice != null) {
		mRemoteDevice.disconnect();
		@@ -805,7 +837,12 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		// remove request from mCaptureListenerMap
		final int requestId = frameNumberRequestPair.getValue();
		final CaptureListenerHolder holder;
		synchronized (mLock) {
		try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
		if (scopedLock == null) {
		Log.w(TAG, "Camera closed while checking sequences");
		return;
		}

		int index = mCaptureListenerMap.indexOfKey(requestId);
		holder = (index >= 0) ? mCaptureListenerMap.valueAt(index)
		: null;
		@@ -890,9 +927,12 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		@Override
		public void onCameraError(final int errorCode, CaptureResultExtras resultExtras) {
		Runnable r = null;
		if (isClosed()) return;

		synchronized(mLock) {
		try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
		if (scopedLock == null) {
		return; // Camera already closed
		}

		mInError = true;
		switch (errorCode) {
		case ERROR_CAMERA_DISCONNECTED:
		@@ -914,7 +954,6 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		break;
		}
		CameraDeviceImpl.this.mDeviceHandler.post(r);
		}

		// Fire onCaptureSequenceCompleted
		if (DEBUG) {
		@@ -922,17 +961,17 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		}
		mFrameNumberTracker.updateTracker(resultExtras.getFrameNumber(), /error/true);
		checkAndFireSequenceComplete();

		}
		}

		@Override
		public void onCameraIdle() {
		if (isClosed()) return;

		if (DEBUG) {
		Log.d(TAG, "Camera now idle");
		}
		synchronized (mLock) {
		try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
		if (scopedLock == null) return; // Camera already closed

		if (!CameraDeviceImpl.this.mIdle) {
		CameraDeviceImpl.this.mDeviceHandler.post(mCallOnIdle);
		}
		@@ -948,10 +987,11 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		}
		final CaptureListenerHolder holder;

		try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
		if (scopedLock == null) return; // Camera already closed

		// Get the listener for this frame ID, if there is one
		synchronized (mLock) {
		holder = CameraDeviceImpl.this.mCaptureListenerMap.get(requestId);
		}

		if (holder == null) {
		return;
		@@ -972,6 +1012,8 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		}
		}
		});

		}
		}

		@Override
		@@ -984,22 +1026,23 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		+ requestId);
		}

		try (ScopedLock scopedLock = mCloseLock.acquireLock()) {
		if (scopedLock == null) return; // Camera already closed

		// TODO: Handle CameraCharacteristics access from CaptureResult correctly.
		result.set(CameraCharacteristics.LENS_INFO_SHADING_MAP_SIZE,
		getCharacteristics().get(CameraCharacteristics.LENS_INFO_SHADING_MAP_SIZE));

		final CaptureListenerHolder holder;
		synchronized (mLock) {
		holder = CameraDeviceImpl.this.mCaptureListenerMap.get(requestId);
		}
		final CaptureListenerHolder holder =
		CameraDeviceImpl.this.mCaptureListenerMap.get(requestId);

		Boolean quirkPartial = result.get(CaptureResult.QUIRKS_PARTIAL_RESULT);
		boolean quirkIsPartialResult = (quirkPartial != null && quirkPartial);

		// Update tracker (increment counter) when it's not a partial result.
		if (!quirkIsPartialResult) {
		mFrameNumberTracker.updateTracker(resultExtras.getFrameNumber(), /error/false);
		mFrameNumberTracker.updateTracker(resultExtras.getFrameNumber(),
		/error/false);
		}

		// Check if we have a listener for this
		@@ -1067,6 +1110,8 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		if (!quirkIsPartialResult) {
		checkAndFireSequenceComplete();
		}

		}
		}

		}
		@@ -1101,10 +1146,9 @@ public class CameraDeviceImpl extends android.hardware.camera2.CameraDevice {
		}
		}

		/** Whether the camera device has started to close (may not yet have finished) */
		private boolean isClosed() {
		synchronized(mLock) {
		return (mRemoteDevice == null);
		}
		return mClosing;
		}

		private CameraCharacteristics getCharacteristics() {

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

+53 −5

Original line number	Diff line number	Diff line
		@@ -23,7 +23,6 @@ import android.hardware.camera2.impl.CaptureResultExtras;
		import android.hardware.camera2.ICameraDeviceCallbacks;
		import android.hardware.camera2.utils.LongParcelable;
		import android.hardware.camera2.impl.CameraMetadataNative;
		import android.hardware.camera2.utils.CameraBinderDecorator;
		import android.hardware.camera2.utils.CameraRuntimeException;
		import android.os.ConditionVariable;
		import android.os.Handler;
		@@ -34,7 +33,8 @@ import android.view.Surface;

		import java.util.ArrayList;
		import java.util.List;
		import java.util.concurrent.atomic.AtomicInteger;

		import static android.hardware.camera2.utils.CameraBinderDecorator.*;

		/**
		* This class emulates the functionality of a Camera2 device using a the old Camera class.
		@@ -54,6 +54,7 @@ public class LegacyCameraDevice implements AutoCloseable {
		private final int mCameraId;
		private final ICameraDeviceCallbacks mDeviceCallbacks;
		private final CameraDeviceState mDeviceState = new CameraDeviceState();
		private List<Surface> mConfiguredSurfaces;

		private final ConditionVariable mIdle = new ConditionVariable(/open/true);

		@@ -213,16 +214,35 @@ public class LegacyCameraDevice implements AutoCloseable {
		/**
		* Configure the device with a set of output surfaces.
		*
		* <p>Using empty or {@code null} {@code outputs} is the same as unconfiguring.</p>
		*
		* <p>Every surface in {@code outputs} must be non-{@code null}.</p>
		*
		* @param outputs a list of surfaces to set.
		* @return an error code for this binder operation, or {@link CameraBinderDecorator.NO_ERROR}
		* @return an error code for this binder operation, or {@link NO_ERROR}
		* on success.
		*/
		public int configureOutputs(List<Surface> outputs) {
		if (outputs != null) {
		for (Surface output : outputs) {
		if (output == null) {
		Log.e(TAG, "configureOutputs - null outputs are not allowed");
		return BAD_VALUE;
		}
		}
		}

		int error = mDeviceState.setConfiguring();
		if (error == CameraBinderDecorator.NO_ERROR) {
		if (error == NO_ERROR) {
		mRequestThreadManager.configure(outputs);
		error = mDeviceState.setIdle();
		}

		// TODO: May also want to check the surfaces more deeply (e.g. state, formats, sizes..)
		if (error == NO_ERROR) {
		mConfiguredSurfaces = outputs != null ? new ArrayList<>(outputs) : null;
		}

		return error;
		}

		@@ -239,7 +259,35 @@ public class LegacyCameraDevice implements AutoCloseable {
		*/
		public int submitRequestList(List<CaptureRequest> requestList, boolean repeating,
		/out/LongParcelable frameNumber) {
		// TODO: validate request here
		if (requestList == null \|\| requestList.isEmpty()) {
		Log.e(TAG, "submitRequestList - Empty/null requests are not allowed");
		return BAD_VALUE;
		}

		// Make sure that there all requests have at least 1 surface; all surfaces are non-null
		for (CaptureRequest request : requestList) {
		if (request.getTargets().isEmpty()) {
		Log.e(TAG, "submitRequestList - "
		+ "Each request must have at least one Surface target");
		return BAD_VALUE;
		}

		for (Surface surface : request.getTargets()) {
		if (surface == null) {
		Log.e(TAG, "submitRequestList - Null Surface targets are not allowed");
		return BAD_VALUE;
		} else if (mConfiguredSurfaces == null) {
		Log.e(TAG, "submitRequestList - must configure " +
		" device with valid surfaces before submitting requests");
		return INVALID_OPERATION;
		} else if (!mConfiguredSurfaces.contains(surface)) {
		Log.e(TAG, "submitRequestList - cannot use a surface that wasn't configured");
		return BAD_VALUE;
		}
		}
		}

		// TODO: further validation of request here
		mIdle.close();
		return mRequestThreadManager.submitCaptureRequests(requestList, repeating,
		frameNumber);

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

+19 −18

Original line number	Diff line number	Diff line
		@@ -16,7 +16,6 @@

		package android.hardware.camera2.legacy;

		import android.graphics.ImageFormat;
		import android.graphics.SurfaceTexture;
		import android.hardware.Camera;
		import android.hardware.camera2.CaptureRequest;
		@@ -76,8 +75,8 @@ public class RequestThreadManager {
		private volatile RequestHolder mInFlightPreview;
		private volatile RequestHolder mInFlightJpeg;

		private List<Surface> mPreviewOutputs = new ArrayList<Surface>();
		private List<Surface> mCallbackOutputs = new ArrayList<Surface>();
		private final List<Surface> mPreviewOutputs = new ArrayList<Surface>();
		private final List<Surface> mCallbackOutputs = new ArrayList<Surface>();
		private GLThreadManager mGLThreadManager;
		private SurfaceTexture mPreviewTexture;
		private Camera.Parameters mParams;
		@@ -315,7 +314,7 @@ public class RequestThreadManager {
		mInFlightPreview = null;
		mInFlightJpeg = null;


		if (outputs != null) {
		for (Surface s : outputs) {
		int format = LegacyCameraDevice.nativeDetectSurfaceType(s);
		switch (format) {
		@@ -327,6 +326,7 @@ public class RequestThreadManager {
		break;
		}
		}
		}
		mParams = mCamera.getParameters();
		if (mPreviewOutputs.size() > 0) {
		List<Size> outputSizes = new ArrayList<>(outputs.size());
		@@ -370,8 +370,6 @@ public class RequestThreadManager {
		}
		}



		// TODO: Detect and optimize single-output paths here to skip stream teeing.
		if (mGLThreadManager == null) {
		mGLThreadManager = new GLThreadManager(mCameraId);
		@@ -432,7 +430,7 @@ public class RequestThreadManager {

		private final Handler.Callback mRequestHandlerCb = new Handler.Callback() {
		private boolean mCleanup = false;
		private List<RequestHolder> mRepeating = null;
		private final List<RequestHolder> mRepeating = null;

		@SuppressWarnings("unchecked")
		@Override
		@@ -447,7 +445,8 @@ public class RequestThreadManager {
		switch (msg.what) {
		case MSG_CONFIGURE_OUTPUTS:
		ConfigureHolder config = (ConfigureHolder) msg.obj;
		Log.i(TAG, "Configure outputs: " + config.surfaces.size() +
		int sizes = config.surfaces != null ? config.surfaces.size() : 0;
		Log.i(TAG, "Configure outputs: " + sizes +
		" surfaces configured.");
		try {
		configureOutputs(config.surfaces);
		@@ -620,12 +619,14 @@ public class RequestThreadManager {


		/**
		* Configure with the current output Surfaces.
		* Configure with the current list of output Surfaces.
		*
		* <p>
		* This operation blocks until the configuration is complete.
		* </p>
		*
		* <p>Using a {@code null} or empty {@code outputs} list is the equivalent of unconfiguring.</p>
		*
		* @param outputs a {@link java.util.Collection} of outputs to configure.
		*/
		public void configure(Collection<Surface> outputs) {

core/java/android/hardware/camera2/utils/CloseableLock.java

0 → 100644

+330 −0

File added.

Preview size limit exceeded, changes collapsed.