Opened 16 years ago

Closed 15 years ago

Last modified 11 months ago

#8284 closed patch

Better GS support and percussion remapping

Reported by: SF/logicdeluxe Owned by: sev-
Priority: normal Component: Audio: MT32
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)

gs-setup.diff (2.7 KB ) - added by SF/logicdeluxe 16 years ago.
gs-setup-and-rythm-emulation.diff (3.2 KB ) - added by SF/logicdeluxe 16 years ago.
revised verion also takes care of the rythm map
GM_and_GS_setup.diff (3.7 KB ) - added by SF/logicdeluxe 16 years ago.
This time it is the 3rd and fully working version

Download all attachments as: .zip

Change History (28)

by SF/logicdeluxe, 16 years ago

Attachment: gs-setup.diff added

comment:1 by SF/logicdeluxe, 16 years ago

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.

comment:2 by fingolfin, 16 years ago

Owner: set to SF/jamieson630

comment:3 by SF/jamieson630, 16 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 SF/jamieson630, 16 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 SF/logicdeluxe, 16 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 SF/logicdeluxe, 16 years ago

revised verion also takes care of the rythm map

comment:6 by SF/logicdeluxe, 16 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 SF/logicdeluxe, 16 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 SF/logicdeluxe, 16 years ago

Summary: Better GS supportBetter GS support and percussion remapping

comment:9 by SF/logicdeluxe, 16 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 SF/logicdeluxe, 16 years ago

Attachment: GM_and_GS_setup.diff added

This time it is the 3rd and fully working version

comment:10 by SF/jamieson630, 16 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 SF/logicdeluxe, 16 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 SF/jamieson630, 16 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 SF/logicdeluxe, 16 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:14 by fingolfin, 16 years ago

What is the status of this item?

comment:15 by SF/jamieson630, 16 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:16 by fingolfin, 16 years ago

Fair enough :-) Thanks.

comment:17 by sev-, 15 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:18 by SF/ender, 15 years ago

I don't see any reason why not...

comment:19 by sev-, 15 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 SF/jamieson630, 15 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 sev-, 15 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 SF/tbcarey, 15 years ago

Er, that last comment was by me. I guess my login timed out
while I was writing it.

comment:23 by sev-, 15 years ago

Since renewed patch was commited closing this.

comment:24 by sev-, 15 years ago

Owner: changed from SF/jamieson630 to sev-
Status: newclosed

comment:25 by digitall, 11 months ago

Component: Audio: MT32
Note: See TracTickets for help on using tickets.