Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

_load_metadata does not load lyrics correctly #214

Open
holgerkirchhoff opened this issue Aug 30, 2022 · 5 comments
Open

_load_metadata does not load lyrics correctly #214

holgerkirchhoff opened this issue Aug 30, 2022 · 5 comments

Comments

@holgerkirchhoff
Copy link

_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
Copy link
Owner

craffel commented Aug 30, 2022

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
Copy link
Author

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
Copy link
Owner

craffel commented Aug 30, 2022

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
Copy link
Author

holgerkirchhoff commented Aug 31, 2022

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
Copy link
Owner

craffel commented Aug 31, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants