Skip to content
This repository was archived by the owner on Apr 10, 2024. It is now read-only.

Commit e16fed2

Browse files
shuzhenwangshuzhenwang
committed
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
1 parent 115a829 commit e16fed2

File tree

5 files changed

+278
-61
lines changed

5 files changed

+278
-61
lines changed

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

Lines changed: 138 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,7 +1072,7 @@ public int captureBurst(List<CaptureRequest> requests, CaptureCallback callback,
10721072
* @param lastFrameNumber last frame number returned from binder.
10731073
* @param repeatingRequestTypes the repeating requests' types.
10741074
*/
1075-
private void checkEarlyTriggerSequenceComplete(
1075+
private void checkEarlyTriggerSequenceCompleteLocked(
10761076
final int requestId, final long lastFrameNumber,
10771077
final int[] repeatingRequestTypes) {
10781078
// lastFrameNumber being equal to NO_FRAMES_CAPTURED means that the request
@@ -1212,7 +1212,7 @@ private int submitCaptureRequest(List<CaptureRequest> requestList, CaptureCallba
12121212

12131213
if (repeating) {
12141214
if (mRepeatingRequestId != REQUEST_ID_NONE) {
1215-
checkEarlyTriggerSequenceComplete(mRepeatingRequestId,
1215+
checkEarlyTriggerSequenceCompleteLocked(mRepeatingRequestId,
12161216
requestInfo.getLastFrameNumber(),
12171217
mRepeatingRequestTypes);
12181218
}
@@ -1269,7 +1269,7 @@ public void stopRepeating() throws CameraAccessException {
12691269
return;
12701270
}
12711271

1272-
checkEarlyTriggerSequenceComplete(requestId, lastFrameNumber, requestTypes);
1272+
checkEarlyTriggerSequenceCompleteLocked(requestId, lastFrameNumber, requestTypes);
12731273
}
12741274
}
12751275
}
@@ -1302,7 +1302,7 @@ public void flush() throws CameraAccessException {
13021302

13031303
long lastFrameNumber = mRemoteDevice.flush();
13041304
if (mRepeatingRequestId != REQUEST_ID_NONE) {
1305-
checkEarlyTriggerSequenceComplete(mRepeatingRequestId, lastFrameNumber,
1305+
checkEarlyTriggerSequenceCompleteLocked(mRepeatingRequestId, lastFrameNumber,
13061306
mRepeatingRequestTypes);
13071307
mRepeatingRequestId = REQUEST_ID_NONE;
13081308
mRepeatingRequestTypes = null;
@@ -1442,78 +1442,137 @@ private void checkAndFireSequenceComplete() {
14421442
long completedFrameNumber = mFrameNumberTracker.getCompletedFrameNumber();
14431443
long completedReprocessFrameNumber = mFrameNumberTracker.getCompletedReprocessFrameNumber();
14441444
long completedZslStillFrameNumber = mFrameNumberTracker.getCompletedZslStillFrameNumber();
1445-
boolean isReprocess = false;
1445+
14461446
Iterator<RequestLastFrameNumbersHolder> iter = mRequestLastFrameNumbersList.iterator();
14471447
while (iter.hasNext()) {
14481448
final RequestLastFrameNumbersHolder requestLastFrameNumbers = iter.next();
1449-
boolean sequenceCompleted = false;
14501449
final int requestId = requestLastFrameNumbers.getRequestId();
14511450
final CaptureCallbackHolder holder;
1452-
synchronized(mInterfaceLock) {
1453-
if (mRemoteDevice == null) {
1454-
Log.w(TAG, "Camera closed while checking sequences");
1455-
return;
1451+
if (mRemoteDevice == null) {
1452+
Log.w(TAG, "Camera closed while checking sequences");
1453+
return;
1454+
}
1455+
if (!requestLastFrameNumbers.isSequenceCompleted()) {
1456+
long lastRegularFrameNumber =
1457+
requestLastFrameNumbers.getLastRegularFrameNumber();
1458+
long lastReprocessFrameNumber =
1459+
requestLastFrameNumbers.getLastReprocessFrameNumber();
1460+
long lastZslStillFrameNumber =
1461+
requestLastFrameNumbers.getLastZslStillFrameNumber();
1462+
if (lastRegularFrameNumber <= completedFrameNumber
1463+
&& lastReprocessFrameNumber <= completedReprocessFrameNumber
1464+
&& lastZslStillFrameNumber <= completedZslStillFrameNumber) {
1465+
if (DEBUG) {
1466+
Log.v(TAG, String.format(
1467+
"Mark requestId %d as completed, because lastRegularFrame %d "
1468+
+ "is <= %d, lastReprocessFrame %d is <= %d, "
1469+
+ "lastZslStillFrame %d is <= %d", requestId,
1470+
lastRegularFrameNumber, completedFrameNumber,
1471+
lastReprocessFrameNumber, completedReprocessFrameNumber,
1472+
lastZslStillFrameNumber, completedZslStillFrameNumber));
1473+
}
1474+
requestLastFrameNumbers.markSequenceCompleted();
14561475
}
14571476

1477+
// Call onCaptureSequenceCompleted
14581478
int index = mCaptureCallbackMap.indexOfKey(requestId);
14591479
holder = (index >= 0) ?
14601480
mCaptureCallbackMap.valueAt(index) : null;
1461-
if (holder != null) {
1462-
long lastRegularFrameNumber =
1463-
requestLastFrameNumbers.getLastRegularFrameNumber();
1464-
long lastReprocessFrameNumber =
1465-
requestLastFrameNumbers.getLastReprocessFrameNumber();
1466-
long lastZslStillFrameNumber =
1467-
requestLastFrameNumbers.getLastZslStillFrameNumber();
1468-
// check if it's okay to remove request from mCaptureCallbackMap
1469-
if (lastRegularFrameNumber <= completedFrameNumber
1470-
&& lastReprocessFrameNumber <= completedReprocessFrameNumber
1471-
&& lastZslStillFrameNumber <= completedZslStillFrameNumber) {
1472-
sequenceCompleted = true;
1473-
mCaptureCallbackMap.removeAt(index);
1474-
if (DEBUG) {
1475-
Log.v(TAG, String.format(
1476-
"Remove holder for requestId %d, because lastRegularFrame %d "
1477-
+ "is <= %d, lastReprocessFrame %d is <= %d, "
1478-
+ "lastZslStillFrame %d is <= %d", requestId,
1479-
lastRegularFrameNumber, completedFrameNumber,
1480-
lastReprocessFrameNumber, completedReprocessFrameNumber,
1481-
lastZslStillFrameNumber, completedZslStillFrameNumber));
1481+
if (holder != null && requestLastFrameNumbers.isSequenceCompleted()) {
1482+
Runnable resultDispatch = new Runnable() {
1483+
@Override
1484+
public void run() {
1485+
if (!CameraDeviceImpl.this.isClosed()){
1486+
if (DEBUG) {
1487+
Log.d(TAG, String.format(
1488+
"fire sequence complete for request %d",
1489+
requestId));
1490+
}
1491+
1492+
holder.getCallback().onCaptureSequenceCompleted(
1493+
CameraDeviceImpl.this,
1494+
requestId,
1495+
requestLastFrameNumbers.getLastFrameNumber());
1496+
}
14821497
}
1498+
};
1499+
final long ident = Binder.clearCallingIdentity();
1500+
try {
1501+
holder.getExecutor().execute(resultDispatch);
1502+
} finally {
1503+
Binder.restoreCallingIdentity(ident);
14831504
}
14841505
}
14851506
}
14861507

1487-
// If no callback is registered for this requestId or sequence completed, remove it
1488-
// from the frame number->request pair because it's not needed anymore.
1489-
if (holder == null || sequenceCompleted) {
1508+
if (requestLastFrameNumbers.isSequenceCompleted() &&
1509+
requestLastFrameNumbers.isInflightCompleted()) {
1510+
int index = mCaptureCallbackMap.indexOfKey(requestId);
1511+
if (index >= 0) {
1512+
mCaptureCallbackMap.removeAt(index);
1513+
}
1514+
if (DEBUG) {
1515+
Log.v(TAG, String.format(
1516+
"Remove holder for requestId %d", requestId));
1517+
}
14901518
iter.remove();
14911519
}
1520+
}
1521+
}
14921522

1493-
// Call onCaptureSequenceCompleted
1494-
if (sequenceCompleted) {
1495-
Runnable resultDispatch = new Runnable() {
1496-
@Override
1497-
public void run() {
1498-
if (!CameraDeviceImpl.this.isClosed()){
1499-
if (DEBUG) {
1500-
Log.d(TAG, String.format(
1501-
"fire sequence complete for request %d",
1502-
requestId));
1503-
}
1523+
private void removeCompletedCallbackHolderLocked(long lastCompletedRegularFrameNumber,
1524+
long lastCompletedReprocessFrameNumber, long lastCompletedZslStillFrameNumber) {
1525+
if (DEBUG) {
1526+
Log.v(TAG, String.format("remove completed callback holders for "
1527+
+ "lastCompletedRegularFrameNumber %d, "
1528+
+ "lastCompletedReprocessFrameNumber %d, "
1529+
+ "lastCompletedZslStillFrameNumber %d",
1530+
lastCompletedRegularFrameNumber,
1531+
lastCompletedReprocessFrameNumber,
1532+
lastCompletedZslStillFrameNumber));
1533+
}
15041534

1505-
holder.getCallback().onCaptureSequenceCompleted(
1506-
CameraDeviceImpl.this,
1507-
requestId,
1508-
requestLastFrameNumbers.getLastFrameNumber());
1509-
}
1535+
Iterator<RequestLastFrameNumbersHolder> iter = mRequestLastFrameNumbersList.iterator();
1536+
while (iter.hasNext()) {
1537+
final RequestLastFrameNumbersHolder requestLastFrameNumbers = iter.next();
1538+
final int requestId = requestLastFrameNumbers.getRequestId();
1539+
final CaptureCallbackHolder holder;
1540+
if (mRemoteDevice == null) {
1541+
Log.w(TAG, "Camera closed while removing completed callback holders");
1542+
return;
1543+
}
1544+
1545+
long lastRegularFrameNumber =
1546+
requestLastFrameNumbers.getLastRegularFrameNumber();
1547+
long lastReprocessFrameNumber =
1548+
requestLastFrameNumbers.getLastReprocessFrameNumber();
1549+
long lastZslStillFrameNumber =
1550+
requestLastFrameNumbers.getLastZslStillFrameNumber();
1551+
1552+
if (lastRegularFrameNumber <= lastCompletedRegularFrameNumber
1553+
&& lastReprocessFrameNumber <= lastCompletedReprocessFrameNumber
1554+
&& lastZslStillFrameNumber <= lastCompletedZslStillFrameNumber) {
1555+
1556+
if (requestLastFrameNumbers.isSequenceCompleted()) {
1557+
int index = mCaptureCallbackMap.indexOfKey(requestId);
1558+
if (index >= 0) {
1559+
mCaptureCallbackMap.removeAt(index);
15101560
}
1511-
};
1512-
final long ident = Binder.clearCallingIdentity();
1513-
try {
1514-
holder.getExecutor().execute(resultDispatch);
1515-
} finally {
1516-
Binder.restoreCallingIdentity(ident);
1561+
if (DEBUG) {
1562+
Log.v(TAG, String.format(
1563+
"Remove holder for requestId %d, because lastRegularFrame %d "
1564+
+ "is <= %d, lastReprocessFrame %d is <= %d, "
1565+
+ "lastZslStillFrame %d is <= %d", requestId,
1566+
lastRegularFrameNumber, lastCompletedRegularFrameNumber,
1567+
lastReprocessFrameNumber, lastCompletedReprocessFrameNumber,
1568+
lastZslStillFrameNumber, lastCompletedZslStillFrameNumber));
1569+
}
1570+
iter.remove();
1571+
} else {
1572+
if (DEBUG) {
1573+
Log.v(TAG, "Sequence not yet completed for request id " + requestId);
1574+
}
1575+
requestLastFrameNumbers.markInflightCompleted();
15171576
}
15181577
}
15191578
}
@@ -1702,6 +1761,12 @@ public void onDeviceIdle() {
17021761
return;
17031762
}
17041763

1764+
// Remove all capture callbacks now that device has gone to IDLE state.
1765+
removeCompletedCallbackHolderLocked(
1766+
Long.MAX_VALUE, /*lastCompletedRegularFrameNumber*/
1767+
Long.MAX_VALUE, /*lastCompletedReprocessFrameNumber*/
1768+
Long.MAX_VALUE /*lastCompletedZslStillFrameNumber*/);
1769+
17051770
if (!CameraDeviceImpl.this.mIdle) {
17061771
final long ident = Binder.clearCallingIdentity();
17071772
try {
@@ -1747,7 +1812,7 @@ public void onRepeatingRequestError(long lastFrameNumber, int repeatingRequestId
17471812
return;
17481813
}
17491814

1750-
checkEarlyTriggerSequenceComplete(mRepeatingRequestId, lastFrameNumber,
1815+
checkEarlyTriggerSequenceCompleteLocked(mRepeatingRequestId, lastFrameNumber,
17511816
mRepeatingRequestTypes);
17521817
// Check if there is already a new repeating request
17531818
if (mRepeatingRequestId == repeatingRequestId) {
@@ -1766,9 +1831,18 @@ public void onDeviceIdle() {
17661831
public void onCaptureStarted(final CaptureResultExtras resultExtras, final long timestamp) {
17671832
int requestId = resultExtras.getRequestId();
17681833
final long frameNumber = resultExtras.getFrameNumber();
1834+
final long lastCompletedRegularFrameNumber =
1835+
resultExtras.getLastCompletedRegularFrameNumber();
1836+
final long lastCompletedReprocessFrameNumber =
1837+
resultExtras.getLastCompletedReprocessFrameNumber();
1838+
final long lastCompletedZslFrameNumber =
1839+
resultExtras.getLastCompletedZslFrameNumber();
17691840

17701841
if (DEBUG) {
1771-
Log.d(TAG, "Capture started for id " + requestId + " frame number " + frameNumber);
1842+
Log.d(TAG, "Capture started for id " + requestId + " frame number " + frameNumber
1843+
+ ": completedRegularFrameNumber " + lastCompletedRegularFrameNumber
1844+
+ ", completedReprocessFrameNUmber " + lastCompletedReprocessFrameNumber
1845+
+ ", completedZslFrameNumber " + lastCompletedZslFrameNumber);
17721846
}
17731847
final CaptureCallbackHolder holder;
17741848

@@ -1784,6 +1858,12 @@ public void onCaptureStarted(final CaptureResultExtras resultExtras, final long
17841858
return;
17851859
}
17861860

1861+
// Check if it's okay to remove completed callbacks from mCaptureCallbackMap.
1862+
// A callback is completed if the corresponding inflight request has been removed
1863+
// from the inflight queue in cameraservice.
1864+
removeCompletedCallbackHolderLocked(lastCompletedRegularFrameNumber,
1865+
lastCompletedReprocessFrameNumber, lastCompletedZslFrameNumber);
1866+
17871867
// Get the callback for this frame ID, if there is one
17881868
holder = CameraDeviceImpl.this.mCaptureCallbackMap.get(requestId);
17891869

0 commit comments

Comments
 (0)