MusicXML alter element not exported

[infojunkie]

Issue type

Import/export issue

Bug description

The MusicXML alter element is not exported when a note has a tuning.

Steps to reproduce

• Load the attached score
• Verify that E in measure 1 is tuned -50 cents
• Export to MusicXML
• Examine the exported E note in measure 1: the <alter> element is missing. It should show:

<alter>-0.5</alter>

Screenshots/Screen recordings

No response

MuseScore Version

4.3.1-241490902, revision: github-musescore-musescore-026c26b

Regression

I don't know

Operating system

Pop!_OS 22.04 LTS

Additional context

No response

[lvinken]

As far as I remember, this was never implemented.

[Jojo-Schmitz]

So it more like a feature request

[pacebes]

Hi

I have been looking into the code and it should work with the following patch (please, read the complete comment)

pacebes@cozuelos:/c/D/MuseScore$ git diff
diff --git a/src/importexport/musicxml/internal/musicxml/exportxml.cpp b/src/importexport/musicxml/internal/musicxml/exportxml.cpp
index 480cb00500..f769edb61d 100644
--- a/src/importexport/musicxml/internal/musicxml/exportxml.cpp
+++ b/src/importexport/musicxml/internal/musicxml/exportxml.cpp
@@ -4168,6 +4168,14 @@ static void writePitch(XmlWriter& xml, const Note* const note, const bool useDru
     if (!alter && alter2) {
         xml.tag("alter", alter2);
     }
+
+    if (!alter && !alter2) {
+        double tuning = note->tuning();
+        if (tuning != 0.0) {
+            xml.tag("alter", tuning / 100);
+        }
+    }
+
     // TODO what if both alter and alter2 are present? For Example: playing with transposing instruments
     xml.tag(useDrumset ? "display-octave" : "octave", octave);
     xml.endElement();

With this patch you would get this xml: rast-tetrachord.zip. Could you please tell me @infojunkie if this is correct ?

However, I think this should be reviewed by an expert because if you dig into the code it seems that there are different types of "alter"

• Current "alter", it comes from "tpc"

tpc in note.h

//   @P tpc              int              tonal pitch class, as per concert pitch setting
//   @P tpc1             int              tonal pitch class, non transposed
//   @P tpc2             int              tonal pitch class, transposed

.....
    int tpc() const;
    int tpc1() const { return m_tpc[0]; }                  // non transposed tpc
    int tpc2() const { return m_tpc[1]; }                  // transposed tpc

and it's used in exportxml.cpp

static void pitch2xml(const Note* note, String& s, int& alter, int& octave)
{
    const Staff* st = note->staff();
    const Fraction tick = note->tick();
    const Instrument* instr = st->part()->instrument(tick);
    const Interval intval = note->concertPitch() ? 0 : instr->transpose();

    s      = tpc2stepName(note->tpc());
    alter  = tpc2alterByKey(note->tpc(), Key::C);

• There is an "alter2" (exportxml.cpp again)

    double alter2 = 0.0;
    if (acc) {
        switch (acc->accidentalType()) {
        case AccidentalType::MIRRORED_FLAT:  alter2 = -0.5;
            break;
        case AccidentalType::SHARP_SLASH:    alter2 = 0.5;
            break;
        case AccidentalType::MIRRORED_FLAT2: alter2 = -1.5;
            break;
        case AccidentalType::SHARP_SLASH4:   alter2 = 1.5;
            break;
        default:                                             break;
        }
    }

• The method does know how to generate the xml if both alter and alter2 are present (exportxml.cpp)

 // TODO what if both alter and alter2 are present? For Example: playing with transposing instruments

Any case I think the proposed patch is harmless because only if current alter and alter2 are empty the program adds the "alter" tag with the desired logic and would not impact at all if current alter or alter2 are used to generate the XML

Kind regards

[pacebes]

Hi

OK @Jojo-Schmitz , ¿ Could you please asign this issue to me ?

Kind regards

[pacebes]

Hi

I have already created a pull request as you can see above. After that it ocurred to me to make a different test:

• Export rast-tetrachord.mscz to xml
• Import the previously exported xml
• Export again the xml and compare them

Results are:

• First exported file: rast-tetrachord - exported.zip

• Exported + imported + exported again: rast-tetrachord - exported-imported-exported.zip

They should not be different but if you analyze the result in the exported-imported-exported file:

• This is added to the first "E"
  <accidental cautionary="yes" parentheses="no">quarter-flat</accidental>

The notes in both files sound the same to me, but they look different in MuseScore
Original:

Exported and imported:

• The second "D" loses;
  <alter>0.05</alter>
  The notes in both files sound different.

• The second "E" loses:
  <alter>-0.15</alter>
  The notes in both files sound different.

So, perhaps we should also review the import methods.

Kind regard

[Jojo-Schmitz]

Yes, I noticed that too when loading those 2 files. Seems indeed an import issue, see also https://musescore.org/en/node/303920

See also the 4 years old issue https://musescore.org/en/node/311792

[miiizen]

With regards to the import issues: microtonal accidentals are guessed from the alter value on import. This is fine in itself, but the note's tuning must also be set when setting the note's accidental (in MusicXMLParserPass2::note).
Also, the <alter> value is converted to an int, so any microtonal information is discarded (in MxmlNotePitch::pitch).

[Jojo-Schmitz]

Would this work for import?

diff --git a/src/importexport/musicxml/internal/musicxml/importmxmlnotepitch.cpp b/src/importexport/musicxml/internal/musicxml/importmxmlnotepitch.cpp
index a55773b1aa..20524bdbb5 100644
--- a/src/importexport/musicxml/internal/musicxml/importmxmlnotepitch.cpp
+++ b/src/importexport/musicxml/internal/musicxml/importmxmlnotepitch.cpp
@@ -129,6 +129,7 @@ void MxmlNotePitch::pitch(muse::XmlStreamReader& e)
     // defaults
     m_step = -1;
     m_alter = 0;
+    m_microtonalAlter = 0.0;
     m_octave = -1;

     while (e.readNextStartElement()) {
@@ -145,6 +146,7 @@ void MxmlNotePitch::pitch(muse::XmlStreamReader& e)
                     m_accType = microtonalGuess(altervalue);
                 }
                 m_alter = 0;
+                m_microtonalAlter = altervalue;
             }
         } else if (e.name() == "octave") {
             const String oct = e.readText();
diff --git a/src/importexport/musicxml/internal/musicxml/importmxmlnotepitch.h b/src/importexport/musicxml/internal/musicxml/importmxmlnotepitch.h
index 3c4ebf5e5d..746bb2cae1 100644
--- a/src/importexport/musicxml/internal/musicxml/importmxmlnotepitch.h
+++ b/src/importexport/musicxml/internal/musicxml/importmxmlnotepitch.h
@@ -48,6 +48,7 @@ public:
     Accidental* acc() const { return m_acc; }
     AccidentalType accType() const { return m_accType; }
     int alter() const { return m_alter; }
+    int microtonalAlter() const { return m_microtonalAlter; }
     int displayOctave() const { return m_displayOctave; }
     int displayStep() const { return m_displayStep; }
     void displayStepOctave(muse::XmlStreamReader& e);
@@ -59,6 +60,7 @@ private:
     Accidental* m_acc = nullptr;                             // created based on accidental element
     AccidentalType m_accType = AccidentalType::NONE;         // set by pitch() based on alter value (can be microtonal)
     int m_alter = 0;
+    double m_microtonalAlter = 0.0;
     int m_displayStep = -1;                                  // invalid
     int m_displayOctave = -1;                                // invalid
     int m_octave = -1;
diff --git a/src/importexport/musicxml/internal/musicxml/importmxmlpass2.cpp b/src/importexport/musicxml/internal/musicxml/importmxmlpass2.cpp
index 4b566615b3..6d80d46382 100644
--- a/src/importexport/musicxml/internal/musicxml/importmxmlpass2.cpp
+++ b/src/importexport/musicxml/internal/musicxml/importmxmlpass2.cpp
@@ -6405,6 +6405,7 @@ static void setPitch(Note* note, const MusicXMLInstruments& instruments, const S
         }
     } else {
         xmlSetPitch(note, mnp.step(), mnp.alter(), mnp.octave(), octaveShift, instrument);
+        note->setTuning(mnp.microtonalAlter());
     }
 }

