Fluidsynth still doesn't ignore unknown INFO chunks

[spessasus]

FluidSynth version

2.4.7

I know that 2.4.8 released a few days ago but it didn't change anything about info parsing.

Describe the bug

Related to #1580

this file is rejected by fluidsynth for some reason.

It says:

fluidsynth: error: INFO sub chunk LIST has invalid chunk size of 113602 bytes

Yet xdta should be ignored as it's unknown but it is not.

Expected behavior

Ignore the chunk like DMOD.

Steps to reproduce

1. Get the xdta version of GeneralUserGS from the link above
2. Attempt to load the file
3. fluidsynth: error: INFO sub chunk LIST has invalid chunk size of 113602 bytes

Additional context

Xdta chunk uses the LIST fourCC and xdta as a sub ID. But a LIST does not appear in the SF2 info, so I don't know why it isn't triggering the UNKN_ID check.

[derselbst]

The INFO LIST chunk is quite big, 0x01C66C bytes. This includes the LIST chunk found at offset 0xAB6. LIST is a well known chunk id to fluidsynth, which is why it's not triggering the UNKN_ID check. This LIST chunk is 0x01BBC2 bytes in size, which is bigger than the permitted 65536 bytes for info subchunks. At a first glance I couldn't find a hard requirement in the spec for this 65536 bytes limit. It looks like this is a leftover check meant solely for the ICMT chunk, which has such a size constraint.

This check needs to be changed to only consider the well known INFO subchunks along with their size constraints as defined by the spec. The question remains what to do if any INFO subchunk exceeds its size: Reject the entire file, or only ignore that particular subchunk? I tend to do the latter.

[spessasus]

The question remains what to do if any INFO subchunk exceeds its size: Reject the entire file, or only ignore that particular subchunk? I tend to do the latter.

It's not like a few more characters in the name or similar will impact how the file sounds.

Honestly, I don't understand the SF2 spec:

(A small rant below)

The entire point of RIFF is to make the software ignore unknown chunks. It's clearly marked in the DLS spec. If there are some chunks from a newer version or proprietary extensions, the software can just skip them and read what it understands.

Yet the SF2 specification prohibits any unknown chunks, so why even bother with RIFF? Just structure it with some weird way that doesn't permit any new chunks. Like, the sm24 chunk for example. This violates the 2.01 specification! No sm24 is defined and the software is required to reject soundfonts with unknown chunks. So 2.01 only synths adhering to the spec would have to reject the file even though they could happily ignore the chunk and play the smpl data just fine.

Same about modulators, which didn't exist in 2.00. No backwards compatibility at all! This makes developing extensions really limiting as SF2 software not supporting them has to reject the file even though it has no real reason to do so. It knows the size of the chunk so it can just skip over it, but not according to Creative. If I wanted to add the DMOD chunk to DLS, every software (even the 20 year old proprietary software) will happily ignore it and move on. For example the famous gm.dls has a few proprietary chunks of its own. Yet SF2 doesn't allow anything in the format which uses a structure specifically designed to allow doing so. I really don't understand. What about you?

[derselbst]

EMU's intention was probably to keep a rigid chunk structure that can be easily and widely recognized as SF2, while only permitting unknown / proprietary chunks at a very specific location, which is the INFO chunk. This aids to implement SF2 parsers while preventing the format to the heavily abused by proprietary programs. This seems legit to me.

In fluidsynth's case, the INFO chunk parsing has never been really questioned or tested. It's all legacy code. Anyway, I have now revised it and proposed a different approach, see #1647. (Not sure if we need any additional unit tests for this.)

Having a LIST chunk within INFO is still somewhat unexpected, so I didn't implement any special handling for this.

Loading the file still fails later on, because instrument generator indices are not monotonic.