commit 0b2113c54f716e710aff60989cba5d5f0b080f0f
author Ari Hausman-Cohen <arihc@google.com> Mon Oct 03 13:43:13 2016 -0700
committer Ari Hausman-Cohen <arihc@google.com> Fri Nov 11 15:04:57 2016 -0800
tree dba06a63f6df9e344f9f4cef21914a9e0e9ee704
parent fb161115c80816a90f252313d53bbb599b90dd6c

Track in-flight requests

This allows for verification of callbacks, and will also be helpful
when implementing flush.

BUG: 31653306
TEST: unit tests pass, test program runs
Change-Id: Id43d6cb3c2b9ca4adc96fc08282f88e0b7b904e1

modules/camera/3_4/camera.cpp

diff --git a/modules/camera/3_4/camera.cpp b/modules/camera/3_4/camera.cpp
index 04d4c89..d4be03d 100644
--- a/modules/camera/3_4/camera.cpp
+++ b/modules/camera/3_4/camera.cpp
@@ -58,7 +58,8 @@
     mBusy(false),
     mCallbackOps(NULL),
     mStreams(NULL),
-    mNumStreams(0)
+    mNumStreams(0),
+    mInFlightTracker(new RequestTracker)
 {
     memset(&mTemplates, 0, sizeof(mTemplates));
     memset(&mDevice, 0, sizeof(mDevice));
@@ -249,6 +250,9 @@
     mStreams = newStreams;
     mNumStreams = stream_config->num_streams;
 
+    // Update the request tracker.
+    mInFlightTracker->SetStreamConfiguration(*stream_config);
+
     return 0;
 
 err_out:
@@ -256,7 +260,10 @@
     destroyStreams(newStreams, stream_config->num_streams);
     // Set error if it wasn't specified.
     if (!res) {
-      res = -EINVAL;
+        res = -EINVAL;
+    } else if (res != -EINVAL) {
+        // Fatal error, clear stream configuration.
+        mInFlightTracker->ClearStreamConfiguration();
     }
     return res;
 }
@@ -437,27 +444,40 @@
             return -ENODEV;
     }
 
+    // Add the request to tracking.
+    if (!mInFlightTracker->Add(request)) {
+        ALOGE("%s:%d: Failed to track request for frame %d.",
+              __func__, mId, request->frame_number);
+        return -ENODEV;
+    }
+
     // Send the request off to the device for completion.
     enqueueRequest(request);
 
     // Request is now in flight. The device will call completeRequest
     // asynchronously when it is done filling buffers and metadata.
-    // TODO(b/31653306): Track requests in flight to ensure not too many are
-    // sent at a time, and so they can be dumped even if the device loses them.
     return 0;
 }
 
 void Camera::completeRequest(std::shared_ptr<CaptureRequest> request, int err)
 {
-    // TODO(b/31653306): make sure this is actually a request in flight,
-    // and not a random new one or a cancelled one. If so, stop tracking.
-    if (err) {
-        ALOGE("%s:%d: Error completing request for frame %d.",
-              __func__, mId, request->frame_number);
-        completeRequestWithError(request);
+    if (!mInFlightTracker->Remove(request)) {
+        ALOGE("%s:%d: Completed request %p is not being tracked.",
+              __func__, mId, request.get());
         return;
     }
 
+    // Since |request| has been removed from the tracking, this method
+    // MUST call sendResult (can still return a result in an error state, e.g.
+    // through completeRequestWithError) so the frame doesn't get lost.
+
+    if (err) {
+      ALOGE("%s:%d: Error completing request for frame %d.",
+            __func__, mId, request->frame_number);
+      completeRequestWithError(request);
+      return;
+    }
+
     // Notify the framework with the shutter time (extracted from the result).
     int64_t timestamp = 0;
     // TODO(b/31360070): The general metadata methods should be part of the