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

Add a warning when tuplet.start_note.voice != tuplet.end_note.voice #423

Merged
merged 1 commit into from
Mar 6, 2025

Conversation

leleogere
Copy link
Collaborator

@leleogere leleogere commented Jan 28, 2025

As discussed in #420, the start and end notes of a tuplet are a good indicator of an XML encoding error. This happens when there is a missing tag <tuplet type="start"> or <tuplet type="stop">. For instance, it is the case in the score Chopin/Sonata_2/1st_no_repeat in ASAP (see fix at fosfrancesco/asap-dataset#14).

This PR adds a warning when this happens, providing information to the user that there might be a missing tuplet tag.

/home/user/partitura/partitura/io/importmusicxml.py:1577: UserWarning: Tuplet start and end notes do not belong to the same voice (5 != 6). This might indicate a missing <tuplet type="start"> or <tuplet type="stop">.
  warnings.warn(

MuseScore is able to fix this kind of encoding error, should we try too? Here is an overview of a naive approach presented in #420:

  • Find an additional <tuplet type="stop"> without pending <tuplet type="start"> registered.
  • In that case, iterate backward over previous notes of the same voice, as long as they have the same <time-> modification> as the "end" note, and they do not have any <tuplet> attribute.
    • Should we search only on the current measure? Tuplets crossing bar lines are extremely rare;
  • Set the start_note of the tuplet to the last note verifying the previous conditions.

In case of missing <tuplet type="stop">, the same could be applied but iterating forward from the <tuplet > type="start"> rather than backward. However, detecting missing stops might be a bit more difficult. Indeed, in the case of a missing start, you detect it right away when you encounter a stop without having found a start before, but for missing stops, you need some rules that say "the tuplet should be ended by now, there must be a stop missing".

However, there might be more complex cases that wouldn't be covered by this approach (I think of nested tuplets, but there might be other weird cases).


For reference, here is a related part of the code of MuseScore:
https://github.com/musescore/MuseScore/blob/559bf1eb46e467a000a64a36b13173f3e58e2a32/src/importexport/musicxml/internal/musicxml/import/musicxmltupletstate.cpp#L344-L366

    // Tuplets are stopped by the tuplet stop
    // or when the tuplet is filled completely
    // (either with knowledge of the normal type
    // or as a last resort calculated based on
    // actual and normal notes plus total duration)
    // or when the time-modification is not found.

    if (inTuplet) {
        if (tupletStartStop == MusicXmlStartStop::STOP
            || (implicit && isTupletFilled(normalType, timeMod))
            || (actNotes == 1 && norNotes == 1)) {           // incorrect ??? check scenario incomplete tuplet w/o start
            if (actNotes > norNotes && !isTupletFilled(normalType, timeMod)) {
                missingCurrentDuration = missingTupletDuration(duration);
                LOGD("current tuplet incomplete, missing %s", muPrintable(missingCurrentDuration.toString()));
            }

            *this = {};
            res |= MusicXmlTupletFlag::STOP_CURRENT;
        }
    }

    return res;
}

I'm not really used to C++, but it might be worth to have a look at what's happening in here.
One thing they seem to do (among others) it to keep track of the time in the tuplet, which might be a lot of work to include in Partitura if we choose that way.

Copy link
Member

@manoskary manoskary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR addresses potential errors in tuplets by raising a warning, it does not propose a solution or re-assignment but rather notifies the user about potentially problematic scores.

@manoskary manoskary merged commit 2e37725 into develop Mar 6, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tuplet parsing issue in measure 125 of Chopin/Sonata_2/1st_no_repeat
2 participants