[fluid-dev] Re: Patch for channel_type, also XG drum-channel autoswitch

[jimmy]

> Hey jimmy,
> 
> Thanks for the research. I've committed the patch now (with
> some trivial 
> changes). Thanks for your contribution!
> 
> And to the rest of you - this bank select handling seems to
> be a never 
> ending debate, and it's not my area of expertise, so let me
> know if this 
> change screwed something up for you.
> 
> // David


Your change is not correct in both MSB, and LSB handling for XG bank 
calculation.

-----

The problem with fluid_channel_set_bank_lsb():

  if (style == FLUID_BANK_STYLE_XG)
      newval = (oldval & ~BANK_MASKVAL) | (banklsb << BANK_SHIFTVAL);

is that any existing MSB values will be zerroes out (your code is using 
~BANK_MASKVAL, instead of ~BANKLSB_MASKVAL).  So it should be:

  if (style == FLUID_BANK_STYLE_XG)
      newval = (oldval & ~BANKLSB_MASKVAL) | (banklsb << BANK_SHIFTVAL);

which is the same as MMA style calculation.

My patch also saves the LSB with XG drum-channels.  The current flow ignores 
drum channels LSB for XG mode.


------

And in fluid_channel_set_bank_msb() you modified it as:

  if (style == FLUID_BANK_STYLE_XG)
  {
    /* XG bank (128*MSB+LSB), save MSB, do drum-channel auto-switch */
    /* The number "120" was based on several keyboards having drums at 120 - 
127, 
       reference: 
http://lists.nongnu.org/archive/html/fluid-dev/2011-02/msg00003.html */
    chan->channel_type = (120 <= bankmsb) ? CHANNEL_TYPE_DRUM : 
CHANNEL_TYPE_MELODIC;
    return;
  }

Don't "return" there, it has not save MSB yet.  That's why the "if-statement" 
in my patch was written without the "return", so it would flow through to the 
code below.

So if a "return" is preferred from within the block, then the code bock above 
should be changed to:

  if (style == FLUID_BANK_STYLE_XG)
  {
    /* XG bank (128*MSB+LSB), save MSB, zero out LSB, do drum-channel 
auto-switch */
    /* The number "120" was based on several keyboards having drums at 120 - 
127, 
       reference: 
http://lists.nongnu.org/archive/html/fluid-dev/2011-02/msg00003.html */
    chan->channel_type = (120 <= bankmsb) ? CHANNEL_TYPE_DRUM : 
CHANNEL_TYPE_MELODIC;
    oldval = chan->sfont_bank_prog;
    newval = (oldval & ~BANKMSB_MASKVAL) | (bankmsb << (BANK_SHIFTVAL + 7));
    chan->sfont_bank_prog = newval;
    return;
  }

My original patch shares the MMA calculation, using "~BANKMSB_MASKVAL" to 
update the MSB value.


----

There are some midi files which use 2 drum channels in XG mode in:

   psrtutorial.com/songs/Yamaha/XGCurrent.zip

Trying Fluidsynth in XG mode with Unison.sf2:

   fluidsynth  -o synth.midi-bank-select=xg  Unison.sf2  

Playing the midi file "JazzJung Yamaha '96.mid".  Without this patch, using 
latest SVN code, the message I get from fluid command interface:

   fluidsynth: warning: Instrument not found on channel 6 [bank=0 prog=1], 
substituted [bank=0 prog=0]

with this patch, the message is:

   fluidsynth: warning: Instrument not found on channel 6 [bank=16128 prog=1], 
substituted [bank=16128 prog=0]

Which will help figuring the real midi processing behind the scene, 16128 = 
(126 * 128).  So that's [MSB=126,LSB=0,prog=1] it is looking to use.


Jimmy