Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Buffers are not always recycled automatically #111

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
9 changes: 9 additions & 0 deletions droidmediabuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* Authored by: Mohammed Hassan <[email protected]>
*/

#include <inttypes.h>
#include "droidmediabuffer.h"
#include "private.h"

Expand Down Expand Up @@ -127,7 +128,15 @@ DroidMediaBuffer *droid_media_buffer_create(uint32_t w, uint32_t h,

void droid_media_buffer_destroy(DroidMediaBuffer *buffer)
{
#if ANDROID_MAJOR > 5
if (buffer->m_queue != NULL) {
buffer->m_queue->releaseBufferResources(buffer);
return;
}
ALOGW("Destroying buffer refs but no queue %" PRIxPTR, (uintptr_t)buffer);
#else
buffer->decStrong(0);
#endif
}

void droid_media_buffer_set_user_data(DroidMediaBuffer *buffer, void *data)
Expand Down
2 changes: 1 addition & 1 deletion droidmediacodec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
46 changes: 39 additions & 7 deletions private.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -63,6 +63,9 @@ void DroidMediaBufferQueueListener::onBuffersReleased()
}

_DroidMediaBufferQueue::_DroidMediaBufferQueue(const char *name) :
#if ANDROID_MAJOR > 5
m_unslotted({}),
#endif
m_listener(new DroidMediaBufferQueueListener(this)),
m_data(0) {

Expand Down Expand Up @@ -171,8 +174,19 @@ void _DroidMediaBufferQueue::frameAvailable() {
DroidMediaBufferSlot &slot = m_slots[slotIndex(item)];

if (item.mGraphicBuffer != NULL) {
#if ANDROID_MAJOR > 5
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;
slot.droidBuffer->m_buffer.clear();
// 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);
}
#endif
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
Expand Down Expand Up @@ -218,18 +232,36 @@ 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;
#if ANDROID_MAJOR > 5
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);
}
}
#endif

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;
}
#if ANDROID_MAJOR > 5
// Releasing DroidMediaBuffer memory for unslotted buffers
for (auto& droidBufferPair : m_unslotted) {
droidBufferPair.second.clear();
}
m_unslotted.clear();
#endif
}

int _DroidMediaBufferQueue::releaseMediaBuffer(int index, EGLDisplay dpy, EGLSyncKHR fence) {
Expand Down
14 changes: 12 additions & 2 deletions private.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@
#if ANDROID_MAJOR >=5
#include <media/stagefright/foundation/ALooper.h>
#endif

#if ANDROID_MAJOR > 5
#include <unordered_map>
#endif
struct _DroidMediaBufferQueue;

class DroidMediaBufferQueueListener :
Expand Down Expand Up @@ -81,6 +83,9 @@ struct _DroidMediaBufferQueue : public android::RefBase {

void buffersReleased();

#if ANDROID_MAJOR > 5
void releaseBufferResources(DroidMediaBuffer *buffer);
#endif
private:
friend class DroidMediaBufferQueueListener;

Expand All @@ -97,6 +102,11 @@ struct _DroidMediaBufferQueue : public android::RefBase {

DroidMediaBufferSlot m_slots[android::BufferQueue::NUM_BUFFER_SLOTS];

#if ANDROID_MAJOR > 5
// 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;
b100dian marked this conversation as resolved.
Show resolved Hide resolved
#endif
android::sp<DroidMediaBufferQueueListener> m_listener;
android::Mutex m_lock;

Expand All @@ -111,5 +121,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 */