diff --git a/importexport/musicxml/importmxmlnoteduration.cpp b/importexport/musicxml/importmxmlnoteduration.cpp index f2d8cd2cd0c73..830a379443a12 100644 --- a/importexport/musicxml/importmxmlnoteduration.cpp +++ b/importexport/musicxml/importmxmlnoteduration.cpp @@ -102,30 +102,43 @@ QString mxmlNoteDuration::checkTiming(const QString& type, const bool rest, cons QString errorStr; // normalize duration - if (_dura.isValid()) - _dura.reduce(); + if (_specDura.isValid()) + _specDura.reduce(); - const auto calcDura = calculateFraction(type, _dots, _timeMod); - if (_dura.isValid() && calcDura.isValid()) { - if (_dura != calcDura) { + // default: use calculated duration + _calcDura = calculateFraction(type, _dots, _timeMod); + if (_calcDura.isValid()) { + _dura = _calcDura; + } + + if (_specDura.isValid() && _calcDura.isValid()) { + if (_specDura != _calcDura) { errorStr = QString("calculated duration (%1) not equal to specified duration (%2)") - .arg(calcDura.print(), _dura.print()); + .arg(_calcDura.print(), _specDura.print()); //qDebug("rest %d type '%s' timemod %s", rest, qPrintable(type), qPrintable(_timeMod.print())); - if (rest && type == "whole" && _dura.isValid()) { + if (rest && type == "whole" && _specDura.isValid()) { // Sibelius whole measure rest (not an error) errorStr = ""; + _dura = _specDura; } - else if (grace && _dura == Fraction(0, 1)) { + else if (grace && _specDura == Fraction(0, 1)) { // grace note (not an error) errorStr = ""; + _dura = _specDura; } else { - const int maxDiff = 3; // maximum difference considered a rounding error - if (qAbs(calcDura.ticks() - _dura.ticks()) <= maxDiff) { + // note: + // rounding error detection may conflict with changed duration detection (Overture). + // exporters that intentionally change note duration typically produce a large difference, + // most likely in practice this will not be an issue + const int maxDiff = 2; // maximum difference considered a rounding error + if (qAbs(_calcDura.ticks() - _specDura.ticks()) <= maxDiff) { errorStr += " -> assuming rounding error"; - _dura = calcDura; + _dura = _calcDura; } + else + errorStr += " -> using calculated duration"; } // Special case: @@ -134,22 +147,25 @@ QString mxmlNoteDuration::checkTiming(const QString& type, const bool rest, cons // based on note type. If actual is 2/3 of expected, the rest is part // of a tuplet. if (rest) { - if (2 * calcDura.ticks() == 3 * _dura.ticks()) { + if (2 * _calcDura.ticks() == 3 * _specDura.ticks()) { _timeMod = Fraction(2, 3); - errorStr += " -> assuming triplet"; + errorStr += errorStr.isEmpty() ? " ->" : ","; + errorStr += " assuming triplet"; + _dura = _specDura; } } } } - else if (_dura.isValid()) { + else if (_specDura.isValid() && !_calcDura.isValid()) { // do not report an error for typeless (whole measure) rests - if (!(rest && type == "")) + if (!(rest && type == "")) { errorStr = "calculated duration invalid, using specified duration"; + } + _dura = _specDura; } - else if (calcDura.isValid()) { + else if (!_specDura.isValid() && _calcDura.isValid()) { if (!grace) { errorStr = "specified duration invalid, using calculated duration"; - _dura = calcDura; // overrule dura } } else { @@ -173,19 +189,19 @@ void mxmlNoteDuration::duration(QXmlStreamReader& e) Q_ASSERT(e.isStartElement() && e.name() == "duration"); _logger->logDebugTrace("MusicXMLParserPass1::duration", &e); - _dura.set(0, 0); // invalid unless set correctly + _specDura.set(0, 0); // invalid unless set correctly int intDura = e.readElementText().toInt(); if (intDura > 0) { if (_divs > 0) { - _dura.set(intDura, 4 * _divs); - _dura.reduce(); // prevent overflow in later Fraction operations + _specDura.set(intDura, 4 * _divs); + _specDura.reduce(); // prevent overflow in later Fraction operations } else _logger->logError("illegal or uninitialized divisions", &e); } else _logger->logError("illegal duration", &e); - //qDebug("duration %s valid %d", qPrintable(dura.print()), dura.isValid()); + //qDebug("specified duration %s valid %d", qPrintable(_specDura.print()), _specDura.isValid()); } //--------------------------------------------------------- diff --git a/importexport/musicxml/importmxmlnoteduration.h b/importexport/musicxml/importmxmlnoteduration.h index c4d6e231b1fa7..22aa39227c002 100644 --- a/importexport/musicxml/importmxmlnoteduration.h +++ b/importexport/musicxml/importmxmlnoteduration.h @@ -33,7 +33,9 @@ class mxmlNoteDuration public: mxmlNoteDuration(int divs, MxmlLogger* logger) : _divs(divs), _logger(logger) { /* nothing so far */ } QString checkTiming(const QString& type, const bool rest, const bool grace); - Fraction dura() const { return _dura; } + Fraction duration() const { return _dura; } // duration to use + Fraction calculatedDuration() const { return _calcDura; } // value calculated from note type etcetera + Fraction specifiedDuration() const { return _specDura; } // value read from the duration element int dots() const { return _dots; } TDuration normalType() const { return _normalType; } bool readProperties(QXmlStreamReader& e); @@ -44,6 +46,8 @@ class mxmlNoteDuration void timeModification(QXmlStreamReader& e); const int _divs; // the current divisions value int _dots = 0; + Fraction _calcDura; + Fraction _specDura; Fraction _dura; TDuration _normalType; Fraction _timeMod { 1, 1 }; // default to no time modification diff --git a/importexport/musicxml/importmxmlpass1.cpp b/importexport/musicxml/importmxmlpass1.cpp index 2ecede25f8032..041e452d2012a 100644 --- a/importexport/musicxml/importmxmlpass1.cpp +++ b/importexport/musicxml/importmxmlpass1.cpp @@ -3343,7 +3343,7 @@ void MusicXMLParserPass1::note(const QString& partId, // check for timing error(s) and set dura // keep in this order as checkTiming() might change dura auto errorStr = mnd.checkTiming(type, bRest, grace); - dura = mnd.dura(); + dura = mnd.duration(); if (errorStr != "") _logger->logError(errorStr, &_e); @@ -3358,7 +3358,7 @@ void MusicXMLParserPass1::note(const QString& partId, // do tuplet auto timeMod = mnd.timeMod(); auto& tupletState = tupletStates[voice]; - tupletState.determineTupletAction(mnd.dura(), timeMod, tupletStartStop, mnd.normalType(), missingPrev, missingCurr); + tupletState.determineTupletAction(mnd.duration(), timeMod, tupletStartStop, mnd.normalType(), missingPrev, missingCurr); } // store result diff --git a/importexport/musicxml/importmxmlpass2.cpp b/importexport/musicxml/importmxmlpass2.cpp index b42ee4aa955cb..3934844a5bb9e 100644 --- a/importexport/musicxml/importmxmlpass2.cpp +++ b/importexport/musicxml/importmxmlpass2.cpp @@ -5450,7 +5450,7 @@ Note* MusicXMLParserPass2::note(const QString& partId, // check for timing error(s) and set dura // keep in this order as checkTiming() might change dura auto errorStr = mnd.checkTiming(type, rest, grace); - dura = mnd.dura(); + dura = mnd.duration(); if (errorStr != "") _logger->logError(errorStr, &_e); @@ -5481,7 +5481,7 @@ Note* MusicXMLParserPass2::note(const QString& partId, if (!chord && !grace) { auto& tuplet = tuplets[voice]; auto& tupletState = tupletStates[voice]; - tupletAction = tupletState.determineTupletAction(mnd.dura(), timeMod, notations.tupletDesc().type, mnd.normalType(), missingPrev, missingCurr); + tupletAction = tupletState.determineTupletAction(mnd.duration(), timeMod, notations.tupletDesc().type, mnd.normalType(), missingPrev, missingCurr); if (tupletAction & MxmlTupletFlag::STOP_PREVIOUS) { // tuplet start while already in tuplet if (missingPrev.isValid() && missingPrev > Fraction(0, 1)) { diff --git a/mtest/musicxml/io/testDurationLargeError.xml b/mtest/musicxml/io/testDurationLargeError.xml new file mode 100644 index 0000000000000..e5049d278da47 --- /dev/null +++ b/mtest/musicxml/io/testDurationLargeError.xml @@ -0,0 +1,106 @@ + + + + + MuseScore testfile + Duration Large Error + + + Leon Vinken + + MuseScore 0.7.0 + 2007-09-10 + + + + + + + + + Test handling of large errors in durations (larger than rounding errors). + Expect MuseScore to use the calculated duration, resulting in a normal + MusicXML file (with divsions 1 and duration 2 for the half notes). + + + + + + Voice + Vo. + + Voice + + + + 1 + 53 + 78.7402 + 0 + + + + + + + 100 + + 0 + + + + G + 2 + + + + + G + 4 + + 180 + 1 + half + up + + + + A + 4 + + 200 + 1 + half + up + + + + + + B + 4 + + 220 + 1 + half + down + + + + C + 5 + + 200 + 1 + half + down + + + light-heavy + + + + diff --git a/mtest/musicxml/io/testDurationLargeError_ref.xml b/mtest/musicxml/io/testDurationLargeError_ref.xml new file mode 100644 index 0000000000000..bfe22049ca186 --- /dev/null +++ b/mtest/musicxml/io/testDurationLargeError_ref.xml @@ -0,0 +1,99 @@ + + + + + MuseScore testfile + Duration Large Error + + + Leon Vinken + + MuseScore 0.7.0 + 2007-09-10 + + + + + + + + + + Voice + Vo. + + Voice + + + + 1 + 53 + 78.7402 + 0 + + + + + + + 1 + + 0 + + + + G + 2 + + + + + G + 4 + + 2 + 1 + half + up + + + + A + 4 + + 2 + 1 + half + up + + + + + + B + 4 + + 2 + 1 + half + down + + + + C + 5 + + 2 + 1 + half + down + + + light-heavy + + + + diff --git a/mtest/musicxml/io/tst_mxml_io.cpp b/mtest/musicxml/io/tst_mxml_io.cpp index e8f080577fa49..fb7eb03bc4fbf 100644 --- a/mtest/musicxml/io/tst_mxml_io.cpp +++ b/mtest/musicxml/io/tst_mxml_io.cpp @@ -105,6 +105,7 @@ private slots: void drumset2() { mxmlIoTest("testDrumset2"); } void dsalCoda() { mxmlImportTestRef("testDSalCoda"); } void dsalCodaMisplaced() { mxmlImportTestRef("testDSalCodaMisplaced"); } + void durationLargeError() { mxmlIoTestRef("testDurationLargeError"); } void durationRoundingError() { mxmlIoTestRef("testDurationRoundingError"); } void dynamics1() { mxmlIoTest("testDynamics1"); } void dynamics2() { mxmlIoTest("testDynamics2"); }