melanchall / drywetmidi

.NET library to read, write, process MIDI files and to work with MIDI devices
https://melanchall.github.io/drywetmidi
MIT License
540 stars 75 forks source link

TempoEvent Parsing Issue #19

Closed Yorgg closed 5 years ago

Yorgg commented 5 years ago

Hello,

I am using Logic Pro X 10.0.0 to create piano midi files.

Each of these files has a single tempo, and it is found in the last midi event (as a tempo-change event).

I used these midi files in another program, and the playback was fine. However, Drywetmidi does not seem to correctly parse them and always uses the default (120bpm) tempo.

For a quick fix (untested) I modified the CollectTempoChanges() method in TempoMapManager.cs

 private void CollectTempoChanges()
        {
            int count = 0;
            var lastEvent = _timedEventsManagers.SelectMany(m => m.Events).Last();

            foreach (var tempoEvent in GetTimedEvents(IsTempoEvent))
            {
                count += 1;

                var setTempoEvent = tempoEvent.Event as SetTempoEvent;

                if (setTempoEvent == null)
                    continue;

                // if:
                //   - the last event in the midi file is the tempo event, and
                //   - there is only one tempo event, 
                // then make the time of the tempo event = 0

                // this is so if the last event is the tempo event 
                // we capture that tempo for the entire file 
                // (a fix for Logic Pro X) 

                bool oneTempoChangeAndIsLastEvent = count == 1 && lastEvent == tempoEvent;
                long time = oneTempoChangeAndIsLastEvent ? 0 : tempoEvent.Time;

                TempoMap.Tempo.SetValue(time, new Tempo(setTempoEvent.MicrosecondsPerQuarterNote));
            }
        }

I also attached an example midi file, which was exported from Logic Pro X 10.0.0: prelude_124bpm.mid.zip

melanchall commented 5 years ago

Hi @Yorgg,

Thank you for reporting the issue. But I don't see a file you've attached. Please attach it again. And the problem is last tempo change event is missed in TempoMap, yes?

Yorgg commented 5 years ago

Hi,

Yes, correct. Here is the file: prelude_124bpm.mid.zip

melanchall commented 5 years ago

Thanks, I see that last tempo change event. But this is the only tempo change event in the file and it is the last event as you said. This event presented in tempo map. And... What's wrong?

What does this mean: Drywetmidi does not seem to correctly parse them and always uses the default (120bpm) tempo? Events in MIDI file are processed according to time where they are placed. If you expect that this tempo change should apply to entire file from its start, it is wrong assumption.

Yorgg commented 5 years ago

Hi,

Thanks for clarifying.
I wasn't sure if this is a bug for Logic Pro X. I guess it is probably a bug?
I'm not sure why Logic is placing the tempo change event at the end, and not at the beginning.

melanchall commented 5 years ago

Hmm, strange behavior.. especially from professional DAW like Logic Pro. Please describe entire workflow starting from placing tempo change in Logic Pro and ending with exporting a MIDI file.

Also: are you sure you're placing tempo change at the start of MIDI file in Logic Pro? Maybe at this moment cursor is placed at the end of file?

melanchall commented 5 years ago

@Yorgg Did you solve the problem?

Yorgg commented 5 years ago

Hi, I wasn't able to figure out what was causing Logic to save the file in this way.
I will close this issue. Thanks for the help!

melanchall commented 5 years ago

@Yorgg Although you've closed the issue please try to install latest version of Logic Pro since it can have fixes for some bugs including the one you discovered.