SCI: Add some comments a possible TODO regarding song looping

1 files changed, 16 insertions, 1 deletions

diff --git a/engines/sci/sound/midiparser_sci.cpp b/engines/sci/sound/midiparser_sci.cpp
index 00089cd4c3..938455c24a 100644
--- a/engines/sci/sound/midiparser_sci.cpp
+++ b/engines/sci/sound/midiparser_sci.cpp
@@ -536,6 +536,11 @@ void MidiParser_SCI::parseNextEvent(EventInfo &info) {
 				// Check if the hold ID marker is the same as the hold ID
 				// marker set for that song by cmdSetSoundHold.
 				// If it is, loop back, but don't stop notes when jumping.
+				// We need to wait for the delta of the current event before
+				// jumping, as in LSL6, this ends up jumping forward one tick
+				// (the hold marker occurs at playtick 27, with loopTick
+				// being 15 and the event itself having a tick of 13,
+				// total = 28) - bug #3614566.
 				if (info.basic.param2 == _pSnd->hold) {
 					_jumpToHoldTick = true;
 				}
@@ -640,7 +645,17 @@ void MidiParser_SCI::parseNextEvent(EventInfo &info) {
 				// (e.g. song 110, during the intro). The original interpreter
 				// treats this case as an infinite loop (bug #3311911).
 				if (_pSnd->loop || _pSnd->hold > 0) {
-					// We need to play it again...
+					// TODO: this jump is also vulnerable to the same lockup as
+					// the MIDI hold one above. However, we can't perform the
+					// jump on the next tick like with the MIDI hold jump above,
+					// as there aren't any subsequent MIDI events after this one.
+					// This assert is here to detect cases where the song jumps
+					// forward, like with the hold jump above (however, this
+					// scenario sounds less likely to occur with this jump). Note
+					// that we haven't yet found a scene where this jump might
+					// lock up,
+					assert(_loopTick + info.delta < _position._playTick);
+
 					uint32 extraDelta = info.delta;
 					jumpToTick(_loopTick);
 					_nextEvent.delta += extraDelta;