Cannot load large (2GiB+) SF2 file on Windows

[trolley813]

FluidSynth version

2.0.4 (but it probably affects later version, according to the code in the master branch)

Describe the bug

On Windows, attempting to load a SF2 file larger than 2GiB (32-bit signed integer limit) fails with the following error:

fluidsynth: error: Get end of file position failed

Expected behavior

The SF2 file should load as usual (is there is enough RAM to do this).

Steps to reproduce

Start FluidSynth on Windows with a SF2 file of 2GiB+.

Additional context

The problem is caused by calling ftell() function (see here), which is hardcoded to return long. On x86_64 Linux, there is no problem here because long is usually defined as 64 bits on this architecture, but Windows is not the case, and long is 32 bits even on 64-bit Windows.

Suggestions:

• for file access functions, replace long return values with size_t, since it will give more reliable bit width.
• unfortunately, ftell() (as well as other related functions like fseek()) is defined in the standard to return long, so on Windows the regular ftell() won't work. It'll be necessary to use Windows-specific things like _ftelli64() (wrapping them into #ifdef WIN32 etc.)

If some consensus on this is reached, I will try to make a PR fixing that.

[derselbst]

Damn.

Technically, your suggestions are correct. However, we are exposing those file callbacks via our API and changing long to size_t would require an SOVERION bump. That's not too dramatic though.

The more difficult question that arises, is how to deal with fseek? Consequently, it should be replaced by a 64 bit equivalent as well. But which argument type should its offset parameter be? We cannot use size_t here, because fseek must receive a signed integer. And C89 doesn't have signed 64 bit integers. I currently don't have a solution for this.

[trolley813]

And C89 doesn't have signed 64 bit integers.

Why not use int64_t from C99 then? The <stdint.h> header (as well as the most part of C99 standard) was introduced in GCC 4.5 released back in 2010, so C99 can be safely assumed as being available on most platforms released since then.
Of course, breaking compatibility can be very painful, but it's almost always unavoidable.

[trolley813]

Another (very hacky) possibility is to define a custom type depending on the platform, like this:

#ifdef WIN64
typedef int64_t fluid_long;
#else
typedef long fluid_long;
#endif

This would not break ABI on Linux but would fix the Windows behaviour.

[derselbst]

C99 can be safely assumed as being available on most platforms released since then.

Not really. Microsoft just added fully compliant support for C99 in VS2015. And we have a few commercial users who still use VS2010. The day we will switch to C99 will come, but surely not for the next 2.1.2 bugfix release. In the meantime, we are stuck with C89.

[trolley813]

As far as I remember, 64-bit integers and functions like _fseeki64 are available even in VS2008 (and probably earlier). To fix the loading, we don't need full support for C99.

[derselbst]

You are missing the fact that those file-operations are exposed via our public API. Here, we need a standard compliant signed 64 bit int that we can expose in our API (*). And for that we need to make fluidsynth switching to C99.

If this was only a question of an implementation detail, i.e. which function(s) to use under the hood, it would be trivial.

Still, you are welcome to propose a PR. Perhaps this will help to better understand the scope of this change. However, I would vote for using long long rather than size_t, because ftelli64() and fseeki64() both only use the value range of a singed integer.

---

(*): Ofc one might think about simply implementing our own 64int type:

struct fluid_int64{
int hi;
int lo;}

But how to make sure that int is really 32 bits wide? One would need stdint.h, but this is a C99 feature. Here we go again.

[trolley813]

Yes, I agree that this breaks not only API but ABI, so the fixing can be probably done in no earlier than 3.0. So, probably the only remaining workaround is to build FluidSynth on Cygwin (I've tested it, and it seems to work), which defines the long as 64 bits wide.

[derselbst]

We've discussed this issue on the mailing list:

https://lists.nongnu.org/archive/html/fluid-dev/2020-03/msg00009.html

We came up with a solution, which I'll be implementing in the next days. This will be included in the 2.2.0 release. But there is no release date for it yet.

In the meantime, the Cygwin workaround must be sufficient. Thanks for the hint!