search

google / oboe

Oboe is a C++ library that makes it easy to build high-performance audio apps on Android.
Apache License 2.0
3.7k stars 566 forks source link

RhythmGame leaks sound assets #295

Closed philburk closed 5 years ago

philburk commented 5 years ago

SoundRecording::loadFromAssets opens an AAset but does not close it.

https://github.com/google/oboe/blob/master/samples/RhythmGame/src/main/cpp/audio/SoundRecording.cpp

The SoundRecording uses the buffer that seems to be owned by the Asset. This may be OK for the RhythmGame because the assets are opened once and then used for the duration of the app. But an app that needs to load and unload assets might inherit a memory leak if they use the code without modification.

dturner commented 5 years ago

Thanks for spotting this phil. I'll fix.

dturner commented 5 years ago

Implementation notes (mainly for my own reference).

The challenge is that calling AAsset_close will destroy the buffer from AAsset_getBuffer. There are 2 solutions that spring to mind:

1) Copy the entire contents of the asset into memory which is owned by SoundRecording then call AAsset_close - this separates SoundRecording and AAsset but requires more memory temporarily 2) Avoid calling AAsset_close until the SoundRecording is destroyed. This requires keeping a pointer to the AAsset for the lifetime of the SoundRecording.

Option 2 is simpler but does tightly couple the construction of a SoundRecording with an AAsset, which isn't necessarily a bad thing.

From discussion with Phil: Decouple the source from the SoundRecording and don't have it take ownership of the source.