Bug in f_mus.c when playing same tune twice without reloading from disk

[Bill-Paul]

Hi:

I'm working on a personal project where I have Doom running on an ST Micro STM32F746 Discovery board, and I wanted to implement Doom music support. As with most recent hardware, the STM32F746 has only digital audio output (e.g. I2S), so I needed a way to to synthesize the music in software. Since WIldMidi supports the Doom MUS format, is written in C and is fairly small, it seemed like the best fit. Turns out it worked great, except for one problem:

MUS tracks are a bit like MIDI, except that percussion is always on track 15 whereas I think for regular MIDI percussion is always on track 9, so the _WM_ParseNewMus() function has this fix-up code:

#if 1
        // Mus drums happen on channel 15, swap channel 9 & 15
        // DEBUG
        MUS_EVENT_DEBUG("Before", mus_data[mus_data_ofs], 0);
        
        if ((mus_data[mus_data_ofs] & 0x0f) == 0x0f) {
            mus_data[mus_data_ofs] = (mus_data[mus_data_ofs] & 0xf0) | 0x09;
        } else if ((mus_data[mus_data_ofs] & 0x0f) == 0x09) {
            mus_data[mus_data_ofs] = (mus_data[mus_data_ofs] & 0xf0) | 0x0f;
        }
        // DEBUG
        MUS_EVENT_DEBUG("After", mus_data[mus_data_ofs], 0);
#endif

This is fine, but the change is done by directly modifying the note data that's been read into memory from disk. This works ok if you use the sample player program, because then the MUS data is reloaded from disk each time you play it, so you always start off with a fresh copy.

But with Doom, the MUS data is in the WAD file, which Doom reads into memory only once for each play session. To render a tune, I use WildMidi_OpenBuffer() and give it a pointer to the MUS data that's already loaded in RAM, and the same buffer is always used each time the same tune is played. The only time you re-load the data is if you quit Doom and then re-launch it.

The problem is that the code above modifies the contents of that original buffer in order to fix up the track info for the percussion notes, but then afterwards it doesn't back the fix-up out again. The result is that if you play the same tune from the same RAM buffer twice, on the first playback the code above does the fix-up and everything sounds right, but on the second playback the same code does the fix-up again (effectively reversing it) and then everything sounds wrong.

I failed to notice this for a while because Doom doesn't often play the same music twice while you're playing, since each map has its own separate tune. However the intermission screen between maps always uses the sane tune, and I realized that every second time I completed a map, the intermission music would sound messed up. :)

The fix is fairly simple: later in the _WM_ParseNewMus() function, after the note info is parsed, you just need to un-do the fix-up. You can see my fix I used in my local repo here:

netik/dc28_badge@9a6f58d

In case the link breaks, here's the diff:

diff --git a/software/firmware/wildmidi-wildmidi-0.4.4/src/f_mus.c b/software/firmware/wildmidi-wildmidi-0.4.4/src/f_mus.c
index 6060d1a2..bed55322 100644
--- a/software/firmware/wildmidi-wildmidi-0.4.4/src/f_mus.c
+++ b/software/firmware/wildmidi-wildmidi-0.4.4/src/f_mus.c
@@ -327,6 +327,23 @@ _WM_ParseNewMus(uint8_t *mus_data, uint32_t mus_size) {
         }
 
     _mus_next_data:
+
+#if 1
+        // Un-do the channel 9/15 swap if we did it before.
+       // Otherwise, the second time we play the same tune,
+       // it'll sound wrong.
+        // DEBUG
+        MUS_EVENT_DEBUG("Before", mus_data[mus_data_ofs], 0);
+
+        if ((mus_data[mus_data_ofs] & 0x0f) == 0x0f) {
+            mus_data[mus_data_ofs] = (mus_data[mus_data_ofs] & 0xf0) | 0x09;
+        } else if ((mus_data[mus_data_ofs] & 0x0f) == 0x09) {
+            mus_data[mus_data_ofs] = (mus_data[mus_data_ofs] & 0xf0) | 0x0f;
+        }
+        // DEBUG
+        MUS_EVENT_DEBUG("After", mus_data[mus_data_ofs], 0);
+#endif
+
         if (!(mus_data[mus_data_ofs] & 0x80)) {
             mus_data_ofs += mus_event_size;
             goto _mus_build_event;

I know this project isn't very active so I don't know when/if this will ever get officially fixed, but I figured I should document it in case anyone is interested.

-Bill