#1245 closed defect (fixed)
ALL: pitchbend range with MT32
| Reported by: | SF/logicdeluxe | Owned by: | SF/jamieson630 |
|---|---|---|---|
| Priority: | normal | Component: | Audio: MT32 |
| Version: | 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)
Change History (13)
comment:1 by , 21 years ago
| Owner: | set to |
|---|
comment:2 by , 21 years ago
comment:3 by , 21 years ago
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 by , 21 years ago
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 by , 21 years ago
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 by , 21 years ago
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 by , 21 years ago
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 by , 21 years ago
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 by , 21 years ago
| Status: | new → closed |
|---|
comment:10 by , 21 years ago
| Resolution: | → fixed |
|---|
comment:12 by , 6 years ago
| Component: | --Unset-- → Audio: MT32 |
|---|---|
| Game: | → Day of the Tentacle |
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.