From 8dc55396fc9bc425b5e2c82e76a38080f2a655ff Mon Sep 17 00:00:00 2001 From: Dan Stoza Date: Tue, 4 Nov 2014 11:37:46 -0800 Subject: [PATCH] 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 --- include/gui/BufferItem.h | 1 + include/gui/BufferQueue.h | 2 +- include/gui/BufferQueueProducer.h | 10 +++++ include/gui/ConsumerBase.h | 4 +- include/gui/IConsumerListener.h | 19 +++++++- include/gui/StreamSplitter.h | 2 +- libs/gui/BufferItem.cpp | 2 + libs/gui/BufferQueue.cpp | 5 ++- libs/gui/BufferQueueProducer.cpp | 44 +++++++++++++++---- libs/gui/ConsumerBase.cpp | 4 +- libs/gui/IConsumerListener.cpp | 20 +++++---- libs/gui/StreamSplitter.cpp | 2 +- libs/gui/tests/BufferQueue_test.cpp | 2 +- libs/gui/tests/DisconnectWaiter.h | 2 +- libs/gui/tests/FrameWaiter.h | 2 +- .../gui/tests/IGraphicBufferProducer_test.cpp | 2 +- libs/gui/tests/StreamSplitter_test.cpp | 2 +- libs/gui/tests/SurfaceTextureGLThreadToGL.h | 2 +- opengl/tests/EGLTest/EGL_test.cpp | 2 +- .../DisplayHardware/FramebufferSurface.cpp | 2 +- .../DisplayHardware/FramebufferSurface.h | 2 +- services/surfaceflinger/Layer.cpp | 2 +- services/surfaceflinger/Layer.h | 2 +- 23 files changed, 99 insertions(+), 38 deletions(-) diff --git a/include/gui/BufferItem.h b/include/gui/BufferItem.h index 5effd10e0..01b6ff4b5 100644 --- a/include/gui/BufferItem.h +++ b/include/gui/BufferItem.h @@ -44,6 +44,7 @@ class BufferItem : public Flattenable { // 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 3297b1044..118883745 100644 --- a/include/gui/BufferQueue.h +++ b/include/gui/BufferQueue.h @@ -62,7 +62,7 @@ public: public: ProxyConsumerListener(const wp& 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 c619a111c..ed1056a59 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 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 100bb260f..f7ab5ac59 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 260099e8d..2ef7c4dc1 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() = 0; /* Asynchronous */ + 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 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 f9279539a..8f47eb47a 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 d3fa43e75..e6fc79119 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 c49a8868f..61fd8c42d 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 listener(mConsumerListener.promote()); if (listener != NULL) { - listener->onFrameAvailable(); + listener->onFrameAvailable(item); } } diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp index 284ddb2bf..4d3b008df 100644 --- a/libs/gui/BufferQueueProducer.cpp +++ b/libs/gui/BufferQueueProducer.cpp @@ -39,7 +39,11 @@ BufferQueueProducer::BufferQueueProducer(const sp& 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 listener; + sp frameAvailableListener; + sp 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 f19b6c773..210e98e44 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 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 4ccf0ac82..409dfe4ee 100644 --- a/libs/gui/IConsumerListener.cpp +++ b/libs/gui/IConsumerListener.cpp @@ -21,6 +21,7 @@ #include #include +#include // --------------------------------------------------------------------------- namespace android { @@ -39,9 +40,10 @@ public: : BpInterface(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 771b263e7..5f39905e5 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 c7813664e..96de11f20 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 56e96c295..6e6915b29 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 bdedba67a..f78fa001c 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 aadfe6177..8d5fd8f58 100644 --- a/libs/gui/tests/IGraphicBufferProducer_test.cpp +++ b/libs/gui/tests/IGraphicBufferProducer_test.cpp @@ -65,7 +65,7 @@ const sp 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 32ec90d55..4e63a6f4d 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 6410516d2..14e42acfe 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 a4364c688..d69a27507 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 b76798329..22d3cecbf 100644 --- a/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp +++ b/services/surfaceflinger/DisplayHardware/FramebufferSurface.cpp @@ -122,7 +122,7 @@ status_t FramebufferSurface::nextBuffer(sp& outBuffer, sp& } // Overrides ConsumerBase::onFrameAvailable(), does not call base class impl. -void FramebufferSurface::onFrameAvailable() { +void FramebufferSurface::onFrameAvailable(const BufferItem& /* item */) { sp buf; sp acquireFence; status_t err = nextBuffer(buf, acquireFence); diff --git a/services/surfaceflinger/DisplayHardware/FramebufferSurface.h b/services/surfaceflinger/DisplayHardware/FramebufferSurface.h index d0bf22be8..8605862e5 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 f6ad503b7..acc277531 100644 --- a/services/surfaceflinger/Layer.cpp +++ b/services/surfaceflinger/Layer.cpp @@ -159,7 +159,7 @@ void Layer::onLayerDisplayed(const sp& /* 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 1f8eff0c8..e2100fcd1 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();