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

Create more specific instruments from MusicXML import #988

Closed
retorquere opened this issue Apr 28, 2021 · 18 comments · Fixed by #1056
Closed

Create more specific instruments from MusicXML import #988

retorquere opened this issue Apr 28, 2021 · 18 comments · Fixed by #1056

Comments

@retorquere
Copy link

op-voor-op-achter.txt

When I open this file directly with musicxml, this shows and plays as a drum score. When I open it using this:

from music21 import *
s = converter.parse('op-voor-op-achter.musicxml')
s.show()

it opens as a piano score, and the notes that used to be hihats (top of the bar) are now below the bar

@retorquere
Copy link
Author

Also this:

import music21 as m

s = m.converter.parse('op-voor-op-achter.musicxml')
for note in s.recurse().notes:
  print(note.name)

shows all notes to be C in the attached file? That isn't right, there's 3 different notes in the score.

@jacobtylerwalls
Copy link
Member

Hello, and welcome. In the future please use the bug template. For instance, what version of music21 are you using? On the latest version (6.7.1), MuseScore and Finale both show percussion clef and drumset:

Screen Shot 2021-04-28 at 10 08 54 AM

Regarding the unpitched noteheads appearing below the staff, this is a known missing feature on the roadmap for v7 (#235).

No guarantees what features will make it in to v7, but stay tuned for announcements about the release of v7 on the mailing list if you like. Cheers

@retorquere
Copy link
Author

Apologies - I didn't see a template, I just got a blank issue form.

@retorquere
Copy link
Author

I'm on music21 6.7.1 and musescore 3.6.2.538020600

@jacobtylerwalls
Copy link
Member

Thanks for the additional info. Can you clarify what you mean by piano score?

@retorquere
Copy link
Author

Sorry - I meant that it sounded like a piano when I played it.

@jacobtylerwalls
Copy link
Member

Thanks. If you install the dev version you might find that piece is already resolved -- see #961 .

@retorquere
Copy link
Author

Superb -- how do I install the dev version?

@jacobtylerwalls
Copy link
Member

pip3 uninstall music21
pip3 install git+https://github.com/cuthbertLab/music21.git

@retorquere
Copy link
Author

retorquere commented Apr 28, 2021

I don't know how I got to the issue submission screen previously but I got here through https://github.com/cuthbertLab/music21/issues/new/ rather than https://github.com/cuthbertLab/music21/issues/choose/. I believe that can be disabled somewhere.

When I run the dev version with this:

#!/usr/bin/env python3

import music21 as m

s = m.converter.parse('op-voor-op-achter.musicxml')
for note in s.recurse().notes:
  print(note.name)

s = m.converter.parse('op-voor-op-achter.musicxml')
s.show()

I still get only C notes listed, and when musescore opens it shows
Screenshot 2021-04-28 at 20 18 36

and it doesn't have percussion instrument selected.

@jacobtylerwalls
Copy link
Member

I believe that can be disabled somewhere.

You're quite right, we just weren't ready to do that until we added a feature request template last week. You are quite knowledgable on this, so forgive me if I sounded out of breath! All is well.

So the C notes are the missing feature we haven't developed yet. We're not reading that info from musicXML yet because we need to decide on the internal model for unpitched notes. Hopefully coming later this year.

This is the instrument m21 found:
<music21.instrument.Instrument 'P1: Drumset: Acoustic Bass Drum'>

We should have created at the very least a BassDrum object; better yet, a Drum Set. While all the particulars with percussion are going to need to wait until the rest of the percussion work is attacked, at the very least we should be able to perform better instrument identification either from the part name or the midi program given. I can reproduce this on a piece not involving percussion:

>>> s = corpus.parse('beach')
>>> s.parts.first().getInstrument()  # expect instrument.Soprano
<music21.instrument.Instrument 'P1: Soprano I: Grand Piano'>

So I will accept this issue on that basis, this is a manageable chunk someone could work on independently from the percussion work. Thanks for using the package and for writing us this report! Cheers.

@jacobtylerwalls jacobtylerwalls changed the title Musicxml file opened through music21 changes instruments Create more specific instruments from MusicXML import Apr 28, 2021
@jacobtylerwalls
Copy link
Member

Related to #159 but this should be worked on first. The default instrument should be parsed correctly, then the parser should have knowledge of all correct instruments, and THEN we can do #159 to insert instrument changes into m21 streams from <sound> tags.

@retorquere
Copy link
Author

You're quite right, we just weren't ready to do that until we added a feature request template last week. You are quite knowledgable on this, so forgive me if I sounded out of breath! All is well.

I recognize the exasperation as I'm dealing with the same issue with my own repo, to the point I've written a bot to cheerily complain on my behalf when info is missing. All is well.

It's good to know that this is something in the pipeline. I'll just use midi parsing for the interim.

@jacobtylerwalls
Copy link
Member

I'll just use midi parsing for the interim.

Then you'll definitely want to be on the dev version, we made a lot of MIDI improvements in it. One more fix for rests in voice (small thing, not every MIDI file affected) will be merged likely this week in #986 if not tonight.

@retorquere
Copy link
Author

I am on the dev version, but still not much luck for percussion (although I'll be happy to open a new issue for this):

import music21 as m
mf = m.midi.MidiFile()
mf.open('op-voor-op-achter.mid')
mf.read()
mf.close()
print(len(mf.tracks))
s = m.midi.translate.midiFileToStream(mf)

outputs

1
Traceback (most recent call last):
  File "/Users/emile/Documents/./mxl2dtx", line 21, in <module>
    s = m.midi.translate.midiFileToStream(mf)
  File "/usr/local/lib/python3.9/site-packages/music21/midi/translate.py", line 2733, in midiFileToStream
    midiTracksToStreams(mf.tracks,
  File "/usr/local/lib/python3.9/site-packages/music21/midi/translate.py", line 2473, in midiTracksToStreams
    midiTrackToStream(mt,
  File "/usr/local/lib/python3.9/site-packages/music21/midi/translate.py", line 1833, in midiTrackToStream
    metaEvents = getMetaEvents(events)
  File "/usr/local/lib/python3.9/site-packages/music21/midi/translate.py", line 1718, in getMetaEvents
    metaObj = midiEventsToInstrument(e)
  File "/usr/local/lib/python3.9/site-packages/music21/midi/translate.py", line 766, in midiEventsToInstrument
    i = pm.midiPitchToInstrument(event.data + 1)
  File "/usr/local/lib/python3.9/site-packages/music21/midi/percussion.py", line 159, in midiPitchToInstrument
    raise MIDIPercussionException(f'{midiNumber!r} does not map to a valid instrument!')
music21.midi.percussion.MIDIPercussionException: 1 does not map to a valid instrument!

although I can't say for sure the midi file is perfect. mido can parse it though.
op-voor-op-achter.zip

@jacobtylerwalls
Copy link
Member

Thanks for the report. Here are the relevant events m21 tries to read for instruments:

<music21.midi.MidiEvent SEQUENCE_TRACK_NAME, track=0, channel=None, data=b'Drumset\x00'>
[<music21.midi.MidiEvent PROGRAM_CHANGE, track=0, channel=10, data=0>]

I don't know if the first one is handled, and I don't know if the second event is legal--it's the one that's crashing. Given the several problems with this file (null-terminated instrument name, note events in the first track instead of conductor track, "0" program change), I'm guessing MuseScore wrote it? We generally let these errors bubble up for software-coded (as opposed to hand-coded) formats, but I think we could reduce the severity of this failure, given that we at least had "drumset" in another event.

@retorquere
Copy link
Author

Yep, this one was written by musescore.

@jacobtylerwalls
Copy link
Member

No need for a second issue -- I'll open a PR to reduce severity of this failure. THANKS for catching it -- always love to have more testers before we put out a release

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

Successfully merging a pull request may close this issue.

2 participants