Glitchy MIDI import/export

[GoogleCodeExporter]

As reported here:
http://www.fretsonfire.net/forums/viewtopic.php?p=535442#p535442

The chart appears to be beat synced and notes are placed in grid snap 
positions, but the MIDI file that is created has the notes descynced starting 
at around 0:32.  Here are the chart files:
http://www.mediafire.com/?uzzmdjwywzg

I tried just removing the time signature and saving, but the problem was still 
reproduced.

Original issue reported on code.google.com by raynebc on 5 Jul 2010 at 6:33

[GoogleCodeExporter]

The beat markers do appear to be in the correct places.  I think the problem is 
due to the way EOF trims note lengths to deal with phrase markings like star 
power.

At the SP chord that extends past the 30 second marker, I noticed that the note 
that was written in the MIDI was shortened by about 91ms.  This appears to be 
the same amount of length that the following notes were skewed, as all notes 
that followed the phrase were made to be 91ms earlier.  This throws the rest of 
the chart off-sync.

If this trimming was a work-around for problems with how FoF deals with phrase 
markers, it may be best to end the marker 0 or 1 deltas after the last note in 
the phrase ends, and the FoFiX developers can fix their problem.  I couldn't 
find anywhere in the Harmonix documentation where it said there had to be 
padding before the end of the phrases.  Otherwise if the work-around is 
preferred to be kept, it will just need to be reworked to not affect notes 
outside the phrase.

Original comment by raynebc on 6 Jul 2010 at 6:42

[GoogleCodeExporter]

By now, many users have reported problems when importing a MIDI that is created 
with Fruity Loops Studio or Feedback, and that it also affects Feedback chart 
import.  This may be an issue beyond what may is introduced with any phrase 
marker problem I described earlier.

Original comment by raynebc on 9 Jul 2010 at 10:57

• Changed title: Notes desynced during import

[GoogleCodeExporter]

Original comment by raynebc on 9 Jul 2010 at 10:58

• Changed title: Notes desynced during import/export

[GoogleCodeExporter]

I'm examining a sample MIDI created in Fruity Loops Studio by Iroken:
http://www.mediafire.com/?qy30ygvxnzm

The note positions more or less agree with what I see in Reaper, but Anvil 
Studio seems to be completely wrong with the real timestamps it is displaying.  
I have emailed their support address asking them to explain.  I have found that 
AS does not display timings in minutes:seconds:millis format, it displays it in 
minutes:seconds:frames.  Theoretically, I can't find that the number of frames 
per second is defined unless the time division in the MIDI header is >= 0x8000. 
 In the case of the MIDIs being reported, they use ticks per frame timing 
instead, so it's not known why AS uses SMPTE format.

Unfortunately, the note timestamps logged by my lyric converter don't agree 
with the timestamps presented by EOF.  So I may have broken my logic somehow, 
or the implementation is different from EOF's..

Original comment by raynebc on 10 Jul 2010 at 4:24

• Added labels: Priority-High
• Removed labels: Priority-Medium

[GoogleCodeExporter]

I fixed my program's logging, it was using the last defined tempo instead of 
the tempo currently active for the current track.  My analysis so far seems to 
indicate two separate bugs that cause sync issues during import:

Regarding SUSAltd's chart (Joe Satriani - Day at the Beach):
The beat markers look right, but the desync happens around the overdrive phrase 
at 0:29.  I still believe this is a bug in the padding logic.

Regarding Iroken's sample MIDI:
My program's logging agrees with the note timings reported by EOF and Reaper, 
but not the beat markers.  So far, it seems like the notes themselves are 
imported in sync, but the beat markers are not.  According to my program's 
logging, the anchors in the chart should be at:  0.000000ms, 3428.568000ms, 
6628.568000ms and 9628.568000ms.  When I manually re-anchored the chart based 
on those timings, the beats lined up with the notes.

Original comment by raynebc on 10 Jul 2010 at 5:59

[GoogleCodeExporter]

Regarding padding, I am certain that is not the real problem. The MIDI export 
code doesn't distinguish between the various types of MIDI events it exports so 
regardless of padding the Note On and Note Off events will be sorted and 
exported correctly.

The real issue probably has to do with the sudden BPM change that occurs just 
before the desync (in the chart I saw on FoF-FF). It's possible the MIDI deltas 
are still being calculated incorrectly somehow. Either the way I calculate the 
deltas is not good enough or the beat markers floating point positions haven't 
been updated after some operation.

