Skip to content

Introduced MEI ornament expansions and their conversion to MPM ornamentation - #33

Open
LarsEngeln wants to merge 129 commits into
cemfi:masterfrom
LarsEngeln:dev-feature/ornaments
Open

Introduced MEI ornament expansions and their conversion to MPM ornamentation#33
LarsEngeln wants to merge 129 commits into
cemfi:masterfrom
LarsEngeln:dev-feature/ornaments

Conversation

@LarsEngeln

Copy link
Copy Markdown

v0.11.15

Introduced MEI ornament expansions and their conversion to MPM ornamentation.

  • adds Helper functions to create Element, find childs where Attribute has a certain value, find sibling, simple way to add a short UUID, remove all Attributes except given list, get halfsteps between notes, shift the given note diatonicly by a given number of semitones
  • ntroduces RichElement as object-oriented wrapper of the Helper functions, being the base class for MeiElement and MsmElement
  • adds the possibility to expand ornaments a MEI by adding notes to be played as . Mainly this is for rendering via MPM.
  • introduces an ornaments.dict(ionary) (congruent with the instruments.dict) that is used by the MeiInstructifier
  • extends ornamentData to match MPM's ornament model
  • adds possibility to not set a namespace for the id (important for MSM)
  • adds note generation for ornaments in ornamentMap-processing
  • adds TemporalValue as class that interfaces temporal domains to handle dates and durations
  • adds standard OrnamentDef's for basic ornaments
  • adds graces as likewise the other ornaments
  • adds the use of relative framestart and framelength for temporalSpread of ornaments
  • adds support for (measured) tremolo rendering
  • adds support for multiple ornaments applied to the same principal note with options to place (render) these at the start or at the end of the principal note
  • adds a guard that an exported midi files have no negative date occur, as this could result with an ornament at the very beginning, which may lead to a midi files that cannot be interpreted by other software
  • adds preventing leftovers notes if they are too short (e.g. due to numeric/rounding)
  • adds expanding ornaments into the export function, with flag to prevent this
  • prevents expanding ornaments multiple times
  • adds returning added notes while ornamentation rendering
  • adds articulations to generated notes of the rendered ornaments
  • applies ornament rendering to be fully "measured" (in ms) after Tempo-rendering
  • adds support for fioritura's (ornamental runs), in MEI these are displayed as besides a

LarsEngeln added 30 commits May 6, 2026 17:04
…has a certain value, find sibling, including a simple way to add a short UUID
…o be played as <supplied>. Mainly this is for rendering via MPM.
…ion of halfsteps between two note. "intm" is now in halfsteps, not interval anymore
@axelberndt

Copy link
Copy Markdown
Member
  • In class Mei2MsmMpmConverter lines 2985 and 3003: The ornament's ID should be the ID it has in the MEI.

@axelberndt

axelberndt commented Aug 11, 2026

Copy link
Copy Markdown
Member
  • For what is method getRptString() meant to be a helper method? Not for barlines, right? But for ornaments? Please clarify in the method description.

@axelberndt

Copy link
Copy Markdown
Member
  • There is a TODO comment in line 3082. Is it still valid?

@axelberndt

Copy link
Copy Markdown
Member
  • Line 3565: Is it necessary to add a rest to the hashmap? Please test what happens when a slur ends on a rest. Esp. is there an effect to the articulation of subsequent notes?

@axelberndt

axelberndt commented Aug 11, 2026

Copy link
Copy Markdown
Member
  • Rename method MeiNote2MsmNote() to meiNote2MsmNote(). All methohd names sould start lower case.

  • Lines 3832-3839: replace by

MsmElement msmElement = meiNote2MsmNote(new MeiElement(note));
if (msmElement == null)
    return;
Element s = msmElement.getElement();

