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

ERROR - Element beat-unit #22974

Open
Dima-S-Jr opened this issue May 25, 2024 · 15 comments
Open

ERROR - Element beat-unit #22974

Dima-S-Jr opened this issue May 25, 2024 · 15 comments
Labels
MusicXML P2 Priority: Medium

Comments

@Dima-S-Jr
Copy link

Dima-S-Jr commented May 25, 2024

Issue type

Import/export issue

Bug description

The mscz file I attached below is exported with errors in the code. Because of this, Dorico freezes when trying to play the score.

Steps to reproduce

MusicXML file error when importing to Dorico:

  1. Open the attached mscz file
  2. Export the file you opened to MusicXML
  3. Import the MusicXML file into Dorico
  4. A dialog box about the failed validation will appear.

Снимок экрана (368)

  1. Click on OK
  2. Click on Play
  3. Dorico freezes.
21-15-52_compressed.mp4

MusicXMLBug.zip

Screenshots/Screen recordings

  1. Dorico:
    Снимок экрана (368)

  2. MuseScore Studio 4:
    Снимок экрана (369)

MuseScore Version

MuseScore Studio version (64-bit): 4.3.0-241231433, revision: github-musescore-musescore-5f36e74

Regression

I don't know

Operating system

Windows 11

Additional context

I do not know if there has already been something like this on GitHub. This situation is happening so far only with the score I have attached.
Since I need to import a file to Dorico in a short time, I really ask the participants who understand issues of this kind to explain to me why this is happening. I will try to resort to some workarounds to avoid such errors.

@bkunda
Copy link

bkunda commented May 27, 2024

It seems like this issue is occurring at the export stage. We can investigate it, but unfortunately it's not something we can prioritise as urgent at the moment.

@bkunda bkunda added the P2 Priority: Medium label May 27, 2024
@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented May 27, 2024

Doesn't crash for me, same OS, same MuseScore version, so I can only reproduce steps 1-6

