Opened 18 years ago

Closed 17 years ago

Last modified 17 months ago

#8064 closed patch

Pitch bend range implementation

Reported by: SF/jamieson630 Owned by: fingolfin
Priority: normal Component: Port: Win32
Keywords: Cc:
Game: Day of the Tentacle

Description

This patch corrects the pitch bend effect when SCUMMVM is using Windows MMSYSTEM (the - ewindows option).

Pitch bend information was being manipulated before being stored, in order to accomodate value ranges for Adlib output. Consequently, it became almost impossible to reconstruct the appropriate values for MIDI messages -- and in fact, the values being sent out were way out of whack.

The original pitch bend values (which are MIDI- compliant) are now what is being stored. Transformations needed for Adlib are now being performed within Adlib functions.

I tested this patch under -ewindows and -eadlib on Day of the Tentacle, primarily using Green Tentacle's band music and the opening logo sounds. I also checked out more subtle pitch bend effects used during the second cutscene (after Bernard finds the secret lab, through the vortex sequence).

If anybody is using other music systems, please test this! There is a highly convoluted relationship between pitch bends, detunes and transpositions that changes from one driver to the next, so I'm not completely sure that I didn't corrupt the pitch bend values needed by other drivers.

The following revisions are patched: imuse.cpp - Revision 1.12 mididrv.cpp - Revision 1.5

(Note: There are a couple miscellaneous tab-indent corrections in this patch as well, just to get things in line with cosmetic guidelines.)

Ticket imported from: #614005. Ticket imported from: patches/169.

Attachments (8)

pitchbend.diff (3.1 KB ) - added by SF/jamieson630 18 years ago.
Patch against a September 24 CVS snapshot
sushi_adlib.mp3 (91.5 KB ) - added by SF/jamieson630 18 years ago.
-eadlib rendering (44KHz 16-bit M, 24 Kbps)
sushi_windows.mp3 (100.9 KB ) - added by SF/jamieson630 18 years ago.
-ewindows, with patch (44KHz 16-bit M, 24 Kbps)
sushi_windows.bad.mp3 (102.5 KB ) - added by SF/jamieson630 18 years ago.
-ewindows, without patch (44KHz 16-bit M, 24 Kbps)
mi2-carnival.diff (753 bytes ) - added by eriktorbjorn 17 years ago.
Patch against an October 4 CVS snapshot
monkey2.s60 (46.1 KB ) - added by eriktorbjorn 17 years ago.
MI2 savegame right before the carnival
imuse.pitchbend2.diff (4.0 KB ) - added by SF/jamieson630 17 years ago.
Patch against iMuse.cpp 1.54 (October 12, 2002)
pitchbend3.diff (4.5 KB ) - added by SF/jamieson630 17 years ago.
Patch against October 21 snapshot

Download all attachments as: .zip

Change History (32)

by SF/jamieson630, 18 years ago

Attachment: pitchbend.diff added

Patch against a September 24 CVS snapshot

comment:1 by SF/jamieson630, 18 years ago

Would help if I actually attach the patch. ;) Thanks to Trinity78 for pointing this out.

comment:2 by fingolfin, 18 years ago

Seems to work OK here on Mac OS X - that is i can't notice any negative effects in the DOTT "rock music" made by green tentacle. Then again I am not sure whether I notice any effects :-) Maybe pitchbend is not implemented here anyway.

comment:3 by SF/ender, 18 years ago

Owner: set to fingolfin

comment:4 by SF/ender, 18 years ago

Fingolfin, didn't you check this in already? :)

comment:5 by SF/jamieson630, 18 years ago

Fingolfin, are you staying that you don't hear ANY pitch bending on Mac OS X? That makes the guitar solo sound really unnatural, more like a keyboard than a wailing guitar.

Would it help if I try to sample the -eadlib and -ewindows output off my box to demonstrate the target sound for that riff?

comment:6 by fingolfin, 18 years ago

Ender, indeed I did - but accidentally :-( I was not being careful enough, shame on me.

jamieson, yeah that would be helpful.

by SF/jamieson630, 18 years ago

Attachment: sushi_adlib.mp3 added

-eadlib rendering (44KHz 16-bit M, 24 Kbps)

comment:7 by SF/jamieson630, 18 years ago

I am attaching three MP3 files to illustrate the pitch bend problem. These are all variations on the stereo music "Green T and the Sushi Platter".

sushi_adlib.mp3 --------------- This is the -eadlib rendering. Since the iMuse code seems to be geared around Adlib output, the pitch bend has always worked fine. In the patch, I moved Adlib-specific transformations to the IMuse_Adlib class, so the output remains unchanged.

sushi_windows.mp3 ----------------- This is the -ewindows rendering with this patch enabled.

sushi_windows.bad.mp3 --------------------- This is the -ewindows rendering without this patch. Notice how all of the pitch bends are virtually non-existent. This is because 1) the pitch bend range for the sound card was not initialized to +/- 12 semitones and is still in its default state of +/- 2 semitones; and 2) the pitch bend values were getting factored down internally for Adlib, which is not an appropriate transformation for MPU-401 output. Consequently, the final pitch bend values would only change the pitch by a few cents.

by SF/jamieson630, 18 years ago

Attachment: sushi_windows.mp3 added

-ewindows, with patch (44KHz 16-bit M, 24 Kbps)

by SF/jamieson630, 18 years ago

Attachment: sushi_windows.bad.mp3 added

-ewindows, without patch (44KHz 16-bit M, 24 Kbps)

comment:8 by fingolfin, 18 years ago

OK so now the QuickTime output sounds aweful. This is because QTMA actually expects the pitchbend data as an 8.8 fixed point, and allows you to specify arbitrary pitchbend this way. So I changed the code to scale the incoming data to +/- 12 semitones, but the result still sounds pretty bad. I am wondering if there is really everything correct? I tried to scale the range to +/- 6 and while that doesn't sound completly correct either, it's much better. Hrm.

comment:9 by fingolfin, 18 years ago

Sorry for that weird wrapping, Internet Explore 5 here is buggy :-(

Anyway I forgot to mention that when I change the pitch bend range for the CoreAudio backend (the other OS X midi backend) by sending it exactly the midi commands you use for the windows backend, it results in the same "wrong" pitch bend sounds. I think that internally the QT driver ultimatly uses CoreAudio anyway, which might explain this shared odditiy, but I am not sure.

comment:10 by SF/jamieson630, 18 years ago

Fingolfin, are you talking about the pitch bends in Sam & Max or DOTT?

For DOTT, the MIDI tracks apparently assume +/- 12 semitones, although the MIDI files don't contain explicit SysEx or control change messages to that effect. (Exception: the starting logo music did include SysEx to do explicit pitch bend sensitivity changes.)

For Sam & Max, I've already noticed a problem with pitch bending. I suspect that the Sam & Max MIDI files finally started using the Standardized MIDI control change messages for changin pitch bend sensitivity. Right now ScummVM has no functionality to interpret those. (In fact, this patch here doesn't even deal with SysEx messages to change pitch bend sensitivity -- it just assumes it will be +/- 12 semitones for everything.)

The above gaps in functionality are something I can address once I'm off work next week. In the meantime, Fingolfin, can you verify for me that DOTT at least sounds okay on OS X, and if not, can you upload an MP3 recording of the Green T output so I can hear how the pitch bends sound on that version? Thanks.

comment:11 by fingolfin, 18 years ago

Actually all my tests were made with Green Ts rock music!

Sorry I have no clue how I could capture the audio output here to a file... I'll look into it maybe I'll find a way.

by eriktorbjorn, 17 years ago

Attachment: mi2-carnival.diff added

Patch against an October 4 CVS snapshot

comment:12 by eriktorbjorn, 17 years ago

I have to apply this tiny patch, or the carnival music at the end of Monkey Island 2 sounds terrible.

It simply makes the same pitchbend change to mc_key_on() that the original patch does to part_changed().

