.it max instument count
yousernaym opened this issue · 7 comments
The song hyo-cher.it from Modarchive fails to load because it uses instrument numbers greater than 99. I changed line 280 in load_it.c to allow it. Not sure about the implications but it seems to have solved the problem without breaking anything.
if((ins)&&(ins<100)) //<- Changed to 253
UniInstrument(ins-1);
else if(ins==253)
UniWriteByte(UNI_KEYOFF);
else if(ins!=255) { /* crap */
_mm_errno=MMERR_LOADING_PATTERN;
return NULL;
}It's your commit yousernaym@6611099 yes?
@AliceLR: Can you comment?
Yeah
Ok, something is still wrong though. First instrument stops playing after a few seconds.
Regarding the change to the instrument bound, I believe <253 is correct. I don't know why it's <100.
The first instrument breaks after a few seconds because this IT misuses the channel volume effect and MikMod stores the channel volume as a signed byte. MFF causes the volume to be -1. Values over M40 should probably be ignored but I'm not 100% sure (OpenMPT ignores them and this is a MPT 1.16 IT...).
I'll check the behavior for both of these issues in Impulse Tracker.
Impulse Tracker only allows up to 99 instruments and crashes when it encounters instruments >=100, hence that original bound. That said, the file format supports them just fine and I don't see the harm in allowing them (especially considering MPT and OpenMPT are both capable of creating ITs with that many instruments).
Impulse Tracker, OpenMPT, Modplug Tracker 1.16, and libxmp all ignore invalid Mxx parameters. The fix for that bug is to just change playercode/mlutil.c line 219 to this:
case 0xd: /* Mxx Set Channel Volume */
/* Ignore invalid values >64. */
if (inf <= 0x40)
UniEffect(UNI_ITEFFECTM,inf);
break;OK then, I will apply the following patch soon:
diff --git a/libmikmod/loaders/load_it.c b/libmikmod/loaders/load_it.c
index 5b6f5bc..f19bada 100644
--- a/libmikmod/loaders/load_it.c
+++ b/libmikmod/loaders/load_it.c
@@ -277,7 +277,10 @@ static UBYTE* IT_ConvertTrack(ITNOTE* tr,UWORD numrows)
UniNote(note);
}
+ /* Impulse Tracker only allows up to 99 instruments and crashes when it
+ encounters instruments >=100. But the file format supports them just
+ fine and there are many MPT-created ITs with that many instruments. */
- if((ins)&&(ins<100))
+ if((ins)&&(ins<253))
UniInstrument(ins-1);
else if(ins==253)
UniWriteByte(UNI_KEYOFF);
diff --git a/libmikmod/playercode/mlutil.c b/libmikmod/playercode/mlutil.c
index 1c8005b..dacc6fb 100644
--- a/libmikmod/playercode/mlutil.c
+++ b/libmikmod/playercode/mlutil.c
@@ -217,7 +217,9 @@ void S3MIT_ProcessCmd(UBYTE cmd, UBYTE inf, unsigned int flags)
UniEffect(UNI_S3MEFFECTD,inf);
break;
case 0xd: /* Mxx Set Channel Volume */
- UniEffect(UNI_ITEFFECTM,inf);
+ /* Ignore invalid values > 64. */
+ if (inf <= 0x40)
+ UniEffect(UNI_ITEFFECTM,inf);
break;
case 0xe: /* Nxy Slide Channel Volume */
UniEffect(UNI_ITEFFECTN,inf);