Re: [fluid-dev] invalid instrument/drum selection problem

[Josh Green]

Hello Bernhard,

On Tue, 2008-01-08 at 00:40 +0100, Bernhard Schelling wrote:
> Hi guys
> 
> Sorry I'm late to the party and I'm not even sure if I can be helpful :-)
> 
> We, too, are using FluidSynth's MIDI file reading and playing capabilities so 
> we naturally stumbled on the same issues over development/testing.
> Here are two patches which show all our fixes concerning these features.
> 
> I may add, that I don't know much about the MIDI standards and my fixes were 
> made 'on-the-fly' during testing with many different MIDIs from all over the 
> web.
> 
> I actually wanted to post all my changes a few months ago but then I thought 
> to wait until our project is actually in a more finished/releasable state. 
> Then we 
> somehow got stuck and had to devote ourself to other (commercial, eww) work, 
> so the project is kinda on hold at the moment. Other changes include external 
> use 
> of FluidSynth's MIDI reading/parsing functionality and a 'hacked' 
> implementation of a sound font format with ogg vorbis compressed sample data 
> (including a 
> vorbis compressing soundfont converter implemented/hacked directly into 
> FluidSynth's code).
> 
> The first one is my solution to the drumset (and other instruments actually, 
> too) selection problem.
> The second one fixes a few things with starting and stopping voices when 
> playing MIDI files. I had test midis which actually played the same 
> channel/key note 
> again without releasing the first one before (and with two note offs later). 
> I solved this by storing the exact start time of the noteon in the voice and 
> only 
> releasing the newest voices (reversing the for loop in fluid_synth_noteoff) 
> which were started at the same time.
> 


Looks like some good stuff there.  Some comments on your patch:

Some of the percussion bank/program selection logic can likely be
removed:

I think DRUM_INST_MASK is useless in the current code (not sure how it
got there to begin with).

According to the SoundFont spec, there shouldn't be any bank numbers
above 128 (well actually it says that anything above 128 won't sound,
but the value should be maintained, both program and bank are 16 bit
#s).  In this respect FluidSynth seems broken (since it combines LSB and
MSB to make a 14 bit number).

I can see an issue though if someone decides to use channel #10 for
something other than drums (non GM/GS mode).  In that case that part of
your patch would cause a bit of headache for the user.  This raises the
question again of having a GM/GS mode versus a default mode and how this
mode gets selected.  In default mode, bank/programs will be tried first.
In the GM mode bank changes would be ignored and GS mode would always
use bank 128 for channel 10.  At least that is my current thinking.

The instrument fallback to bank 0 looks good for other channels.

I see your patch makes a change to a public function
fluid_synth_alloc_voice, which shouldn't be changed in the current
FluidSynth version, since it would break backwards compatibility.  It
looks like you are essentially creating the idea of a "group" of voices
(with a time stamp).  What could be done in a backwards compatible
manner, would be to add 2 new functions.

int fluid_voice_group_new (fluid_synth_t *synth);
void fluid_voice_group_start (fluid_synth_t *synth, int group_num);


The first would be called before any voices are created by a note on
event and would return a unique group #.  The second function would be
called after all voices have been created and would start all voices in
that group simultaneously.  This would also fix 2 bugs relating to
portions of an instrument or stereo pair getting started within
different audio buffers (causing unwanted variance/phasing changes in
instrument sounds).


Thanks again for the patch!  It should be helpful in ultimately finding
the right solution for all cases.

        Josh