We should try and figure this out but this whole issue with MIDI delta time 
conversion will be bypassed when we get the new MIDI timing system in place. At 
that point we will only be using millisecond timing to represent what the user 
sees on screen. MIDI timing will be used to create the MIDI file which means no 
more loss of precision. This is high on my to-do list and will probably be the 
first thing I work on as soon as I get back to this project.

Original comment by xander4j...@yahoo.com on 11 Jul 2010 at 5:55

[GoogleCodeExporter]

It's just odd, because one bug causes only the beat markers to desync, and the 
other causes only the notes to desync.

Original comment by raynebc on 11 Jul 2010 at 6:19

[GoogleCodeExporter]

I found the cause of the issue with Iroken's chart.  The MIDI editor he used 
placed multiple Tempo events at the same delta time.  EOF ended up treating 
each of those events as a separate beat.  So the 2 extra tempo events at delta 
time 0 caused all other anchors to be applied two beats late.  This behavior is 
corrected in r262.

Original comment by raynebc on 31 Jul 2010 at 5:57

[GoogleCodeExporter]

The claim that the Feedback import causes desync has been retracted.

Original comment by raynebc on 31 Jul 2010 at 7:41

[GoogleCodeExporter]

I can't figure out the issue with the other chart.  SUSAltd posted some 
instructions for removing the desync glitchiness from the chart:
http://www.fretsonfire.net/forums/viewtopic.php?f=11&t=32725&view=unread#p541963

It seems like he's implying that the presence of the original EOF file affects 
MIDI import.

Original comment by raynebc on 31 Jul 2010 at 8:28

[GoogleCodeExporter]

I found an issue during MIDI export:  The math used to calculate determine the 
relative delta time for events being written is vulnerable to rounding errors, 
since the integer "delta" variable is not rounded up to the nearest whole 
number.  This causes problems where events can be one delta sooner or later 
than they should be.  For example, in SUSAltd's chart, many anchors are placed 
every four beats with a time division of 120, but EOF writes some of those 
anchors with delta times of 479 or 481 instead of the necessary 480 deltas.  
The use of the "accumulator" variable should likely be discontinued in favor of 
writing a delta time that was always rounded up.

Original comment by raynebc on 1 Aug 2010 at 6:37

[GoogleCodeExporter]

After studying the issue some more, it appears that the reason for the desync 
is because the chord note immediately before the glitch extends over a tempo 
change.  When I delete the anchor just before 0:30 and save the chart, the 
exported MIDI has no sync issues.  This seems to confirm my theory.  MIDI 
export will need to have special handling for determining the correct number of 
deltas for a note duration when tempo changes are traversed.

Original comment by raynebc on 1 Aug 2010 at 6:59

[GoogleCodeExporter]

I made a simplified version of the chart by deleting all notes that lead up to 
the glitch:
http://www.sendspace.com/file/3cuked

I'm finding that MIDI exported when this chart is saved doesn't have accurate 
timing for this first note, the delta time being incorrect.  Watching with the 
debugger during MIDI export, it seems that eof_add_midi_event() stores the 
correct timestamp for the glitched note, but I suspect it may be the subsequent 
conversion to deltas (eof_calculate_delta()) that ends up causing this issue.  
This problem doesn't have to do with a duration glitch, but it should be 
resolved before continued attempts to fix the original problem exhibited by 
SUSAltd's chart.

Original comment by raynebc on 2 Aug 2010 at 10:43

[GoogleCodeExporter]

r267 enables the first note to export to the correct MIDI timing, but the other 
notes drift out of sync.

Original comment by raynebc on 3 Aug 2010 at 8:41

[GoogleCodeExporter]

I've tried to fix one problem at a time, but at this point, it seems that MIDI 
import and export a sort of glitchy.  I've tried reverting back to r262, which 
was the first of my major changes to the MIDI import/export code.  I am a tad 
nervous to continue poking at it when I can't be sure what will change, so I am 
going to try to leave this alone until you have an opportunity to look at it.

Original comment by raynebc on 3 Aug 2010 at 11:23

• Changed title: Glitchy MIDI import/export

[GoogleCodeExporter]

When I get back to working on EOF I am going straight to implementing the 
MIDI-based timing method. Regarding the accumulator, that was necessary due to 
the way MIDI delta times work. I needed a way to keep track of the fractional 
time remaining when going from milliseconds to MIDI delta times. The 
accumulator just collects the pieces of a delta until a full delta is reached 
adding 1 to the calculated delta time when this happens.

