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

Commit ecc50404 authored by Dan Stoza's avatar Dan Stoza
Browse files

SurfaceFlinger: Fix PTS on stale buffers

SurfaceFlinger's (Layer's) shadow copy of the BufferQueue queue was
getting out of sync for a few reasons. This change fixes these by
doing the following:

- Adds a check to re-synchronize the shadow copy every time we
  successfully acquire a buffer by first dropping stale buffers before
  removing the current buffer.
- Avoids trying to perform updates for buffers which have been rejected
  (for incorrect dimensions) by SurfaceFlinger.
- Adds IGraphicBufferConsumer::setShadowQueueSize, which allows the
  consumer to notify the BufferQueue that it is maintaining a shadow
  copy of the queue and prevents it from dropping so many buffers
  during acquireBuffer that it ends up returning a buffer for which the
  consumer has not yet received an onFrameAvailable call.

Bug: 20096136
Change-Id: I78d0738428005fc19b3be85cc8f1db498043612f
(cherry picked from commit 2e36f228)
parent 7b2fc930
Loading
Loading
Loading
Loading
+3 −0
Original line number Diff line number Diff line
@@ -148,6 +148,9 @@ public:
    // Retrieve the sideband buffer stream, if any.
    virtual sp<NativeHandle> getSidebandStream() const;

    // See IGraphicBufferConsumer::setShadowQueueSize
    virtual void setShadowQueueSize(size_t size);

    // dump our state in a String
    virtual void dump(String8& result, const char* prefix) const;

+7 −0
Original line number Diff line number Diff line
@@ -274,6 +274,13 @@ private:
    // mBufferAge tracks the age of the contents of the most recently dequeued
    // buffer as the number of frames that have elapsed since it was last queued
    uint64_t mBufferAge;

    // mConsumerHasShadowQueue determines if acquireBuffer should be more
    // cautious about dropping buffers so that it always returns a buffer that
    // is represented in the consumer's shadow queue.
    bool mConsumerHasShadowQueue;
    size_t mConsumerShadowQueueSize;

}; // class BufferQueueCore

} // namespace android
+5 −0
Original line number Diff line number Diff line
@@ -248,6 +248,11 @@ public:
    // Retrieve the sideband buffer stream, if any.
    virtual sp<NativeHandle> getSidebandStream() const = 0;

    // setShadowQueueSize notifies the BufferQueue that the consumer is
    // shadowing its queue and allows it to limit the number of buffers it is
    // permitted to drop during acquire so as to not get out of sync.
    virtual void setShadowQueueSize(size_t size) = 0;

    // dump state into a string
    virtual void dump(String8& result, const char* prefix) const = 0;

+22 −0
Original line number Diff line number Diff line
@@ -89,7 +89,20 @@ status_t BufferQueueConsumer::acquireBuffer(BufferItem* outBuffer,
        // the timestamps are being auto-generated by Surface. If the app isn't
        // generating timestamps explicitly, it probably doesn't want frames to
        // be discarded based on them.
        //
        // If the consumer is shadowing our queue, we also make sure that we
        // don't drop so many buffers that the consumer hasn't received the
        // onFrameAvailable callback for the buffer it acquires. That is, we
        // want the buffer we return to be in the consumer's shadow queue.
        size_t droppableBuffers = mCore->mConsumerShadowQueueSize > 1 ?
                mCore->mConsumerShadowQueueSize - 1 : 0;
        while (mCore->mQueue.size() > 1 && !mCore->mQueue[0].mIsAutoTimestamp) {
            if (mCore->mConsumerHasShadowQueue && droppableBuffers == 0) {
                BQ_LOGV("acquireBuffer: no droppable buffers in consumer's"
                        " shadow queue, continuing");
                break;
            }

            // If entry[1] is timely, drop entry[0] (and repeat). We apply an
            // additional criterion here: we only drop the earlier buffer if our
            // desiredPresent falls within +/- 1 second of the expected present.
@@ -124,6 +137,7 @@ status_t BufferQueueConsumer::acquireBuffer(BufferItem* outBuffer,
            }
            mCore->mQueue.erase(front);
            front = mCore->mQueue.begin();
            --droppableBuffers;
        }

        // See if the front buffer is due
@@ -537,6 +551,14 @@ sp<NativeHandle> BufferQueueConsumer::getSidebandStream() const {
    return mCore->mSidebandStream;
}

void BufferQueueConsumer::setShadowQueueSize(size_t size) {
    ATRACE_CALL();
    BQ_LOGV("setShadowQueueSize: %zu", size);
    Mutex::Autolock lock(mCore->mMutex);
    mCore->mConsumerHasShadowQueue = true;
    mCore->mConsumerShadowQueueSize = size;
}

void BufferQueueConsumer::dump(String8& result, const char* prefix) const {
    mCore->dump(result, prefix);
}
+3 −1
Original line number Diff line number Diff line
@@ -71,7 +71,9 @@ BufferQueueCore::BufferQueueCore(const sp<IGraphicBufferAlloc>& allocator) :
    mIsAllocating(false),
    mIsAllocatingCondition(),
    mAllowAllocation(true),
    mBufferAge(0)
    mBufferAge(0),
    mConsumerHasShadowQueue(false),
    mConsumerShadowQueueSize(0)
{
    if (allocator == NULL) {
        sp<ISurfaceComposer> composer(ComposerService::getComposerService());
Loading