Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 0f56c567 authored by Shuzhen Wang's avatar Shuzhen Wang
Browse files

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
parent c9fa6a8e

camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp

+9 −74
Original line number Diff line number Diff line
@@ -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,

camera/provider/aidl/vts/VtsAidlHalCameraProvider_TargetTest.cpp

+2 −0
Original line number Diff line number Diff line
@@ -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);

camera/provider/aidl/vts/camera_aidl_test.cpp

+21 −0
Original line number Diff line number Diff line
@@ -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);

camera/provider/aidl/vts/camera_aidl_test.h

+3 −0
Original line number Diff line number Diff line
@@ -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;

camera/provider/aidl/vts/device_cb.cpp

+0 −14
Original line number Diff line number Diff line
@@ -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.