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
diff --git a/camera/ProCamera.cpp b/camera/ProCamera.cpp
index d4a9556..7c66d62 100644
--- a/camera/ProCamera.cpp
+++ b/camera/ProCamera.cpp
@@ -432,20 +432,21 @@
// 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 @@
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 @@
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 33c9179..f93e5cd 100644
--- a/camera/tests/ProCameraTests.cpp
+++ b/camera/tests/ProCameraTests.cpp
@@ -55,6 +55,8 @@
#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 @@
// 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 @@
// 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 @@
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());
+}
diff --git a/include/camera/ProCamera.h b/include/camera/ProCamera.h
index f813c1c..cd2772c 100644
--- a/include/camera/ProCamera.h
+++ b/include/camera/ProCamera.h
@@ -196,9 +196,12 @@
// 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 @@
// - 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<IProCameraUser> remote();
protected:
@@ -286,7 +297,7 @@
StreamInfo(int streamId) {
this->streamID = streamId;
cpuStream = false;
- frameReady = false;
+ frameReady = 0;
}
StreamInfo() {
@@ -299,7 +310,7 @@
sp<CpuConsumer> cpuConsumer;
sp<ProFrameListener> frameAvailableListener;
sp<Surface> stc;
- bool frameReady;
+ int frameReady;
};
Condition mWaitCondition;