commit c98bd8d9bf81663b5cd9c79e79d6e7869c1146e6
author Zhijun He <zhijunhe@google.com> Mon Jul 07 12:44:10 2014 -0700
committer Zhijun He <zhijunhe@google.com> Mon Jul 07 19:48:24 2014 +0000
tree cdb126f44d22c81c3271757b7c61f4a4d6cec52e
parent 5c68f959eaa2e02fed5643c78e281fff42bcc0a2

Camera3: only return input buffer when it is sent in request

This is to WAR the case where HAL sends non-NULL input_buffer in capture
result even capture framework doesn't send input buffer in the request.
It's very likely the input_buffer is uninitialized, and we shouldn't
use it. Log a warning for such case as well.

Bug: 16115675
Bug: 16117312
Change-Id: Ib299b45fbfe084059a9f546ded239c8094b039e2

services/camera/libcameraservice/device3/Camera3Device.cpp

diff --git a/services/camera/libcameraservice/device3/Camera3Device.cpp b/services/camera/libcameraservice/device3/Camera3Device.cpp
index 8fce191..bbb1e1c 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.cpp
+++ b/services/camera/libcameraservice/device3/Camera3Device.cpp
@@ -1533,12 +1533,12 @@
  */
 
 status_t Camera3Device::registerInFlight(uint32_t frameNumber,
-        int32_t numBuffers, CaptureResultExtras resultExtras) {
+        int32_t numBuffers, CaptureResultExtras resultExtras, bool hasInput) {
     ATRACE_CALL();
     Mutex::Autolock l(mInFlightLock);
 
     ssize_t res;
-    res = mInFlightMap.add(frameNumber, InFlightRequest(numBuffers, resultExtras));
+    res = mInFlightMap.add(frameNumber, InFlightRequest(numBuffers, resultExtras, hasInput));
     if (res < 0) return res;
 
     return OK;
@@ -1729,6 +1729,7 @@
     bool partialResultQuirk = false;
     CameraMetadata collectedQuirkResult;
     CaptureResultExtras resultExtras;
+    bool hasInputBufferInRequest = false;
 
     // Get capture timestamp and resultExtras from list of in-flight requests,
     // where it was added by the shutter notification for this frame.
@@ -1777,6 +1778,7 @@
 
         timestamp = request.captureTimestamp;
         resultExtras = request.resultExtras;
+        hasInputBufferInRequest = request.hasInputBuffer;
 
         /**
          * One of the following must happen before it's legal to call process_capture_result,
@@ -1805,8 +1807,17 @@
             request.haveResultMetadata = true;
         }
 
-        request.numBuffersLeft -= result->num_output_buffers;
-        request.numBuffersLeft -= (result->input_buffer != NULL) ? 1 : 0;
+        uint32_t numBuffersReturned = result->num_output_buffers;
+        if (result->input_buffer != NULL) {
+            if (hasInputBufferInRequest) {
+                numBuffersReturned += 1;
+            } else {
+                ALOGW("%s: Input buffer should be NULL if there is no input"
+                        " buffer sent in the request",
+                        __FUNCTION__);
+            }
+        }
+        request.numBuffersLeft -= numBuffersReturned;
         if (request.numBuffersLeft < 0) {
             SET_ERR("Too many buffers returned for frame %d",
                     frameNumber);
@@ -1908,15 +1919,21 @@
     }
 
     if (result->input_buffer != NULL) {
-        Camera3Stream *stream =
-            Camera3Stream::cast(result->input_buffer->stream);
-        res = stream->returnInputBuffer(*(result->input_buffer));
-        // Note: stream may be deallocated at this point, if this buffer was the
-        // last reference to it.
-        if (res != OK) {
-            ALOGE("%s: RequestThread: Can't return input buffer for frame %d to"
-                    "  its stream:%s (%d)",  __FUNCTION__,
-                    frameNumber, strerror(-res), res);
+        if (hasInputBufferInRequest) {
+            Camera3Stream *stream =
+                Camera3Stream::cast(result->input_buffer->stream);
+            res = stream->returnInputBuffer(*(result->input_buffer));
+            // Note: stream may be deallocated at this point, if this buffer was the
+            // last reference to it.
+            if (res != OK) {
+                ALOGE("%s: RequestThread: Can't return input buffer for frame %d to"
+                      "  its stream:%s (%d)",  __FUNCTION__,
+                      frameNumber, strerror(-res), res);
+           } else {
+               ALOGW("%s: Input buffer should be NULL if there is no input"
+                       " buffer sent in the request",
+                       __FUNCTION__);
+           }
         }
     }
 
@@ -2375,7 +2392,8 @@
     }
 
     res = parent->registerInFlight(request.frame_number,
-            totalNumBuffers, nextRequest->mResultExtras);
+            totalNumBuffers, nextRequest->mResultExtras,
+            /*hasInput*/request.input_buffer != NULL);
     ALOGVV("%s: registered in flight requestId = %" PRId32 ", frameNumber = %" PRId64
            ", burstId = %" PRId32 ".",
             __FUNCTION__,