craigsapp/midifile

MidiFile::mergeTracks needs to fix track field in all events that are moved

beldenfox opened this issue · 2 comments

As mergeTracks moves the event lists upward to get rid of aTrack2 it doesn't update the track field of the events being moved. The loop is

for (int i=aTrack2; i<length-1; i++) {
	m_events[i] = m_events[i+1];
}

After this loop all the events in m_events[aTrack2] on up will have track fields with their old track indices instead of their new ones.

Correct again! Obviously I have not used MidiFile::mergeTracks very often...

I updated the above code snippet to:

   for (int i=aTrack2; i<length-1; i++) {
      m_events[i] = m_events[i+1];
      for (int j=0; j<(int)m_events[i]->size(); j++) {
         (*m_events[i])[j].track = i;
      }
   }

Which updates the track field in the individual events of the moved tracks.

In general I do not look at the MidiEvent::track variable when the file is in type-1 mode (multi-track). When the file is in type-0 mode (single-track), it is often useful to know what the original multi-track track number was. So previously the track information will be incorrect after a mergeTracks operation followed by a joinTracks. Hopefully it is now better... It might be best to do the track assignment in joinTracks before the tracks are merged to guarantee that they are correct, but this should be good enough for now. Currently a library user could assign whatever they want as a track number (which might be another way to move tracks around by giving new track numbers, then doing joinTracks followed by splitTracks.

Fixed with commit 66c471a

The fix you provided is working fine for me. FYI, I'm using MidiEvent::track field to properly handle the MIDI Port meta-event which only affects other events in the same track.
Thanks for writing this library, it's been very useful.