fluidsynth icon indicating copy to clipboard operation
fluidsynth copied to clipboard

Expected XG System ON sysex message is wrong

Open jjazzboss opened this issue 2 years ago • 1 comments

XG specs page 42 says XG System Mode ON sysex depends on the deviceId n coded on 4 bits: F0H, 43H, 1nH (0001nnnn), 4CH, ...

That's why everywhere on the web the XG System ON sequence is described as F0H, 43H, 10H, 4CH, ..., assuming that most of the time deviceId is 0.

However in method fluid_synth_sysex() in fluid_synth_.c, the data[1] comparison is directly done on the deviceId value:

if(data[0] == MIDI_SYSEX_MANUF_YAMAHA
            && (data[1] == synth->device_id || data[1] == MIDI_SYSEX_DEVICE_ID_ALL)
            && data[2] == MIDI_SYSEX_XG_ID)

I understand it's consistent with this doc, but doing so it ignores the standard XG initialization sequence found in most of the XG midi files out there.

jjazzboss avatar May 02 '22 17:05 jjazzboss

An easy improvement would be to update the doc of fluid_synth_sysex() to detail the expected XG ON sysex sequence, which is not 100% standard. A workaround for users wanting to be able to react to the "standard XG ON sequence" is to set their synth.device-id=16.

jjazzboss avatar May 02 '22 17:05 jjazzboss

Ok, I found some time looking into this and proposed a fix in #1171, pls. have a look. @kode54 originally implemented this XG sysex handling, he might be interested as well.

derselbst avatar Oct 08 '22 11:10 derselbst

Will be fixed in 2.3.1

derselbst avatar Oct 12 '22 18:10 derselbst

I took the liberty to add the fix for Attempt to fix XG System ON sysex message parsing #1171 https://github.com/FluidSynth/fluidsynth/pull/1171/commits/bc8ddc167cee5e3895eaa76cba5e1c5d67b6a6b6 in the code. Indeed MIDI XG files are properly detected as XG files now.

But this change has a wide consequence. There are lots of MIDI XG files out there which use channel 10 for drums but have MSB == 0 or <= 120.

In fluid_chan.c

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

with the correct detection of the XG format by the change in #1171 and MSB <= 120, the default CHANNEL_TYPE_DRUM is changed to CHANNEL_TYPE_MELODIC.

The consequence is that before the fix of #1171 a XG format was NOT correctly detected. The drums have been used from the soundfont bank 128 and have sounded properly as drums. With the fix #1171 the XG format is properly detected but with MSB == 0, the drum channel is identified as CHANNEL_TYPE_MELODIC and the drums sound as piano.

One can debate whether the XG format was not correctly used with MSB == 0 in those MIDI files, but it is what it is. XG files with MSB <= 120 exist. Before #1171 the drums of certain MIDI files sounded as drums. With #1171 all of a sudden the drum tracks sound as piano. Usually, Yamaha keyboard owners are familiar that MSB has to be == 127, but people who just play MIDI files on PCs are not.

I am attaching such a MIDI file in the zip folder where the drum track is with MSB == 0.

Before #1171 becomes officially available the consequences should be clear that the drum track of certain MIDI files won't sound correctly any longer and potentially need a correction, or the /* XG bank, do drum-channel auto-switch */ logic in fluid_chan.c needs to be re-considered.

Oh_When_The_Saints_Go_Marching_In.zip

ReinholdH avatar Nov 29 '22 06:11 ReinholdH

Thanks @ReinholdH, I'll try to look into this in the upcoming days. @jjazzboss When you came across this issue, did you acutally had a MIDI that did not behave correctly or were you just "digging" in the specs? And if you have a MIDI which did not behave correcly before the fix in #1171 but now does, would you be able to share that MIDI as well? (that would potentially help to understand the issue and make a design decision here)

derselbst avatar Nov 29 '22 09:11 derselbst

@derselbst I'm not familiar with XG Midi files, I ran into the issue while trying to control FluidSynth from my JJazzLab app in XG mode.

I checked the attached Midi file from @ReinholdH: it has first a GM System ON sysex message, then a XG System ON sysex. And because all tracks use BankSelect MSB=0 and LSB=0, indeed it makes fluidsynth believe all tracks are melodic.

I was curious so I Googled "XG midi files" and randomly downloaded 10 files on different web sites: all files were XG-correct, ie they used MSB=127 for the drums channel. But maybe I was lucky...

One solution could be to add a setting for the fluidsynth Midi player which says "treat the Midi file as GM", i.e. ignore the (non-GM) sysex messages and bank select messages. This way users could get the same behavior as before introducing #1171.

jjazzboss avatar Nov 29 '22 21:11 jjazzboss

I had some time to look into this and can basically confirm what Jerome already said. However, I'm not in favor of introducing yet another setting for controlling this tiny aspect. Instead I would fallback to Jermoe's suggestion in his second comment that people are advised to set deviceID to 16 (or whatever is used in their SysEx initialization sequence). Hope that's ok for you guys.

derselbst avatar Dec 19 '22 10:12 derselbst

Ok, we can live with that. Meanwhile we added a code change in our app that for an XG file a drum channel's MSB=0 is changed to MSB=127. This change comes with our next version when we will use fluidsynth 2.3.1 or higher. The discussion here was very fruitful. Thx.

ReinholdH avatar Dec 19 '22 11:12 ReinholdH