MusicXML: xml file causes crash due to failed assertion

• Apr 20, 2018 - 16:31
Reported version
3.0
Type
Functional
Severity
S2 - Critical
Status
closed
Project

Came up in https://musescore.org/en/node/271604, the xml file there, created by Sibelius directly, not via Dolet, (attached here too) crashes 2.1 and master (287f99d), 2.2.1 loads it without problems though.

Failed assertion:
Fatal: ASSERT: "e == 0 || e->isText()" in file .../MuseScore/libmscore/scoreElement.h, line 511 (.../MuseScore/libmscore/scoreElement.h:511, )

This is the toText() macro, so got to be happening either in mscore/musicxmlsupport.cpp, line 223, domError(), or in mscore/musicxmlsupport.cpp, line 250, domNotImplemented(), which are the only 2 places in the entire score what that macro is used.
Well not quite. it is the only places where that marco is used without a parameter, but as a method of Element?!?

Might have to do with lyrics on grace notes (which in not supported by MuseScore currently), as the 'famos last words just before the assertion are:
Debug: Info at line 14802 col 64: ignoring lyrics on grace notes (...\MuseScore\mscore\importmxmllogger.cpp:49, void Ms::log(Ms::MxmlLogger::Level, const QString&, const QXmlStreamReader*))

Attachment Size
TRAVIATA ACT 1 BRINDISI and on.xml 438.24 KB

Comments

The crash occurs in Timeline::tempo_meta() in mscore/timeline.cpp when casting an object that is known to be of type TempoText to type Text. Replacing the line

Text* text = toText(element);

with

TempoText* text = toTempoText(element);

will prevent the crash. But later, when autosave kicks in, another crash occurs when XmlWriter::tag(const QString& name, QVariant data) encounters a data object with a typeName() of "float". This crash is prevented by adding

else if (strcmp(type, "float") == 0) {
*this << "<" << name << ">";
*this << data.toFloat();
*this << "\n";
}

right before the final else in the function.

Whatever bug was causing the crash in 2.1 must have been fixed in 2.2. The casting macros are new in 3.0, and so is mscore/timeline.cpp, so it must not have been the same bug.

Not really sure about your 2nd change. (it crashes not only when autosave kicks in, but also on a regular save or save as BTW).
For one it might better be

                  else if (strcmp(type, "float") == 0)
                        *this << QString("<%1>%2</%1>\n").arg(name).arg(data.toFloat());

just one term and also having an end tag, but maybe it should get handled further up, together with QVariant::Double
Here that float is used for LyricsOddFontSize and LyricsEvenFontSize, so the better fix might be in mscore/importmxml.cpp to change from

            score->style().set(Sid::lyricsOddFontSize, fLyricSize);
            score->style().set(Sid::lyricsEvenFontSize, fLyricSize);

to

            score->style().set(Sid::lyricsOddFontSize, QVariant(fLyricSize));
            score->style().set(Sid::lyricsEvenFontSize, QVariant(fLyricSize));

It makes sense to rewrite it as you say. I had copied the code from the QVariant::Double case. The end tag somehow got lost when I pasted it here. Perhaps the cases that don't already use this format could be rewritten as well.

There is no such thing as a QVariant::Float, so I figured floats had to be handled in the default case for data.type().