Add a BufferItem parameter to onFrameAvailable

Passes the BufferItem for the queued buffer to the onFrameAvailable
callback so the consumer can track the BufferQueue's contents. Also
adds an onFrameReplaced callback, which is necessary if the consumer
wants to do anything more than simple queue length tracking.

Bug: 18111837
Change-Id: If9d07229c9b586c668e5f99074e9b63b0468feb0

23 files changed, 99 insertions, 38 deletions

diff --git a/include/gui/BufferItem.h b/include/gui/BufferItem.h
index 5effd10..01b6ff4 100644
--- a/include/gui/BufferItem.h
+++ b/include/gui/BufferItem.h
@@ -44,6 +44,7 @@ class BufferItem : public Flattenable<BufferItem> {
     // The default value of mBuf, used to indicate this doesn't correspond to a slot.
     enum { INVALID_BUFFER_SLOT = -1 };
     BufferItem();
+    ~BufferItem();
     operator IGraphicBufferConsumer::BufferItem() const;
 
     static const char* scalingModeName(uint32_t scalingMode);
diff --git a/include/gui/BufferQueue.h b/include/gui/BufferQueue.h
index 3297b10..1188837 100644
--- a/include/gui/BufferQueue.h
+++ b/include/gui/BufferQueue.h
@@ -62,7 +62,7 @@ public:
     public:
         ProxyConsumerListener(const wp<ConsumerListener>& consumerListener);
         virtual ~ProxyConsumerListener();
-        virtual void onFrameAvailable();
+        virtual void onFrameAvailable(const android::BufferItem& item);
         virtual void onBuffersReleased();
         virtual void onSidebandStreamChanged();
     private:
diff --git a/include/gui/BufferQueueProducer.h b/include/gui/BufferQueueProducer.h
index c619a11..ed1056a 100644
--- a/include/gui/BufferQueueProducer.h
+++ b/include/gui/BufferQueueProducer.h
@@ -203,6 +203,16 @@ private:
     // since the previous buffer might have already been acquired.
     sp<Fence> mLastQueueBufferFence;
 
+    // Take-a-ticket system for ensuring that onFrame* callbacks are called in
+    // the order that frames are queued. While the BufferQueue lock
+    // (mCore->mMutex) is held, a ticket is retained by the producer. After
+    // dropping the BufferQueue lock, the producer must wait on the condition
+    // variable until the current callback ticket matches its retained ticket.
+    Mutex mCallbackMutex;
+    int mNextCallbackTicket; // Protected by mCore->mMutex
+    int mCurrentCallbackTicket; // Protected by mCallbackMutex
+    Condition mCallbackCondition;
+
 }; // class BufferQueueProducer
 
 } // namespace android
diff --git a/include/gui/ConsumerBase.h b/include/gui/ConsumerBase.h
index 100bb26..f7ab5ac 100644
--- a/include/gui/ConsumerBase.h
+++ b/include/gui/ConsumerBase.h
@@ -46,7 +46,7 @@ public:
         //
         // This is called without any lock held and can be called concurrently
         // by multiple threads.
-        virtual void onFrameAvailable() = 0;
+        virtual void onFrameAvailable(const BufferItem& item) = 0;
     };
 
     virtual ~ConsumerBase();
@@ -106,7 +106,7 @@ protected:
     // the ConsumerBase implementation must be called from the derived class.
     // The ConsumerBase version of onSidebandStreamChanged does nothing and can
     // be overriden by derived classes if they want the notification.
-    virtual void onFrameAvailable();
+    virtual void onFrameAvailable(const BufferItem& item);
     virtual void onBuffersReleased();
     virtual void onSidebandStreamChanged();
 
diff --git a/include/gui/IConsumerListener.h b/include/gui/IConsumerListener.h
index 260099e..2ef7c4d 100644
--- a/include/gui/IConsumerListener.h
+++ b/include/gui/IConsumerListener.h
@@ -28,6 +28,8 @@
 namespace android {
 // ----------------------------------------------------------------------------
 
+class BufferItem;
+
 // 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
@@ -43,11 +45,24 @@ public:
     // 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.
+    // always trigger the callback. The item passed to the callback will contain
+    // all of the information about the queued frame except for its
+    // GraphicBuffer pointer, which will always be null.
+    //
+    // This is called without any lock held and can be called concurrently
+    // by multiple threads.
+    virtual void onFrameAvailable(const BufferItem& item) = 0; /* Asynchronous */
+
+    // onFrameReplaced is called from queueBuffer if the frame being queued is
+    // replacing an existing slot in the queue. Any call to queueBuffer that
+    // doesn't call onFrameAvailable will call this callback instead. The item
+    // passed to the callback will contain all of the information about the
+    // queued frame except for its GraphicBuffer pointer, which will always be
+    // null.
     //
     // This is called without any lock held and can be called concurrently
     // by multiple threads.
-    virtual void onFrameAvailable() = 0; /* Asynchronous */
+    virtual void onFrameReplaced(const BufferItem& item) {} /* Asynchronous */
 
     // onBuffersReleased is called to notify the buffer consumer that the
     // BufferQueue has released its references to one or more GraphicBuffers
diff --git a/include/gui/StreamSplitter.h b/include/gui/StreamSplitter.h
index f927953..8f47eb4 100644
--- a/include/gui/StreamSplitter.h
+++ b/include/gui/StreamSplitter.h
@@ -74,7 +74,7 @@ private:
     // can block if there are too many outstanding buffers. If it blocks, it
     // will resume when onBufferReleasedByOutput releases a buffer back to the
     // input.
-    virtual void onFrameAvailable();
+    virtual void onFrameAvailable(const BufferItem& item);
 
     // From IConsumerListener
     // We don't care about released buffers because we detach each buffer as
diff --git a/libs/gui/BufferItem.cpp b/libs/gui/BufferItem.cpp
index d3fa43e..e6fc791 100644
--- a/libs/gui/BufferItem.cpp
+++ b/libs/gui/BufferItem.cpp
@@ -36,6 +36,8 @@ BufferItem::BufferItem() :
     mCrop.makeInvalid();
 }
 
