Opened 16 years ago

Closed 16 years ago

Last modified 8 months ago

#1245 closed defect (fixed)

ALL: pitchbend range with MT32

Reported by: SF/logicdeluxe Owned by: SF/jamieson630
Priority: normal Component: Audio: MT32
Keywords: Cc:
Game: Day of the Tentacle

Description

ScummVM 0.5.3cvs (Sep 25 2003 20:58:26) with the MT32
patch applied, but also happens without it.
Even with --native-mt32 the pitchbend range is
converted to GM and the MT-32 remains on its defaults,
which sounds wrong in most cases. For instance listen
to the bends when the team enters the car in the DOTT
intro, which are way too much!

Ticket imported from: #812737. Ticket imported from: bugs/1245.

Attachments (1)

mt32-bend-range.diff (1.9 KB) - added by SF/logicdeluxe 16 years ago.
fixes MT-32 pitch bends

Download all attachments as: .zip

Change History (13)

comment:1 Changed 16 years ago by fingolfin

Owner: set to SF/jamieson630

comment:2 Changed 16 years ago by SF/jamieson630

Ref. patch 814285. I have no way of testing my fix since I
don't have an MT-32. Please test the patch, logicdeluxe, and
post results there.

Changed 16 years ago by SF/logicdeluxe

Attachment: mt32-bend-range.diff added

fixes MT-32 pitch bends

comment:3 Changed 16 years ago by SF/logicdeluxe

That patch doesn't work that way. It doesn't patch the code
in the send all code. And even worse, the SysEx is wrong
anyway, if I can trust my MT-32 documents. The SysEx you had
in mind would probably look more like this:
byte buffer[] = { 0x41, 0x16, 0x16, 0x12, 0x03, 0x00,
0x04, 0x00, 0x00 };
buffer[6] = 4 + 0x10 * _mc->getNumber();
I tried this as well, but it didn't work either. But never
mind, I figured out a solution anyway. See my patch!

I just observed a difference to the original DOTT which
seemed to be important to me:
When starting DOTT in GM mode, it sets the bend range for
all channels to 16 and stay there forever whiles ScummVM
sets the bend range dynamically for the current cue and
resets to the ScummVM internal defaults whenever the cannel
is allocated to another cue. Those completely different ways
of archiving the same result might work well in GM but asks
for trouble in MT-32 mode. Unfortunately I have no way to
find out what the original does during initializing to the
MT-32 exactly (if there is even anything bend range related)
since my displays doesn't show that information. My guess
is, that it doesn't do anything about the bend ranges and
always assumes it set to 12, as the MT-32 initializes itself
that way.
In fact, all the music in DOTT never seem to go deeper than
12 half tones, so it is a completely mystory to me why it is
set up to 16 instead of 12. My patch simply scales the pitch
bends relatively to the internally kept pitchbend_factor in
order to match the depths of 12 and it works pretty well.

Will I be added to the credits as MT-32 tester and fixer, now?

comment:4 Changed 16 years ago by SF/jamieson630

First, you are correct, sendAll() doesn't correctly send the
picthbend range. A separate method to send the data should
be called from any client code now calling _mc-
>pitchBendRange().

Second, for a justification of the approach I used in my SysEx
message, see scummvm/scumm/instrument.cpp. The code to
send SysEx Roland instrument data was adapted from code
found online (if you want to know where, I'll have to find it
again), and verified by khalek. Apparently you can write into
the first-position temp area, using the device ID to specify a
MIDI channel (since SysEx messages don't have an intrinsic
MIDI channel). This works for the Timbre temp area, at least
(note how we write every instrument into 40 00 00).

Your approach, anyway, would have to be based on _mc-
>getNumber() - 1, not _mc->getNumber(). Offset 0 is
actually MIDI channel 1 (zero-based index).

Third, the problem I have with your alternative solution is that
it is storing persistent data differently based on the hardware
device. We have gone through a great deal of trouble to get
away from that, so that savegames from one hardware device
will still sound reasonable when run with another hardward
device. By your solution, a savegame from a native MT32 run
would sound odd when run on GM because _pitchbend has
been "adjusted". What we want to do is store the value the
same way regardless of output device, and do any hardware-
specific transformations on-the-fly. We could do that with a
Part::sendPitchBend() method, but I would prefer to get the
SysEx pitchbend range working -- if for no other reason than
that we need more working code examples in ScummVM of
how to send SysEx data to the MT-32.

comment:5 Changed 16 years ago by SF/logicdeluxe