I can see how this accumulator thing could be problematic if the BPM changes 
happened to be offset incorrectly by one delta either way. That would 
definitely mess up the note timings, especially if the incorrectly timed beat 
marker was a large BPM change. A better fix would be to change the way the 
tempo map is written to the MIDI file. Instead of running the beat marker 
positions through eof_calculate_delta() we could just write the tempo changes 
at the correct deltas (beat# * MIDI_RESOLUTION). This would guarantee the tempo 
map was correct in the MIDI file and would probably fix the original issue. 
Code would be something like this:

int current_ppqn = DEFAULT; // can't remember the default off the top of my head
int current_delta = 0;
// insert code to write track header here
for(i = 0; i < eof_song->beats; i++)
{
    if(eof_song->beat[i].ppqn != current_ppqn)
    {
        // write MIDI delta followed by BPM change event
        current_delta = 0;
    }
    else
    {
        current_delta += MIDI_RESOLUTION;
    }
}

This code is untested but should be basically what you need to do to write the 
correct tempo map.

Original comment by xander4j...@yahoo.com on 4 Aug 2010 at 2:42

[GoogleCodeExporter]

I'm going to work on some code to build a list of anchors, storing the delta 
times (using multiples of the time division being used) and real time of each.  
This will make allow the realtime->delta conversion to be simpler and hopefully 
more reliable.  This will also allow the tempo map to be written with perfect 
accuracy, as each beat is (time division) number of deltas ahead of the 
previous beat.

Original comment by raynebc on 4 Aug 2010 at 11:48

[GoogleCodeExporter]

ansem28 posted a chart that also imports incorrectly.  He also claims that it 
plays one or more seconds out of sync in the game, but that may be due to 
incorrect chart sync or AV delay:
http://www.megaupload.com/?d=KGY3EFRU

Original comment by raynebc on 5 Aug 2010 at 2:13

[GoogleCodeExporter]

On ansem28's chart, I noticed that if I import the MIDI and change the MIDI 
delay to 0, the sync is largely corrected.  This may be a clue.

Original comment by raynebc on 5 Aug 2010 at 2:15

[GoogleCodeExporter]

r269 implements NewCreature's suggested tempo map logic.  I implemented this in 
a way that will allow me to to re-use my program's ConvertToDeltaTime() 
function to provide the delta time conversions for all other written MIDI 
events.  The rounding errors should be minimal, due to the way that absolute 
delta and real times are used, and that the math involved is all double 
floating point clear up to when rounding the converted delta time to an integer 
type value.

Original comment by raynebc on 5 Aug 2010 at 9:12

• Changed state: Started

[GoogleCodeExporter]

r271 implements the tempo map logic for the rest of MIDI export.  It appears to 
be working wonderfully.  Now all that's left is to work on the import logic 
some more.

Original comment by raynebc on 5 Aug 2010 at 11:36

[GoogleCodeExporter]

I noticed that when importing a Rock Band MIDI (Hungry Like The Wolf):
http://www.sendspace.com/file/1iifrh

The last few guitar notes are loaded with the wrong times.  Some logic that has 
been changed since EOF 1.65 is causing this, because that stable release 
imports it correctly.

Original comment by raynebc on 6 Aug 2010 at 1:48

[GoogleCodeExporter]

r272 fixes the issue where MIDI notes at or after the final anchor don't import 
with the right timing.  Hungry Like The Wolf imports correctly now.

Original comment by raynebc on 6 Aug 2010 at 2:13

[GoogleCodeExporter]

r274 makes significant progress, but there's some lingering issue where notes 
incorrectly have "crazy" status applied to them.

Original comment by raynebc on 6 Aug 2010 at 11:00

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Here are some things I noticed regarding MIDI import:

ptotal_events is only incremented for events that are interesting, making the 
percent counter inaccurate.  It could probably be incremented for all events as 
long as the switch's default case (unidentified event) is not reached.

The text/lyric event import logic has a weakness in that if the text is >= 256 
characters long, it could cause a buffer overflow.

Original comment by raynebc on 11 Aug 2010 at 9:07

[GoogleCodeExporter]

r294 fixes the note skew during MIDI import.  The HOPO detection still needs 
work though, as the imported chart seems to miss the HOPO phrases.

Original comment by raynebc on 14 Aug 2010 at 7:06

[GoogleCodeExporter]

With r295, HOPOs now import correctly.  The majority of MIDI import issues 
people will probably see will be due to previous glitched saves.  With my 
testing so far, a fresh save will import as expected, and Rock Band MIDIs 
import as expected (issue 81 still occurs, but I've only seen missed events in 
PART VOCALS, so it will remain its own separate issue) so I will consider this 
fixed.

Original comment by raynebc on 14 Aug 2010 at 7:50

• Changed state: Done