Merge "Fixed disconnect bug in SurfaceTexture"

This commit is contained in:
Jamie Gennis 2012-04-06 09:05:49 -07:00 committed by Android (Google) Code Review
commit 8cce8a9a75
5 changed files with 227 additions and 58 deletions

View File

@ -42,6 +42,7 @@ public:
enum { NUM_BUFFER_SLOTS = 32 };
enum { NO_CONNECTED_API = 0 };
enum { INVALID_BUFFER_SLOT = -1 };
enum { STALE_BUFFER_SLOT = 1 };
// ConsumerListener is the interface through which the BufferQueue notifies
// the consumer of events that the consumer may wish to react to. Because
@ -297,7 +298,8 @@ private:
mTimestamp(0),
mFrameNumber(0),
mFence(EGL_NO_SYNC_KHR),
mAcquireCalled(false) {
mAcquireCalled(false),
mNeedsCleanupOnRelease(false) {
mCrop.makeInvalid();
}
@ -376,6 +378,9 @@ private:
// Indicates whether this buffer has been seen by a consumer yet
bool mAcquireCalled;
// Indicates whether this buffer needs to be cleaned up by consumer
bool mNeedsCleanupOnRelease;
};
// mSlots is the array of buffer slots that must be mirrored on the client

View File

@ -185,7 +185,6 @@ public:
status_t setConsumerUsageBits(uint32_t usage);
status_t setTransformHint(uint32_t hint);
virtual status_t setSynchronousMode(bool enabled);
virtual status_t setBufferCount(int bufferCount);
virtual status_t connect(int api,
uint32_t* outWidth, uint32_t* outHeight, uint32_t* outTransform);

View File

