TheUnderscores / midi-beeper-orchestra

MIDI Beeper Orchestra
GNU Affero General Public License v3.0
1 stars 0 forks source link

Notes extracted from MIDI files not syncing properly #10

Open jcbrockschmidt opened 9 years ago

jcbrockschmidt commented 9 years ago

It is not yet known whether the source of this issue lies with the manager (src/server/manager.py), MIDI parser (src/server/midiparser.py), or possibly just because music does not sound nice when played through multiple computer buzzers. Songs reportedly work fine when played on a single computer. However, when played on multiple computers, music sounds disjointed and generally unmelodious.

jcbrockschmidt commented 9 years ago

I have run simple tests on the manager, and it seems to be working as intended. Again, these tests are simple, but I have no reason to believe that scaling them up would cause any issues to arise. My primary theory at the moment is that the cause of the problem is simple miscommunication between the developer of the manager (Joshua, me) and the developer of the MIDI parser (Shelvacu). It is possible that it was not thoroughly explained how layers should be structured for optimal performance with the manager. I have yet to assiduously examine the MIDI parser.

jcbrockschmidt commented 9 years ago

Based on a few tests I ran with custom single-layered MIDI files, it was observed that the server repeatedly failed to replicate the desired tune. Only half of the notes seem to have played. A sufficient number of error messages, "WARNING: wasn't able to allocate space for event", seems to have accounted for these missing notes. I suspect the error in this particular case lies with the MIDI parser, as all of these notes should have easily fit within a single layer.

It may be worth noting that some notes in this track began right after another finished. The MIDI parser may be detecting these notes as overlapping rather than touching.

shelvacu commented 9 years ago

Well I can save you a bit of time and attempt to explain it to you Midi is made up of tracks, each track has note-on and note-off events happening at times. My code First creates a layer for each client that is connected (it was like 4am when I wrote it!), as well as some information for each layer regarding whether the layer is 'currently' playing a note and what the layers timing is. More on both of those later. So I take all the midi tracks and baisicly merge them all into one big megatrack, which I'm calling it that because I have an excuse to use the word megatrack. It's a dictionary of points in time (measured in ticks) to arrays of note on/off events Then, I convert the dictionary to a 2d array so that I can sort it by time, and then I run through that in orderd of what time it takes place, first first, and then each note on I assign to a layer and mark that layer as 'playing x note', and unmark when I see a note off. If there are no layers left, a warning is displayed, and the event is discarded. Theres also a layers timing array, which is because the layers class takes the timing in an offset from The Beginning, whereas midi is stored as the time since the last event. So, layers_timing starts at 0 for each layer, and then as

OH MY RICHARD STALLMAN I JUST REALIZED THE BUG IN THE CODE

Edit: See later comment

layers_timing should be global, not per layer. Hence when theres only one client theres only one layer and it's as if it's global.

jcbrockschmidt commented 9 years ago

I might have to read that again later when it's not midnight and I'm a little more functional. But, I think I get the gist of it.

A thought comes to mind. Might it be possible that the note-off events are are being shoved into layers when they don't need to be, confusing layer composition and restricting note-on events that appear consecutively in a MIDI file from being placed in the same layer consecutively?

shelvacu commented 9 years ago

Wait, no, what? In the event class, the first argument, is an amount of time in microseconds, from the beginning, or the last event?

I assumed the latter, so let me ammend my previous statement: the layers_timing array stores how 'far in' we are, and since the time (the key of the dictionary) is a value measured from The Beginnning, it gets the difference, and that should be the delta between the last event and this one. So it should work. So just kidding, bug not found

jcbrockschmidt commented 9 years ago

You assumed correctly. Yeah, I understood that. My second paragraph there was not intended to relate to the topic that was being discussed when I posted it. I am going to look into the issue of why notes are being lost more extensively later. It feels like a problem that could be due to something as tiny as a < being used instead of a <=. I'm just extrapolating that assumption from previous time-related programmings issues I've encountered in my excursions with game-development.