[Scummvm-git-logs] scummvm master -> 715c9f699363ea7fa5d05e6dfd42455a2118eb2f

[athrxx]

This automated email contains information about 2 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
3b4515b337 GRIFFON: get rid of "too many sound handles" error
715c9f6993 GRIFFON: fix invalid mem access / crash


Commit: 3b4515b33795a51f8089d88b02c02286b9fb1321
    https://github.com/scummvm/scummvm/commit/3b4515b33795a51f8089d88b02c02286b9fb1321
Author: athrxx (athrxx at scummvm.org)
Date: 2021-02-26T14:48:42+01:00

Commit Message:
GRIFFON: get rid of "too many sound handles" error

The error is triggered quite easily. You can get that right at the start, walking 1 screen left, engaging multiple monsters simultaneously.

That code looks a bit suspicious per se, since it still tries to return a value of -1 after the error call. But maybe that's just to satisfy some compiler. I've replaced the error with proper handling of the return value and I've also added in-bounds-checks for all uses of the sound handles.

Changed paths:
    engines/griffon/sound.cpp


diff --git a/engines/griffon/sound.cpp b/engines/griffon/sound.cpp
index 00a27eb4b5..c15f6e7567 100644
--- a/engines/griffon/sound.cpp
+++ b/engines/griffon/sound.cpp
@@ -46,7 +46,8 @@
 namespace Griffon {
 
 void GriffonEngine::setChannelVolume(int channel, int volume) {
-	_mixer->setChannelVolume(_handles[channel], volume);
+	if (channel >= 0 && channel < kSoundHandles)
+		_mixer->setChannelVolume(_handles[channel], volume);
 }
 
 int GriffonEngine::getSoundHandle() {
@@ -56,24 +57,23 @@ int GriffonEngine::getSoundHandle() {
 		}
 	}
 
-	error("getSoundHandle(): Too many sound handles");
-
 	return -1;
 }
 
 int GriffonEngine::playSound(DataChunk *chunk, bool looped) {
-	int ch = getSoundHandle();
+	int ch = -1;
 
 #ifdef USE_VORBIS
-	Audio::SeekableAudioStream *audioStream = Audio::makeVorbisStream(new Common::MemoryReadStream(chunk->data, chunk->size), DisposeAfterUse::YES);
-
+	if ((ch = getSoundHandle()) != -1) {
+		Audio::SeekableAudioStream *audioStream = Audio::makeVorbisStream(new Common::MemoryReadStream(chunk->data, chunk->size), DisposeAfterUse::YES);
 
-	if (looped) {
-		Audio::AudioStream *loopingStream = new Audio::LoopingAudioStream(audioStream, 0, DisposeAfterUse::YES);
-		_mixer->playStream(Audio::Mixer::kSFXSoundType, &_handles[ch], loopingStream, -1, Audio::Mixer::kMaxChannelVolume, 0, DisposeAfterUse::YES, false, false);
+		if (looped) {
+			Audio::AudioStream *loopingStream = new Audio::LoopingAudioStream(audioStream, 0, DisposeAfterUse::YES);
+			_mixer->playStream(Audio::Mixer::kSFXSoundType, &_handles[ch], loopingStream, -1, Audio::Mixer::kMaxChannelVolume, 0, DisposeAfterUse::YES, false, false);
 
-	} else {
-		_mixer->playStream(Audio::Mixer::kSFXSoundType, &_handles[ch], audioStream, -1, Audio::Mixer::kMaxChannelVolume, 0, DisposeAfterUse::YES, false, false);
+		} else {
+			_mixer->playStream(Audio::Mixer::kSFXSoundType, &_handles[ch], audioStream, -1, Audio::Mixer::kMaxChannelVolume, 0, DisposeAfterUse::YES, false, false);
+		}
 	}
 #endif // USE_VORBIS
 
@@ -81,24 +81,26 @@ int GriffonEngine::playSound(DataChunk *chunk, bool looped) {
 }
 
 void GriffonEngine::pauseSoundChannel(int channel) {
-	_mixer->pauseHandle(_handles[channel], true);
+	if (channel >= 0 && channel < kSoundHandles)
+		_mixer->pauseHandle(_handles[channel], true);
 }
 
 void GriffonEngine::haltSoundChannel(int channel) {
 	if (channel == -1) {
 		for (int i = 0; i < kSoundHandles; i++)
 			_mixer->stopHandle(_handles[i]);
-	} else {
+	} else if (channel >= 0 && channel < kSoundHandles) {
 		_mixer->stopHandle(_handles[channel]);
 	}
 }
 
 void GriffonEngine::resumeSoundChannel(int channel) {
-	_mixer->pauseHandle(_handles[channel], false);
+	if (channel >= 0 && channel < kSoundHandles)
+		_mixer->pauseHandle(_handles[channel], false);
 }
 
 bool GriffonEngine::isSoundChannelPlaying(int channel) {
-	return _mixer->isSoundHandleActive(_handles[channel]);
+	return (channel >= 0 && channel < kSoundHandles) ? _mixer->isSoundHandleActive(_handles[channel]) : false;
 }
 
 DataChunk *cacheSound(const char *name) {


Commit: 715c9f699363ea7fa5d05e6dfd42455a2118eb2f
    https://github.com/scummvm/scummvm/commit/715c9f699363ea7fa5d05e6dfd42455a2118eb2f
Author: athrxx (athrxx at scummvm.org)
Date: 2021-02-26T14:48:48+01:00

Commit Message:
GRIFFON: fix invalid mem access / crash

This can be triggered by walking 1 screen left and 1 screen down from the start.

When loading a new map the game resets only onMap for all members of the _npcInfo array, but does not always check that value when iterating over the (valid and invalid) entries of the array.

Changed paths:
    engines/griffon/logic.cpp


diff --git a/engines/griffon/logic.cpp b/engines/griffon/logic.cpp
index 1a6a6c962e..20c7a067c5 100644
--- a/engines/griffon/logic.cpp
+++ b/engines/griffon/logic.cpp
@@ -109,6 +109,9 @@ void GriffonEngine::updateY() {
 	_lasty = 0;
 
 	for (int i = 1; i <= _lastNpc; i++) {
+		if (!_npcInfo[i].onMap)
+			continue;
+
 		int yy = (int)(_npcInfo[i].y * 10);
 
 		do {