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

Unable to parse soundfont banks greater than 2 GB #43

Open
aguerrieri82 opened this issue Dec 18, 2023 · 2 comments
Open

Unable to parse soundfont banks greater than 2 GB #43

aguerrieri82 opened this issue Dec 18, 2023 · 2 comments

Comments

@aguerrieri82
Copy link

aguerrieri82 commented Dec 18, 2023

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:

var size = reader.ReadInt32();

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
Copy link
Owner

sinshu commented Dec 19, 2023

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:

  • In MeltySynth, I prioritize code simplicity over compatibility with all SoundFonts. In fact, the Modulator is not supported at all due to its complexity, and SoundFonts that use this feature will not produce the correct sound.

  • The Synthesizer object may be used in an audio thread with strict timing, and it is not permissible for its methods to perform disk IO. Designing to handle memory-mapped files to meet this requirement would likely complicate the design.

I hope for your understanding 🙏

@aguerrieri82
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants