diff --git a/src/sfloader/fluid_defsfont.c b/src/sfloader/fluid_defsfont.c index 0b088a0b..7de2ad58 100644 --- a/src/sfloader/fluid_defsfont.c +++ b/src/sfloader/fluid_defsfont.c @@ -1850,28 +1850,20 @@ fluid_sample_import_sfont(fluid_sample_t* sample, SFSample* sfsample, fluid_defs sample->pitchadj = sfsample->pitchadj; sample->sampletype = sfsample->sampletype; - if (sample->sampletype & FLUID_SAMPLETYPE_OGG_VORBIS) + if (fluid_sample_validate(sample, sfont->samplesize) == FLUID_FAILED) { - int ret = fluid_sample_decompress_vorbis(sample); - if (sample->data == NULL || ret == FLUID_FAILED) - { - sample->valid = 0; - return ret; - } + return FLUID_OK; } - if (sample->sampletype & FLUID_SAMPLETYPE_ROM) { - sample->valid = 0; - FLUID_LOG(FLUID_WARN, "Ignoring sample '%s': can't use ROM samples", sample->name); - } - else if (sample->end - sample->start < 8) { - sample->valid = 0; - FLUID_LOG(FLUID_WARN, "Ignoring sample '%s': too few sample data points", sample->name); - } - else { - sample->valid = TRUE; + if ((sample->sampletype & FLUID_SAMPLETYPE_OGG_VORBIS) + && fluid_sample_decompress_vorbis(sample) == FLUID_FAILED) + { + return FLUID_FAILED; } + fluid_sample_sanitize_loop(sample, sfont->samplesize); + + sample->valid = TRUE; + return FLUID_OK; } - diff --git a/src/sfloader/fluid_sffile.c b/src/sfloader/fluid_sffile.c index cf86026c..c6474367 100644 --- a/src/sfloader/fluid_sffile.c +++ b/src/sfloader/fluid_sffile.c @@ -274,7 +274,6 @@ static int load_igen(SFData *sf, int size); static int load_shdr(SFData *sf, unsigned int size); static int fixup_pgen(SFData *sf); static int fixup_igen(SFData *sf); -static int fixup_sample(SFData *sf); static int chunkid(unsigned int id); static int read_listchunk(SFData *sf, SFChunk *chunk); @@ -490,8 +489,6 @@ static int load_body(SFData *sf, unsigned int size) return FALSE; if (!fixup_igen(sf)) return FALSE; - if (!fixup_sample(sf)) - return FALSE; /* sort preset list by bank, preset # */ sf->preset = fluid_list_sort(sf->preset, (fluid_compare_func_t)preset_compare_func); @@ -651,7 +648,7 @@ static int process_sdta(SFData *sf, unsigned int size) /* sample data follows */ sf->samplepos = sf->fcbs->ftell(sf->sffd); - /* used in fixup_sample() to check validity of sample headers */ + /* used to check validity of sample headers */ sf->samplesize = chunk.size; FSKIP(sf, chunk.size); @@ -1679,119 +1676,6 @@ static int fixup_igen(SFData *sf) return TRUE; } -/* Make sure sample start/end and loopstart/loopend pointers are valid */ -static int fixup_sample(SFData *sf) -{ - fluid_list_t *p; - SFSample *sam; - int invalid_loops = FALSE; - int invalid_loopstart; - int invalid_loopend, loopend_end_mismatch; - unsigned int total_bytes = sf->samplesize; - unsigned int total_samples = total_bytes / sizeof(short); - - p = sf->sample; - while (p) - { - unsigned int max_end; - - sam = (SFSample *)(p->data); - - /* Standard SoundFont files (SF2) use sample word indices for sample start and end pointers, - * but SF3 files with Ogg Vorbis compression use byte indices for start and end. */ - max_end = (sam->sampletype & FLUID_SAMPLETYPE_OGG_VORBIS) ? total_bytes : total_samples; - - /* ROM samples are unusable for us by definition, so simply ignore them. */ - if (sam->sampletype & FLUID_SAMPLETYPE_ROM) - { - sam->start = sam->end = sam->loopstart = sam->loopend = 0; - goto next_sample; - } - - /* If end is over the sample data chunk or sam start is greater than 4 - * less than the end (at least 4 samples). - * - * FIXME: where does this number 4 come from? And do we need a different number for SF3 - * files? - * Maybe we should check for the minimum Ogg Vorbis headers size? */ - if ((sam->end > max_end) || (sam->start > (sam->end - 4))) - { - FLUID_LOG(FLUID_WARN, "Sample '%s' start/end file positions are invalid," - " disabling and will not be saved", - sam->name); - sam->start = sam->end = sam->loopstart = sam->loopend = 0; - goto next_sample; - } - - /* The SoundFont 2.4 spec defines the loopstart index as the first sample point of the loop - */ - invalid_loopstart = (sam->loopstart < sam->start) || (sam->loopstart >= sam->loopend); - /* while loopend is the first point AFTER the last sample of the loop. - * this is as it should be. however we cannot be sure whether any of sam.loopend or sam.end - * is correct. hours of thinking through this have concluded, that it would be best practice - * to mangle with loops as little as necessary by only making sure loopend is within - * max_end. incorrect soundfont shall preferably fail loudly. */ - invalid_loopend = (sam->loopend > max_end) || (sam->loopstart >= sam->loopend); - - loopend_end_mismatch = (sam->loopend > sam->end); - - if (sam->sampletype & FLUID_SAMPLETYPE_OGG_VORBIS) - { - /* - * compressed samples get fixed up after decompression - * - * however we cant use the logic below, because uncompressed samples are stored in - * individual buffers - */ - } - else if (invalid_loopstart || invalid_loopend || - loopend_end_mismatch) /* loop is fowled?? (cluck cluck :) */ - { - /* though illegal, loopend may be set to loopstart to disable loop */ - /* is it worth informing the user? */ - invalid_loops |= (sam->loopend != sam->loopstart); - - /* force incorrect loop points into the sample range, ignore padding */ - if (invalid_loopstart) - { - FLUID_LOG(FLUID_DBG, "Sample '%s' has unusable loop start '%d'," - " setting to sample start at '%d'", - sam->name, sam->loopstart, sam->start); - sam->loopstart = sam->start; - } - - if (invalid_loopend) - { - FLUID_LOG(FLUID_DBG, "Sample '%s' has unusable loop stop '%d'," - " setting to sample stop at '%d'", - sam->name, sam->loopend, sam->end); - /* since at this time sam->end points after valid sample data (will correct that few - * lines below), - * set loopend to that first invalid sample, since it should never be played, but - * instead the last - * valid sample will be played */ - sam->loopend = sam->end; - } - else if (loopend_end_mismatch) - { - FLUID_LOG(FLUID_DBG, "Sample '%s' has invalid loop stop '%d'," - " sample stop at '%d', using it anyway", - sam->name, sam->loopend, sam->end); - } - } - - next_sample: - p = fluid_list_next(p); - } - - if (invalid_loops) - { - FLUID_LOG(FLUID_WARN, "Found samples with invalid loops, audible glitches possible."); - } - - return TRUE; -} - static void delete_preset(SFPreset *preset) { fluid_list_t *entry; diff --git a/src/sfloader/fluid_sfont.c b/src/sfloader/fluid_sfont.c index 1db6c0d9..e2be547e 100644 --- a/src/sfloader/fluid_sfont.c +++ b/src/sfloader/fluid_sfont.c @@ -554,6 +554,119 @@ int fluid_sample_set_pitch(fluid_sample_t* sample, int root_key, int fine_tune) } +/** + * Validate parameters of a sample + * + */ +int fluid_sample_validate(fluid_sample_t *sample, unsigned int buffer_size) +{ + /* ROM samples are unusable for us by definition */ + if (sample->sampletype & FLUID_SAMPLETYPE_ROM) + { + FLUID_LOG(FLUID_WARN, "Sample '%s': ROM sample ignored", sample->name); + return FLUID_FAILED; + } + + /* Ogg vorbis compressed samples in the SF3 format use byte indices for + * sample start and end pointers before decompression. Standard SF2 samples + * use sample word indices for all pointers, so use half the buffer_size + * for validation. */ + if (!(sample->sampletype & FLUID_SAMPLETYPE_OGG_VORBIS)) + { + if (buffer_size % 2) + { + FLUID_LOG(FLUID_WARN, "Sample '%s': invalid buffer size", sample->name); + return FLUID_FAILED; + } + buffer_size /= 2; + } + + if ((sample->end > buffer_size) || (sample->start >= sample->end)) + { + FLUID_LOG(FLUID_WARN, "Sample '%s': invalid start/end file positions", sample->name); + return FLUID_FAILED; + } + + return TRUE; +} + +/* Check the sample loop pointers and optionally convert them to something + * usable in case they are broken. Return a boolean indicating if the pointers + * have been modified, so the user can be notified of possible audio glitches. + */ +int fluid_sample_sanitize_loop(fluid_sample_t *sample, unsigned int buffer_size) +{ + int modified = FALSE; + unsigned int max_end = buffer_size / 2; + /* In fluid_sample_t the sample end pointer points to the last sample, not + * to the data word after the last sample. FIXME: why? */ + unsigned int sample_end = sample->end + 1; + + /* Checking loops on compressed samples makes no sense at all and is really + * a programming error. Disable the loop to be on the safe side. */ + if (sample->sampletype & FLUID_SAMPLETYPE_OGG_VORBIS) + { + FLUID_LOG(FLUID_ERR, "Sample '%s': checking loop on compressed sample, disabling loop", + sample->name); + sample->loopstart = sample->loopend = 0; + return TRUE; + } + + if (sample->loopstart == sample->loopend) + { + /* Some SoundFonts disable loops by setting loopstart = loopend. While + * technically invalid, we decided to accept those samples anyway. Just + * ensure that those two pointers are within the sampledata by setting + * them to 0. Don't set modified here, as this change has no audible + * effect. */ + sample->loopstart = sample->loopend = 0; + } + else if (sample->loopstart > sample->loopend) + { + unsigned int tmp; + + /* If loop start and end are reversed, try to swap them around and + * continue validation */ + FLUID_LOG(FLUID_DBG, "Sample '%s': reversed loop pointers '%d' - '%d', trying to fix", + sample->name, sample->loopstart, sample->loopend); + tmp = sample->loopstart; + sample->loopstart = sample->loopend; + sample->loopend = tmp; + modified = TRUE; + } + + /* The SoundFont 2.4 spec defines the loopstart index as the first sample + * point of the loop while loopend is the first point AFTER the last sample + * of the loop. However we cannot be sure whether any of loopend or end is + * correct. Hours of thinking through this have concluded that it would be + * best practice to mangle with loops as little as necessary by only making + * sure the pointers are within sample->start to max_end. Incorrect + * soundfont shall preferably fail loudly. */ + if ((sample->loopstart < sample->start) || (sample->loopstart > max_end)) + { + FLUID_LOG(FLUID_DBG, "Sample '%s': invalid loop start '%d', setting to sample start '%d'", + sample->name, sample->loopstart, sample->start); + sample->loopstart = sample->start; + modified = TRUE; + } + + if ((sample->loopend < sample->start) || (sample->loopend > max_end)) + { + FLUID_LOG(FLUID_DBG, "Sample '%s': invalid loop end '%d', setting to sample end '%d'", + sample->name, sample->loopend, sample_end); + sample->loopend = sample_end; + modified = TRUE; + } + + if ((sample->loopstart > sample_end) || (sample->loopend > sample_end)) + { + FLUID_LOG(FLUID_DBG, "Sample '%s': loop range '%d - %d' after sample end '%d', using it anyway", + sample->name, sample->loopstart, sample->loopend, sample_end); + } + + return modified; +} + #if LIBSNDFILE_SUPPORT // virtual file access rountines to allow for handling diff --git a/src/sfloader/fluid_sfont.h b/src/sfloader/fluid_sfont.h index b845e500..53f10a8c 100644 --- a/src/sfloader/fluid_sfont.h +++ b/src/sfloader/fluid_sfont.h @@ -24,7 +24,10 @@ #include "fluidsynth.h" +FLUIDSYNTH_API int fluid_sample_validate(fluid_sample_t *sample, unsigned int max_end); +FLUIDSYNTH_API int fluid_sample_sanitize_loop(fluid_sample_t *sample, unsigned int max_end); FLUIDSYNTH_API int fluid_sample_decompress_vorbis(fluid_sample_t *sample); + /* * Utility macros to access soundfonts, presets, and samples */