schellingb / TinySoundFont

SoundFont2 synthesizer library in a single C/C++ file
MIT License
623 stars 72 forks source link

Add "tsf_copy" function for linked copies (rebased) #65

Closed ell1e closed 2 years ago

ell1e commented 2 years ago

This adds a "tsf_copy" function for linked tsf* copies that share the underlying soundfont, which allows rendering out multiple independent playbacks with the same soundfont without loading it multiple times. It was originally written by @paladin-t (github.com/paladin-t), also see https://github.com/schellingb/TinySoundFont/pull/1 - GitHub wouldn't let me rebase the actual original commit to retain this info because it is apparently being complicated about deleted origin repos.

In my first tests, this causes a quite radical memory reduction for the soundfont I use (30MB on disk, but apparently larger in memory) by about 90MB of program memory used compared to just naively loading the soundfont twice for two parallel playbacks.

Please note however I did this rebase quite naively, this was e.g. also written before channels even were a thing. So I'm not super confident it's error free, and doesn't have leaks or something. However, my program does use channels and they appear to still work. But more eyes on it, and more testing might be helpful.

Closes #62 ...I think?

ell1e commented 2 years ago

Found a small leak bug (I failed to copy over 2 lines), and fixed a potential crash/corruption whenever channels were created in the source instance before a tsf_copy took place (the original code was written before TSF channels existed). In any case, more testing and more eyes on it by anyone interested are highly welcome.

schellingb commented 2 years ago

Thanks for this updated PR! Looks like it should work :-)

One thing I just thought of, not sure that I like how tsf_render_short can free/allocate outputSamples used by all copies... Maybe it would be better for that to be not shared? Otherwise the documentation probably should mention that tsf_render_short can only be called by a single thread.

ell1e commented 2 years ago

Yes you are correct, that seems kind of too overly optimized and ill-advisable. I changed it now, in my first test it seems to work.

schellingb commented 2 years ago

Awesome! It simplifies this patch a bunch as well :-) While I think in some cases it might be crucial to not allocate outputSamples multiple times but in that case it shouldn't be too hard to re-implement a custom tsf_render_short which uses a shared float buffer that works with whatever thread model that is used by the application.

ell1e commented 2 years ago

As a side note, I think this allocation is likely needless anyway. Instead it appears to me it could just use a fixed size stack buffer, with a surrounding loop that just renders & converts this buffer size as a maximum in one go. and just repeats until the requested amounts of samples is iteration by iteration written out.