Clean-up egl image preallocation

Allocate EGL images in onFrameAvailable, instead of a custom
onBuffersAllocated callback. This way we reduce traffic over binder
while still performing GL work ahead of time in queueBuffer().

Bug: 130567928
Test: systrace
Change-Id: I4070e9ddbd379dac3d809d0e7edb2855fc8b7a80

10 files changed, 17 insertions, 59 deletions

diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp
index d87228fbbd..5fb3f0b80f 100644
--- a/libs/gui/BufferQueue.cpp
+++ b/libs/gui/BufferQueue.cpp
@@ -59,13 +59,6 @@ void BufferQueue::ProxyConsumerListener::onFrameReplaced(
     }
 }
 
-void BufferQueue::ProxyConsumerListener::onBufferAllocated(const BufferItem& item) {
-    sp<ConsumerListener> listener(mConsumerListener.promote());
-    if (listener != nullptr) {
-        listener->onBufferAllocated(item);
-    }
-}
-
 void BufferQueue::ProxyConsumerListener::onBuffersReleased() {
     sp<ConsumerListener> listener(mConsumerListener.promote());
     if (listener != nullptr) {
diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp
index c94c6b31c6..3928bb9e40 100644
--- a/libs/gui/BufferQueueProducer.cpp
+++ b/libs/gui/BufferQueueProducer.cpp
@@ -539,13 +539,6 @@ status_t BufferQueueProducer::dequeueBuffer(int* outSlot, sp<android::Fence>* ou
                 return NO_INIT;
             }
 
-            if (mCore->mConsumerListener != nullptr) {
-                BufferItem item;
-                item.mGraphicBuffer = graphicBuffer;
-                item.mSlot = *outSlot;
-                mCore->mConsumerListener->onBufferAllocated(item);
-            }
-
             VALIDATE_CONSISTENCY();
         } // Autolock scope
     }
@@ -975,9 +968,6 @@ status_t BufferQueueProducer::queueBuffer(int slot,
         item.mGraphicBuffer.clear();
     }
 
-    // Don't send the slot number through the callback since the consumer shouldn't need it
-    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
 
@@ -1433,13 +1423,6 @@ void BufferQueueProducer::allocateBuffers(uint32_t width, uint32_t height,
                 BQ_LOGV("allocateBuffers: allocated a new buffer in slot %d",
                         *slot);
 
-                if (mCore->mConsumerListener != nullptr) {
-                    BufferItem item;
-                    item.mGraphicBuffer = buffers[i];
-                    item.mSlot = *slot;
-                    mCore->mConsumerListener->onBufferAllocated(item);
-                }
-
                 // Make sure the erase is done after all uses of the slot
                 // iterator since it will be invalid after this point.
                 mCore->mFreeSlots.erase(slot);
diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp
index 1e94cc13cd..abd9921fa9 100644
--- a/libs/gui/ConsumerBase.cpp
+++ b/libs/gui/ConsumerBase.cpp
@@ -131,8 +131,6 @@ void ConsumerBase::onFrameReplaced(const BufferItem &item) {
     }
 }
 
-void ConsumerBase::onBufferAllocated(const BufferItem& /*item*/) {}
-
 void ConsumerBase::onBuffersReleased() {
     Mutex::Autolock lock(mMutex);
 
diff --git a/libs/gui/IConsumerListener.cpp b/libs/gui/IConsumerListener.cpp
index ea9045cad9..85ac304ab8 100644
--- a/libs/gui/IConsumerListener.cpp
+++ b/libs/gui/IConsumerListener.cpp
@@ -28,8 +28,7 @@ enum class Tag : uint32_t {
     ON_FRAME_REPLACED,
     ON_BUFFERS_RELEASED,
     ON_SIDEBAND_STREAM_CHANGED,
-    ON_BUFFER_ALLOCATED,
-    LAST = ON_BUFFER_ALLOCATED,
+    LAST = ON_SIDEBAND_STREAM_CHANGED,
 };
 
 } // Anonymous namespace
@@ -55,11 +54,6 @@ public:
                                                                        item);
     }
 
