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

Commit 4bc4a384 authored by Igor Murashkin's avatar Igor Murashkin
Browse files

ProCamera: Fix waitForFrameBuffer not handling multiple outstanding frames 

If the CpuConsumer triggered multiple onFrameAvailable callbacks in between
a single waitForFrameBuffer call, the old code would only handle 1 callback.

This meant on two subsequent waitForFrameBuffer calls the second would always
timeout when two buffers were already available to be unlocked.

Bug: 8238112
Change-Id: Ibefca35005ac5c408e5ada97ec4a4344a9e3e497
parent a140a6ef

camera/ProCamera.cpp

+33 −8
Original line number Original line Diff line number Diff line
@@ -432,20 +432,21 @@ void ProCamera::onFrameAvailable(int streamId) { 
    // Unblock waitForFrame(id) callers

    // Unblock waitForFrame(id) callers

    {

    {

        Mutex::Autolock al(mWaitMutex);

        Mutex::Autolock al(mWaitMutex);

        getStreamInfo(streamId).frameReady = true;

        getStreamInfo(streamId).frameReady++;

        mWaitCondition.broadcast();

        mWaitCondition.broadcast();

    }

    }

}

}





status_t ProCamera::waitForFrameBuffer(int streamId) {

int ProCamera::waitForFrameBuffer(int streamId) {

    status_t stat = BAD_VALUE;

    status_t stat = BAD_VALUE;

    Mutex::Autolock al(mWaitMutex);

    Mutex::Autolock al(mWaitMutex);





    StreamInfo& si = getStreamInfo(streamId);

    StreamInfo& si = getStreamInfo(streamId);





    if (si.frameReady) {

    if (si.frameReady > 0) {

        si.frameReady = false;

        int numFrames = si.frameReady;

        return OK;

        si.frameReady = 0;

        return numFrames;

    } else {

    } else {

        while (true) {

        while (true) {

            stat = mWaitCondition.waitRelative(mWaitMutex,

            stat = mWaitCondition.waitRelative(mWaitMutex,
@@ -456,9 +457,10 @@ status_t ProCamera::waitForFrameBuffer(int streamId) { 
                return stat;

                return stat;

            }

            }





            if (si.frameReady) {

            if (si.frameReady > 0) {

                si.frameReady = false;

                int numFrames = si.frameReady;

                return OK;

                si.frameReady = 0;

                return numFrames;

            }

            }

            // else it was some other stream that got unblocked

            // else it was some other stream that got unblocked

        }

        }
@@ -467,6 +469,29 @@ status_t ProCamera::waitForFrameBuffer(int streamId) { 
    return stat;

    return stat;

}

}





int ProCamera::dropFrameBuffer(int streamId, int count) {

    StreamInfo& si = getStreamInfo(streamId);



    if (!si.cpuStream) {

        return BAD_VALUE;

    } else if (count < 0) {

        return BAD_VALUE;

    }



    int numDropped = 0;

    for (int i = 0; i < count; ++i) {

        CpuConsumer::LockedBuffer buffer;

        if (si.cpuConsumer->lockNextBuffer(&buffer) != OK) {

            break;

        }



        si.cpuConsumer->unlockBuffer(buffer);

        numDropped++;

    }



    return numDropped;

}



status_t ProCamera::waitForFrameMetadata() {

status_t ProCamera::waitForFrameMetadata() {

    status_t stat = BAD_VALUE;

    status_t stat = BAD_VALUE;

    Mutex::Autolock al(mWaitMutex);

    Mutex::Autolock al(mWaitMutex);

camera/tests/ProCameraTests.cpp

+55 −2
Original line number Original line Diff line number Diff line
@@ -55,6 +55,8 @@ namespace client { 
#define TEST_CPU_FRAME_COUNT 2

#define TEST_CPU_FRAME_COUNT 2

#define TEST_CPU_HEAP_COUNT 5

#define TEST_CPU_HEAP_COUNT 5





#define TEST_FRAME_PROCESSING_DELAY_US 200000 // 200 ms



#if TEST_DEBUGGING

#if TEST_DEBUGGING

#define dout std::cerr

#define dout std::cerr

#else

#else
@@ -888,7 +890,7 @@ TEST_F(ProCameraTest, WaitForSingleStreamBuffer) { 




    // Consume a couple of results

    // Consume a couple of results

    for (int i = 0; i < TEST_CPU_FRAME_COUNT; ++i) {

    for (int i = 0; i < TEST_CPU_FRAME_COUNT; ++i) {

        EXPECT_OK(mCamera->waitForFrameBuffer(streamId));

        EXPECT_EQ(1, mCamera->waitForFrameBuffer(streamId));





        CpuConsumer::LockedBuffer buf;

        CpuConsumer::LockedBuffer buf;

        EXPECT_OK(consumer->lockNextBuffer(&buf));

        EXPECT_OK(consumer->lockNextBuffer(&buf));
@@ -942,7 +944,7 @@ TEST_F(ProCameraTest, WaitForDualStreamBuffer) { 




        // Get the buffers

        // Get the buffers





        EXPECT_OK(mCamera->waitForFrameBuffer(depthStreamId));

        EXPECT_EQ(1, mCamera->waitForFrameBuffer(depthStreamId));





        /**

        /**

          * Guaranteed to be able to consume the depth frame,

          * Guaranteed to be able to consume the depth frame,
@@ -978,7 +980,58 @@ TEST_F(ProCameraTest, WaitForDualStreamBuffer) { 
    EXPECT_OK(mCamera->exclusiveUnlock());

    EXPECT_OK(mCamera->exclusiveUnlock());

}

}





TEST_F(ProCameraTest, WaitForSingleStreamBufferAndDropFrames) {

    if (HasFatalFailure()) {

        return;

    }



    const int NUM_REQUESTS = 20 * TEST_CPU_FRAME_COUNT;



    int streamId = -1;

    sp<CpuConsumer> consumer;

    EXPECT_OK(mCamera->createStreamCpu(/*width*/1280, /*height*/960,

                  TEST_FORMAT_MAIN, TEST_CPU_HEAP_COUNT, &consumer, &streamId));

    EXPECT_NE(-1, streamId);



    EXPECT_OK(mCamera->exclusiveTryLock());



    uint8_t streams[] = { streamId };

    ASSERT_NO_FATAL_FAILURE(createSubmitRequestForStreams(streams, /*count*/1,

                                                     /*requests*/NUM_REQUESTS));



    // Consume a couple of results

    for (int i = 0; i < NUM_REQUESTS; ++i) {

        // Process at 10fps, stream is at 15fps.

        // This means we will definitely fill up the buffer queue with

        // extra buffers and need to drop them.

        usleep(TEST_FRAME_PROCESSING_DELAY_US);



        int numFrames;

        EXPECT_TRUE((numFrames = mCamera->waitForFrameBuffer(streamId)) > 0);



        // Drop all but the newest framebuffer

        EXPECT_EQ(numFrames-1, mCamera->dropFrameBuffer(streamId, numFrames-1));



        dout << "Dropped " << (numFrames - 1) << " frames" << std::endl;



        // Skip the counter ahead, don't try to consume these frames again

        i += numFrames-1;



        // "Consume" the buffer

        CpuConsumer::LockedBuffer buf;

        EXPECT_OK(consumer->lockNextBuffer(&buf));



        dout << "Buffer synchronously received on streamId = " << streamId <<

                ", dataPtr = " << (void*)buf.data <<

                ", timestamp = " << buf.timestamp << std::endl;



        EXPECT_OK(consumer->unlockBuffer(buf));

    }





    // Done: clean up

    EXPECT_OK(mCamera->deleteStream(streamId));

    EXPECT_OK(mCamera->exclusiveUnlock());

}

include/camera/ProCamera.h

+14 −3
Original line number Original line Diff line number Diff line
@@ -196,9 +196,12 @@ public: 
    // Blocks until a frame is available (CPU streams only)

    // Blocks until a frame is available (CPU streams only)

    // - Obtain the frame data by calling CpuConsumer::lockNextBuffer

    // - Obtain the frame data by calling CpuConsumer::lockNextBuffer

    // - Release the frame data after use with CpuConsumer::unlockBuffer

    // - Release the frame data after use with CpuConsumer::unlockBuffer

    // Return value:

    // - >0 - number of frames available to be locked

    // - <0 - error (refer to error codes)

    // Error codes:

    // Error codes:

    // -ETIMEDOUT if it took too long to get a frame

    // -ETIMEDOUT if it took too long to get a frame

    status_t waitForFrameBuffer(int streamId);

    int waitForFrameBuffer(int streamId);





    // Blocks until a metadata result is available

    // Blocks until a metadata result is available

    // - Obtain the metadata by calling consumeFrameMetadata()

    // - Obtain the metadata by calling consumeFrameMetadata()
@@ -211,6 +214,14 @@ public: 
    // - Use waitForFrameMetadata to sync until new data is available.

    // - Use waitForFrameMetadata to sync until new data is available.

    CameraMetadata consumeFrameMetadata();

    CameraMetadata consumeFrameMetadata();





    // Convenience method to drop frame buffers (CPU streams only)

    // Return values:

    //  >=0 - number of frames dropped (up to count)

    //  <0  - error code

    // Error codes:

    //   BAD_VALUE - invalid streamId or count passed

    int dropFrameBuffer(int streamId, int count);



    sp<IProCameraUser>         remote();

    sp<IProCameraUser>         remote();





protected:

protected:
@@ -286,7 +297,7 @@ private: 
        StreamInfo(int streamId) {

        StreamInfo(int streamId) {

            this->streamID = streamId;

            this->streamID = streamId;

            cpuStream = false;

            cpuStream = false;

            frameReady = false;

            frameReady = 0;

        }

        }





        StreamInfo() {

        StreamInfo() {
@@ -299,7 +310,7 @@ private: 
        sp<CpuConsumer> cpuConsumer;

        sp<CpuConsumer> cpuConsumer;

        sp<ProFrameListener> frameAvailableListener;

        sp<ProFrameListener> frameAvailableListener;

        sp<Surface> stc;

        sp<Surface> stc;

        bool frameReady;

        int frameReady;

    };

    };





    Condition mWaitCondition;

    Condition mWaitCondition;