#8284 closed patch
Better GS support and percussion remapping
| Reported by: | SF/logicdeluxe | Owned by: | sev- |
|---|---|---|---|
| Priority: | normal | Component: | Audio: MT32 |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
I did a patch which sets up GS wavetable synthesizers to similar settings LEC uses with MT-32, while stay compatible with General MIDI at the same time. So it can be used without disturbing GM tone generators. However, one thing still has to be done: Add a check weather a SCUMM v6 game is played, which don't have to emulate MT-32 since it provides GM already, or is it a SCUMM game prior v6 played which has to handle with MT-32 MIDI. There are two blocks in my patch handling this, with comments indicating which one is for which case. Also with that setup the MT-32 to GM velocity conversion turned out to be still a bit too dynamic. I tweaked that to 2/3 instead of 3/4 which also sounds good on plain GM devices.
Ticket imported from: #818939. Ticket imported from: patches/389.
Attachments (3)
Change History (28)
by , 21 years ago
| Attachment: | gs-setup.diff added |
|---|
comment:1 by , 21 years ago
comment:2 by , 21 years ago
| Owner: | set to |
|---|
comment:3 by , 21 years ago
I'm about to look at your patch, but just wanted to point out that initMidiDriver() will indeed get called twice if (1) a native music driver is selected, (2) --multi-midi is enabled, and (3) the game tries to play an Adlib track. In MI2, this happens right at the beginning of the copy prot screen. If you use - eadlib or --no-multi-midi, you should only see one call to initMidiDriver().
comment:4 by , 21 years ago
logicdeluxe, could you tell me from what sources you got these init sequences (the <V6 in particular; the V6+ stuff is straightforward enough) and what you've tested them on? Thanks.
comment:5 by , 21 years ago
I admid, the code is quite uncomplete as it even broke other games. However I bet you can fix that. Monkey Island 1 and Loom EGA have no initialization at all, it seems. Either the progammers thought the user would turn on the MT-32 just before playing and would require the device in the default settings state or they just forgot to do a initialization. I think it doesn't hurt to initialize the hardware anyway, and it would be also a good idea with GM/GS devices. (With MT-32 ScummVM already does this pretty well.)
I heavily testet a lot of MIDI devices as they are my Yamaha MU5 (GM), Terratec Wavetable Professional (GS), Roland VSC-88 (GS), SB-Live Software mode (GS) and Hardware mode (GM) both with the driver's default sound fonts, and of course on my MT-32. I find it sounds well with my initialization on all these devices and GM devices just ignore the GS-SysExes. The original initialization sets the MT-32 to the hall effect called "room" and I just send the corresponding GS SysEx which does the same settings. The GM reset should not just be sent to reset GM devices but also let MIDI devices which understand several standards know that the following is GM compilant so they will switch to a GM mode. The counterpart is the GS reset I do for the V5 games, which is just ignored by GM only devices. That way, we can be sure the music will play as the composers intended them to play. Some specs state that during the reset procedure which take 200 ms no MIDI events should be sent, as I implemented it that way.
There is another thing I'm still fidiling with: I'm still not quite sure about the velocity conversion which should be used, sometimes 2/3 compression sounds correct to me, some other times 3/4 sounds better. I will check, what DOTT and SAM do there as those games support both GM and MT-32, to figure out the "original" way. Does anybody know if DOTT is still composed on MT-32 and then converted to GM or was it the other way around?
And that's I got all those information: I modified DOSBox a bit, so I am able to record all the MPU401 output to a MIDI type 0 file on the fly during playing the game. (See the SAM MIDI I posted to the other bug report!).
by , 21 years ago
| Attachment: | gs-setup-and-rythm-emulation.diff added |
|---|
revised verion also takes care of the rythm map
comment:6 by , 21 years ago
MT-32, but unfortunately not with GM. Even worse, the standard GS drum set already has notes assigned there which are completely different from the LEC setup. (Did you ever notice such strange things like the scratching sound played in the Monkey2 intro for example?) Another difference between MT-32 and GM percussion default setup is that MT-32 defaults to reverb on while GM turns percussion reverb of by default. And that's what LEC did: They assigned some copies of the standard percussion to the keys 24-34, but with no reverb while all the standard percussion have reverb remained on. In particular we have to remap the keys 24 - 34, to GM standard percussion notes 36, 37, 38, 39, 40, 41, 66, 47, 65, 48 and 56. Also we should turn on reverb. I tried turning reverb off while notes 24 - 34 are playing and on when the other notes are playing, but MIDI devices doesn't seem to like those reverb events between notes at all and behave kinda strange at times. Even if I saved the current value and only sent it when it really has changed. This is not the way it would ever work, I afraid. At least, since we can do this remapping, we should use the dafault GM set and not the MT-32 compatible set, which I suggested before. This has turned out not to be fully compatible due to that rythm setup SysEx, anyway. I already did the remapping and removed some of my previous SysEx which set the same as the MIDI resets already did. At least the correct percussion instruments are playing now on GM and GS devices. I also reduced the MIDI traffic for the rythm volume, which really makes a audible difference on some wavetables. There is no need to set the same value hundreds of times.
comment:7 by , 21 years ago
Oops! I accidentally cutted off the beginning of my last post. Here is what I was to post:
You should ignore my last patch! I revised a lot of it, which makes it even more compatible to plain GM devices. However, the proper "if" statements in the GM/GS setup still has to be written. We can emulate those SysEx-Setup for the percussion channel. I figured out the details it actually does: (Note range given in 0-127) In MT-32 and also in the GM specs, the lower notes bellow 35 are not defined for the rythm part, and they actually don't play anything when those notes are sent. Unless, you assign some sounds to them. Well, this can be done with MT-32, but unfortunately not with GM. Even worse, the standard GS drum set already has notes assigned there which are completely different from the LEC setup. (Did you ever notice such strange things like the scratching sound played in the Monkey2 intro for example?)
comment:8 by , 21 years ago
| Summary: | Better GS support → Better GS support and percussion remapping |
|---|
comment:9 by , 21 years ago
I read myself through lots of ScummVM code now. I think I understand it slightly better now and figured out the way it is ment to be implemented. I hope. ;) So here it is: this time a fully working version of my "GS/GM-Setup and percussion remapping for MT-32 emulation"-patch. (just uploading the new patch.)
I noticed one thing, which seems wrong with the Towns driver: The _midi_native flag is set and therefor uint32 IMuseInternal::property(int prop, uint32 value) assumes native MIDI hardware and tries to initialize it, even the MidiDriver instance isn't native. I did a version check to prevent this and a FIXME comment there. It is not a problem with my patch, but I thought you wanna know.
Just another thing: I removed the line // mc->programChange(_bank); as this isn't even ment to be done on the percussion channel neither with the GM nor with GS proposals. It is not defined for that channel! And in fact, as you know, it once did confuse one of my wavetables before it was commented out.
by , 21 years ago
| Attachment: | GM_and_GS_setup.diff added |
|---|
This time it is the 3rd and fully working version
comment:10 by , 21 years ago
A couple notes, and then I'll try to look at this again when I have more time.
_midi_native is not a flag, it is a MidiDriver reference. It points to any non-Adlib MidiDriver -- the only other reference is _midi_adlib. This is how our multi-MIDI works for games like MI2 and FOA.
Sending init strings to the YM2612 MidiDriver shouldn't be a problem because it will essentially ignore everything that's sent. Consequently, there shouldn't be a need to put a version-specific exception into the init code. Did you find reason to think otherwise?
The programChange message has not been removed yet because, though it may not be defined in GM or GS as the proper way to switch percussion banks, it may have meaning to an MT-32. This code was derived from the MI2 logic and may yet be significant for one of the devices for which the game was designed. On the other hand, I haven't heard any reports of my "non-zero _bank" warning trace being encountered, which is a good sign.
comment:11 by , 21 years ago
Well, as I commented, I got an SDL crash in my GS setup routine when I run ZAK256. I didn't investigate this further. Maybe it's the midi->getPercussionChannel() call. The sysex commands doesn't harm, hence it doesn't crash when I run "scummvm --native-mt32 zak256" which actually shouldn't do anything to zak256. I didn't encounter any issues with other games. This crash does not happen with the Indy4town demo, so it apparently isn't a general YM2612 MidiDriver issue, but a problem when using zak256. So there really better should be a way of checking rather the MidiDriver is a native device.
And the MT-32 wouldn't need a bank change for sure. This controller was first documented in the GS standard, afaIk. I have no clue why this was there. The original interpreter doesn't seem to do something like this, check my MIDI files!
I am aware that _midi_native is a pointer, although I assumed it would be a NULL pointer if the MidiDriver instance is an emulation rather than a native device. Maybe it should be renamed to something more clear. Besides this, my patch works very well to me and the percussion remap sounds correct with all my MIDI devices. Very noticable in the MI2 intro, when the monkeys appear for the second time.
comment:12 by , 21 years ago
Ah yes, several MidiDrivers are returning null for getPercussionChannel(), because there is no need for such a distinction. Consequently, the init routine probably just needs to make sure it has a valid pointer before making percussion init calls.
As for the bank change, notice that it's a programChange() call, not a controlChange() call. We aren't sending a control 0 (bank change) on the percussion channel; the idea is to actually switch banks by using a program change message. As I understand it, this is exactly how Roland GS defines the method by which a specific percussion set is to be selected. Whether it also has any meaning for an MT-32, I'm not sure.
comment:13 by , 21 years ago
Then I should enclose the entire GM setup in a block like: if (MidiChannel *p = midi->getPercussionChannel()) and use p at the appropriate place. This should fix the problem then, I guess, and the version check can be removed.
I still wonder why it only crashed with zak256 but not with indy4towns.
Sure, you are right. A program change is valid with GS, but it certainly is not ment to be sent with each single note. The way MT-32 set up the percussion is via SysEx. See my long SysEx which sets up the percussion map like the way I remapped it for GM. And even many GM deviced understand that program change for percussion, I didn't ever saw this used in any LucasArts game. DOTT and SAM would be the only candidates which might doing this anyway, as they are made for GM output. I don't know on what device DOTT was composed. SAM was composed on a Roland SC-55, a GS device, but no GS specific functions are used, as far as I know. If it turns out, there actually is a place, where the program change is used on a percussion channel, it should be only sent when really necessary, like I did it with the volume in my patch.
comment:15 by , 21 years ago
This item is on hold until I can find enough time to revise it to remove g_scumm dependencies. That basically entails setting up a new "GS-compatibility" property that can be set from within Scumm itself. I just need to find a spare hour and a shred of focus. :) (I lost steam on MT-32 code once I found that my own MPU-401 is fried.)
comment:17 by , 20 years ago
As I understand, only g_scumm dependency prevents this patch from being applied. I could untie it. Will it be applied then?
comment:19 by , 20 years ago
I tried this patch with MT-32 emulator. What it basically does is convering MT-32 tracks into GM, i.e. it sounds very close to Adlib emulator. I don't know what to do with it.
Also, speaking of g_scumm dependency, it alsready present in current iMUSE code, so this patch applies as is.
comment:20 by , 20 years ago
g_scumm dependency in any part of the iMUSE code is unacceptable, and the goal is to get away from it completely. The iMUSE module should be able to stand on its own without being tied specifically to a controlling engine.
So where else in iMUSE do we have g_scumm dependencies that need to be obliterated?
comment:21 by , 20 years ago
> grep -n "g_scumm->" imuse*cpp imuse.cpp:746: if (g_scumm->_gameId != GID_SAMNMAX) { imuse.cpp:761: if (g_scumm->_gameId != GID_SAMNMAX) { imuse.cpp:809: if (g_scumm->_gameId == GID_SAMNMAX) { imuse.cpp:819: if (g_scumm->_gameId == GID_SAMNMAX) imuse.cpp:1693: if (g_scumm->_imuse) imuse.cpp:1694: g_scumm->_imuse->on_timer(driver); imuse_player.cpp:400: if (g_scumm->_gameId != GID_SAMNMAX) {
comment:22 by , 20 years ago
Er, that last comment was by me. I guess my login timed out while I was writing it.
comment:24 by , 20 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → closed |
comment:25 by , 6 years ago
| Component: | → Audio: MT32 |
|---|
Note that I noticed my "if ((_midi_native) && (!_native_mt32))" doesn't work as expected. The check is always true with loomcd and zak256 and ScummVM crashes with those games. It also is called even when --native-mt32 is used. I guess you know how to fix this.
Btw. "void IMuseInternal::initMidiDriver(MidiDriver *midi)" is called twice with monkey2.