-    void onBufferAllocated(const BufferItem& item) override {
-        callRemoteAsync<decltype(&IConsumerListener::onBufferAllocated)>(Tag::ON_BUFFER_ALLOCATED,
-                                                                         item);
-    }
-
     void onBuffersReleased() override {
         callRemoteAsync<decltype(&IConsumerListener::onBuffersReleased)>(Tag::ON_BUFFERS_RELEASED);
     }
@@ -95,8 +89,6 @@ status_t BnConsumerListener::onTransact(uint32_t code, const Parcel& data, Parce
             return callLocalAsync(data, reply, &IConsumerListener::onFrameAvailable);
         case Tag::ON_FRAME_REPLACED:
             return callLocalAsync(data, reply, &IConsumerListener::onFrameReplaced);
-        case Tag::ON_BUFFER_ALLOCATED:
-            return callLocalAsync(data, reply, &IConsumerListener::onBufferAllocated);
         case Tag::ON_BUFFERS_RELEASED:
             return callLocalAsync(data, reply, &IConsumerListener::onBuffersReleased);
         case Tag::ON_SIDEBAND_STREAM_CHANGED:
diff --git a/libs/gui/include/gui/BufferQueue.h b/libs/gui/include/gui/BufferQueue.h
index 721427be7b..da952744f3 100644
--- a/libs/gui/include/gui/BufferQueue.h
+++ b/libs/gui/include/gui/BufferQueue.h
@@ -61,7 +61,6 @@ public:
         void onDisconnect() override;
         void onFrameAvailable(const BufferItem& item) override;
         void onFrameReplaced(const BufferItem& item) override;
-        void onBufferAllocated(const BufferItem& item) override;
         void onBuffersReleased() override;
         void onSidebandStreamChanged() override;
         void addAndGetFrameTimestamps(
diff --git a/libs/gui/include/gui/ConsumerBase.h b/libs/gui/include/gui/ConsumerBase.h
index 7c26482509..366ced380b 100644
--- a/libs/gui/include/gui/ConsumerBase.h
+++ b/libs/gui/include/gui/ConsumerBase.h
@@ -141,7 +141,6 @@ protected:
     // classes if they want the notification.
     virtual void onFrameAvailable(const BufferItem& item) override;
     virtual void onFrameReplaced(const BufferItem& item) override;
-    virtual void onBufferAllocated(const BufferItem& item) override;
     virtual void onBuffersReleased() override;
     virtual void onSidebandStreamChanged() override;
 
diff --git a/libs/gui/include/gui/IConsumerListener.h b/libs/gui/include/gui/IConsumerListener.h
index 03fefbe90c..c0828820e3 100644
--- a/libs/gui/include/gui/IConsumerListener.h
+++ b/libs/gui/include/gui/IConsumerListener.h
@@ -61,13 +61,6 @@ public:
     // This is called without any lock held and can be called concurrently by multiple threads.
     virtual void onFrameReplaced(const BufferItem& /* item */) {} /* Asynchronous */
 
-    // onBufferAllocated is called to notify the buffer consumer that the BufferQueue has allocated
-    // a GraphicBuffer for a particular slot. Only the GraphicBuffer pointer and the slot ID will
-    // be populated.
-    //
-    // This is called without any lock held and can be called concurrently by multiple threads.
-    virtual void onBufferAllocated(const BufferItem& /* item */) {} /* Asynchronous */
-
     // 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.
diff --git a/services/surfaceflinger/BufferLayerConsumer.cpp b/services/surfaceflinger/BufferLayerConsumer.cpp
index fc98dc836a..6709fb4b48 100644
--- a/services/surfaceflinger/BufferLayerConsumer.cpp
+++ b/services/surfaceflinger/BufferLayerConsumer.cpp
@@ -478,20 +478,15 @@ void BufferLayerConsumer::onSidebandStreamChanged() {
     }
 }
 
-void BufferLayerConsumer::onBufferAllocated(const BufferItem& item) {
-    if (item.mGraphicBuffer != nullptr) {
-        std::shared_ptr<Image> image = std::make_shared<Image>(item.mGraphicBuffer, mRE);
-        std::shared_ptr<Image> oldImage;
-        {
-            std::lock_guard<std::mutex> lock(mImagesMutex);
-            oldImage = mImages[item.mSlot];
-            if (oldImage == nullptr || oldImage->graphicBuffer() == nullptr ||
-                oldImage->graphicBuffer()->getId() != item.mGraphicBuffer->getId()) {
-                mImages[item.mSlot] = std::make_shared<Image>(item.mGraphicBuffer, mRE);
-            }
-            image = mImages[item.mSlot];
+void BufferLayerConsumer::onBufferAvailable(const BufferItem& item) {
+    if (item.mGraphicBuffer != nullptr && item.mSlot != BufferQueue::INVALID_BUFFER_SLOT) {
+        std::lock_guard<std::mutex> lock(mImagesMutex);
+        const std::shared_ptr<Image>& oldImage = mImages[item.mSlot];
+        if (oldImage == nullptr || oldImage->graphicBuffer() == nullptr ||
+            oldImage->graphicBuffer()->getId() != item.mGraphicBuffer->getId()) {
+            mImages[item.mSlot] = std::make_shared<Image>(item.mGraphicBuffer, mRE);
+            mRE.cacheExternalTextureBuffer(item.mGraphicBuffer);
         }
-        mRE.cacheExternalTextureBuffer(image->graphicBuffer());
     }
 }
 
@@ -533,5 +528,4 @@ BufferLayerConsumer::Image::~Image() {
         mRE.unbindExternalTextureBuffer(mGraphicBuffer->getId());
     }
 }
-
 }; // namespace android
diff --git a/services/surfaceflinger/BufferLayerConsumer.h b/services/surfaceflinger/BufferLayerConsumer.h
index 0f0655d705..e3f6100c35 100644
--- a/services/surfaceflinger/BufferLayerConsumer.h
+++ b/services/surfaceflinger/BufferLayerConsumer.h
@@ -176,6 +176,7 @@ public:
     // setConsumerUsageBits overrides the ConsumerBase method to OR
     // DEFAULT_USAGE_FLAGS to usage.
     status_t setConsumerUsageBits(uint64_t usage);
+    void onBufferAvailable(const BufferItem& item) EXCLUDES(mImagesMutex);
 
 protected:
     // abandonLocked overrides the ConsumerBase method to clear
@@ -241,7 +242,6 @@ private:
 
     // IConsumerListener interface
     void onDisconnect() override;
-    void onBufferAllocated(const BufferItem& item) override EXCLUDES(mImagesMutex);
     void onSidebandStreamChanged() override;
     void addAndGetFrameTimestamps(const NewFrameEventsEntry* newTimestamps,
                                   FrameEventHistoryDelta* outDelta) override;
diff --git a/services/surfaceflinger/BufferQueueLayer.cpp b/services/surfaceflinger/BufferQueueLayer.cpp
index 5d729f5b4f..3d51ec33b2 100644
--- a/services/surfaceflinger/BufferQueueLayer.cpp
+++ b/services/surfaceflinger/BufferQueueLayer.cpp
@@ -14,6 +14,9 @@
  * limitations under the License.
  */
 
+#undef LOG_TAG
+#define LOG_TAG "BufferQueueLayer"
+#define ATRACE_TAG ATRACE_TAG_GRAPHICS
 #include <compositionengine/Display.h>
 #include <compositionengine/Layer.h>
 #include <compositionengine/OutputLayer.h>
@@ -435,6 +438,7 @@ void BufferQueueLayer::fakeVsync() {
 }
 
 void BufferQueueLayer::onFrameAvailable(const BufferItem& item) {
+    ATRACE_CALL();
     // Add this buffer from our internal queue tracker
     { // Autolock scope
         if (mFlinger->mUseSmart90ForVideo) {
@@ -475,9 +479,11 @@ void BufferQueueLayer::onFrameAvailable(const BufferItem& item) {
     } else {
         mFlinger->signalLayerUpdate();
     }
+    mConsumer->onBufferAvailable(item);
 }
 
 void BufferQueueLayer::onFrameReplaced(const BufferItem& item) {
+    ATRACE_CALL();
     { // Autolock scope
         Mutex::Autolock lock(mQueueItemLock);
 
@@ -499,6 +505,7 @@ void BufferQueueLayer::onFrameReplaced(const BufferItem& item) {
         mLastFrameNumberReceived = item.mFrameNumber;
         mQueueItemCondition.broadcast();
     }
+    mConsumer->onBufferAvailable(item);
 }
 
 void BufferQueueLayer::onSidebandStreamChanged() {