Merge "Support for writing output data to a provided callback function instead of a file"
diff --git a/include/gui/SurfaceTexture.h b/include/gui/SurfaceTexture.h
index e558dfd..c82fb9b 100644
--- a/include/gui/SurfaceTexture.h
+++ b/include/gui/SurfaceTexture.h
@@ -46,11 +46,14 @@
     enum { NUM_BUFFER_SLOTS = 32 };
 
     struct FrameAvailableListener : public virtual RefBase {
-        // onFrameAvailable() is called from queueBuffer() is the FIFO is
-        // empty. You can use SurfaceTexture::getQueuedCount() to
-        // figure out if there are more frames waiting.
-        // This is called without any lock held can be called concurrently by
-        // multiple threads.
+        // 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;
     };
 
@@ -101,11 +104,6 @@
     // target texture belongs is bound to the calling thread.
     status_t updateTexImage();
 
-    // getqueuedCount returns the number of queued frames waiting in the
-    // FIFO. In asynchronous mode, this always returns 0 or 1 since
-    // frames are not accumulating in the FIFO.
-    size_t getQueuedCount() const;
-
     // setBufferCountServer set the buffer count. If the client has requested
     // a buffer count using setBufferCount, the server-buffer count will
     // take effect once the client sets the count back to zero.
diff --git a/libs/gui/SurfaceTexture.cpp b/libs/gui/SurfaceTexture.cpp
index 37e6d11..0925001 100644
--- a/libs/gui/SurfaceTexture.cpp
+++ b/libs/gui/SurfaceTexture.cpp
@@ -417,17 +417,22 @@
             return -EINVAL;
         }
 
-        if (mQueue.empty()) {
-            listener = mFrameAvailableListener;
-        }
-
         if (mSynchronousMode) {
-            // in synchronous mode we queue all buffers in a FIFO
+            // In synchronous mode we queue all buffers in a FIFO.
             mQueue.push_back(buf);
+
+            // Synchronous mode always signals that an additional frame should
+            // be consumed.
+            listener = mFrameAvailableListener;
         } else {
-            // in asynchronous mode we only keep the most recent buffer
+            // In asynchronous mode we only keep the most recent buffer.
             if (mQueue.empty()) {
                 mQueue.push_back(buf);
+
+                // 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;
             } else {
                 Fifo::iterator front(mQueue.begin());
                 // buffer currently queued is freed
@@ -483,24 +488,14 @@
 
 status_t SurfaceTexture::updateTexImage() {
     LOGV("SurfaceTexture::updateTexImage");
-
     Mutex::Autolock lock(mMutex);
 
-    int buf = mCurrentTexture;
+    // In asynchronous mode the list is guaranteed to be one buffer
+    // deep, while in synchronous mode we use the oldest buffer.
     if (!mQueue.empty()) {
-        // in asynchronous mode the list is guaranteed to be one buffer deep,
-        // while in synchronous mode we use the oldest buffer
         Fifo::iterator front(mQueue.begin());
-        buf = *front;
-        mQueue.erase(front);
-        if (mQueue.isEmpty()) {
-            mDequeueCondition.signal();
-        }
-    }
+        int buf = *front;
 
-    // Initially both mCurrentTexture and buf are INVALID_BUFFER_SLOT,
-    // so this check will fail until a buffer gets queued.
-    if (mCurrentTexture != buf) {
         // Update the GL texture object.
         EGLImageKHR image = mSlots[buf].mEglImage;
         if (image == EGL_NO_IMAGE_KHR) {
@@ -538,7 +533,7 @@
         }
 
         if (mCurrentTexture != INVALID_BUFFER_SLOT) {
-            // the current buffer becomes FREE if it was still in the queued
+            // 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[mCurrentTexture].mBufferState == BufferSlot::QUEUED)
@@ -553,17 +548,17 @@
         mCurrentTransform = mSlots[buf].mTransform;
         mCurrentTimestamp = mSlots[buf].mTimestamp;
         computeCurrentTransformMatrix();
+
+        // Now that we've passed the point at which failures can happen,
+        // it's safe to remove the buffer from the front of the queue.
+        mQueue.erase(front);
         mDequeueCondition.signal();
     } else {
         // We always bind the texture even if we don't update its contents.
         glBindTexture(mCurrentTextureTarget, mTexName);
     }
-    return OK;
-}
 
-size_t SurfaceTexture::getQueuedCount() const {
-    Mutex::Autolock lock(mMutex);
-    return mQueue.size();
+    return OK;
 }
 
 bool SurfaceTexture::isExternalFormat(uint32_t format)
diff --git a/libs/gui/tests/SurfaceTexture_test.cpp b/libs/gui/tests/SurfaceTexture_test.cpp
index f219639..dfa9211 100644
--- a/libs/gui/tests/SurfaceTexture_test.cpp
+++ b/libs/gui/tests/SurfaceTexture_test.cpp
@@ -419,6 +419,31 @@
         ASSERT_EQ(GLenum(GL_NO_ERROR), glGetError());
     }
 
+    class FrameWaiter : public SurfaceTexture::FrameAvailableListener {
+    public:
+        FrameWaiter():
+                mPendingFrames(0) {
+        }
+
+        void waitForFrame() {
+            Mutex::Autolock lock(mMutex);
+            while (mPendingFrames == 0) {
+                mCondition.wait(mMutex);
+            }
+            mPendingFrames--;
+        }
+
+        virtual void onFrameAvailable() {
+            Mutex::Autolock lock(mMutex);
+            mPendingFrames++;
+            mCondition.signal();
+        }
+
+        int mPendingFrames;
+        Mutex mMutex;
+        Condition mCondition;
+    };
+
     sp<SurfaceTexture> mST;
     sp<SurfaceTextureClient> mSTC;
     sp<ANativeWindow> mANW;
@@ -648,6 +673,157 @@
     }
 }
 
