Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ca81c05b authored by Patrick Williams's avatar Patrick Williams
Browse files

Fix stale buffer release issue.

BufferItemConsumer sometimes returns BufferItems where mIsStale is already set to true, breaking the logic for ignoring buffers that were released via a disconnect. This CL adds a new field for BLAST to use for this purpose.

Bug: 294133380
Flag: com.android.graphics.libgui.flags.buffer_release_channel
Test: CtsMediaDecoderTestCases, CtsDeqpTestCases
Change-Id: Ie32c9c6e0e26e68b794a1ca78cdd3c2395ac0966
parent 493e36c9
Loading
Loading
Loading
Loading
+14 −5
Original line number Diff line number Diff line
@@ -510,7 +510,7 @@ void BLASTBufferQueue::releaseBuffer(const ReleaseCallbackId& callbackId,
        return;
    }
#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
    if (!it->second.mIsStale) {
    if (!it->second.disconnectedAfterAcquired) {
        mNumAcquired--;
    }
#else
@@ -566,7 +566,7 @@ status_t BLASTBufferQueue::acquireNextBufferLocked(
        applyTransaction = false;
    }

    BufferItem bufferItem;
    BLASTBufferItem bufferItem;

    status_t status =
            mBufferItemConsumer->acquireBuffer(&bufferItem, 0 /* expectedPresent */, false);
@@ -1130,9 +1130,9 @@ public:
// can be non-blocking when the producer is in the client process.
class BBQBufferQueueProducer : public BufferQueueProducer {
public:
    BBQBufferQueueProducer(const sp<BufferQueueCore>& core, wp<BLASTBufferQueue> bbq)
    BBQBufferQueueProducer(const sp<BufferQueueCore>& core, const wp<BLASTBufferQueue>& bbq)
          : BufferQueueProducer(core, false /* consumerIsSurfaceFlinger*/),
            mBLASTBufferQueue(std::move(bbq)) {}
            mBLASTBufferQueue(bbq) {}

    status_t connect(const sp<IProducerListener>& listener, int api, bool producerControlledByApp,
                     QueueBufferOutput* output) override {
@@ -1156,10 +1156,19 @@ public:
            return status;
        }

        // We need to reset dequeued and acquired counts because BufferQueueProducer::disconnect
        // calls BufferQueueCore::freeAllBuffersLocked which frees all dequeued and acquired
        // buffers. We don't reset mNumFrameAvailable because these buffers are still available
        // in BufferItemConsumer.
        bbq->mNumDequeued = 0;
        bbq->mNumAcquired = 0;
        // SurfaceFlinger sends release callbacks for buffers that have been acquired after a
        // disconnect. We set disconnectedAfterAcquired to true so that we can ignore any stale
        // releases that come in after the producer is disconnected. Otherwise, releaseBuffer will
        // decrement mNumAcquired for a buffer that was acquired before we reset mNumAcquired to
        // zero.
        for (auto& [releaseId, bufferItem] : bbq->mSubmitted) {
            bufferItem.mIsStale = true;
            bufferItem.disconnectedAfterAcquired = true;
        }

        return OK;
+8 −1
Original line number Diff line number Diff line
@@ -198,9 +198,16 @@ private:
    // latch stale buffers and that we don't wait on barriers from an old producer.
    uint32_t mProducerId = 0;

    class BLASTBufferItem : public BufferItem {
    public:
        // True if BBQBufferQueueProducer is disconnected after the buffer is acquried but
        // before it is released.
        bool disconnectedAfterAcquired{false};
    };

    // Keep a reference to the submitted buffers so we can release when surfaceflinger drops the
    // buffer or the buffer has been presented and a new buffer is ready to be presented.
    std::unordered_map<ReleaseCallbackId, BufferItem, ReleaseBufferCallbackIdHash> mSubmitted
    std::unordered_map<ReleaseCallbackId, BLASTBufferItem, ReleaseBufferCallbackIdHash> mSubmitted
            GUARDED_BY(mMutex);

    // Keep a queue of the released buffers instead of immediately releasing