Opened 16 years ago

Closed 16 years ago

Last modified 16 years ago

#1063 closed defect (fixed)

DOTT: Hanging notes (regression?)

Reported by: eriktorbjorn Owned by: SF/jamieson630
Priority: high Component: Engine: SCUMM
Keywords: Cc:
Game: Day of the Tentacle

Description

[Extracted from bug report #775097]

English CD "talkie" version.

At the end of the game, when the kids are hiding from
purple tentacle, the music changes. When it does, some
notes are usually (always?) left hanging. The same
thing also happens sometimes when I push nurse Edna out
of her room, but that's harder for me to reproduce.

I've look a bit closer at the first of these cases, and
I think it must have something to do with the "smart
jumping" logic in the MIDI parser, because if I disable
this property I no longer get any hanging notes.

From what I understand, smart jumping means that
instead of silencing all notes at once and then jump to
a new position, it leaves the current notes to fade out
on their own, thus allowing for a smoother transition.
This involves scanning the track for note off events or
the end of the track, to see when the notes would have
been turned off.

I don't know what the unit of time measurement is, so I
don't know what are reasonable values and what aren't,
but when the track loops the time_left parameter is
somewhere around 10000, and it only has to scan a few
events ahead to figure that out.

Which makes the following calls to hangingNote(), which
I captured during one test run, look rather suspicious
to me:

hangingNote(11, 67, 8099928, 0)
hangingNote(11, 71, 8615520, 0)
hangingNote(11, 74, 16625052, 0)
hangingNote(1, 35, 72028872, 0)
hangingNote(2, 67, 78392304, 0)
hangingNote(6, 67, 104966496, 0)

Not only that, but it had to scan hundreds - maybe
thousands - of events ahead to find them.

Ticket imported from: #775654. Ticket imported from: bugs/1063.

Attachments (3)

tentacle.s32.gz (17.1 KB) - added by eriktorbjorn 16 years ago.
Savegame near the end of the game
tentacle.s50.gz (10.9 KB) - added by eriktorbjorn 16 years ago.
Savegame closer to the problem
tentacle.s22.gz (19.0 KB) - added by eriktorbjorn 16 years ago.
Savegame in nurse Edna's room

Download all attachments as: .zip

Change History (16)

Changed 16 years ago by eriktorbjorn

Attachment: tentacle.s32.gz added

Savegame near the end of the game

Changed 16 years ago by eriktorbjorn

Attachment: tentacle.s50.gz added

Savegame closer to the problem

comment:1 Changed 16 years ago by eriktorbjorn

Owner: set to SF/jamieson630

Changed 16 years ago by eriktorbjorn

Attachment: tentacle.s22.gz added

Savegame in nurse Edna's room

comment:2 Changed 16 years ago by eriktorbjorn

What I'm wondering is, is "smart jump" a necessary feature
for iMUSE, or is it just icing on the cake? If it's the
latter, then one solution (albeit a rather heavy-handed one)
would be to simply disable it for 0.5.0.

Or maybe the bug is easy to fix and I'm just worrying over
nothing.

comment:3 Changed 16 years ago by SF/jamieson630

The functionality now called Smart Jump has been
implemented in the earliest versions of ScummVM
as "sustaining notes". Eliminating it does produce a noticeable
difference during interactive jumps in the music (usually it
sounds a bit "thin" during the jump, because too many
instruments suddenly go silent at once). However, I agree
that if the current Smart Jump code is still producing hanging
notes, disabling it might be preferable for the release.

The numbers in your data spew are intriguing. I hope to
address this one soon. (Computer emergencies are keeping
me occupied right now.) Any problems with Smart Jump are of
substantial concern to me.

comment:4 Changed 16 years ago by SF/jamieson630

Priority: normalhigh

comment:5 Changed 16 years ago by eriktorbjorn

A less drastic hack might be to clamp the time_left value to
something sensible.

By the way, it seems to happen during the Sam & Max intro as
well, but it's much less noticeable there. The DOTT case is
a bit extreme in that respect I guess, since it changes from
a quite loud track to a quite soft one.

comment:6 Changed 16 years ago by eriktorbjorn

Is it really enough for hangAllActiveNotes() to just check
for note off and end of track events? The "note on with
velocity 0" case seems to be covered by midiparser_smf.cpp
silently transforming it into a "note off" event, but what
about controller events?

On the other hand, my attempt to add checks for "all notes
off" and "all sounds off" had absolutely no effect at all,
so maybe I'm just spouting nonsense...

comment:7 Changed 16 years ago by SF/ender

Can we get a solution on this soon? I wanna be able to
finalise 0.5.0 this week...

comment:8 Changed 16 years ago by SF/jamieson630

Give me a drop-dead date, Ender. I'm in the middle of an
emergency and won't even be able to look at this in depth
until Tuesday at the earliest. If a solution can't be delivered
by the drop-dead date of your choice, I recommend going
with eriktorbjorn's suggestion that we disable SmartJump
altogether for 0.5.0.

eriktorbjorn: Controller events shouldn't be a problem because
they were also not accounted for in the original Sustaining
Notes code. That code also just looked for Note Off and End
of Track events. (It also looked for Note On events under
some very strange and convoluted circumstances.) As to why
it would end up scanning thousands of events and coming up
with such outrageously large sustaining times (which are in
microseconds, BTW), I could not begin to postulate on until
I've actually traced through the code at the problem area.

comment:9 Changed 16 years ago by SF/jamieson630

Resolution: fixed

comment:10 Changed 16 years ago by SF/jamieson630

I love it. As soon as I post that I don't have time, I find a
little window in which to do some debugging.

I have committed a fix to the trunk that takes care of the
hanging notes when Purple Tentacle is chasing the Three-In-
One Monster. The regression occurred when I modified the
relationship between sustaining notes and active notes to
correct MI2 bug 766426 (V5 games: Adlib SFX not looped).
Sustaining notes were left in the active notes list, which
causes a problem if a track switch occurs. (Normally a track
switch is followed immediately by a jump to tick. If sustaining
notes are still in the active notes list, that's a problem
because those notes were NOT part of this new track, so of
course reasonably timed Note Off events for those notes are
not going to be found.)

I do not have time to test this with the Edna scene.
eriktorbjorn? If this works fine there as well, someone needs
to commit it to the 0.5.0 branch. I had a problem doing so
last time I tried to get something from the trunk to the
branch.

Will leave this bug open until fix is verified.

comment:11 Changed 16 years ago by eriktorbjorn

I don't seem to be able to reproduce the Edna bug any more
in the trunk, even though I made several attempts. (I never
figured out exactly how to trigger it, but I only needed
three attempt with the 0.5.0 branch.)

So it's probably fixed there as well.

comment:12 Changed 16 years ago by SF/jamieson630

Fix committed to branch-0-5-0. Closing.

comment:13 Changed 16 years ago by SF/jamieson630

Status: newclosed
Note: See TracTickets for help on using tickets.