search

craffel / pretty-midi

Utility functions for handling MIDI data in a nice/intuitive way.
MIT License
856 stars 151 forks source link

_load_metadata does not load lyrics correctly #214

Open holgerkirchhoff opened 2 years ago

holgerkirchhoff commented 2 years ago

_load_metadata seems to assume that lyrics meta events -- similar to tempo and time signature -- are always found on track 0. Looking at the Standard MIDI Files Specification this is not required, and I have MIDI files in which the lyrics events are part of the vocal track, which makes sense to me.

In effect, when loading and saving a MIDI file in the current version (0.2.9), all lyrics events of the vocal track are lost. I assume the same applies to key signature changes that are not on track 0.

craffel commented 2 years ago

I'm not sure where I read that all meta events (including lyrics events) should be on track 0. Feel free to make a PR.

holgerkirchhoff commented 2 years ago

I wonder if lyrics would need to be handled a bit differently in the class? You are currently keeping it as a single member. However, I guess you could have multiple vocal tracks in a MIDI file with different lyrics. Hence it needs to be part of the Instrument class.

craffel commented 2 years ago

I'm not aware of many pieces of music where different singers are singing different words at the same time, but if this ever happens I agree that making it part of the Instrument class makes sense.

holgerkirchhoff commented 2 years ago

I guess it is quite common when you have a lead singer and backing vocals. Or just think about classic choral works, opera, etc.

Having lyrics in the right vocal track also makes it easier to assign syllables to corresponding notes. If all lyrics were on track 0, it would not be immediately clear what notes the lyrics belong to.

I'm not sure if I know enough details about the code to make these changes.

craffel commented 2 years ago

That makes sense. I won't be able to make these changes myself but if someone makes a PR I can review.