Camera2: On stopping preview, wait until captures are completed.

If preview is stopped while a still capture or a video snapshot is
underway, wait until the capture completes before stopping preview.

Also use the same behavior for disconnect - do not shut down the
camera device until captures are complete.

This works around HAL implementations which report to be done
(get_in_progress_count returns 0) even when a capture is still
outstanding.

Bug: 7276954

Change-Id: I66290acf1740cc330eadecbcded0c59fa9c5d2fd

4 files changed, 57 insertions, 44 deletions

diff --git a/services/camera/libcameraservice/Camera2Client.cpp b/services/camera/libcameraservice/Camera2Client.cpp
index 3239483..e073e39 100644
--- a/services/camera/libcameraservice/Camera2Client.cpp
+++ b/services/camera/libcameraservice/Camera2Client.cpp
@@ -378,16 +378,7 @@ void Camera2Client::disconnect() {
 
     ALOGV("Camera %d: Shutting down", mCameraId);
 
-    res = mStreamingProcessor->stopStream();
-    if (res != OK) {
-        ALOGE("%s: Problem stopping streaming: %s (%d)",
-                __FUNCTION__, strerror(-res), res);
-    }
-    res = mDevice->waitUntilDrained();
-    if (res != OK) {
-        ALOGE("%s: Problem waiting for HAL: %s (%d)",
-                __FUNCTION__, strerror(-res), res);
-    }
+    stopPreviewL();
 
     {
         SharedParameters::Lock l(mParameters);
@@ -733,6 +724,7 @@ void Camera2Client::stopPreview() {
 void Camera2Client::stopPreviewL() {
     ATRACE_CALL();
     status_t res;
+    const nsecs_t kStopCaptureTimeout = 3000000000LL; // 3 seconds
     Parameters::State state;
     {
         SharedParameters::Lock l(mParameters);
@@ -745,13 +737,11 @@ void Camera2Client::stopPreviewL() {
                     __FUNCTION__, mCameraId);
             break;
         case Parameters::STOPPED:
-            break;
+        case Parameters::VIDEO_SNAPSHOT:
         case Parameters::STILL_CAPTURE:
-            ALOGE("%s: Camera %d: Cannot stop preview during still capture.",
-                    __FUNCTION__, mCameraId);
-            break;
+            mCaptureSequencer->waitUntilIdle(kStopCaptureTimeout);
+            // no break
         case Parameters::RECORD:
-            // no break - identical to preview
         case Parameters::PREVIEW:
             mStreamingProcessor->stopStream();
             res = mDevice->waitUntilDrained();
diff --git a/services/camera/libcameraservice/camera2/CaptureSequencer.cpp b/services/camera/libcameraservice/camera2/CaptureSequencer.cpp
index a849246..5156539 100644
--- a/services/camera/libcameraservice/camera2/CaptureSequencer.cpp
+++ b/services/camera/libcameraservice/camera2/CaptureSequencer.cpp
@@ -72,6 +72,23 @@ status_t CaptureSequencer::startCapture() {
     return OK;
 }
 
+status_t CaptureSequencer::waitUntilIdle(nsecs_t timeout) {
+    ATRACE_CALL();
+    ALOGV("%s: Waiting for idle", __FUNCTION__);
+    Mutex::Autolock l(mStateMutex);
+    status_t res = -1;
+    while (mCaptureState != IDLE) {
+        nsecs_t startTime = systemTime();
+
+        res = mStateChanged.waitRelative(mStateMutex, timeout);
+        if (res != OK) return res;
+
+        timeout -= (systemTime() - startTime);
+    }
+    ALOGV("%s: Now idle", __FUNCTION__);
+    return OK;
+}
+
 void CaptureSequencer::notifyAutoExposure(uint8_t newState, int triggerId) {
     ATRACE_CALL();
     Mutex::Autolock l(mInputMutex);
@@ -137,8 +154,9 @@ const char* CaptureSequencer::kStateNames[CaptureSequencer::NUM_CAPTURE_STATES+1
     "ZSL_WAITING",
     "ZSL_REPROCESSING",
     "STANDARD_START",
-    "STANDARD_PRECAPTURE",
-    "STANDARD_CAPTURING",
+    "STANDARD_PRECAPTURE_WAIT",
+    "STANDARD_CAPTURE",
+    "STANDARD_CAPTURE_WAIT",
     "BURST_CAPTURE_START",
     "BURST_CAPTURE_WAIT",
     "DONE",
@@ -168,15 +186,26 @@ bool CaptureSequencer::threadLoop() {
     sp<Camera2Client> client = mClient.promote();
     if (client == 0) return false;
 
-    if (mCaptureState < ERROR) {
-        CaptureState oldState = mCaptureState;
-        mCaptureState = (this->*kStateManagers[mCaptureState])(client);
-        if (ATRACE_ENABLED() && oldState != mCaptureState) {
-            ATRACE_INT("cam2_capt_state", mCaptureState);
-        }
-    } else {
-        ALOGE("%s: Bad capture state: %s",
-                __FUNCTION__, kStateNames[mCaptureState]);
+    CaptureState currentState;
+    {
+        Mutex::Autolock l(mStateMutex);
+        currentState = mCaptureState;
+    }
+
+    currentState = (this->*kStateManagers[currentState])(client);
+
+    Mutex::Autolock l(mStateMutex);
+    if (currentState != mCaptureState) {
+        mCaptureState = currentState;
+        ATRACE_INT("cam2_capt_state", mCaptureState);
+        ALOGV("Camera %d: New capture state %s",
+                client->getCameraId(), kStateNames[mCaptureState]);
+        mStateChanged.signal();
+    }
+
+    if (mCaptureState == ERROR) {
+        ALOGE("Camera %d: Stopping capture sequencer due to error",
+                client->getCameraId());
         return false;
     }
 
@@ -214,6 +243,11 @@ CaptureSequencer::CaptureState CaptureSequencer::manageDone(sp<Camera2Client> &c
     {
         SharedParameters::Lock l(client->getParameters());
         switch (l.mParameters.state) {
+            case Parameters::DISCONNECTED:
+                ALOGW("%s: Camera %d: Discarding image data during shutdown ",
+                        __FUNCTION__, client->getCameraId());
+                res = INVALID_OPERATION;
+                break;
             case Parameters::STILL_CAPTURE:
                 l.mParameters.state = Parameters::STOPPED;
                 break;
diff --git a/services/camera/libcameraservice/camera2/CaptureSequencer.h b/services/camera/libcameraservice/camera2/CaptureSequencer.h
index 07e4c01..8df6d95 100644
--- a/services/camera/libcameraservice/camera2/CaptureSequencer.h
+++ b/services/camera/libcameraservice/camera2/CaptureSequencer.h
@@ -53,6 +53,11 @@ class CaptureSequencer:
     // Begin still image capture
     status_t startCapture();
 
+    // Wait until current image capture completes; returns immediately if no
+    // capture is active. Returns TIMED_OUT if capture does not complete during
+    // the specified duration.
+    status_t waitUntilIdle(nsecs_t timeout);
+
     // Notifications about AE state changes
     void notifyAutoExposure(uint8_t newState, int triggerId);
 
@@ -118,6 +123,8 @@ class CaptureSequencer:
         NUM_CAPTURE_STATES
     } mCaptureState;
     static const char* kStateNames[];
+    Mutex mStateMutex; // Guards mCaptureState
+    Condition mStateChanged;
 
     typedef CaptureState (CaptureSequencer::*StateManager)(sp<Camera2Client> &client);
     static const StateManager kStateManagers[];
diff --git a/services/camera/libcameraservice/camera2/JpegProcessor.cpp b/services/camera/libcameraservice/camera2/JpegProcessor.cpp
index a353679..ffc072b 100644
--- a/services/camera/libcameraservice/camera2/JpegProcessor.cpp
+++ b/services/camera/libcameraservice/camera2/JpegProcessor.cpp
@@ -209,24 +209,6 @@ status_t JpegProcessor::processNewCapture(sp<Camera2Client> &client) {
     ALOGV("%s: Camera %d: Still capture available", __FUNCTION__,
             client->getCameraId());
 
-    // TODO: Signal errors here upstream
-    {
-        SharedParameters::Lock l(client->getParameters());
-
-        switch (l.mParameters.state) {
-            case Parameters::STILL_CAPTURE:
-            case Parameters::VIDEO_SNAPSHOT:
-                break;
-            default:
-                ALOGE("%s: Camera %d: Still image produced unexpectedly "
-                        "in state %s!",
-                        __FUNCTION__, client->getCameraId(),
-                        Parameters::getStateName(l.mParameters.state));
-                mCaptureConsumer->unlockBuffer(imgBuffer);
-                return BAD_VALUE;
-        }
-    }
-
     if (imgBuffer.format != HAL_PIXEL_FORMAT_BLOB) {
         ALOGE("%s: Camera %d: Unexpected format for still image: "
                 "%x, expected %x", __FUNCTION__, client->getCameraId(),