Camera: VTS: Wait for release fence before consuming buffers

The camera HAL may signal release fence after processCaptureResult.
If the VTS test waits for the release fence in the context of the
capture result, there is possibility of deadlock.

Rather, we should wait for the releaseFence in a different thread
context to really emulate the real application behavior.

Test: atest VtsAidlHalCameraProvider_TargetTest
Bug: 241281568
Change-Id: Id1d92e901aae1cab084846d252ef090fcda182d7

5 files changed, 35 insertions, 88 deletions

diff --git a/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp b/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp
index b0ae20eae2..2a5f90f20b 100644
--- a/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp
+++ b/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp
@@ -929,7 +929,7 @@ public:
 
     static Status getMandatoryConcurrentStreams(const camera_metadata_t* staticMeta,
                                                 std::vector<AvailableStream>* outputStreams);
-    static bool supportsPreviewStabilization(const std::string& name, sp<ICameraProvider> provider);
+
     static Status getJpegBufferSize(camera_metadata_t *staticMeta,
             uint32_t* outBufSize);
     static Status isConstrainedModeAvailable(camera_metadata_t *staticMeta);
@@ -976,9 +976,6 @@ public:
 
     void processCaptureRequestInternal(uint64_t bufferusage, RequestTemplate reqTemplate,
                                        bool useSecureOnlyCameras);
-    void processPreviewStabilizationCaptureRequestInternal(
-            bool previewStabilizationOn,
-            /*inout*/ std::unordered_map<std::string, nsecs_t>& cameraDeviceToTimeLag);
 
     // Used by switchToOffline where a new result queue is created for offline reqs
     void updateInflightResultQueue(std::shared_ptr<ResultMetadataQueue> resultQueue);
@@ -1032,11 +1029,7 @@ protected:
 
         // Buffers are added by process_capture_result when output buffers
         // return from HAL but framework.
-        struct StreamBufferAndTimestamp {
-            StreamBuffer buffer;
-            nsecs_t timeStamp;
-        };
-        ::android::Vector<StreamBufferAndTimestamp> resultOutputBuffers;
+        ::android::Vector<StreamBuffer> resultOutputBuffers;
 
         std::unordered_set<std::string> expectedPhysicalResults;
 
@@ -1453,25 +1446,8 @@ bool CameraHidlTest::DeviceCb::processCaptureResultLocked(const CaptureResult& r
         return notify;
     }
 