@ -163,48 +163,58 @@ status_t BufferQueue::setTransformHint(uint32_t hint) {
status_t BufferQueue::setBufferCount(int bufferCount) {
ST_LOGV("setBufferCount: count=%d", bufferCount);
Mutex::Autolock lock(mMutex);
if (mAbandoned) {
ST_LOGE("setBufferCount: SurfaceTexture has been abandoned!");
return NO_INIT;
}
if (bufferCount > NUM_BUFFER_SLOTS) {
ST_LOGE("setBufferCount: bufferCount larger than slots available");
return BAD_VALUE;
}
sp<ConsumerListener> listener;
{
Mutex::Autolock lock(mMutex);
// Error out if the user has dequeued buffers
for (int i=0 ; i<mBufferCount ; i++) {
if (mSlots[i].mBufferState == BufferSlot::DEQUEUED) {
ST_LOGE("setBufferCount: client owns some buffers");
return -EINVAL;
if (mAbandoned) {
ST_LOGE("setBufferCount: SurfaceTexture has been abandoned!");
return NO_INIT;
}
if (bufferCount > NUM_BUFFER_SLOTS) {
ST_LOGE("setBufferCount: bufferCount larger than slots available");
return BAD_VALUE;
}
// Error out if the user has dequeued buffers
for (int i=0 ; i<mBufferCount ; i++) {
if (mSlots[i].mBufferState == BufferSlot::DEQUEUED) {
ST_LOGE("setBufferCount: client owns some buffers");
return -EINVAL;
}
}
const int minBufferSlots = mSynchronousMode ?
MIN_SYNC_BUFFER_SLOTS : MIN_ASYNC_BUFFER_SLOTS;
if (bufferCount == 0) {
mClientBufferCount = 0;
bufferCount = (mServerBufferCount >= minBufferSlots) ?
mServerBufferCount : minBufferSlots;
return setBufferCountServerLocked(bufferCount);
}
if (bufferCount < minBufferSlots) {
ST_LOGE("setBufferCount: requested buffer count (%d) is less than "
"minimum (%d)", bufferCount, minBufferSlots);
return BAD_VALUE;
}
// here we're guaranteed that the client doesn't have dequeued buffers
// and will release all of its buffer references.
freeAllBuffersLocked();
mBufferCount = bufferCount;
mClientBufferCount = bufferCount;
mBufferHasBeenQueued = false;
mQueue.clear();
mDequeueCondition.broadcast();
listener = mConsumerListener;
} // scope for lock
if (listener != NULL) {
listener->onBuffersReleased();
}
const int minBufferSlots = mSynchronousMode ?
MIN_SYNC_BUFFER_SLOTS : MIN_ASYNC_BUFFER_SLOTS;
if (bufferCount == 0) {
mClientBufferCount = 0;
bufferCount = (mServerBufferCount >= minBufferSlots) ?
mServerBufferCount : minBufferSlots;
return setBufferCountServerLocked(bufferCount);
}
if (bufferCount < minBufferSlots) {
ST_LOGE("setBufferCount: requested buffer count (%d) is less than "
"minimum (%d)", bufferCount, minBufferSlots);
return BAD_VALUE;
}
// here we're guaranteed that the client doesn't have dequeued buffers
// and will release all of its buffer references.
freeAllBuffersLocked();
mBufferCount = bufferCount;
mClientBufferCount = bufferCount;
mBufferHasBeenQueued = false;
mQueue.clear();
mDequeueCondition.broadcast();
return OK;
}
@ -780,6 +790,9 @@ void BufferQueue::dump(String8& result, const char* prefix,
void BufferQueue::freeBufferLocked(int i) {
mSlots[i].mGraphicBuffer = 0;
if (mSlots[i].mBufferState == BufferSlot::ACQUIRED) {
mSlots[i].mNeedsCleanupOnRelease = true;
}
mSlots[i].mBufferState = BufferSlot::FREE;
mSlots[i].mFrameNumber = 0;
mSlots[i].mAcquireCalled = false;
@ -853,15 +866,19 @@ status_t BufferQueue::releaseBuffer(int buf, EGLDisplay display,
mSlots[buf].mEglDisplay = display;
mSlots[buf].mFence = fence;
// The current buffer becomes FREE if it was still in the queued
// state. If it has already been given to the client
// (synchronous mode), then it stays in DEQUEUED state.
if (mSlots[buf].mBufferState == BufferSlot::QUEUED
|| mSlots[buf].mBufferState == BufferSlot::ACQUIRED) {
// The buffer can now only be released if its in the acquired state
if (mSlots[buf].mBufferState == BufferSlot::ACQUIRED) {
mSlots[buf].mBufferState = BufferSlot::FREE;
} else if (mSlots[buf].mNeedsCleanupOnRelease) {
ST_LOGV("releasing a stale buf %d its state was %d", buf, mSlots[buf].mBufferState);
mSlots[buf].mNeedsCleanupOnRelease = false;
return STALE_BUFFER_SLOT;
} else {
ST_LOGE("attempted to release buf %d but its state was %d", buf, mSlots[buf].mBufferState);
return -EINVAL;
}
mDequeueCondition.broadcast();
mDequeueCondition.broadcast();
return OK;
}

View File

@ -176,6 +176,8 @@ status_t SurfaceTexture::updateTexImage() {
ST_LOGV("updateTexImage");
Mutex::Autolock lock(mMutex);
status_t err = NO_ERROR;
if (mAbandoned) {
ST_LOGE("updateTexImage: SurfaceTexture is abandoned!");
return NO_INIT;
@ -269,11 +271,17 @@ status_t SurfaceTexture::updateTexImage() {
mCurrentTextureBuf != NULL ? mCurrentTextureBuf->handle : 0,
buf, item.mGraphicBuffer != NULL ? item.mGraphicBuffer->handle : 0);
// Release the old buffer
// release old buffer
if (mCurrentTexture != BufferQueue::INVALID_BUFFER_SLOT) {
mBufferQueue->releaseBuffer(mCurrentTexture, dpy,
mEGLSlots[mCurrentTexture].mFence);
status_t status = mBufferQueue->releaseBuffer(mCurrentTexture, dpy, mEGLSlots[mCurrentTexture].mFence);
mEGLSlots[mCurrentTexture].mFence = EGL_NO_SYNC_KHR;
if (status == BufferQueue::STALE_BUFFER_SLOT) {
freeBufferLocked(mCurrentTexture);
} else if (status != OK) {
ST_LOGE("updateTexImage: released invalid buffer");
err = status;
}
}
// Update the SurfaceTexture state.
@ -289,7 +297,7 @@ status_t SurfaceTexture::updateTexImage() {
glBindTexture(mTexTarget, mTexName);
}
return OK;
return err;
}
status_t SurfaceTexture::detachFromContext() {
@ -630,6 +638,9 @@ bool SurfaceTexture::isSynchronousMode() const {
void SurfaceTexture::freeBufferLocked(int slotIndex) {
ST_LOGV("freeBufferLocked: slotIndex=%d", slotIndex);
mEGLSlots[slotIndex].mGraphicBuffer = 0;
if (slotIndex == mCurrentTexture) {
mCurrentTexture = BufferQueue::INVALID_BUFFER_SLOT;
}
if (mEGLSlots[slotIndex].mEglImage != EGL_NO_IMAGE_KHR) {
EGLImageKHR img = mEGLSlots[slotIndex].mEglImage;
if (img != EGL_NO_IMAGE_KHR) {
@ -692,12 +703,6 @@ sp<BufferQueue> SurfaceTexture::getBufferQueue() const {
return mBufferQueue;
}
// Used for refactoring, should not be in final interface
status_t SurfaceTexture::setBufferCount(int bufferCount) {
Mutex::Autolock lock(mMutex);
return mBufferQueue->setBufferCount(bufferCount);
}
// Used for refactoring, should not be in final interface
status_t SurfaceTexture::connect(int api,
uint32_t* outWidth, uint32_t* outHeight, uint32_t* outTransform) {
@ -737,8 +742,6 @@ void SurfaceTexture::onBuffersReleased() {
freeBufferLocked(i);
}
}
mCurrentTexture = BufferQueue::INVALID_BUFFER_SLOT;
}
void SurfaceTexture::dump(String8& result) const

View File

@ -132,7 +132,7 @@ protected:
}
virtual void TearDown() {
// Display the result
// Display the result
if (mDisplaySecs > 0 && mEglSurface != EGL_NO_SURFACE) {
eglSwapBuffers(mEglDisplay, mEglSurface);
sleep(mDisplaySecs);
@ -486,6 +486,55 @@ protected:
Condition mCondition;
};
// Note that SurfaceTexture will lose the notifications
// onBuffersReleased and onFrameAvailable as there is currently
// no way to forward the events. This DisconnectWaiter will not let the
// disconnect finish until finishDisconnect() is called. It will
// also block until a disconnect is called
class DisconnectWaiter : public BufferQueue::ConsumerListener {
public:
DisconnectWaiter () :
mWaitForDisconnect(false),
mPendingFrames(0) {
}
void waitForFrame() {
Mutex::Autolock lock(mMutex);
while (mPendingFrames == 0) {
mFrameCondition.wait(mMutex);
}
mPendingFrames--;
}
virtual void onFrameAvailable() {
Mutex::Autolock lock(mMutex);
mPendingFrames++;
mFrameCondition.signal();
}
virtual void onBuffersReleased() {
Mutex::Autolock lock(mMutex);
while (!mWaitForDisconnect) {
mDisconnectCondition.wait(mMutex);
}
}
void finishDisconnect() {
Mutex::Autolock lock(mMutex);
mWaitForDisconnect = true;
mDisconnectCondition.signal();
}
private:
Mutex mMutex;
bool mWaitForDisconnect;
Condition mDisconnectCondition;
int mPendingFrames;
Condition mFrameCondition;
};
sp<SurfaceTexture> mST;
sp<SurfaceTextureClient> mSTC;
sp<ANativeWindow> mANW;
@ -984,6 +1033,102 @@ TEST_F(SurfaceTextureGLTest, TexturingFromCpuFilledRGBABufferPow2) {
EXPECT_TRUE(checkPixel( 3, 52, 35, 231, 35, 35));
}
// Tests if SurfaceTexture and BufferQueue are robust enough
// to handle a special case where updateTexImage is called
// in the middle of disconnect. This ordering is enforced
// by blocking in the disconnect callback.
TEST_F(SurfaceTextureGLTest, DisconnectStressTest) {
class ProducerThread : public Thread {
public:
ProducerThread(const sp<ANativeWindow>& anw):
mANW(anw) {
}
virtual ~ProducerThread() {
}
virtual bool threadLoop() {
ANativeWindowBuffer* anb;
native_window_api_connect(mANW.get(), NATIVE_WINDOW_API_EGL);
for (int numFrames =0 ; numFrames < 2; numFrames ++) {
if (mANW->dequeueBuffer(mANW.get(), &anb) != NO_ERROR) {
return false;
}
if (anb == NULL) {
return false;
}
if (mANW->queueBuffer(mANW.get(), anb)
!= NO_ERROR) {
return false;
}
}
native_window_api_disconnect(mANW.get(), NATIVE_WINDOW_API_EGL);
return false;
}
private:
sp<ANativeWindow> mANW;
};
ASSERT_EQ(OK, mST->setSynchronousMode(true));
sp<DisconnectWaiter> dw(new DisconnectWaiter());
mST->getBufferQueue()->consumerConnect(dw);
sp<Thread> pt(new ProducerThread(mANW));
pt->run();
// eat a frame so SurfaceTexture will own an at least one slot
dw->waitForFrame();
EXPECT_EQ(OK,mST->updateTexImage());
dw->waitForFrame();
// Could fail here as SurfaceTexture thinks it still owns the slot
// but bufferQueue has released all slots
EXPECT_EQ(OK,mST->updateTexImage());
dw->finishDisconnect();
}
// This test ensures that the SurfaceTexture clears the mCurrentTexture
// when it is disconnected and reconnected. Otherwise it will
// attempt to release a buffer that it does not owned
TEST_F(SurfaceTextureGLTest, DisconnectClearsCurrentTexture) {
ASSERT_EQ(OK, mST->setSynchronousMode(true));
native_window_api_connect(mANW.get(), NATIVE_WINDOW_API_EGL);
ANativeWindowBuffer *anb;
EXPECT_EQ (OK, mANW->dequeueBuffer(mANW.get(), &anb));
EXPECT_EQ(OK, mANW->queueBuffer(mANW.get(), anb));
EXPECT_EQ (OK, mANW->dequeueBuffer(mANW.get(), &anb));
EXPECT_EQ(OK, mANW->queueBuffer(mANW.get(), anb));
EXPECT_EQ(OK,mST->updateTexImage());
EXPECT_EQ(OK,mST->updateTexImage());
native_window_api_disconnect(mANW.get(), NATIVE_WINDOW_API_EGL);
native_window_api_connect(mANW.get(), NATIVE_WINDOW_API_EGL);
ASSERT_EQ(OK, mST->setSynchronousMode(true));
EXPECT_EQ (OK, mANW->dequeueBuffer(mANW.get(), &anb));
EXPECT_EQ(OK, mANW->queueBuffer(mANW.get(), anb));
// Will fail here if mCurrentTexture is not cleared properly
EXPECT_EQ(OK,mST->updateTexImage());
}
TEST_F(SurfaceTextureGLTest, AbandonUnblocksDequeueBuffer) {
class ProducerThread : public Thread {
public: