Opened 17 years ago

Closed 17 years ago

Last modified 12 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 17 years ago.
Patch against a September 24 CVS snapshot
sushi_adlib.mp3 (91.5 KB ) - added by SF/jamieson630 17 years ago.
-eadlib rendering (44KHz 16-bit M, 24 Kbps)
sushi_windows.mp3 (100.9 KB ) - added by SF/jamieson630 17 years ago.
-ewindows, with patch (44KHz 16-bit M, 24 Kbps)
sushi_windows.bad.mp3 (102.5 KB ) - added by SF/jamieson630 17 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, 17 years ago

Attachment: pitchbend.diff added

Patch against a September 24 CVS snapshot

comment:1 by SF/jamieson630, 17 years ago

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

comment:2 by fingolfin, 17 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, 17 years ago

Owner: set to fingolfin

comment:4 by SF/ender, 17 years ago

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

comment:5 by SF/jamieson630, 17 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, 17 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, 17 years ago

Attachment: sushi_adlib.mp3 added

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

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

Attachment: sushi_windows.mp3 added

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

by SF/jamieson630, 17 years ago

Attachment: sushi_windows.bad.mp3 added

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

comment:8 by fingolfin, 17 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, 17 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, 17 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, 17 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, 12 months ago

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