Camera: prevent requestStreamBuffers break IDLE state
Test: CTS
Bug: 109829698
Change-Id: Ib9092e4e6b7d6cee2fbd96a3209e3b760f9fdaf0
diff --git a/services/camera/libcameraservice/device3/Camera3Device.cpp b/services/camera/libcameraservice/device3/Camera3Device.cpp
index f829b65..fa4e036 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.cpp
+++ b/services/camera/libcameraservice/device3/Camera3Device.cpp
@@ -224,6 +224,17 @@
ANDROID_INFO_SUPPORTED_BUFFER_MANAGEMENT_VERSION_HIDL_DEVICE_3_5);
}
+ if (mUseHalBufManager) {
+ res = mRequestBufferSM.initialize(mStatusTracker);
+ if (res != OK) {
+ SET_ERR_L("Unable to start request buffer state machine: %s (%d)",
+ strerror(-res), res);
+ mInterface->close();
+ mStatusTracker.clear();
+ return res;
+ }
+ }
+
/** Start up request queue thread */
mRequestThread = new RequestThread(
this, mStatusTracker, mInterface, sessionParamKeys, mUseHalBufManager);
@@ -951,6 +962,13 @@
return hardware::Void();
}
+ if (bufReqs.size() > mOutputStreams.size()) {
+ ALOGE("%s: too many buffer requests (%zu > # of output streams %zu)",
+ __FUNCTION__, bufReqs.size(), mOutputStreams.size());
+ _hidl_cb(BufferRequestStatus::FAILED_ILLEGAL_ARGUMENTS, bufRets);
+ return hardware::Void();
+ }
+
// Check for repeated streamId
for (const auto& bufReq : bufReqs) {
if (streamIds.indexOf(bufReq.streamId) != NAME_NOT_FOUND) {
@@ -962,19 +980,10 @@
streamIds.add(bufReq.streamId);
}
- // TODO: check we are not configuring streams. If so return FAILED_CONFIGURING
- // Probably need to hook CameraDeviceClient::beginConfigure and figure something
- // out for API1 client... maybe grab mLock and check mNeedConfig but then we will
- // need to wait until mLock is released...
- // _hidl_cb(BufferRequestStatus::FAILED_CONFIGURING, bufRets);
- // return hardware::Void();
-
- // TODO: here we start accessing mOutputStreams, might need mLock, but that
- // might block incoming API calls. Not sure how bad is it.
- if (bufReqs.size() > mOutputStreams.size()) {
- ALOGE("%s: too many buffer requests (%zu > # of output streams %zu)",
- __FUNCTION__, bufReqs.size(), mOutputStreams.size());
- _hidl_cb(BufferRequestStatus::FAILED_ILLEGAL_ARGUMENTS, bufRets);
+ if (!mRequestBufferSM.startRequestBuffer()) {
+ ALOGE("%s: request buffer disallowed while camera service is configuring",
+ __FUNCTION__);
+ _hidl_cb(BufferRequestStatus::FAILED_CONFIGURING, bufRets);
return hardware::Void();
}
@@ -991,6 +1000,7 @@
ALOGE("%s: Output stream id %d not found!", __FUNCTION__, streamId);
hardware::hidl_vec<StreamBufferRet> emptyBufRets;
_hidl_cb(BufferRequestStatus::FAILED_ILLEGAL_ARGUMENTS, emptyBufRets);
+ mRequestBufferSM.endRequestBuffer();
return hardware::Void();
}
@@ -1077,12 +1087,12 @@
returnOutputBuffers(streamBuffers.data(), numAllocatedBuffers, 0);
}
}
- // End of mOutputStreams access
_hidl_cb(allReqsSucceeds ? BufferRequestStatus::OK :
oneReqSucceeds ? BufferRequestStatus::FAILED_PARTIAL :
BufferRequestStatus::FAILED_UNKNOWN,
bufRets);
+ mRequestBufferSM.endRequestBuffer();
return hardware::Void();
}
@@ -2132,6 +2142,15 @@
mStatusWaiters++;
+ // Notify HAL to start draining. We need to notify the HalInterface layer
+ // even when the device is already IDLE, so HalInterface can reject incoming
+ // requestStreamBuffers call.
+ if (!active && mUseHalBufManager) {
+ auto streamIds = mOutputStreams.getStreamIds();
+ mRequestThread->signalPipelineDrain(streamIds);
+ mRequestBufferSM.onWaitUntilIdle();
+ }
+
bool stateSeen = false;
do {
if (active == (mStatus == STATUS_ACTIVE)) {
@@ -2139,12 +2158,6 @@
break;
}
- // Notify HAL to start draining
- if (!active && mUseHalBufManager) {
- auto streamIds = mOutputStreams.getStreamIds();
- mRequestThread->signalPipelineDrain(streamIds);
- }
-
res = mStatusChanged.waitRelative(mLock, timeout);
if (res != OK) break;
@@ -2889,6 +2902,10 @@
return rc;
}
+ if (mDummyStreamId == NO_STREAM) {
+ mRequestBufferSM.onStreamsConfigured();
+ }
+
return OK;
}
@@ -3059,6 +3076,7 @@
// Indicate idle inFlightMap to the status tracker
if (mInFlightMap.size() == 0) {
+ mRequestBufferSM.onInflightMapEmpty();
// Hold a separate dedicated tracker lock to prevent race with disconnect and also
// avoid a deadlock during reprocess requests.
Mutex::Autolock l(mTrackerLock);
@@ -5097,6 +5115,13 @@
nsecs_t tRequestEnd = systemTime(SYSTEM_TIME_MONOTONIC);
mRequestLatency.add(tRequestStart, tRequestEnd);
+ if (submitRequestSuccess) {
+ sp<Camera3Device> parent = mParent.promote();
+ if (parent != nullptr) {
+ parent->mRequestBufferSM.onRequestSubmitted();
+ }
+ }
+
if (useFlushLock) {
mFlushLock.unlock();
}
@@ -5430,7 +5455,8 @@
Mutex::Autolock pl(mPauseLock);
if (mPaused) {
- return mInterface->signalPipelineDrain(streamIds);
+ mInterface->signalPipelineDrain(streamIds);
+ return;
}
// If request thread is still busy, wait until paused then notify HAL
mNotifyPipelineDrain = true;
@@ -5619,6 +5645,10 @@
mNotifyPipelineDrain = false;
mStreamIdsToBeDrained.clear();
}
+ sp<Camera3Device> parent = mParent.promote();
+ if (parent != nullptr) {
+ parent->mRequestBufferSM.onRequestThreadPaused();
+ }
}
// Stop waiting for now and let thread management happen
return NULL;
@@ -5708,6 +5738,10 @@
mNotifyPipelineDrain = false;
mStreamIdsToBeDrained.clear();
}
+ sp<Camera3Device> parent = mParent.promote();
+ if (parent != nullptr) {
+ parent->mRequestBufferSM.onRequestThreadPaused();
+ }
}
res = mDoPauseSignal.waitRelative(mPauseLock, kRequestTimeout);
@@ -6163,4 +6197,105 @@
return true;
}
+status_t Camera3Device::RequestBufferStateMachine::initialize(
+ sp<camera3::StatusTracker> statusTracker) {
+ if (statusTracker == nullptr) {
+ ALOGE("%s: statusTracker is null", __FUNCTION__);
+ return BAD_VALUE;
+ }
+
+ std::lock_guard<std::mutex> lock(mLock);
+ mStatusTracker = statusTracker;
+ mRequestBufferStatusId = statusTracker->addComponent();
+ return OK;
+}
+
+bool Camera3Device::RequestBufferStateMachine::startRequestBuffer() {
+ std::lock_guard<std::mutex> lock(mLock);
+ if (mStatus == RB_STATUS_READY) {
+ mRequestBufferOngoing = true;
+ return true;
+ }
+ return false;
+}
+
+void Camera3Device::RequestBufferStateMachine::endRequestBuffer() {
+ std::lock_guard<std::mutex> lock(mLock);
+ if (!mRequestBufferOngoing) {
+ ALOGE("%s called without a successful startRequestBuffer call first!", __FUNCTION__);
+ return;
+ }
+ mRequestBufferOngoing = false;
+ if (mStatus == RB_STATUS_PENDING_STOP) {
+ checkSwitchToStopLocked();
+ }
+}
+
+void Camera3Device::RequestBufferStateMachine::onStreamsConfigured() {
+ std::lock_guard<std::mutex> lock(mLock);
+ RequestBufferState oldStatus = mStatus;
+ mStatus = RB_STATUS_READY;
+ if (oldStatus != RB_STATUS_READY) {
+ notifyTrackerLocked(/*active*/true);
+ }
+ return;
+}
+
+void Camera3Device::RequestBufferStateMachine::onRequestSubmitted() {
+ std::lock_guard<std::mutex> lock(mLock);
+ mRequestThreadPaused = false;
+ mInflightMapEmpty = false;
+ if (mStatus == RB_STATUS_STOPPED) {
+ mStatus = RB_STATUS_READY;
+ notifyTrackerLocked(/*active*/true);
+ }
+ return;
+}
+
+void Camera3Device::RequestBufferStateMachine::onRequestThreadPaused() {
+ std::lock_guard<std::mutex> lock(mLock);
+ mRequestThreadPaused = true;
+ if (mStatus == RB_STATUS_PENDING_STOP) {
+ checkSwitchToStopLocked();
+ }
+ return;
+}
+
+void Camera3Device::RequestBufferStateMachine::onInflightMapEmpty() {
+ std::lock_guard<std::mutex> lock(mLock);
+ mInflightMapEmpty = true;
+ if (mStatus == RB_STATUS_PENDING_STOP) {
+ checkSwitchToStopLocked();
+ }
+ return;
+}
+
+void Camera3Device::RequestBufferStateMachine::onWaitUntilIdle() {
+ std::lock_guard<std::mutex> lock(mLock);
+ if (!checkSwitchToStopLocked()) {
+ mStatus = RB_STATUS_PENDING_STOP;
+ }
+ return;
+}
+
+void Camera3Device::RequestBufferStateMachine::notifyTrackerLocked(bool active) {
+ sp<StatusTracker> statusTracker = mStatusTracker.promote();
+ if (statusTracker != nullptr) {
+ if (active) {
+ statusTracker->markComponentActive(mRequestBufferStatusId);
+ } else {
+ statusTracker->markComponentIdle(mRequestBufferStatusId, Fence::NO_FENCE);
+ }
+ }
+}
+
+bool Camera3Device::RequestBufferStateMachine::checkSwitchToStopLocked() {
+ if (mInflightMapEmpty && mRequestThreadPaused && !mRequestBufferOngoing) {
+ mStatus = RB_STATUS_STOPPED;
+ notifyTrackerLocked(/*active*/false);
+ return true;
+ }
+ return false;
+}
+
}; // namespace android
diff --git a/services/camera/libcameraservice/device3/Camera3Device.h b/services/camera/libcameraservice/device3/Camera3Device.h
index 5c0f570..4f5be6b 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.h
+++ b/services/camera/libcameraservice/device3/Camera3Device.h
@@ -1249,12 +1249,81 @@
// b/79972865
Mutex mTrackerLock;
- // Whether HAL request buffers through requestStreamBuffer API
+ // Whether HAL request buffers through requestStreamBuffers API
bool mUseHalBufManager = false;
// Lock to ensure requestStreamBuffers() callbacks are serialized
std::mutex mRequestBufferInterfaceLock;
+ // The state machine to control when requestStreamBuffers should allow
+ // HAL to request buffers.
+ enum RequestBufferState {
+ /**
+ * This is the initial state.
+ * requestStreamBuffers call will return FAILED_CONFIGURING in this state.
+ * Will switch to RB_STATUS_READY after a successful configureStreams or
+ * processCaptureRequest call.
+ */
+ RB_STATUS_STOPPED,
+
+ /**
+ * requestStreamBuffers call will proceed in this state.
+ * When device is asked to stay idle via waitUntilStateThenRelock() call:
+ * - Switch to RB_STATUS_STOPPED if there is no inflight requests and
+ * request thread is paused.
+ * - Switch to RB_STATUS_PENDING_STOP otherwise
+ */
+ RB_STATUS_READY,
+
+ /**
+ * requestStreamBuffers call will proceed in this state.
+ * Switch to RB_STATUS_STOPPED when all inflight requests are fulfilled
+ * and request thread is paused
+ */
+ RB_STATUS_PENDING_STOP,
+ };
+
+ class RequestBufferStateMachine {
+ public:
+ status_t initialize(sp<camera3::StatusTracker> statusTracker);
+
+ // Return if the state machine currently allows for requestBuffers
+ // If the state allows for it, mRequestBufferOngoing will be set to true
+ // and caller must call endRequestBuffer() later to unset the flag
+ bool startRequestBuffer();
+ void endRequestBuffer();
+
+ // Events triggered by application API call
+ void onStreamsConfigured();
+ void onWaitUntilIdle();
+
+ // Events usually triggered by hwBinder processCaptureResult callback thread
+ // But can also be triggered on request thread for failed request, or on
+ // hwbinder notify callback thread for shutter/error callbacks
+ void onInflightMapEmpty();
+
+ // Events triggered by RequestThread
+ void onRequestSubmitted();
+ void onRequestThreadPaused();
+
+ private:
+ void notifyTrackerLocked(bool active);
+
+ // Switch to STOPPED state and return true if all conditions allows for it.
+ // Otherwise do nothing and return false.
+ bool checkSwitchToStopLocked();
+
+ std::mutex mLock;
+ RequestBufferState mStatus = RB_STATUS_STOPPED;
+
+ bool mRequestThreadPaused = true;
+ bool mInflightMapEmpty = true;
+ bool mRequestBufferOngoing = false;
+
+ wp<camera3::StatusTracker> mStatusTracker;
+ int mRequestBufferStatusId;
+ } mRequestBufferSM;
+
}; // class Camera3Device
}; // namespace android