From 4bc4a3845e456fd464556d79d20650a107e873e5 Mon Sep 17 00:00:00 2001 From: Igor Murashkin Date: Wed, 20 Feb 2013 13:36:17 -0800 Subject: [PATCH] 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 --- camera/ProCamera.cpp | 41 +++++++++++++++++++++++------ camera/tests/ProCameraTests.cpp | 57 +++++++++++++++++++++++++++++++++++++++-- include/camera/ProCamera.h | 17 +++++++++--- 3 files changed, 102 insertions(+), 13 deletions(-) diff --git a/camera/ProCamera.cpp b/camera/ProCamera.cpp index d4a95563e9..7c66d621ea 100644 --- a/camera/ProCamera.cpp +++ b/camera/ProCamera.cpp @@ -432,20 +432,21 @@ void ProCamera::onFrameAvailable(int streamId) { // Unblock waitForFrame(id) callers { Mutex::Autolock al(mWaitMutex); - getStreamInfo(streamId).frameReady = true; + getStreamInfo(streamId).frameReady++; mWaitCondition.broadcast(); } } -status_t ProCamera::waitForFrameBuffer(int streamId) { +int ProCamera::waitForFrameBuffer(int streamId) { status_t stat = BAD_VALUE; Mutex::Autolock al(mWaitMutex); StreamInfo& si = getStreamInfo(streamId); - if (si.frameReady) { - si.frameReady = false; - return OK; + if (si.frameReady > 0) { + int numFrames = si.frameReady; + si.frameReady = 0; + return numFrames; } else { while (true) { stat = mWaitCondition.waitRelative(mWaitMutex, @@ -456,9 +457,10 @@ status_t ProCamera::waitForFrameBuffer(int streamId) { return stat; } - if (si.frameReady) { - si.frameReady = false; - return OK; + if (si.frameReady > 0) { + int numFrames = si.frameReady; + si.frameReady = 0; + return numFrames; } // else it was some other stream that got unblocked } @@ -467,6 +469,29 @@ status_t ProCamera::waitForFrameBuffer(int streamId) { 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 stat = BAD_VALUE; Mutex::Autolock al(mWaitMutex); diff --git a/camera/tests/ProCameraTests.cpp b/camera/tests/ProCameraTests.cpp index 33c91795eb..f93e5cdfbb 100644 --- a/camera/tests/ProCameraTests.cpp +++ b/camera/tests/ProCameraTests.cpp @@ -55,6 +55,8 @@ namespace client { #define TEST_CPU_FRAME_COUNT 2 #define TEST_CPU_HEAP_COUNT 5 +#define TEST_FRAME_PROCESSING_DELAY_US 200000 // 200 ms + #if TEST_DEBUGGING #define dout std::cerr #else @@ -888,7 +890,7 @@ TEST_F(ProCameraTest, WaitForSingleStreamBuffer) { // Consume a couple of results for (int i = 0; i < TEST_CPU_FRAME_COUNT; ++i) { - EXPECT_OK(mCamera->waitForFrameBuffer(streamId)); + EXPECT_EQ(1, mCamera->waitForFrameBuffer(streamId)); CpuConsumer::LockedBuffer buf; EXPECT_OK(consumer->lockNextBuffer(&buf)); @@ -942,7 +944,7 @@ TEST_F(ProCameraTest, WaitForDualStreamBuffer) { // Get the buffers - EXPECT_OK(mCamera->waitForFrameBuffer(depthStreamId)); + EXPECT_EQ(1, mCamera->waitForFrameBuffer(depthStreamId)); /** * Guaranteed to be able to consume the depth frame, @@ -978,7 +980,58 @@ TEST_F(ProCameraTest, WaitForDualStreamBuffer) { EXPECT_OK(mCamera->exclusiveUnlock()); } +TEST_F(ProCameraTest, WaitForSingleStreamBufferAndDropFrames) { + if (HasFatalFailure()) { + return; + } + + const int NUM_REQUESTS = 20 * TEST_CPU_FRAME_COUNT; + + int streamId = -1; + sp 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()); +} diff --git a/include/camera/ProCamera.h b/include/camera/ProCamera.h index f813c1c746..cd2772cb99 100644 --- a/include/camera/ProCamera.h +++ b/include/camera/ProCamera.h @@ -196,9 +196,12 @@ public: // Blocks until a frame is available (CPU streams only) // - Obtain the frame data by calling CpuConsumer::lockNextBuffer // - 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: // -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 // - Obtain the metadata by calling consumeFrameMetadata() @@ -211,6 +214,14 @@ public: // - Use waitForFrameMetadata to sync until new data is available. 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 remote(); protected: @@ -286,7 +297,7 @@ private: StreamInfo(int streamId) { this->streamID = streamId; cpuStream = false; - frameReady = false; + frameReady = 0; } StreamInfo() { @@ -299,7 +310,7 @@ private: sp cpuConsumer; sp frameAvailableListener; sp stc; - bool frameReady; + int frameReady; }; Condition mWaitCondition; -- 2.11.0