View Single Post
Old 1st December 2012, 14:15   #1
jschultz
Junior Member
 
Join Date: Dec 2012
Posts: 13
in_mod: XM files made with OpenMPT won't load correctly

It has come to my attention that Winamp still won't load some XM files, even though they are perfectly valid. First off, let me say that this is not a problem with OpenMPT but with mikmod, because XM files made with OpenMPT load just fine with about any other player I've tried - including Fasttracker 2. The problem is that mikmod seems to ignore the XM header size when loading XM files, which breaks stuff when header sizes are not the same as the defaults.
OpenMPT has adapted a trick from BoobieSqueezer, a utility to create tiny XM files by removing unnecessary information from such files. In this particular case, OpenMPT does not write the full 256-item order list in the file header, but only writes as many orders as necessary, and thus reduces the "header size" field as much as required. By default, the header size is 276 (20 bytes + 256 order items), but in reality it just needs to be 20 + x, where x is the number of orders. Since Renoise had the same problem with XM files, I have reported this issue to the Renoise development team here and it has since been fixed. I hope that the information posted in that thread will help you fixing this bug. If not, feel free to ask me about more details here. These test cases (plus the ones from the issue tracker link) should help you with verifying the bug.

Now, I still think that Winamp's age-old mikmod is rather abysmal and I know that you don't really fix anything but security-related bugs in it, but this is such a basic thing that it should really be fixed, since OpenMPT is actually a quite popular tracker and many people use it to create XM files. I don't want to be blamed every now and then that my application writes out "broken" XM files that Winamp can't load, since it's really Winamp's fault.

I had a look at the libmikmod-3.1.12 code, and I think you should replace
code:
_mm_read_UBYTES(mh->orders,256,modreader);

with
code:
_mm_read_UBYTES(mh->orders,min(mh->songlength,256),modreader);
_mm_fseek(modreader,mh->headersize+60,SEEK_SET);


in load_xm.c.

By the way, this bug is fixed in the latest libmikmod (libmikmod-3.2.0) already. Their fix was to change the line to
code:
_mm_read_UBYTES(mh->orders,mh->headersize-20,modreader);

which should work just as well, but I'm not sure if it is as secure.
jschultz is offline   Reply With Quote