diff options
author | Lajos Molnar <lajos@google.com> | 2013-05-03 14:50:50 -0700 |
---|---|---|
committer | Android (Google) Code Review <android-gerrit@google.com> | 2013-05-23 22:17:54 +0000 |
commit | c5d7b7d323bba8772a9005f7d300ad983a04733a (patch) | |
tree | 07479c25735c0bbaccd497db21a90a87c3926242 /libs/gui | |
parent | d837969640efbc97eda2034c7811dda807d4174f (diff) | |
download | frameworks_native-c5d7b7d323bba8772a9005f7d300ad983a04733a.zip frameworks_native-c5d7b7d323bba8772a9005f7d300ad983a04733a.tar.gz frameworks_native-c5d7b7d323bba8772a9005f7d300ad983a04733a.tar.bz2 |
BufferQueue: track buffer-queue by instance vs. by reference
Instead of representing the buffer-queue as a vector of buffer
indices, represent them as a vector of BufferItems (copies).
This allows modifying the buffer slots independent of the queued
buffers.
As part of this change, BufferSlot properties that are only
been relevant in the buffer-queue have been removed.
Also, invalid scalingMode in queueBuffer now returns an error.
ConsumerBase has also changed to allow reuse of the same
buffer slots by different buffers.
Change-Id: If2a698fa142b67c69ad41b8eaca6e127eb3ef75b
Signed-off-by: Lajos Molnar <lajos@google.com>
Related-to-bug: 7093648
Diffstat (limited to 'libs/gui')
-rw-r--r-- | libs/gui/BufferItemConsumer.cpp | 4 | ||||
-rw-r--r-- | libs/gui/BufferQueue.cpp | 198 | ||||
-rw-r--r-- | libs/gui/ConsumerBase.cpp | 47 | ||||
-rw-r--r-- | libs/gui/CpuConsumer.cpp | 4 | ||||
-rw-r--r-- | libs/gui/GLConsumer.cpp | 28 |
5 files changed, 193 insertions, 88 deletions
diff --git a/libs/gui/BufferItemConsumer.cpp b/libs/gui/BufferItemConsumer.cpp index 7db1b84..ba04bdf 100644 --- a/libs/gui/BufferItemConsumer.cpp +++ b/libs/gui/BufferItemConsumer.cpp @@ -82,9 +82,9 @@ status_t BufferItemConsumer::releaseBuffer(const BufferItem &item, Mutex::Autolock _l(mMutex); - err = addReleaseFenceLocked(item.mBuf, releaseFence); + err = addReleaseFenceLocked(item.mBuf, item.mGraphicBuffer, releaseFence); - err = releaseBufferLocked(item.mBuf, EGL_NO_DISPLAY, + err = releaseBufferLocked(item.mBuf, item.mGraphicBuffer, EGL_NO_DISPLAY, EGL_NO_SYNC_KHR); if (err != OK) { BI_LOGE("Failed to release buffer: %s (%d)", diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp index 942151f..34dbd71 100644 --- a/libs/gui/BufferQueue.cpp +++ b/libs/gui/BufferQueue.cpp @@ -418,6 +418,7 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp<Fence>* outFence, return NO_INIT; } + mSlots[*outBuf].mFrameNumber = ~0; mSlots[*outBuf].mGraphicBuffer = graphicBuffer; } } @@ -435,7 +436,8 @@ status_t BufferQueue::dequeueBuffer(int *outBuf, sp<Fence>* outFence, eglDestroySyncKHR(dpy, eglFence); } - ST_LOGV("dequeueBuffer: returning slot=%d buf=%p flags=%#x", *outBuf, + ST_LOGV("dequeueBuffer: returning slot=%d/%llu buf=%p flags=%#x", *outBuf, + mSlots[*outBuf].mFrameNumber, mSlots[*outBuf].mGraphicBuffer->handle, returnFlags); return returnFlags; @@ -491,15 +493,22 @@ status_t BufferQueue::queueBuffer(int buf, return BAD_VALUE; } - ST_LOGV("queueBuffer: slot=%d time=%#llx crop=[%d,%d,%d,%d] tr=%#x " - "scale=%s", - buf, timestamp, crop.left, crop.top, crop.right, crop.bottom, - transform, scalingModeName(scalingMode)); + switch (scalingMode) { + case NATIVE_WINDOW_SCALING_MODE_FREEZE: + case NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW: + case NATIVE_WINDOW_SCALING_MODE_SCALE_CROP: + case NATIVE_WINDOW_SCALING_MODE_NO_SCALE_CROP: + break; + default: + ST_LOGE("unknown scaling mode: %d", scalingMode); + return -EINVAL; + } sp<ConsumerListener> listener; { // scope for the lock Mutex::Autolock lock(mMutex); + if (mAbandoned) { ST_LOGE("queueBuffer: BufferQueue has been abandoned!"); return NO_INIT; @@ -519,6 +528,12 @@ status_t BufferQueue::queueBuffer(int buf, return -EINVAL; } + ST_LOGV("queueBuffer: slot=%d/%llu time=%#llx crop=[%d,%d,%d,%d] " + "tr=%#x scale=%s", + buf, mFrameCounter + 1, timestamp, + crop.left, crop.top, crop.right, crop.bottom, + transform, scalingModeName(scalingMode)); + const sp<GraphicBuffer>& graphicBuffer(mSlots[buf].mGraphicBuffer); Rect bufferRect(graphicBuffer->getWidth(), graphicBuffer->getHeight()); Rect croppedCrop; @@ -529,9 +544,25 @@ status_t BufferQueue::queueBuffer(int buf, return -EINVAL; } + mSlots[buf].mFence = fence; + mSlots[buf].mBufferState = BufferSlot::QUEUED; + mFrameCounter++; + mSlots[buf].mFrameNumber = mFrameCounter; + + BufferItem item; + item.mAcquireCalled = mSlots[buf].mAcquireCalled; + item.mGraphicBuffer = mSlots[buf].mGraphicBuffer; + item.mCrop = crop; + item.mTransform = transform; + item.mScalingMode = scalingMode; + item.mTimestamp = timestamp; + item.mFrameNumber = mFrameCounter; + item.mBuf = buf; + item.mFence = fence; + if (mSynchronousMode) { // In synchronous mode we queue all buffers in a FIFO. - mQueue.push_back(buf); + mQueue.push_back(item); // Synchronous mode always signals that an additional frame should // be consumed. @@ -539,7 +570,7 @@ status_t BufferQueue::queueBuffer(int buf, } else { // In asynchronous mode we only keep the most recent buffer. if (mQueue.empty()) { - mQueue.push_back(buf); + mQueue.push_back(item); // Asynchronous mode only signals that a frame should be // consumed if no previous frame was pending. If a frame were @@ -547,34 +578,15 @@ status_t BufferQueue::queueBuffer(int buf, listener = mConsumerListener; } else { Fifo::iterator front(mQueue.begin()); - // buffer currently queued is freed - mSlots[*front].mBufferState = BufferSlot::FREE; + // buffer slot currently queued is marked free if still tracked + if (stillTracking(front)) { + mSlots[front->mBuf].mBufferState = BufferSlot::FREE; + } // and we record the new buffer index in the queued list - *front = buf; + *front = item; } } - mSlots[buf].mTimestamp = timestamp; - mSlots[buf].mCrop = crop; - mSlots[buf].mTransform = transform; - mSlots[buf].mFence = fence; - - switch (scalingMode) { - case NATIVE_WINDOW_SCALING_MODE_FREEZE: - case NATIVE_WINDOW_SCALING_MODE_SCALE_TO_WINDOW: - case NATIVE_WINDOW_SCALING_MODE_SCALE_CROP: - break; - default: - ST_LOGE("unknown scaling mode: %d (ignoring)", scalingMode); - scalingMode = mSlots[buf].mScalingMode; - break; - } - - mSlots[buf].mBufferState = BufferSlot::QUEUED; - mSlots[buf].mScalingMode = scalingMode; - mFrameCounter++; - mSlots[buf].mFrameNumber = mFrameCounter; - mBufferHasBeenQueued = true; mDequeueCondition.broadcast(); @@ -718,7 +730,14 @@ void BufferQueue::dump(String8& result, const char* prefix) const { int fifoSize = 0; Fifo::const_iterator i(mQueue.begin()); while (i != mQueue.end()) { - fifo.appendFormat("%02d ", *i++); + fifo.appendFormat("%02d:%p crop=[%d,%d,%d,%d], " + "xform=0x%02x, time=%#llx, scale=%s\n", + i->mBuf, i->mGraphicBuffer.get(), + i->mCrop.left, i->mCrop.top, i->mCrop.right, + i->mCrop.bottom, i->mTransform, i->mTimestamp, + scalingModeName(i->mScalingMode) + ); + i++; fifoSize++; } @@ -746,14 +765,10 @@ void BufferQueue::dump(String8& result, const char* prefix) const { for (int i=0 ; i<maxBufferCount ; i++) { const BufferSlot& slot(mSlots[i]); result.appendFormat( - "%s%s[%02d] " - "state=%-8s, crop=[%d,%d,%d,%d], " - "xform=0x%02x, time=%#llx, scale=%s", + "%s%s[%02d:%p] state=%-8s", prefix, (slot.mBufferState == BufferSlot::ACQUIRED)?">":" ", i, - stateName(slot.mBufferState), - slot.mCrop.left, slot.mCrop.top, slot.mCrop.right, - slot.mCrop.bottom, slot.mTransform, slot.mTimestamp, - scalingModeName(slot.mScalingMode) + slot.mGraphicBuffer.get(), + stateName(slot.mBufferState) ); const sp<GraphicBuffer>& buf(slot.mGraphicBuffer); @@ -820,27 +835,27 @@ status_t BufferQueue::acquireBuffer(BufferItem *buffer) { // deep, while in synchronous mode we use the oldest buffer. if (!mQueue.empty()) { Fifo::iterator front(mQueue.begin()); - int buf = *front; - + int buf = front->mBuf; + *buffer = *front; ATRACE_BUFFER_INDEX(buf); - if (mSlots[buf].mAcquireCalled) { + ST_LOGV("acquireBuffer: acquiring { slot=%d/%llu, buffer=%p }", + front->mBuf, front->mFrameNumber, + front->mGraphicBuffer->handle); + // if front buffer still being tracked update slot state + if (stillTracking(front)) { + mSlots[buf].mAcquireCalled = true; + mSlots[buf].mNeedsCleanupOnRelease = false; + mSlots[buf].mBufferState = BufferSlot::ACQUIRED; + mSlots[buf].mFence = Fence::NO_FENCE; + } + + // If the buffer has previously been acquired by the consumer, set + // mGraphicBuffer to NULL to avoid unnecessarily remapping this + // buffer on the consumer side. + if (buffer->mAcquireCalled) { buffer->mGraphicBuffer = NULL; - } else { - buffer->mGraphicBuffer = mSlots[buf].mGraphicBuffer; } - buffer->mCrop = mSlots[buf].mCrop; - buffer->mTransform = mSlots[buf].mTransform; - buffer->mScalingMode = mSlots[buf].mScalingMode; - buffer->mFrameNumber = mSlots[buf].mFrameNumber; - buffer->mTimestamp = mSlots[buf].mTimestamp; - buffer->mBuf = buf; - buffer->mFence = mSlots[buf].mFence; - - mSlots[buf].mAcquireCalled = true; - mSlots[buf].mNeedsCleanupOnRelease = false; - mSlots[buf].mBufferState = BufferSlot::ACQUIRED; - mSlots[buf].mFence = Fence::NO_FENCE; mQueue.erase(front); mDequeueCondition.broadcast(); @@ -853,7 +868,8 @@ status_t BufferQueue::acquireBuffer(BufferItem *buffer) { return NO_ERROR; } -status_t BufferQueue::releaseBuffer(int buf, EGLDisplay display, +status_t BufferQueue::releaseBuffer( + int buf, uint64_t frameNumber, EGLDisplay display, EGLSyncKHR eglFence, const sp<Fence>& fence) { ATRACE_CALL(); ATRACE_BUFFER_INDEX(buf); @@ -864,12 +880,35 @@ status_t BufferQueue::releaseBuffer(int buf, EGLDisplay display, return BAD_VALUE; } - mSlots[buf].mEglDisplay = display; - mSlots[buf].mEglFence = eglFence; - mSlots[buf].mFence = fence; + // Check if this buffer slot is on the queue + bool slotQueued = false; + Fifo::iterator front(mQueue.begin()); + while (front != mQueue.end() && !slotQueued) { + if (front->mBuf == buf) + slotQueued = true; + front++; + } + + // If the frame number has changed because buffer has been reallocated, + // we can ignore this releaseBuffer for the old buffer. + if (frameNumber != mSlots[buf].mFrameNumber) { + // This should only occur if new buffer is still in the queue + ALOGE_IF(!slotQueued, + "received old buffer(#%lld) after new buffer(#%lld) on same " + "slot #%d already acquired", frameNumber, + mSlots[buf].mFrameNumber, buf); + return STALE_BUFFER_SLOT; + } + // this should never happen + ALOGE_IF(slotQueued, + "received new buffer(#%lld) on slot #%d that has not yet been " + "acquired", frameNumber, buf); // The buffer can now only be released if its in the acquired state if (mSlots[buf].mBufferState == BufferSlot::ACQUIRED) { + mSlots[buf].mEglDisplay = display; + mSlots[buf].mEglFence = eglFence; + mSlots[buf].mFence = fence; mSlots[buf].mBufferState = BufferSlot::FREE; } else if (mSlots[buf].mNeedsCleanupOnRelease) { ST_LOGV("releasing a stale buf %d its state was %d", buf, mSlots[buf].mBufferState); @@ -934,6 +973,17 @@ status_t BufferQueue::getReleasedBuffers(uint32_t* slotMask) { mask |= 1 << i; } } + + // Remove buffers in flight (on the queue) from the mask where acquire has + // been called, as the consumer will not receive the buffer address, so + // it should not free these slots. + Fifo::iterator front(mQueue.begin()); + while (front != mQueue.end()) { + if (front->mAcquireCalled) + mask &= ~(1 << front->mBuf); + front++; + } + *slotMask = mask; ST_LOGV("getReleasedBuffers: returning mask %#x", mask); @@ -977,16 +1027,14 @@ status_t BufferQueue::setMaxAcquiredBufferCount(int maxAcquiredBuffers) { } void BufferQueue::freeAllBuffersExceptHeadLocked() { - int head = -1; - if (!mQueue.empty()) { - Fifo::iterator front(mQueue.begin()); - head = *front; - } + // only called if mQueue is not empty + Fifo::iterator front(mQueue.begin()); mBufferHasBeenQueued = false; for (int i = 0; i < NUM_BUFFER_SLOTS; i++) { - if (i != head) { + const BufferSlot &slot = mSlots[i]; + if (slot.mGraphicBuffer == NULL || + slot.mGraphicBuffer->handle != front->mGraphicBuffer->handle) freeBufferLocked(i); - } } } @@ -1052,6 +1100,22 @@ int BufferQueue::getMaxBufferCountLocked() const { return maxBufferCount; } +bool BufferQueue::stillTracking(const BufferItem *item) const { + const BufferSlot &slot = mSlots[item->mBuf]; + + ST_LOGV("stillTracking?: item: { slot=%d/%llu, buffer=%p }, " + "slot: { slot=%d/%llu, buffer=%p }", + item->mBuf, item->mFrameNumber, + (item->mGraphicBuffer.get() ? item->mGraphicBuffer->handle : 0), + item->mBuf, slot.mFrameNumber, + (slot.mGraphicBuffer.get() ? slot.mGraphicBuffer->handle : 0)); + + // Compare item with its original buffer slot. We can check the slot + // as the buffer would not be moved to a different slot by the producer. + return (slot.mGraphicBuffer != NULL && + item->mGraphicBuffer->handle == slot.mGraphicBuffer->handle); +} + BufferQueue::ProxyConsumerListener::ProxyConsumerListener( const wp<BufferQueue::ConsumerListener>& consumerListener): mConsumerListener(consumerListener) {} diff --git a/libs/gui/ConsumerBase.cpp b/libs/gui/ConsumerBase.cpp index 8d911c9..fd9d153 100644 --- a/libs/gui/ConsumerBase.cpp +++ b/libs/gui/ConsumerBase.cpp @@ -95,6 +95,7 @@ void ConsumerBase::freeBufferLocked(int slotIndex) { CB_LOGV("freeBufferLocked: slotIndex=%d", slotIndex); mSlots[slotIndex].mGraphicBuffer = 0; mSlots[slotIndex].mFence = Fence::NO_FENCE; + mSlots[slotIndex].mFrameNumber = 0; } // Used for refactoring, should not be in final interface @@ -191,21 +192,31 @@ status_t ConsumerBase::acquireBufferLocked(BufferQueue::BufferItem *item) { mSlots[item->mBuf].mGraphicBuffer = item->mGraphicBuffer; } + mSlots[item->mBuf].mFrameNumber = item->mFrameNumber; mSlots[item->mBuf].mFence = item->mFence; - CB_LOGV("acquireBufferLocked: -> slot=%d", item->mBuf); + CB_LOGV("acquireBufferLocked: -> slot=%d/%llu", + item->mBuf, item->mFrameNumber); return OK; } -status_t ConsumerBase::addReleaseFence(int slot, const sp<Fence>& fence) { +status_t ConsumerBase::addReleaseFence(int slot, + const sp<GraphicBuffer> graphicBuffer, const sp<Fence>& fence) { Mutex::Autolock lock(mMutex); - return addReleaseFenceLocked(slot, fence); + return addReleaseFenceLocked(slot, graphicBuffer, fence); } -status_t ConsumerBase::addReleaseFenceLocked(int slot, const sp<Fence>& fence) { +status_t ConsumerBase::addReleaseFenceLocked(int slot, + const sp<GraphicBuffer> graphicBuffer, const sp<Fence>& fence) { CB_LOGV("addReleaseFenceLocked: slot=%d", slot); + // If consumer no longer tracks this graphicBuffer, we can safely + // drop this fence, as it will never be received by the producer. + if (!stillTracking(slot, graphicBuffer)) { + return OK; + } + if (!mSlots[slot].mFence.get()) { mSlots[slot].mFence = fence; } else { @@ -225,11 +236,20 @@ status_t ConsumerBase::addReleaseFenceLocked(int slot, const sp<Fence>& fence) { return OK; } -status_t ConsumerBase::releaseBufferLocked(int slot, EGLDisplay display, - EGLSyncKHR eglFence) { - CB_LOGV("releaseBufferLocked: slot=%d", slot); - status_t err = mBufferQueue->releaseBuffer(slot, display, eglFence, - mSlots[slot].mFence); +status_t ConsumerBase::releaseBufferLocked( + int slot, const sp<GraphicBuffer> graphicBuffer, + EGLDisplay display, EGLSyncKHR eglFence) { + // If consumer no longer tracks this graphicBuffer (we received a new + // buffer on the same slot), the buffer producer is definitely no longer + // tracking it. + if (!stillTracking(slot, graphicBuffer)) { + return OK; + } + + CB_LOGV("releaseBufferLocked: slot=%d/%llu", + slot, mSlots[slot].mFrameNumber); + status_t err = mBufferQueue->releaseBuffer(slot, mSlots[slot].mFrameNumber, + display, eglFence, mSlots[slot].mFence); if (err == BufferQueue::STALE_BUFFER_SLOT) { freeBufferLocked(slot); } @@ -239,4 +259,13 @@ status_t ConsumerBase::releaseBufferLocked(int slot, EGLDisplay display, return err; } +bool ConsumerBase::stillTracking(int slot, + const sp<GraphicBuffer> graphicBuffer) { + if (slot < 0 || slot >= BufferQueue::NUM_BUFFER_SLOTS) { + return false; + } + return (mSlots[slot].mGraphicBuffer != NULL && + mSlots[slot].mGraphicBuffer->handle == graphicBuffer->handle); +} + } // namespace android diff --git a/libs/gui/CpuConsumer.cpp b/libs/gui/CpuConsumer.cpp index 0543649..0834361 100644 --- a/libs/gui/CpuConsumer.cpp +++ b/libs/gui/CpuConsumer.cpp @@ -189,7 +189,9 @@ status_t CpuConsumer::releaseAcquiredBufferLocked(int lockedIdx) { // disconnected after this buffer was acquired. if (CC_LIKELY(mAcquiredBuffers[lockedIdx].mGraphicBuffer == mSlots[buf].mGraphicBuffer)) { - releaseBufferLocked(buf, EGL_NO_DISPLAY, EGL_NO_SYNC_KHR); + releaseBufferLocked( + buf, mAcquiredBuffers[lockedIdx].mGraphicBuffer, + EGL_NO_DISPLAY, EGL_NO_SYNC_KHR); } AcquiredBuffer &ab = mAcquiredBuffers.editItemAt(lockedIdx); diff --git a/libs/gui/GLConsumer.cpp b/libs/gui/GLConsumer.cpp index 344a93a..6d29edc 100644 --- a/libs/gui/GLConsumer.cpp +++ b/libs/gui/GLConsumer.cpp @@ -188,12 +188,16 @@ status_t GLConsumer::acquireBufferLocked(BufferQueue::BufferItem *item) { return NO_ERROR; } -status_t GLConsumer::releaseBufferLocked(int buf, EGLDisplay display, - EGLSyncKHR eglFence) { - status_t err = ConsumerBase::releaseBufferLocked(buf, display, eglFence); - +status_t GLConsumer::releaseBufferLocked(int buf, + sp<GraphicBuffer> graphicBuffer, + EGLDisplay display, EGLSyncKHR eglFence) { + // release the buffer if it hasn't already been discarded by the + // BufferQueue. This can happen, for example, when the producer of this + // buffer has reallocated the original buffer slot after this buffer + // was acquired. + status_t err = ConsumerBase::releaseBufferLocked( + buf, graphicBuffer, display, eglFence); mEglSlots[buf].mEglFence = EGL_NO_SYNC_KHR; - return err; } @@ -237,7 +241,10 @@ status_t GLConsumer::releaseAndUpdateLocked(const BufferQueue::BufferItem& item) if (err != NO_ERROR) { // Release the buffer we just acquired. It's not safe to // release the old buffer, so instead we just drop the new frame. - releaseBufferLocked(buf, mEglDisplay, EGL_NO_SYNC_KHR); + // As we are still under lock since acquireBuffer, it is safe to + // release by slot. + releaseBufferLocked(buf, mSlots[buf].mGraphicBuffer, + mEglDisplay, EGL_NO_SYNC_KHR); return err; } @@ -248,7 +255,8 @@ status_t GLConsumer::releaseAndUpdateLocked(const BufferQueue::BufferItem& item) // release old buffer if (mCurrentTexture != BufferQueue::INVALID_BUFFER_SLOT) { - status_t status = releaseBufferLocked(mCurrentTexture, mEglDisplay, + status_t status = releaseBufferLocked( + mCurrentTexture, mCurrentTextureBuf, mEglDisplay, mEglSlots[mCurrentTexture].mEglFence); if (status != NO_ERROR && status != BufferQueue::STALE_BUFFER_SLOT) { ST_LOGE("releaseAndUpdate: failed to release buffer: %s (%d)", @@ -334,7 +342,8 @@ status_t GLConsumer::checkAndUpdateEglStateLocked() { void GLConsumer::setReleaseFence(const sp<Fence>& fence) { if (fence->isValid() && mCurrentTexture != BufferQueue::INVALID_BUFFER_SLOT) { - status_t err = addReleaseFence(mCurrentTexture, fence); + status_t err = addReleaseFence(mCurrentTexture, + mCurrentTextureBuf, fence); if (err != OK) { ST_LOGE("setReleaseFence: failed to add the fence: %s (%d)", strerror(-err), err); @@ -503,7 +512,8 @@ status_t GLConsumer::syncForReleaseLocked(EGLDisplay dpy) { return UNKNOWN_ERROR; } sp<Fence> fence(new Fence(fenceFd)); - status_t err = addReleaseFenceLocked(mCurrentTexture, fence); + status_t err = addReleaseFenceLocked(mCurrentTexture, + mCurrentTextureBuf, fence); if (err != OK) { ST_LOGE("syncForReleaseLocked: error adding release fence: " "%s (%d)", strerror(-err), err); |