From 27c4a7487697db9b1027cbb2202c0a12ce66412b Mon Sep 17 00:00:00 2001 From: Manolo Carrasco Date: Sun, 2 Aug 2026 19:09:55 +0200 Subject: [PATCH 1/5] mpe: make the articulation string utils public Move articulationstringutils.h out of internal/ so other modules can reuse the articulation name table. --- framework/mpe/CMakeLists.txt | 2 +- framework/mpe/{internal => }/articulationstringutils.h | 4 ++-- framework/mpe/internal/articulationprofilesrepository.cpp | 2 +- framework/mpe/qml/Muse/Mpe/articulationpatternitem.cpp | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) rename framework/mpe/{internal => }/articulationstringutils.h (98%) diff --git a/framework/mpe/CMakeLists.txt b/framework/mpe/CMakeLists.txt index 58a5eae3df..0800073be8 100644 --- a/framework/mpe/CMakeLists.txt +++ b/framework/mpe/CMakeLists.txt @@ -30,7 +30,7 @@ target_sources(muse_mpe PRIVATE playbacksetupdata.h iarticulationprofilesrepository.h - internal/articulationstringutils.h + articulationstringutils.h internal/articulationprofilesrepository.cpp internal/articulationprofilesrepository.h ) diff --git a/framework/mpe/internal/articulationstringutils.h b/framework/mpe/articulationstringutils.h similarity index 98% rename from framework/mpe/internal/articulationstringutils.h rename to framework/mpe/articulationstringutils.h index 29747bc398..ca480372ee 100644 --- a/framework/mpe/internal/articulationstringutils.h +++ b/framework/mpe/articulationstringutils.h @@ -28,7 +28,7 @@ #include "mpe/mpetypes.h" namespace muse::mpe { -static const std::unordered_map ARTICULATION_FAMILY_NAMES { +inline const std::unordered_map ARTICULATION_FAMILY_NAMES { { ArticulationFamily::Undefined, "Undefined" }, { ArticulationFamily::Keyboards, "KeyboardsArticulation" }, { ArticulationFamily::Strings, "StringsArticulation" }, @@ -37,7 +37,7 @@ static const std::unordered_map ARTICULATION_FAMILY { ArticulationFamily::Voices, "VoicesArticulation" }, }; -static const std::unordered_map ARTICULATION_TYPE_NAMES { +inline const std::unordered_map ARTICULATION_TYPE_NAMES { { ArticulationType::Undefined, "Undefined" }, { ArticulationType::Standard, "Standard" }, { ArticulationType::Staccato, "Staccato" }, diff --git a/framework/mpe/internal/articulationprofilesrepository.cpp b/framework/mpe/internal/articulationprofilesrepository.cpp index dab97c5a4f..dbd9a18497 100644 --- a/framework/mpe/internal/articulationprofilesrepository.cpp +++ b/framework/mpe/internal/articulationprofilesrepository.cpp @@ -29,7 +29,7 @@ #include "log.h" -#include "internal/articulationstringutils.h" +#include "articulationstringutils.h" using namespace muse; using namespace muse::mpe; diff --git a/framework/mpe/qml/Muse/Mpe/articulationpatternitem.cpp b/framework/mpe/qml/Muse/Mpe/articulationpatternitem.cpp index f86ea29ce7..6aefd95b56 100644 --- a/framework/mpe/qml/Muse/Mpe/articulationpatternitem.cpp +++ b/framework/mpe/qml/Muse/Mpe/articulationpatternitem.cpp @@ -24,7 +24,7 @@ #include -#include "internal/articulationstringutils.h" +#include "articulationstringutils.h" using namespace muse::mpe; From 832d70c6591b1814d1dc25fa73248e8c8fad4289 Mon Sep 17 00:00:00 2001 From: Manolo Carrasco Date: Sun, 2 Aug 2026 20:10:08 +0200 Subject: [PATCH 2/5] vst: translate note articulations to VST3 keyswitches Query the plugin's IKeyswitchController and, when it advertises keyswitches, send the articulation as a keyswitch note: latched per articulation, re-sent to retrigger a span (tremolo), and forwarded as a range span for ranged modifiers (legato). Adds unit tests for the sequencer. --- framework/vst/CMakeLists.txt | 4 + framework/vst/internal/synth/vstsequencer.cpp | 132 +++++++- framework/vst/internal/synth/vstsequencer.h | 27 +- .../vst/internal/synth/vstsynthesiser.cpp | 59 +++- framework/vst/tests/CMakeLists.txt | 34 ++ framework/vst/tests/vstsequencertest.cpp | 320 ++++++++++++++++++ 6 files changed, 570 insertions(+), 6 deletions(-) create mode 100644 framework/vst/tests/CMakeLists.txt create mode 100644 framework/vst/tests/vstsequencertest.cpp diff --git a/framework/vst/CMakeLists.txt b/framework/vst/CMakeLists.txt index 28cb43128a..93d00c9a16 100644 --- a/framework/vst/CMakeLists.txt +++ b/framework/vst/CMakeLists.txt @@ -94,3 +94,7 @@ target_no_warning(muse_vst -Wno-deprecated-declarations) if (MUSE_MODULE_VST_QML) add_subdirectory(qml/Muse/Vst) endif() + +if (MUSE_MODULE_VST_TESTS) + add_subdirectory(tests) +endif() diff --git a/framework/vst/internal/synth/vstsequencer.cpp b/framework/vst/internal/synth/vstsequencer.cpp index 48555b3e6c..4ccf90588d 100644 --- a/framework/vst/internal/synth/vstsequencer.cpp +++ b/framework/vst/internal/synth/vstsequencer.cpp @@ -46,10 +46,31 @@ static const mpe::ArticulationTypeSet BEND_SUPPORTED_TYPES { mpe::ArticulationType::Multibend, mpe::ArticulationType::ContinuousGlissando, }; -void VstSequencer::init(ParamsMapping&& mapping, bool useDynamicEvents) +// "Span" articulations expand into many rapid sub-notes sharing one meta.timestamp; an instrument +// may render them as a single sustained gesture. A new span re-sends the same keyswitch so the +// instrument can retrigger. Other articulations are one note per keyswitch and never retrigger. +static const mpe::ArticulationTypeSet SPAN_ARTICULATION_TYPES { + mpe::ArticulationType::Tremolo8th, mpe::ArticulationType::Tremolo16th, + mpe::ArticulationType::Tremolo32nd, mpe::ArticulationType::Tremolo64th, +}; + +static bool isSpanArticulation(mpe::ArticulationType type) +{ + return muse::contains(SPAN_ARTICULATION_TYPES, type); +} + +// Keyswitch note the profile assigns to this articulation, or nullopt if it maps none. +static std::optional keyswitchFor(const VstKeyswitchProfile& profile, mpe::ArticulationType type) +{ + auto it = profile.keyswitches.find(type); + return it != profile.keyswitches.cend() ? std::optional(it->second) : std::nullopt; +} + +void VstSequencer::init(ParamsMapping&& mapping, bool useDynamicEvents, std::optional keyswitchProfile) { m_mapping = std::move(mapping); m_useDynamicEvents = useDynamicEvents; + m_keyswitchProfile = std::move(keyswitchProfile); m_inited = true; updateMainStreamEvents(m_playbackData.originEvents, m_playbackData.dynamics); @@ -111,11 +132,12 @@ muse::audio::gain_t VstSequencer::currentGain() const void VstSequencer::addPlaybackEvents(EventSequenceMap& destination, const mpe::PlaybackEventsMap& events) { SostenutoTimeAndDurations sostenutoTimeAndDurations; + LastKeyswitchPerTimestamp lastKeyswitch; for (const auto& evPair : events) { for (const mpe::PlaybackEvent& event : evPair.second) { if (std::holds_alternative(event)) { - addNoteEvent(destination, std::get(event), sostenutoTimeAndDurations); + addNoteEvent(destination, std::get(event), sostenutoTimeAndDurations, lastKeyswitch); } else if (std::holds_alternative(event)) { addControlChangeEvent(destination, evPair.first, std::get(event)); } @@ -160,7 +182,8 @@ void VstSequencer::addDynamicEvents(EventSequenceMap& destination, const mpe::Dy } void VstSequencer::addNoteEvent(EventSequenceMap& destination, const mpe::NoteEvent& noteEvent, - SostenutoTimeAndDurations& sostenutoTimeAndDurations) + SostenutoTimeAndDurations& sostenutoTimeAndDurations, + LastKeyswitchPerTimestamp& lastKeyswitch) { const mpe::ArrangementContext& arrangementCtx = noteEvent.arrangementCtx(); const int32_t noteId = noteIndex(noteEvent.pitchCtx().nominalPitchLevel); @@ -180,6 +203,57 @@ void VstSequencer::addNoteEvent(EventSequenceMap& destination, const mpe::NoteEv destination[timestampTo].emplace_back(buildEvent(VstEvent::kNoteOffEvent, noteId, velocityFraction, tuning)); } + // Latching keyswitch with span retrigger: send a keyswitch NoteOn when the articulation changes, + // or when a span articulation starts a new span. The keyswitch persists until a different one is + // sent (no NoteOff); re-sending the same one signals a span retrigger. + if (m_keyswitchProfile.has_value() && arrangementCtx.hasStart()) { + const VstKeyswitchProfile& profile = *m_keyswitchProfile; + // Default to the Standard keyswitch, or to nothing when the instrument does not advertise it: + // never fabricate a pitch the instrument did not advertise. + std::optional keyswitchPitch = keyswitchFor(profile, mpe::ArticulationType::Standard); + mpe::timestamp_t spanStart = -1; + + // Pick by precedence: a specific timbre (pizzicato, mute, harmonic, staccato...) beats a + // tremolo, which beats normal. On a tie between two primary timbres on one note, take the + // lowest keyswitch note, so the choice is deterministic and not the hash map's iteration order. + int bestRank = -1; + for (const auto& artPair : noteEvent.expressionCtx().articulations) { + if (mpe::isRangedArticulation(artPair.first)) { + continue; // forwarded as its own span below, never the primary keyswitch + } + const std::optional mapped = keyswitchFor(profile, artPair.first); + if (!mapped.has_value()) { + continue; + } + const bool span = isSpanArticulation(artPair.first); + const bool standard = (artPair.first == mpe::ArticulationType::Standard); + const int rank = standard ? 0 : (span ? 1 : 2); + if (rank > bestRank || (rank == bestRank && mapped.value() < keyswitchPitch.value())) { + bestRank = rank; + keyswitchPitch = mapped; + spanStart = span ? artPair.second.meta.timestamp : -1; + } + } + + // upper_bound, not lower_bound: std::prev then yields the most recent state at or before this + // timestamp even when an earlier note of the same chord already inserted an entry at it, so a + // chord sharing one articulation does not re-emit a duplicate keyswitch per note. + auto it = lastKeyswitch.upper_bound(arrangementCtx.actualTimestamp); + LastKeyswitchState prevState; + if (it != lastKeyswitch.begin()) { + prevState = std::prev(it)->second; + } + + if (keyswitchPitch.has_value() + && (keyswitchPitch.value() != prevState.keyswitch || (spanStart != -1 && spanStart != prevState.spanStart))) { + // Full velocity: a keyswitch is a control signal, not a note. A soft dynamic rounds to + // velocity 0, which a receiver reads as a note-off and drops, losing the switch. + destination[arrangementCtx.actualTimestamp].emplace_back( + buildEvent(VstEvent::kNoteOnEvent, keyswitchPitch.value(), 1.f, 0.f)); + lastKeyswitch[arrangementCtx.actualTimestamp] = { keyswitchPitch.value(), spanStart }; + } + } + for (const auto& artPair : noteEvent.expressionCtx().articulations) { if (artPair.first == mpe::ArticulationType::Standard) { continue; @@ -202,6 +276,58 @@ void VstSequencer::addNoteEvent(EventSequenceMap& destination, const mpe::NoteEv sostenutoTimeAndDurations.push_back(mpe::TimestampAndDuration { timestamp, meta.overallDuration }); continue; } + + // A ranged articulation the instrument advertises as a keyswitch is forwarded as a keyswitch + // spanning the range: pressed at the start, released at the end. The instrument decides what + // the modifier means; the sequencer only reports when it is active. + if (m_keyswitchProfile.has_value() && mpe::isRangedArticulation(meta.type)) { + if (const std::optional pitch = keyswitchFor(*m_keyswitchProfile, meta.type)) { + addKeyswitchSpanEvent(destination, meta, arrangementCtx.actualTimestamp, *pitch); + } + continue; + } + } +} + +void VstSequencer::addKeyswitchSpanEvent(EventSequenceMap& destination, const mpe::ArticulationMeta& meta, + const mpe::timestamp_t noteTimestamp, int keyswitchPitch) +{ + // Press at every covered note's onset, not once at the range start, so starting playback partway + // through the range still engages it. Dedup within one onset (a chord shares it), like the + // release below, so we do not queue N identical note-ons at one instant. Full velocity (a + // keyswitch is a control signal, see above). + EventSequence& onset = destination[noteTimestamp]; + bool alreadyPressed = false; + for (const EventType& queued : onset) { + if (std::holds_alternative(queued)) { + const VstEvent& ev = std::get(queued); + if (ev.type == VstEvent::kNoteOnEvent && ev.noteOn.pitch == keyswitchPitch) { + alreadyPressed = true; + break; + } + } + } + if (!alreadyPressed) { + onset.emplace_back(buildEvent(VstEvent::kNoteOnEvent, keyswitchPitch, 1.f, 0.f)); + } + + if (meta.hasEnd()) { + // Dedup the release: every covered note computes the same range-end, so without this we queue + // N identical note-offs at one instant and can overflow the host block's fixed-size event list. + EventSequence& bucket = destination[meta.timestamp + meta.overallDuration]; + bool alreadyReleased = false; + for (const EventType& queued : bucket) { + if (std::holds_alternative(queued)) { + const VstEvent& ev = std::get(queued); + if (ev.type == VstEvent::kNoteOffEvent && ev.noteOff.pitch == keyswitchPitch) { + alreadyReleased = true; + break; + } + } + } + if (!alreadyReleased) { + bucket.emplace_back(buildEvent(VstEvent::kNoteOffEvent, keyswitchPitch, 1.f, 0.f)); + } } } diff --git a/framework/vst/internal/synth/vstsequencer.h b/framework/vst/internal/synth/vstsequencer.h index 8e979306ea..d0aee8824d 100644 --- a/framework/vst/internal/synth/vstsequencer.h +++ b/framework/vst/internal/synth/vstsequencer.h @@ -22,15 +22,26 @@ #pragma once +#include +#include + #include "audio/engine/internal/abstracteventsequencer.h" #include "vsttypes.h" +#include "mpe/mpetypes.h" namespace muse::vst { +// How a VST instrument wants each articulation delivered as a keyswitch note. Queried from the +// plugin via the VST3 IKeyswitchController interface. +struct VstKeyswitchProfile { + // Articulation -> keyswitch note. A note with no mapped articulation uses the Standard entry, or 0. + std::map keyswitches; +}; + class VstSequencer : public audio::engine::AbstractEventSequencer { public: - void init(ParamsMapping&& mapping, bool useDynamicEvents); + void init(ParamsMapping&& mapping, bool useDynamicEvents, std::optional keyswitchProfile = std::nullopt); muse::audio::gain_t currentGain() const; @@ -40,10 +51,21 @@ class VstSequencer : public audio::engine::AbstractEventSequencer; + // Last keyswitch sent, plus the start of the last span articulation, to latch keyswitches while + // retriggering a span (tremolo) when a new span begins. + struct LastKeyswitchState { + int keyswitch = -1; + mpe::timestamp_t spanStart = -1; + }; + using LastKeyswitchPerTimestamp = std::map; + void addPlaybackEvents(EventSequenceMap& destination, const mpe::PlaybackEventsMap& events); void addDynamicEvents(EventSequenceMap& destination, const mpe::DynamicAutomationLayers& layers); - void addNoteEvent(EventSequenceMap& destination, const mpe::NoteEvent& noteEvent, SostenutoTimeAndDurations& sostenutoTimeAndDurations); + void addNoteEvent(EventSequenceMap& destination, const mpe::NoteEvent& noteEvent, SostenutoTimeAndDurations& sostenutoTimeAndDurations, + LastKeyswitchPerTimestamp& lastKeyswitch); void addPedalEvent(EventSequenceMap& destination, const mpe::ArticulationMeta& meta); + void addKeyswitchSpanEvent(EventSequenceMap& destination, const mpe::ArticulationMeta& meta, const mpe::timestamp_t noteTimestamp, + int keyswitchPitch); void addControlChangeEvent(EventSequenceMap& destination, const mpe::timestamp_t timestamp, const mpe::ControllerChangeEvent& event); void addParamChange(EventSequenceMap& destination, const mpe::timestamp_t timestamp, const ControlIdx controlIdx, const PluginParamValue value); @@ -64,5 +86,6 @@ class VstSequencer : public audio::engine::AbstractEventSequencer m_keyswitchProfile; }; } diff --git a/framework/vst/internal/synth/vstsynthesiser.cpp b/framework/vst/internal/synth/vstsynthesiser.cpp index 79bc46b513..18be13baf7 100644 --- a/framework/vst/internal/synth/vstsynthesiser.cpp +++ b/framework/vst/internal/synth/vstsynthesiser.cpp @@ -21,6 +21,13 @@ */ #include "vstsynthesiser.h" +#include + +#include "mpe/articulationstringutils.h" + +#include "pluginterfaces/vst/ivstnoteexpression.h" +#include "public.sdk/source/vst/utility/stringconvert.h" + #include "log.h" using namespace muse; @@ -37,6 +44,54 @@ static const std::set SUPPORTED_CONTROLLERS = { Steinberg::Vst::kPitchBend, }; +// Query the plugin's IKeyswitchController and build a keyswitch profile. Each keyswitch is advertised +// with the exact mpe::ArticulationType name, so titles convert straight to the type via MuseScore's +// own table; the host does no fuzzy matching. Only keyswitchMin on bus 0 / channel 0 is read: min +// alone selects the articulation. +static std::optional queryKeyswitchProfile(const PluginControllerPtr& controller) +{ + using namespace Steinberg; + using namespace Steinberg::Vst; + + if (!controller) { + return std::nullopt; + } + + FUnknownPtr keyswitchCtrl(controller); + if (!keyswitchCtrl) { + return std::nullopt; // the plugin does not support keyswitches + } + + const int32 busIndex = 0; + const int16 channel = 0; + const int32 count = keyswitchCtrl->getKeyswitchCount(busIndex, channel); + if (count <= 0) { + return std::nullopt; + } + + VstKeyswitchProfile profile; + + for (int32 i = 0; i < count; ++i) { + KeyswitchInfo info; + if (keyswitchCtrl->getKeyswitchInfo(busIndex, channel, i, info) != kResultTrue) { + continue; + } + + // The title is the canonical articulation name; unknown titles map to Undefined and are skipped. + const std::string title = VST3::StringConvert::convert(info.title); + const mpe::ArticulationType type = mpe::articulationTypeFromString(QString::fromStdString(title)); + if (type != mpe::ArticulationType::Undefined) { + profile.keyswitches[type] = info.keyswitchMin; + } + } + + if (profile.keyswitches.empty()) { + return std::nullopt; + } + + return profile; +} + VstSynthesiser::VstSynthesiser(const TrackId trackId, const muse::audio::AudioInputParams& params) : AbstractSynthesizer(params), m_vstAudioClient(std::make_unique()), @@ -65,7 +120,9 @@ void VstSynthesiser::init(const OutputSpec& spec) m_pluginPtr->updatePluginConfig(m_params.configuration); m_vstAudioClient->setOutputSpec(m_outputSpec); m_vstAudioClient->loadSupportedParams(); - m_sequencer.init(m_vstAudioClient->paramsMapping(SUPPORTED_CONTROLLERS), m_useDynamicEvents); + + const std::optional keyswitchProfile = queryKeyswitchProfile(m_pluginPtr->controller()); + m_sequencer.init(m_vstAudioClient->paramsMapping(SUPPORTED_CONTROLLERS), m_useDynamicEvents, keyswitchProfile); m_inited = true; }; diff --git a/framework/vst/tests/CMakeLists.txt b/framework/vst/tests/CMakeLists.txt new file mode 100644 index 0000000000..b7b232764e --- /dev/null +++ b/framework/vst/tests/CMakeLists.txt @@ -0,0 +1,34 @@ +# SPDX-License-Identifier: GPL-3.0-only +# MuseScore-CLA-applies +# +# MuseScore Studio +# Music Composition & Notation +# +# Copyright (C) 2025 MuseScore Limited and others +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 3 as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +set(MODULE_TEST muse_vst_tests) + +# The vst headers include their siblings unprefixed (vstsequencer.h -> "vsttypes.h"), so the module +# directory must be on the include path, as it is when the module itself is built. +set(MODULE_TEST_INCLUDE ${CMAKE_CURRENT_LIST_DIR}/..) + +set(MODULE_TEST_SRC + ${CMAKE_CURRENT_LIST_DIR}/vstsequencertest.cpp +) + +# muse_vst pulls in its QML view (VstViewDialog), so the test links the UI stack it needs. +set(MODULE_TEST_LINK muse_vst muse_audio muse_uicomponents muse_ui muse_ui_qml muse_mpe vst3sdk Qt::Quick Qt::Widgets) + +include(SetupGTest) diff --git a/framework/vst/tests/vstsequencertest.cpp b/framework/vst/tests/vstsequencertest.cpp new file mode 100644 index 0000000000..88c003ab38 --- /dev/null +++ b/framework/vst/tests/vstsequencertest.cpp @@ -0,0 +1,320 @@ +/* + * SPDX-License-Identifier: GPL-3.0-only + * MuseScore-CLA-applies + * + * MuseScore Studio + * Music Composition & Notation + * + * Copyright (C) 2025 MuseScore Limited and others + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include + +#include +#include + +#include "mpe/events.h" +#include "mpe/tests/utils/articulationutils.h" + +#include "vst/vsttypes.h" +#include "vst/internal/synth/vstsequencer.h" + +using namespace muse; +using namespace muse::mpe; +using namespace muse::mpe::tests; +using namespace muse::vst; + +namespace { +// A plain articulation pattern segment (no arrangement/pitch/dynamic modification), enough for the +// NoteEvent constructor to build its curves. Reused for every articulation in these tests. +ArticulationPattern standardScope() +{ + ArticulationPatternSegment seg; + seg.arrangementPattern = createArrangementPattern(HUNDRED_PERCENT /*durationFactor*/, 0 /*timestampOffset*/); + seg.pitchPattern = createSimplePitchPattern(0); + seg.expressionPattern = createSimpleExpressionPattern(dynamicLevelFromType(DynamicType::Natural)); + + ArticulationPattern scope; + scope.emplace(0, seg); + return scope; +} + +struct Artic { + ArticulationType type; + timestamp_t timestamp; // the articulation range start (== the note onset for a span start) + duration_t overallDuration; // the articulation range length (a slur may span several notes) +}; + +// Build a note carrying a set of articulations, each with its own range meta. +NoteEvent makeNote(timestamp_t noteTimestamp, duration_t noteDuration, const std::vector& artics) +{ + ArticulationMap map; + for (const Artic& a : artics) { + ArticulationMeta meta; + meta.type = a.type; + meta.pattern = standardScope(); + meta.timestamp = a.timestamp; + meta.overallDuration = a.overallDuration; + map.emplace(a.type, ArticulationAppliedData(std::move(meta), 0, HUNDRED_PERCENT)); + } + map.preCalculateAverageData(); + + return NoteEvent(noteTimestamp, noteDuration, 0 /*voiceIdx*/, 0 /*staffIdx*/, + pitchLevel(PitchClass::A, 4), dynamicLevelFromType(DynamicType::Natural), + std::move(map), 0); +} + +struct Emitted { + int64_t timestamp; // msecs (the sequencer's event-map key) + int type; // Steinberg::Vst::Event::EventTypes + int pitch; // keyswitch note (or the played note) +}; + +// Drive the sequencer over the whole timeline and collect every VST note-on/note-off it emitted. +std::vector collect(VstSequencer& seq) +{ + seq.setActive(true); + seq.setPlaybackPosition(0); + const auto out = seq.movePlaybackForward(1000000); + + std::vector result; + for (const auto& seqPair : out) { + for (const auto& variantEvent : seqPair.second) { + if (!std::holds_alternative(variantEvent)) { + continue; + } + const VstEvent& e = std::get(variantEvent); + if (e.type == VstEvent::kNoteOnEvent) { + result.push_back({ seqPair.first, e.type, e.noteOn.pitch }); + } else if (e.type == VstEvent::kNoteOffEvent) { + result.push_back({ seqPair.first, e.type, e.noteOff.pitch }); + } + } + } + return result; +} + +bool has(const std::vector& events, int64_t timestamp, int type, int pitch) +{ + for (const Emitted& e : events) { + if (e.timestamp == timestamp && e.type == type && e.pitch == pitch) { + return true; + } + } + return false; +} + +// How many events match the given timestamp, type and pitch. +int count(const std::vector& events, int64_t timestamp, int type, int pitch) +{ + int n = 0; + for (const Emitted& e : events) { + if (e.timestamp == timestamp && e.type == type && e.pitch == pitch) { + ++n; + } + } + return n; +} + +// No event ever touches the given keyswitch pitch (on or off, at any time). +bool none(const std::vector& events, int pitch) +{ + for (const Emitted& e : events) { + if (e.pitch == pitch) { + return false; + } + } + return true; +} + +VstKeyswitchProfile makeProfile() +{ + VstKeyswitchProfile profile; + profile.keyswitches[ArticulationType::Standard] = 0; + profile.keyswitches[ArticulationType::Pizzicato] = 1; + profile.keyswitches[ArticulationType::Mute] = 5; + profile.keyswitches[ArticulationType::Tremolo16th] = 8; + profile.keyswitches[ArticulationType::Legato] = 12; + return profile; +} + +constexpr int NOTE_ON = VstEvent::kNoteOnEvent; +constexpr int NOTE_OFF = VstEvent::kNoteOffEvent; +constexpr int PIZZICATO_KS = 1; +constexpr int MUTE_KS = 5; +constexpr int TREMOLO_KS = 8; +constexpr int LEGATO_KS = 12; +} + +// A tremolo under a slur must forward BOTH the tremolo keyswitch (the primary, unchanged) and the +// Legato modifier as a keyswitch span: pressed at the note onset, released at the range end. Legato +// (a ranged modifier) must never hijack the primary keyswitch. +TEST(VstSequencerKeyswitchTest, SlurredTremoloForwardsTremoloPlusLegatoSpan) +{ + VstSequencer seq; + seq.init({}, false, makeProfile()); + + PlaybackData data; + data.originEvents[0].push_back(makeNote(0, 500, { + { ArticulationType::Tremolo16th, 0, 500 }, + { ArticulationType::Legato, 0, 500 }, + })); + seq.load(data); + + const auto events = collect(seq); + EXPECT_TRUE(has(events, 0, NOTE_ON, TREMOLO_KS)); // tremolo keyswitch still selected + EXPECT_TRUE(has(events, 0, NOTE_ON, LEGATO_KS)); // legato pressed at the note onset + EXPECT_TRUE(has(events, 500, NOTE_OFF, LEGATO_KS)); // legato released at the range end +} + +// The Legato keyswitch is pressed at EVERY note the slur covers, not once at the range start, so +// starting playback partway through the slur still engages it (a single start event would be +// dropped by a seek past it). +TEST(VstSequencerKeyswitchTest, LegatoIsPressedAtEveryCoveredNote) +{ + VstSequencer seq; + seq.init({}, false, makeProfile()); + + PlaybackData data; + // Two tremolo notes under one slur: the Legato range (0..1000) covers both; each tremolo note + // is its own span (distinct meta.timestamp), so both re-send the tremolo keyswitch. + data.originEvents[0].push_back(makeNote(0, 500, { + { ArticulationType::Tremolo16th, 0, 500 }, + { ArticulationType::Legato, 0, 1000 }, + })); + data.originEvents[500].push_back(makeNote(500, 500, { + { ArticulationType::Tremolo16th, 500, 500 }, + { ArticulationType::Legato, 0, 1000 }, + })); + seq.load(data); + + const auto events = collect(seq); + EXPECT_TRUE(has(events, 0, NOTE_ON, LEGATO_KS)); // pressed at the first covered note + EXPECT_TRUE(has(events, 500, NOTE_ON, LEGATO_KS)); // and re-pressed at the next covered note +} + +// A ranged modifier never wins the single primary keyswitch: a slurred pizzicato still selects the +// pizzicato timbre, with Legato forwarded alongside as its own span. +TEST(VstSequencerKeyswitchTest, RangedModifierDoesNotHijackTheTimbreKeyswitch) +{ + VstSequencer seq; + seq.init({}, false, makeProfile()); + + PlaybackData data; + data.originEvents[0].push_back(makeNote(0, 500, { + { ArticulationType::Pizzicato, 0, 500 }, + { ArticulationType::Legato, 0, 500 }, + })); + seq.load(data); + + const auto events = collect(seq); + EXPECT_TRUE(has(events, 0, NOTE_ON, PIZZICATO_KS)); // the timbre keyswitch is still selected + EXPECT_TRUE(has(events, 0, NOTE_ON, LEGATO_KS)); // legato forwarded alongside, not instead + EXPECT_TRUE(none(events, TREMOLO_KS)); // and no tremolo keyswitch leaks in +} + +// A standalone tremolo (no slur) carries no Legato, so no legato keyswitch is emitted at all. +TEST(VstSequencerKeyswitchTest, StandaloneTremoloEmitsNoLegato) +{ + VstSequencer seq; + seq.init({}, false, makeProfile()); + + PlaybackData data; + data.originEvents[0].push_back(makeNote(0, 500, { + { ArticulationType::Tremolo16th, 0, 500 }, + })); + seq.load(data); + + const auto events = collect(seq); + EXPECT_TRUE(has(events, 0, NOTE_ON, TREMOLO_KS)); // the tremolo keyswitch is selected + EXPECT_TRUE(none(events, LEGATO_KS)); // but nothing legato, since there is no slur +} + +// A chord is several notes at one timestamp. When they share an articulation the keyswitch must be +// sent once for the chord, not re-sent per note: a stale per-note lookup of the latched state would +// emit a duplicate NoteOn at the same instant, wasting the host block's fixed-size event list. +TEST(VstSequencerKeyswitchTest, ChordSharingAnArticulationEmitsOneKeyswitch) +{ + VstSequencer seq; + seq.init({}, false, makeProfile()); + + PlaybackData data; + data.originEvents[0].push_back(makeNote(0, 500, { { ArticulationType::Pizzicato, 0, 500 } })); + data.originEvents[0].push_back(makeNote(0, 500, { { ArticulationType::Pizzicato, 0, 500 } })); + seq.load(data); + + const auto events = collect(seq); + EXPECT_EQ(count(events, 0, NOTE_ON, PIZZICATO_KS), 1); // one keyswitch for the whole chord +} + +// Two mapped primary timbres on one note (the articulation set is a hash map) must pick the same +// keyswitch whatever the iteration order: the lowest keyswitch note wins, deterministically. +TEST(VstSequencerKeyswitchTest, TwoPrimaryTimbresPickTheLowestKeyswitch) +{ + auto keyswitchesFor = [](const std::vector& artics) { + VstSequencer seq; + seq.init({}, false, makeProfile()); + PlaybackData data; + data.originEvents[0].push_back(makeNote(0, 500, artics)); + seq.load(data); + return collect(seq); + }; + + const auto a = keyswitchesFor({ { ArticulationType::Pizzicato, 0, 500 }, { ArticulationType::Mute, 0, 500 } }); + const auto b = keyswitchesFor({ { ArticulationType::Mute, 0, 500 }, { ArticulationType::Pizzicato, 0, 500 } }); + + EXPECT_TRUE(has(a, 0, NOTE_ON, PIZZICATO_KS)); // pizzicato (lower note) wins either way + EXPECT_TRUE(none(a, MUTE_KS)); + EXPECT_TRUE(has(b, 0, NOTE_ON, PIZZICATO_KS)); + EXPECT_TRUE(none(b, MUTE_KS)); +} + +// With no Standard mapping and no mapped articulation on the note, the sequencer must not fabricate +// a pitch-zero keyswitch the instrument never advertised. +TEST(VstSequencerKeyswitchTest, NoKeyswitchWhenNothingIsAdvertised) +{ + VstKeyswitchProfile profile; + profile.keyswitches[ArticulationType::Pizzicato] = 1; // deliberately no Standard mapping + + VstSequencer seq; + seq.init({}, false, profile); + + PlaybackData data; + data.originEvents[0].push_back(makeNote(0, 500, { { ArticulationType::Standard, 0, 500 } })); + seq.load(data); + + const auto events = collect(seq); + EXPECT_TRUE(none(events, 0)); // no fabricated pitch-zero keyswitch +} + +// A slurred chord shares one onset. The legato span keyswitch must be pressed once at that onset, +// not once per chord note, or the duplicates waste the host block's fixed-size event list. +TEST(VstSequencerKeyswitchTest, SlurredChordPressesLegatoOnce) +{ + VstSequencer seq; + seq.init({}, false, makeProfile()); + + PlaybackData data; + for (int i = 0; i < 2; ++i) { + data.originEvents[0].push_back(makeNote(0, 500, { + { ArticulationType::Tremolo16th, 0, 500 }, + { ArticulationType::Legato, 0, 500 }, + })); + } + seq.load(data); + + const auto events = collect(seq); + EXPECT_EQ(count(events, 0, NOTE_ON, LEGATO_KS), 1); // one legato press for the whole chord +} From 088d4c2ecb466c328560b5b58910917ad2a32e22 Mon Sep 17 00:00:00 2001 From: Manolo Carrasco Date: Sun, 2 Aug 2026 21:08:28 +0200 Subject: [PATCH 3/5] audio: forward the host track name to instruments via VST3 channel context Thread the track name to the synthesizer and, for VST instruments, push it as the VST3 channel context name, so a plugin can auto-select its sound from the staff name. --- .../audio/engine/internal/audiocontext.cpp | 3 +- .../audio/engine/internal/audiofactory.cpp | 5 +-- .../audio/engine/internal/audiofactory.h | 3 +- .../audio/engine/internal/iaudiofactory.h | 4 +-- .../engine/internal/nodes/eventaudionode.cpp | 5 +-- .../engine/internal/nodes/eventaudionode.h | 4 ++- framework/audio/engine/isynthesizer.h | 4 +++ .../vst/internal/synth/vstsynthesiser.cpp | 34 +++++++++++++++++++ framework/vst/internal/synth/vstsynthesiser.h | 5 +++ 9 files changed, 58 insertions(+), 9 deletions(-) diff --git a/framework/audio/engine/internal/audiocontext.cpp b/framework/audio/engine/internal/audiocontext.cpp index ed01e2cfee..191c78a8c7 100644 --- a/framework/audio/engine/internal/audiocontext.cpp +++ b/framework/audio/engine/internal/audiocontext.cpp @@ -196,7 +196,8 @@ RetVal2 AudioContext::addTrack(const std::string& trackNam }; // Make source - RetVal source = audioFactory()->makeEventSource(trackId, playbackData, params.source, onOffStreamReceived); + RetVal source = audioFactory()->makeEventSource(trackId, trackName, playbackData, params.source, + onOffStreamReceived); if (!source.ret) { return RetType::make_ret(source.ret); } diff --git a/framework/audio/engine/internal/audiofactory.cpp b/framework/audio/engine/internal/audiofactory.cpp index ba38a9d756..e416a2a072 100644 --- a/framework/audio/engine/internal/audiofactory.cpp +++ b/framework/audio/engine/internal/audiofactory.cpp @@ -69,11 +69,12 @@ void AudioFactory::clearSynthSources() synthResolver()->clearSources(); } -RetVal AudioFactory::makeEventSource(const TrackId trackId, const mpe::PlaybackData& playbackData, +RetVal AudioFactory::makeEventSource(const TrackId trackId, const std::string& trackName, + const mpe::PlaybackData& playbackData, const AudioInputParams& params, const std::function onOffStreamReceived) const { - EventAudioNodePtr source = std::make_shared(trackId, playbackData, onOffStreamReceived); + EventAudioNodePtr source = std::make_shared(trackId, trackName, playbackData, onOffStreamReceived); source->setOutputSpec(audioEngine()->outputSpec()); source->applyInputParams(params); return RetVal::make_ok(source); diff --git a/framework/audio/engine/internal/audiofactory.h b/framework/audio/engine/internal/audiofactory.h index 44f3cedb94..519822f045 100644 --- a/framework/audio/engine/internal/audiofactory.h +++ b/framework/audio/engine/internal/audiofactory.h @@ -49,7 +49,8 @@ class AudioFactory : public IAudioFactory RetVal makeDefaultSynth(const TrackId trackId) const override; void clearSynthSources() override; - RetVal makeEventSource(const TrackId trackId, const mpe::PlaybackData& playbackData, const AudioInputParams& params, + RetVal makeEventSource(const TrackId trackId, const std::string& trackName, const mpe::PlaybackData& playbackData, + const AudioInputParams& params, const std::function onOffStreamReceived = nullptr) const override; // Make FX diff --git a/framework/audio/engine/internal/iaudiofactory.h b/framework/audio/engine/internal/iaudiofactory.h index 22ec1a1402..2648340d1b 100644 --- a/framework/audio/engine/internal/iaudiofactory.h +++ b/framework/audio/engine/internal/iaudiofactory.h @@ -53,8 +53,8 @@ class IAudioFactory : MODULE_GLOBAL_INTERFACE // This method clears this registry. virtual void clearSynthSources() = 0; - virtual RetVal makeEventSource(const TrackId trackId, const mpe::PlaybackData& playbackData, - const AudioInputParams& params, + virtual RetVal makeEventSource(const TrackId trackId, const std::string& trackName, + const mpe::PlaybackData& playbackData, const AudioInputParams& params, const std::function onOffStreamReceived = nullptr) const = 0; // Make FX virtual FxChainPtr makeMasterFxChain(const AudioFxChain& fxChain) const = 0; diff --git a/framework/audio/engine/internal/nodes/eventaudionode.cpp b/framework/audio/engine/internal/nodes/eventaudionode.cpp index 26c8402931..2744e32c06 100644 --- a/framework/audio/engine/internal/nodes/eventaudionode.cpp +++ b/framework/audio/engine/internal/nodes/eventaudionode.cpp @@ -32,9 +32,9 @@ using namespace muse::audio::engine; using namespace muse::audio::synth; using namespace muse::mpe; -EventAudioNode::EventAudioNode(TrackId trackId, const mpe::PlaybackData& playbackData, +EventAudioNode::EventAudioNode(TrackId trackId, const std::string& hostTrackName, const mpe::PlaybackData& playbackData, OnOffStreamEventsReceived onOffStreamReceived) - : m_trackId(trackId), m_playbackData(playbackData) + : m_trackId(trackId), m_hostTrackName(hostTrackName), m_playbackData(playbackData) { ONLY_AUDIO_ENGINE_THREAD; @@ -145,6 +145,7 @@ void EventAudioNode::applyInputParams(const AudioInputParams& requiredParams) } m_synth = synth.val; + m_synth->setHostTrackName(m_hostTrackName); m_synth->paramsChanged().onReceive(this, [this](const AudioInputParams& params) { m_paramsChanges.send(params); diff --git a/framework/audio/engine/internal/nodes/eventaudionode.h b/framework/audio/engine/internal/nodes/eventaudionode.h index 52022b50f1..b03f516082 100644 --- a/framework/audio/engine/internal/nodes/eventaudionode.h +++ b/framework/audio/engine/internal/nodes/eventaudionode.h @@ -39,7 +39,8 @@ class EventAudioNode : public AudioSourceNode, public async::Asyncable public: using OnOffStreamEventsReceived = std::function; - explicit EventAudioNode(TrackId trackId, const mpe::PlaybackData& playbackData, OnOffStreamEventsReceived onOffStreamReceived); + explicit EventAudioNode(TrackId trackId, const std::string& hostTrackName, const mpe::PlaybackData& playbackData, + OnOffStreamEventsReceived onOffStreamReceived); ~EventAudioNode() override; @@ -78,6 +79,7 @@ class EventAudioNode : public AudioSourceNode, public async::Asyncable void restoreSynthCtx(const SynthCtx& ctx); TrackId m_trackId = -1; + std::string m_hostTrackName; mpe::PlaybackData m_playbackData; synth::ISynthesizerPtr m_synth = nullptr; AudioInputParams m_params; diff --git a/framework/audio/engine/isynthesizer.h b/framework/audio/engine/isynthesizer.h index bfb81f7b05..c813df42cd 100644 --- a/framework/audio/engine/isynthesizer.h +++ b/framework/audio/engine/isynthesizer.h @@ -48,6 +48,10 @@ class ISynthesizer virtual void setup(const mpe::PlaybackData& playbackData) = 0; virtual const mpe::PlaybackData& playbackData() const = 0; + // Optional host track/instrument name, forwarded to instruments that auto-select their sound + // from it (e.g. via VST3 channel context). Ignored by default. + virtual void setHostTrackName(const std::string& /*name*/) {} + virtual const audio::AudioInputParams& params() const = 0; virtual async::Channel paramsChanged() const = 0; diff --git a/framework/vst/internal/synth/vstsynthesiser.cpp b/framework/vst/internal/synth/vstsynthesiser.cpp index 18be13baf7..056dfc675b 100644 --- a/framework/vst/internal/synth/vstsynthesiser.cpp +++ b/framework/vst/internal/synth/vstsynthesiser.cpp @@ -26,6 +26,8 @@ #include "mpe/articulationstringutils.h" #include "pluginterfaces/vst/ivstnoteexpression.h" +#include "pluginterfaces/vst/ivstchannelcontextinfo.h" +#include "public.sdk/source/vst/hosting/hostclasses.h" #include "public.sdk/source/vst/utility/stringconvert.h" #include "log.h" @@ -124,6 +126,7 @@ void VstSynthesiser::init(const OutputSpec& spec) const std::optional keyswitchProfile = queryKeyswitchProfile(m_pluginPtr->controller()); m_sequencer.init(m_vstAudioClient->paramsMapping(SUPPORTED_CONTROLLERS), m_useDynamicEvents, keyswitchProfile); m_inited = true; + sendChannelContext(); }; if (m_pluginPtr->isLoaded()) { @@ -180,6 +183,37 @@ std::string VstSynthesiser::name() const return m_pluginPtr->name(); } +void VstSynthesiser::setHostTrackName(const std::string& name) +{ + m_hostTrackName = name; + if (m_inited) { + sendChannelContext(); + } +} + +void VstSynthesiser::sendChannelContext() +{ + if (m_hostTrackName.empty() || !m_pluginPtr) { + return; + } + + PluginControllerPtr controller = m_pluginPtr->controller(); + if (!controller) { + return; + } + + Steinberg::FUnknownPtr infoListener(controller); + if (!infoListener) { + return; // the plugin does not use channel context + } + + Steinberg::IPtr list = Steinberg::Vst::HostAttributeList::make(); + Steinberg::Vst::String128 name128 = {}; + Steinberg::Vst::StringConvert::convert(m_hostTrackName, name128); + list->setString(Steinberg::Vst::ChannelContext::kChannelNameKey, name128); + infoListener->setChannelContextInfos(list); +} + void VstSynthesiser::flushSound() { m_sequencer.flushOffstream(); diff --git a/framework/vst/internal/synth/vstsynthesiser.h b/framework/vst/internal/synth/vstsynthesiser.h index 4bbcd4e843..cb4fbe081e 100644 --- a/framework/vst/internal/synth/vstsynthesiser.h +++ b/framework/vst/internal/synth/vstsynthesiser.h @@ -51,6 +51,8 @@ class VstSynthesiser : public muse::audio::synth::AbstractSynthesizer muse::audio::AudioSourceType type() const override; std::string name() const override; + void setHostTrackName(const std::string& name) override; + void flushSound() override; void setupSound(const mpe::PlaybackSetupData& setupData) override; @@ -70,10 +72,13 @@ class VstSynthesiser : public muse::audio::synth::AbstractSynthesizer void toggleVolumeGain(const bool isActive); audio::samples_t processSequence(const VstSequencer::EventSequence& sequence, const audio::samples_t samples, float* buffer); + void sendChannelContext(); // push the host track name to the plugin (VST3 channel context) + IVstPluginInstancePtr m_pluginPtr = nullptr; std::unique_ptr m_vstAudioClient = nullptr; audio::OutputSpec m_outputSpec; + std::string m_hostTrackName; VstSequencer m_sequencer; From d1b8e4d231321d040db9ba92ccd814033fca2b08 Mon Sep 17 00:00:00 2001 From: Manolo Carrasco Date: Sat, 15 Aug 2026 07:49:02 +0200 Subject: [PATCH 4/5] Apply CodeRabbit suggestion: sort off-stream keyswitch events --- framework/vst/internal/synth/vstsequencer.cpp | 1 + framework/vst/tests/vstsequencertest.cpp | 52 +++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/framework/vst/internal/synth/vstsequencer.cpp b/framework/vst/internal/synth/vstsequencer.cpp index 4ccf90588d..405307aecf 100644 --- a/framework/vst/internal/synth/vstsequencer.cpp +++ b/framework/vst/internal/synth/vstsequencer.cpp @@ -101,6 +101,7 @@ void VstSequencer::updateMainStreamEvents(const mpe::PlaybackEventsMap& events, void VstSequencer::updateOffStreamEvents(const mpe::PlaybackEventsMap& events) { addPlaybackEvents(m_offStreamEvents, events); + sortNoteOnEventsByPitch(m_offStreamEvents); updateOffSequenceIterator(); } diff --git a/framework/vst/tests/vstsequencertest.cpp b/framework/vst/tests/vstsequencertest.cpp index 88c003ab38..69d40175f8 100644 --- a/framework/vst/tests/vstsequencertest.cpp +++ b/framework/vst/tests/vstsequencertest.cpp @@ -318,3 +318,55 @@ TEST(VstSequencerKeyswitchTest, SlurredChordPressesLegatoOnce) const auto events = collect(seq); EXPECT_EQ(count(events, 0, NOTE_ON, LEGATO_KS), 1); // one legato press for the whole chord } + +// Off-stream (audition) playback must order NoteOns like the main stream: the keyswitch is a low +// pitch that has to reach the instrument before the played note, or the audited note sounds before +// its articulation is selected. updateMainStreamEvents sorts for this; updateOffStreamEvents must too. +TEST(VstSequencerKeyswitchTest, OffStreamKeyswitchPrecedesPlayedNote) +{ + VstSequencer seq; + seq.init({}, false, makeProfile()); + + PlaybackData data; + seq.load(data); // no main-stream events; drive the off-stream (audition) path only + + // Audition a single pizzicato note through the off-stream channel. + PlaybackEventsMap offStreamEvents; + offStreamEvents[0].push_back(makeNote(0, 500, { { ArticulationType::Pizzicato, 0, 500 } })); + data.offStream.send(offStreamEvents, true /*flush*/); + + // The off-stream path is only delivered while inactive (movePlaybackForward serves the main + // stream when active), so collect it directly rather than through collect(). + seq.setActive(false); + std::vector events; + for (const auto& seqPair : seq.movePlaybackForward(1000000)) { + for (const auto& variantEvent : seqPair.second) { + if (!std::holds_alternative(variantEvent)) { + continue; + } + const VstEvent& e = std::get(variantEvent); + if (e.type == VstEvent::kNoteOnEvent) { + events.push_back({ seqPair.first, e.type, e.noteOn.pitch }); + } + } + } + + // The pizzicato keyswitch (a low pitch) must be the first NoteOn at the onset, before the higher + // played note. Without the sort the played note would be emitted first. + ASSERT_TRUE(has(events, 0, NOTE_ON, PIZZICATO_KS)); + int keyswitchIdx = -1; + int playedNoteIdx = -1; + for (size_t i = 0; i < events.size(); ++i) { + if (events[i].timestamp != 0) { + continue; + } + if (events[i].pitch == PIZZICATO_KS && keyswitchIdx < 0) { + keyswitchIdx = static_cast(i); + } else if (events[i].pitch != PIZZICATO_KS && playedNoteIdx < 0) { + playedNoteIdx = static_cast(i); + } + } + ASSERT_GE(keyswitchIdx, 0); + ASSERT_GE(playedNoteIdx, 0); + EXPECT_LT(keyswitchIdx, playedNoteIdx); // keyswitch precedes the played note +} From a1d7f43d15abff62212e183dea58cc8728365397 Mon Sep 17 00:00:00 2001 From: Manolo Carrasco Date: Sat, 15 Aug 2026 07:58:41 +0200 Subject: [PATCH 5/5] Apply CodeRabbit suggestion: sort only NoteOn events (strict weak ordering) --- framework/vst/internal/synth/vstsequencer.cpp | 33 +++++++++----- framework/vst/tests/vstsequencertest.cpp | 44 +++++++++++++++++++ 2 files changed, 67 insertions(+), 10 deletions(-) diff --git a/framework/vst/internal/synth/vstsequencer.cpp b/framework/vst/internal/synth/vstsequencer.cpp index 405307aecf..4b508e25b3 100644 --- a/framework/vst/internal/synth/vstsequencer.cpp +++ b/framework/vst/internal/synth/vstsequencer.cpp @@ -469,20 +469,33 @@ void VstSequencer::sortNoteOnEventsByPitch(EventSequenceMap& destination) continue; } - std::stable_sort(seq.begin(), seq.end(), [](const EventType& e1, const EventType& e2) { - if (!std::holds_alternative(e1) || !std::holds_alternative(e2)) { - return false; + // Reorder only the NoteOn events (by pitch), leaving every other event (NoteOff, param and + // gain changes) exactly where it is. Sorting the whole sequence with a comparator that + // returned "equal" for non-NoteOn events would not be a strict weak ordering. + std::vector noteOns; + for (const EventType& event : seq) { + if (std::holds_alternative(event)) { + const VstEvent& vstEvent = std::get(event); + if (vstEvent.type == VstEvent::kNoteOnEvent) { + noteOns.push_back(vstEvent); + } } + } - const VstEvent& ve1 = std::get(e1); - const VstEvent& ve2 = std::get(e2); - - if (ve1.type == VstEvent::kNoteOnEvent && ve2.type == VstEvent::kNoteOnEvent) { - return ve1.noteOn.pitch < ve2.noteOn.pitch; - } + if (noteOns.size() <= 1) { + continue; + } - return false; + std::stable_sort(noteOns.begin(), noteOns.end(), [](const VstEvent& e1, const VstEvent& e2) { + return e1.noteOn.pitch < e2.noteOn.pitch; }); + + size_t noteOnIdx = 0; + for (EventType& event : seq) { + if (std::holds_alternative(event) && std::get(event).type == VstEvent::kNoteOnEvent) { + event = noteOns[noteOnIdx++]; + } + } } } diff --git a/framework/vst/tests/vstsequencertest.cpp b/framework/vst/tests/vstsequencertest.cpp index 69d40175f8..ec639ed272 100644 --- a/framework/vst/tests/vstsequencertest.cpp +++ b/framework/vst/tests/vstsequencertest.cpp @@ -370,3 +370,47 @@ TEST(VstSequencerKeyswitchTest, OffStreamKeyswitchPrecedesPlayedNote) ASSERT_GE(playedNoteIdx, 0); EXPECT_LT(keyswitchIdx, playedNoteIdx); // keyswitch precedes the played note } + +// A timestamp bucket can interleave a NoteOff (from a note ending there) with the NoteOns of a note +// starting there. The sort must reorder only the NoteOns and leave the NoteOff in place; its +// comparator must be a strict weak ordering over such a mixed bucket. +TEST(VstSequencerKeyswitchTest, SortOrdersNoteOnsAndKeepsInterleavedNoteOff) +{ + VstSequencer seq; + seq.init({}, false, makeProfile()); + + PlaybackData data; + // Note A ends at t=500; the muted note B starts at t=500. Bucket 500 therefore holds A's NoteOff + // together with B's played NoteOn and its lower-pitched mute keyswitch NoteOn. + data.originEvents[0].push_back(makeNote(0, 500, { { ArticulationType::Standard, 0, 500 } })); + data.originEvents[500].push_back(makeNote(500, 500, { { ArticulationType::Mute, 500, 500 } })); + seq.load(data); + + const auto events = collect(seq); + + // The interleaved NoteOff survives the sort. + bool noteOffAt500 = false; + for (const Emitted& e : events) { + if (e.timestamp == 500 && e.type == NOTE_OFF) { + noteOffAt500 = true; + } + } + EXPECT_TRUE(noteOffAt500); + + // And the mute keyswitch (low pitch) still precedes the played note NoteOn at t=500. + int keyswitchIdx = -1; + int playedNoteIdx = -1; + for (size_t i = 0; i < events.size(); ++i) { + if (events[i].timestamp != 500 || events[i].type != NOTE_ON) { + continue; + } + if (events[i].pitch == MUTE_KS && keyswitchIdx < 0) { + keyswitchIdx = static_cast(i); + } else if (events[i].pitch != MUTE_KS && playedNoteIdx < 0) { + playedNoteIdx = static_cast(i); + } + } + ASSERT_GE(keyswitchIdx, 0); + ASSERT_GE(playedNoteIdx, 0); + EXPECT_LT(keyswitchIdx, playedNoteIdx); +}