Comment thread src/meico/mei/MeiElementHelper.java
Comment thread src/meico/mei/MeiElementHelper.java
Comment thread src/meico/mei/MeiElement.java Outdated
Comment thread src/meico/mei/MeiOrnamentExpander.java Outdated
Comment thread src/meico/mei/ornament/OrnamentExpander.java Outdated
Comment thread src/meico/xml/RichElement.java Outdated
Comment thread src/meico/xml/RichElement.java Outdated
Comment thread src/meico/xml/RichElement.java
Comment thread src/meico/msm/MsmNoteElement.java Outdated
Comment thread src/meico/msm/MsmNoteElement.java
@LarsEngeln
LarsEngeln requested a review from axelberndt August 13, 2026 13:14
…es only), and changes the OrnamentProcessor to only use MeiElementHelper (instead of Helper./XOM directly)
* @param chord an mei chord, bTrem or fTrem element
*/
private void processChord(Element chord) {
public void processChord(Element chord) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not public. If private is too strict use protected.

}
}

public MsmNoteElement meiNote2MsmNote(MeiElementHelper meiNote) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not public!
Missing documentationen above method header.

import java.util.ArrayList;
import java.util.HashMap;

public interface Context {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Missing documentation

Comment thread src/meico/mei/Mei.java

public class Mei extends meico.xml.XmlBase implements Cloneable {

boolean areOrnamentsExpanded = false;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Still, this should be removed.

Comment thread src/meico/mei/Mei.java
* @return 'this' Mei for chaining
*/
public Mei expandOrnaments() {
if(areOrnamentsExpanded)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Unnecessary. If the application wants this to be done twice, it should be allowed.

* This class interfaces temporal domain with a value
* @author Lars Engeln
*/
public class TemporalValue {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Lots of missing documentation in this class. What is the meaning of the class variables? Please also document all methods.

@@ -0,0 +1,549 @@
package meico.mpm.elements;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is the wrong place for this class. We need to discuss this.

return getDomain() == Domain.Ticks;
}

/** TODO

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

dead code?


double greaterValue = getValue();
double lesserValue = value;
if(getValue() < value) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Put the above two lines in an else, so the value assignments are done only once.

}
/**
* returns if the relation stack has an absolute root
*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copilot AI lite review requested due to automatic review settings August 20, 2026 12:04

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces an end-to-end pipeline for expanding MEI ornaments into explicit note material (<supplied>/<graceGrp>) and converting that information into MPM ornamentation data, including new temporal-domain handling for ornament timing/spread.

Changes:

  • Added MEI ornament expansion/preprocessing (meico.mei.ornament) plus conversion support into MPM ornamentationMap.
  • Introduced new XML helper abstractions (RichElement, MeiElementHelper, MsmNoteElement) and additional MEI helper utilities.
  • Extended ornamentation/temporal rendering capabilities (relative/ticks/ms domains, alignment “at start/end”, tremolo support, multi-ornament distribution).

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
src/resources/ornaments.dict Adds ornament name → pitch-alteration patterns used for ornament expansion/matching.
src/meico/xml/RichElement.java Introduces an OO wrapper around XOM Element with attribute/child/id utilities.
src/meico/supplementary/Stopwatch.java Adds a simple stopwatch utility for console timing output.
src/meico/msm/MsmNoteElement.java Adds MSM-focused RichElement wrapper utilities for note access/comparison.
src/meico/msm/Msm.java Extends expressive MIDI export API and clamps negative dates during MIDI event creation.
src/meico/mpm/elements/TemporalValue.java Adds temporal value abstraction with unit/domain parsing and relative/absolute operations.
src/meico/mpm/elements/styles/defs/OrnamentDef.java Extends ornament temporal spread model (TemporalValue, alignment) and render logic.
src/meico/mpm/elements/Performance.java Wires ornament expansion/rendering into performance pipeline; attempts map cloning.
src/meico/mpm/elements/maps/OrnamentationMap.java Major rework: supports child notes/repetitions, note generation, grouping/spacing logic.
src/meico/mpm/elements/maps/GenericMap.java Adds removeElement(String id) convenience removal by xml:id.
src/meico/mpm/elements/maps/data/OrnamentData.java Extends ornament data to include correspondence, notes, repetitions, and new apply API.
src/meico/mpm/elements/maps/ArticulationMap.java Adds articulation lookup/copy helper for ornament-generated notes.
src/meico/mei/ornament/package-info.java Adds package docs for new ornament subpackage.
src/meico/mei/ornament/OrnamentProcessor.java Implements MEI→MPM ornament extraction from generated <supplied> expansions.
src/meico/mei/ornament/OrnamentExpansion.java Defines the in-memory model for generated ornament expansions and merge behavior.
src/meico/mei/ornament/OrnamentExpander.java Adds MEI preprocessing step that generates <supplied> ornament expansions.
src/meico/mei/NoteProcessor.java Introduces interface to decouple note/chord processing for ornament processors.
src/meico/mei/MeiElementHelper.java Adds MEI-specific wrapper over RichElement (namespace + .ges fallback).
src/meico/mei/Mei2MsmMpmConverter.java Integrates ornament processor; exposes context/hooks for ornament conversion.
src/meico/mei/Mei.java Adds expandOrnaments() and export flag to optionally bypass ornament expansion.
src/meico/mei/Helper.java Adds multiple MEI XML utilities and pitch/interval helper functions.
src/meico/mei/Context.java Adds context interface used by ornament processing to access converter state.
src/meico/mei/AttributesWithIds.java Adjusts visibility (package-private).
README.md Updates contributors and adds CLI flag --ignore-ornaments; updates formatting/content.
MEI2MSM_Coverage_Documentation.md Updates documentation to reflect new ornament/grace/tremolo preprocessing behavior.
history.md Adds v0.11.15 changelog documenting the ornament/temporal spread feature set.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +496 to +502
Pattern pattern = Pattern.compile("^(\\d+)(ms|th|%|ticks|\\?)$"); // checks string if it is a valid value + unit string
Matcher m = pattern.matcher(valueDomain.trim());
if (m.matches()) {
setValue(Double.parseDouble(m.group(1)));
setDomain(fromDomainString(m.group(2)));
return;
}
Comment on lines +525 to +526
dateAttName = "ornament.milliseconds.date.offset";
durAttName = "ornament.milliseconds.duration";
Comment on lines +349 to +352
ornament.copyValue("date", principalNote);
ArrayList<MsmNoteElement> children = ornament.getChildrenAsMsmElements();
ArrayList<String> noteOrder = new ArrayList<>(Arrays.asList(ornament.get("note.order").replaceAll(":\\|:", ":| |:").split(" ")));
Map<Integer, Integer> repeats = new HashMap<>();
Comment on lines +66 to +67
this.notes = new ArrayList<>();
xml.getChildElements("ornamentNote").forEach(note -> { this.notes.add(note); });
Comment on lines +528 to +533
try {
ornamentationMap = OrnamentationMap.createOrnamentationMap((OrnamentationMap) mpmPart.getDated().getMap(Mpm.ORNAMENTATION_MAP)); // get ornamentationMap
}
catch (Exception e) {
String exc = e.getLocalizedMessage();
}
Comment on lines +435 to +437
String ornamFullName = getOrnamentFullName(ornament);
if(ornamFullName == null || ornamFullName.equals(""))
return; // if I am not yet supported
od.xmlId = correspId != null ? correspId : UUID.randomUUID().toString();
od.correspondence = elementId;
od.date = (Double) timingData.get(0);
od.ornamentDefName = segmentLabels.get(s);
Comment thread src/meico/mei/Helper.java
Comment on lines +1008 to +1016
/**
* returns halfsteps between pname1 and pname2 (always positive)
* @param pname1
* @param pname2
* @return
*/
public static int getHalfstepsBetween(String pname1, String pname2) {
return Math.abs(getHalfstepsFromC(pname2) - getHalfstepsFromC(pname1));
}
Comment thread src/meico/msm/Msm.java
Comment on lines 1261 to +1264
dateEnd = Math.round(Double.parseDouble(endAtt.getValue()));
if(dateEnd < 0) {
dateEnd = -(date-dateEnd);
}
Comment on lines +280 to 285
public Map<String, ArrayList<String>> renderGlobalOrnamentationMap(ArrayList<GenericMap> maps) {
if ((maps == null) || maps.isEmpty())
return;
return new HashMap<>();

this.apply(maps);
return new HashMap<>(); // this.apply(maps);
}
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.

3 participants