Add support for sound fonts in SF3 format that contain Ogg Vorbis compressed samples #183
Conversation
…pressed samples Author: Fabian Greffrath <fabian@debian.org> Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=740710 Forwarded: https://sourceforge.net/p/fluidsynth/tickets/142/ Last-Update: 2015-11-26
| #if LIBSNDFILE_SUPPORT | ||
| // virtual file access rountines to allow for handling | ||
| // samples as virtual files in memory | ||
| static sf_count_t sfvio_pos; |
There was a problem hiding this comment.
I dont think introducing this global var here is correct. It may work because we only open one sample at a time, do all the necessary work and never come back to it again. However _fluid_sample_t has a member void* userdata, I wouldnt mind at all abusing this one for storing a sf_count_t sfvio_pos for that specific sample. Imagine something like
case SEEK_SET:
sample->userdata = (void*) offset;
...
sfvio_tell (void* user_data)
{
fluid_sample_t *sample = (fluid_sample_t *)user_data;
return (sf_count_t)sample->userdata;
}
... not nice but more suited IMO.
| fluid_sample_t *sample = (fluid_sample_t *)user_data; | ||
|
|
||
| return (sf_count_t)(sample->end + 1 - sample->start); | ||
| }; |
There was a problem hiding this comment.
Why have you placed these lonesome ; here and below? Could you remove them? They may cause compiler warnings.
Sure, there is one in the latest musescore sources. It can be considered the reference implementation, since these guys originally came up with the idea of the SF3 format: https://github.com/musescore/MuseScore/tree/master/share/sound
Ok, will do so, but it was your idea, in case anyone asks! ;)
They are there by accident, will remove them! |
| sample->userdata = (void *)offset; | ||
| break; | ||
| case SEEK_CUR: | ||
| sample->userdata = (void *)((intptr_t)sample->userdata + offset); |
There was a problem hiding this comment.
Why are you using (intptr_t) here and below?
There was a problem hiding this comment.
I am using it for intermediate casts between pointer and int. Shouldn't make a difference to sf_count_t, though.
There was a problem hiding this comment.
But it confuses as you're not mangling with pointers, only with sf_count_t. So I'd prefer sf_count_t.
for FLUID_SAMPLETYPE_OGG_VORBIS
in order to stay in sync with FluidSynth#184
|
Ok, thanks so far for your contribution! After getting #184 to work, I'll have some tests in the next days. If it all goes well we might already have this in 1.1.7, lets see. |
|
@fabiangreffrath I merged it in and made some changes to mangling with incorrect sample loops. Hopefully this is correct now, at least I tested with FluidR3.sf3 and it sounds really nice. So we'll have this feature in 1.1.7. Anyway, feel free to build recent git and test it yourself. Thanks again! |
|
Thank you very much for merging this! I am really looking forward to testing the new feature, though this will have to wait until next week, as I am currently on vacation. ;) |
|
Just checked out some Doom music from the Alien Vendetta PWAD with Musescore's SF3 sound font and it worked as expected! \o/ |
|
Awesome. Thanks for that guys ! |
|
I've been very happy to find out about SF3 format today and see this recently merged PR here. |
|
@mmontag Thanks! 😁 |
|
Hm, unfortunately I didn't make it neither into the AUTHORS nor into the THANKS file. |
|
If you want to be credited, you are welcome to open a PR and add yourself to either one of the files you prefer. |
Sorry, I forgot about this but will open a PR shortly. |
Author: Fabian Greffrath fabian@debian.org
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=740710
Forwarded: https://sourceforge.net/p/fluidsynth/tickets/142/
Last-Update: 2015-11-26
Fixes #140