+BufferItem::~BufferItem() {}
+
 BufferItem::operator IGraphicBufferConsumer::BufferItem() const {
     IGraphicBufferConsumer::BufferItem bufferItem;
     bufferItem.mGraphicBuffer = mGraphicBuffer;
diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp
index c49a886..61fd8c4 100644
--- a/libs/gui/BufferQueue.cpp
+++ b/libs/gui/BufferQueue.cpp
@@ -31,10 +31,11 @@ BufferQueue::ProxyConsumerListener::ProxyConsumerListener(
 
 BufferQueue::ProxyConsumerListener::~ProxyConsumerListener() {}
 
-void BufferQueue::ProxyConsumerListener::onFrameAvailable() {
+void BufferQueue::ProxyConsumerListener::onFrameAvailable(
+        const android::BufferItem& item) {
     sp<ConsumerListener> listener(mConsumerListener.promote());
     if (listener != NULL) {
-        listener->onFrameAvailable();
+        listener->onFrameAvailable(item);
     }
 }
 
diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp
index 284ddb2..4d3b008 100644
--- a/libs/gui/BufferQueueProducer.cpp
+++ b/libs/gui/BufferQueueProducer.cpp
@@ -39,7 +39,11 @@ BufferQueueProducer::BufferQueueProducer(const sp<BufferQueueCore>& core) :
     mSlots(core->mSlots),
     mConsumerName(),
     mStickyTransform(0),
-    mLastQueueBufferFence(Fence::NO_FENCE) {}
+    mLastQueueBufferFence(Fence::NO_FENCE),
+    mCallbackMutex(),
+    mNextCallbackTicket(0),
+    mCurrentCallbackTicket(0),
+    mCallbackCondition() {}
 
 BufferQueueProducer::~BufferQueueProducer() {}
 
@@ -542,7 +546,10 @@ status_t BufferQueueProducer::queueBuffer(int slot,
             return BAD_VALUE;
     }
 
-    sp<IConsumerListener> listener;
+    sp<IConsumerListener> frameAvailableListener;
+    sp<IConsumerListener> frameReplacedListener;
+    int callbackTicket = 0;
+    BufferItem item;
     { // Autolock scope
         Mutex::Autolock lock(mCore->mMutex);
 
@@ -598,7 +605,6 @@ status_t BufferQueueProducer::queueBuffer(int slot,
         ++mCore->mFrameCounter;
         mSlots[slot].mFrameNumber = mCore->mFrameCounter;
 
-        BufferItem item;
         item.mAcquireCalled = mSlots[slot].mAcquireCalled;
         item.mGraphicBuffer = mSlots[slot].mGraphicBuffer;
         item.mCrop = crop;
@@ -619,7 +625,7 @@ status_t BufferQueueProducer::queueBuffer(int slot,
             // When the queue is empty, we can ignore mDequeueBufferCannotBlock
             // and simply queue this buffer
             mCore->mQueue.push_back(item);
-            listener = mCore->mConsumerListener;
+            frameAvailableListener = mCore->mConsumerListener;
         } else {
             // When the queue is not empty, we need to look at the front buffer
             // state to see if we need to replace it
@@ -635,9 +641,10 @@ status_t BufferQueueProducer::queueBuffer(int slot,
                 }
                 // Overwrite the droppable buffer with the incoming one
                 *front = item;
+                frameReplacedListener = mCore->mConsumerListener;
             } else {
                 mCore->mQueue.push_back(item);
-                listener = mCore->mConsumerListener;
+                frameAvailableListener = mCore->mConsumerListener;
             }
         }
 
@@ -648,6 +655,9 @@ status_t BufferQueueProducer::queueBuffer(int slot,
                 mCore->mTransformHint, mCore->mQueue.size());
 
         ATRACE_INT(mCore->mConsumerName.string(), mCore->mQueue.size());
+
+        // Take a ticket for the callback functions
+        callbackTicket = mNextCallbackTicket++;
     } // Autolock scope
 
     // Wait without lock held
@@ -659,9 +669,27 @@ status_t BufferQueueProducer::queueBuffer(int slot,
         mLastQueueBufferFence = fence;
     }
 
-    // Call back without lock held
-    if (listener != NULL) {
-        listener->onFrameAvailable();
+    // Don't send the GraphicBuffer through the callback, and don't send
+    // the slot number, since the consumer shouldn't need it
+    item.mGraphicBuffer.clear();
+    item.mSlot = BufferItem::INVALID_BUFFER_SLOT;
+
+    // Call back without the main BufferQueue lock held, but with the callback
+    // lock held so we can ensure that callbacks occur in order
+    {
+        Mutex::Autolock lock(mCallbackMutex);
+        while (callbackTicket != mCurrentCallbackTicket) {
+            mCallbackCondition.wait(mCallbackMutex);
+        }
+
+        if (frameAvailableListener != NULL) {
+            frameAvailableListener->onFrameAvailable(item);
+        } else if (frameReplacedListener != NULL) {
+            frameReplacedListener->onFrameReplaced(item);
+        }
+
+        ++mCurrentCallbackTicket;
+        mCallbackCondition.broadcast();
     }
 
     return NO_ERROR;
diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp
index f19b6c7..210e98e 100644
--- a/libs/gui/ConsumerBase.cpp
+++ b/libs/gui/ConsumerBase.cpp
@@ -98,7 +98,7 @@ void ConsumerBase::freeBufferLocked(int slotIndex) {
     mSlots[slotIndex].mFrameNumber = 0;
 }
 
-void ConsumerBase::onFrameAvailable() {
+void ConsumerBase::onFrameAvailable(const BufferItem& item) {
     CB_LOGV("onFrameAvailable");
 
     sp<FrameAvailableListener> listener;
@@ -109,7 +109,7 @@ void ConsumerBase::onFrameAvailable() {
 
     if (listener != NULL) {
         CB_LOGV("actually calling onFrameAvailable");
-        listener->onFrameAvailable();
+        listener->onFrameAvailable(item);
     }
 }
 
diff --git a/libs/gui/IConsumerListener.cpp b/libs/gui/IConsumerListener.cpp
index 4ccf0ac..409dfe4 100644
--- a/libs/gui/IConsumerListener.cpp
+++ b/libs/gui/IConsumerListener.cpp
@@ -21,6 +21,7 @@
 #include <binder/Parcel.h>
 
 #include <gui/IConsumerListener.h>
+#include <gui/BufferItem.h>
 
 // ---------------------------------------------------------------------------
 namespace android {
@@ -39,9 +40,10 @@ public:
         : BpInterface<IConsumerListener>(impl) {
     }
 
-    virtual void onFrameAvailable() {
+    virtual void onFrameAvailable(const BufferItem& item) {
         Parcel data, reply;
         data.writeInterfaceToken(IConsumerListener::getInterfaceDescriptor());
+        data.write(item);
         remote()->transact(ON_FRAME_AVAILABLE, data, &reply, IBinder::FLAG_ONEWAY);
     }
 
@@ -66,18 +68,20 @@ status_t BnConsumerListener::onTransact(
     uint32_t code, const Parcel& data, Parcel* reply, uint32_t flags)
 {
     switch(code) {
-        case ON_FRAME_AVAILABLE:
+        case ON_FRAME_AVAILABLE: {
             CHECK_INTERFACE(IConsumerListener, data, reply);
-            onFrameAvailable();
-            return NO_ERROR;
-        case ON_BUFFER_RELEASED:
+            BufferItem item;
+            data.read(item);
+            onFrameAvailable(item);
+            return NO_ERROR; }
+        case ON_BUFFER_RELEASED: {
             CHECK_INTERFACE(IConsumerListener, data, reply);
             onBuffersReleased();
-            return NO_ERROR;
-        case ON_SIDEBAND_STREAM_CHANGED:
+            return NO_ERROR; }
+        case ON_SIDEBAND_STREAM_CHANGED: {
             CHECK_INTERFACE(IConsumerListener, data, reply);
             onSidebandStreamChanged();
-            return NO_ERROR;
+            return NO_ERROR; }
     }
     return BBinder::onTransact(code, data, reply, flags);
 }
diff --git a/libs/gui/StreamSplitter.cpp b/libs/gui/StreamSplitter.cpp
index 771b263..5f39905 100644
--- a/libs/gui/StreamSplitter.cpp
+++ b/libs/gui/StreamSplitter.cpp
@@ -98,7 +98,7 @@ void StreamSplitter::setName(const String8 &name) {
     mInput->setConsumerName(name);
 }
 
-void StreamSplitter::onFrameAvailable() {
+void StreamSplitter::onFrameAvailable(const BufferItem& /* item */) {
     ATRACE_CALL();
     Mutex::Autolock lock(mMutex);
 
diff --git a/libs/gui/tests/BufferQueue_test.cpp b/libs/gui/tests/BufferQueue_test.cpp
index c781366..96de11f 100644
--- a/libs/gui/tests/BufferQueue_test.cpp
+++ b/libs/gui/tests/BufferQueue_test.cpp
@@ -67,7 +67,7 @@ protected:
 };
 
 struct DummyConsumer : public BnConsumerListener {
-    virtual void onFrameAvailable() {}
+    virtual void onFrameAvailable(const BufferItem& /* item */) {}
     virtual void onBuffersReleased() {}
     virtual void onSidebandStreamChanged() {}
 };
diff --git a/libs/gui/tests/DisconnectWaiter.h b/libs/gui/tests/DisconnectWaiter.h
index 56e96c2..6e6915b 100644
--- a/libs/gui/tests/DisconnectWaiter.h
+++ b/libs/gui/tests/DisconnectWaiter.h
@@ -44,7 +44,7 @@ public:
         mPendingFrames--;
     }
 
-    virtual void onFrameAvailable() {
+    virtual void onFrameAvailable(const BufferItem& /* item */) {
         Mutex::Autolock lock(mMutex);
         mPendingFrames++;
         mFrameCondition.signal();
diff --git a/libs/gui/tests/FrameWaiter.h b/libs/gui/tests/FrameWaiter.h
index bdedba6..f78fa00 100644
--- a/libs/gui/tests/FrameWaiter.h
+++ b/libs/gui/tests/FrameWaiter.h
@@ -35,7 +35,7 @@ public:
         mPendingFrames--;
     }
 
-    virtual void onFrameAvailable() {
+    virtual void onFrameAvailable(const BufferItem& /* item */) {
         Mutex::Autolock lock(mMutex);
         mPendingFrames++;
         mCondition.signal();
diff --git a/libs/gui/tests/IGraphicBufferProducer_test.cpp b/libs/gui/tests/IGraphicBufferProducer_test.cpp
index aadfe61..8d5fd8f 100644
--- a/libs/gui/tests/IGraphicBufferProducer_test.cpp
+++ b/libs/gui/tests/IGraphicBufferProducer_test.cpp
@@ -65,7 +65,7 @@ const sp<Fence> QUEUE_BUFFER_INPUT_FENCE = Fence::NO_FENCE;
 }; // namespace anonymous
 
 struct DummyConsumer : public BnConsumerListener {
-    virtual void onFrameAvailable() {}
+    virtual void onFrameAvailable(const BufferItem& /* item */) {}
     virtual void onBuffersReleased() {}
     virtual void onSidebandStreamChanged() {}
 };
diff --git a/libs/gui/tests/StreamSplitter_test.cpp b/libs/gui/tests/StreamSplitter_test.cpp
index 32ec90d..4e63a6f 100644
--- a/libs/gui/tests/StreamSplitter_test.cpp
+++ b/libs/gui/tests/StreamSplitter_test.cpp
@@ -46,7 +46,7 @@ protected:
 };
 
 struct DummyListener : public BnConsumerListener {
-    virtual void onFrameAvailable() {}
+    virtual void onFrameAvailable(const BufferItem& /* item */) {}
     virtual void onBuffersReleased() {}
     virtual void onSidebandStreamChanged() {}
 };
diff --git a/libs/gui/tests/SurfaceTextureGLThreadToGL.h b/libs/gui/tests/SurfaceTextureGLThreadToGL.h
index 6410516..14e42ac 100644
--- a/libs/gui/tests/SurfaceTextureGLThreadToGL.h
+++ b/libs/gui/tests/SurfaceTextureGLThreadToGL.h
@@ -130,7 +130,7 @@ protected:
         }
 
         // This should be called by GLConsumer on the producer thread.
-        virtual void onFrameAvailable() {
+        virtual void onFrameAvailable(const BufferItem& /* item */) {
             Mutex::Autolock lock(mMutex);
             ALOGV("+onFrameAvailable");
             mFrameAvailable = true;
diff --git a/opengl/tests/EGLTest/EGL_test.cpp b/opengl/tests/EGLTest/EGL_test.cpp
index a4364c6..d69a275 100644
--- a/opengl/tests/EGLTest/EGL_test.cpp
+++ b/opengl/tests/EGLTest/EGL_test.cpp
@@ -101,7 +101,7 @@ TEST_F(EGLTest, EGLTerminateSucceedsWithRemainingObjects) {
     EXPECT_TRUE(eglChooseConfig(mEglDisplay, attrs, &config, 1, &numConfigs));
 
     struct DummyConsumer : public BnConsumerListener {
-        virtual void onFrameAvailable() {}
+        virtual void onFrameAvailable(const BufferItem& /* item */) {}
         virtual void onBuffersReleased() {}
         virtual void onSidebandStreamChanged() {}
     };
diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp
index b767983..22d3cec 100644
--- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp
+++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp
@@ -122,7 +122,7 @@ status_t FramebufferSurface::nextBuffer(sp<GraphicBuffer>& outBuffer, sp<Fence>&
 }
 
 // Overrides ConsumerBase::onFrameAvailable(), does not call base class impl.
-void FramebufferSurface::onFrameAvailable() {
+void FramebufferSurface::onFrameAvailable(const BufferItem& /* item */) {
     sp<GraphicBuffer> buf;
     sp<Fence> acquireFence;
     status_t err = nextBuffer(buf, acquireFence);
diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h
index d0bf22b..8605862 100644
--- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h
+++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h
@@ -56,7 +56,7 @@ public:
 private:
     virtual ~FramebufferSurface() { }; // this class cannot be overloaded
 
-    virtual void onFrameAvailable();
+    virtual void onFrameAvailable(const BufferItem& item);
     virtual void freeBufferLocked(int slotIndex);
 
     virtual void dumpLocked(String8& result, const char* prefix) const;
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index f6ad503..acc2775 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -159,7 +159,7 @@ void Layer::onLayerDisplayed(const sp<const DisplayDevice>& /* hw */,
     }
 }
 
-void Layer::onFrameAvailable() {
+void Layer::onFrameAvailable(const BufferItem& /* item */) {
     android_atomic_inc(&mQueuedFrames);
     mFlinger->signalLayerUpdate();
 }
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 1f8eff0..e2100fc 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -330,7 +330,7 @@ protected:
 
 private:
     // Interface implementation for SurfaceFlingerConsumer::ContentsChangedListener
-    virtual void onFrameAvailable();
+    virtual void onFrameAvailable(const BufferItem& item);
     virtual void onSidebandStreamChanged();
 
     void commitTransaction();