SCI: Improve audio volume & settings sync code

This patch includes enhancements to the ScummVM integration with
SCI engine, with particular focus on SCI32 support.

1. Fixes audio volumes syncing erroneously to ScummVM in games
   that modify the audio volume without user action (e.g. SCI1.1
   talkies that reduce music volume during speech playback). Now,
   volumes will only be synchronised when the user interacts with
   the game's audio settings. This mechanism works by looking for
   a known volume control object in the stack, and only syncing
   when the control object is present. (Ports and planes were
   researched and found unreliable.)

2. Fixes audio syncing in SCI32 games that do not set game
   volumes through kDoSoundMasterVolume/kDoAudioVolume, like GK1,
   GK2, Phant1, and Torin.

3. Fixes speech/subtitles syncing in SCI32 games that do not use
   global 90, like LSL6hires.

4. Fixes in-game volume controls in SCI32 games reflecting
   outdated audio volumes when a change is made during the game
   from the ScummVM launcher.

5. Fixes SCI32 games that would restore volumes from save games
   or reset volumes on startup, which caused game volumes to be
   out-of-sync with ScummVM when started.

6. ScummVM integration code for audio sync has been abstracted
   into a new GuestAdditions class. This keeps the ScummVM-
   specific code all in one place, with only small hooks into the
   engine code. ScummVM integrated save/load code should probably
   also go here in the future.

Fixes Trac#9700.

1 files changed, 72 insertions, 59 deletions

diff --git a/engines/sci/sound/soundcmd.cpp b/engines/sci/sound/soundcmd.cpp
index b9a764c93a..e0cbb97d19 100644
--- a/engines/sci/sound/soundcmd.cpp
+++ b/engines/sci/sound/soundcmd.cpp
@@ -29,6 +29,7 @@
 #include "sci/sound/soundcmd.h"
 
 #include "sci/engine/features.h"
+#include "sci/engine/guest_additions.h"
 #include "sci/engine/kernel.h"
 #include "sci/engine/object.h"
 #include "sci/engine/selector.h"
@@ -57,10 +58,10 @@ SoundCommandParser::~SoundCommandParser() {
 	delete _music;
 }
 
