Camera: API1 shim: notify shutter correctly

Previous implementation only notifies the callback when we receive
full capture result. This implementation notifies the callback
once HAL sends capture start callback.

Bug: 12530628
Change-Id: Ibf71d532b5cf649514b316e35683c217021698b4

4 files changed, 64 insertions, 11 deletions

diff --git a/services/camera/libcameraservice/api1/Camera2Client.cpp b/services/camera/libcameraservice/api1/Camera2Client.cpp
index 05ede92..f2d6ab2 100644
--- a/services/camera/libcameraservice/api1/Camera2Client.cpp
+++ b/services/camera/libcameraservice/api1/Camera2Client.cpp
@@ -1881,6 +1881,16 @@ void Camera2Client::notifyAutoExposure(uint8_t newState, int triggerId) {
     mCaptureSequencer->notifyAutoExposure(newState, triggerId);
 }
 
+void Camera2Client::notifyShutter(const CaptureResultExtras& resultExtras,
+                                  nsecs_t timestamp) {
+    (void)resultExtras;
+    (void)timestamp;
+
+    ALOGV("%s: Shutter notification for request id %" PRId32 " at time %" PRId64,
+            __FUNCTION__, resultExtras.requestId, timestamp);
+    mCaptureSequencer->notifyShutter(resultExtras, timestamp);
+}
+
 camera2::SharedParameters& Camera2Client::getParameters() {
     return mParameters;
 }
diff --git a/services/camera/libcameraservice/api1/Camera2Client.h b/services/camera/libcameraservice/api1/Camera2Client.h
index a988037..3784aab 100644
--- a/services/camera/libcameraservice/api1/Camera2Client.h
+++ b/services/camera/libcameraservice/api1/Camera2Client.h
@@ -106,6 +106,8 @@ public:
 
     virtual void notifyAutoFocus(uint8_t newState, int triggerId);
     virtual void notifyAutoExposure(uint8_t newState, int triggerId);
+    virtual void notifyShutter(const CaptureResultExtras& resultExtras,
+                               nsecs_t timestamp);
 
     /**
      * Interface used by independent components of Camera2Client.
diff --git a/services/camera/libcameraservice/api1/client2/CaptureSequencer.cpp b/services/camera/libcameraservice/api1/client2/CaptureSequencer.cpp
index 9849f4d..d847e0f 100644
--- a/services/camera/libcameraservice/api1/client2/CaptureSequencer.cpp
+++ b/services/camera/libcameraservice/api1/client2/CaptureSequencer.cpp
@@ -43,6 +43,8 @@ CaptureSequencer::CaptureSequencer(wp<Camera2Client> client):
         mNewFrameReceived(false),
         mNewCaptureReceived(false),
         mShutterNotified(false),
+        mHalNotifiedShutter(false),
+        mShutterCaptureId(-1),
         mClient(client),
         mCaptureState(IDLE),
         mStateTransitionCount(0),
@@ -106,6 +108,16 @@ void CaptureSequencer::notifyAutoExposure(uint8_t newState, int triggerId) {
     }
 }
 
+void CaptureSequencer::notifyShutter(const CaptureResultExtras& resultExtras,
+                                     nsecs_t timestamp) {
+    ATRACE_CALL();
+    Mutex::Autolock l(mInputMutex);
+    if (!mHalNotifiedShutter && resultExtras.requestId == mShutterCaptureId) {
+        mHalNotifiedShutter = true;
+        mShutterNotifySignal.signal();
+    }
+}
+
 void CaptureSequencer::onResultAvailable(const CaptureResult &result) {
     ATRACE_CALL();
     ALOGV("%s: New result available.", __FUNCTION__);
@@ -335,6 +347,11 @@ CaptureSequencer::CaptureState CaptureSequencer::manageStart(
     } else {
         nextState = STANDARD_START;
     }
+    {
+        Mutex::Autolock l(mInputMutex);
+        mShutterCaptureId = mCaptureId;
+        mHalNotifiedShutter = false;
+    }
     mShutterNotified = false;
 
     return nextState;
@@ -541,6 +558,7 @@ CaptureSequencer::CaptureState CaptureSequencer::manageStandardCapture(
             return DONE;
         }
     }
+
     // TODO: Capture should be atomic with setStreamingRequest here
     res = client->getCameraDevice()->capture(captureCopy);
     if (res != OK) {
@@ -560,6 +578,31 @@ CaptureSequencer::CaptureState CaptureSequencer::manageStandardCaptureWait(
     ATRACE_CALL();
     Mutex::Autolock l(mInputMutex);
 
+
+    // Wait for shutter callback
+    while (!mHalNotifiedShutter) {
+        if (mTimeoutCount <= 0) {
+            break;
+        }
+        res = mShutterNotifySignal.waitRelative(mInputMutex, kWaitDuration);
+        if (res == TIMED_OUT) {
+            mTimeoutCount--;
+            return STANDARD_CAPTURE_WAIT;
+        }
+    }
+
+    if (mHalNotifiedShutter) {
+        if (!mShutterNotified) {
+            SharedParameters::Lock l(client->getParameters());
+            /* warning: this also locks a SharedCameraCallbacks */
+            shutterNotifyLocked(l.mParameters, client, mMsgType);
+            mShutterNotified = true;
+        }
+    } else if (mTimeoutCount <= 0) {
+        ALOGW("Timed out waiting for shutter notification");
+        return DONE;
+    }
+
     // Wait for new metadata result (mNewFrame)
     while (!mNewFrameReceived) {
         res = mNewFrameSignal.waitRelative(mInputMutex, kWaitDuration);
@@ -569,15 +612,6 @@ CaptureSequencer::CaptureState CaptureSequencer::manageStandardCaptureWait(
         }
     }
 
