[Scummvm-git-logs] scummvm master -> cf842571c2e150cafe051f1a5378465a4f3f39fe

[sluicebox]

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

Summary:
cf842571c2 SCI: Fix SCI0 MIDI filtering when multiple stop events


Commit: cf842571c2e150cafe051f1a5378465a4f3f39fe
    https://github.com/scummvm/scummvm/commit/cf842571c2e150cafe051f1a5378465a4f3f39fe
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-06-30T00:49:20-07:00

Commit Message:
SCI: Fix SCI0 MIDI filtering when multiple stop events

SCI0 MIDI filtering creates a new event stream, but it was generating
malformed events when there were multiple stop events (status byte FC).
This caused MidiParser_SCI::parseEvent to miss the stop event written by
the filter code. The parser would continue reading and parsing out of
bounds heap memory until crashing seemingly randomly.

Now the filter always stops on the first stop event and always writes
out its own stop event correctly.

Fixes PQ2 airport music, bug #15233

Thanks to @antoniou79 for triaging the bug report

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 329d930bb1b..eeffe27383d 100644
--- a/engines/sci/sound/midiparser_sci.cpp
+++ b/engines/sci/sound/midiparser_sci.cpp
@@ -280,7 +280,10 @@ void MidiParser_SCI::midiFilterChannels(int channelMask) {
 		if (!validateNextRead(channelData))
 			goto end;
 		curDelta = *channelData++;
-		if (curDelta == 0xF8) {
+		if (curDelta == kEndOfTrack) {
+			// kEndOfTrack status byte can potentially appear without delta.
+			goto end;
+		} else if (curDelta == 0xF8) {
 			delta += 240;
 			continue;
 		}
@@ -306,6 +309,13 @@ void MidiParser_SCI::midiFilterChannels(int channelMask) {
 			if (curChannel != 0xF)
 				containsMidiData = true;
 
+			// Stop at first kEndOfTrack.
+			// There can be duplicate end of track events afterwards,
+			// or junk bytes, or other leftover events.
+			if (command == kEndOfTrack) {
+				goto end;
+			}
+
 			// Write delta
 			while (delta > 240) {
 				*outData++ = 0xF8;
@@ -327,13 +337,6 @@ void MidiParser_SCI::midiFilterChannels(int channelMask) {
 				lastCommand = command;
 				break;
 
-			case kEndOfTrack: // end of channel
-				// At least KQ4 sound 104 has a doubled kEndOfTrack marker at
-				// the end of the file, which breaks filtering
-				if (channelData.size() < 2)
-					goto end;
-				break;
-
 			default: // MIDI command
 				// remember which channel got used for channel remapping
 				byte midiChannel = command & 0xF;
@@ -369,7 +372,14 @@ void MidiParser_SCI::midiFilterChannels(int channelMask) {
 
 end:
 	// Insert stop event
-	// (Delta is already output above)
+
+	// Write final delta
+	while (delta > 240) {
+		*outData++ = 0xF8;
+		delta -= 240;
+	}
+	*outData++ = (byte)delta;
+
 	*outData++ = 0xFF; // Meta event
 	*outData++ = 0x2F; // End of track (EOT)
 	*outData++ = 0x00;