AliceLR
commented
3 years ago As far as the removal of that division by 4, I've tested a few tracks and I'm fairly confident that 1) nothing relies on that division by 4, and pretty much every tempo mode track that uses 9xx relies on it NOT dividing by 4; 2) that change doesn't negatively affect tracks that didn't have a buggy 9xx before (either in BPM mode or by luck of the command translating to ProTracker F00 and being ignored).
Tested playback times vs. OctaMEDPlayer v0.903. OctaMEDPlayer times come from measuring a stereo mix recording in Audacity (~ means ambiguous loop so it's a weak estimate), xmp/MikMod times are as reported by their respective players. (!) means there was another bug for that particular file. All files ending in .mmd0 and .mmd1 are just files from the OctaMed MMD0/1 folders on modland.com since I didn't feel like linking them all directly ;-(
edit: also, xmp's accuracy for OctaMED files is impressive enough that I'm probably not going to bother with the OctaMEDPlayer timing step in future testing unless something seems suspicious.
| File | Type | OctaMEDPlayer | xmp | MikMod | MikMod (patch) | Notes |
|---|---|---|---|---|---|---|
| wizardry.med | MMD0/Tempo | 1:28'77 | 1:28 | 0:12 | 1:28 | |
| alain.med | MMD0/Tempo | 2:52'99 | 2:52'5 | 0:29 | 2:53 | [1] out of tune. |
| yellybeansong.med | MMD0/Tempo | ~4:04'27 | 4:03'4 | 0:41 | 4:04 | Portamento inaccuracy? |
| Dean Evans/terminator 2 - puzzle.mmd0 | MMD0/Tempo | ~1:19'33 | 1:19'9 | 1:20 | 1:20 | 902 -> PT F00 ignored prior to patch. |
| Faroul/sad song.mmd0 | MMD0/Tempo | ~2:11'55 | 2:10'5 | 0:24 | 2:10 | |
| HJE/high speed.mmd0 | MMD0/Tempo | ~0:43'5 | 0:42'0 | 0:08 | 0:43 | |
| James Pimentel-Pinto/amiganuts power.mmd0 | MMD0/Tempo | ~1:55 | 1:54'7 | 0:19 | 1:55 | |
| Kevin Stannard/lunar-c-title.mmd0 | MMD0/Tempo | ~1:08.5 | 1:07'9 | 1:08 | 1:08 | 903 -> PT F00 ignored prior to patch. [1] chords out of tune. |
| - unknown/red herring.mmd0 | MMD0/BPM | 3:54.22 | 3:54'1 | 3:54 | 3:54 | |
| Basehead/high velocity.mmd0 (sequence 0) | MMD0/BPM | ~1:39'53 | 1:39'3 | ????? (!) | 1:40+ (!) | [1] out of tune. Reaches the end of the first sequence at the correct time. |
| Dodge/agony title tune remixed.mmd0 | MMD0/BPM | 4:51'72 | 4:51'7 | 4:51 | 4:51 | |
| Dodge/rollin & bollin.mmd0 | MMD0/BPM | 5:27'70 | 5:27'6 | 5:27 | 5:27 | |
| Orpheus/electrosound 64.mmd0 | MMD0/BPM | >1:55 | 1:55'4 | 3:16 (!) | 3:16 (!) | [2] MikMod plays at half speed. |
| - unknown/5th beathoven.mmd1 | MMD1/Tempo | 1:01'67 | 1:01'3 | 0:10 | 1:01 | 906 -> PT F01 |
| - unknown/gotterfunken.mmd1 | MMD1/Tempo | 3:59'82 | 3:58'8 | 0:43 | 3:58 | 906/905 -> PT F01. [1] out of tune. |
| Andy Baxter/cheryl heaven.mmd1 | MMD1/Tempo | 2:13'61 | 2:12'9 | 0:33 | 2:13 | 908 -> PT F02 |
| Andy Soar/blood net - combat.mmd1 | MMD1/Tempo | 1:47'88 | 1:47'4 | 0:17 | 1:47 | 906 -> PT F01 |
| Blair Zuppicich/roadkill - end.mmd1 | MMD1/Tempo | ~1:00 | 1:00'6 | ? (!) | ? (!) | [3] MikMod gets stuck in a loop (1B02 -> B02). |
| Blockhead/spaceship.mmd1 | MMD1/Tempo | ~2:18'50 | 2:18'0 | 0:30 | 2:18 | 909 -> PT F02 |
Brooker/(brooker) #01.mmd1 |
MMD1/Tempo | ~3:05'59 | 3:05'3 | 1:01 | 2:22 (!) | [3] Patch is closer to correct; affected by 1Fxx -> Fxx. |
| Caramel/liver paste.mmd1 | MMD1/Tempo | ~2:41'54 | 2:40'9 | 1:02 | 2:41 | 90A -> PT F02 |
| Chris the Highlander/afraid.mmd1 | MMD1/Tempo | 1:50'47 | 1:48'6 | 0:15 | 1:27 (!) | [3] Patch correct aside from 16xx -> 6xx at the end. |
| Cobalt/drum sampler (sunsa del luya).mmd1 | MMD1/Tempo | ~2:26'05 | 2:25'8 | 0:24 | 2:25 | 906 -> PT F01 |
| Cosmiq/heavy stuff.mmd1 | MMD1/Tempo | 1:43'15 | 1:42'6 | 0:25 | 1:42 | 904 -> PT F01 |
| Dean Evans/jurassic park - music1.mmd1 | MMD1/Tempo | ~1:30 | 1:29'6 | 0:14 | 1:33 | 907 -> PT F01 (silent loop at end probably is the cause of the slightly different times) |
| Dimitrios Charalampidis/black waltz.mmd1 | MMD1/Tempo | ~1:32'15 | 1:32'0 | 0:20 | 1:32 | 906 -> PT F01 |
| Dr. Tomastik/charless.mmd1 | MMD1/Tempo | 1:32'55 | 1:32'0 | 0:16 | 1:32 | 906 -> PT F01 |
| - unknown/alien breed ii.mmd1 | MMD1/BPM | 3:53'3 | 3:53'1 | ? (!) | ? (!) | [3] MikMod gets stuck in a loop (1B01 -> B01). |
| - unknown/digi-t-ice.mmd1 | MMD1/BPM | 1:37'27 | 1:37'2 | 1:37 | 1:37 | [1] Out of tune. |
| - unknown/fair.mmd1 | MMD1/BPM | 0:44'81 | 0:44'6 | 0:44 | 0:44 | |
| Appi-Ukko/happovaiva.mmd1 | MMD1/BPM | 1:27'66 | 1:27'4 | 0:55 (!) | 0:55 (!) | [3] 1Bxx -> Bxx; 16xx -> 6xx; 1EEF -> EEF |
| C-Frog/soft vision.mmd1 | MMD1/BPM | ~2:24 | 2:21'7 | 1:48 (!) | 1:48 (!) | [3] 19xx -> 9xx and probably others. |
| Chroust/pentium.mmd1 | MMD1/BPM | 2:18'26 | 2:18'0 | 2:18 | 2:18 | |
| Cybarite/dark obsession title.mmd1 | MMD1/BPM | ~2:21 | 2:20'6 | 5:10 (!) | 5:10 (!) | [3] 1F30 -> F30 causes it to play slow starting from order 2. |
| Darkman/mindphaser.mmd1 | MMD1/BPM | 1:59'08 | 1:58'9 | 1:59 | 1:59 | |
| Debol/debol'97-msx3+.mmd1 | MMD1/BPM | 1:24'53 | 1:24'4 | 1:24 | 1:24 | |
| DJ Hotcakes/double dragon.mmd1 | MMD1/BPM | 2:26'64 | 2:28'4 | >3:00 (!) | ? (!) | [3] 19xx -> 9xx |
| Dreamland Fantasy/china girl.mmd1 | MMD1/BPM | 2:59'25 | 2:59'0 | 2:07 (!) | 2:07 | [3] 1F03 -> F03; other bugs?... |
| Dreamland Fantasy/discovery.mmd1 | MMD1/BPM | 1:42'41 | 1:42'2 | 1:42 | 1:42 | |
| Dreamland Fantasy/doctor who.mmd1 | MMD1/BPM | 0:30'72 | 0:30'6 | 0:46 | 0:46 | Difference is just due to MikMod playing past the loop (not a bug). |
| Flash (PL)/daj mi te noc.mmd1 | MMD1/BPM | 1:49'33 | 1:49'2 | >3:00 (!) | ? (!) | [3] 19xx -> 9xx; 1F40 -> F40 |
New bugs found during testing:
[1] Seems to be a tuning bug in both MMD0 and MMD1 .MEDs caused by the MED loader not handling either the song or instrument transpose values (this needs verification). I'm not familiar enough with MikMod to tell how hard this would be to implement.
[2] The speed of BPM mode modules sometimes starts as or changes to something very wrong. From examining electrosound 64.med (and currently no other modules) this appears to be because of the loader factoring the GCD of beat rows (10) and initial speed (4) out of both the initial BPM (60) and speed (resulting in 60*10/4/2=75 tempo and 4/2=2 speed). The 9xx command is then immediately used to set the original speed of 4 (for an effective halving of overall speed). Haven't checked other modules for this yet. Considering high speed.med reached 909 tempo with no issue I'm skeptical that this GCD algorithm does anything useful (speed can never be set >255; the UWORD range should be adequate for sane BPM/beat row combos, but maybe tempo was a UBYTE when the algorithm was added). Another bug that could be caused by this factoring is, if the divisor were consistently applied to speeds provided by a 9xx command and those speeds were not a multiple of the GCD, the resulting speed would be truncated. Tentatively suggesting that the entire GCD thing be removed, but it needs more analysis and testing. (Also, the beat rows value may need to be provided to EffectCvt() and Fxx might need to switch to using UNI_MEDSPEED for BPM mode.)
[3] OctaMED 4.00 commands unimplemented:
- 0x11 Portamento up once (same as PT E1x -- could use UNI_XMEFFECTE1) (should this ignore 0 like PT?)
- 0x12 Portamento down once (same as PT E2x -- could use UNI_XMEFFECTE2) (should this ignore 0 like PT?)
- 0x14 Vibrato (same as PT vibrato; depth is halved relative to MED 4xx)
- 0x15 Set current instrument finetune (same as PT E5x -- parameter is signed, valid range is -8 through +7, and MED uses the same frequences as MOD, so this can trivially translate to the PT effect!)
- 0x16 Loop (same as PT E6x) (needs a new effect due to extended range)
- 0x18 Stop note (same as PT ECx) (needs a new effect due to extended range)
- 0x19 Sample start offset (same as PT 9xx)
- 0x1A Slide volume up once (same as PT EAx -- could use UNI_XMEFFECTEA) (should this ignore 0 like PT?)
- 0x1B Slide volume down once (same as PT EBx -- could use UNI_XMEFFECTEB) (should this ignore 0 like PT?)
- 0x1D Pattern break (same as PT Dxx)
- 0x1E Play line x times (same as PT pattern delay) (needs a new effect due to extended range)
- 0x1F Combined note-delay retrigger - 0xXY -> X=delay, Y=retrigger count) (needs a new effect + maybe the MED FF1/2/3 effect implementations coulds be merged into this.)
I'd like to investigate bug 2 further and include a patch for it with this patch if possible. I'll submit a separate patch for the unimplemented OctaMED 4 commands (are there any compatibility concerns when adding new UNIMOD commands I should be aware of before implementing those?).
Until then, the working patch for this issue is simply:
diff --git a/libmikmod/loaders/load_med.c b/libmikmod/loaders/load_med.c
index 84cfb1f..1caebaa 100644
--- a/libmikmod/loaders/load_med.c
+++ b/libmikmod/loaders/load_med.c
@@ -219,8 +219,6 @@ static void EffectCvt(UBYTE eff, UBYTE dat)
if (dat <= 0x20) {
if (!dat)
dat = of.initspeed;
- else
- dat /= 4;
UniPTEffect(0xf, dat);
} else
UniEffect(UNI_MEDSPEED, ((UWORD)dat * 125) / (33 * 4));
Creating an issue for this one so my research doesn't clutter the (eventual) PR.
This bug occurs with .MED files that use the .MOD secondary tempo command
9xxin tempo mode, and appears to be caused by this line of code: https://github.com/sezero/mikmod/blob/master/libmikmod/loaders/load_med.c#L223The OctaMED documentation suggests that this should convert directly to/from the .MOD speed command and, considering MikMod's tempo conversion, it seems that there is no reason to divide this value by 4. https://modland.com/pub/documents/format_documentation/Music%20Editor%20v3.2%20(.med).txt
This change dates back to MikMod 3.1, where it appeared in the form of a
speeddivisorvariable that was always equal to 4 during tempo mode. This variable is applied to the9xxcommand and the speed case of theFxxcommand in different ways!By 3.1.5 this variable was completely removed, just leaving a fixed division by 4 in the
9xxcommand and not in theFxxcommand. This looks like an oversight that never got caught to me, but there might some reason I'm missing for this being there.Removing that line makes these example .MEDs play at the same speeds that xmp plays them at. I don't have OctaMED set up to test with right now.
alain.med.zip wizardry.med.zip yellybeansong.med.zip
Since this division by 4 was presumably added for a reason (even if it might have been a mistake), I'm going to exhaustively test (with OctaMED, xmp, and MikMod) every .MED MMD0 and MMD1 file I can find that uses this to make sure removing the bugged line doesn't cause any breakage.
edit: maybe that was too optimistic, there are 379 MMD0 files and 816 MMD1 files that use
9xxwith values <=0x20...