Opened 4 years ago

Closed 2 years ago

#11435 closed defect (fixed)

SCI: SQ1 (Macintosh VGA Remake) - broken music

Reported by: vanfanel Owned by: waltervn
Priority: normal Component: Engine: SCI
Version: Keywords: Macintosh, SCI, music
Cc: vanfanel Game: Space Quest 1

Description

Hi there,

I recently got the Macintosh version of Space Quest 1 VGA Remake, and its datafiles are recognized by Scummvm.
Game seems to work in general, but music is broken: notes keep sounding after another note plays in that channel, etc.
It can be clearly seen in the Arcada music, as soon as the game starts.

Apparently, there IS a patch that fixes music in Macintosh games, bu waltervn. The branch with the fixed Macintosh music is here:
https://github.com/waltervn/scummvm/tree/sci_sound
It has been talked about on the forums already, years ago:
https://forums.scummvm.org/viewtopic.php?t=14293

However, this patch never got merger into master so it now seems to have been lost for years.
Maybe merging this patch into current master can be enough?

I am using latest git version, HEAD at https://github.com/scummvm/scummvm/commit/e6fa61f0e2b7319aba0060b26718f518bd252bdc

Change History (13)

comment:1 by m-kiewitz, 4 years ago

I doubt walter forgot about it.
My guess is that he is rather carefully checking everything to make sure nothing gets broken.

comment:2 by waltervn, 4 years ago

One of the issues is that our SCI sound system implementation has known issues, which makes it more difficult to properly test any drivers.

In any case, I'll start rebasing my branch, and then let's see where we go from there.

comment:3 by ZvikaZ, 4 years ago

Hi @waltervn.

I've fixed few SCI sound issues in accepted PRs, and have few more fixes that still need review.
It'd be great if you can review them...

https://github.com/scummvm/scummvm/pull/2128
https://github.com/scummvm/scummvm/pull/2165

I'm willing to try and fix other SCI sound bugs (as best as I can...), can you please point me to the open bugs you were referring to?
Or, if there aren't bugs describing the problems, elaborate here on the known issues in the SCI sound system?

comment:4 by m-kiewitz, 4 years ago

Hello ZvikaZ,

Unless this has been changed already, one of the major issues is that we use the common ScummVM Midi-parser and it isn't really doing what SCI was actually doing for MIDI. It works somewhat, but it isn't accurate and this causes quite a few issues.

comment:5 by m-kiewitz, 4 years ago

If I remember correctly one issue is for example that the MIDI parser is seeking to specific ticks, but the SCI parser was seeking to exact byte offsets. This may not sound as a major issue, but it is, especially when loops and such are involved.
For certain tracks we start at the wrong offset because of this (too early, or too late), which then makes it parse MIDI commands that should or should not get processed.

If that is still the case, I personally don't see a way to fix this without actually rewriting the whole MIDI parser for SCI specifically.

comment:6 by ZvikaZ, 4 years ago

Thanks for the explanation.
Can you point me to specific situation in a game that sounds bad because of this?

comment:7 by m-kiewitz, 4 years ago

Oh and I mean writing a SCI specific MIDI parser. I would avoid touching the common MIDI parser that much, because it works fine for all the other games. SCI simply did some things quite differently.

@ZvikaZ There should be open bug reports for these cases. In the worst case they were closed at some point. I can't remember specific games, but there were a few. I looked into certain bugs and also looked into the SCI 16-bit MIDI parser and found out about the byte offsets.

For example in SCI MIDI data there are byte codes for setting up a loop and SCI remembered that exact byte offset and when a loop got triggered, it went to that exact offset.

In ScummVM this code remembers the tick and seeks to that tick. At least back then when I looked, the ScummVM common MIDI parser did not support seeking to byte offsets, only ticks.

I think some bug reports were about incorrect instruments for example.

It isn't possible to quickly fix it all, especially because so many games are involved and most work right now and putting band aids on this doesn't really make sense.

comment:8 by ZvikaZ, 4 years ago

Well, you guys did an amazing work. Starting from investigating Sierra's SCI, building FreeSCI, and ScummVM's SCI engine.
It's really huge.
And IMO, it's better than what you think ;-)

I've started to learn and investigate the SCI sound bugs few months ago, and the situation (again, IMO) is very good.
I contributed few fixes, that aren't specific band-aid type, but catching some small general case mismatches.

Please let me give a short overview of the recent sound bug fixes, and the remaining sound bugs still open:

https://github.com/scummvm/scummvm/pull/2074 fixed:
https://bugs.scummvm.org/ticket/6686 (General MIDI using SC-55 misses instruments) that was opened for 6 years,
https://bugs.scummvm.org/ticket/9578 (SQ6 wrong volume) that was opened for 3 years,
https://bugs.scummvm.org/ticket/9735 (GK1 missing score jingle) that was opened for 3 years,
https://bugs.scummvm.org/ticket/10297 (QFG1VGA: Wrong instruments),
https://bugs.scummvm.org/ticket/11012 (MIDI volume is not reduced when character voices are speaking)

https://github.com/scummvm/scummvm/pull/2146 fixed https://bugs.scummvm.org/ticket/7073 (KQ6-CD: Savegame that cannot be resumed - but it's due to sound problem) that was open for 4 years and https://bugs.scummvm.org/ticket/10685 (GK1 music issue on restore)

