Revert "Camera: Add lastCompletedFrameNumber in CaptureResultExtras"
Revert "Camera: Fix race for onCaptureBufferLost callback"
Revert submission 11415576-bufferErrorLossFix
Reason for revert: Breaks Camera on at least the wembley device
Reverted Changes:
I12b716acc:Camera: Fix race for onCaptureBufferLost callback
I43f0f5ea1:Camera: Add lastCompletedFrameNumber in CaptureRes...
Bug: 158622719
Change-Id: I98440c52f61d571e1cb6692667fb067020746795
Test: Locally tried this revert on rvc-dev on a wembley, and now the Camera works
diff --git a/camera/ndk/impl/ACameraDevice.cpp b/camera/ndk/impl/ACameraDevice.cpp
index c15c5a5..0d7180a 100644
--- a/camera/ndk/impl/ACameraDevice.cpp
+++ b/camera/ndk/impl/ACameraDevice.cpp
@@ -1336,97 +1336,56 @@
void
CameraDevice::checkAndFireSequenceCompleteLocked() {
int64_t completedFrameNumber = mFrameNumberTracker.getCompletedFrameNumber();
+ //std::map<int, int64_t> mSequenceLastFrameNumberMap;
auto it = mSequenceLastFrameNumberMap.begin();
while (it != mSequenceLastFrameNumberMap.end()) {
int sequenceId = it->first;
- int64_t lastFrameNumber = it->second.lastFrameNumber;
- bool hasCallback = true;
+ int64_t lastFrameNumber = it->second;
+ bool seqCompleted = false;
+ bool hasCallback = true;
if (mRemote == nullptr) {
ALOGW("Camera %s closed while checking sequence complete", getId());
return;
}
- ALOGV("%s: seq %d's last frame number %" PRId64 ", completed %" PRId64,
- __FUNCTION__, sequenceId, lastFrameNumber, completedFrameNumber);
- if (!it->second.isSequenceCompleted) {
- // Check if there is callback for this sequence
- // This should not happen because we always register callback (with nullptr inside)
- if (mSequenceCallbackMap.count(sequenceId) == 0) {
- ALOGW("No callback found for sequenceId %d", sequenceId);
- hasCallback = false;
- }
- if (lastFrameNumber <= completedFrameNumber) {
- ALOGV("Mark sequenceId %d as sequence completed", sequenceId);
- it->second.isSequenceCompleted = true;
- }
-
- if (it->second.isSequenceCompleted && hasCallback) {
- auto cbIt = mSequenceCallbackMap.find(sequenceId);
- CallbackHolder cbh = cbIt->second;
-
- // send seq complete callback
- sp<AMessage> msg = new AMessage(kWhatCaptureSeqEnd, mHandler);
- msg->setPointer(kContextKey, cbh.mContext);
- msg->setObject(kSessionSpKey, cbh.mSession);
- msg->setPointer(kCallbackFpKey, (void*) cbh.mOnCaptureSequenceCompleted);
- msg->setInt32(kSequenceIdKey, sequenceId);
- msg->setInt64(kFrameNumberKey, lastFrameNumber);
-
- // Clear the session sp before we send out the message
- // This will guarantee the rare case where the message is processed
- // before cbh goes out of scope and causing we call the session
- // destructor while holding device lock
- cbh.mSession.clear();
- postSessionMsgAndCleanup(msg);
- }
+ // Check if there is callback for this sequence
+ // This should not happen because we always register callback (with nullptr inside)
+ if (mSequenceCallbackMap.count(sequenceId) == 0) {
+ ALOGW("No callback found for sequenceId %d", sequenceId);
+ hasCallback = false;
}
- if (it->second.isSequenceCompleted && it->second.isInflightCompleted) {
- if (mSequenceCallbackMap.find(sequenceId) != mSequenceCallbackMap.end()) {
- mSequenceCallbackMap.erase(sequenceId);
- }
+ if (lastFrameNumber <= completedFrameNumber) {
+ ALOGV("seq %d reached last frame %" PRId64 ", completed %" PRId64,
+ sequenceId, lastFrameNumber, completedFrameNumber);
+ seqCompleted = true;
+ }
+
+ if (seqCompleted && hasCallback) {
+ // remove callback holder from callback map
+ auto cbIt = mSequenceCallbackMap.find(sequenceId);
+ CallbackHolder cbh = cbIt->second;
+ mSequenceCallbackMap.erase(cbIt);
+ // send seq complete callback
+ sp<AMessage> msg = new AMessage(kWhatCaptureSeqEnd, mHandler);
+ msg->setPointer(kContextKey, cbh.mContext);
+ msg->setObject(kSessionSpKey, cbh.mSession);
+ msg->setPointer(kCallbackFpKey, (void*) cbh.mOnCaptureSequenceCompleted);
+ msg->setInt32(kSequenceIdKey, sequenceId);
+ msg->setInt64(kFrameNumberKey, lastFrameNumber);
+
+ // Clear the session sp before we send out the message
+ // This will guarantee the rare case where the message is processed
+ // before cbh goes out of scope and causing we call the session
+ // destructor while holding device lock
+ cbh.mSession.clear();
+ postSessionMsgAndCleanup(msg);
+ }
+
+ // No need to track sequence complete if there is no callback registered
+ if (seqCompleted || !hasCallback) {
it = mSequenceLastFrameNumberMap.erase(it);
- ALOGV("%s: Remove holder for sequenceId %d", __FUNCTION__, sequenceId);
- } else {
- ++it;
- }
- }
-}
-
-void
-CameraDevice::removeCompletedCallbackHolderLocked(int64_t lastCompletedRegularFrameNumber) {
- auto it = mSequenceLastFrameNumberMap.begin();
- while (it != mSequenceLastFrameNumberMap.end()) {
- int sequenceId = it->first;
- int64_t lastFrameNumber = it->second.lastFrameNumber;
-
- if (mRemote == nullptr) {
- ALOGW("Camera %s closed while checking sequence complete", getId());
- return;
- }
-
- ALOGV("%s: seq %d's last frame number %" PRId64
- ", completed inflight frame number %" PRId64,
- __FUNCTION__, sequenceId, lastFrameNumber,
- lastCompletedRegularFrameNumber);
- if (lastFrameNumber <= lastCompletedRegularFrameNumber) {
- if (it->second.isSequenceCompleted) {
- // Check if there is callback for this sequence
- // This should not happen because we always register callback (with nullptr inside)
- if (mSequenceCallbackMap.count(sequenceId) == 0) {
- ALOGW("No callback found for sequenceId %d", sequenceId);
- } else {
- mSequenceCallbackMap.erase(sequenceId);
- }
-
- it = mSequenceLastFrameNumberMap.erase(it);
- ALOGV("%s: Remove holder for sequenceId %d", __FUNCTION__, sequenceId);
- } else {
- ALOGV("Mark sequenceId %d as inflight completed", sequenceId);
- it->second.isInflightCompleted = true;
- ++it;
- }
} else {
++it;
}
@@ -1521,9 +1480,6 @@
return ret;
}
- dev->removeCompletedCallbackHolderLocked(
- std::numeric_limits<int64_t>::max()/*lastCompletedRegularFrameNumber*/);
-
if (dev->mIdle) {
// Already in idle state. Possibly other thread did waitUntilIdle
return ret;
@@ -1566,9 +1522,6 @@
return ret;
}
- dev->removeCompletedCallbackHolderLocked(
- resultExtras.lastCompletedRegularFrameNumber);
-
int sequenceId = resultExtras.requestId;
int32_t burstId = resultExtras.burstId;
diff --git a/camera/ndk/impl/ACameraDevice.h b/camera/ndk/impl/ACameraDevice.h
index 2f2299f..6c2ceb3 100644
--- a/camera/ndk/impl/ACameraDevice.h
+++ b/camera/ndk/impl/ACameraDevice.h
@@ -267,15 +267,8 @@
static const int REQUEST_ID_NONE = -1;
int mRepeatingSequenceId = REQUEST_ID_NONE;
- // sequence id -> last frame number holder map
- struct RequestLastFrameNumberHolder {
- int64_t lastFrameNumber;
- bool isSequenceCompleted = false;
- bool isInflightCompleted = false;
- RequestLastFrameNumberHolder(int64_t lastFN) :
- lastFrameNumber(lastFN) {}
- };
- std::map<int, RequestLastFrameNumberHolder> mSequenceLastFrameNumberMap;
+ // sequence id -> last frame number map
+ std::map<int, int64_t> mSequenceLastFrameNumberMap;
struct CallbackHolder {
CallbackHolder(sp<ACameraCaptureSession> session,
@@ -345,7 +338,6 @@
void checkRepeatingSequenceCompleteLocked(const int sequenceId, const int64_t lastFrameNumber);
void checkAndFireSequenceCompleteLocked();
- void removeCompletedCallbackHolderLocked(int64_t lastCompletedRegularFrameNumber);
// Misc variables
int32_t mShadingMapSize[2]; // const after constructor