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

Commit e16fed20 authored by Shuzhen Wang's avatar Shuzhen Wang
Browse files

Camera: Fix race for onCaptureBufferLost callback (take 2) 

The callback holder was removed when the capture sequence is
completed, which is too soon because the buffer loss callback could
potentially arrives later than the capture sequence completion.

Defer the deletion of the callback holder to when the native inflight
request is removed, which takes into consideration of error
notifications.

Test: Camera CTS
Bug: 155353799
Change-Id: I56b9bfbe182ba6fc0ec2cb543fc32774ed3f6f1a
parent 115a8293

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

+138 −58
Original line number Diff line number Diff line
@@ -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;

            }



                int index = mCaptureCallbackMap.indexOfKey(requestId);

                holder = (index >= 0) ?

                        mCaptureCallbackMap.valueAt(index) : null;

                if (holder != null) {

            if (!requestLastFrameNumbers.isSequenceCompleted()) {

                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));

                    }

                    }

                }

            }



            // 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();

                    requestLastFrameNumbers.markSequenceCompleted();

                }



                // 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

+73 −0
Original line number Diff line number Diff line
@@ -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

+27 −1
Original line number Diff line number Diff line
@@ -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

+37 −0
Original line number Diff line number Diff line
@@ -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

+3 −2
Original line number Diff line number Diff line
@@ -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);

    }



    /**