GraphicBufferSource fixes

Various fixes:
- Set the maximum number of BQ buffers we're allowed to
  acquire equal to the actual number of codec buffers.  That
  way we keep the codec as full as possible, and never try to
  acquire more than we're allowed from the BufferQueue.
- Actually use "end of stream sent" flag.
- Name the BufferQueue (for debug messages).

Bug 8359403

Change-Id: I3b8c1f679bbebf6a89e623e13ca029eda7f657ba

diff --git a/media/libstagefright/omx/GraphicBufferSource.cpp b/media/libstagefright/omx/GraphicBufferSource.cpp
index 211e1d1..3854e52 100644 (file)
--- a/media/libstagefright/omx/GraphicBufferSource.cpp
+++ b/media/libstagefright/omx/GraphicBufferSource.cpp
@@ -32,7 +32,7 @@ static const bool EXTRA_CHECK = true;
 
 
 GraphicBufferSource::GraphicBufferSource(OMXNodeInstance* nodeInstance,
-        uint32_t bufferWidth, uint32_t bufferHeight) :
+        uint32_t bufferWidth, uint32_t bufferHeight, uint32_t bufferCount) :
     mInitCheck(UNKNOWN_ERROR),
     mNodeInstance(nodeInstance),
     mExecuting(false),
@@ -40,20 +40,31 @@ GraphicBufferSource::GraphicBufferSource(OMXNodeInstance* nodeInstance,
     mEndOfStream(false),
     mEndOfStreamSent(false) {
 
-    ALOGV("GraphicBufferSource w=%u h=%u", bufferWidth, bufferHeight);
+    ALOGV("GraphicBufferSource w=%u h=%u c=%u",
+            bufferWidth, bufferHeight, bufferCount);
 
     if (bufferWidth == 0 || bufferHeight == 0) {
-        ALOGE("Invalid dimensions %dx%d", bufferWidth, bufferHeight);
+        ALOGE("Invalid dimensions %ux%u", bufferWidth, bufferHeight);
         mInitCheck = BAD_VALUE;
         return;
     }
 
+    String8 name("GraphicBufferSource");
+
     mBufferQueue = new BufferQueue(true);
+    mBufferQueue->setConsumerName(name);
     mBufferQueue->setDefaultBufferSize(bufferWidth, bufferHeight);
     mBufferQueue->setSynchronousMode(true);
     mBufferQueue->setConsumerUsageBits(GRALLOC_USAGE_HW_VIDEO_ENCODER |
             GRALLOC_USAGE_HW_TEXTURE);
 
+    mInitCheck = mBufferQueue->setMaxAcquiredBufferCount(bufferCount);
+    if (mInitCheck != NO_ERROR) {
+        ALOGE("Unable to set BQ max acquired buffer count to %u: %d",
+                bufferCount, mInitCheck);
+        return;
+    }
+
     // Note that we can't create an sp<...>(this) in a ctor that will not keep a
     // reference once the ctor ends, as that would cause the refcount of 'this'
     // dropping to 0 at the end of the ctor.  Since all we need is a wp<...>
@@ -64,21 +75,23 @@ GraphicBufferSource::GraphicBufferSource(OMXNodeInstance* nodeInstance,
     sp<BufferQueue::ConsumerListener> proxy;
     proxy = new BufferQueue::ProxyConsumerListener(listener);
 
-    status_t err = mBufferQueue->consumerConnect(proxy);
-    if (err != NO_ERROR) {
+    mInitCheck = mBufferQueue->consumerConnect(proxy);
+    if (mInitCheck != NO_ERROR) {
         ALOGE("Error connecting to BufferQueue: %s (%d)",
-                strerror(-err), err);
+                strerror(-mInitCheck), mInitCheck);
         return;
     }
 
-    mInitCheck = OK;
+    CHECK(mInitCheck == NO_ERROR);
 }
 
 GraphicBufferSource::~GraphicBufferSource() {
     ALOGV("~GraphicBufferSource");
-    status_t err = mBufferQueue->consumerDisconnect();
-    if (err != NO_ERROR) {
-        ALOGW("consumerDisconnect failed: %d", err);
+    if (mBufferQueue != NULL) {
+        status_t err = mBufferQueue->consumerDisconnect();
+        if (err != NO_ERROR) {
+            ALOGW("consumerDisconnect failed: %d", err);
+        }
     }
 }
 
@@ -98,8 +111,12 @@ void GraphicBufferSource::omxExecuting() {
     // one codec buffer simultaneously.  (We could instead try to submit
     // all BQ buffers whenever any codec buffer is freed, but if we get the
     // initial conditions right that will never be useful.)
-    while (mNumFramesAvailable && isCodecBufferAvailable_l()) {
-        fillCodecBuffer_l();
+    while (mNumFramesAvailable) {
+        if (!fillCodecBuffer_l()) {
+            ALOGV("stop load with frames available (codecAvail=%d)",
+                    isCodecBufferAvailable_l());
+            break;
+        }
     }
 
     ALOGV("done loading initial frames, avail=%d", mNumFramesAvailable);
@@ -166,7 +183,7 @@ void GraphicBufferSource::codecBufferEmptied(OMX_BUFFERHEADERTYPE* header) {
     // see if the GraphicBuffer reference was null, which should only ever
     // happen for EOS.
     if (codecBuffer.mGraphicBuffer == NULL) {
-        CHECK(mEndOfStream);
+        CHECK(mEndOfStream && mEndOfStreamSent);
         // No GraphicBuffer to deal with, no additional input or output is
         // expected, so just return.
         return;
@@ -216,8 +233,9 @@ void GraphicBufferSource::codecBufferEmptied(OMX_BUFFERHEADERTYPE* header) {
 
     if (mNumFramesAvailable) {
         // Fill this codec buffer.
-        CHECK(!mEndOfStream);
-        ALOGV("buffer freed, %d frames avail", mNumFramesAvailable);
+        CHECK(!mEndOfStreamSent);
+        ALOGV("buffer freed, %d frames avail (eos=%d)",
+                mNumFramesAvailable, mEndOfStream);
         fillCodecBuffer_l();
     } else if (mEndOfStream) {
         // No frames available, but EOS is pending, so use this buffer to
@@ -228,56 +246,58 @@ void GraphicBufferSource::codecBufferEmptied(OMX_BUFFERHEADERTYPE* header) {
     return;
 }
 
-status_t GraphicBufferSource::fillCodecBuffer_l() {
+bool GraphicBufferSource::fillCodecBuffer_l() {
     CHECK(mExecuting && mNumFramesAvailable > 0);
+
     int cbi = findAvailableCodecBuffer_l();
     if (cbi < 0) {
         // No buffers available, bail.
         ALOGV("fillCodecBuffer_l: no codec buffers, avail now %d",
                 mNumFramesAvailable);
-    } else {
-        ALOGV("fillCodecBuffer_l: acquiring buffer, avail=%d",
-                mNumFramesAvailable);
-        BufferQueue::BufferItem item;
-        status_t err = mBufferQueue->acquireBuffer(&item);
-        if (err == BufferQueue::NO_BUFFER_AVAILABLE) {
-            // shouldn't happen
-            ALOGW("fillCodecBuffer_l: frame was not available");
-            return err;
-        } else if (err != OK) {
-            // now what? fake end-of-stream?
-            ALOGW("fillCodecBuffer_l: acquireBuffer returned err=%d", err);
-            return err;
-        }
+        return false;
+    }
 
-        mNumFramesAvailable--;
+    ALOGV("fillCodecBuffer_l: acquiring buffer, avail=%d",
+            mNumFramesAvailable);
+    BufferQueue::BufferItem item;
+    status_t err = mBufferQueue->acquireBuffer(&item);
+    if (err == BufferQueue::NO_BUFFER_AVAILABLE) {
+        // shouldn't happen
+        ALOGW("fillCodecBuffer_l: frame was not available");
+        return false;
+    } else if (err != OK) {
+        // now what? fake end-of-stream?
+        ALOGW("fillCodecBuffer_l: acquireBuffer returned err=%d", err);
+        return false;
+    }
 
-        // Wait for it to become available.
-        err = item.mFence->waitForever(1000,
-                "GraphicBufferSource::fillCodecBuffer_l");
-        if (err != OK) {
-            ALOGW("failed to wait for buffer fence: %d", err);
-            // keep going
-        }
+    mNumFramesAvailable--;
 
-        // If this is the first time we're seeing this buffer, add it to our
-        // slot table.
-        if (item.mGraphicBuffer != NULL) {
-            ALOGV("fillCodecBuffer_l: setting mBufferSlot %d", item.mBuf);
-            mBufferSlot[item.mBuf] = item.mGraphicBuffer;
-        }
+    // Wait for it to become available.
+    err = item.mFence->waitForever(1000,
+            "GraphicBufferSource::fillCodecBuffer_l");
+    if (err != OK) {
+        ALOGW("failed to wait for buffer fence: %d", err);
+        // keep going
+    }
 
-        err = submitBuffer_l(mBufferSlot[item.mBuf], item.mTimestamp, cbi);
-        if (err != OK) {
-            ALOGV("submitBuffer_l failed, releasing bq buf %d", item.mBuf);
-            mBufferQueue->releaseBuffer(item.mBuf, EGL_NO_DISPLAY,
+    // If this is the first time we're seeing this buffer, add it to our
+    // slot table.
+    if (item.mGraphicBuffer != NULL) {
+        ALOGV("fillCodecBuffer_l: setting mBufferSlot %d", item.mBuf);
+        mBufferSlot[item.mBuf] = item.mGraphicBuffer;
+    }
+
+    err = submitBuffer_l(mBufferSlot[item.mBuf], item.mTimestamp, cbi);
+    if (err != OK) {
+        ALOGV("submitBuffer_l failed, releasing bq buf %d", item.mBuf);
+        mBufferQueue->releaseBuffer(item.mBuf, EGL_NO_DISPLAY,
                 EGL_NO_SYNC_KHR, Fence::NO_FENCE);
-        } else {
-            ALOGV("buffer submitted (bq %d, cbi %d)", item.mBuf, cbi);
-        }
+    } else {
+        ALOGV("buffer submitted (bq %d, cbi %d)", item.mBuf, cbi);
     }
 
-    return OK;
+    return true;
 }
 
 status_t GraphicBufferSource::signalEndOfInputStream() {
@@ -372,6 +392,7 @@ void GraphicBufferSource::submitEndOfInputStream_l() {
     } else {
         ALOGV("submitEndOfInputStream_l: buffer submitted, header=%p cbi=%d",
                 header, cbi);
+        mEndOfStreamSent = true;
     }
 }
 
@@ -400,7 +421,8 @@ int GraphicBufferSource::findMatchingCodecBuffer_l(
 void GraphicBufferSource::onFrameAvailable() {
     Mutex::Autolock autoLock(mMutex);
 
-    ALOGV("onFrameAvailable exec=%d avail=%d", mExecuting, mNumFramesAvailable);
+    ALOGV("onFrameAvailable exec=%d avail=%d",
+            mExecuting, mNumFramesAvailable);
 
     if (mEndOfStream) {
         // This should only be possible if a new buffer was queued after

diff --git a/media/libstagefright/omx/GraphicBufferSource.h b/media/libstagefright/omx/GraphicBufferSource.h
index 6a34bc5..7f1f22e 100644 (file)
--- a/media/libstagefright/omx/GraphicBufferSource.h
+++ b/media/libstagefright/omx/GraphicBufferSource.h
@@ -47,7 +47,7 @@ namespace android {
 class GraphicBufferSource : public BufferQueue::ConsumerListener {
 public:
     GraphicBufferSource(OMXNodeInstance* nodeInstance,
-            uint32_t bufferWidth, uint32_t bufferHeight);
+            uint32_t bufferWidth, uint32_t bufferHeight, uint32_t bufferCount);
     virtual ~GraphicBufferSource();
 
     // We can't throw an exception if the constructor fails, so we just set
@@ -124,7 +124,9 @@ private:
     // in the onFrameAvailable callback, or if we're in codecBufferEmptied
     // and mNumFramesAvailable is nonzero).  Returns without doing anything if
     // we don't have a codec buffer available.
-    status_t fillCodecBuffer_l();
+    //
+    // Returns true if we successfully filled a codec buffer with a BQ buffer.
+    bool fillCodecBuffer_l();
 
     // Marks the mCodecBuffers entry as in-use, copies the GraphicBuffer
     // reference into the codec buffer, and submits the data to the codec.

diff --git a/media/libstagefright/omx/OMXNodeInstance.cpp b/media/libstagefright/omx/OMXNodeInstance.cpp
index f3d8d14..46ff22f 100644 (file)
--- a/media/libstagefright/omx/OMXNodeInstance.cpp
+++ b/media/libstagefright/omx/OMXNodeInstance.cpp
@@ -590,7 +590,8 @@ status_t OMXNodeInstance::createInputSurface(
     }
 
     GraphicBufferSource* bufferSource = new GraphicBufferSource(
-            this, def.format.video.nFrameWidth, def.format.video.nFrameHeight);
+            this, def.format.video.nFrameWidth, def.format.video.nFrameHeight,
+            def.nBufferCountActual);
     if ((err = bufferSource->initCheck()) != OK) {
         delete bufferSource;
         return err;