This is the bug/patch tracker for Rockbox. Click here for more information.
Quick links: Bugs · Patches · Rockbox frontpage
FS#8806 - MikMod MOD, S3M, IT, XM player
Attached to Project:
Rockbox
Opened by Jason Yu (captainkewl) - Wednesday, 26 March 2008, 07:22 GMT+2
Opened by Jason Yu (captainkewl) - Wednesday, 26 March 2008, 07:22 GMT+2
|
DetailsPlayer for various tracker module formats supported by MikMod 3.1.12 -- http://sourceforge.net/projects/mikmod/
* All MikMod-supported formats should be playable in theory, though viewers.config is set to recognize only MOD, S3M, IT, and XM. Those four formats are the only ones tested. * Only tested on iPod 5G. Realtime playback on most mods; some stuttering on more complex IT files like snowf.it * Had to butcher libmikmod a good bit to get it to compile. As has been noted elsewhere (http://forums.rockbox.org/index.php?topic=4716.msg38535#msg38535), there are colliding namespaces (BOOL, nop) and undefined types (FILE). * Based on midiplay by Karl Kurbjun * dbestfit malloc() replacement by Daniel Stenberg 
mikmod-plugin.zip
(386.7 KiB)
|
This task depends upon
What's the word on building things around existing libraries of code? Problems like what we have here could be dealt with definitively by just cutting out the offending blocks, and we could probably do away with a good number of source files altogether. But I'm hesitant to make any substantial changes to anything as it's not really my code and merging any future updates to the original sources might become difficult.
* Tweaked the Makefile for Coldfire building
* Full (Mikmod internal) volume. Should fix some problems with fading.
* Increased get_more call to render 32768, which fixes stuttering in more complex files. I'm not sure if this will cause any adverse effects though -- I didn't see a value this high used in any other plugin.
* Pause uses internal Mikmod pause function. Pausing a song sounds a whole lot nicer now.
Plugging my iPod into my computer's USB port while playing a file causes the file to stop playing and then crashes the iPod when I try to exit. I'm not sure where to even begin to look.
only at the moment patch doesn't apply cleanly on svn, might need a resync?
Here's a patch that i used after i patched my source with mikmod-plugin-3.zip.
It seems mikmod is running atleast at 200% of it's normal speed. sounds funny tho.
Not sure about the Gigabeat problem as I don't know anything about it and don't have any way to test it, but first guess would be a bad value for SAMPLE_RATE in mikmod_supp.h.
Compiled under r18934 and works without problems on my Sansa c240.
The new 1MB codec buffer isn't intended to stay at that size forever - the intention is to reduce it as much as possible, which will be especially important as Rockbox starts to work on new targets, some of which only have 1MB or 2MB RAM in total.
I know almost nothing about the formats mikmod plays, but looking briefly at the source code, I see that it supports about 18 different file formats - are these 18 different codecs, or just different ways to store the same thing?
You mention in the first comment that you've only tested four formats - MOD, S3M, IT, and XM. How much of the code is shared between these formats? I'm wondering if (with some build-system magic) it would make sense to build separate codecs for each format? This would minimise the codec size, and also give more scope for optimisation (e.g. IRAM usage).
But even that won't solve the main issue of memory usage. Do you know how much memory each of the four main formats requires? Ideally, codecs like this should use the main audio buffer as working memory, which means the buffering API would need changing to meet the needs of these kinds of codecs. The first step towards this would be to try and define this API - i.e. what memory is needed?
It may be easier to discuss this on IRC.
On occasion, when refilling the buffer, the music playback will briefly pause for about a second before resuming.
Also i have some tiny issues when refilling the buffer.
* now uses WPS keymappings
* select previous/next track in current directory
* menu to toggle CPU boosting and a few interesting if not really useful playback settings
Regardless of how the codec goes, I think a plugin is still worthwhile, as it provides features that don't really work in a WPS, like comments and instrument/sample listings.
Definitely agreed here. Dare I say the WPS feels cumbersome for chiptunes?
Builds under r19793.
With that said, I am able to apply the patch without any problems, but of course your mileage varies. :)
mikmod.c: In function 'codec_main':
mikmod.c:119: warning: comparison between pointer and integer
Line 119 is "quit = samples < BUF_SIZE" which certainly seems wrong. Samples is a pointer to the buffer and BUF_SIZE is its size. As a result, quit is always false. This doesn't seem to lead to problems because Player_Active() causes that loop to end.
Now I'm listening to some IT, S3M, MOD and XM files on my 5G 30GB iPod. Some files just result in a codec failure, some files result in what seem like underruns, the interface is sometimes unresponsive, and the progress bar never behaves correctly. However, the majority of files play and sound very good.
The codec patch is working fine on my Gigabeast, also applied to r20681. I'm not getting the unresponsive interface issue, but the progress bar does act very strange sometimes.
The MOD player in SVN has a serious problem which this codec does not: I've never seen it move on to the next track. It just keeps repeating the same track, and "Repeat One" is not set. The only way out of the while (1) loop in apps/codecs/mod.c in SVN is (ci->stop_codec || ci->new_track). So, this patch is already better than what's currently in SVN.
Does anyone know of any MOD metadata code which is licensed under the GPL? I was looking at a few different MikMod plugins for other software, and I didn't see proper metadata reading anywhere. It seems MOD doesn't have a convenient length field; one has to go through the whole file and calculate length based on the number of patterns.
The performance issues are due to buffer underruns and the general performance hit of mixing samples on the fly... could possibly use separate threads to keep the buffer full.
The codec failures are due to a lack of memory. If you're serious about committing this, cutting out support for everything except MOD support might be the way to go, as anything more complex than that stands a good chance of not loading (unless someone can figure out how to exploit the audio buffer). I think we can add this to the list of reasons why Mikmod as a separate plugin would be worthwhile.
And please don't kill support for the other formats, if at all possible. May I suggest at least keeping the plugin for those?
I really like the patch. Just sounds great.
I attached a small fix for the "/apps/codecs/SOURCES". Just runs this after the "mikmod_codec.patch" was applies.
I haven't time to test it again, but it should work fine.
On my gigabeat S it works as expected.
CC apps/codecs/mikmod.c
LD mikmod.codec
/home/michael/rockbox/build/apps/codecs/mikmod.o: In function `codec_main':
mikmod.c:(.text+0x3bc): undefined reference to `add_pool'
mikmod.c:(.text+0x3c0): undefined reference to `drv_nos'
mikmod.c:(.text+0x3c4): undefined reference to `MikMod_RegisterDriver'
mikmod.c:(.text+0x3c8): undefined reference to `load_mod'
mikmod.c:(.text+0x3cc): undefined reference to `MikMod_RegisterLoader'
mikmod.c:(.text+0x3d0): undefined reference to `load_s3m'
mikmod.c:(.text+0x3d4): undefined reference to `load_xm'
mikmod.c:(.text+0x3d8): undefined reference to `load_it'
mikmod.c:(.text+0x3dc): undefined reference to `md_mixfreq'
mikmod.c:(.text+0x3e4): undefined reference to `MikMod_Init'
mikmod.c:(.text+0x3f8): undefined reference to `_mm_new_mem_reader'
mikmod.c:(.text+0x400): undefined reference to `Player_LoadGeneric'
mikmod.c:(.text+0x408): undefined reference to `dfree'
mikmod.c:(.text+0x40c): undefined reference to `Player_Start'
mikmod.c:(.text+0x418): undefined reference to `Player_SetPosition'
mikmod.c:(.text+0x41c): undefined reference to `VC_WriteBytes'
mikmod.c:(.text+0x428): undefined reference to `Player_Active'
mikmod.c:(.text+0x42c): undefined reference to `Player_Stop'
mikmod.c:(.text+0x430): undefined reference to `Player_Free'
mikmod.c:(.text+0x434): undefined reference to `MikMod_Exit'
collect2: ld returned 1 exit status
make: *** [/home/michael/rockbox/build/apps/codecs/mikmod.codec] Error 1
I'll check on this tonight
Attached is a sample IT. It worked when I was using the plugin patch (I don't anymore, having checked out a clean copy of the SVN).
MikMod copies and converts all samples during the loading process potentially converting 8bit samples to 16bit, because that is the type the mixing routines can handle. Due to that its not easy to apply the trick the current modplayer in the SVN uses, abusing the filebuffer as read-only storage for the samples(and patterns? Didnt look too close but that could be done i guess), circumventing the memory limitation that way. The pattern data takes up quite a lot of space as well, compared to the samples its neglectable though.
This need to copy/convert samples on loading is what is breaking the neck of this codec right now in my opinion. At least the IT format supports compressed samples. There will be no way around unpacking these into a usable form and copying them into the codec space.
Modifying the mixing routines to be able to handle all common types of samples natively, only copying/converting compressed samples until we run out of memory and then using the filebuffer as read only storage for the sample data as much as we can like the SVN modplayer does sounds like the way to go. For low memory targets dumbing down mikmod to support individual formats to lower the memory consumption further might still be necessary. Each dumbed down version would be a codec on its own then. I wonder what the best way to go here is? Input on that will be welcome.
I will start working on the mixing changes, trying to get mikmod to use the filebuffer.
Updated metatag parser to properly trim null characters from filenames. Misc small fixes. Also made a first real attempt to extend the codec api to provide scratch buffer allocation. Modify line 52 in mikmod.c to change the scratch buffer size. Right now it only works if you're listening to mod files exclusively -- switching codecs and playing other formats renders the audio buffer inaccessible and the bufalloc() fails.
Builds under 24213.