Opened 15 years ago

Closed 15 years ago

Last modified 12 months ago

#1955 closed defect (fixed)

MONKEYVGA: PCjr Causes Indefinite Pause At 'Part' Screens

Reported by: SF/tbcarey Owned by: SF/hoenicke
Priority: normal Component: Engine: SCUMM
Keywords: Cc:
Game: Monkey Island 1

Description

Using PCjr emulation in MONKEYVGA [Monkey 1 256 Color
Floppy English] causes the game to pause at every
'Part' screen after the accompanying music finishes.
For example, at the 'Part I: The Three Trials' screen,
the game will not advance until the escape key is
pressed. This only occurs with PCjr emulation (all
other music types tested) and only at those specific
screens, which leads me to believe it's linked somehow
to that particular song / WM block. This does not occur
under DOSBox using PCjr output with the original
interpreter, so it is related to ScummVM's parser and
not the block itself.

Ticket imported from: #1163202. Ticket imported from: bugs/1955.

Attachments (1)

player_v2.diff (537 bytes ) - added by eriktorbjorn 15 years ago.
Patch against a March 15 CVS snapshot

Download all attachments as: .zip

Change History (9)

comment:1 by SF/tbcarey, 15 years ago

Debug info shows that ScummVM gets caught in an infinite
loop at:

getResourceAddress(Room,96) == 029BAE9C
Script 200, offset 0x3383: [FC] o5_isSoundRunning()
readvar(81)
writeVar(0, 1)
Script 200, offset 0x3388: [28] o5_equalZero()
readvar(0)
Script 200, offset 0x3382: [80] o5_breakHere()

by eriktorbjorn, 15 years ago

Attachment: player_v2.diff added

Patch against a March 15 CVS snapshot

comment:2 by eriktorbjorn, 15 years ago

That means ScummVM is waiting for the music to finish,
without realizing that it already has. I have a possible fix
for this - see attachment - but I don't know for sure if
it's the correct one. Particularly since it makes the
chapter screen disappear just before the music ends...

comment:3 by eriktorbjorn, 15 years ago

Owner: set to SF/hoenicke

comment:4 by eriktorbjorn, 15 years ago

On the other hand, the chapter screen seems to disappear a
split second before the music ends with PC speaker and Adlib
music as well, so maybe that's normal. Or I'm imagining things.

Perhaps I should clarify what the patch does:

If I understand things correctly, do_mix() is the function
that's ultimately responsible for generating the sound data,
and nextTick() is the one that's responsible for updating
the states of the sound channels.

The nextTick() function will call next_freqs() on any
channel where time_left is non-zero. The next_freqs()
function will decrease time_left, and if it's still non-zero
it will call execute_cmd(). So as long as there are still
channels with a non-zero time_left, new samples will be
generated. Of course, execute_cmd() may increase the
channel's time_left.

However, getSoundStatus() does not look at time_left, it
looks at _current_nr and _next_nr.

The problem, as far as I can tell, happens because the last
calls to execute_cmd() - the ones that bring all time_left
values down to 0 - are called on channels where next_cmd is
also 0. And in this case the function terminates almost at
once. With my patch, it will do the "end of track" / "go to
next track" checks at the end of the the function first.

My main worry is, of course, that this will cause something
else to break. I can't think of what, but I'm not overly
familiar with this part of the code.

comment:5 by SF/hoenicke, 15 years ago

Resolution: fixed
Status: newclosed

comment:6 by SF/hoenicke, 15 years ago

It looks like the original code does some kind of counting
of the sound-script channels and stops when the last script
channel finishes. I don't want to copy this complicated
code, though. It looks a bit buggy (probably it works only
because there is only one sound-script channel in all
sounds). I'm not even sure whether it is compatible to v2 games.

The fix I just checked in is almost the same as the one
eriktorbjorn proposed.

comment:7 by SF/hoenicke, 15 years ago

I'd like to comment a bit on eriktorbjorn's comment :)

The "split second early"-problem is probably due to sound
buffering. We calculate 100ms sound in advance and there is
probably further buffering in backend, library, OS, and/or
hardware.

To understand the problem with execute_cmd, one needs to
know a bit more about the channels. In player_v2 there are
four channels, each channel can play a single note
(corresponding directly to the four hardware channels in the
PCjr) and/or it can have a script, that determines which
notes to play.

Although in principle each channel can have its own script,
normally only one of the four channels has a script and this
script puts notes into the other channels. In V2 games,
when this script has finished it checks that the other
channels are idle too and terminates the sound. This fails
for the "part screen" song as there is still a note in
channel 2 playing. In V3 games it seems to be slightly
different, as it only checks if the current channel is idle
and decreases some counter. When the counter is 0, the
sound is terminated. I have not investigated when the
counter is increased.

What eriktorbjorn and my patch do is to check if all
channels are idle (time_left == 0), even if the channel was
only playing a note and not executing a script (next_cmd ==
0). This way it will stop the sound after the last note has
finished.

I don't think that can break anything. If all channels are
idle, there can't be any progress so it is right to stop the
sound in this case.

comment:8 by digitall, 12 months ago

Component: Engine: SCUMM
Game: Monkey Island 1
Note: See TracTickets for help on using tickets.