Buffers are not always recycled automatically

droidmediabuffer.cpp

@@ -17,6 +17,7 @@
  * Authored by: Mohammed Hassan <[email protected]>
  */
 
+#include <inttypes.h>
 #include "droidmediabuffer.h"
 #include "private.h"
 
@@ -127,7 +128,12 @@ DroidMediaBuffer *droid_media_buffer_create(uint32_t w, uint32_t h,
 
 void droid_media_buffer_destroy(DroidMediaBuffer *buffer)
 {
-  buffer->decStrong(0);
+  if (buffer->m_queue) {
+    buffer->m_queue->releaseBufferResources(buffer);
+  } else {
+    ALOGW("Decrementing buffer refs without a queue %" PRIxPTR, (uintptr_t)buffer);
+    buffer->decStrong(0);
+  }
 }
 
 void droid_media_buffer_set_user_data(DroidMediaBuffer *buffer, void *data)

droidmediacodec.cpp

@@ -181,7 +181,7 @@ class Source : public android::MediaSource {
         for (android::List<android::MediaBuffer *>::iterator iter = m_framesBeingProcessed.buffers.begin();
              iter != m_framesBeingProcessed.buffers.end(); iter++) {
             if (*iter == buffer) {
-                m_framesBeingProcessed.buffers.erase(iter);                
+                m_framesBeingProcessed.buffers.erase(iter);
                 m_framesBeingProcessed.cond.signal();
                 m_framesBeingProcessed.lock.unlock();
                 return;

private.cpp

@@ -16,7 +16,7 @@
  *
  * Authored by: Mohammed Hassan <[email protected]>
  */
-
+#include <inttypes.h>
 #include "private.h"
 #include "droidmediabuffer.h"
 #if (ANDROID_MAJOR == 4 && ANDROID_MINOR < 4)
@@ -63,6 +63,7 @@ void DroidMediaBufferQueueListener::onBuffersReleased()
 }
 
 _DroidMediaBufferQueue::_DroidMediaBufferQueue(const char *name) :
+  m_unslotted({}),
   m_listener(new DroidMediaBufferQueueListener(this)),
   m_data(0) {
 
@@ -171,8 +172,17 @@ void _DroidMediaBufferQueue::frameAvailable() {
   DroidMediaBufferSlot &slot = m_slots[slotIndex(item)];
 
   if (item.mGraphicBuffer != NULL) {
-    static_cast<DroidMediaBufferItem &>(slot) = item;
+    if (slot.droidBuffer.get()) {
+      // It seems the buffers are not recycled. We're manually releasing this previous slot.
+      ALOGW("Releasing only resources, keeping memory %" PRIxPTR, (uintptr_t)slot.droidBuffer.get());
+      slot.mGraphicBuffer = 0;
+      // instead of clear-ing the entire memory which leads to droid_media_buffer_destroy crashing.
+      releaseBufferResources(slot.droidBuffer.get());
+      // Add the buffer to the list of unslotted buffers that droid_media_buffer_destroy will check and don't do anything.
+      m_unslotted.emplace(slot.droidBuffer.get(), slot.droidBuffer);
+    }
 
+    static_cast<DroidMediaBufferItem &>(slot) = item;
     slot.droidBuffer = new DroidMediaBuffer(slot, this);
 
     // Keep the original reference count for ourselves and give one to the buffer_created
@@ -218,18 +228,33 @@ void _DroidMediaBufferQueue::frameAvailable() {
   }
 }
 
-void _DroidMediaBufferQueue::buffersReleased() {
-  for (int i = 0; i < android::BufferQueue::NUM_BUFFER_SLOTS; ++i) {
-    DroidMediaBufferSlot &slot = m_slots[i];
-    slot.droidBuffer.clear();
-    slot.mGraphicBuffer = 0;
+void _DroidMediaBufferQueue::releaseBufferResources(DroidMediaBuffer *buffer) {
+  if (m_unslotted.find(buffer) != m_unslotted.end()) {
+    ALOGI("Unslotted buffer, resources already released %" PRIxPTR, (uintptr_t)buffer);
+  } else {
+    buffer->decStrong(0);
+    buffer->m_buffer.clear();
   }
+}
 
+void _DroidMediaBufferQueue::buffersReleased() {
   android::AutoMutex locker(&m_lock);
 
   if (m_data) {
     m_cb.buffers_released(m_data);
   }
+
+  // Releasing DroidMediaBuffer memory for slots
+  for (int i = 0; i < android::BufferQueue::NUM_BUFFER_SLOTS; ++i) {
+      DroidMediaBufferSlot &slot = m_slots[i];
+      slot.droidBuffer.clear();
+      slot.mGraphicBuffer = 0;
+  }
+  // Releasing DroidMediaBuffer memory for unslotted buffers
+  for (auto& droidBufferPair : m_unslotted) {
+    droidBufferPair.second.clear();
+  }
+  m_unslotted.clear();
 }
 
 int _DroidMediaBufferQueue::releaseMediaBuffer(int index, EGLDisplay dpy, EGLSyncKHR fence) {

private.h

@@ -81,6 +81,8 @@ struct _DroidMediaBufferQueue : public android::RefBase {
 
   void buffersReleased();
 
+  void releaseBufferResources(DroidMediaBuffer *buffer);
+
 private:
   friend class DroidMediaBufferQueueListener;
 
@@ -97,6 +99,10 @@ struct _DroidMediaBufferQueue : public android::RefBase {
 
   DroidMediaBufferSlot m_slots[android::BufferQueue::NUM_BUFFER_SLOTS];
 
+  // Storage for buffers that have their resources cleaned up, but the client still holds a raw reference.
+  // The first part is used to identify the raw pointer from client, the second to actually clean the memory.
+  std::unordered_map<DroidMediaBuffer*, android::sp<DroidMediaBuffer>> m_unslotted;
+
   android::sp<DroidMediaBufferQueueListener> m_listener;
   android::Mutex m_lock;
 
@@ -111,5 +117,5 @@ android::sp<android::MediaSource> droid_media_codec_create_encoder_raw(DroidMedi
 							      android::sp<android::ALooper> looper,
 #endif
 							      android::sp<android::MediaSource> src);
- 
+
 #endif /* DROID_MEDIA_PRIVATE_H */