sinshu / meltysynth

A SoundFont MIDI synthesizer for .NET
Other
136 stars 16 forks source link

Unable to parse soundfont banks greater than 2 GB #43

Closed aguerrieri82 closed 3 months ago

aguerrieri82 commented 9 months ago

Hi and congratulations on this amazing project!

I have encountered an issue when reading a packed SF file larger than 2GB. Upon inspecting the code, I noticed that the size is read as a signed Int32. This could lead to an overflow if the size is greater than 2^32 / 2 - 1.

Changing this line:

https://github.com/sinshu/meltysynth/blob/60756cc1b12438c00b8b8e85f84f0710303a7461/MeltySynth/src/SoundFontSampleData.cs#L33

to ReadUInt32 might resolve the overflow issue, but it results in a subsequent error soon after at MemoryMarshal.Cast because Span<TFrom> has an int length and cannot exceed 2,147,483,647 bytes.

In any case, storing 2+ GB of wave samples in RAM as an array of short[] Samples might not be a wise choice IMHO :) My general advice would be to use a memory-mapped file. In short, this approach allows you to read a file as a contiguous area of virtual memory, dynamically swapped by the OS from disk to a physical page of RAM.

Here's a quick sample code for reference:


unsafe
{
    using var file = MemoryMappedFile.CreateFromFile("path");
    using var view = file.CreateViewAccessor();

    byte* ptr = null;
    view.SafeMemoryMappedViewHandle.AcquirePointer(ref ptr);
    try
    {
        var span = new ReadOnlySpan<short>(ptr + offset, length);

        // Do things with the span

        for (int i = 0; i < span.Length; i++)
            Console.WriteLine(span[i]);
    }
    finally
    {
        if (ptr != null)
            view.SafeMemoryMappedViewHandle.ReleasePointer();
    }
}
sinshu commented 9 months ago

Thanks 😊

I understand there is a demand for supporting large SoundFonts, but for the following reasons, I've decided not to support them in MeltySynth:

I hope for your understanding πŸ™

aguerrieri82 commented 9 months ago

Hi, here my implemetation if you want to have a look

https://github.com/aguerrieri82/meltysynth/

I added an abstraction layer for the samples buffer (instead of a short[] I use a new interface ISamplesBuffer, implemented in FileMapSamplesBuffer and ArraySamplesBuffer) I added an abstraction layer for the BinaryReader (now IFileReader, implemented in StreamFileReader and FileMapReader) So basically you can use it in the current way (default in the constructor SoundFont(string path, bool useMemoryMap = false)) or the new way. I conducted a quick test and did not observe any decrease in performance. Additionally, the loading time is significantly faster.

sinshu commented 3 months ago

I've done some benchmarking on this and found that using memory-mapped files results in about a 16% slowdown due to the abstraction layer. https://github.com/sinshu/meltysynth-benchmark/tree/mmf

Since keeping CPU usage low is crucial for me, I've decided to stick with the current implementation. However, your code was very insightful and educational. Thanks for the suggestion ☺️