IT envelope bug.

[AliceLR]

Found in a bad module I made in 2007: if the volume envelope has two overlapping points at the end, the first of the two points will be treated as the end of the envelope, even if the first point is the sustain loop. This module plays correctly in Impulse Tracker. (Note: this envelope is from a module made with Modplug Tracker 1.16 that I copied into a new module. OpenMPT doesn't allow constructing envelopes like this anymore, but it appears to somehow be completely valid in IT regardless.)

dmad4_test.it.zip

edit: my description of the bug is slightly wrong. libxmp actually skips the sustain point entirely and ends up using the second point instead thanks to a loop in get_envelope.

[AliceLR]

Unfinished patch to work around this. It fails 11 tests and hasn't been verified at all against other formats with envelopes, so it needs further work.

diff --git a/src/player.c b/src/player.c
index 2b1bf6ea..efd3c0c1 100644
--- a/src/player.c
+++ b/src/player.c
@@ -89,22 +89,23 @@ static int get_envelope(struct xmp_envelope *env, int x, int def)
 {
 	int x1, x2, y1, y2;
 	int16 *data = env->data;
-	int idx;
+	int idx, stop;
 
 	if (x < 0 || ~env->flg & XMP_ENVELOPE_ON || env->npt <= 0)
 		return def;
 
-	idx = (env->npt - 1) * 2;
+	stop = (env->npt - 1) * 2;
+	idx = 0;
 
-	x1 = data[idx]; /* last node */
-	if (x >= x1 || idx == 0) {
+	x1 = data[idx];
+	if (x <= x1 || idx >= stop) {
 		return data[idx + 1];
 	}
 
-	do {
-		idx -= 2;
+	while (idx < stop && data[idx + 2] <= x) {
+		idx += 2;
 		x1 = data[idx];
-	} while (idx > 0 && x1 > x);
+	}
 
 	/* interpolate */
 	y1 = data[idx + 1];

[sagamusix]

Maybe worth mentioning here for documentation's sake, but should probably not influence the fix for this:

Every envelope segment is at least one tick long in IT, meaning that placing two points on the same x value causes the second point to be delayed by one tick in IT. This causes envelopes from older MPT/OpenMPT modules to end up desynchronizing with the rest of the track.