Merge "Camera: Fix race for onCaptureBufferLost callback (take 2)" into rvc-dev

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 checkEarlyTriggerSequenceComplete(
+    private void checkEarlyTriggerSequenceCompleteLocked(
             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) {
-                    checkEarlyTriggerSequenceComplete(mRepeatingRequestId,
+                    checkEarlyTriggerSequenceCompleteLocked(mRepeatingRequestId,
                             requestInfo.getLastFrameNumber(),
                             mRepeatingRequestTypes);
                 }
@@ -1269,7 +1269,7 @@ public class CameraDeviceImpl extends CameraDevice
                     return;
                 }
 
-                checkEarlyTriggerSequenceComplete(requestId, lastFrameNumber, requestTypes);
+                checkEarlyTriggerSequenceCompleteLocked(requestId, lastFrameNumber, requestTypes);
             }
         }
     }
@@ -1302,7 +1302,7 @@ public class CameraDeviceImpl extends CameraDevice
 
             long lastFrameNumber = mRemoteDevice.flush();
             if (mRepeatingRequestId != REQUEST_ID_NONE) {
-                checkEarlyTriggerSequenceComplete(mRepeatingRequestId, lastFrameNumber,
+                checkEarlyTriggerSequenceCompleteLocked(mRepeatingRequestId, lastFrameNumber,
                         mRepeatingRequestTypes);
                 mRepeatingRequestId = REQUEST_ID_NONE;
                 mRepeatingRequestTypes = null;
@@ -1442,56 +1442,43 @@ 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;
-            synchronized(mInterfaceLock) {
             if (mRemoteDevice == null) {
                 Log.w(TAG, "Camera closed while checking sequences");
                 return;
             }
-
+            if (!requestLastFrameNumbers.isSequenceCompleted()) {
-                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();
-                    // 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 "
+                                "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();
-                }
-            }
-
-            // 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();
                 }
 
                 // Call onCaptureSequenceCompleted
-            if (sequenceCompleted) {
+                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() {
@@ -1517,6 +1504,78 @@ public class CameraDeviceImpl extends CameraDevice
                     }
                 }
             }
+
+            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));
+                }
+                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) {
+
+                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);
+                    }
+                    requestLastFrameNumbers.markInflightCompleted();
+                }
+            }
+        }
     }
 
     public void onDeviceError(final int errorCode, CaptureResultExtras resultExtras) {
@@ -1702,6 +1761,12 @@ 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 {
@@ -1747,7 +1812,7 @@ public class CameraDeviceImpl extends CameraDevice
                     return;
                 }
 
-                checkEarlyTriggerSequenceComplete(mRepeatingRequestId, lastFrameNumber,
+                checkEarlyTriggerSequenceCompleteLocked(mRepeatingRequestId, lastFrameNumber,
                         mRepeatingRequestTypes);
                 // Check if there is already a new repeating request
                 if (mRepeatingRequestId == repeatingRequestId) {
@@ -1766,9 +1831,18 @@ 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);
+                Log.d(TAG, "Capture started for id " + requestId + " frame number " + frameNumber
+                        + ": completedRegularFrameNumber " + lastCompletedRegularFrameNumber
+                        + ", completedReprocessFrameNUmber " + lastCompletedReprocessFrameNumber
+                        + ", completedZslFrameNumber " + lastCompletedZslFrameNumber);
             }
             final CaptureCallbackHolder holder;
 
@@ -1784,6 +1858,12 @@ 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);
 

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

@@ -182,6 +182,12 @@ 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() {
@@ -204,10 +210,22 @@ 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);
 
@@ -601,6 +619,61 @@ 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() {

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

@@ -30,6 +30,9 @@ 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>() {
@@ -51,7 +54,9 @@ public class CaptureResultExtras implements Parcelable {
     public CaptureResultExtras(int requestId, int subsequenceId, int afTriggerId,
                                int precaptureTriggerId, long frameNumber,
                                int partialResultCount, int errorStreamId,
-                               String errorPhysicalCameraId) {
+                               String errorPhysicalCameraId, long lastCompletedRegularFrameNumber,
+                               long lastCompletedReprocessFrameNumber,
+                               long lastCompletedZslFrameNumber) {
         this.requestId = requestId;
         this.subsequenceId = subsequenceId;
         this.afTriggerId = afTriggerId;
@@ -60,6 +65,9 @@ public class CaptureResultExtras implements Parcelable {
         this.partialResultCount = partialResultCount;
         this.errorStreamId = errorStreamId;
         this.errorPhysicalCameraId = errorPhysicalCameraId;
+        this.lastCompletedRegularFrameNumber = lastCompletedRegularFrameNumber;
+        this.lastCompletedReprocessFrameNumber = lastCompletedReprocessFrameNumber;
+        this.lastCompletedZslFrameNumber = lastCompletedZslFrameNumber;
     }
 
     @Override
@@ -82,6 +90,9 @@ public class CaptureResultExtras implements Parcelable {
         } else {
             dest.writeBoolean(false);
         }
+        dest.writeLong(lastCompletedRegularFrameNumber);
+        dest.writeLong(lastCompletedReprocessFrameNumber);
+        dest.writeLong(lastCompletedZslFrameNumber);
     }
 
     public void readFromParcel(Parcel in) {
@@ -96,6 +107,9 @@ public class CaptureResultExtras implements Parcelable {
         if (errorPhysicalCameraIdPresent) {
             errorPhysicalCameraId = in.readString();
         }
+        lastCompletedRegularFrameNumber = in.readLong();
+        lastCompletedReprocessFrameNumber = in.readLong();
+        lastCompletedZslFrameNumber = in.readLong();
     }
 
     public String getErrorPhysicalCameraId() {
@@ -129,4 +143,16 @@ 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;
+    }
 }

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

@@ -38,6 +38,10 @@ 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
@@ -89,6 +93,8 @@ public class RequestLastFrameNumbersHolder {
         mLastReprocessFrameNumber = lastReprocessFrameNumber;
         mLastZslStillFrameNumber = lastZslStillFrameNumber;
         mRequestId = requestInfo.getRequestId();
+        mSequenceCompleted = false;
+        mInflightCompleted = false;
     }
 
     /**
@@ -137,6 +143,8 @@ public class RequestLastFrameNumbersHolder {
         mLastZslStillFrameNumber = lastZslStillFrameNumber;
         mLastReprocessFrameNumber = CameraCaptureSession.CaptureCallback.NO_FRAMES_CAPTURED;
         mRequestId = requestId;
+        mSequenceCompleted = false;
+        mInflightCompleted = false;
     }
 
     /**
@@ -177,5 +185,34 @@ 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;
+    }
+
 }
 

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

@@ -109,11 +109,12 @@ 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, null,
+                    ILLEGAL_VALUE, ILLEGAL_VALUE, ILLEGAL_VALUE);
         }
         return new CaptureResultExtras(holder.getRequestId(), holder.getSubsequeceId(),
                 /*afTriggerId*/0, /*precaptureTriggerId*/0, holder.getFrameNumber(),
-                /*partialResultCount*/1, errorStreamId, null);
+                /*partialResultCount*/1, errorStreamId, null, holder.getFrameNumber(), -1, -1);
     }
 
     /**