soundfont loading is very inefficient

• Jun 5, 2021 - 17:24
Reported version
3.6
Type
Performance
Frequency
Once
Severity
S5 - Suggestion
Reproducibility
Always
Status
active
Regression
No
Workaround
No
Project
MuseScore

The soundfont headers are read when the soundfont is loaded, but the actual sample data is only loaded (and Vorbis-decompressed for SF3) once the instrument is assigned… but the file is opened and closed once per sample, not once per “loading action”, there, so there’s potential for quite the speedup…

Preset::loadSamples() could probably open the file once, then pass the opened handle to Sample::load() for each individual subcall.

Comments

• Jun 5, 2021 - 22:24

Fully agree that it is not a bug.
But a good example of why I hate the expression "By design" used here.
By "design"? Really? It was designed to be slow?
No, it was implemented like that just because it was easy, or there were no design at all, or ...
But not "by design"

Reply

• Jun 6, 2021 - 11:06

?? "no design at all" ?? - not possible; this loading code did not just design itself.

Simplicity can have advantages and the speed trade-off is sometimes worth it. A previous colleague was a really clever bloke who could write in a single line of code what would take most of us 3 lines. The result was code that probably ran faster but was hard to decipher due to clever functions and lateral thinking. We called his software "crystalware" - touch it and it breaks - as it was hard for anyone to make changes to it. When he left the company any changes to his code meant starting from scratch.

This current case may not be such an example but simplicity can have its benefits.

Reply

• Jun 6, 2021 - 13:49

Assuming all corresponding instances of Preset, Zone, Instrument and Sample share the same SFont, a patch like the following will, most likely, do the trick (I didn’t even compile-test this), so it’s so trivial it’s not even a matter of simplicity.

node-321949.diff_.txt

Someone should probably look that over and make into a PR if it’s indeed correct.

Attachment Size
node-321949.diff_.txt 3.53 KB

Reply

• Jun 6, 2021 - 14:25

I don’t know; this is why I haven’t submitted this as PR yet. I assume (hah!) so, from a cursory look over the code and where I think these are all instantiated.

Reply