by eriktorbjorn, 17 years ago

Attachment: monkey2.s60 added

MI2 savegame right before the carnival

comment:13 by SF/ender, 17 years ago

Can this be closed yet?

comment:14 by SF/jamieson630, 17 years ago

Still working on some issues with QT and CoreAudio -- this is slow going since I don't have access to a Mac locally. The QT problems seem to be more than just the *range* of pitch bending, but still related to pitch bend issues in general, so I'd like to see it addressed in this patch.

Any feedback yet on ALSA pitch bend behaviors?

by SF/jamieson630, 17 years ago

Attachment: imuse.pitchbend2.diff added

Patch against iMuse.cpp 1.54 (October 12, 2002)

comment:15 by SF/jamieson630, 17 years ago

I'm including imuse.pitchbend2.diff, which provides more thorough support for pitch bend ranges. Sam & Max use on- the-fly changes to the pitch bend range in various musical passages. Without tracking the current pitch bend range, and sending out appropriate signals, several of the musical passages (including the opening cutscene) sound out-of-tune or otherwise screwy.

I kind of hesitated to upgrade the part_changed parameter to a 16-bit value, but I couldn't really see any other option. Pitch bend range needs to be tracked just like any of those other parameters -- and there weren't any more bits to play with.

(Note: this patch does not address any QT issues I mentioned earlier. Still mulling over those. Fingolfin and I are about ready to chalk it up to QTMA pitch bend implementation bugs.... So, Ender, if you want to close this, we can always just open up something new if we hit on a solution.)

comment:16 by SF/ender, 17 years ago

Applied. Fingolfin, it's up to you whether to close this or not.

comment:17 by eriktorbjorn, 17 years ago

I tried the DOTT intro and Green T music with the ALSA driver, and it didn't sound correct. It wasn't bending the notes far enough. Unfortunately I don't know how to capture the sound to a file.

I tried humming the note at the end of the "boing" sound at the beginning of the intro and comparing it to my piano, and I think it came out as an E flat. Does that help at all?

comment:18 by eriktorbjorn, 17 years ago

Hmm... Now the pitch bend for MI2 and DOTT is broken. The problem seems to have been introduced when the default _pitchbend_factor was changed from 2 to 12.

Which games is it that needs 12 by default?

comment:19 by SF/jamieson630, 17 years ago

Okay, the +/- 12 semitone pitch bend range is not a valid assumption for DOTT after all, but apparently the channels were initially set up with no range at all. I have gone back to +/- 2 semitones unless the MIDI track calls for something else, but the new architecture based on part_changed seems to still play the DOTT music (and relevant MI2 music, e.g. Largo's theme) correctly under Adlib and Windows.

I have committed changes that should greatly improve pitch bend behavior under QTMA, especially in Sam & Max. However, being without a Mac I have no idea what the results will be. Fingolfin?

(BTW, looking at the QTMA code, I would think that the control change messages used to set pitch bend range for Windows MMSYSTEM would cause an "Unknown MIDI effect" error under QTMA. This may need to be reclassified as a warning to avoid unwarranted program termination.)

comment:20 by SF/jamieson630, 17 years ago

Summary: Pitch bend for MMSYSTEMPitch bend range implementation

by SF/jamieson630, 17 years ago

Attachment: pitchbend3.diff added

Patch against October 21 snapshot

comment:21 by SF/jamieson630, 17 years ago

Ack -- I broke the Mac compile with the latest commit. That's what comes of doing Mac changes on Windows at 4 in the morning. ;)

I am attaching a patch with corrections, but I won't directly commit this time; Fingolfin, can you review these for syntactical correctness and then let me know how it affects QT output?

comment:22 by fingolfin, 17 years ago

Status: newclosed

comment:23 by fingolfin, 17 years ago

Closing this, pitch bend works mostly fine, any remaining problem can be resolved outside of this tracker item.

comment:24 by digitall, 17 months ago

Component: Port: Win32
Game: Day of the Tentacle
Note: See TracTickets for help on using tickets.