When freeing an OMX node, attempt to transition it from its current state all the way to "Loaded" in order to properly free any allocated buffers.

3 files changed, 100 insertions, 1 deletions

diff --git a/media/libstagefright/include/OMXNodeInstance.h b/media/libstagefright/include/OMXNodeInstance.h
index 181742e..09a8816 100644
--- a/media/libstagefright/include/OMXNodeInstance.h
+++ b/media/libstagefright/include/OMXNodeInstance.h
@@ -85,8 +85,18 @@ private:
     OMX_HANDLETYPE mHandle;
     sp<IOMXObserver> mObserver;
 
+    struct ActiveBuffer {
+        OMX_U32 mPortIndex;
+        OMX::buffer_id mID;
+    };
+    Vector<ActiveBuffer> mActiveBuffers;
+
     ~OMXNodeInstance();
 
+    void addActiveBuffer(OMX_U32 portIndex, OMX::buffer_id id);
+    void removeActiveBuffer(OMX_U32 portIndex, OMX::buffer_id id);
+    void freeActiveBuffers();
+
     static OMX_ERRORTYPE OnEvent(
             OMX_IN OMX_HANDLETYPE hComponent,
             OMX_IN OMX_PTR pAppData,
diff --git a/media/libstagefright/omx/OMX.cpp b/media/libstagefright/omx/OMX.cpp
index 3800a26..e361018 100644
--- a/media/libstagefright/omx/OMX.cpp
+++ b/media/libstagefright/omx/OMX.cpp
@@ -98,7 +98,7 @@ void OMX::CallbackDispatcher::post(const omx_message &msg) {
 void OMX::CallbackDispatcher::dispatch(const omx_message &msg) {
     OMXNodeInstance *instance = mOwner->findInstance(msg.node);
     if (instance == NULL) {
-        LOGW("Would have dispatched a message to a node that's already gone.");
+        LOGV("Would have dispatched a message to a node that's already gone.");
         return;
     }
     instance->onMessage(msg);
diff --git a/media/libstagefright/omx/OMXNodeInstance.cpp b/media/libstagefright/omx/OMXNodeInstance.cpp
index d5d45d7..ab8be53 100644
--- a/media/libstagefright/omx/OMXNodeInstance.cpp
+++ b/media/libstagefright/omx/OMXNodeInstance.cpp
@@ -107,6 +107,55 @@ static status_t StatusFromOMXError(OMX_ERRORTYPE err) {
 }
 
 status_t OMXNodeInstance::freeNode() {
+    // Transition the node from its current state all the way down
+    // to "Loaded".
+    // This ensures that all active buffers are properly freed even
+    // for components that don't do this themselves on a call to
+    // "FreeHandle".
+
+    OMX_STATETYPE state;
+    CHECK_EQ(OMX_GetState(mHandle, &state), OMX_ErrorNone);
+    switch (state) {
+        case OMX_StateExecuting:
+        {
+            LOGV("forcing Executing->Idle");
+            sendCommand(OMX_CommandStateSet, OMX_StateIdle);
+            OMX_ERRORTYPE err;
+            while ((err = OMX_GetState(mHandle, &state)) == OMX_ErrorNone
+                   && state != OMX_StateIdle) {
+                usleep(100000);
+            }
+            CHECK_EQ(err, OMX_ErrorNone);
+
+            // fall through
+        }
+
+        case OMX_StateIdle:
+        {
+            LOGV("forcing Idle->Loaded");
+            sendCommand(OMX_CommandStateSet, OMX_StateLoaded);
+
+            freeActiveBuffers();
+
+            OMX_ERRORTYPE err;
+            while ((err = OMX_GetState(mHandle, &state)) == OMX_ErrorNone
+                   && state != OMX_StateLoaded) {
+                LOGV("waiting for Loaded state...");
+                usleep(100000);
+            }
+            CHECK_EQ(err, OMX_ErrorNone);
+
+            // fall through
+        }
+
+        case OMX_StateLoaded:
+            break;
+
+        default:
+            CHECK(!"should not be here, unknown state.");
+            break;
+    }
+
     OMX_ERRORTYPE err = OMX_MasterFreeHandle(mHandle);
     mHandle = NULL;
 
@@ -193,6 +242,8 @@ status_t OMXNodeInstance::useBuffer(
 
     *buffer = header;
 
+    addActiveBuffer(portIndex, *buffer);
+
     return OK;
 }
 
@@ -220,6 +271,8 @@ status_t OMXNodeInstance::allocateBuffer(
 
     *buffer = header;
 
+    addActiveBuffer(portIndex, *buffer);
+
     return OK;
 }
 
@@ -248,6 +301,8 @@ status_t OMXNodeInstance::allocateBufferWithBackup(
 
     *buffer = header;
 
+    addActiveBuffer(portIndex, *buffer);
+
     return OK;
 }
 
@@ -255,6 +310,8 @@ status_t OMXNodeInstance::freeBuffer(
         OMX_U32 portIndex, OMX::buffer_id buffer) {
     Mutex::Autolock autoLock(mLock);
 
+    removeActiveBuffer(portIndex, buffer);
+
     OMX_BUFFERHEADERTYPE *header = (OMX_BUFFERHEADERTYPE *)buffer;
     BufferMeta *buffer_meta = static_cast<BufferMeta *>(header->pAppPrivate);
 
@@ -367,5 +424,37 @@ OMX_ERRORTYPE OMXNodeInstance::OnFillBufferDone(
     return instance->owner()->OnFillBufferDone(instance->nodeID(), pBuffer);
 }
 
+void OMXNodeInstance::addActiveBuffer(OMX_U32 portIndex, OMX::buffer_id id) {
+    ActiveBuffer active;
+    active.mPortIndex = portIndex;
+    active.mID = id;
+    mActiveBuffers.push(active);
+}
+
+void OMXNodeInstance::removeActiveBuffer(
+        OMX_U32 portIndex, OMX::buffer_id id) {
+    bool found = false;
+    for (size_t i = 0; i < mActiveBuffers.size(); ++i) {
+        if (mActiveBuffers[i].mPortIndex == portIndex
+            && mActiveBuffers[i].mID == id) {
+            found = true;
+            mActiveBuffers.removeItemsAt(i);
+            break;
+        }
+    }
+
+    if (!found) {
+        LOGW("Attempt to remove an active buffer we know nothing about...");
+    }
+}
+
+void OMXNodeInstance::freeActiveBuffers() {
+    // Make sure to count down here, as freeBuffer will in turn remove
+    // the active buffer from the vector...
+    for (size_t i = mActiveBuffers.size(); i--;) {
+        freeBuffer(mActiveBuffers[i].mPortIndex, mActiveBuffers[i].mID);
+    }
+}
+
 }  // namespace android