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

2 files changed, 51 insertions, 19 deletions

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 @@ void Camera3Device::setErrorStateLockedV(const char *fmt, va_list args) {
  */
 
 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 @@ void Camera3Device::processCaptureResult(const camera3_capture_result *result) {
     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 @@ void Camera3Device::processCaptureResult(const camera3_capture_result *result) {
 
         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 @@ void Camera3Device::processCaptureResult(const camera3_capture_result *result) {
             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 @@ void Camera3Device::processCaptureResult(const camera3_capture_result *result) {
     }
 
     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 @@ bool Camera3Device::RequestThread::threadLoop() {
     }
 
     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__,
diff --git a/services/camera/libcameraservice/device3/Camera3Device.h b/services/camera/libcameraservice/device3/Camera3Device.h
index d7545d0..ea958b7 100644
--- a/services/camera/libcameraservice/device3/Camera3Device.h
+++ b/services/camera/libcameraservice/device3/Camera3Device.h
@@ -504,6 +504,8 @@ class Camera3Device :
         // and input buffers
         int     numBuffersLeft;
         CaptureResultExtras resultExtras;
+        // If this request has any input buffer
+        bool hasInputBuffer;
 
         // Fields used by the partial result quirk only
         struct PartialResultQuirkInFlight {
@@ -522,14 +524,16 @@ class Camera3Device :
                 captureTimestamp(0),
                 requestStatus(OK),
                 haveResultMetadata(false),
-                numBuffersLeft(0) {
+                numBuffersLeft(0),
+                hasInputBuffer(false){
         }
 
         InFlightRequest(int numBuffers) :
                 captureTimestamp(0),
                 requestStatus(OK),
                 haveResultMetadata(false),
-                numBuffersLeft(numBuffers) {
+                numBuffersLeft(numBuffers),
+                hasInputBuffer(false){
         }
 
         InFlightRequest(int numBuffers, CaptureResultExtras extras) :
@@ -537,9 +541,19 @@ class Camera3Device :
                 requestStatus(OK),
                 haveResultMetadata(false),
                 numBuffersLeft(numBuffers),
-                resultExtras(extras) {
+                resultExtras(extras),
+                hasInputBuffer(false){
         }
-    };
+
+        InFlightRequest(int numBuffers, CaptureResultExtras extras, bool hasInput) :
+                captureTimestamp(0),
+                requestStatus(OK),
+                haveResultMetadata(false),
+                numBuffersLeft(numBuffers),
+                resultExtras(extras),
+                hasInputBuffer(hasInput){
+        }
+};
     // Map from frame number to the in-flight request state
     typedef KeyedVector<uint32_t, InFlightRequest> InFlightMap;
 
@@ -547,7 +561,7 @@ class Camera3Device :
     InFlightMap            mInFlightMap;
 
     status_t registerInFlight(uint32_t frameNumber,
-            int32_t numBuffers, CaptureResultExtras resultExtras);
+            int32_t numBuffers, CaptureResultExtras resultExtras, bool hasInput);
 
     /**
      * For the partial result quirk, check if all 3A state fields are available