[Scummvm-git-logs] scummvm master -> 231d6a51839f3295bc7dc289f3fd870c2e7c6848

[sluicebox]

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

Summary:
922d5480dc SCI: Fix mixing SCI1 sound channels when none exist
231d6a5183 SCI: Validate SCI1 sound headers


Commit: 922d5480dc6828b2d55023acca0c6176a542839e
    https://github.com/scummvm/scummvm/commit/922d5480dc6828b2d55023acca0c6176a542839e
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2025-10-03T10:15:05-07:00

Commit Message:
SCI: Fix mixing SCI1 sound channels when none exist

Several games contain truncated sound resources. When parsing a SCI1
sound header, we detect this and ignore channels with invalid offsets.
This can leave a track with no channels, but our channel mixing code
did not expect this and crashed when writing to an empty buffer.

Fixes Longbow Amiga crashing when entering room 440 outside the abbey.

Changed paths:
    engines/sci/sound/midiparser_sci.cpp


diff --git a/engines/sci/sound/midiparser_sci.cpp b/engines/sci/sound/midiparser_sci.cpp
index 831dac5368c..abc308f1ae7 100644
--- a/engines/sci/sound/midiparser_sci.cpp
+++ b/engines/sci/sound/midiparser_sci.cpp
@@ -153,7 +153,7 @@ static inline bool validateNextRead(const SoundResource::Channel *channel) {
 }
 
 void MidiParser_SCI::midiMixChannels() {
-	int totalSize = 0;
+	size_t totalChannelDataSize = 0;
 
 	for (int i = 0; i < _track->channelCount; i++) {
 		_track->channels[i].time = 0;
@@ -162,27 +162,39 @@ void MidiParser_SCI::midiMixChannels() {
 		// Ignore the digital channel data, if it exists - it's not MIDI data
 		if (i == _track->digitalChannelNr)
 			continue;
-		totalSize += _track->channels[i].data.size();
+		totalChannelDataSize += _track->channels[i].data.size();
 	}
 
-	SciSpan<byte> outData = _mixedData->allocate(totalSize * 2, Common::String::format("mixed sound.%d", _pSnd ? _pSnd->resourceId : -1)); // FIXME: creates overhead and still may be not enough to hold all data
+	// Allocate twice the channel data to hold the mixed data. If there are
+	// no channels due to a truncated sound resource (Longbow Amiga sound 461)
+	// then allocate the five bytes for the stop event below.
+	int resourceId = _pSnd ? _pSnd->resourceId : -1;
+	size_t mixedDataSize;
+	if (totalChannelDataSize != 0) {
+		// FIXME: creates overhead and still may not be enough to hold all data.
+		mixedDataSize = totalChannelDataSize * 2;
+	} else {
+		warning("Sound %d has no channels to mix", resourceId);
+		mixedDataSize = 5;
+	}
+
+	Common::String mixedDataName = Common::String::format("mixed sound.%d", resourceId);
+	SciSpan<byte> outData = _mixedData->allocate(mixedDataSize, mixedDataName);
 
 	long ticker = 0;
 	byte channelNr;
 	byte midiCommand = 0, midiParam, globalPrev = 0;
-	long newDelta;
-	SoundResource::Channel *channel;
 	bool breakOut = false;
 
 	while ((channelNr = midiGetNextChannel(ticker)) != 0xFF) { // there is still an active channel
-		channel = &_track->channels[channelNr];
+		SoundResource::Channel *channel = &_track->channels[channelNr];
 		if (!validateNextRead(channel))
 			break;
 		byte curDelta = channel->data[channel->curPos++];
 		channel->time += (curDelta == 0xF8 ? 240 : curDelta); // when the command is supposed to occur
 		if (curDelta == 0xF8)
 			continue;
-		newDelta = channel->time - ticker;
+		long newDelta = channel->time - ticker;
 		ticker += newDelta;
 
 		if (channelNr == _track->digitalChannelNr)


Commit: 231d6a51839f3295bc7dc289f3fd870c2e7c6848
    https://github.com/scummvm/scummvm/commit/231d6a51839f3295bc7dc289f3fd870c2e7c6848
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2025-10-03T10:15:06-07:00

Commit Message:
SCI: Validate SCI1 sound headers

When parsing a sound resource as SCI1, we now validate that there is
a header, and that it is complete, instead of reading out of bounds.

Fixes KQ1 Amiga crashing when incorrectly guessing the gnome's name.
The game uses SCI1 sounds, but sound 92 is still in the older SCI0
format from the DOS version, so there is no SCI1 header to parse.

Changed paths:
    engines/sci/resource/resource_audio.cpp


diff --git a/engines/sci/resource/resource_audio.cpp b/engines/sci/resource/resource_audio.cpp
index f53641cb3a2..69a31e05b4f 100644
--- a/engines/sci/resource/resource_audio.cpp
+++ b/engines/sci/resource/resource_audio.cpp
@@ -859,17 +859,36 @@ SoundResource::SoundResource(uint32 resourceNr, ResourceManager *resMan, SciVers
 			}
 		}
 	} else if (_soundVersion >= SCI_VERSION_1_EARLY && _soundVersion <= SCI_VERSION_2_1_MIDDLE) {
-		SciSpan<const byte> data = *_resource;
-		// Count # of tracks
-		_trackCount = 0;
-		while ((*data++) != 0xFF) {
+		// Count the tracks by parsing the SCI1 sound header.
+		// Must detect if the header is truncated or does not exist, because
+		// some games with SCI1 sounds accidentally include a few SCI0 sounds.
+		// Example: KQ1 Amiga sound 92 when guessing the gnome's name incorrectly.
+		size_t headerPos = 0;
+		bool isHeaderComplete = false;
+		while (headerPos < _resource->size()) {
+			if (_resource->getUint8At(headerPos++) == 0xff) {
+				isHeaderComplete = true;
+				break;
+			}
+			while (headerPos < _resource->size()) {
+				if (_resource->getUint8At(headerPos++) == 0xff) {
+					break;
+				}
+				headerPos += 5;
+			}
 			_trackCount++;
-			while (*data != 0xFF)
-				data += 6;
-			++data;
 		}
+		if (!isHeaderComplete || _trackCount == 0) {
+			warning("Invalid header in sound resource %d", resourceNr);
+			// unload resource so that exists() returns false and the sound is not used
+			_trackCount = 0;
+			resMan->unlockResource(_resource);
+			_resource = nullptr;
+			return;
+		}
+
 		_tracks = new Track[_trackCount];
-		data = *_resource;
+		SciSpan<const byte> data = *_resource;
 
 		for (int trackNr = 0; trackNr < _trackCount; trackNr++) {
 			// Track info starts with track type:BYTE