-    for (const auto& buffer : results.outputBuffers) {
-        // wait for the fence timestamp and store it along with the buffer
-        // TODO: Check if we really need the dup here
-        sp<android::Fence> releaseFence = nullptr;
-        if (buffer.releaseFence && (buffer.releaseFence->numFds == 1) &&
-            buffer.releaseFence->data[0] >= 0) {
-            releaseFence = new android::Fence(dup(buffer.releaseFence->data[0]));
-        }
-        InFlightRequest::StreamBufferAndTimestamp streamBufferAndTimestamp;
-        streamBufferAndTimestamp.buffer = buffer;
-        streamBufferAndTimestamp.timeStamp = systemTime();
-        if (releaseFence && releaseFence->isValid()) {
-            releaseFence->wait(/*ms*/ 300);
-            nsecs_t releaseTime = releaseFence->getSignalTime();
-            if (streamBufferAndTimestamp.timeStamp < releaseTime)
-                streamBufferAndTimestamp.timeStamp = releaseTime;
-        }
-        request->resultOutputBuffers.push_back(streamBufferAndTimestamp);
-    }
+    request->resultOutputBuffers.appendArray(results.outputBuffers.data(),
+                                             results.outputBuffers.size());
     // If shutter event is received notify the pending threads.
     if (request->shutterTimestamp != 0) {
         notify = true;
@@ -4841,7 +4817,7 @@ void CameraHidlTest::processCaptureRequestInternal(uint64_t bufferUsage,
 
             ASSERT_FALSE(inflightReq.errorCodeValid);
             ASSERT_NE(inflightReq.resultOutputBuffers.size(), 0u);
-            ASSERT_EQ(testStream.id, inflightReq.resultOutputBuffers[0].buffer.streamId);
+            ASSERT_EQ(testStream.id, inflightReq.resultOutputBuffers[0].streamId);
 
             request.frameNumber++;
             // Empty settings should be supported after the first call
@@ -4879,7 +4855,7 @@ void CameraHidlTest::processCaptureRequestInternal(uint64_t bufferUsage,
 
             ASSERT_FALSE(inflightReq.errorCodeValid);
             ASSERT_NE(inflightReq.resultOutputBuffers.size(), 0u);
-            ASSERT_EQ(testStream.id, inflightReq.resultOutputBuffers[0].buffer.streamId);
+            ASSERT_EQ(testStream.id, inflightReq.resultOutputBuffers[0].streamId);
         }
 
         if (useHalBufManager) {
@@ -5460,7 +5436,7 @@ TEST_P(CameraHidlTest, processCaptureRequestBurstISO) {
 
             ASSERT_FALSE(inflightReqs[i].errorCodeValid);
             ASSERT_NE(inflightReqs[i].resultOutputBuffers.size(), 0u);
-            ASSERT_EQ(previewStream.id, inflightReqs[i].resultOutputBuffers[0].buffer.streamId);
+            ASSERT_EQ(previewStream.id, inflightReqs[i].resultOutputBuffers[0].streamId);
             ASSERT_FALSE(inflightReqs[i].collectedResult.isEmpty());
             ASSERT_TRUE(inflightReqs[i].collectedResult.exists(ANDROID_SENSOR_SENSITIVITY));
             camera_metadata_entry_t isoResult = inflightReqs[i].collectedResult.find(
@@ -5744,7 +5720,7 @@ TEST_P(CameraHidlTest, switchToOffline) {
 
             ASSERT_FALSE(inflightReqs[i].errorCodeValid);
             ASSERT_NE(inflightReqs[i].resultOutputBuffers.size(), 0u);
-            ASSERT_EQ(stream.id, inflightReqs[i].resultOutputBuffers[0].buffer.streamId);
+            ASSERT_EQ(stream.id, inflightReqs[i].resultOutputBuffers[0].streamId);
             ASSERT_FALSE(inflightReqs[i].collectedResult.isEmpty());
         }
 
@@ -5940,7 +5916,7 @@ TEST_P(CameraHidlTest, flushPreviewRequest) {
 
             if (!inflightReq.errorCodeValid) {
                 ASSERT_NE(inflightReq.resultOutputBuffers.size(), 0u);
-                ASSERT_EQ(previewStream.id, inflightReq.resultOutputBuffers[0].buffer.streamId);
+                ASSERT_EQ(previewStream.id, inflightReq.resultOutputBuffers[0].streamId);
             } else {
                 switch (inflightReq.errorCode) {
                     case ErrorCode::ERROR_REQUEST:
@@ -7425,47 +7401,6 @@ void CameraHidlTest::configurePreviewStream(const std::string &name, int32_t dev
                           previewStream, halStreamConfig, supportsPartialResults,
                           partialResultCount, useHalBufManager, outCb, streamConfigCounter);
 }
-
-bool CameraHidlTest::supportsPreviewStabilization(const std::string& name,
-                                                  sp<ICameraProvider> provider) {
-    Return<void> ret;
-    sp<ICameraDevice> device3_x = nullptr;
-    ret = provider->getCameraDeviceInterface_V3_x(name, [&](auto status, const auto& device) {
-        ALOGI("getCameraDeviceInterface_V3_x returns status:%d", (int)status);
-        ASSERT_EQ(Status::OK, status);
-        ASSERT_NE(device, nullptr);
-        device3_x = device;
-    });
-    if (!(ret.isOk())) {
-        ADD_FAILURE() << "Failed to get camera device interface for " << name;
-    }
-
-    camera_metadata_t* staticMeta = nullptr;
-    ret = device3_x->getCameraCharacteristics([&](Status s, CameraMetadata metadata) {
-        ASSERT_EQ(Status::OK, s);
-        staticMeta =
-                clone_camera_metadata(reinterpret_cast<const camera_metadata_t*>(metadata.data()));
-    });
-    if (!(ret.isOk())) {
-        ADD_FAILURE() << "Failed to get camera characteristics for " << name;
-    }
-    // Go through the characteristics and see if video stabilization modes have
-    // preview stabilization
-    camera_metadata_ro_entry entry;
-
-    int retcode = find_camera_metadata_ro_entry(
-            staticMeta, ANDROID_CONTROL_AVAILABLE_VIDEO_STABILIZATION_MODES, &entry);
-    if ((0 == retcode) && (entry.count > 0)) {
-        for (auto i = 0; i < entry.count; i++) {
-            if (entry.data.u8[i] ==
-                ANDROID_CONTROL_VIDEO_STABILIZATION_MODE_PREVIEW_STABILIZATION) {
-                return true;
-            }
-        }
-    }
-    return false;
-}
-
 // Open a device session and configure a preview stream.
 void CameraHidlTest::configureSingleStream(
         const std::string& name, int32_t deviceVersion, sp<ICameraProvider> provider,
diff --git a/camera/provider/aidl/vts/VtsAidlHalCameraProvider_TargetTest.cpp b/camera/provider/aidl/vts/VtsAidlHalCameraProvider_TargetTest.cpp
index 70ab7a02b3..be5bcb5966 100644
--- a/camera/provider/aidl/vts/VtsAidlHalCameraProvider_TargetTest.cpp
+++ b/camera/provider/aidl/vts/VtsAidlHalCameraProvider_TargetTest.cpp
@@ -2000,6 +2000,8 @@ TEST_P(CameraAidlTest, process10BitDynamicRangeRequest) {
                     ASSERT_NE(std::cv_status::timeout, mResultCondition.wait_until(l, timeout));
                 }
 
+                waitForReleaseFence(inflightReq->resultOutputBuffers);
+
                 ASSERT_FALSE(inflightReq->errorCodeValid);
                 ASSERT_NE(inflightReq->resultOutputBuffers.size(), 0u);
                 verify10BitMetadata(mHandleImporter, *inflightReq, profile);
diff --git a/camera/provider/aidl/vts/camera_aidl_test.cpp b/camera/provider/aidl/vts/camera_aidl_test.cpp
index 2c2f1b2c66..c352d7d38a 100644
--- a/camera/provider/aidl/vts/camera_aidl_test.cpp
+++ b/camera/provider/aidl/vts/camera_aidl_test.cpp
@@ -34,6 +34,7 @@
 #include <grallocusage/GrallocUsageConversion.h>
 #include <hardware/gralloc1.h>
 #include <simple_device_cb.h>
+#include <ui/Fence.h>
 #include <ui/GraphicBufferAllocator.h>
 #include <regex>
 #include <typeinfo>
@@ -139,6 +140,25 @@ void CameraAidlTest::TearDown() {
     }
 }
 
+void CameraAidlTest::waitForReleaseFence(
+        std::vector<InFlightRequest::StreamBufferAndTimestamp>& resultOutputBuffers) {
+    for (auto& bufferAndTimestamp : resultOutputBuffers) {
+        // wait for the fence timestamp and store it along with the buffer
+        android::sp<android::Fence> releaseFence = nullptr;
+        const native_handle_t* releaseFenceHandle = bufferAndTimestamp.buffer.releaseFence;
+        if (releaseFenceHandle != nullptr && releaseFenceHandle->numFds == 1 &&
+            releaseFenceHandle->data[0] >= 0) {
+            releaseFence = new android::Fence(releaseFenceHandle->data[0]);
+        }
+        if (releaseFence && releaseFence->isValid()) {
+            releaseFence->wait(/*ms*/ 300);
+            nsecs_t releaseTime = releaseFence->getSignalTime();
+            if (bufferAndTimestamp.timeStamp < releaseTime)
+                bufferAndTimestamp.timeStamp = releaseTime;
+        }
+    }
+}
+
 std::vector<std::string> CameraAidlTest::getCameraDeviceNames(
         std::shared_ptr<ICameraProvider>& provider, bool addSecureOnly) {
     std::vector<std::string> cameraDeviceNames;
@@ -2373,6 +2393,7 @@ void CameraAidlTest::processPreviewStabilizationCaptureRequestInternal(
                                std::chrono::seconds(kStreamBufferTimeoutSec);
                 ASSERT_NE(std::cv_status::timeout, mResultCondition.wait_until(l, timeout));
             }
+            waitForReleaseFence(inflightReq->resultOutputBuffers);
 
             ASSERT_FALSE(inflightReq->errorCodeValid);
             ASSERT_NE(inflightReq->resultOutputBuffers.size(), 0u);
diff --git a/camera/provider/aidl/vts/camera_aidl_test.h b/camera/provider/aidl/vts/camera_aidl_test.h
index d828cee4a8..9fa84d36d1 100644
--- a/camera/provider/aidl/vts/camera_aidl_test.h
+++ b/camera/provider/aidl/vts/camera_aidl_test.h
@@ -488,6 +488,9 @@ class CameraAidlTest : public ::testing::TestWithParam<std::string> {
             aidl::android::hardware::camera::metadata::RequestAvailableDynamicRangeProfilesMap
                     profile);
 
+    static void waitForReleaseFence(
+            std::vector<InFlightRequest::StreamBufferAndTimestamp>& resultOutputBuffers);
+
     // Map from frame number to the in-flight request state
     typedef std::unordered_map<uint32_t, std::shared_ptr<InFlightRequest>> InFlightMap;
 
diff --git a/camera/provider/aidl/vts/device_cb.cpp b/camera/provider/aidl/vts/device_cb.cpp
index 4698b4ae89..b23c691d34 100644
--- a/camera/provider/aidl/vts/device_cb.cpp
+++ b/camera/provider/aidl/vts/device_cb.cpp
@@ -19,7 +19,6 @@
 #include <aidl/android/hardware/graphics/common/PixelFormat.h>
 #include <aidlcommonsupport/NativeHandle.h>
 #include <grallocusage/GrallocUsageConversion.h>
-#include <ui/Fence.h>
 #include <cinttypes>
 
 using ::aidl::android::hardware::camera::device::BufferStatus;
@@ -419,13 +418,6 @@ bool DeviceCb::processCaptureResultLocked(
     }
 
     for (const auto& buffer : results.outputBuffers) {
-        // wait for the fence timestamp and store it along with the buffer
-        // TODO: Check if we really need the dup here
-        android::sp<android::Fence> releaseFence = nullptr;
-        if (buffer.releaseFence.fds.size() == 1 && buffer.releaseFence.fds[0].get() >= 0) {
-            releaseFence = new android::Fence(dup(buffer.releaseFence.fds[0].get()));
-        }
-
         CameraAidlTest::InFlightRequest::StreamBufferAndTimestamp streamBufferAndTimestamp;
         auto outstandingBuffers = mUseHalBufManager ? mOutstandingBufferIds :
             request->mOutstandingBufferIds;
@@ -438,12 +430,6 @@ bool DeviceCb::processCaptureResultLocked(
                                            ::android::makeFromAidl(buffer.acquireFence),
                                            ::android::makeFromAidl(buffer.releaseFence)};
         streamBufferAndTimestamp.timeStamp = systemTime();
-        if (releaseFence && releaseFence->isValid()) {
-            releaseFence->wait(/*ms*/ 300);
-            nsecs_t releaseTime = releaseFence->getSignalTime();
-            if (streamBufferAndTimestamp.timeStamp < releaseTime)
-                streamBufferAndTimestamp.timeStamp = releaseTime;
-        }
         request->resultOutputBuffers.push_back(streamBufferAndTimestamp);
     }
     // If shutter event is received notify the pending threads.