diff options
author | TreeHugger Robot <treehugger-gerrit@google.com> | 2020-06-10 16:33:37 +0000 |
---|---|---|
committer | Android (Google) Code Review <android-gerrit@google.com> | 2020-06-10 16:33:37 +0000 |
commit | a27465258acbc7e4f0007cf2ab3d0cbfd1294893 (patch) | |
tree | 178fba2ce12033d62985be92e6e5425f14a009ea | |
parent | e445872013bdbe53c62e0e3719a70fac0000f060 (diff) | |
parent | 22abb8d0c1f0f2c8abaf8eb2f061d35c5f759c01 (diff) |
Merge "Revert "Camera: Fix race for onCaptureBufferLost callback"" into rvc-dev
5 files changed, 61 insertions, 276 deletions
diff --git a/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java b/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java index 23c86029f3be..6d49add65c5b 100644 --- a/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java +++ b/core/java/android/hardware/camera2/impl/CameraDeviceImpl.java @@ -1072,7 +1072,7 @@ public class CameraDeviceImpl extends CameraDevice * @param lastFrameNumber last frame number returned from binder. * @param repeatingRequestTypes the repeating requests' types. */ - private void checkEarlyTriggerSequenceCompleteLocked( + private void checkEarlyTriggerSequenceComplete( final int requestId, final long lastFrameNumber, final int[] repeatingRequestTypes) { // lastFrameNumber being equal to NO_FRAMES_CAPTURED means that the request @@ -1212,7 +1212,7 @@ public class CameraDeviceImpl extends CameraDevice if (repeating) { if (mRepeatingRequestId != REQUEST_ID_NONE) { - checkEarlyTriggerSequenceCompleteLocked(mRepeatingRequestId, + checkEarlyTriggerSequenceComplete(mRepeatingRequestId, requestInfo.getLastFrameNumber(), mRepeatingRequestTypes); } @@ -1269,7 +1269,7 @@ public class CameraDeviceImpl extends CameraDevice return; } - checkEarlyTriggerSequenceCompleteLocked(requestId, lastFrameNumber, requestTypes); + checkEarlyTriggerSequenceComplete(requestId, lastFrameNumber, requestTypes); } } } @@ -1302,7 +1302,7 @@ public class CameraDeviceImpl extends CameraDevice long lastFrameNumber = mRemoteDevice.flush(); if (mRepeatingRequestId != REQUEST_ID_NONE) { - checkEarlyTriggerSequenceCompleteLocked(mRepeatingRequestId, lastFrameNumber, + checkEarlyTriggerSequenceComplete(mRepeatingRequestId, lastFrameNumber, mRepeatingRequestTypes); mRepeatingRequestId = REQUEST_ID_NONE; mRepeatingRequestTypes = null; @@ -1442,135 +1442,78 @@ public class CameraDeviceImpl extends CameraDevice long completedFrameNumber = mFrameNumberTracker.getCompletedFrameNumber(); long completedReprocessFrameNumber = mFrameNumberTracker.getCompletedReprocessFrameNumber(); long completedZslStillFrameNumber = mFrameNumberTracker.getCompletedZslStillFrameNumber(); - + boolean isReprocess = false; Iterator<RequestLastFrameNumbersHolder> iter = mRequestLastFrameNumbersList.iterator(); while (iter.hasNext()) { final RequestLastFrameNumbersHolder requestLastFrameNumbers = iter.next(); + boolean sequenceCompleted = false; final int requestId = requestLastFrameNumbers.getRequestId(); final CaptureCallbackHolder holder; - if (mRemoteDevice == null) { - Log.w(TAG, "Camera closed while checking sequences"); - return; - } - if (!requestLastFrameNumbers.isSequenceCompleted()) { - long lastRegularFrameNumber = - requestLastFrameNumbers.getLastRegularFrameNumber(); - long lastReprocessFrameNumber = - requestLastFrameNumbers.getLastReprocessFrameNumber(); - long lastZslStillFrameNumber = - requestLastFrameNumbers.getLastZslStillFrameNumber(); - if (lastRegularFrameNumber <= completedFrameNumber - && lastReprocessFrameNumber <= completedReprocessFrameNumber - && lastZslStillFrameNumber <= completedZslStillFrameNumber) { - Log.v(TAG, String.format( - "Mark requestId %d as completed, because lastRegularFrame %d " - + "is <= %d, lastReprocessFrame %d is <= %d, " - + "lastZslStillFrame %d is <= %d", requestId, - lastRegularFrameNumber, completedFrameNumber, - lastReprocessFrameNumber, completedReprocessFrameNumber, - lastZslStillFrameNumber, completedZslStillFrameNumber)); - requestLastFrameNumbers.markSequenceCompleted(); + synchronized(mInterfaceLock) { + if (mRemoteDevice == null) { + Log.w(TAG, "Camera closed while checking sequences"); + return; } - // Call onCaptureSequenceCompleted int index = mCaptureCallbackMap.indexOfKey(requestId); holder = (index >= 0) ? mCaptureCallbackMap.valueAt(index) : null; - if (holder != null && requestLastFrameNumbers.isSequenceCompleted()) { - Runnable resultDispatch = new Runnable() { - @Override - public void run() { - if (!CameraDeviceImpl.this.isClosed()){ - if (DEBUG) { - Log.d(TAG, String.format( - "fire sequence complete for request %d", - requestId)); - } - - holder.getCallback().onCaptureSequenceCompleted( - CameraDeviceImpl.this, - requestId, - requestLastFrameNumbers.getLastFrameNumber()); - } + if (holder != null) { + long lastRegularFrameNumber = + requestLastFrameNumbers.getLastRegularFrameNumber(); + long lastReprocessFrameNumber = + requestLastFrameNumbers.getLastReprocessFrameNumber(); + long lastZslStillFrameNumber = + requestLastFrameNumbers.getLastZslStillFrameNumber(); + // check if it's okay to remove request from mCaptureCallbackMap + if (lastRegularFrameNumber <= completedFrameNumber + && lastReprocessFrameNumber <= completedReprocessFrameNumber + && lastZslStillFrameNumber <= completedZslStillFrameNumber) { + sequenceCompleted = true; + mCaptureCallbackMap.removeAt(index); + if (DEBUG) { + Log.v(TAG, String.format( + "Remove holder for requestId %d, because lastRegularFrame %d " + + "is <= %d, lastReprocessFrame %d is <= %d, " + + "lastZslStillFrame %d is <= %d", requestId, + lastRegularFrameNumber, completedFrameNumber, + lastReprocessFrameNumber, completedReprocessFrameNumber, + lastZslStillFrameNumber, completedZslStillFrameNumber)); } - }; - final long ident = Binder.clearCallingIdentity(); - try { - holder.getExecutor().execute(resultDispatch); - } finally { - Binder.restoreCallingIdentity(ident); } } } - if (requestLastFrameNumbers.isSequenceCompleted() && - requestLastFrameNumbers.isInflightCompleted()) { - int index = mCaptureCallbackMap.indexOfKey(requestId); - if (index >= 0) { - mCaptureCallbackMap.removeAt(index); - } - if (DEBUG) { - Log.v(TAG, String.format( - "Remove holder for requestId %d", requestId)); - } + // If no callback is registered for this requestId or sequence completed, remove it + // from the frame number->request pair because it's not needed anymore. + if (holder == null || sequenceCompleted) { iter.remove(); } - } - } - - private void removeCompletedCallbackHolderLocked(long lastCompletedRegularFrameNumber, - long lastCompletedReprocessFrameNumber, long lastCompletedZslStillFrameNumber) { - if (DEBUG) { - Log.v(TAG, String.format("remove completed callback holders for " - + "lastCompletedRegularFrameNumber %d, " - + "lastCompletedReprocessFrameNumber %d, " - + "lastCompletedZslStillFrameNumber %d", - lastCompletedRegularFrameNumber, - lastCompletedReprocessFrameNumber, - lastCompletedZslStillFrameNumber)); - } - - Iterator<RequestLastFrameNumbersHolder> iter = mRequestLastFrameNumbersList.iterator(); - while (iter.hasNext()) { - final RequestLastFrameNumbersHolder requestLastFrameNumbers = iter.next(); - final int requestId = requestLastFrameNumbers.getRequestId(); - final CaptureCallbackHolder holder; - if (mRemoteDevice == null) { - Log.w(TAG, "Camera closed while removing completed callback holders"); - return; - } - long lastRegularFrameNumber = - requestLastFrameNumbers.getLastRegularFrameNumber(); - long lastReprocessFrameNumber = - requestLastFrameNumbers.getLastReprocessFrameNumber(); - long lastZslStillFrameNumber = - requestLastFrameNumbers.getLastZslStillFrameNumber(); - - if (lastRegularFrameNumber <= lastCompletedRegularFrameNumber - && lastReprocessFrameNumber <= lastCompletedReprocessFrameNumber - && lastZslStillFrameNumber <= lastCompletedZslStillFrameNumber) { + // Call onCaptureSequenceCompleted + if (sequenceCompleted) { + Runnable resultDispatch = new Runnable() { + @Override + public void run() { + if (!CameraDeviceImpl.this.isClosed()){ + if (DEBUG) { + Log.d(TAG, String.format( + "fire sequence complete for request %d", + requestId)); + } - if (requestLastFrameNumbers.isSequenceCompleted()) { - int index = mCaptureCallbackMap.indexOfKey(requestId); - if (index >= 0) { - mCaptureCallbackMap.removeAt(index); - } - if (DEBUG) { - Log.v(TAG, String.format( - "Remove holder for requestId %d, because lastRegularFrame %d " - + "is <= %d, lastReprocessFrame %d is <= %d, " - + "lastZslStillFrame %d is <= %d", requestId, - lastRegularFrameNumber, lastCompletedRegularFrameNumber, - lastReprocessFrameNumber, lastCompletedReprocessFrameNumber, - lastZslStillFrameNumber, lastCompletedZslStillFrameNumber)); - } - iter.remove(); - } else { - if (DEBUG) { - Log.v(TAG, "Sequence not yet completed for request id " + requestId); + holder.getCallback().onCaptureSequenceCompleted( + CameraDeviceImpl.this, + requestId, + requestLastFrameNumbers.getLastFrameNumber()); + } } - requestLastFrameNumbers.markInflightCompleted(); + }; + final long ident = Binder.clearCallingIdentity(); + try { + holder.getExecutor().execute(resultDispatch); + } finally { + Binder.restoreCallingIdentity(ident); } } } @@ -1759,12 +1702,6 @@ public class CameraDeviceImpl extends CameraDevice return; } - // Remove all capture callbacks now that device has gone to IDLE state. - removeCompletedCallbackHolderLocked( - Long.MAX_VALUE, /*lastCompletedRegularFrameNumber*/ - Long.MAX_VALUE, /*lastCompletedReprocessFrameNumber*/ - Long.MAX_VALUE /*lastCompletedZslStillFrameNumber*/); - if (!CameraDeviceImpl.this.mIdle) { final long ident = Binder.clearCallingIdentity(); try { @@ -1810,7 +1747,7 @@ public class CameraDeviceImpl extends CameraDevice return; } - checkEarlyTriggerSequenceCompleteLocked(mRepeatingRequestId, lastFrameNumber, + checkEarlyTriggerSequenceComplete(mRepeatingRequestId, lastFrameNumber, mRepeatingRequestTypes); // Check if there is already a new repeating request if (mRepeatingRequestId == repeatingRequestId) { @@ -1829,18 +1766,9 @@ public class CameraDeviceImpl extends CameraDevice public void onCaptureStarted(final CaptureResultExtras resultExtras, final long timestamp) { int requestId = resultExtras.getRequestId(); final long frameNumber = resultExtras.getFrameNumber(); - final long lastCompletedRegularFrameNumber = - resultExtras.getLastCompletedRegularFrameNumber(); - final long lastCompletedReprocessFrameNumber = - resultExtras.getLastCompletedReprocessFrameNumber(); - final long lastCompletedZslFrameNumber = - resultExtras.getLastCompletedZslFrameNumber(); if (DEBUG) { - Log.d(TAG, "Capture started for id " + requestId + " frame number " + frameNumber - + ": completedRegularFrameNumber " + lastCompletedRegularFrameNumber - + ", completedReprocessFrameNUmber " + lastCompletedReprocessFrameNumber - + ", completedZslFrameNumber " + lastCompletedZslFrameNumber); + Log.d(TAG, "Capture started for id " + requestId + " frame number " + frameNumber); } final CaptureCallbackHolder holder; @@ -1856,12 +1784,6 @@ public class CameraDeviceImpl extends CameraDevice return; } - // Check if it's okay to remove completed callbacks from mCaptureCallbackMap. - // A callback is completed if the corresponding inflight request has been removed - // from the inflight queue in cameraservice. - removeCompletedCallbackHolderLocked(lastCompletedRegularFrameNumber, - lastCompletedReprocessFrameNumber, lastCompletedZslFrameNumber); - // Get the callback for this frame ID, if there is one holder = CameraDeviceImpl.this.mCaptureCallbackMap.get(requestId); diff --git a/core/java/android/hardware/camera2/impl/CameraOfflineSessionImpl.java b/core/java/android/hardware/camera2/impl/CameraOfflineSessionImpl.java index 413caf5e22e0..1d9d644c9306 100644 --- a/core/java/android/hardware/camera2/impl/CameraOfflineSessionImpl.java +++ b/core/java/android/hardware/camera2/impl/CameraOfflineSessionImpl.java @@ -182,12 +182,6 @@ public class CameraOfflineSessionImpl extends CameraOfflineSession return; } - // Remove all capture callbacks now that device has gone to IDLE state. - removeCompletedCallbackHolderLocked( - Long.MAX_VALUE, /*lastCompletedRegularFrameNumber*/ - Long.MAX_VALUE, /*lastCompletedReprocessFrameNumber*/ - Long.MAX_VALUE /*lastCompletedZslStillFrameNumber*/); - Runnable idleDispatch = new Runnable() { @Override public void run() { @@ -210,22 +204,10 @@ public class CameraOfflineSessionImpl extends CameraOfflineSession public void onCaptureStarted(final CaptureResultExtras resultExtras, final long timestamp) { int requestId = resultExtras.getRequestId(); final long frameNumber = resultExtras.getFrameNumber(); - final long lastCompletedRegularFrameNumber = - resultExtras.getLastCompletedRegularFrameNumber(); - final long lastCompletedReprocessFrameNumber = - resultExtras.getLastCompletedReprocessFrameNumber(); - final long lastCompletedZslFrameNumber = - resultExtras.getLastCompletedZslFrameNumber(); final CaptureCallbackHolder holder; synchronized(mInterfaceLock) { - // Check if it's okay to remove completed callbacks from mCaptureCallbackMap. - // A callback is completed if the corresponding inflight request has been removed - // from the inflight queue in cameraservice. - removeCompletedCallbackHolderLocked(lastCompletedRegularFrameNumber, - lastCompletedReprocessFrameNumber, lastCompletedZslFrameNumber); - // Get the callback for this frame ID, if there is one holder = CameraOfflineSessionImpl.this.mCaptureCallbackMap.get(requestId); @@ -619,61 +601,6 @@ public class CameraOfflineSessionImpl extends CameraOfflineSession } } - private void removeCompletedCallbackHolderLocked(long lastCompletedRegularFrameNumber, - long lastCompletedReprocessFrameNumber, long lastCompletedZslStillFrameNumber) { - if (DEBUG) { - Log.v(TAG, String.format("remove completed callback holders for " - + "lastCompletedRegularFrameNumber %d, " - + "lastCompletedReprocessFrameNumber %d, " - + "lastCompletedZslStillFrameNumber %d", - lastCompletedRegularFrameNumber, - lastCompletedReprocessFrameNumber, - lastCompletedZslStillFrameNumber)); - } - - boolean isReprocess = false; - Iterator<RequestLastFrameNumbersHolder> iter = - mOfflineRequestLastFrameNumbersList.iterator(); - while (iter.hasNext()) { - final RequestLastFrameNumbersHolder requestLastFrameNumbers = iter.next(); - final int requestId = requestLastFrameNumbers.getRequestId(); - final CaptureCallbackHolder holder; - - int index = mCaptureCallbackMap.indexOfKey(requestId); - holder = (index >= 0) ? - mCaptureCallbackMap.valueAt(index) : null; - if (holder != null) { - long lastRegularFrameNumber = - requestLastFrameNumbers.getLastRegularFrameNumber(); - long lastReprocessFrameNumber = - requestLastFrameNumbers.getLastReprocessFrameNumber(); - long lastZslStillFrameNumber = - requestLastFrameNumbers.getLastZslStillFrameNumber(); - if (lastRegularFrameNumber <= lastCompletedRegularFrameNumber - && lastReprocessFrameNumber <= lastCompletedReprocessFrameNumber - && lastZslStillFrameNumber <= lastCompletedZslStillFrameNumber) { - if (requestLastFrameNumbers.isSequenceCompleted()) { - mCaptureCallbackMap.removeAt(index); - if (DEBUG) { - Log.v(TAG, String.format( - "Remove holder for requestId %d, because lastRegularFrame %d " - + "is <= %d, lastReprocessFrame %d is <= %d, " - + "lastZslStillFrame %d is <= %d", requestId, - lastRegularFrameNumber, lastCompletedRegularFrameNumber, - lastReprocessFrameNumber, lastCompletedReprocessFrameNumber, - lastZslStillFrameNumber, lastCompletedZslStillFrameNumber)); - } - - iter.remove(); - } else { - Log.e(TAG, "Sequence not yet completed for request id " + requestId); - continue; - } - } - } - } - } - public void notifyFailedSwitch() { synchronized(mInterfaceLock) { Runnable switchFailDispatch = new Runnable() { diff --git a/core/java/android/hardware/camera2/impl/CaptureResultExtras.java b/core/java/android/hardware/camera2/impl/CaptureResultExtras.java index 5d9da73fd5c0..1ff5bd562f2e 100644 --- a/core/java/android/hardware/camera2/impl/CaptureResultExtras.java +++ b/core/java/android/hardware/camera2/impl/CaptureResultExtras.java @@ -30,9 +30,6 @@ public class CaptureResultExtras implements Parcelable { private int partialResultCount; private int errorStreamId; private String errorPhysicalCameraId; - private long lastCompletedRegularFrameNumber; - private long lastCompletedReprocessFrameNumber; - private long lastCompletedZslFrameNumber; public static final @android.annotation.NonNull Parcelable.Creator<CaptureResultExtras> CREATOR = new Parcelable.Creator<CaptureResultExtras>() { @@ -54,9 +51,7 @@ public class CaptureResultExtras implements Parcelable { public CaptureResultExtras(int requestId, int subsequenceId, int afTriggerId, int precaptureTriggerId, long frameNumber, int partialResultCount, int errorStreamId, - String errorPhysicalCameraId, long lastCompletedRegularFrameNumber, - long lastCompletedReprocessFrameNumber, - long lastCompletedZslFrameNumber) { + String errorPhysicalCameraId) { this.requestId = requestId; this.subsequenceId = subsequenceId; this.afTriggerId = afTriggerId; @@ -65,9 +60,6 @@ public class CaptureResultExtras implements Parcelable { this.partialResultCount = partialResultCount; this.errorStreamId = errorStreamId; this.errorPhysicalCameraId = errorPhysicalCameraId; - this.lastCompletedRegularFrameNumber = lastCompletedRegularFrameNumber; - this.lastCompletedReprocessFrameNumber = lastCompletedReprocessFrameNumber; - this.lastCompletedZslFrameNumber = lastCompletedZslFrameNumber; } @Override @@ -90,9 +82,6 @@ public class CaptureResultExtras implements Parcelable { } else { dest.writeBoolean(false); } - dest.writeLong(lastCompletedRegularFrameNumber); - dest.writeLong(lastCompletedReprocessFrameNumber); - dest.writeLong(lastCompletedZslFrameNumber); } public void readFromParcel(Parcel in) { @@ -107,9 +96,6 @@ public class CaptureResultExtras implements Parcelable { if (errorPhysicalCameraIdPresent) { errorPhysicalCameraId = in.readString(); } - lastCompletedRegularFrameNumber = in.readLong(); - lastCompletedReprocessFrameNumber = in.readLong(); - lastCompletedZslFrameNumber = in.readLong(); } public String getErrorPhysicalCameraId() { @@ -143,16 +129,4 @@ public class CaptureResultExtras implements Parcelable { public int getErrorStreamId() { return errorStreamId; } - - public long getLastCompletedRegularFrameNumber() { - return lastCompletedRegularFrameNumber; - } - - public long getLastCompletedReprocessFrameNumber() { - return lastCompletedReprocessFrameNumber; - } - - public long getLastCompletedZslFrameNumber() { - return lastCompletedZslFrameNumber; - } } diff --git a/core/java/android/hardware/camera2/impl/RequestLastFrameNumbersHolder.java b/core/java/android/hardware/camera2/impl/RequestLastFrameNumbersHolder.java index 0ee4ebc1aa87..bd1df9e1ac7d 100644 --- a/core/java/android/hardware/camera2/impl/RequestLastFrameNumbersHolder.java +++ b/core/java/android/hardware/camera2/impl/RequestLastFrameNumbersHolder.java @@ -38,10 +38,6 @@ public class RequestLastFrameNumbersHolder { // The last ZSL still capture frame number for this request ID. It's // CaptureCallback.NO_FRAMES_CAPTURED if the request ID has no zsl request. private final long mLastZslStillFrameNumber; - // Whether the sequence is completed. (only consider capture result) - private boolean mSequenceCompleted; - // Whether the inflight request is completed. (consider result, buffers, and notifies) - private boolean mInflightCompleted; /** * Create a request-last-frame-numbers holder with a list of requests, request ID, and @@ -93,8 +89,6 @@ public class RequestLastFrameNumbersHolder { mLastReprocessFrameNumber = lastReprocessFrameNumber; mLastZslStillFrameNumber = lastZslStillFrameNumber; mRequestId = requestInfo.getRequestId(); - mSequenceCompleted = false; - mInflightCompleted = false; } /** @@ -143,8 +137,6 @@ public class RequestLastFrameNumbersHolder { mLastZslStillFrameNumber = lastZslStillFrameNumber; mLastReprocessFrameNumber = CameraCaptureSession.CaptureCallback.NO_FRAMES_CAPTURED; mRequestId = requestId; - mSequenceCompleted = false; - mInflightCompleted = false; } /** @@ -185,34 +177,5 @@ public class RequestLastFrameNumbersHolder { public int getRequestId() { return mRequestId; } - - /** - * Return whether the capture sequence is completed. - */ - public boolean isSequenceCompleted() { - return mSequenceCompleted; - } - - /** - * Mark the capture sequence as completed. - */ - public void markSequenceCompleted() { - mSequenceCompleted = true; - } - - /** - * Return whether the inflight capture is completed. - */ - public boolean isInflightCompleted() { - return mInflightCompleted; - } - - /** - * Mark the inflight capture as completed. - */ - public void markInflightCompleted() { - mInflightCompleted = true; - } - } diff --git a/core/java/android/hardware/camera2/legacy/LegacyCameraDevice.java b/core/java/android/hardware/camera2/legacy/LegacyCameraDevice.java index fdd578c419d8..fbc9ac3229c3 100644 --- a/core/java/android/hardware/camera2/legacy/LegacyCameraDevice.java +++ b/core/java/android/hardware/camera2/legacy/LegacyCameraDevice.java @@ -109,12 +109,11 @@ public class LegacyCameraDevice implements AutoCloseable { } if (holder == null) { return new CaptureResultExtras(ILLEGAL_VALUE, ILLEGAL_VALUE, ILLEGAL_VALUE, - ILLEGAL_VALUE, ILLEGAL_VALUE, ILLEGAL_VALUE, ILLEGAL_VALUE, null, - ILLEGAL_VALUE, ILLEGAL_VALUE, ILLEGAL_VALUE); + ILLEGAL_VALUE, ILLEGAL_VALUE, ILLEGAL_VALUE, ILLEGAL_VALUE, null); } return new CaptureResultExtras(holder.getRequestId(), holder.getSubsequeceId(), /*afTriggerId*/0, /*precaptureTriggerId*/0, holder.getFrameNumber(), - /*partialResultCount*/1, errorStreamId, null, holder.getFrameNumber(), -1, -1); + /*partialResultCount*/1, errorStreamId, null); } /** |