From f0eaf25e9247edf4d124bedaeb863f7abdf35a3e Mon Sep 17 00:00:00 2001 From: Dan Stoza Date: Fri, 21 Mar 2014 13:05:51 -0700 Subject: [PATCH] BufferQueue: Add producer buffer-released callback Add a callback to the producer side, onBufferReleased, which will be called every time the consumer releases a buffer back to the BufferQueue. This will enable a buffer stream splitter to work autonomously without having to block on dequeueBuffer. The binder object used for the callback replaces the generic IBinder token that was passed into IGraphicBufferProducer::connect to detect the death of the producer. If a producer does not wish to listen for buffer release events, it can pass in an instance of the DummyProducerListener class defined in IProducerListener.h, if it even cares about death events (BufferQueue doesn't enforce the token being non-NULL, though perhaps we should). Change-Id: I23935760673524abeafea2b58dccc3583b368710 --- include/gui/BufferQueue.h | 12 +--- include/gui/BufferQueueCore.h | 4 +- include/gui/BufferQueueProducer.h | 2 +- include/gui/IGraphicBufferProducer.h | 11 +-- include/gui/IProducerListener.h | 67 +++++++++++++++++++ libs/gui/Android.mk | 1 + libs/gui/BufferQueue.cpp | 4 +- libs/gui/BufferQueueCore.cpp | 3 +- libs/gui/BufferQueueProducer.cpp | 20 +++--- libs/gui/IGraphicBufferProducer.cpp | 17 +++-- libs/gui/IProducerListener.cpp | 53 +++++++++++++++ libs/gui/Surface.cpp | 5 +- libs/gui/tests/BufferQueue_test.cpp | 27 ++++---- .../gui/tests/IGraphicBufferProducer_test.cpp | 3 +- .../DisplayHardware/VirtualDisplaySurface.cpp | 5 +- .../DisplayHardware/VirtualDisplaySurface.h | 3 +- services/surfaceflinger/MonitoredProducer.cpp | 6 +- services/surfaceflinger/MonitoredProducer.h | 4 +- 18 files changed, 191 insertions(+), 56 deletions(-) create mode 100644 include/gui/IProducerListener.h create mode 100644 libs/gui/IProducerListener.cpp diff --git a/include/gui/BufferQueue.h b/include/gui/BufferQueue.h index f74dc2624..85bf5a9ac 100644 --- a/include/gui/BufferQueue.h +++ b/include/gui/BufferQueue.h @@ -232,16 +232,8 @@ public: // will usually be the one obtained from dequeueBuffer. virtual void cancelBuffer(int buf, const sp& fence); - // connect attempts to connect a producer API to the BufferQueue. This - // must be called before any other IGraphicBufferProducer methods are - // called except for getAllocator. A consumer must already be connected. - // - // This method will fail if connect was previously called on the - // BufferQueue and no corresponding disconnect call was made (i.e. if - // it's still connected to a producer). - // - // APIs are enumerated in window.h (e.g. NATIVE_WINDOW_API_CPU). - virtual status_t connect(const sp& token, + // See IGraphicBufferProducer::connect + virtual status_t connect(const sp& listener, int api, bool producerControlledByApp, QueueBufferOutput* output); // disconnect attempts to disconnect a producer API from the BufferQueue. diff --git a/include/gui/BufferQueueCore.h b/include/gui/BufferQueueCore.h index 89f277996..cfcb7a5a8 100644 --- a/include/gui/BufferQueueCore.h +++ b/include/gui/BufferQueueCore.h @@ -46,9 +46,9 @@ namespace android { class BufferItem; -class IBinder; class IConsumerListener; class IGraphicBufferAlloc; +class IProducerListener; class BufferQueueCore : public virtual RefBase { @@ -162,7 +162,7 @@ private: // mConnectedProducerToken is used to set a binder death notification on // the producer. - sp mConnectedProducerToken; + sp mConnectedProducerListener; // mSlots is an array of buffer slots that must be mirrored on the producer // side. This allows buffer ownership to be transferred between the producer diff --git a/include/gui/BufferQueueProducer.h b/include/gui/BufferQueueProducer.h index 0013b0a9f..04692a578 100644 --- a/include/gui/BufferQueueProducer.h +++ b/include/gui/BufferQueueProducer.h @@ -140,7 +140,7 @@ public: // it's still connected to a producer). // // APIs are enumerated in window.h (e.g. NATIVE_WINDOW_API_CPU). - virtual status_t connect(const sp& token, + virtual status_t connect(const sp& listener, int api, bool producerControlledByApp, QueueBufferOutput* output); // disconnect attempts to disconnect a producer API from the BufferQueue. diff --git a/include/gui/IGraphicBufferProducer.h b/include/gui/IGraphicBufferProducer.h index 0874f03fa..2b61ab3a9 100644 --- a/include/gui/IGraphicBufferProducer.h +++ b/include/gui/IGraphicBufferProducer.h @@ -32,6 +32,7 @@ namespace android { // ---------------------------------------------------------------------------- +class IProducerListener; class NativeHandle; class Surface; @@ -344,9 +345,11 @@ public: // This method will fail if the connect was previously called on the // IGraphicBufferProducer and no corresponding disconnect call was made. // - // The token needs to be any opaque binder object that lives in the - // producer process -- it is solely used for obtaining a death notification - // when the producer is killed. + // The listener is an optional binder callback object that can be used if + // the producer wants to be notified when the consumer releases a buffer + // back to the BufferQueue. It is also used to detect the death of the + // producer. If only the latter functionality is desired, there is a + // DummyProducerListener class in IProducerListener.h that can be used. // // The api should be one of the NATIVE_WINDOW_API_* values in // @@ -370,7 +373,7 @@ public: // // Additional negative errors may be returned by the internals, they // should be treated as opaque fatal unrecoverable errors. - virtual status_t connect(const sp& token, + virtual status_t connect(const sp& listener, int api, bool producerControlledByApp, QueueBufferOutput* output) = 0; // disconnect attempts to disconnect a client API from the diff --git a/include/gui/IProducerListener.h b/include/gui/IProducerListener.h new file mode 100644 index 000000000..3848a6c85 --- /dev/null +++ b/include/gui/IProducerListener.h @@ -0,0 +1,67 @@ +/* + * Copyright 2014 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef ANDROID_GUI_IPRODUCERLISTENER_H +#define ANDROID_GUI_IPRODUCERLISTENER_H + +#include + +#include + +namespace android { + +// ProducerListener is the interface through which the BufferQueue notifies the +// producer of events that the producer may wish to react to. Because the +// producer will generally have a mutex that is locked during calls from the +// producer to the BufferQueue, these calls from the BufferQueue to the +// producer *MUST* be called only when the BufferQueue mutex is NOT locked. + +class ProducerListener : public virtual RefBase +{ +public: + ProducerListener() {} + virtual ~ProducerListener() {} + + // onBufferReleased is called from IGraphicBufferConsumer::releaseBuffer to + // notify the producer that a new buffer is free and ready to be dequeued. + // + // This is called without any lock held and can be called concurrently by + // multiple threads. + virtual void onBufferReleased() = 0; // Asynchronous +}; + +class IProducerListener : public ProducerListener, public IInterface +{ +public: + DECLARE_META_INTERFACE(ProducerListener) +}; + +class BnProducerListener : public BnInterface +{ +public: + virtual status_t onTransact(uint32_t code, const Parcel& data, + Parcel* reply, uint32_t flags = 0); +}; + +class DummyProducerListener : public BnProducerListener +{ +public: + virtual void onBufferReleased() {} +}; + +} // namespace android + +#endif diff --git a/libs/gui/Android.mk b/libs/gui/Android.mk index 0a77317ea..04a8a8d50 100644 --- a/libs/gui/Android.mk +++ b/libs/gui/Android.mk @@ -21,6 +21,7 @@ LOCAL_SRC_FILES:= \ IDisplayEventConnection.cpp \ IGraphicBufferAlloc.cpp \ IGraphicBufferProducer.cpp \ + IProducerListener.cpp \ ISensorEventConnection.cpp \ ISensorServer.cpp \ ISurfaceComposer.cpp \ diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp index c306f9d65..6d2cb25bd 100644 --- a/libs/gui/BufferQueue.cpp +++ b/libs/gui/BufferQueue.cpp @@ -119,9 +119,9 @@ void BufferQueue::cancelBuffer(int buf, const sp& fence) { mProducer->cancelBuffer(buf, fence); } -status_t BufferQueue::connect(const sp& token, +status_t BufferQueue::connect(const sp& listener, int api, bool producerControlledByApp, QueueBufferOutput* output) { - return mProducer->connect(token, api, producerControlledByApp, output); + return mProducer->connect(listener, api, producerControlledByApp, output); } status_t BufferQueue::disconnect(int api) { diff --git a/libs/gui/BufferQueueCore.cpp b/libs/gui/BufferQueueCore.cpp index 300b23a4f..28bbb1bd1 100644 --- a/libs/gui/BufferQueueCore.cpp +++ b/libs/gui/BufferQueueCore.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -47,7 +48,7 @@ BufferQueueCore::BufferQueueCore(const sp& allocator) : mConsumerListener(), mConsumerUsageBits(0), mConnectedApi(NO_CONNECTED_API), - mConnectedProducerToken(), + mConnectedProducerListener(), mSlots(), mQueue(), mOverrideMaxBufferCount(0), diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp index 9dd90bab5..82d931ec7 100644 --- a/libs/gui/BufferQueueProducer.cpp +++ b/libs/gui/BufferQueueProducer.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include @@ -654,7 +655,7 @@ int BufferQueueProducer::query(int what, int *outValue) { return NO_ERROR; } -status_t BufferQueueProducer::connect(const sp &token, +status_t BufferQueueProducer::connect(const sp& listener, int api, bool producerControlledByApp, QueueBufferOutput *output) { ATRACE_CALL(); Mutex::Autolock lock(mCore->mMutex); @@ -711,16 +712,16 @@ status_t BufferQueueProducer::connect(const sp &token, // Set up a death notification so that we can disconnect // automatically if the remote producer dies - if (token != NULL && token->remoteBinder() != NULL) { - status = token->linkToDeath( + if (listener != NULL && + listener->asBinder()->remoteBinder() != NULL) { + status = listener->asBinder()->linkToDeath( static_cast(this)); - if (status == NO_ERROR) { - mCore->mConnectedProducerToken = token; - } else { + if (status != NO_ERROR) { BQ_LOGE("connect(P): linkToDeath failed: %s (%d)", strerror(-status), status); } } + mCore->mConnectedProducerListener = listener; break; default: BQ_LOGE("connect(P): unknown API %d", api); @@ -759,14 +760,15 @@ status_t BufferQueueProducer::disconnect(int api) { mCore->freeAllBuffersLocked(); // Remove our death notification callback if we have one - sp token = mCore->mConnectedProducerToken; - if (token != NULL) { + if (mCore->mConnectedProducerListener != NULL) { + sp token = + mCore->mConnectedProducerListener->asBinder(); // This can fail if we're here because of the death // notification, but we just ignore it token->unlinkToDeath( static_cast(this)); } - mCore->mConnectedProducerToken = NULL; + mCore->mConnectedProducerListener = NULL; mCore->mConnectedApi = BufferQueueCore::NO_CONNECTED_API; mCore->mSidebandStream.clear(); mCore->mDequeueCondition.broadcast(); diff --git a/libs/gui/IGraphicBufferProducer.cpp b/libs/gui/IGraphicBufferProducer.cpp index 1d4ec1c0d..50490af1a 100644 --- a/libs/gui/IGraphicBufferProducer.cpp +++ b/libs/gui/IGraphicBufferProducer.cpp @@ -27,6 +27,7 @@ #include #include +#include namespace android { // ---------------------------------------------------------------------------- @@ -171,11 +172,16 @@ public: return result; } - virtual status_t connect(const sp& token, + virtual status_t connect(const sp& listener, int api, bool producerControlledByApp, QueueBufferOutput* output) { Parcel data, reply; data.writeInterfaceToken(IGraphicBufferProducer::getInterfaceDescriptor()); - data.writeStrongBinder(token); + if (listener != NULL) { + data.writeInt32(1); + data.writeStrongBinder(listener->asBinder()); + } else { + data.writeInt32(0); + } data.writeInt32(api); data.writeInt32(producerControlledByApp); status_t result = remote()->transact(CONNECT, data, &reply); @@ -308,13 +314,16 @@ status_t BnGraphicBufferProducer::onTransact( } break; case CONNECT: { CHECK_INTERFACE(IGraphicBufferProducer, data, reply); - sp token = data.readStrongBinder(); + sp listener; + if (data.readInt32() == 1) { + listener = IProducerListener::asInterface(data.readStrongBinder()); + } int api = data.readInt32(); bool producerControlledByApp = data.readInt32(); QueueBufferOutput* const output = reinterpret_cast( reply->writeInplace(sizeof(QueueBufferOutput))); - status_t res = connect(token, api, producerControlledByApp, output); + status_t res = connect(listener, api, producerControlledByApp, output); reply->writeInt32(res); return NO_ERROR; } break; diff --git a/libs/gui/IProducerListener.cpp b/libs/gui/IProducerListener.cpp new file mode 100644 index 000000000..efe4069e2 --- /dev/null +++ b/libs/gui/IProducerListener.cpp @@ -0,0 +1,53 @@ +/* + * Copyright 2014 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +namespace android { + +enum { + ON_BUFFER_RELEASED = IBinder::FIRST_CALL_TRANSACTION, +}; + +class BpProducerListener : public BpInterface +{ +public: + BpProducerListener(const sp& impl) + : BpInterface(impl) {} + + virtual void onBufferReleased() { + Parcel data, reply; + data.writeInterfaceToken(IProducerListener::getInterfaceDescriptor()); + remote()->transact(ON_BUFFER_RELEASED, data, &reply, IBinder::FLAG_ONEWAY); + } +}; + +IMPLEMENT_META_INTERFACE(ProducerListener, "android.gui.IProducerListener") + +status_t BnProducerListener::onTransact(uint32_t code, const Parcel& data, + Parcel* reply, uint32_t flags) { + switch (code) { + case ON_BUFFER_RELEASED: + CHECK_INTERFACE(IProducerListener, data, reply); + onBufferReleased(); + return NO_ERROR; + } + return BBinder::onTransact(code, data, reply, flags); +} + +} // namespace android diff --git a/libs/gui/Surface.cpp b/libs/gui/Surface.cpp index 95f408426..d1ef503f8 100644 --- a/libs/gui/Surface.cpp +++ b/libs/gui/Surface.cpp @@ -27,6 +27,7 @@ #include +#include #include #include #include @@ -513,10 +514,10 @@ int Surface::dispatchUnlockAndPost(va_list args __attribute__((unused))) { int Surface::connect(int api) { ATRACE_CALL(); ALOGV("Surface::connect"); - static sp sLife = new BBinder(); + static sp listener = new DummyProducerListener(); Mutex::Autolock lock(mMutex); IGraphicBufferProducer::QueueBufferOutput output; - int err = mGraphicBufferProducer->connect(sLife, api, mProducerControlledByApp, &output); + int err = mGraphicBufferProducer->connect(listener, api, mProducerControlledByApp, &output); if (err == NO_ERROR) { uint32_t numPendingBuffers = 0; output.deflate(&mDefaultWidth, &mDefaultHeight, &mTransformHint, diff --git a/libs/gui/tests/BufferQueue_test.cpp b/libs/gui/tests/BufferQueue_test.cpp index 7943476a6..e578eef72 100644 --- a/libs/gui/tests/BufferQueue_test.cpp +++ b/libs/gui/tests/BufferQueue_test.cpp @@ -17,17 +17,19 @@ #define LOG_TAG "BufferQueue_test" //#define LOG_NDEBUG 0 -#include - -#include -#include +#include +#include #include #include #include #include -#include + +#include +#include + +#include namespace android { @@ -141,7 +143,8 @@ TEST_F(BufferQueueTest, AcquireBuffer_ExceedsMaxAcquireCount_Fails) { sp dc(new DummyConsumer); mConsumer->consumerConnect(dc, false); IGraphicBufferProducer::QueueBufferOutput qbo; - mProducer->connect(NULL, NATIVE_WINDOW_API_CPU, false, &qbo); + mProducer->connect(new DummyProducerListener, NATIVE_WINDOW_API_CPU, false, + &qbo); mProducer->setBufferCount(4); int slot; @@ -207,8 +210,8 @@ TEST_F(BufferQueueTest, DetachAndReattachOnProducerSide) { sp dc(new DummyConsumer); ASSERT_EQ(OK, mConsumer->consumerConnect(dc, false)); IGraphicBufferProducer::QueueBufferOutput output; - ASSERT_EQ(OK, - mProducer->connect(NULL, NATIVE_WINDOW_API_CPU, false, &output)); + ASSERT_EQ(OK, mProducer->connect(new DummyProducerListener, + NATIVE_WINDOW_API_CPU, false, &output)); ASSERT_EQ(BAD_VALUE, mProducer->detachBuffer(-1)); // Index too low ASSERT_EQ(BAD_VALUE, mProducer->detachBuffer( @@ -260,8 +263,8 @@ TEST_F(BufferQueueTest, DetachAndReattachOnConsumerSide) { sp dc(new DummyConsumer); ASSERT_EQ(OK, mConsumer->consumerConnect(dc, false)); IGraphicBufferProducer::QueueBufferOutput output; - ASSERT_EQ(OK, - mProducer->connect(NULL, NATIVE_WINDOW_API_CPU, false, &output)); + ASSERT_EQ(OK, mProducer->connect(new DummyProducerListener, + NATIVE_WINDOW_API_CPU, false, &output)); int slot; sp fence; @@ -318,8 +321,8 @@ TEST_F(BufferQueueTest, MoveFromConsumerToProducer) { sp dc(new DummyConsumer); ASSERT_EQ(OK, mConsumer->consumerConnect(dc, false)); IGraphicBufferProducer::QueueBufferOutput output; - ASSERT_EQ(OK, - mProducer->connect(NULL, NATIVE_WINDOW_API_CPU, false, &output)); + ASSERT_EQ(OK, mProducer->connect(new DummyProducerListener, + NATIVE_WINDOW_API_CPU, false, &output)); int slot; sp fence; diff --git a/libs/gui/tests/IGraphicBufferProducer_test.cpp b/libs/gui/tests/IGraphicBufferProducer_test.cpp index c2653c219..70a89cd5f 100644 --- a/libs/gui/tests/IGraphicBufferProducer_test.cpp +++ b/libs/gui/tests/IGraphicBufferProducer_test.cpp @@ -25,13 +25,14 @@ #include #include +#include #include #define ASSERT_OK(x) ASSERT_EQ(OK, (x)) #define EXPECT_OK(x) EXPECT_EQ(OK, (x)) -#define TEST_TOKEN ((IBinder*)(NULL)) +#define TEST_TOKEN ((IProducerListener*)(NULL)) #define TEST_API NATIVE_WINDOW_API_CPU #define TEST_API_OTHER NATIVE_WINDOW_API_EGL // valid API that's not TEST_API #define TEST_CONTROLLED_BY_APP false diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp index f4cb8b5d5..67229d50d 100644 --- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp @@ -454,11 +454,12 @@ int VirtualDisplaySurface::query(int what, int* value) { return mSource[SOURCE_SINK]->query(what, value); } -status_t VirtualDisplaySurface::connect(const sp& token, +status_t VirtualDisplaySurface::connect(const sp& listener, int api, bool producerControlledByApp, QueueBufferOutput* output) { QueueBufferOutput qbo; - status_t result = mSource[SOURCE_SINK]->connect(token, api, producerControlledByApp, &qbo); + status_t result = mSource[SOURCE_SINK]->connect(listener, api, + producerControlledByApp, &qbo); if (result == NO_ERROR) { updateQueueBufferOutput(qbo); *output = mQueueBufferOutput; diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h index 09e5544fa..165224a1c 100644 --- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h @@ -27,6 +27,7 @@ namespace android { // --------------------------------------------------------------------------- class HWComposer; +class IProducerListener; /* This DisplaySurface implementation supports virtual displays, where GLES * and/or HWC compose into a buffer that is then passed to an arbitrary @@ -105,7 +106,7 @@ private: const QueueBufferInput& input, QueueBufferOutput* output); virtual void cancelBuffer(int pslot, const sp& fence); virtual int query(int what, int* value); - virtual status_t connect(const sp& token, + virtual status_t connect(const sp& listener, int api, bool producerControlledByApp, QueueBufferOutput* output); virtual status_t disconnect(int api); virtual status_t setSidebandStream(const sp& stream); diff --git a/services/surfaceflinger/MonitoredProducer.cpp b/services/surfaceflinger/MonitoredProducer.cpp index 8fed676ac..f6010b7c2 100644 --- a/services/surfaceflinger/MonitoredProducer.cpp +++ b/services/surfaceflinger/MonitoredProducer.cpp @@ -88,9 +88,9 @@ int MonitoredProducer::query(int what, int* value) { return mProducer->query(what, value); } -status_t MonitoredProducer::connect(const sp& token, int api, - bool producerControlledByApp, QueueBufferOutput* output) { - return mProducer->connect(token, api, producerControlledByApp, output); +status_t MonitoredProducer::connect(const sp& listener, + int api, bool producerControlledByApp, QueueBufferOutput* output) { + return mProducer->connect(listener, api, producerControlledByApp, output); } status_t MonitoredProducer::disconnect(int api) { diff --git a/services/surfaceflinger/MonitoredProducer.h b/services/surfaceflinger/MonitoredProducer.h index f78be2001..fdb63e611 100644 --- a/services/surfaceflinger/MonitoredProducer.h +++ b/services/surfaceflinger/MonitoredProducer.h @@ -21,7 +21,7 @@ namespace android { -class IBinder; +class IProducerListener; class NativeHandle; class SurfaceFlinger; @@ -45,7 +45,7 @@ public: QueueBufferOutput* output); virtual void cancelBuffer(int slot, const sp& fence); virtual int query(int what, int* value); - virtual status_t connect(const sp& token, int api, + virtual status_t connect(const sp& token, int api, bool producerControlledByApp, QueueBufferOutput* output); virtual status_t disconnect(int api); virtual status_t setSidebandStream(const sp& stream);