-    // Approximation of the shutter being closed
-    // - TODO: use the hal3 exposure callback in Camera3Device instead
-    if (mNewFrameReceived && !mShutterNotified) {
-        SharedParameters::Lock l(client->getParameters());
-        /* warning: this also locks a SharedCameraCallbacks */
-        shutterNotifyLocked(l.mParameters, client, mMsgType);
-        mShutterNotified = true;
-    }
-
     // Wait until jpeg was captured by JpegProcessor
     while (mNewFrameReceived && !mNewCaptureReceived) {
         res = mNewCaptureSignal.waitRelative(mInputMutex, kWaitDuration);
@@ -591,6 +625,7 @@ CaptureSequencer::CaptureState CaptureSequencer::manageStandardCaptureWait(
         return DONE;
     }
     if (mNewFrameReceived && mNewCaptureReceived) {
+
         if (mNewFrameId != mCaptureId) {
             ALOGW("Mismatched capture frame IDs: Expected %d, got %d",
                     mCaptureId, mNewFrameId);
@@ -667,7 +702,6 @@ CaptureSequencer::CaptureState CaptureSequencer::manageBurstCaptureWait(
         sp<Camera2Client> &/*client*/) {
     status_t res;
     ATRACE_CALL();
-
     while (!mNewCaptureReceived) {
         res = mNewCaptureSignal.waitRelative(mInputMutex, kWaitDuration);
         if (res == TIMED_OUT) {
diff --git a/services/camera/libcameraservice/api1/client2/CaptureSequencer.h b/services/camera/libcameraservice/api1/client2/CaptureSequencer.h
index d42ab13..10252fb 100644
--- a/services/camera/libcameraservice/api1/client2/CaptureSequencer.h
+++ b/services/camera/libcameraservice/api1/client2/CaptureSequencer.h
@@ -62,6 +62,10 @@ class CaptureSequencer:
     // Notifications about AE state changes
     void notifyAutoExposure(uint8_t newState, int triggerId);
 
+    // Notifications about shutter (capture start)
+    void notifyShutter(const CaptureResultExtras& resultExtras,
+                       nsecs_t timestamp);
+
     // Notification from the frame processor
     virtual void onResultAvailable(const CaptureResult &result);
 
@@ -95,7 +99,10 @@ class CaptureSequencer:
     sp<MemoryBase> mCaptureBuffer;
     Condition mNewCaptureSignal;
 
-    bool mShutterNotified;
+    bool mShutterNotified; // Has CaptureSequencer sent shutter to Client
+    bool mHalNotifiedShutter; // Has HAL sent shutter to CaptureSequencer
+    int32_t mShutterCaptureId; // The captureId which is waiting for shutter notification
+    Condition mShutterNotifySignal;
 
     /**
      * Internal to CaptureSequencer