Leaking memory when using a MemoryStream with NVorbis

Jan 10 at 7:37 PM
First off thanks for such a handy project! It's been a major help to me on my current project that I'm developing in Unity.

I did run into a bit of an issue though. I'm not sure if it's limited to running NVorbis in Mono under the unity engine, but basically, if you create a VorbisReader by passing in a MemoryStream object, NVorbis will prevent the underlying byte array from being freed, even after both the VorbisReader and MemoryStream have been properly disposed. Effectively, the memory is leaked and unable to be recovered while the application is running.

I did some digging and found that the reason this happens is due to the implementation of the StreamReadBuffer class. What appears to occur is that even if the MemoryStream is closed, because the stream is never removed from the static Dictionary _lockObjects, the underlying data is never picked up by the garbage collector. For the time being, I've managed to get the memory returned by modifying the code so that the stream is removed from the dictionary when a BufferedReadStream is disposed, but I'm certain there's unintended consequences of doing so as there's rare occurrences where an exception can occur (usually from OggContainerReader.GatherNextPage).

Anyways I thought I'd bring it up, perhaps you might have some insight on the problem. Thanks again for such a great project!
Coordinator
Jan 10 at 7:47 PM
Wow, good catch... The _lockObjects dictionary is for concurrency support when two (or more) instances of StreamReadBuffer are accessing the same underlying stream. Just thinking about it, there really should be some kind of refcounting involved so we can cleanly remove the references once no one is using a stream... That should handle it nicely.

Sorry about the memory leak, though. I usually test against a file or a wrapper of some kind, so I just plain missed it.
Coordinator
Jan 16 at 4:34 PM
Please try the latest release... It refcounts the _lockObjects entries and should release the stream correctly when finished.
Jan 24 at 9:07 PM
Edited Jan 24 at 9:09 PM
Thanks, it seems to solve the memory leaking issue entirely!

I'm not sure if it's related though, I do seem to have an issue where sometimes after creating a VorbisReader, attempting to call ReadSamples will sometimes give garbage data at the very start of a stream. This only lasts the first few ms of the file, and it only occurs sometimes (though it occurs more common in larger or specific files). There's no error output in these cases, but one hint is that attempting to check vorbisReader.DecodedTime while the issue is occurring returns an exception:

InvalidDataException: Could not find next page.
NVorbis.Ogg.ContainerReader.GatherNextPage (Int32 streamSerial, NVorbis.Ogg.PageReaderLock pageLock)
NVorbis.Ogg.PacketReader.ReadAllPages ()
NVorbis.Ogg.PacketReader.GetLastPacket ()
NVorbis.Ogg.PacketReader.GetGranuleCount ()
NVorbis.VorbisStreamDecoder.GetLastGranulePos ()
NVorbis.VorbisReader.get_TotalTime ()
Assets.Scripts.Core.Audio.AudioLayer.Update () (at Assets/Scripts/Core/Audio/AudioLayer.cs:173)

After digging around at random trying to figure out why that would happen, I stumbled across a rather unusual solution: accessing vorbisReader.Stats[0].TotalPages appears to fix the problem. How bizarre, but looking at the code, the getter for TotalPages calls GetTotalPageCount() of the OggPacketReader, which in turn calls ReadAllPages.

I think I have something working now, but hopefully the info is enough to understand what's going on. I don't really understand it myself, but I think it should be a good place to start from!

Thanks again for you help!
Coordinator
Jan 26 at 7:39 PM
Are you playing the stream while trying to get the total time? I have a feeling it's a sequencing (threading?) issue that is leaving the read buffer in an invalid state...
Coordinator
Feb 25 at 3:28 AM
OK, new release up... I figured out the threading issue and fixed it. You should be able to use reader.TotalTime while in playback and have it work correctly. Please test and let me know.

Thanks!