-reg_t SoundCommandParser::kDoSoundInit(int argc, reg_t *argv, reg_t acc) {
+reg_t SoundCommandParser::kDoSoundInit(EngineState *s, int argc, reg_t *argv) {
 	debugC(kDebugLevelSound, "kDoSound(init): %04x:%04x", PRINT_REG(argv[0]));
 	processInitSound(argv[0]);
-	return acc;
+	return s->r_acc;
 }
 
 int SoundCommandParser::getSoundResourceId(reg_t obj) {
@@ -153,13 +154,13 @@ void SoundCommandParser::processInitSound(reg_t obj) {
 	}
 }
 
-reg_t SoundCommandParser::kDoSoundPlay(int argc, reg_t *argv, reg_t acc) {
+reg_t SoundCommandParser::kDoSoundPlay(EngineState *s, int argc, reg_t *argv) {
 	debugC(kDebugLevelSound, "kDoSound(play): %04x:%04x", PRINT_REG(argv[0]));
 	bool playBed = false;
 	if (argc >= 2 && !argv[1].isNull())
 		playBed = true;
 	processPlaySound(argv[0], playBed);
-	return acc;
+	return s->r_acc;
 }
 
 void SoundCommandParser::processPlaySound(reg_t obj, bool playBed) {
@@ -225,10 +226,10 @@ void SoundCommandParser::processPlaySound(reg_t obj, bool playBed) {
 	musicSlot->fadeStep = 0;
 }
 
-reg_t SoundCommandParser::kDoSoundDispose(int argc, reg_t *argv, reg_t acc) {
+reg_t SoundCommandParser::kDoSoundDispose(EngineState *s, int argc, reg_t *argv) {
 	debugC(kDebugLevelSound, "kDoSound(dispose): %04x:%04x", PRINT_REG(argv[0]));
 	processDisposeSound(argv[0]);
-	return acc;
+	return s->r_acc;
 }
 
 void SoundCommandParser::processDisposeSound(reg_t obj) {
@@ -248,10 +249,10 @@ void SoundCommandParser::processDisposeSound(reg_t obj) {
 		writeSelectorValue(_segMan, obj, SELECTOR(state), kSoundStopped);
 }
 
-reg_t SoundCommandParser::kDoSoundStop(int argc, reg_t *argv, reg_t acc) {
+reg_t SoundCommandParser::kDoSoundStop(EngineState *s, int argc, reg_t *argv) {
 	debugC(kDebugLevelSound, "kDoSound(stop): %04x:%04x", PRINT_REG(argv[0]));
 	processStopSound(argv[0], false);
-	return acc;
+	return s->r_acc;
 }
 
 void SoundCommandParser::processStopSound(reg_t obj, bool sampleFinishedPlaying) {
@@ -282,7 +283,7 @@ void SoundCommandParser::processStopSound(reg_t obj, bool sampleFinishedPlaying)
 	_music->soundStop(musicSlot);
 }
 
-reg_t SoundCommandParser::kDoSoundPause(int argc, reg_t *argv, reg_t acc) {
+reg_t SoundCommandParser::kDoSoundPause(EngineState *s, int argc, reg_t *argv) {
 	if (argc == 1)
 		debugC(kDebugLevelSound, "kDoSound(pause): %04x:%04x", PRINT_REG(argv[0]));
 	else
@@ -333,7 +334,7 @@ reg_t SoundCommandParser::kDoSoundPause(int argc, reg_t *argv, reg_t acc) {
 		if (!musicSlot) {
 			// This happens quite frequently
 			debugC(kDebugLevelSound, "kDoSound(pause): Slot not found (%04x:%04x)", PRINT_REG(obj));
-			return acc;
+			return s->r_acc;
 		}
 
 #ifdef ENABLE_SCI32
@@ -354,17 +355,17 @@ reg_t SoundCommandParser::kDoSoundPause(int argc, reg_t *argv, reg_t acc) {
 #endif
 			_music->soundToggle(musicSlot, shouldPause);
 	}
-	return acc;
+	return s->r_acc;
 }
 
 // SCI0 only command
 //  It's called right after restoring a game - it's responsible to kick off playing music again
 //  we don't need this at all, so we don't do anything here
-reg_t SoundCommandParser::kDoSoundResumeAfterRestore(int argc, reg_t *argv, reg_t acc) {
-	return acc;
+reg_t SoundCommandParser::kDoSoundResumeAfterRestore(EngineState *s, int argc, reg_t *argv) {
+	return s->r_acc;
 }
 
-reg_t SoundCommandParser::kDoSoundMute(int argc, reg_t *argv, reg_t acc) {
+reg_t SoundCommandParser::kDoSoundMute(EngineState *s, int argc, reg_t *argv) {
 	uint16 previousState = _music->soundGetSoundOn();
 	if (argc > 0) {
 		debugC(kDebugLevelSound, "kDoSound(mute): %d", argv[0].toUint16());
@@ -374,37 +375,33 @@ reg_t SoundCommandParser::kDoSoundMute(int argc, reg_t *argv, reg_t acc) {
 	return make_reg(0, previousState);
 }
 
-reg_t SoundCommandParser::kDoSoundMasterVolume(int argc, reg_t *argv, reg_t acc) {
-	acc = make_reg(0, _music->soundGetMasterVolume());
+reg_t SoundCommandParser::kDoSoundMasterVolume(EngineState *s, int argc, reg_t *argv) {
+	s->r_acc = make_reg(0, _music->soundGetMasterVolume());
 
 	if (argc > 0) {
 		debugC(kDebugLevelSound, "kDoSound(masterVolume): %d", argv[0].toSint16());
 		int vol = CLIP<int16>(argv[0].toSint16(), 0, MUSIC_MASTERVOLUME_MAX);
-		vol = vol * Audio::Mixer::kMaxMixerVolume / MUSIC_MASTERVOLUME_MAX;
-		ConfMan.setInt("music_volume", vol);
-		// In SCI32, digital audio volume is controlled separately by
-		// kDoAudioVolume
-		if (_soundVersion < SCI_VERSION_2_1_EARLY) {
-			ConfMan.setInt("sfx_volume", vol);
+
+		if (!g_sci->_guestAdditions->kDoSoundMasterVolumeHook(vol)) {
+			setMasterVolume(vol);
 		}
-		g_engine->syncSoundSettings();
 	}
-	return acc;
+	return s->r_acc;
 }
 
-reg_t SoundCommandParser::kDoSoundFade(int argc, reg_t *argv, reg_t acc) {
+reg_t SoundCommandParser::kDoSoundFade(EngineState *s, int argc, reg_t *argv) {
 	reg_t obj = argv[0];
 
 	// The object can be null in several SCI0 games (e.g. Camelot, KQ1, KQ4, MUMG).
 	// Check bugs #3035149, #3036942 and #3578335.
 	// In this case, we just ignore the call.
 	if (obj.isNull() && argc == 1)
-		return acc;
+		return s->r_acc;
 
 	MusicEntry *musicSlot = _music->getSlot(obj);
 	if (!musicSlot) {
 		debugC(kDebugLevelSound, "kDoSound(fade): Slot not found (%04x:%04x)", PRINT_REG(obj));
-		return acc;
+		return s->r_acc;
 	}
 
 	int volume = musicSlot->volume;
@@ -412,7 +409,7 @@ reg_t SoundCommandParser::kDoSoundFade(int argc, reg_t *argv, reg_t acc) {
 #ifdef ENABLE_SCI32
 	if (_soundVersion >= SCI_VERSION_2_1_EARLY && musicSlot->isSample) {
 		g_sci->_audio32->fadeChannel(ResourceId(kResourceTypeAudio, musicSlot->resourceId), musicSlot->soundObj, argv[1].toSint16(), argv[2].toSint16(), argv[3].toSint16(), argc > 4 ? (bool)argv[4].toSint16() : false);
-		return acc;
+		return s->r_acc;
 	}
 #endif
 
@@ -420,7 +417,7 @@ reg_t SoundCommandParser::kDoSoundFade(int argc, reg_t *argv, reg_t acc) {
 	if (musicSlot->status != kSoundPlaying) {
 		debugC(kDebugLevelSound, "kDoSound(fade): %04x:%04x fading requested, but sound is currently not playing", PRINT_REG(obj));
 		writeSelectorValue(_segMan, obj, SELECTOR(signal), SIGNAL_OFFSET);
-		return acc;
+		return s->r_acc;
 	}
 
 	switch (argc) {
@@ -439,7 +436,7 @@ reg_t SoundCommandParser::kDoSoundFade(int argc, reg_t *argv, reg_t acc) {
 		// Check if the song is already at the requested volume. If it is, don't
 		// perform any fading. Happens for example during the intro of Longbow.
 		if (musicSlot->fadeTo == musicSlot->volume)
-			return acc;
+			return s->r_acc;
 
 		// Sometimes we get objects in that position, so fix the value (refer to workarounds.cpp)
 		if (!argv[1].getSegment())
@@ -464,14 +461,14 @@ reg_t SoundCommandParser::kDoSoundFade(int argc, reg_t *argv, reg_t acc) {
 	}
 
 	debugC(kDebugLevelSound, "kDoSound(fade): %04x:%04x to %d, step %d, ticker %d", PRINT_REG(obj), musicSlot->fadeTo, musicSlot->fadeStep, musicSlot->fadeTickerStep);
-	return acc;
+	return s->r_acc;
 }
 
-reg_t SoundCommandParser::kDoSoundGetPolyphony(int argc, reg_t *argv, reg_t acc) {
+reg_t SoundCommandParser::kDoSoundGetPolyphony(EngineState *s, int argc, reg_t *argv) {
 	return make_reg(0, _music->soundGetVoices());	// Get the number of voices
 }
 
-reg_t SoundCommandParser::kDoSoundUpdate(int argc, reg_t *argv, reg_t acc) {
+reg_t SoundCommandParser::kDoSoundUpdate(EngineState *s, int argc, reg_t *argv) {
 	reg_t obj = argv[0];
 
 	debugC(kDebugLevelSound, "kDoSound(update): %04x:%04x", PRINT_REG(argv[0]));
@@ -479,7 +476,7 @@ reg_t SoundCommandParser::kDoSoundUpdate(int argc, reg_t *argv, reg_t acc) {
 	MusicEntry *musicSlot = _music->getSlot(obj);
 	if (!musicSlot) {
 		warning("kDoSound(update): Slot not found (%04x:%04x)", PRINT_REG(obj));
-		return acc;
+		return s->r_acc;
 	}
 
 	musicSlot->loop = readSelectorValue(_segMan, obj, SELECTOR(loop));
@@ -489,12 +486,12 @@ reg_t SoundCommandParser::kDoSoundUpdate(int argc, reg_t *argv, reg_t acc) {
 	int16 objPrio = readSelectorValue(_segMan, obj, SELECTOR(priority));
 	if (objPrio != musicSlot->priority)
 		_music->soundSetPriority(musicSlot, objPrio);
-	return acc;
+	return s->r_acc;
 }
 
-reg_t SoundCommandParser::kDoSoundUpdateCues(int argc, reg_t *argv, reg_t acc) {
+reg_t SoundCommandParser::kDoSoundUpdateCues(EngineState *s, int argc, reg_t *argv) {
 	processUpdateCues(argv[0]);
-	return acc;
+	return s->r_acc;
 }
 
 void SoundCommandParser::processUpdateCues(reg_t obj) {
@@ -608,7 +605,7 @@ void SoundCommandParser::processUpdateCues(reg_t obj) {
 	}
 }
 
-reg_t SoundCommandParser::kDoSoundSendMidi(int argc, reg_t *argv, reg_t acc) {
+reg_t SoundCommandParser::kDoSoundSendMidi(EngineState *s, int argc, reg_t *argv) {
 	// The 4 parameter variant of this call is used in at least LSL1VGA, room
 	// 110 (Lefty's bar), to distort the music when Larry is drunk and stands
 	// up - bug #3614447.
@@ -637,13 +634,13 @@ reg_t SoundCommandParser::kDoSoundSendMidi(int argc, reg_t *argv, reg_t acc) {
 		// if so, allow it
 		//_music->sendMidiCommand(_midiCommand);
 		warning("kDoSound(sendMidi): Slot not found (%04x:%04x)", PRINT_REG(obj));
-		return acc;
+		return s->r_acc;
 	}
 	_music->sendMidiCommand(musicSlot, midiCommand);
-	return acc;
+	return s->r_acc;
 }
 
-reg_t SoundCommandParser::kDoSoundGlobalReverb(int argc, reg_t *argv, reg_t acc) {
+reg_t SoundCommandParser::kDoSoundGlobalReverb(EngineState *s, int argc, reg_t *argv) {
 	byte prevReverb = _music->getCurrentReverb();
 	byte reverb = argv[0].toUint16() & 0xF;
 
@@ -656,7 +653,7 @@ reg_t SoundCommandParser::kDoSoundGlobalReverb(int argc, reg_t *argv, reg_t acc)
 	return make_reg(0, prevReverb);
 }
 
-reg_t SoundCommandParser::kDoSoundSetHold(int argc, reg_t *argv, reg_t acc) {
+reg_t SoundCommandParser::kDoSoundSetHold(EngineState *s, int argc, reg_t *argv) {
 	reg_t obj = argv[0];
 
 	debugC(kDebugLevelSound, "doSoundSetHold: %04x:%04x, %d", PRINT_REG(argv[0]), argv[1].toUint16());
@@ -664,24 +661,24 @@ reg_t SoundCommandParser::kDoSoundSetHold(int argc, reg_t *argv, reg_t acc) {
 	MusicEntry *musicSlot = _music->getSlot(obj);
 	if (!musicSlot) {
 		warning("kDoSound(setHold): Slot not found (%04x:%04x)", PRINT_REG(obj));
-		return acc;
+		return s->r_acc;
 	}
 
 	// Set the special hold marker ID where the song should be looped at.
 	musicSlot->hold = argv[1].toSint16();
-	return acc;
+	return s->r_acc;
 }
 
-reg_t SoundCommandParser::kDoSoundGetAudioCapability(int argc, reg_t *argv, reg_t acc) {
+reg_t SoundCommandParser::kDoSoundGetAudioCapability(EngineState *s, int argc, reg_t *argv) {
 	// Tests for digital audio support
 	return make_reg(0, 1);
 }
 
-reg_t SoundCommandParser::kDoSoundStopAll(int argc, reg_t *argv, reg_t acc) {
+reg_t SoundCommandParser::kDoSoundStopAll(EngineState *s, int argc, reg_t *argv) {
 	// TODO: this can't be right, this gets called in kq1 - e.g. being in witch house, getting the note
 	//  now the point jingle plays and after a messagebox they call this - and would stop the background effects with it
 	//  this doesn't make sense, so i disable it for now
-	return acc;
+	return s->r_acc;
 
 	Common::StackLock(_music->_mutex);
 
@@ -697,10 +694,10 @@ reg_t SoundCommandParser::kDoSoundStopAll(int argc, reg_t *argv, reg_t acc) {
 		(*i)->dataInc = 0;
 		_music->soundStop(*i);
 	}
-	return acc;
+	return s->r_acc;
 }
 
-reg_t SoundCommandParser::kDoSoundSetVolume(int argc, reg_t *argv, reg_t acc) {
+reg_t SoundCommandParser::kDoSoundSetVolume(EngineState *s, int argc, reg_t *argv) {
 	reg_t obj = argv[0];
 	int16 value = argv[1].toSint16();
 
@@ -711,7 +708,7 @@ reg_t SoundCommandParser::kDoSoundSetVolume(int argc, reg_t *argv, reg_t acc) {
 		// the drum sounds of the energizer bunny at the beginning), so this is
 		// normal behavior.
 		//warning("cmdSetSoundVolume: Slot not found (%04x:%04x)", PRINT_REG(obj));
-		return acc;
+		return s->r_acc;
 	}
 
 	debugC(kDebugLevelSound, "kDoSound(setVolume): %d", value);
@@ -724,15 +721,20 @@ reg_t SoundCommandParser::kDoSoundSetVolume(int argc, reg_t *argv, reg_t acc) {
 		_music->soundSetVolume(musicSlot, value);
 	}
 #endif
+
 	if (musicSlot->volume != value) {
 		musicSlot->volume = value;
 		_music->soundSetVolume(musicSlot, value);
 		writeSelectorValue(_segMan, obj, SELECTOR(vol), value);
+#ifdef ENABLE_SCI32
+		g_sci->_guestAdditions->kDoSoundSetVolumeHook(obj, value);
+#endif
 	}
-	return acc;
+
+	return s->r_acc;
 }
 
-reg_t SoundCommandParser::kDoSoundSetPriority(int argc, reg_t *argv, reg_t acc) {
+reg_t SoundCommandParser::kDoSoundSetPriority(EngineState *s, int argc, reg_t *argv) {
 	reg_t obj = argv[0];
 	int16 value = argv[1].toSint16();
 
@@ -741,7 +743,7 @@ reg_t SoundCommandParser::kDoSoundSetPriority(int argc, reg_t *argv, reg_t acc)
 	MusicEntry *musicSlot = _music->getSlot(obj);
 	if (!musicSlot) {
 		debugC(kDebugLevelSound, "kDoSound(setPriority): Slot not found (%04x:%04x)", PRINT_REG(obj));
-		return acc;
+		return s->r_acc;
 	}
 
 	if (value == -1) {
@@ -759,10 +761,10 @@ reg_t SoundCommandParser::kDoSoundSetPriority(int argc, reg_t *argv, reg_t acc)
 
 		_music->soundSetPriority(musicSlot, value);
 	}
-	return acc;
+	return s->r_acc;
 }
 
-reg_t SoundCommandParser::kDoSoundSetLoop(int argc, reg_t *argv, reg_t acc) {
+reg_t SoundCommandParser::kDoSoundSetLoop(EngineState *s, int argc, reg_t *argv) {
 	reg_t obj = argv[0];
 	int16 value = argv[1].toSint16();
 
@@ -780,7 +782,7 @@ reg_t SoundCommandParser::kDoSoundSetLoop(int argc, reg_t *argv, reg_t acc) {
 		} else {
 			// Doesn't really matter
 		}
-		return acc;
+		return s->r_acc;
 	}
 
 #ifdef ENABLE_SCI32
@@ -805,13 +807,13 @@ reg_t SoundCommandParser::kDoSoundSetLoop(int argc, reg_t *argv, reg_t acc) {
 	}
 #endif
 
-	return acc;
+	return s->r_acc;
 }
 
-reg_t SoundCommandParser::kDoSoundSuspend(int argc, reg_t *argv, reg_t acc) {
+reg_t SoundCommandParser::kDoSoundSuspend(EngineState *s, int argc, reg_t *argv) {
 	// TODO
 	warning("kDoSound(suspend): STUB");
-	return acc;
+	return s->r_acc;
 }
 
 void SoundCommandParser::updateSci0Cues() {
@@ -879,6 +881,17 @@ void SoundCommandParser::setMasterVolume(int vol) {
 	_music->soundSetMasterVolume(vol);
 }
 
+#ifdef ENABLE_SCI32
+void SoundCommandParser::setVolume(const reg_t obj, const int volume) {
+	MusicEntry *slot = _music->getSlot(obj);
+	if (slot != nullptr) {
+		slot->volume = volume;
+		writeSelectorValue(_segMan, obj, SELECTOR(vol), volume);
+		_music->soundSetVolume(slot, volume);
+	}
+}
+#endif
+
 void SoundCommandParser::pauseAll(bool pause) {
 	_music->pauseAll(pause);
 }