From fa5b40ebb8923133df12dc70591bfe35b3f1c9b3 Mon Sep 17 00:00:00 2001 From: Jamie Gennis Date: Thu, 15 Mar 2012 14:01:24 -0700 Subject: [PATCH] libgui: add BQ consumer buffer free notifications This change adds a new callback for BufferQueue consumers to be notified when the BufferQueue frees some or all of its buffers. This is needed to retain SurfaceTexture behavior where all buffers would be freed when the producer disconnects. This change also modifies the SurfaceTextureGLToGLTest.EglDestroySurfaceUnrefsBuffers test to catch when the buffers are not freed. The implementation is a little complicated because it needs to avoid circular sp<> references across what will be a binder interface (so wp<> can't be used directly). It also needs to avoid the possibility of locking the BufferQueue and consumer (e.g. SurfaceTexture) mutexes in the wrong order. This change also includes a few additional fixes and test cleanups. Change-Id: I27b77d0af15cb4b135f4b63573f634f5f0da2182 --- include/gui/BufferQueue.h | 95 ++++++++--- include/gui/SurfaceTexture.h | 54 +++++-- libs/gui/BufferQueue.cpp | 160 +++++++++++++------ libs/gui/SurfaceTexture.cpp | 111 ++++++++++--- libs/gui/tests/SurfaceTextureClient_test.cpp | 20 ++- libs/gui/tests/SurfaceTexture_test.cpp | 39 +++-- 6 files changed, 364 insertions(+), 115 deletions(-) diff --git a/include/gui/BufferQueue.h b/include/gui/BufferQueue.h index 46d185413..c01f2be42 100644 --- a/include/gui/BufferQueue.h +++ b/include/gui/BufferQueue.h @@ -42,18 +42,57 @@ public: enum { NO_CONNECTED_API = 0 }; enum { INVALID_BUFFER_SLOT = -1 }; - struct FrameAvailableListener : public virtual RefBase { - // onFrameAvailable() is called from queueBuffer() each time an - // additional frame becomes available for consumption. This means that - // frames that are queued while in asynchronous mode only trigger the - // callback if no previous frames are pending. Frames queued while in - // synchronous mode always trigger the callback. + // ConsumerListener is the interface through which the BufferQueue notifies + // the consumer of events that the consumer may wish to react to. Because + // the consumer will generally have a mutex that is locked during calls from + // teh consumer to the BufferQueue, these calls from the BufferQueue to the + // consumer *MUST* be called only when the BufferQueue mutex is NOT locked. + struct ConsumerListener : public virtual RefBase { + // onFrameAvailable is called from queueBuffer each time an additional + // frame becomes available for consumption. This means that frames that + // are queued while in asynchronous mode only trigger the callback if no + // previous frames are pending. Frames queued while in synchronous mode + // always trigger the callback. // // This is called without any lock held and can be called concurrently // by multiple threads. virtual void onFrameAvailable() = 0; + + // onBuffersReleased is called to notify the buffer consumer that the + // BufferQueue has released its references to one or more GraphicBuffers + // contained in its slots. The buffer consumer should then call + // BufferQueue::getReleasedBuffers to retrieve the list of buffers + // + // This is called without any lock held and can be called concurrently + // by multiple threads. + virtual void onBuffersReleased() = 0; }; + // ProxyConsumerListener is a ConsumerListener implementation that keeps a weak + // reference to the actual consumer object. It forwards all calls to that + // consumer object so long as it exists. + // + // This class exists to avoid having a circular reference between the + // BufferQueue object and the consumer object. The reason this can't be a weak + // reference in the BufferQueue class is because we're planning to expose the + // consumer side of a BufferQueue as a binder interface, which doesn't support + // weak references. + class ProxyConsumerListener : public BufferQueue::ConsumerListener { + public: + + ProxyConsumerListener(const wp& consumerListener); + virtual ~ProxyConsumerListener(); + virtual void onFrameAvailable(); + virtual void onBuffersReleased(); + + private: + + // mConsumerListener is a weak reference to the ConsumerListener. This is + // the raison d'etre of ProxyConsumerListener. + wp mConsumerListener; + }; + + // BufferQueue manages a pool of gralloc memory slots to be used // by producers and consumers. // allowSynchronousMode specifies whether or not synchronous mode can be @@ -168,21 +207,39 @@ public: // The following public functions is the consumer facing interface - // acquire consumes a buffer by transferring its ownership to a consumer. - // buffer contains the GraphicBuffer and its corresponding information. - // buffer.mGraphicsBuffer will be NULL when the buffer has been already - // acquired by the consumer. - - status_t acquire(BufferItem *buffer); + // acquireBuffer attempts to acquire ownership of the next pending buffer in + // the BufferQueue. If no buffer is pending then it returns -EINVAL. If a + // buffer is successfully acquired, the information about the buffer is + // returned in BufferItem. If the buffer returned had previously been + // acquired then the BufferItem::mGraphicBuffer field of buffer is set to + // NULL and it is assumed that the consumer still holds a reference to the + // buffer. + status_t acquireBuffer(BufferItem *buffer); // releaseBuffer releases a buffer slot from the consumer back to the // BufferQueue pending a fence sync. + // + // Note that the dependencies on EGL will be removed once we switch to using + // the Android HW Sync HAL. status_t releaseBuffer(int buf, EGLDisplay display, EGLSyncKHR fence); + // consumerConnect connects a consumer to the BufferQueue. Only one + // consumer may be connected, and when that consumer disconnects the + // BufferQueue is placed into the "abandoned" state, causing most + // interactions with the BufferQueue by the producer to fail. + status_t consumerConnect(const sp& consumer); + // consumerDisconnect disconnects a consumer from the BufferQueue. All - // buffers will be freed. + // buffers will be freed and the BufferQueue is placed in the "abandoned" + // state, causing most interactions with the BufferQueue by the producer to + // fail. status_t consumerDisconnect(); + // getReleasedBuffers sets the value pointed to by slotMask to a bit mask + // indicating which buffer slots the have been released by the BufferQueue + // but have not yet been released by the consumer. + status_t getReleasedBuffers(uint32_t* slotMask); + // setDefaultBufferSize is used to set the size of buffers returned by // requestBuffers when a with and height of zero is requested. status_t setDefaultBufferSize(uint32_t w, uint32_t h); @@ -199,10 +256,6 @@ public: // setConsumerName sets the name used in logging void setConsumerName(const String8& name); - // setFrameAvailableListener sets the listener object that will be notified - // when a new frame becomes available. - void setFrameAvailableListener(const sp& listener); - // setDefaultBufferFormat allows the BufferQueue to create // GraphicBuffers of a defaultFormat if no format is specified // in dequeueBuffer @@ -384,10 +437,10 @@ private: // allocate new GraphicBuffer objects. sp mGraphicBufferAlloc; - // mFrameAvailableListener is the listener object that will be called when a - // new frame becomes available. If it is not NULL it will be called from - // queueBuffer. - sp mFrameAvailableListener; + // mConsumerListener is used to notify the connected consumer of + // asynchronous events that it may wish to react to. It is initially set + // to NULL and is written by consumerConnect and consumerDisconnect. + sp mConsumerListener; // mSynchronousMode whether we're in synchronous mode or not bool mSynchronousMode; diff --git a/include/gui/SurfaceTexture.h b/include/gui/SurfaceTexture.h index 2ab2ab7a4..f8dc3ace2 100644 --- a/include/gui/SurfaceTexture.h +++ b/include/gui/SurfaceTexture.h @@ -39,12 +39,20 @@ namespace android { class String8; -class SurfaceTexture : public virtual RefBase { +class SurfaceTexture : public virtual RefBase, + protected BufferQueue::ConsumerListener { public: - // This typedef allows external code to continue referencing - // SurfaceTexture::FrameAvailableListener during refactoring - typedef BufferQueue::FrameAvailableListener FrameAvailableListener; - + struct FrameAvailableListener : public virtual RefBase { + // onFrameAvailable() is called each time an additional frame becomes + // available for consumption. This means that frames that are queued + // while in asynchronous mode only trigger the callback if no previous + // frames are pending. Frames queued while in synchronous mode always + // trigger the callback. + // + // This is called without any lock held and can be called concurrently + // by multiple threads. + virtual void onFrameAvailable() = 0; + }; // SurfaceTexture constructs a new SurfaceTexture object. tex indicates the // name of the OpenGL ES texture to which images are to be streamed. This @@ -175,6 +183,12 @@ public: protected: + // Implementation of the BufferQueue::ConsumerListener interface. These + // calls are used to notify the SurfaceTexture of asynchronous events in the + // BufferQueue. + virtual void onFrameAvailable(); + virtual void onBuffersReleased(); + static bool isExternalFormat(uint32_t format); private: @@ -183,6 +197,13 @@ private: EGLImageKHR createImage(EGLDisplay dpy, const sp& graphicBuffer); + // freeBufferLocked frees up the given buffer slot. If the slot has been + // initialized this will release the reference to the GraphicBuffer in that + // slot and destroy the EGLImage in that slot. Otherwise it has no effect. + // + // This method must be called with mMutex locked. + void freeBufferLocked(int slotIndex); + // computeCurrentTransformMatrix computes the transform matrix for the // current texture. It uses mCurrentTransform and the current GraphicBuffer // to compute this matrix and stores it in mCurrentTransformMatrix. @@ -234,8 +255,8 @@ private: // browser's tile cache exceeds. const GLenum mTexTarget; - // SurfaceTexture maintains EGL information about GraphicBuffers that corresponds - // directly with BufferQueue's buffers + // EGLSlot contains the information and object references that + // SurfaceTexture maintains about a BufferQueue buffer slot. struct EGLSlot { EGLSlot() : mEglImage(EGL_NO_IMAGE_KHR), @@ -258,6 +279,13 @@ private: EGLSyncKHR mFence; }; + // mEGLSlots stores the buffers that have been allocated by the BufferQueue + // for each buffer slot. It is initialized to null pointers, and gets + // filled in with the result of BufferQueue::acquire when the + // client dequeues a buffer from a + // slot that has not yet been used. The buffer allocated to a slot will also + // be replaced if the requested buffer usage or geometry differs from that + // of the buffer allocated to a slot. EGLSlot mEGLSlots[BufferQueue::NUM_BUFFER_SLOTS]; // mAbandoned indicates that the BufferQueue will no longer be used to @@ -271,10 +299,10 @@ private: // It can be set by the setName method. String8 mName; - // mMutex is the mutex used to prevent concurrent access to the member - // variables of SurfaceTexture objects. It must be locked whenever the - // member variables are accessed. - mutable Mutex mMutex; + // mFrameAvailableListener is the listener object that will be called when a + // new frame becomes available. If it is not NULL it will be called from + // queueBuffer. + sp mFrameAvailableListener; // mCurrentTexture is the buffer slot index of the buffer that is currently // bound to the OpenGL texture. It is initialized to INVALID_BUFFER_SLOT, @@ -288,6 +316,10 @@ private: // if none is supplied sp mBufferQueue; + // mMutex is the mutex used to prevent concurrent access to the member + // variables of SurfaceTexture objects. It must be locked whenever the + // member variables are accessed. + mutable Mutex mMutex; }; // ---------------------------------------------------------------------------- diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp index 0f9c7c5c4..ffc5fa02a 100644 --- a/libs/gui/BufferQueue.cpp +++ b/libs/gui/BufferQueue.cpp @@ -15,8 +15,8 @@ */ #define LOG_TAG "BufferQueue" -//#define LOG_NDEBUG 0 #define ATRACE_TAG ATRACE_TAG_GRAPHICS +//#define LOG_NDEBUG 0 #define GL_GLEXT_PROTOTYPES #define EGL_EGLEXT_PROTOTYPES @@ -146,13 +146,6 @@ void BufferQueue::setConsumerName(const String8& name) { mConsumerName = name; } -void BufferQueue::setFrameAvailableListener( - const sp& listener) { - ST_LOGV("setFrameAvailableListener"); - Mutex::Autolock lock(mMutex); - mFrameAvailableListener = listener; -} - status_t BufferQueue::setDefaultBufferFormat(uint32_t defaultFormat) { Mutex::Autolock lock(mMutex); mDefaultBufferFormat = defaultFormat; @@ -531,7 +524,7 @@ status_t BufferQueue::queueBuffer(int buf, int64_t timestamp, ST_LOGV("queueBuffer: slot=%d time=%lld", buf, timestamp); - sp listener; + sp listener; { // scope for the lock Mutex::Autolock lock(mMutex); @@ -559,7 +552,7 @@ status_t BufferQueue::queueBuffer(int buf, int64_t timestamp, // Synchronous mode always signals that an additional frame should // be consumed. - listener = mFrameAvailableListener; + listener = mConsumerListener; } else { // In asynchronous mode we only keep the most recent buffer. if (mQueue.empty()) { @@ -568,7 +561,7 @@ status_t BufferQueue::queueBuffer(int buf, int64_t timestamp, // Asynchronous mode only signals that a frame should be // consumed if no previous frame was pending. If a frame were // pending then the consumer would have already been notified. - listener = mFrameAvailableListener; + listener = mConsumerListener; } else { Fifo::iterator front(mQueue.begin()); // buffer currently queued is freed @@ -682,6 +675,11 @@ status_t BufferQueue::connect(int api, return NO_INIT; } + if (mConsumerListener == NULL) { + ST_LOGE("connect: BufferQueue has no consumer!"); + return NO_INIT; + } + int err = NO_ERROR; switch (api) { case NATIVE_WINDOW_API_EGL: @@ -712,38 +710,49 @@ status_t BufferQueue::connect(int api, status_t BufferQueue::disconnect(int api) { ATRACE_CALL(); ST_LOGV("disconnect: api=%d", api); - Mutex::Autolock lock(mMutex); - - if (mAbandoned) { - // it is not really an error to disconnect after the surface - // has been abandoned, it should just be a no-op. - return NO_ERROR; - } int err = NO_ERROR; - switch (api) { - case NATIVE_WINDOW_API_EGL: - case NATIVE_WINDOW_API_CPU: - case NATIVE_WINDOW_API_MEDIA: - case NATIVE_WINDOW_API_CAMERA: - if (mConnectedApi == api) { - drainQueueAndFreeBuffersLocked(); - mConnectedApi = NO_CONNECTED_API; - mNextCrop.makeInvalid(); - mNextScalingMode = NATIVE_WINDOW_SCALING_MODE_FREEZE; - mNextTransform = 0; - mDequeueCondition.broadcast(); - } else { - ST_LOGE("disconnect: connected to another api (cur=%d, req=%d)", - mConnectedApi, api); + sp listener; + + { // Scope for the lock + Mutex::Autolock lock(mMutex); + + if (mAbandoned) { + // it is not really an error to disconnect after the surface + // has been abandoned, it should just be a no-op. + return NO_ERROR; + } + + switch (api) { + case NATIVE_WINDOW_API_EGL: + case NATIVE_WINDOW_API_CPU: + case NATIVE_WINDOW_API_MEDIA: + case NATIVE_WINDOW_API_CAMERA: + if (mConnectedApi == api) { + drainQueueAndFreeBuffersLocked(); + mConnectedApi = NO_CONNECTED_API; + mNextCrop.makeInvalid(); + mNextScalingMode = NATIVE_WINDOW_SCALING_MODE_FREEZE; + mNextTransform = 0; + mDequeueCondition.broadcast(); + listener = mConsumerListener; + } else { + ST_LOGE("disconnect: connected to another api (cur=%d, req=%d)", + mConnectedApi, api); + err = -EINVAL; + } + break; + default: + ST_LOGE("disconnect: unknown API %d", api); err = -EINVAL; - } - break; - default: - ST_LOGE("disconnect: unknown API %d", api); - err = -EINVAL; - break; + break; + } } + + if (listener != NULL) { + listener->onBuffersReleased(); + } + return err; } @@ -841,7 +850,7 @@ void BufferQueue::freeAllBuffersLocked() { } } -status_t BufferQueue::acquire(BufferItem *buffer) { +status_t BufferQueue::acquireBuffer(BufferItem *buffer) { ATRACE_CALL(); Mutex::Autolock _l(mMutex); // check if queue is empty @@ -855,8 +864,7 @@ status_t BufferQueue::acquire(BufferItem *buffer) { if (mSlots[buf].mAcquireCalled) { buffer->mGraphicBuffer = NULL; - } - else { + } else { buffer->mGraphicBuffer = mSlots[buf].mGraphicBuffer; } buffer->mCrop = mSlots[buf].mCrop; @@ -872,8 +880,7 @@ status_t BufferQueue::acquire(BufferItem *buffer) { mDequeueCondition.broadcast(); ATRACE_INT(mConsumerName.string(), mQueue.size()); - } - else { + } else { // should be a better return code? return -EINVAL; } @@ -907,17 +914,58 @@ status_t BufferQueue::releaseBuffer(int buf, EGLDisplay display, return OK; } -status_t BufferQueue::consumerDisconnect() { +status_t BufferQueue::consumerConnect(const sp& consumerListener) { + ST_LOGV("consumerConnect"); Mutex::Autolock lock(mMutex); - mAbandoned = true; + if (mAbandoned) { + ST_LOGE("consumerConnect: BufferQueue has been abandoned!"); + return NO_INIT; + } + mConsumerListener = consumerListener; + + return OK; +} + +status_t BufferQueue::consumerDisconnect() { + ST_LOGV("consumerDisconnect"); + Mutex::Autolock lock(mMutex); + + if (mConsumerListener == NULL) { + ST_LOGE("consumerDisconnect: No consumer is connected!"); + return -EINVAL; + } + + mAbandoned = true; + mConsumerListener = NULL; mQueue.clear(); freeAllBuffersLocked(); mDequeueCondition.broadcast(); return OK; } +status_t BufferQueue::getReleasedBuffers(uint32_t* slotMask) { + ST_LOGV("getReleasedBuffers"); + Mutex::Autolock lock(mMutex); + + if (mAbandoned) { + ST_LOGE("getReleasedBuffers: BufferQueue has been abandoned!"); + return NO_INIT; + } + + uint32_t mask = 0; + for (int i = 0; i < NUM_BUFFER_SLOTS; i++) { + if (!mSlots[i].mAcquireCalled) { + mask |= 1 << i; + } + } + *slotMask = mask; + + ST_LOGV("getReleasedBuffers: returning mask %#x", mask); + return NO_ERROR; +} + status_t BufferQueue::setDefaultBufferSize(uint32_t w, uint32_t h) { ST_LOGV("setDefaultBufferSize: w=%d, h=%d", w, h); @@ -982,4 +1030,24 @@ status_t BufferQueue::drainQueueAndFreeBuffersLocked() { return err; } +BufferQueue::ProxyConsumerListener::ProxyConsumerListener( + const wp& consumerListener): + mConsumerListener(consumerListener) {} + +BufferQueue::ProxyConsumerListener::~ProxyConsumerListener() {} + +void BufferQueue::ProxyConsumerListener::onFrameAvailable() { + sp listener(mConsumerListener.promote()); + if (listener != NULL) { + listener->onFrameAvailable(); + } +} + +void BufferQueue::ProxyConsumerListener::onBuffersReleased() { + sp listener(mConsumerListener.promote()); + if (listener != NULL) { + listener->onBuffersReleased(); + } +} + }; // namespace android diff --git a/libs/gui/SurfaceTexture.cpp b/libs/gui/SurfaceTexture.cpp index 5fb933be1..d8dd5bd75 100644 --- a/libs/gui/SurfaceTexture.cpp +++ b/libs/gui/SurfaceTexture.cpp @@ -122,17 +122,32 @@ SurfaceTexture::SurfaceTexture(GLuint tex, bool allowSynchronousMode, mName = String8::format("unnamed-%d-%d", getpid(), createProcessUniqueId()); ST_LOGV("SurfaceTexture"); if (bufferQueue == 0) { - ST_LOGV("Creating a new BufferQueue"); mBufferQueue = new BufferQueue(allowSynchronousMode); } else { mBufferQueue = bufferQueue; } - mBufferQueue->setConsumerName(mName); memcpy(mCurrentTransformMatrix, mtxIdentity, sizeof(mCurrentTransformMatrix)); + + // Note that we can't create an sp<...>(this) in a ctor that will not keep a + // reference once the ctor ends, as that would cause the refcount of 'this' + // dropping to 0 at the end of the ctor. Since all we need is a wp<...> + // that's what we create. + wp listener; + sp proxy; + listener = static_cast(this); + proxy = new BufferQueue::ProxyConsumerListener(listener); + + status_t err = mBufferQueue->consumerConnect(proxy); + if (err != NO_ERROR) { + ST_LOGE("SurfaceTexture: error connecting to BufferQueue: %s (%d)", + strerror(-err), err); + } else { + mBufferQueue->setConsumerName(mName); + } } SurfaceTexture::~SurfaceTexture() { @@ -167,7 +182,7 @@ status_t SurfaceTexture::updateTexImage() { // In asynchronous mode the list is guaranteed to be one buffer // deep, while in synchronous mode we use the oldest buffer. - if (mBufferQueue->acquire(&item) == NO_ERROR) { + if (mBufferQueue->acquireBuffer(&item) == NO_ERROR) { int buf = item.mBuf; // This buffer was newly allocated, so we need to clean up on our side if (item.mGraphicBuffer != NULL) { @@ -394,7 +409,7 @@ void SurfaceTexture::setFrameAvailableListener( const sp& listener) { ST_LOGV("setFrameAvailableListener"); Mutex::Autolock lock(mMutex); - mBufferQueue->setFrameAvailableListener(listener); + mFrameAvailableListener = listener; } EGLImageKHR SurfaceTexture::createImage(EGLDisplay dpy, @@ -438,24 +453,36 @@ bool SurfaceTexture::isSynchronousMode() const { return mBufferQueue->isSynchronousMode(); } -void SurfaceTexture::abandon() { - Mutex::Autolock lock(mMutex); - mAbandoned = true; - mCurrentTextureBuf.clear(); - - // destroy all egl buffers - for (int i =0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) { - mEGLSlots[i].mGraphicBuffer = 0; - if (mEGLSlots[i].mEglImage != EGL_NO_IMAGE_KHR) { - eglDestroyImageKHR(mEGLSlots[i].mEglDisplay, - mEGLSlots[i].mEglImage); - mEGLSlots[i].mEglImage = EGL_NO_IMAGE_KHR; - mEGLSlots[i].mEglDisplay = EGL_NO_DISPLAY; +void SurfaceTexture::freeBufferLocked(int slotIndex) { + ST_LOGV("freeBufferLocked: slotIndex=%d", slotIndex); + mEGLSlots[slotIndex].mGraphicBuffer = 0; + if (mEGLSlots[slotIndex].mEglImage != EGL_NO_IMAGE_KHR) { + EGLImageKHR img = mEGLSlots[slotIndex].mEglImage; + if (img != EGL_NO_IMAGE_KHR) { + eglDestroyImageKHR(mEGLSlots[slotIndex].mEglDisplay, img); } + mEGLSlots[slotIndex].mEglImage = EGL_NO_IMAGE_KHR; + mEGLSlots[slotIndex].mEglDisplay = EGL_NO_DISPLAY; } +} - // disconnect from the BufferQueue - mBufferQueue->consumerDisconnect(); +void SurfaceTexture::abandon() { + ST_LOGV("abandon"); + Mutex::Autolock lock(mMutex); + + if (!mAbandoned) { + mAbandoned = true; + mCurrentTextureBuf.clear(); + + // destroy all egl buffers + for (int i =0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) { + freeBufferLocked(i); + } + + // disconnect from the BufferQueue + mBufferQueue->consumerDisconnect(); + mBufferQueue.clear(); + } } void SurfaceTexture::setName(const String8& name) { @@ -505,6 +532,40 @@ status_t SurfaceTexture::connect(int api, return mBufferQueue->connect(api, outWidth, outHeight, outTransform); } +void SurfaceTexture::onFrameAvailable() { + ST_LOGV("onFrameAvailable"); + + sp listener; + { // scope for the lock + Mutex::Autolock lock(mMutex); + listener = mFrameAvailableListener; + } + + if (listener != NULL) { + ST_LOGV("actually calling onFrameAvailable"); + listener->onFrameAvailable(); + } +} + +void SurfaceTexture::onBuffersReleased() { + ST_LOGV("onBuffersReleased"); + + Mutex::Autolock lock(mMutex); + + if (mAbandoned) { + // Nothing to do if we're already abandoned. + return; + } + + uint32_t mask = 0; + mBufferQueue->getReleasedBuffers(&mask); + for (int i = 0; i < BufferQueue::NUM_BUFFER_SLOTS; i++) { + if (mask & (1 << i)) { + freeBufferLocked(i); + } + } +} + void SurfaceTexture::dump(String8& result) const { char buffer[1024]; @@ -515,19 +576,21 @@ void SurfaceTexture::dump(String8& result, const char* prefix, char* buffer, size_t SIZE) const { Mutex::Autolock _l(mMutex); - snprintf(buffer, SIZE, "%smTexName=%d\n", prefix, mTexName); + snprintf(buffer, SIZE, "%smTexName=%d, mAbandoned=%d\n", prefix, mTexName, + int(mAbandoned)); result.append(buffer); snprintf(buffer, SIZE, - "%snext : {crop=[%d,%d,%d,%d], transform=0x%02x, current=%d}\n" - ,prefix, mCurrentCrop.left, + "%snext : {crop=[%d,%d,%d,%d], transform=0x%02x, current=%d}\n", + prefix, mCurrentCrop.left, mCurrentCrop.top, mCurrentCrop.right, mCurrentCrop.bottom, mCurrentTransform, mCurrentTexture ); result.append(buffer); - - mBufferQueue->dump(result, prefix, buffer, SIZE); + if (!mAbandoned) { + mBufferQueue->dump(result, prefix, buffer, SIZE); + } } static void mtxMul(float out[16], const float a[16], const float b[16]) { diff --git a/libs/gui/tests/SurfaceTextureClient_test.cpp b/libs/gui/tests/SurfaceTextureClient_test.cpp index c1a3c98d6..aa1f94e65 100644 --- a/libs/gui/tests/SurfaceTextureClient_test.cpp +++ b/libs/gui/tests/SurfaceTextureClient_test.cpp @@ -14,10 +14,14 @@ * limitations under the License. */ +#define LOG_TAG "SurfaceTextureClient_test" +//#define LOG_NDEBUG 0 + #include #include #include -#include +#include +#include namespace android { @@ -30,6 +34,11 @@ protected: } virtual void SetUp() { + const ::testing::TestInfo* const testInfo = + ::testing::UnitTest::GetInstance()->current_test_info(); + ALOGV("Begin test: %s.%s", testInfo->test_case_name(), + testInfo->name()); + mST = new SurfaceTexture(123); mSTC = new SurfaceTextureClient(mST); mANW = mSTC; @@ -76,6 +85,11 @@ protected: eglDestroyContext(mEglDisplay, mEglContext); eglDestroySurface(mEglDisplay, mEglSurface); eglTerminate(mEglDisplay); + + const ::testing::TestInfo* const testInfo = + ::testing::UnitTest::GetInstance()->current_test_info(); + ALOGV("End test: %s.%s", testInfo->test_case_name(), + testInfo->name()); } virtual EGLint const* getConfigAttribs() { @@ -147,6 +161,10 @@ TEST_F(SurfaceTextureClientTest, EglCreateWindowSurfaceSucceeds) { EXPECT_NE(EGL_NO_SURFACE, eglSurface); EXPECT_EQ(EGL_SUCCESS, eglGetError()); + if (eglSurface != EGL_NO_SURFACE) { + eglDestroySurface(dpy, eglSurface); + } + eglTerminate(dpy); } diff --git a/libs/gui/tests/SurfaceTexture_test.cpp b/libs/gui/tests/SurfaceTexture_test.cpp index 8c6defec0..71cf57712 100644 --- a/libs/gui/tests/SurfaceTexture_test.cpp +++ b/libs/gui/tests/SurfaceTexture_test.cpp @@ -47,6 +47,11 @@ protected: } virtual void SetUp() { + const ::testing::TestInfo* const testInfo = + ::testing::UnitTest::GetInstance()->current_test_info(); + ALOGV("Begin test: %s.%s", testInfo->test_case_name(), + testInfo->name()); + mEglDisplay = eglGetDisplay(EGL_DEFAULT_DISPLAY); ASSERT_EQ(EGL_SUCCESS, eglGetError()); ASSERT_NE(EGL_NO_DISPLAY, mEglDisplay); @@ -148,6 +153,11 @@ protected: eglTerminate(mEglDisplay); } ASSERT_EQ(EGL_SUCCESS, eglGetError()); + + const ::testing::TestInfo* const testInfo = + ::testing::UnitTest::GetInstance()->current_test_info(); + ALOGV("End test: %s.%s", testInfo->test_case_name(), + testInfo->name()); } virtual EGLint const* getConfigAttribs() { @@ -1188,7 +1198,10 @@ TEST_F(SurfaceTextureGLToGLTest, TexturingFromGLFilledRGBABufferPow2) { } TEST_F(SurfaceTextureGLToGLTest, EglDestroySurfaceUnrefsBuffers) { - sp buffers[3]; + sp buffers[2]; + + sp fw(new FrameWaiter); + mST->setFrameAvailableListener(fw); // This test requires async mode to run on a single thread. EXPECT_TRUE(eglMakeCurrent(mEglDisplay, mProducerEglSurface, @@ -1197,7 +1210,7 @@ TEST_F(SurfaceTextureGLToGLTest, EglDestroySurfaceUnrefsBuffers) { EXPECT_TRUE(eglSwapInterval(mEglDisplay, 0)); ASSERT_EQ(EGL_SUCCESS, eglGetError()); - for (int i = 0; i < 3; i++) { + for (int i = 0; i < 2; i++) { // Produce a frame EXPECT_TRUE(eglMakeCurrent(mEglDisplay, mProducerEglSurface, mProducerEglSurface, mProducerEglContext)); @@ -1209,6 +1222,7 @@ TEST_F(SurfaceTextureGLToGLTest, EglDestroySurfaceUnrefsBuffers) { EXPECT_TRUE(eglMakeCurrent(mEglDisplay, mEglSurface, mEglSurface, mEglContext)); ASSERT_EQ(EGL_SUCCESS, eglGetError()); + fw->waitForFrame(); mST->updateTexImage(); buffers[i] = mST->getCurrentBuffer(); } @@ -1220,24 +1234,23 @@ TEST_F(SurfaceTextureGLToGLTest, EglDestroySurfaceUnrefsBuffers) { // Destroy the EGLSurface EXPECT_TRUE(eglDestroySurface(mEglDisplay, mProducerEglSurface)); ASSERT_EQ(EGL_SUCCESS, eglGetError()); + mProducerEglSurface = EGL_NO_SURFACE; - // Release the ref that the SurfaceTexture has on buffers[2]. - mST->abandon(); - + // This test should have the only reference to buffer 0. EXPECT_EQ(1, buffers[0]->getStrongCount()); - EXPECT_EQ(1, buffers[1]->getStrongCount()); - // Depending on how lazily the GL driver dequeues buffers, we may end up - // with either two or three total buffers. If there are three, make sure - // the last one was properly down-ref'd. - if (buffers[2] != buffers[0]) { - EXPECT_EQ(1, buffers[2]->getStrongCount()); - } + // The SurfaceTexture should hold a single reference to buffer 1 in its + // mCurrentBuffer member. All of the references in the slots should have + // been released. + EXPECT_EQ(2, buffers[1]->getStrongCount()); } TEST_F(SurfaceTextureGLToGLTest, EglDestroySurfaceAfterAbandonUnrefsBuffers) { sp buffers[3]; + sp fw(new FrameWaiter); + mST->setFrameAvailableListener(fw); + // This test requires async mode to run on a single thread. EXPECT_TRUE(eglMakeCurrent(mEglDisplay, mProducerEglSurface, mProducerEglSurface, mProducerEglContext)); @@ -1258,6 +1271,7 @@ TEST_F(SurfaceTextureGLToGLTest, EglDestroySurfaceAfterAbandonUnrefsBuffers) { EXPECT_TRUE(eglMakeCurrent(mEglDisplay, mEglSurface, mEglSurface, mEglContext)); ASSERT_EQ(EGL_SUCCESS, eglGetError()); + fw->waitForFrame(); ASSERT_EQ(NO_ERROR, mST->updateTexImage()); buffers[i] = mST->getCurrentBuffer(); } @@ -1273,6 +1287,7 @@ TEST_F(SurfaceTextureGLToGLTest, EglDestroySurfaceAfterAbandonUnrefsBuffers) { // Destroy the EGLSurface. EXPECT_TRUE(eglDestroySurface(mEglDisplay, mProducerEglSurface)); ASSERT_EQ(EGL_SUCCESS, eglGetError()); + mProducerEglSurface = EGL_NO_SURFACE; EXPECT_EQ(1, buffers[0]->getStrongCount()); EXPECT_EQ(1, buffers[1]->getStrongCount());