[pacebes]

HI

I have been also working on a patch and I have just finished now. It works

diff --git a/src/importexport/musicxml/internal/musicxml/importmxmlnotepitch.cpp b/src/importexport/musicxml/internal/musicxml/importmxmlnotepitch.cpp
index a55773b1aa..67bd663da9 100644
--- a/src/importexport/musicxml/internal/musicxml/importmxmlnotepitch.cpp
+++ b/src/importexport/musicxml/internal/musicxml/importmxmlnotepitch.cpp
@@ -143,8 +143,14 @@ void MxmlNotePitch::pitch(muse::XmlStreamReader& e)
                 if (ok2 && (std::abs(altervalue) < 2.0) && (m_accType == AccidentalType::NONE)) {
                     // try to see if a microtonal accidental is needed
                     m_accType = microtonalGuess(altervalue);
+
+                    // If it's not a microtonal accidental we will use tuning
+                    if (m_accType == AccidentalType::NONE) {
+                        m_tuning = 100 * altervalue;
+                    }
                 }
                 m_alter = 0;
+
             }
         } else if (e.name() == "octave") {
             const String oct = e.readText();
diff --git a/src/importexport/musicxml/internal/musicxml/importmxmlnotepitch.h b/src/importexport/musicxml/internal/musicxml/importmxmlnotepitch.h
index 3c4ebf5e5d..925f6fa085 100644
--- a/src/importexport/musicxml/internal/musicxml/importmxmlnotepitch.h
+++ b/src/importexport/musicxml/internal/musicxml/importmxmlnotepitch.h
@@ -48,6 +48,7 @@ public:
     Accidental* acc() const { return m_acc; }
     AccidentalType accType() const { return m_accType; }
     int alter() const { return m_alter; }
+    double tuning() const { return m_tuning; }
     int displayOctave() const { return m_displayOctave; }
     int displayStep() const { return m_displayStep; }
     void displayStepOctave(muse::XmlStreamReader& e);
@@ -59,6 +60,7 @@ private:
     Accidental* m_acc = nullptr;                             // created based on accidental element
     AccidentalType m_accType = AccidentalType::NONE;         // set by pitch() based on alter value (can be microtonal)
     int m_alter = 0;
+    double m_tuning = 0;
     int m_displayStep = -1;                                  // invalid
     int m_displayOctave = -1;                                // invalid
     int m_octave = -1;
diff --git a/src/importexport/musicxml/internal/musicxml/importmxmlpass2.cpp b/src/importexport/musicxml/internal/musicxml/importmxmlpass2.cpp
index 4b566615b3..ab9a46a2cd 100644
--- a/src/importexport/musicxml/internal/musicxml/importmxmlpass2.cpp
+++ b/src/importexport/musicxml/internal/musicxml/importmxmlpass2.cpp
@@ -267,7 +267,7 @@ static int MusicXMLStepAltOct2Pitch(int step, int alter, int octave)
  Note that n's staff and track have not been set yet
  */

-static void xmlSetPitch(Note* n, int step, int alter, int octave, const int octaveShift, const Instrument* const instr)
+static void xmlSetPitch(Note* n, int step, int alter, double tuning, int octave, const int octaveShift, const Instrument* const instr)
 {
     //LOGD("xmlSetPitch(n=%p, step=%d, alter=%d, octave=%d, octaveShift=%d)",
     //       n, step, alter, octave, octaveShift);
@@ -289,6 +289,7 @@ static void xmlSetPitch(Note* n, int step, int alter, int octave, const int octa
     int tpc2 = step2tpc(step, AccidentalVal(alter));
     int tpc1 = mu::engraving::transposeTpc(tpc2, intval, true);
     n->setPitch(pitch, tpc1, tpc2);
+    n->setTuning(tuning);
     //LOGD("  pitch=%d tpc1=%d tpc2=%d", n->pitch(), n->tpc1(), n->tpc2());
 }

@@ -6401,10 +6402,10 @@ static void setPitch(Note* note, const MusicXMLInstruments& instruments, const S
             note->setTpc(pitch2tpc(unpitched, Key::C, Prefer::NEAREST));             // TODO: necessary ?
         } else {
             //LOGD("disp step %d oct %d", displayStep, displayOctave);
-            xmlSetPitch(note, mnp.displayStep(), 0, mnp.displayOctave(), 0, instrument);
+            xmlSetPitch(note, mnp.displayStep(), 0, 0.0, mnp.displayOctave(), 0, instrument);
         }
     } else {
-        xmlSetPitch(note, mnp.step(), mnp.alter(), mnp.octave(), octaveShift, instrument);
+        xmlSetPitch(note, mnp.step(), mnp.alter(), mnp.tuning(), mnp.octave(), octaveShift, instrument);
     }
 }

[Jojo-Schmitz]

Quite similar to mine (and mine is entirely untested). Feel free to PR it, I try to not interfere again ;-)

[miiizen]

Yep, this looks good @pacebes! If you make a PR I'll add 1 piece of feedback.

[pacebes]

HI

(Edited)

Just one think. As long as this new patch is about "import" instead of "export". Is it right to link the new PR to this issue (export) or should we create another issue or at least change the "title" of this issue ?

[miiizen]

Yes, as all discussion is here it's fine to link this one

[Jojo-Schmitz]

Your code does correctly import the tunings (I can see them being set in the properties), but they don't sound?!?

[pacebes]

HI

It does sound in my computer. There is a Video with the recording:

Imported.xml.with.tuning.mp4

The only difference (format) between the original and the exported-then-imported is the first E

• Original: the E is a "natural" E with a tuning of "-50"
• Exported-then-imported: the E has the accidental "reverse flat (quarter-tone flat) (Stein)" and a tuning of "0"

Both are equivalent.

[Jojo-Schmitz]

The one with the quarter tone accidental sounds quarter tone, the others just regular semitone

[Jojo-Schmitz]

No, you're right, they do sound.