At step 6 (close score just imported), do you save or not? (Doesn't make a difference to me, neither crashes)

At step 2 (export to MusicXML): which options do you use? Asking because I get that import error on line 554 rather than 621, exporting with "Manually added system and page breaks only", your error shows when exporting with "All layout" (but it still doesn't crash for me then)

Culprit XML:

        <direction-type>
          <metronome parentheses="no" default-x="-37.68" default-y="33.73" relative-y="20.00">
            <beat-unit></beat-unit>
            <per-minute>160</per-minute>
            </metronome>
          </direction-type>

Mu3 doesn't have that export issue BTW.

Changing that <beat-unit></beat-unit> to <beat-unit>quarter</beat-unit> fixes that error message on import (and is what Mu3 puts there).
Whether it fixes the crash is nothing I can say, as I can't reproduce that

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented May 27, 2024

Code got to be

//---------------------------------------------------------
// beatUnit
//---------------------------------------------------------
static void beatUnit(XmlWriter& xml, const TDuration dur)
{
int dots = dur.dots();
xml.tag("beat-unit", TConv::toXml(dur.type()));
while (dots > 0) {
xml.tag("beat-unit-dot");
--dots;
}
}

and this:
int len1 = 0;
TDuration dur;
TempoText::findTempoDuration(metroLeft, len1, dur);
beatUnit(xml, dur);
if (TempoText::findTempoDuration(metroRight, len1, dur) != -1) {
beatUnit(xml, dur);
} else {
xml.tag("per-minute", metroRight);
}

both is pretty old code, no recent changes

@Jojo-Schmitz
Copy link
Contributor

This might fix it:

diff --git a/src/importexport/musicxml/internal/musicxml/exportxml.cpp b/src/importexport/musicxml/internal/musicxml/exportxml.cpp
index 020f5ca17e..75d4ca9375 100644
--- a/src/importexport/musicxml/internal/musicxml/exportxml.cpp
+++ b/src/importexport/musicxml/internal/musicxml/exportxml.cpp
@@ -4836,6 +4836,9 @@ static bool findMetronome(const std::list<TextFragment>& list,

 static void beatUnit(XmlWriter& xml, const TDuration dur)
 {
+    IF_ASSERT_FAILED(dur.isValid()) {
+        return;
+    }
     int dots = dur.dots();
     xml.tag("beat-unit", TConv::toXml(dur.type()));
     while (dots > 0) {

@Jojo-Schmitz
Copy link
Contributor

As might:

diff --git a/src/importexport/musicxml/internal/musicxml/exportxml.cpp b/src/importexport/musicxml/internal/musicxml/exportxml.cpp
index 020f5ca17e..75d4ca9375 100644
--- a/src/importexport/musicxml/internal/musicxml/exportxml.cpp
+++ b/src/importexport/musicxml/internal/musicxml/exportxml.cpp
@@ -4880,8 +4883,6 @@ static void wordsMetronome(XmlWriter& xml, const MStyle& s, TextBase const* cons
         xml.startElementRaw(tagName);
         int len1 = 0;
         TDuration dur;
-        TempoText::findTempoDuration(metroLeft, len1, dur);
-        beatUnit(xml, dur);

         if (TempoText::findTempoDuration(metroRight, len1, dur) != -1) {
             beatUnit(xml, dur);

@Jojo-Schmitz
Copy link
Contributor

@miiizen, @rettinghaus, @lvinken?

@Dima-S-Jr
Copy link
Author

Doesn't crash for me, same OS, same MuseScore version, so I can only reproduce steps 1-6

At step 6 (close score just imported), do you save or not? (Doesn't make a difference to me, neither crashes)

@Jojo-Schmitz, I just noticed the following. After the first restart of the system, I no longer observed such a crash. It's very strange. But I will change the issue description accordingly. Thanks for the comment.

At step 2 (export to MusicXML): which options do you use? Asking because I get that import error on line 554 rather than 621, exporting with "Manually added system and page breaks only", your error shows when exporting with "All layout" (but it still doesn't crash for me then)

Снимок экрана (371)
No matter what type of file is being exported (compressed or uncompressed). Errors occur in both.

Changing that <beat-unit></beat-unit> to <beat-unit>quarter</beat-unit> fixes that error message on import (and is what Mu3 puts there).

And what can I do for my part to avoid such errors when exporting? What workarounds could you suggest?

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented May 27, 2024

Short of manually correcting the XML or just ignoring that error there is no workaround

@rettinghaus
Copy link
Contributor

@Jojo-Schmitz Your proposed solution would produce invalid MusicXML because once there is a metronome element there has to be a beat-unit as well. I'll play around with this a bit.

@Jojo-Schmitz
Copy link
Contributor

I see. But code that potentially writes beat-unit twice (or once empty) seems wrong too

@Dima-S-Jr Dima-S-Jr changed the title "ERROR - Element beat-unit" and crash ERROR - Element beat-unit May 27, 2024
@lvinken
Copy link
Contributor

lvinken commented May 28, 2024

Had a quick look. Using current master (commit 77c1c94) no MusicXML metronome element is generated when exporting MusicXMLBug.mscz. I don't have 4.3 available and no time to investigate further right now.

@rettinghaus
Copy link
Contributor

rettinghaus commented May 28, 2024

The root of the problem is that there is a No-Break Space (U+00A0) in the tempo marking, which throws the metronome detection completely off track. If you put a normal space in there it works just fine. So it should be quite easy to fix?

@Dima-S-Jr
Copy link
Author

The root of the problem is that there is a No-Break Space (U+00A0) in the tempo marking, which throws the metronome detection completely off track. If you put a normal space in there it works just fine. So it should be quite easy to fix?

@rettinghaus, thank you so much! Indeed, it helped. I can't even imagine where the non-breaking space could have come from (provided that I enter the tempo from the keyboard and always use a regular space).
Thank you very much again)

@Jojo-Schmitz
Copy link
Contributor

So No-Break Space should get added to the regular expression detecting tempo texts. Apparently Mu3 did better on this.

@rettinghaus
Copy link
Contributor

@miiizen could you please take over?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
MusicXML P2 Priority: Medium
Projects
Status: In the further future
Development

No branches or pull requests

6 participants