https://github.com/scummvm/scummvm/pull/2149 fixed https://bugs.scummvm.org/ticket/9907 (LB1: Sound effect not playing when discovering corpse)

https://github.com/scummvm/scummvm/pull/2182 fixed https://bugs.scummvm.org/ticket/11409 (not-emulated-SC problems, in KQ6 and GK1)

Indeed, I closed https://bugs.scummvm.org/ticket/5115 (Missing instruments in Jones) and https://bugs.scummvm.org/ticket/6165 (Pepper: Hanging note) without applying fixes, because I didn't hear any problem, the user hasn't responded to questions, and they were probably fixed by some sound change in the passing years.

There are 2 PRs that fix more bugs, and need to be reviewed:

https://github.com/scummvm/scummvm/pull/2128 (which @bluegr started reviewing, but stopped - it'd be great if someone can review this...) will fix:
https://bugs.scummvm.org/ticket/10812 (KQ6CD wallflower dance music not restarting, leads to lockup), https://bugs.scummvm.org/ticket/10907 (Wrong music when restoring games) and unreported bug (restoring restarts the playing music from the beginning)

https://github.com/scummvm/scummvm/pull/2165 is the only sound fix that I saw that's of the local band-aid kind - for one of the issues described at https://bugs.scummvm.org/ticket/11265 (SQ4 audio problem), but I think it's due to Sierra's SCI bug, as I wrote inside the PR.

So, what's still open?

https://bugs.scummvm.org/ticket/5307 (PQ3: channel volumes are wrong when using Adlib) - I'm not sure that it's still relevant, as I wrote there.
https://bugs.scummvm.org/ticket/6687 (Quitting while MIDI plays causes hanging notes) - I have suggested a fix, but waiting for someone to verify it, as I don't have the hardware...
https://bugs.scummvm.org/ticket/9579 (Music volume at zero still plays music very softly) - not really a bug, but a reminder
https://bugs.scummvm.org/ticket/9580 (GK2: Improve bad AV sync in GK2 VMDs)
https://bugs.scummvm.org/ticket/9803 (SQ6: Quiet music on loading save game) - should be fixed by the accepted PR #2074
https://bugs.scummvm.org/ticket/10734 (LB1: Turn-crank and open-vault sound fx not playing)
https://bugs.scummvm.org/ticket/11049 (Circus quest - no sound) - I'm not sure that it's a ScummVM bug (as I wrote there)
https://bugs.scummvm.org/ticket/11265 (SQ4 audio problem) - I'm working on it - the aforementioned PR #2165, and it needs more work that I plan to do after the PR will be accepted.
https://bugs.scummvm.org/ticket/11354 (CASTLEBRAIN: Stereo audio flipped in hourglass puzzle) - I understood the problem, but not sure what's the best way to fix it - wrote about it in the bug - comments are welcomed!
https://bugs.scummvm.org/ticket/11435 (this bug...)

lighthouse also has few "sound" problems, but according to the bug reports they are due to script bugs, and not sound system bugs.

So, bottom line, we had several old bugs that were fixed, 2 PRs waiting to be reviewed, and we have very small (reported) issues left after that, none of them requiring massive rewrites of the sound system.

"If I have seen further it is by standing on the shoulders of Giants."

comment:9 by waltervn, 4 years ago

I've rebased the new (although by now pretty old actually) Mac and Amiga drivers here: https://github.com/waltervn/scummvm/tree/sci_sound_amigamac

Since this is mostly untested code it's likely to have some bugs. It'll probably also need refactoring in some places for memory safety and possibly getting rid of some large tables.

There's at least one major bug in the Mac SCI1 code causing samples to loop over and over. I'm trying to track that one down.

I'm not part of the SCI team anymore, but I'm willing to help land this if it is decided to use this stuff.

comment:10 by waltervn, 4 years ago

Just a quick update. I started working on this code again. I'll probably need another two or three weeks to clean it up, then I'll open a PR so we can discuss/test it.

comment:11 by waltervn, 4 years ago

OK, I'm a little burned out on this stuff now, so I think it's probably a good time to ask for some testing (thanks to Trembyle who already did a fair bit of testing with SCI1 Mac). Since SCI0 Mac games do not work, I think we should just skip the testing there and revisit it once these games are supported. So testing should be done for Amiga (SCI0 and SCI1) and Mac (SCI1).

You can find the branch at https://github.com/waltervn/scummvm/tree/sci_sound_amigamac

Version 0, edited 4 years ago by waltervn (next)

comment:12 by raziel-, 4 years ago

Summary: Macintosh Space Quest 1 VGA Remake: broken musicSCI: SQ1 (Macintosh VGA Remake) - broken music

comment:13 by sluicebox, 2 years ago

Owner: set to waltervn
Resolution: fixed
Status: newclosed

SQ1VGA Mac, along with the other SCI1 games, now sound great thanks to waltervn. With his branch having been merged long ago and the original reporter's major issue taken care of, I'm closing this as fixed. Any remaining minor issues can be reported separately.

Note: See TracTickets for help on using tickets.