cuthbertLab / music21

music21 is a Toolkit for Computational Musicology
https://www.music21.org/
Other
2.1k stars 398 forks source link

Retain channel information from MIDI #465

Open nightscape opened 4 years ago

nightscape commented 4 years ago

We're trying to read a MIDI file generated by a Jammy guitar. The Jammy uses 6 MIDI channels for the 6 strings of the guitar. We would like to retain this information during MIDI => Music21 transformation in order to e.g. do analysis later on how a chord was fingered. Is there some way to transform MIDI into a Music21 stream without losing the channel information?

Thanks!

(Note: This is a cross-post from here https://github.com/cuthbertLab/music21/issues/206, I wasn't sure if someone would see this in a closed ticket).

mscuthbert commented 4 years ago

I think that storing this information in the .editorial object would be a good place for it.

jacobtylerwalls commented 3 years ago

hiya @nightscape I put together a little proof of concept for this. Can you take it for a spin and let us know if it suits your needs? I was took your report to mean you had a single midi track for the guitar, and the notes in that track freely used 6 channels. Is that right?

Meekohi commented 3 years ago

Noticing this problem as well. If you a load a multi-track MIDI file, and then immediately save it, the tracks are assigned to seemingly arbitrary channels.

import copy
from music21 import *

orig = converter.parse('test/4part.mid')
orig.write('midi', fp='output.mid')

https://www.dropbox.com/s/oln4y19yn81kqhc/4part.mid?dl=0

The weirdest is that some of the tracks are assigned "Choir Ooos" which wasn't even in the original file. Must be some sort of logic currently in place based on track name?

jacobtylerwalls commented 3 years ago

the tracks are assigned to seemingly arbitrary channels.

Yes, since channel information is not retained, it's created afresh. In your example I see channel 1 for the generic "soloist" and channel 2 for the SATB tracks, each of which have a 0-indexed midiProgram 53, see below...

See channelInstrumentData().

The weirdest is that some of the tracks are assigned "Choir Ooos" which wasn't even in the original file. Must be some sort of logic currently in place based on track name?

This should probably be tracked separately (program, rather than channel). What's happening is that the same routine is used to process both PROGRAM_CHANGE messages and INSTRUMENT_NAME or SEQUENCE_TRACK_NAME messages, and that routine attempts to set the midiProgram if the message has that data, which a SEQUENCE_TRACK_NAME message will not. From the SEQUENCE_TRACK_NAME messages in your file, we normalized each of "Soprano", "Alto", etc. to an instrument.Soprano, etc., instance with default midiProgram = 53 (0-indexed, so = 54: Choir Oohs). We should be smarter about that.

Meekohi commented 3 years ago

I wonder if it would be practical to add some metadata to stream.Part, like part.metadata.midiChannel = 1, part.metadata.midiInstrument = 1 or etc during loading, and then check for that during .write to return the MIDI back to the original condition (or someone could use that to micro-manage how they want the final midi output to look)?

jacobtylerwalls commented 3 years ago

For MIDI program changes, that should be possible today with inserting Instrument instances (which have a midiProgram attribute) into streams. (I believe we write patch change messages when we encounter instrument instances. Do let me know if I'm wrong about that.

The channel stuff is buried a bit deeper. But there would be a way to send an acceptableChannelList to channelInstrumentData() or even potentially override it.

I do think we should start capturing midi channel data (this ticket) and also avoiding false program data based on reading instrument name messages (the ticket I just created), so thanks for the report!

mscuthbert commented 3 years ago

Just a note that there won't be a guarantee ever that channel In = channel Out if pitch bends are involved, since we write different bends to different channels.

It's good to get many of these things in if easy (I don't want midiChannel on metadata; but it can go in editorial), but music21 is not and will never be a full MIDI<->MIDI platform, the same way we'd like to be with MusicXML, so some differences will always remain (and not be a bug)

Meekohi commented 3 years ago

Yeah totally understood @mscuthbert -- for us at least, if everything just defaulted to a "Piano" channel that would actually be fine, but the introduction of new channels that weren't in the original MIDI is causing us some issues. Hopefully there is some low-hanging fruit that will make things useful for simple MIDI applications that aren't using any complicated MIDI events or etc. 👍🏼

duhaime commented 3 years ago

I just wanted to mention that adding channel information will help users like this developer on StackOverflow who want to isolate percussion within data files (as percussion is typically stored in channel 9 or 10 on midi depending on 0 or 1 based indexing). Should that channel be given special treatment to make sure e.g. pitch bends don't end up in there?

Edit: it looks like you're already giving channel 9/10 special treatment. :)

thomasarvanitidis commented 2 years ago

I'm a user like the one @duhaime mentioned above and such fix would make music21 much more viable for the drums MIDI generation community. I also noticed that a "Pitch Bend" event is also added when simply loading -> writing a drums MIDI file with music21, which I don't understand why that might be necessary. Hoping to see this fix in soon! :)

jacobtylerwalls commented 2 years ago

I just wanted to mention that adding channel information will help users like this developer on StackOverflow who want to isolate percussion within data files (as percussion is typically stored in channel 9 or 10 on midi depending on 0 or 1 based indexing). Should that channel be given special treatment to make sure e.g. pitch bends don't end up in there?

Just wanted to let folks visiting the thread know about some recent improvements (the SO thread is very old).

  1. The SO poster was querying part names read from the MIDI file. We're much better with that now. Here's an example using the MIDI file on the SO thread:

    >>> for p in s.parts:
    ...   print(p.partName)
    ... 
    Vocal
    Backup Vocals
    Bass
    Organ
    Acou Guitar
    Pedal Steel
    Mute Guitar
    Ovdr Guitar
    Ovdr Guitar
    Dobro Guitar
    Dobro Guitar
    Dobro Guitar
    Strings
    Drums
  2. We also have Instrument instances you can operate with:

>>> for p in s.parts:
...   print(p.getInstrument(recurse=True))
... 
Vocal: Alto Saxophone
Backup Vocals: Voice
Bass: Bass
Organ: Organ
Acou Guitar: Acou Guitar
Pedal Steel: Electric Guitar
Mute Guitar: Mute Guitar
Ovdr Guitar: Ovdr Guitar
Ovdr Guitar: Ovdr Guitar
Dobro Guitar: Dobro Guitar
Dobro Guitar: Dobro Guitar
Dobro Guitar: Dobro Guitar
Strings: StringInstrument
Drums: Percussion

Edit: it looks like you're already giving channel 9/10 special treatment. :)

Yes, we are creating at the very least instances of UnpitchedPercussion when we encounter events on channel 10 (9 0-indexed), and even more specific instances when we can recognize them (Snare, etc). This is because music21 is designed to translate various formats into a common representation.

I'm a user like the one @duhaime mentioned above and such fix would make music21 much more viable for the drums MIDI generation community.

Some earlier posters on this GitHub issue have shown cases where storing the original MIDI channel would be useful for specific use cases, but just wanted to mention the tooling we already have for the basic case.

I also noticed that a "Pitch Bend" event is also added when simply loading -> writing a drums MIDI file with music21, which I don't understand why that might be necessary.

See https://github.com/cuthbertLab/music21/issues/305#issuecomment-429551892 -- the idea it to ensure it's always cleared from the previous event. Agree that an optimization here would be to avoid writing them when unnecessary. Should track in a separate issue. (Tangent: doing a full-text search in the repo for "pitch bend" I noticed there's a JSymbolic feature GlissandoPrevalenceFeature that once implemented someday would probably benefit from not having superfluous pitch bend events in the sources).