Well, you are right, my last sysEx suggestion was wrong, too.
But this one seems perfectly logical to me, but doesn't work
either. This even produces random crashes in my MT-32 unit.
I added the same sysEx to sendAll() as well, which
definately would be required there as well and has to be
prevented on to be sent on channel 0 there (which seem to
happen when Adlib SFX are played). (Of course this could be
done in one sigle method. I didn't check the latest changes
yet!) Are you sure, setting the temp area is the right way
to do this? Here my try, which unfortunately doesn't work
either:
byte buffer[] = { 0x41, 0x10, 0x16, 0x12, 0x03, 0x00,
0x04, 0x00, 0x00 };
buffer[6] = 0x04 + 0x10 * (_mc->getNumber() - 1);
buffer[7] = value;
buffer[8] = (0 - buffer[4] - buffer[5] - buffer[6] -
buffer[7]) & 0x7F;
_mc->device()->sysEx(buffer, 9);

Again to my last patch, I understand the problem you mention
for savegame compatibility. So you could modify my last
patch to something like that, so it doesn't harm the
consistency of the _pitchbend variable. Also this would be
needed in the sendAll() routine as well. So what's about this?:
_pitchbend = value;
if (_mc) {
if (!_player->_se->isNativeMT32())
_mc->pitchBend(clamp(_pitchbend +
(_detune_eff * 64 / 12) +
(_transpose_eff * 8192 / 12), -8192, 8191));
} else {
_mc->pitchBend(clamp(_pitchbend * _pitchbend_factor / 12 +
(_detune_eff * 64 / 12) +
(_transpose_eff * 8192 / 12), -8192, 8191));
}
}

comment:6 Changed 16 years ago by SF/jamieson630

Like I said, the temp area is how we transmit timbre
information to the MT-32. We do so by writing to the same
slot every time, and just vary the device ID that's passed in.
It works for the timbres, but I couldn't say why, so I wouldn't
know whether it's a stretch to assume it would work for
patches too.

Hmm, there *is* another Patch Temp area at address 00 00
00 (how convenient). But it's accessed differently. My docs
say "accessible on each basic channel", whereas the other
temp areas are "accessible on unit #". The latter, I presume,
means the device # that we make use of. The former makes
little sense to me, since there's no way to specify a channel
number on a SysEx message.

Go ahead and try using address 00 00 00 in conjunction with
the device #, in the manner of my original patch. So...

byte buf[] = { 0x41, 0x00, 0x16, 0x12, 0x00, 0x00, 0x04,
0x00, 0x00 };
buf[1] = _mc->getNumber(); // Device (unit) ID #
buf[7] = value;
buf[8] = (0 - buf[4] - buf[5] - buf[6] - buf[7]) & 0x7F;
_mc->device()->sysEx(buffer, 9);

If that doesn't work, I notice that that address block extends
from 00 00 00 to 01 00 00, so there's room for Patch Temp
blocks at 00 00 10, 00 00 20, etc., per the approach that
you've been advocating. (The docs I'm using don't indicate
that, as they usually do, but who knows.) Try that as well:

byte buf[] = { 0x41, 0x10, 0x16, 0x12, 0x00, 0x00, 0x04,
0x00, 0x00 };
buf[6] += 0x10 * (_mc->getNumber() - 1);
buf[7] = value;
buf[8] = (0 - buf[4] - buf[5] - buf[6] - buf[7]) & 0x7F;
_mc->device()->sysEx(buffer, 9);

Let me know if either of those work. I'd really like to figure
out how to get access to that Bender Range data member
through SysEx. If we can't figure out a way to do that over
the next 24 hours, though, I'll go ahead and implement the
_pitchbend scaling approach for now.

comment:7 Changed 16 years ago by SF/logicdeluxe

Also my docs say "accesible on each basic channel" for the
adress space below 03 00 00 and I don't have a clue what
that means. As you said, sysex have no channels. This is in
deed very confusing.
And while my docs list
03 00 00 Patch Temp Area (part 1)
03 00 10 Patch Temp Area (part 2)
etc. this is not the case for those adresses below 03 00 00.
Anyway, I tried your suggestions. Nothing happens.

comment:8 Changed 16 years ago by SF/jamieson630

Alright, since we can't seem to get any SysEx solutions to
work, I'm implemented on-the-fly manual pitchbend scaling in
CVS for now. A deviation from your patch: since the MT32
does not respond to pitchBendFactor() call, I saw no reason
to add code to filter it out. Check out CVS and let me know if
that does the trick.

comment:9 Changed 16 years ago by SF/jamieson630

Status: newclosed

comment:10 Changed 16 years ago by SF/jamieson630

Resolution: fixed

comment:11 Changed 16 years ago by SF/jamieson630

No further response, I assume it's working. Closing.

comment:12 Changed 8 months ago by digitall

Component: --Unset--Audio: MT32
Game: Day of the Tentacle
Note: See TracTickets for help on using tickets.