gregchapman-dev / converter21

A music21-based music notation file format converter CLI app, and some new sub converter plug-ins
Other
16 stars 1 forks source link

An 8va ottava prints at 2 octaves down instead of 1 #8

Closed gregchapman-dev closed 12 months ago

gregchapman-dev commented 1 year ago

Test humdrum file is converter21/tests/files/valid/ottava.krn.

  1. Use converter21's Humdrum converter to parse this file, then show it (or write to MusicXML and give that to Musescore or Finale).
  2. Comment out the 8va and X8va, and convert/show again.
  3. Compare the rendered scores by eye.
  4. Note that in the score with the ottava, the notes are printed 2 octaves down.

@mscuthbert, can you take a look?

gregchapman-dev commented 1 year ago

Some code I use to register my converter in place of music21's:

from music21 import converter from converter21 import HumdrumConverter converter.unregisterSubconverter(converter.subConverters.ConverterHumdrum) converter.registerSubconverter(HumdrumConverter) s = converter.parse(...)

mscuthbert commented 1 year ago

Can you do a show('text') on what's written? I'd like to see whether the problem is on import or export. Thanks! Away from laptop now.

mscuthbert commented 1 year ago

Btw: there will be some improvements for registering and deregisrering Subconverters in v9.

gregchapman-dev commented 1 year ago

Here's the show('text') output as well as the musicxml output. showtext.txt tmp0h8mta1s.musicxml.zip

gregchapman-dev commented 1 year ago

And here's MuseScore's rendering of the MusicXML. tmpxsojgau6.pdf

gregchapman-dev commented 1 year ago

My reading of the difference between showtext.txt and the MusicXML file is that in the MusicXML the note values have been dropped by an octave and they are in an <octave-shift> that is asking them to be printed an octave below that. So perhaps this is related to the bug @jacobtylerwalls was discussing where the first and last notes in an Ottava dropped an extra octave. But of course in this bug, all the notes dropped an extra octave.

gregchapman-dev commented 1 year ago

@mscuthbert @jacobtylerwalls Any further thoughts here?

jacobtylerwalls commented 1 year ago

@gregchapman-dev Hi Greg. I just used your converter to parse the provided file, and it didn't produce any ottavas. Here's the text dump of the first measure in the RH, notice no ottavas:

    {0.0} <music21.stream.Measure 63 offset=0.0>
        {0.0} <music21.clef.TrebleClef>
        {0.0} <music21.tempo.MetronomeMark Largo e mesto Quarter=40>
        {0.0} <music21.key.Key of d minor>
        {0.0} <music21.meter.TimeSignature 6/8>
        {0.0} <music21.stream.Voice 0x106cc8790>
            {0.0} <music21.chord.Chord E-5 E-6>
            {0.75} <music21.chord.Chord D5 D6>
            {1.0} <music21.chord.Chord C5 C6>
            {1.25} <music21.chord.Chord E-5 E-6>
            {1.5} <music21.chord.Chord F5 F6>
            {2.25} <music21.chord.Chord E5 E6>
            {2.5} <music21.chord.Chord D5 D6>
            {2.75} <music21.chord.Chord F5 F6>
        {0.0} <music21.stream.Voice 0x106ccb670>
            {0.0} <music21.dynamics.Dynamic ffp>
            {0.0} <music21.chord.Chord F#5 A5>
            {1.5} <music21.dynamics.Dynamic ffp>
            {1.5} <music21.chord.Chord G#5 B5>
        {3.0} <music21.bar.Barline type=regular>

Are you testing against the dev version of music21?

jacobtylerwalls commented 1 year ago

Oh, maybe I need to check out your branch.

jacobtylerwalls commented 1 year ago

I'm hoping this is quick, but a repro from scratch would help!

gregchapman-dev commented 1 year ago

Taking a look to see if I can repro with some music21 generated with a code snippet.

jacobtylerwalls commented 1 year ago

All good - I reproduced with your example and looking into it now.

jacobtylerwalls commented 1 year ago

Alright, I stepped through it and see what's going on.

This is partially intersecting with cuthbertLab/music21#1419.

music21 currently runs toWrittenPitch() for musicxml output, which makes sense for instrument transposition, but not for ottava transpositions, as musicxml documents are supposed to encode written pitches w/r/t instrument transpositions but sounding pitches for ottava transpositions. So we need to decouple the functionality, as musicxml requires a squishy sort-of-sounding-sort-of-written-score.

My comment on cuthbertLab/music21#1419 requests some sort of decoupling, but now that I see we have parallel problems in both directions (non-transposing spanner -> transposing spanner, as well as transposing spanner -> non-transposing spanner), we probably don't need undoOttavaTransposition=True but rather something like ottavasToSounding=True to handle both directions.

I'll leave a note on #1419.

The reason this doesn't affect the terminal notes only is that your spanner contains all of the spanned elements--as opposed to skipping/spanning over them--unlike the OP on #1419.

gregchapman-dev commented 1 year ago

Let me know if I can help at all. Thanks!

jacobtylerwalls commented 1 year ago

If you're open to drafting a PR, I'd be glad to review it. I don't know that I've got the time to take the lead right now. The essence if it wasn't evident is that we just need a second mode of toWrittenPitch() that does the opposite transformation for ottavas that we can employ in musicxml export.

gregchapman-dev commented 1 year ago

Ok, I’ll take a shot at it.

On Nov 24, 2022, at 5:45 AM, Jacob Walls @.***> wrote:

 If you're open to drafting a PR, I'd be glad to review it. I don't know that I've got the time to take the lead right now. The essence if it wasn't evident is that we just need a second mode of toWrittenPitch() that does the opposite transformation for ottavas that we can employ in musicxml export.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

gregchapman-dev commented 1 year ago

Working on this in cuthbertLab/music21#1486 ("Ottava transposition fixes").

gregchapman-dev commented 12 months ago

Fixed in music21 (merged into v9 alpha), but not in a music21 official release yet.