XMI: bad reading of high delta values

[jpcima]

Hello. I believe to have discovered a parsing problem of Warcraft II's XMI. (eg. 419.xmi)
The problem about VLQ2's reading is the forced stop at 4th byte, for a 508 maximum value, whereas my XMI file can admit delta which are longer. This will likely produce a misreading of a series junk events.
In case of Warcraft II, I have end-of-track at delta value 166003.

This is a report based on software which uses WildMIDI's XMI parser for preprocessing sequencer inputs.
Here was the proposed fix and the problem description: Wohlstand/libADLMIDI#158

[sezero]

Where can we see the files that are problematic with the current code?

[jpcima]

They are encoded in the game as binary resources which take a special tool to extract.
For greater simplicity I have just made an upload here.

[sezero]

This seems to affect not only WC2, but others as well, such as
System Shock 2, Legend of Kyrandia, etc.: I several of them with
the following patch applied and some of them report at least one
case with i > 4

diff --git a/src/xmi2mid.c b/src/xmi2mid.c
index 2037e74..183feeb 100644
--- a/src/xmi2mid.c
+++ b/src/xmi2mid.c
@@ -600,16 +600,19 @@ static int GetVLQ2(struct xmi_ctx *ctx, uint32_t *quant) {
     int i;
     int32_t data;
 
     *quant = 0;
-    for (i = 0; i < 4; i++) {
+    for (i = 0; /*i < 4*/; i++) {
+        if (getsrcpos(ctx) == getsrcsize(ctx))
+            break; /*  */
         data = read1(ctx);
         if (data & 0x80) {
             skipsrc(ctx, -1);
             break;
         }
         *quant += data;
     }
+    if (i > 4) __builtin_printf("i=%d\n", i);
     return (i);
 }
 
 static int PutVLQ(struct xmi_ctx *ctx, uint32_t value) {

Legend of Kyrandia:

• (all:)
  i=6
  i=7
• intro.xmi:
  i=6
  i=5
  i=6
• kyra5a.xmi
  i=7
  i=7
  i=6
  i=8
  i=5
  i=6
  i=8
  i=6
  i=6
  i=7
• kyra5b.xmi
  i=5
  i=6
  i=36
  i=6
  i=7

SSHOCK (GENMIDI directory):

• THM10.XMI
  i=6
• THM5.XMI
  i=7

SSHOCK (SBLASTER directory):

• END.XMI
  i=8
• THM0.XMI
  i=5

The list can go on.

For the 419.xmi from this report, I got:
i=1308

@psi29a : What do you think of this?

[jpcima]

I've made on experiment myself on 46.xmi of Magic carpet 2 which is also affected.
46.xmi.gz

I've made recordings of synthesis of this music by ADLMIDI on 3 different sequencings.

• the first, I took it from the software XPLAY from AIL, by getting the MIDI In from the DOSBox emulated MPU401.
• the second and third are from xmi2mid, before and after patching

Having fixed the delta calculation, the timing seems now matching with the original playback. See:

[sezero]

PING: Anyone with any opinions on this?

[psi29a]

deal with IRL things at the moment.

[psi29a]

I'm OK with merging this in. I think the assumption, and otherwise 'magic' value of 4 was a bad assumption.

[psi29a]

So this was found in xmi2mid, do we have the same issue in f_xmidi (what the player uses)?

[sezero]

I don't know, let's ask: @jpcima if you play your xmi plainly, w/o converting like wildmidi my.xmi
does it exhibit the same issue?

[sezero]

Pushed fix as 3898cbd
Waiting for the @jpcima to answer question of @psi29a before closing.

[jpcima]

Thanks for merging.

So this was found in xmi2mid, do we have the same issue in f_xmidi (what the player uses)?

I took a recording on Wildmidi 3898cbd with the eawpats.

This does not seem affected by the issue.

[sezero]

OK then, closing.

[psi29a]

Thanks @jpcima and @sezero! :)