+// This test is intended to catch synchronization bugs between the CPU-written
+// and GPU-read buffers.
+TEST_F(SurfaceTextureGLTest, TexturingFromCpuFilledYV12BuffersRepeatedly) {
+    enum { texWidth = 16 };
+    enum { texHeight = 16 };
+    enum { numFrames = 1024 };
+
+    ASSERT_EQ(NO_ERROR, mST->setSynchronousMode(true));
+    ASSERT_EQ(NO_ERROR, mST->setBufferCountServer(2));
+    ASSERT_EQ(NO_ERROR, native_window_set_buffers_geometry(mANW.get(),
+            texWidth, texHeight, HAL_PIXEL_FORMAT_YV12));
+    ASSERT_EQ(NO_ERROR, native_window_set_usage(mANW.get(),
+            GRALLOC_USAGE_SW_WRITE_OFTEN));
+
+    struct TestPixel {
+        int x;
+        int y;
+    };
+    const TestPixel testPixels[] = {
+        {  4, 11 },
+        { 12, 14 },
+        {  7,  2 },
+    };
+    enum {numTestPixels = sizeof(testPixels) / sizeof(testPixels[0])};
+
+    class ProducerThread : public Thread {
+    public:
+        ProducerThread(const sp<ANativeWindow>& anw, const TestPixel* testPixels):
+                mANW(anw),
+                mTestPixels(testPixels) {
+        }
+
+        virtual ~ProducerThread() {
+        }
+
+        virtual bool threadLoop() {
+            for (int i = 0; i < numFrames; i++) {
+                ANativeWindowBuffer* anb;
+                if (mANW->dequeueBuffer(mANW.get(), &anb) != NO_ERROR) {
+                    return false;
+                }
+                if (anb == NULL) {
+                    return false;
+                }
+
+                sp<GraphicBuffer> buf(new GraphicBuffer(anb, false));
+                if (mANW->lockBuffer(mANW.get(), buf->getNativeBuffer())
+                        != NO_ERROR) {
+                    return false;
+                }
+
+                const int yuvTexOffsetY = 0;
+                int stride = buf->getStride();
+                int yuvTexStrideY = stride;
+                int yuvTexOffsetV = yuvTexStrideY * texHeight;
+                int yuvTexStrideV = (yuvTexStrideY/2 + 0xf) & ~0xf;
+                int yuvTexOffsetU = yuvTexOffsetV + yuvTexStrideV * texHeight/2;
+                int yuvTexStrideU = yuvTexStrideV;
+
+                uint8_t* img = NULL;
+                buf->lock(GRALLOC_USAGE_SW_WRITE_OFTEN, (void**)(&img));
+
+                // Gray out all the test pixels first, so we're more likely to
+                // see a failure if GL is still texturing from the buffer we
+                // just dequeued.
+                for (int j = 0; j < numTestPixels; j++) {
+                    int x = mTestPixels[j].x;
+                    int y = mTestPixels[j].y;
+                    uint8_t value = 128;
+                    img[y*stride + x] = value;
+                }
+
+                // Fill the buffer with gray.
+                for (int y = 0; y < texHeight; y++) {
+                    for (int x = 0; x < texWidth; x++) {
+                        img[yuvTexOffsetY + y*yuvTexStrideY + x] = 128;
+                        img[yuvTexOffsetU + (y/2)*yuvTexStrideU + x/2] = 128;
+                        img[yuvTexOffsetV + (y/2)*yuvTexStrideV + x/2] = 128;
+                    }
+                }
+
+                // Set the test pixels to either white or black.
+                for (int j = 0; j < numTestPixels; j++) {
+                    int x = mTestPixels[j].x;
+                    int y = mTestPixels[j].y;
+                    uint8_t value = 0;
+                    if (j == (i % numTestPixels)) {
+                        value = 255;
+                    }
+                    img[y*stride + x] = value;
+                }
+
+                buf->unlock();
+                if (mANW->queueBuffer(mANW.get(), buf->getNativeBuffer())
+                        != NO_ERROR) {
+                    return false;
+                }
+            }
+            return false;
+        }
+
+        sp<ANativeWindow> mANW;
+        const TestPixel* mTestPixels;
+    };
+
+    sp<FrameWaiter> fw(new FrameWaiter);
+    mST->setFrameAvailableListener(fw);
+
+    sp<Thread> pt(new ProducerThread(mANW, testPixels));
+    pt->run();
+
+    glViewport(0, 0, texWidth, texHeight);
+
+    glClearColor(0.2, 0.2, 0.2, 0.2);
+    glClear(GL_COLOR_BUFFER_BIT);
+
+    // We wait for the first two frames up front so that the producer will be
+    // likely to dequeue the buffer that's currently being textured from.
+    fw->waitForFrame();
+    fw->waitForFrame();
+
+    for (int i = 0; i < numFrames; i++) {
+        SCOPED_TRACE(String8::format("frame %d", i).string());
+
+        // We must wait for each frame to come in because if we ever do an
+        // updateTexImage call that doesn't consume a newly available buffer
+        // then the producer and consumer will get out of sync, which will cause
+        // a deadlock.
+        if (i > 1) {
+            fw->waitForFrame();
+        }
+        mST->updateTexImage();
+        drawTexture();
+
+        for (int j = 0; j < numTestPixels; j++) {
+            int x = testPixels[j].x;
+            int y = testPixels[j].y;
+            uint8_t value = 0;
+            if (j == (i % numTestPixels)) {
+                // We must y-invert the texture coords
+                EXPECT_TRUE(checkPixel(x, texHeight-y-1, 255, 255, 255, 255));
+            } else {
+                // We must y-invert the texture coords
+                EXPECT_TRUE(checkPixel(x, texHeight-y-1, 0, 0, 0, 255));
+            }
+        }
+    }
+
+    pt->requestExitAndWait();
+}
+
 // XXX: This test is disabled because there are currently no drivers that can
 // handle RGBA textures with the GL_TEXTURE_EXTERNAL_OES target.
 TEST_F(SurfaceTextureGLTest, DISABLED_TexturingFromCpuFilledRGBABufferNpot) {
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 2bab6a8..35e29a6 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -101,9 +101,8 @@
 }
 
 void Layer::onFrameQueued() {
-    if (android_atomic_or(1, &mQueuedFrames) == 0) {
-        mFlinger->signalEvent();
-    }
+    android_atomic_inc(&mQueuedFrames);
+    mFlinger->signalEvent();
 }
 
 // called with SurfaceFlinger::mStateLock as soon as the layer is entered
@@ -406,20 +405,18 @@
 
 void Layer::lockPageFlip(bool& recomputeVisibleRegions)
 {
-    if (android_atomic_and(0, &mQueuedFrames)) {
+    if (mQueuedFrames > 0) {
+        // signal another event if we have more frames pending
+        if (android_atomic_dec(&mQueuedFrames) > 1) {
+            mFlinger->signalEvent();
+        }
+
         if (mSurfaceTexture->updateTexImage() < NO_ERROR) {
             // something happened!
             recomputeVisibleRegions = true;
             return;
         }
 
-        // signal another event if we have more frames waiting
-        if (mSurfaceTexture->getQueuedCount()) {
-            if (android_atomic_or(1, &mQueuedFrames) == 0) {
-                mFlinger->signalEvent();
-            }
-        }
-
         mActiveBuffer = mSurfaceTexture->getCurrentBuffer();
         mSurfaceTexture->getTransformMatrix(mTextureMatrix);