#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 |
| Version: | 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)
Change History (9)
comment:1 by , 19 years ago
comment:2 by , 19 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 , 19 years ago
| Owner: | set to |
|---|
comment:4 by , 19 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 , 19 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
comment:6 by , 19 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 , 19 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 , 5 years ago
| Component: | → Engine: SCUMM |
|---|---|
| Game: | → Monkey Island 1 |
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()