diff --git a/src/include/mx/api/PartData.h b/src/include/mx/api/PartData.h index e0d69975..4a1e6624 100644 --- a/src/include/mx/api/PartData.h +++ b/src/include/mx/api/PartData.h @@ -28,7 +28,27 @@ struct MidiData { std::string virtualLibrary; std::string virtualName; + + // The name of the playback device or sound bank for this instrument — for example the name of a + // MIDI output device, or a sound or bank label. This is often left empty. std::string device; + + // An optional port (or cable) number, from 1 to 16, that says which output the device uses. It + // has no value when the file doesn't specify one, and most files leave it unset — you only need + // it when you are aiming at a particular MIDI port. You don't have to attach the device to this + // part's instrument yourself; the library keeps them together automatically when the music is + // written out. + std::optional devicePort; + + // Most users can ignore this; leave it unspecified. It only controls whether the saved file + // spells out the little link that ties this part's playback device to its instrument. unspecified + // (the default) does the sensible thing on its own: it leaves that link out, because each part + // here has a single instrument and so the link isn't needed. yes/no force it to be written or left + // out. You never have to set this when you are creating music — when the library reads a file that + // did spell the link out, it sets this for you, so that saving the file brings it back the same way + // it came in. + Bool writeDeviceId; + std::string name; // VALUE_UNSPECIFIED indicates absence of value @@ -56,13 +76,34 @@ struct MidiData bool isElevationSpecified; MidiData() - : virtualLibrary{}, virtualName{}, device{}, name{}, bank{VALUE_UNSPECIFIED}, channel{VALUE_UNSPECIFIED}, - program{VALUE_UNSPECIFIED}, unpitched{VALUE_UNSPECIFIED}, volume{0.0}, isVolumeSpecified{false}, pan{0.0}, - isPanSpecified{false}, elevation{0.0}, isElevationSpecified{false} + : virtualLibrary{}, virtualName{}, device{}, devicePort{}, writeDeviceId{Bool::unspecified}, name{}, + bank{VALUE_UNSPECIFIED}, channel{VALUE_UNSPECIFIED}, program{VALUE_UNSPECIFIED}, unpitched{VALUE_UNSPECIFIED}, + volume{0.0}, isVolumeSpecified{false}, pan{0.0}, isPanSpecified{false}, elevation{0.0}, + isElevationSpecified{false} { } }; +// Describes the instrument that a part is played on. In this library every part has exactly one +// instrument, and the name and playback settings you set here (the playback settings live in +// midiData) apply to the whole part, from the first measure to the last. +// +// MusicXML itself allows a single part to carry more than one instrument. The most common case is +// a drum kit or percussion part, where the bass drum, snare, ride cymbal, and so on are each +// treated as a separate instrument. You also see it when a part layers or doubles two sounds that +// play together — two sampled instruments sounding at once, or a single "Clarinet 1 & 2" line that +// really holds two separate instruments. +// +// For now this library brings in only the first instrument of such a part. Any additional +// instruments in the file are not available here and are not written back out. We're sorry for the +// limitation: we looked for a way to offer several instruments per part without making the ordinary +// single-instrument case — which covers the vast majority of music — harder to understand and use, +// and we didn't find one we were happy with, so it stays one instrument per part for the time +// being. +// +// One more thing worth knowing: MusicXML also lets a piece switch a part's instrument, or change +// its playback settings, partway through the music. This library does not offer those mid-piece +// changes. Whatever you set here is in effect for the entire part, start to finish. struct InstrumentData { std::string uniqueId; @@ -230,6 +271,8 @@ MXAPI_EQUALS_BEGIN(MidiData) MXAPI_EQUALS_MEMBER(virtualLibrary) MXAPI_EQUALS_MEMBER(virtualName) MXAPI_EQUALS_MEMBER(device) +MXAPI_EQUALS_MEMBER(devicePort) +MXAPI_EQUALS_MEMBER(writeDeviceId) MXAPI_EQUALS_MEMBER(name) MXAPI_EQUALS_MEMBER(bank) MXAPI_EQUALS_MEMBER(channel) diff --git a/src/private/mx/impl/PartReader.cpp b/src/private/mx/impl/PartReader.cpp index 27cc090e..748dc74b 100644 --- a/src/private/mx/impl/PartReader.cpp +++ b/src/private/mx/impl/PartReader.cpp @@ -299,6 +299,16 @@ void PartReader::parseMidiDeviceInstrumentGroup(const core::ScorePartMIDIGroup & if (grp.midiDevice().has_value()) { myOutPartData.instrumentData.midiData.device = grp.midiDevice()->value(); + + if (grp.midiDevice()->port().has_value()) + { + myOutPartData.instrumentData.midiData.devicePort = grp.midiDevice()->port()->value(); + } + + if (grp.midiDevice()->id().has_value()) + { + myOutPartData.instrumentData.midiData.writeDeviceId = api::Bool::yes; + } } if (grp.midiInstrument().has_value()) diff --git a/src/private/mx/impl/PartWriter.cpp b/src/private/mx/impl/PartWriter.cpp index 502918e9..cec80b88 100644 --- a/src/private/mx/impl/PartWriter.cpp +++ b/src/private/mx/impl/PartWriter.cpp @@ -177,10 +177,25 @@ core::ScorePart PartWriter::getScorePart() const core::MIDIInstrument midiInstrument{}; midiInstrument.setID(core::Token{myPartData.instrumentData.uniqueId}); - if (myPartData.instrumentData.midiData.device.size() > 0) + const auto &apiMidiData = myPartData.instrumentData.midiData; + if (apiMidiData.device.size() > 0 || apiMidiData.devicePort.has_value()) { addMidiElement = true; - midiDevice.setValue(myPartData.instrumentData.midiData.device); + midiDevice.setValue(apiMidiData.device); + + if (apiMidiData.devicePort.has_value()) + { + midiDevice.setPort(core::MIDI16{*apiMidiData.devicePort}); + } + + // The midi-device attaches to this part's instrument, the same instrument the + // midi-instrument below is written for. Emit that link only when the source stated it; + // for a single-instrument part it is otherwise implied. + if (apiMidiData.writeDeviceId == api::Bool::yes && myPartData.instrumentData.uniqueId.size() > 0) + { + midiDevice.setID(core::Token{myPartData.instrumentData.uniqueId}); + } + midiGroup.setMIDIDevice(midiDevice); } diff --git a/src/private/mxtest/api/MidiNameRoundTripTest.cpp b/src/private/mxtest/api/MidiNameRoundTripTest.cpp index d247bbcc..a33c0ca2 100644 --- a/src/private/mxtest/api/MidiNameRoundTripTest.cpp +++ b/src/private/mxtest/api/MidiNameRoundTripTest.cpp @@ -53,4 +53,33 @@ TEST(midiNameRoundTrip, survivesWriteAndRead) CHECK_EQUAL(expected, out.parts.at(0).instrumentData.midiData.name); } +TEST(midiDeviceRoundTrip, portSurvivesWriteAndRead) +{ + // A midi-device carrying only attributes (empty text) must still round-trip, e.g. + // . writeDeviceId defaults to unspecified, so no id is + // emitted here; only the port is exercised. + auto in = makeScoreWithMidiName("Flute Player One"); + in.parts.at(0).instrumentData.midiData.devicePort = 1; + + const auto out = mxtest::roundTrip(in); + REQUIRE(out.parts.size() == 1); + const auto &midiData = out.parts.at(0).instrumentData.midiData; + REQUIRE(midiData.devicePort.has_value()); + CHECK_EQUAL(1, *midiData.devicePort); +} + +TEST(midiDeviceRoundTrip, writeDeviceIdSurvivesWriteAndRead) +{ + // When a source spells out the device-to-instrument link, mx re-emits it as + // , taking the id from the part's instrument (uniqueId "P1-I1" + // here), so the flag survives the round-trip. + auto in = makeScoreWithMidiName("Flute Player One"); + in.parts.at(0).instrumentData.midiData.devicePort = 1; + in.parts.at(0).instrumentData.midiData.writeDeviceId = Bool::yes; + + const auto out = mxtest::roundTrip(in); + REQUIRE(out.parts.size() == 1); + CHECK(out.parts.at(0).instrumentData.midiData.writeDeviceId == Bool::yes); +} + #endif diff --git a/src/private/mxtest/api/roundtrip-baseline.txt b/src/private/mxtest/api/roundtrip-baseline.txt index f0c4d98c..662abba8 100644 --- a/src/private/mxtest/api/roundtrip-baseline.txt +++ b/src/private/mxtest/api/roundtrip-baseline.txt @@ -425,3 +425,10 @@ musuite/testTempo1.xml # the chord instead of absorbing the main chord's first note. lysuite/ly24b_ChordAsGraceNote.xml musuite/testGrace2.xml + +# Unblocked by MidiData::devicePort: the midi-device port attribute now round-trips +# (its id is written automatically from the part's instrument), and an attribute-only +# (empty-text) midi-device is no longer dropped. +custom/musescore-slur-start-stop.musicxml +musuite/testChord.xml +musuite/testPiano.xml