Merge "Camera: fix per stream error code for HAL buffer manager"

3 files changed, 104 insertions, 95 deletions

diff --git a/camera/device/3.5/default/CameraDeviceSession.cpp b/camera/device/3.5/default/CameraDeviceSession.cpp
index d9c6eef5ad..873ddd0460 100644
--- a/camera/device/3.5/default/CameraDeviceSession.cpp
+++ b/camera/device/3.5/default/CameraDeviceSession.cpp
@@ -186,111 +186,118 @@ camera3_buffer_request_status_t CameraDeviceSession::requestStreamBuffers(
     }
     ATRACE_END();
 
-    if (status == BufferRequestStatus::OK || status == BufferRequestStatus::FAILED_PARTIAL) {
-        if (bufRets.size() != num_buffer_reqs) {
-            ALOGE("%s: expect %d buffer requests returned, only got %zu",
-                    __FUNCTION__, num_buffer_reqs, bufRets.size());
-            return CAMERA3_BUF_REQ_FAILED_UNKNOWN;
-        }
+    switch (status) {
+        case BufferRequestStatus::FAILED_CONFIGURING:
+            return CAMERA3_BUF_REQ_FAILED_CONFIGURING;
+        case BufferRequestStatus::FAILED_ILLEGAL_ARGUMENTS:
+            return CAMERA3_BUF_REQ_FAILED_ILLEGAL_ARGUMENTS;
+        default:
+            break; // Other status Handled by following code
+    }
 
-        for (size_t i = 0; i < num_buffer_reqs; i++) {
-            // maybe we can query all streams in one call to avoid frequent locking device here?
-            Camera3Stream* stream = getStreamPointer(bufRets[i].streamId);
-            if (stream == nullptr) {
-                ALOGE("%s: unknown streamId %d", __FUNCTION__, bufRets[i].streamId);
-                return CAMERA3_BUF_REQ_FAILED_UNKNOWN;
-            }
-            returned_buf_reqs[i].stream = stream;
+    if (status != BufferRequestStatus::OK && status != BufferRequestStatus::FAILED_PARTIAL &&
+            status != BufferRequestStatus::FAILED_UNKNOWN) {
+        ALOGE("%s: unknown buffer request error code %d", __FUNCTION__, status);
+        return CAMERA3_BUF_REQ_FAILED_UNKNOWN;
+    }
+
+    // Only OK, FAILED_PARTIAL and FAILED_UNKNOWN reaches here
+    if (bufRets.size() != num_buffer_reqs) {
+        ALOGE("%s: expect %d buffer requests returned, only got %zu",
+                __FUNCTION__, num_buffer_reqs, bufRets.size());
+        return CAMERA3_BUF_REQ_FAILED_UNKNOWN;
+    }
+
+    *num_returned_buf_reqs = num_buffer_reqs;
+    for (size_t i = 0; i < num_buffer_reqs; i++) {
+        // maybe we can query all streams in one call to avoid frequent locking device here?
+        Camera3Stream* stream = getStreamPointer(bufRets[i].streamId);
+        if (stream == nullptr) {
+            ALOGE("%s: unknown streamId %d", __FUNCTION__, bufRets[i].streamId);
+            return CAMERA3_BUF_REQ_FAILED_UNKNOWN;
         }
+        returned_buf_reqs[i].stream = stream;
+    }
 
-        std::vector<int> importedFences;
-        std::vector<std::pair<buffer_handle_t, int>> importedBuffers;
-        for (size_t i = 0; i < num_buffer_reqs; i++) {
-            int streamId = bufRets[i].streamId;
-            switch (bufRets[i].val.getDiscriminator()) {
-                case StreamBuffersVal::hidl_discriminator::error:
-                    returned_buf_reqs[i].num_output_buffers = 0;
-                    switch (bufRets[i].val.error()) {
-                        case StreamBufferRequestError::NO_BUFFER_AVAILABLE:
-                            returned_buf_reqs[i].status = CAMERA3_PS_BUF_REQ_NO_BUFFER_AVAILABLE;
-                            break;
-                        case StreamBufferRequestError::MAX_BUFFER_EXCEEDED:
-                            returned_buf_reqs[i].status = CAMERA3_PS_BUF_REQ_MAX_BUFFER_EXCEEDED;
-                            break;
-                        case StreamBufferRequestError::STREAM_DISCONNECTED:
-                            returned_buf_reqs[i].status = CAMERA3_PS_BUF_REQ_STREAM_DISCONNECTED;
-                            break;
-                        case StreamBufferRequestError::UNKNOWN_ERROR:
-                            returned_buf_reqs[i].status = CAMERA3_PS_BUF_REQ_UNKNOWN_ERROR;
-                            break;
-                        default:
-                            ALOGE("%s: Unknown StreamBufferRequestError %d",
-                                    __FUNCTION__, bufRets[i].val.error());
-                            cleanupInflightBufferFences(importedFences, importedBuffers);
-                            return CAMERA3_BUF_REQ_FAILED_UNKNOWN;
-                    }
+    // Handle failed streams
+    for (size_t i = 0; i < num_buffer_reqs; i++) {
+        if (bufRets[i].val.getDiscriminator() == StreamBuffersVal::hidl_discriminator::error) {
+            returned_buf_reqs[i].num_output_buffers = 0;
+            switch (bufRets[i].val.error()) {
+                case StreamBufferRequestError::NO_BUFFER_AVAILABLE:
+                    returned_buf_reqs[i].status = CAMERA3_PS_BUF_REQ_NO_BUFFER_AVAILABLE;
+                    break;
+                case StreamBufferRequestError::MAX_BUFFER_EXCEEDED:
+                    returned_buf_reqs[i].status = CAMERA3_PS_BUF_REQ_MAX_BUFFER_EXCEEDED;
+                    break;
+                case StreamBufferRequestError::STREAM_DISCONNECTED:
+                    returned_buf_reqs[i].status = CAMERA3_PS_BUF_REQ_STREAM_DISCONNECTED;
+                    break;
+                case StreamBufferRequestError::UNKNOWN_ERROR:
+                    returned_buf_reqs[i].status = CAMERA3_PS_BUF_REQ_UNKNOWN_ERROR;
                     break;
-                case StreamBuffersVal::hidl_discriminator::buffers: {
-                    const hidl_vec<StreamBuffer>& hBufs = bufRets[i].val.buffers();
-                    camera3_stream_buffer_t* outBufs = returned_buf_reqs[i].output_buffers;
-                    for (size_t b = 0; b < hBufs.size(); b++) {
-                        const StreamBuffer& hBuf = hBufs[b];
-                        camera3_stream_buffer_t& outBuf = outBufs[b];
-                        // maybe add importBuffers API to avoid frequent locking device?
-                        Status s = importBuffer(streamId,
-                                hBuf.bufferId, hBuf.buffer.getNativeHandle(),
-                                /*out*/&(outBuf.buffer),
-                                /*allowEmptyBuf*/false);
-                        if (s != Status::OK) {
-                            ALOGE("%s: import stream %d bufferId %" PRIu64 " failed!",
-                                    __FUNCTION__, streamId, hBuf.bufferId);
-                            cleanupInflightBufferFences(importedFences, importedBuffers);
-                            // Buffer import should never fail - restart HAL since something is very
-                            // wrong.
-                            assert(false);
-                            return CAMERA3_BUF_REQ_FAILED_UNKNOWN;
-                        }
-
-                        pushBufferId(*(outBuf.buffer), hBuf.bufferId, streamId);
-                        importedBuffers.push_back(std::make_pair(*(outBuf.buffer), streamId));
-
-                        if (!sHandleImporter.importFence(
-                                hBuf.acquireFence,
-                                outBuf.acquire_fence)) {
-                            ALOGE("%s: stream %d bufferId %" PRIu64 "acquire fence is invalid",
-                                    __FUNCTION__, streamId, hBuf.bufferId);
-                            cleanupInflightBufferFences(importedFences, importedBuffers);
-                            return CAMERA3_BUF_REQ_FAILED_UNKNOWN;
-                        }
-                        importedFences.push_back(outBuf.acquire_fence);
-                        outBuf.stream = returned_buf_reqs[i].stream;
-                        outBuf.status = CAMERA3_BUFFER_STATUS_OK;
-                        outBuf.release_fence = -1;
-                    }
-                    returned_buf_reqs[i].status = CAMERA3_PS_BUF_REQ_OK;
-                } break;
                 default:
-                    ALOGE("%s: unknown StreamBuffersVal discrimator!", __FUNCTION__);
-                    cleanupInflightBufferFences(importedFences, importedBuffers);
+                    ALOGE("%s: Unknown StreamBufferRequestError %d",
+                            __FUNCTION__, bufRets[i].val.error());
                     return CAMERA3_BUF_REQ_FAILED_UNKNOWN;
             }
         }
+    }
 
-        *num_returned_buf_reqs = num_buffer_reqs;
-
-        return (status == BufferRequestStatus::OK) ?
-                CAMERA3_BUF_REQ_OK : CAMERA3_BUF_REQ_FAILED_PARTIAL;
+    if (status == BufferRequestStatus::FAILED_UNKNOWN) {
+        return CAMERA3_BUF_REQ_FAILED_UNKNOWN;
     }
 
-    switch (status) {
-        case BufferRequestStatus::FAILED_CONFIGURING:
-            return CAMERA3_BUF_REQ_FAILED_CONFIGURING;
-        case BufferRequestStatus::FAILED_ILLEGAL_ARGUMENTS:
-            return CAMERA3_BUF_REQ_FAILED_ILLEGAL_ARGUMENTS;
-        case BufferRequestStatus::FAILED_UNKNOWN:
-        default:
-            return CAMERA3_BUF_REQ_FAILED_UNKNOWN;
+    // Only BufferRequestStatus::OK and BufferRequestStatus::FAILED_PARTIAL reaches here
+    std::vector<int> importedFences;
+    std::vector<std::pair<buffer_handle_t, int>> importedBuffers;
+    for (size_t i = 0; i < num_buffer_reqs; i++) {
+        if (bufRets[i].val.getDiscriminator() !=
+                StreamBuffersVal::hidl_discriminator::buffers) {
+            continue;
+        }
+        int streamId = bufRets[i].streamId;
+        const hidl_vec<StreamBuffer>& hBufs = bufRets[i].val.buffers();
+        camera3_stream_buffer_t* outBufs = returned_buf_reqs[i].output_buffers;
+        for (size_t b = 0; b < hBufs.size(); b++) {
+            const StreamBuffer& hBuf = hBufs[b];
+            camera3_stream_buffer_t& outBuf = outBufs[b];
+            // maybe add importBuffers API to avoid frequent locking device?
+            Status s = importBuffer(streamId,
+                    hBuf.bufferId, hBuf.buffer.getNativeHandle(),
+                    /*out*/&(outBuf.buffer),
+                    /*allowEmptyBuf*/false);
+            if (s != Status::OK) {
+                ALOGE("%s: import stream %d bufferId %" PRIu64 " failed!",
+                        __FUNCTION__, streamId, hBuf.bufferId);
+                cleanupInflightBufferFences(importedFences, importedBuffers);
+                // Buffer import should never fail - restart HAL since something is very
+                // wrong.
+                assert(false);
+                return CAMERA3_BUF_REQ_FAILED_UNKNOWN;
+            }
+
+            pushBufferId(*(outBuf.buffer), hBuf.bufferId, streamId);
+            importedBuffers.push_back(std::make_pair(*(outBuf.buffer), streamId));
+
+            if (!sHandleImporter.importFence(
+                    hBuf.acquireFence,
+                    outBuf.acquire_fence)) {
+                ALOGE("%s: stream %d bufferId %" PRIu64 "acquire fence is invalid",
+                        __FUNCTION__, streamId, hBuf.bufferId);
+                cleanupInflightBufferFences(importedFences, importedBuffers);
+                return CAMERA3_BUF_REQ_FAILED_UNKNOWN;
+            }
+            importedFences.push_back(outBuf.acquire_fence);
+            outBuf.stream = returned_buf_reqs[i].stream;
+            outBuf.status = CAMERA3_BUFFER_STATUS_OK;
+            outBuf.release_fence = -1;
+        }
+        returned_buf_reqs[i].status = CAMERA3_PS_BUF_REQ_OK;
     }
+
+    return (status == BufferRequestStatus::OK) ?
+            CAMERA3_BUF_REQ_OK : CAMERA3_BUF_REQ_FAILED_PARTIAL;
 }
 
 void CameraDeviceSession::returnStreamBuffers(
diff --git a/camera/device/3.5/types.hal b/camera/device/3.5/types.hal
index 7cb97277db..e3c235018f 100644
--- a/camera/device/3.5/types.hal
+++ b/camera/device/3.5/types.hal
@@ -120,7 +120,9 @@ enum BufferRequestStatus : uint32_t {
 
     /**
      * Method call failed for all streams and no buffers are returned at all.
-     * Failure due to unknown reason.
+     * Failure due to unknown reason, or all streams has individual failing
+     * reason. For the latter case, check per stream status for each returned
+     * StreamBufferRet.
      */
     FAILED_UNKNOWN = 4,
 };
diff --git a/current.txt b/current.txt
index fd55d7cb1f..f37a8e25c1 100644
--- a/current.txt
+++ b/current.txt
@@ -444,7 +444,7 @@ f7431f3e3e4e3387fc6f27a6cf423eddcd824a395dc4349d302c995ab44a9895 android.hardwar
 09ab9b24994429d9bb32a3fb420b6f6be3e47eb655139a2c08c4e80d3f33ff95 android.hardware.camera.device@3.5::ICameraDevice
 06237de53c42890029e3f8fe7d1480d078469c0d07608e51c37b4d485d342992 android.hardware.camera.device@3.5::ICameraDeviceCallback
 08c68b196e2fc4e5ba67ba0d0917bde828a87cbe2cffec19d04733972da9eb49 android.hardware.camera.device@3.5::ICameraDeviceSession
-d487ab209944df8987eeca70cf09307fc1429cedf64b0ea9e77c61d8caeb8c15 android.hardware.camera.device@3.5::types
+acaba39216973e58949f50978762bcda1c29f5f7e0bca3e08db21f0767356130 android.hardware.camera.device@3.5::types
 74ec7732fdacb22292c907b49f8f933510851ea1b3ed195c4dcdff35a20387f5 android.hardware.camera.metadata@3.4::types
 0fb39a7809ad1c52b3efbbed5ef4749b06c2a4f1f19cdc3efa2e3d9b28f1205c android.hardware.camera.provider@2.5::ICameraProvider
 f5777403d65135a5407723671bc7a864cdca83aea13ee3ce2894b95e6588ca3a android.hardware.camera.provider@2.5::types