issue with Xm effect L

[sezero]

As reported by Thomas Neumann (@neumatho):

While listening to my FastTracker 2 modules, to test the ported Xm
loader and player, I found a module which uses the XM Effect L (Set
envelope position) and it crashed with an index out of range. It
crashed on this line:
https://github.com/sezero/mikmod/blob/master/libmikmod/playercode/mplayer.c#L1584

                aout->venv.p=aout->venv.env[(dat>points)?points:dat].pos;

dat is 80 and points is 253. I tried with MikMod itself (the C code),
but there it doesn't crash, but it have the same values as I got in dat
and points. Maybe its because of pointer usage it does not always crash?

Well, I do not know much about exactly how envelopes works, but the array
venv.env, is always fixed to 32 elements. As I can see, each element have
a position and a value and what I can see for the ProcessEnvelope(), is
that P is the position and A and B is the indexes to the elements in the
array in use.

So I guess, that this function does not find the right position the way
its implemented, but it should just set P to either dat or points and update
A and B with the right indexes by looking in the array to find where P is
between two elements.

Also OpenMPT says that the panning envelope should only be updated, if the
volume envelope sustain is on:

https://wiki.openmpt.org/Manual:_Effect_Reference#Effect_Column_2

It seems like MikMod always update the panning envelope as well. I do not know, which is the correct way.

Oh, here is a link to the module I have tested with. The effect is used a couple of rows down at position 4.

https://modarchive.org/index.php?request=view_by_moduleid&query=135290

[sezero]

This is the solution suggested by Thomas Neumann (in C#):

/********************************************************************/
        /// <summary>
        /// Effect L: Set envelope position
        /// </summary>
/********************************************************************/
        private int DoXmEffectL(ushort tick, ModuleFlag flags, Mp_Control a, Module mod, short channel)
        {
            byte dat = uniTrk.UniGetByte();

            if ((tick == 0) && (a.Main.I != null))
            {
                Instrument i = a.Main.I;

                Mp_Voice aOut;
                if ((aOut = a.Slave) != null)
                {
                    if (aOut.VEnv.Env != null)
                        SetEnvelopePosition(ref aOut.VEnv, i.VolEnv, dat);

                    if (aOut.PEnv.Env != null)
                    {
                        // Because of a bug in FastTracker II, only the panning envelope
                        // position is set if the volume sustain flag is set. Other players
                        // may set the panning all the time
                        if (((mod.Flags & ModuleFlag.Ft2Quirks) == 0) || ((i.VolFlg & EnvelopeFlag.Sustain) != 0))
                            SetEnvelopePosition(ref aOut.PEnv, i.PanEnv, dat);
                    }
                }
            }

            return 0;
        }

And you need this helper method:

/********************************************************************/
        /// <summary>
        /// Set the envelope tick to the position given
        /// </summary>
/********************************************************************/
        private void SetEnvelopePosition(ref EnvPr t, EnvPt[] p, short pos)
        {
            if (t.Pts > 0)
            {
                bool found = false;

                for (ushort i = 0; i < t.Pts - 1; i++)
                {
                    if ((pos >= p[i].Pos) && (pos < p[i + 1].Pos))
                    {
                        t.A = i;
                        t.B = (ushort)(i + 1);
                        t.P = pos;
                        found = true;
                        break;
                    }
                }

                // If position is after the last envelope point, just set
                // it to the last one
                if (!found)
                {
                    t.A = (ushort)(t.Pts - 1);
                    t.B = t.Pts;
                    t.P = p[t.A].Pos;
                }
            }
        }

I found the attached module here: https://wiki.openmpt.org/Development:_Test_Cases/XM

I do not know if you know anybody who can test it in the real FastTracker II. It cannot be run on Windows 10, but I found a clone, where the attached module sounds the same as with my patch. But it is a clone, so I don't know if it can be used as a real test. I found the clone here:

https://16-bits.org/ft2.php

SetEnvPos.xm.zip

Comments? @AliceLR?

[AliceLR]

Fixed in #56 (https://github.com/sezero/mikmod/commit/f1562b18f51e4c95ccf47c67e954cb21f9c74991).