From 801dc20bd396fdc42cbab37681876dca3838fe63 Mon Sep 17 00:00:00 2001 From: Bonni Date: Tue, 22 Sep 2026 20:49:58 -0300 Subject: [PATCH 1/3] Let several manuals answer one MIDI channel Reported in #29: with one keyboard on Friesach, only two manuals could be mapped to the same channel. Assigning a third silently unassigned the others, and a mapping that shared a channel was stripped of every such binding the next time it loaded. Both rules came from #3, where a saved mapping sent one keyboard to several divisions by accident. Sharing a channel is a legitimate choice, though: it is how one keyboard plays two divisions at once, and a rig with fewer keyboards than manuals has no other way to reach them. Learning a manual from a pressed key still takes the channel, which is what teaching a rig one keyboard at a time means. A channel chosen in Settings now shares it, and the MIDI tab names the manuals that share one, so a player who did it unawares can see why two divisions are sounding. Repair keeps its other work: a binding for a keyboard the organ does not have still goes, and so does one listed twice over in every field. --- src/mp_audio/MasterpieceProcessor.cpp | 53 +++- src/mp_audio/MasterpieceProcessor.h | 18 +- src/mp_control/MidiMap.cpp | 21 +- src/mp_control/MidiMap.h | 15 +- src/mp_core/OdfLoader.cpp | 198 +++++++++++++-- src/mp_core/OrganModel.h | 29 +++ src/mp_ui/Settings.cpp | 39 ++- src/mp_ui/Settings.h | 5 + .../m24.expression.Organ_Hauptwerk_xml | 27 +- tests/test_core.cpp | 240 +++++++++++++++++- 10 files changed, 567 insertions(+), 78 deletions(-) diff --git a/src/mp_audio/MasterpieceProcessor.cpp b/src/mp_audio/MasterpieceProcessor.cpp index 701b431..658ba4f 100644 --- a/src/mp_audio/MasterpieceProcessor.cpp +++ b/src/mp_audio/MasterpieceProcessor.cpp @@ -1642,6 +1642,23 @@ bool MasterpieceProcessor::startPipeLayers(const Pipe& pipe, Id rankId, static_cast(layer.gainDb), -100.0f) * layerLevel(layer); + // How hard the key was struck. The organ states the attenuation at the + // softest touch; full velocity is unattenuated. Inverted, the sense + // swaps. Applied here and not per sample: a pipe keeps the level it + // began with until the next strike, which is what an organ does. + // + // The MAGNITUDE is the attenuation: every set stores one constant for + // its whole pipework, and while some write it +5 dB others write -5 or + // -6 (Alessandria +5, Giubiasco -6, Cracow -10). Read as a signed gain + // the negative sets would get LOUDER when played softly, which no + // tracker organ does; the field's own name is MaxAttenuation. + if (layer.velSensMaxAttenDb != 0.0) { + const double v01 = juce::jlimit(0.0, 1.0, static_cast(velocity) / 127.0); + const double attn = layer.invertVelocitySens ? v01 : 1.0 - v01; + vs.gain *= juce::Decibels::decibelsToGain( + static_cast(-std::fabs(layer.velSensMaxAttenDb) * attn), -100.0f); + } + // The player's own voicing, on top of what the organ declares. // Gain and tuning only: they are a multiply and a ratio at note-on // and cost nothing per sample, so they apply even with the DSP @@ -1677,7 +1694,9 @@ bool MasterpieceProcessor::startPipeLayers(const Pipe& pipe, Id rankId, // Which tremulant reaches this pipe, and how far it moves it. The // organ states the depth per pipe, so a flute and a reed on the - // same chest wobble by different amounts. + // same chest wobble by different amounts — and the LAYER trims that + // depth again, which is how one stop on a chest can be left nearly + // steady while its neighbour shakes. const auto tm = model_.tremulantPipes.find(pipe.pipeId); if (tm != model_.tremulantPipes.end()) { const auto ti = tremIndexOf_.find(tm->second.tremulantId); @@ -1686,9 +1705,12 @@ bool MasterpieceProcessor::startPipeLayers(const Pipe& pipe, Id rankId, // Decibels to a linear swing about unity, and percent of a // semitone to semitones. vs.tremAmpDepth = static_cast( - juce::Decibels::decibelsToGain(tm->second.ampDepthDb, -60.0) - + juce::Decibels::decibelsToGain( + tm->second.ampDepthDb + layer.tremAmpDepthAdjustDb, -60.0) - 1.0); - vs.tremPitchDepth = tm->second.pitchDepthPct / 100.0; + vs.tremPitchDepth = + tm->second.pitchDepthPct / 100.0 * + juce::jlimit(0.0, 4.0, layer.tremPitchDepthAdjustPct / 100.0); } } } @@ -2163,8 +2185,11 @@ void MasterpieceProcessor::buildPalletIndex() { } if (palletPipes_.empty()) return; // nothing to open: keys stay plain keys - for (const auto& [switchId, key] : model_.keyboardKeys) + keySwitchIds_.clear(); + for (const auto& [switchId, key] : model_.keyboardKeys) { keySwitchByKey_[static_cast(key.keyboardId) * 256 + key.midiNote] = switchId; + keySwitchIds_.insert(switchId); + } palletNotes_.reserve(palletPipes_.size()); heldKeySwitches_.reserve(256); } @@ -2213,10 +2238,20 @@ void MasterpieceProcessor::triggerNoiseFor(Id switchId, bool engaged) { if (!engaged && rank.pipes.size() > 1) index = 1; const Pipe& pipe = rank.pipes[index]; + // A key-action noise is the sound of the strike, so it takes the strike's + // velocity; a stop or blower noise is a mechanical event at a medium + // touch. The sets state a velocity response for the former and this is + // the only place their figures can act — the noise is not played by a + // key, so startPipeLayers never sees it. + const int noiseVelocity = + keySwitchIds_.count(switchId) != 0 + ? juce::jlimit(1, 127, palletVelocity_) + : 100; + const uint64_t noteId = nextNoteId_++; for (const auto& layer : pipe.layers) { NoteStrike strike; - strike.velocity = 100; + strike.velocity = noiseVelocity; const int attackIndex = selectAttack(layer, strike); if (attackIndex < 0) continue; @@ -2232,6 +2267,14 @@ void MasterpieceProcessor::triggerNoiseFor(Id switchId, bool engaged) { vs.gain = juce::Decibels::decibelsToGain( static_cast(layer.gainDb), -100.0f) * layerLevel(layer); + // The organ's velocity response reaches noises too, and on every set + // that declares one it is the NOISE layers that carry it. + if (layer.velSensMaxAttenDb != 0.0) { + const double v01 = static_cast(noiseVelocity) / 127.0; + const double attn = layer.invertVelocitySens ? v01 : 1.0 - v01; + vs.gain *= juce::Decibels::decibelsToGain( + static_cast(-std::fabs(layer.velSensMaxAttenDb) * attn), -100.0f); + } // A noise is a one-shot; looping it would leave the console rattling. vs.oneShot = true; vs.busIndex = busForPipe(pipe.pipeId); diff --git a/src/mp_audio/MasterpieceProcessor.h b/src/mp_audio/MasterpieceProcessor.h index 7d3904a..b53308e 100644 --- a/src/mp_audio/MasterpieceProcessor.h +++ b/src/mp_audio/MasterpieceProcessor.h @@ -357,13 +357,15 @@ class MasterpieceProcessor : public juce::AudioProcessor { // Assign a manual to a channel, and optionally to one console. The simple // case of the full manual receiver in MidiMap: whole compass, no transpose, // full velocity. Anything more is set through midiMap().addKeyboardBinding(). - void setKeyboardForChannel(int channel, Id keyboardId, int deviceId = 0) { + // `exclusive` decides what happens to the manuals already on that channel. + // Learning one from a pressed key takes it from them: a player teaching a + // rig one keyboard at a time means this manual, not both. A channel chosen + // by hand shares it, which is how one keyboard is made to play two + // divisions at once. + void setKeyboardForChannel(int channel, Id keyboardId, int deviceId = 0, + bool exclusive = true) { midiMap_.removeKeyboardBindingsFor(keyboardId); - // Take the channel rather than share it. Without this the comment below - // was untrue: three manuals ended up claiming channel 1 on a real saved - // mapping, which made two of them unplayable and sent every manual to the - // pedal. - midiMap_.releaseChannel(channel, deviceId, keyboardId); + if (exclusive) midiMap_.releaseChannel(channel, deviceId, keyboardId); if (keyboardId != 0 && channel > 0) { MidiMap::KeyboardBinding b; b.channel = channel; @@ -1035,6 +1037,10 @@ class MasterpieceProcessor : public juce::AudioProcessor { std::unordered_map>> palletPipes_; // (keyboard, note) -> the switch that key IS, for organs that declare one. std::unordered_map keySwitchByKey_; + // The same switches as a set, for the noise path: a key-action noise fired + // by a KEY switch belongs to the strike and takes its velocity, while one + // fired by a stop switch is a mechanical event at a fixed touch. + std::unordered_set keySwitchIds_; // The key switches held down, by the same key id soundingNotes_ uses, so a // note-off finds the switch its note-on engaged. std::unordered_map heldKeySwitches_; diff --git a/src/mp_control/MidiMap.cpp b/src/mp_control/MidiMap.cpp index 449a9c2..d7c3d9e 100644 --- a/src/mp_control/MidiMap.cpp +++ b/src/mp_control/MidiMap.cpp @@ -389,23 +389,20 @@ int MidiMap::repairKeyboardBindings(const std::vector& playableKeyboards) { playableKeyboards.end(); }; - // Which bindings collide: same channel, overlapping console, different - // keyboard. Channel 0 ("any channel") overlaps every channel. - auto overlaps = [](const KeyboardBinding& x, const KeyboardBinding& y) { - const bool sameChannel = - x.channel == y.channel || x.channel == 0 || y.channel == 0; - const bool sameConsole = - x.deviceId == y.deviceId || x.deviceId == 0 || y.deviceId == 0; - return sameChannel && sameConsole && x.keyboardId != y.keyboardId; + // The same binding twice over. Two manuals on one channel is a choice a + // player can make -- one keyboard playing two divisions is a coupler of + // their own making -- but the same manual, channel, console and compass + // listed twice only doubles the work of every note. + auto identical = [](const KeyboardBinding& x, const KeyboardBinding& y) { + return x.keyboardId == y.keyboardId && x.channel == y.channel && + x.deviceId == y.deviceId && x.lowKey == y.lowKey && + x.highKey == y.highKey && x.transpose == y.transpose; }; std::vector drop(keyboardBindings_.size(), false); for (size_t i = 0; i < keyboardBindings_.size(); ++i) { if (!playable(keyboardBindings_[i].keyboardId)) drop[i] = true; for (size_t j = i + 1; j < keyboardBindings_.size(); ++j) - if (overlaps(keyboardBindings_[i], keyboardBindings_[j])) { - drop[i] = true; - drop[j] = true; - } + if (identical(keyboardBindings_[i], keyboardBindings_[j])) drop[j] = true; } std::vector kept; kept.reserve(keyboardBindings_.size()); diff --git a/src/mp_control/MidiMap.h b/src/mp_control/MidiMap.h index 949f4a3..54468f2 100644 --- a/src/mp_control/MidiMap.h +++ b/src/mp_control/MidiMap.h @@ -242,13 +242,14 @@ class MidiMap { // bindings went. Two things are dropped: // - a binding for a keyboard this organ does not have (a stale file, or // one written for a different set), and - // - EVERY binding in a channel collision: two or more manuals claiming one - // channel on overlapping consoles. There is no way to know which of them - // the player intended, and keeping any one of them keeps a manual - // unreachable, so all of them give way and the organ's own assignment - // takes over, which is correct for every set we have seen. - // Versions up to 0.3.7 could write such collisions; a mapping saved by one - // of them is repaired the first time it is loaded. + // - a binding listed twice over, identical in every field, which only + // doubles the work of each note. + // + // Manuals sharing a channel are NOT a fault. One keyboard playing two + // divisions is a coupler a player can build for themselves, and a rig with + // more manuals than keyboards has no other way to reach them. Up to 0.5.3 + // every binding in such a collision was dropped, which made the mapping + // impossible to keep. int repairKeyboardBindings(const std::vector& playableKeyboards); const std::vector& keyboardBindings() const { return keyboardBindings_; diff --git a/src/mp_core/OdfLoader.cpp b/src/mp_core/OdfLoader.cpp index eee54d9..04a597f 100644 --- a/src/mp_core/OdfLoader.cpp +++ b/src/mp_core/OdfLoader.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -32,6 +33,7 @@ #include #include #include +#include #include namespace mp { @@ -464,6 +466,18 @@ bool OdfLoader::loadFromXmlString(const std::string& xml, const std::string& fil fieldInt(row, "PitchLvl_IncrementingContinuousControlID", nullptr, 0); layer.pitchSensitivityHzPerUnit = fieldDouble( row, "PitchLvl_IncrementingCtsCtrlSensitivityHzPerCtrlUnit", nullptr, 0.0); + // How hard the key is struck reaches the pipe's level, and how far this + // layer trims the chest's tremulant depth. All four are stated per layer + // and two thirds of the corpus fills them; unread they made every note + // one level and every stop on a tremmed chest wobble alike. + layer.velSensMaxAttenDb = fieldDouble( + row, "AmpLvl_VelocitySensitivityMaxAttenuationDecibels", nullptr, 0.0); + layer.invertVelocitySens = + fieldBool(row, "AmpLvl_InvertVelocitySensitivity", nullptr, false); + layer.tremAmpDepthAdjustDb = fieldDouble( + row, "AmpLvl_TremulantModDepthAdjustDecibels", nullptr, 0.0); + layer.tremPitchDepthAdjustPct = fieldDouble( + row, "PitchLvl_TremulantModDepthAdjustPercent", nullptr, 100.0); pipeIt->second->layers.push_back(std::move(layer)); layerById[layerId] = &pipeIt->second->layers.back(); }); @@ -740,6 +754,7 @@ bool OdfLoader::loadFromXmlString(const std::string& xml, const std::string& fil s.controllingSwitchId = fieldInt(row, "ControllingSwitchID", "d", 0); s.defaultAsgnCode = fieldInt(row, "Hint_DefaultAssignmentCodeOfAssocInputOutputSwitch", "e", 0); + s.hintPrimaryRankId = fieldInt(row, "Hint_PrimaryAssociatedRankID", "f", 0); if (s.stopId != 0) outModel.stops[s.stopId] = std::move(s); }); @@ -887,6 +902,73 @@ bool OdfLoader::loadFromXmlString(const std::string& xml, const std::string& fil outModel.switchLinkages.push_back(l); }); + // ---- Stop -> Rank through Hint_PrimaryAssociatedRankID ---- + // A set — and every demo set that ships only part of its pipework — may + // declare no StopRank rows at all and leave the stop's rank in this hint. + // The reference converters follow it, and without it the stop draws, moves + // its switch and plays nothing. It is applied only when the hinted rank's + // pipes are NOT pallet-wired: those organs reach every pipe through the + // switch network (key AND stop AND routing), and a synthesized direct path + // would bypass the very wiring that decides when a pipe speaks. Ranks the + // pallet index owns keep their wiring untouched. + { + std::unordered_set palletRanks; + for (const auto& [rankId, rank] : outModel.ranks) + for (const Pipe& p : rank.pipes) + if (p.palletSwitchId != 0) { + palletRanks.insert(rankId); + break; + } + + // The division's own keyboard gives the compass the hint maps over. A + // division with no keyboard (or one of unstated size) gets Hauptwerk's + // own default of 61 notes from 36, which is what an unstated StopRank + // maps anyway. + auto compassFor = [&outModel](Id divisionId, int& firstNote, int& numKeys) { + firstNote = 36; + numKeys = 61; + const auto divIt = outModel.divisions.find(divisionId); + if (divIt == outModel.divisions.end()) return; + for (Id kbId : divIt->second.keyboardIds) { + const auto kbIt = outModel.keyboards.find(kbId); + if (kbIt == outModel.keyboards.end()) continue; + if (kbIt->second.numKeys <= 0) continue; + firstNote = kbIt->second.firstMidiNote; + numKeys = kbIt->second.numKeys; + return; + } + }; + + int hintStops = 0; + for (auto& [stopId, stop] : outModel.stops) { + if (!stop.ranks.empty()) continue; + if (stop.hintPrimaryRankId == 0) continue; + const auto rankIt = outModel.ranks.find(stop.hintPrimaryRankId); + if (rankIt == outModel.ranks.end()) { + outDiag.danglingIds.push_back(stop.hintPrimaryRankId); + continue; + } + // Only when the rank actually holds pipework. On every set surveyed the + // hint names either a rank the demo does not ship (no Pipe rows) or one + // the pallets own; mapping to an empty rank would sound nothing and + // would erase the stopsWithoutRanks diagnostic that truthfully says the + // stop can never play as installed. + if (rankIt->second.pipes.empty()) continue; + if (palletRanks.count(stop.hintPrimaryRankId) != 0) continue; + StopRankEntry e; + e.rankId = stop.hintPrimaryRankId; + compassFor(stop.divisionId, e.firstMappedDivisionNote, e.numMappedNotes); + stop.ranks.push_back(e); + ++hintStops; + } + if (hintStops > 0) + outDiag.warnings.emplace_back( + std::to_string(hintStops) + + " stop(s) reach their rank through Hint_PrimaryAssociatedRankID " + "(no StopRank rows declared); the hint is followed as the reference " + "converters do"); + } + // ---- M1.3 validator: stops without StopRank rows (full ODF only; // CODM gains its rows at M1.5 compile time) ---- for (const auto& [id, s] : outModel.stops) @@ -1144,10 +1226,9 @@ bool OdfLoader::loadFromXmlString(const std::string& xml, const std::string& fil fieldInt(row, "ShutterPositionContinuousControlID", "c", 0); if (e.continuousControlId == 0) e.continuousControlId = fieldInt(row, "ContinuousControlID", nullptr, 0); - e.closedFilterHz = fieldDouble(row, "ShadesClosedFilterFreqHz", "d", e.closedFilterHz); - e.openFilterHz = fieldDouble(row, "ShadesOpenFilterFreqHz", "e", e.openFilterHz); - e.closedAttnDb = fieldDouble(row, "ShadesClosedAttenuationDb", "f", e.closedAttnDb); - e.openAttnDb = fieldDouble(row, "ShadesOpenAttenuationDb", "g", e.openAttnDb); + // An Enclosure row carries no filter numbers at all — the dictionary gives + // it exactly three attributes (id, name, shutter control). The filter is + // stated per pipe on EnclosurePipe and gathered there, below. if (e.enclosureId == 0) return; if (e.continuousControlId != 0 && outModel.continuousControls.count(e.continuousControlId) == 0) @@ -1158,28 +1239,95 @@ bool OdfLoader::loadFromXmlString(const std::string& xml, const std::string& fil // EnclosurePipe rows say which pipework each box encloses. This is what // makes expression per rank rather than a filter over the whole organ: an // unenclosed Great must stay unenclosed while the Swell shades move. - forEachRow(odfRoot, "EnclosurePipe", [&](pugi::xml_node row) { - const Id encId = fieldInt(row, "EnclosureID", "a", 0); - auto it = outModel.enclosures.find(encId); - if (it == outModel.enclosures.end()) { - if (encId != 0) outDiag.danglingIds.push_back(encId); - return; - } - ++it->second.numShades; - const Id pipeId = fieldInt(row, "PipeID", "b", 0); - if (pipeId != 0) { - // A pipe named by two boxes is an authoring error; first wins and the - // conflict is reported rather than silently resolved. - const auto existing = outModel.pipeEnclosure.find(pipeId); - if (existing != outModel.pipeEnclosure.end() && existing->second != encId) - outDiag.warnings.emplace_back( - "Pipe " + std::to_string(pipeId) + " is enclosed by both " + - std::to_string(existing->second) + " and " + std::to_string(encId) + - "; keeping the first"); - else - outModel.pipeEnclosure[pipeId] = encId; + // + // The row also carries the box's filter, stated against each pipe's own + // pitch: OverallAttnDb insertion loss, and the [MaxFreq, MinFreq] band the + // shades move it between (closed one band, open higher). The engine filters + // one bus per box rather than one filter per voice, so the box gets the + // MEDIAN of its pipes' values — the representative figure for the box — + // and the raw spread stays in the file where a per-voice filter can use it + // later. Read as the dictionary numbers them: c..h. + { + // Per enclosure, the six values from every row, for a median at the end. + std::unordered_map>> shadeParams; + forEachRow(odfRoot, "EnclosurePipe", [&](pugi::xml_node row) { + const Id encId = fieldInt(row, "EnclosureID", "a", 0); + auto it = outModel.enclosures.find(encId); + if (it == outModel.enclosures.end()) { + if (encId != 0) outDiag.danglingIds.push_back(encId); + return; + } + ++it->second.numShades; + const Id pipeId = fieldInt(row, "PipeID", "b", 0); + if (pipeId != 0) { + // A pipe named by two boxes is an authoring error; first wins and the + // conflict is reported rather than silently resolved. + const auto existing = outModel.pipeEnclosure.find(pipeId); + if (existing != outModel.pipeEnclosure.end() && existing->second != encId) + outDiag.warnings.emplace_back( + "Pipe " + std::to_string(pipeId) + " is enclosed by both " + + std::to_string(existing->second) + " and " + std::to_string(encId) + + "; keeping the first"); + else + outModel.pipeEnclosure[pipeId] = encId; + } + std::array v{ + fieldDouble(row, "FiltParamWhenClsd_OverallAttnDb", "c", 0.0), + fieldDouble(row, "FiltParamWhenClsd_MaxFreqHz", "d", 0.0), + fieldDouble(row, "FiltParamWhenClsd_MinFreqHz", "e", 0.0), + fieldDouble(row, "FiltParamWhenClsd_ExtraAttnAtMinDb", "f", 0.0), + fieldDouble(row, "FiltParamWhenOpen_MaxFreqHz", "g", 0.0), + fieldDouble(row, "FiltParamWhenOpen_MinFreqHz", "h", 0.0)}; + if (v[1] > 0.0 || v[4] > 0.0) + shadeParams[encId].push_back(v); + }); + + for (auto& [encId, rows] : shadeParams) { + auto encIt = outModel.enclosures.find(encId); + if (encIt == outModel.enclosures.end() || rows.empty()) continue; + // The 75th percentile, not the median. Each pipe's figure is stated + // against its own pitch, so a box's values climb with the compass and + // its median describes a pipe LOWER than most of what is heard: on + // Bégard the open median is 1.7 kHz, which would leave a box that is + // open sounding permanently closed. The upper quartile is the bus + // filter's honest compromise — the open box stays close to + // transparent, the closed one clearly muffled, and the numbers are + // still the set's own (Bégard closed 932 Hz / open 3.7 kHz, against + // invented 800 Hz / 12 kHz before). A per-pipe filter is the faithful + // model and this is where it would go; see the Enclosure comment in + // OrganModel.h. + auto quantile = [&rows](size_t i, double q) { + std::vector col; + col.reserve(rows.size()); + for (const auto& r : rows) + if (r[i] > 0.0) col.push_back(r[i]); + if (col.empty()) return 0.0; + std::sort(col.begin(), col.end()); + const double pos = q * static_cast(col.size() - 1); + return col[static_cast(pos + 0.5)]; + }; + Enclosure& e = encIt->second; + const double closedMax = quantile(1, 0.75), closedMin = quantile(2, 0.75); + const double openMax = quantile(4, 0.75), openMin = quantile(5, 0.75); + if (closedMax > 0.0) e.closedFilterHz = closedMax; + else if (closedMin > 0.0) e.closedFilterHz = closedMin; + if (openMax > 0.0) e.openFilterHz = openMax; + else if (openMin > 0.0) e.openFilterHz = openMin; + // Closed attenuation is the insertion loss plus the extra at the bottom + // of the band; an open box takes the insertion loss off entirely. + auto meanPositive = [&rows](size_t i) { + double sum = 0.0; + size_t n = 0; + for (const auto& r : rows) { + if (r[i] > 0.0) { sum += r[i]; ++n; } + } + return n > 0 ? sum / static_cast(n) : 0.0; + }; + e.closedAttnDb = -(meanPositive(0) + meanPositive(3)); + e.openAttnDb = 0.0; + e.filterParamsFromPipes = true; } - }); + } // ---- M2.3: Tremulant table ---- forEachRow(odfRoot, "Tremulant", [&](pugi::xml_node row) { diff --git a/src/mp_core/OrganModel.h b/src/mp_core/OrganModel.h index f02acff..184a8e7 100644 --- a/src/mp_core/OrganModel.h +++ b/src/mp_core/OrganModel.h @@ -126,6 +126,21 @@ struct PipeLayer { // this costs nothing until it is used. Id pitchControlId = 0; double pitchSensitivityHzPerUnit = 0.0; + // How hard the key was struck changes how loud the pipe speaks. The organ + // states the attenuation at the softest touch; full velocity is unattenuated. + // Half the corpus declares one, and without it every note plays at one + // level, which is what a tracker action is NOT. `invert` swaps the sense + // for the sets whose couplers or second touch need it (AmpLvl_Invert...). + // The raw sign is kept as the file writes it — producers disagree (+5 on + // Alessandria, -6 on Giubiasco) and the magnitude is the attenuation. + double velSensMaxAttenDb = 0.0; // AmpLvl_VelocitySensitivityMaxAttenuationDecibels + bool invertVelocitySens = false; + // How far THIS layer's tremulant depth is adjusted from the pipe's own. + // The depth belongs to the chest (TremulantWaveformPipe); these are the + // per-layer trims on top of it, and applying them per layer is what keeps a + // flute and a reed on the same tremulant wobbling differently. + double tremAmpDepthAdjustDb = 0.0; // AmpLvl_TremulantModDepthAdjustDecibels + double tremPitchDepthAdjustPct = 100.0; // PitchLvl_TremulantModDepthAdjustPercent // M2+: enclosure/trem/wind depth, EQ, AudioOut codes, reverb-tail truncation. double enclosureDepth01 = 1.0; double tremDepthDb = 0.0; @@ -192,6 +207,12 @@ struct Stop { int defaultAsgnCode = 0; // 20xx-30xx determines capture division + jamb sort std::vector ranks; Id controllingSwitchId = 0; + // Hint_PrimaryAssociatedRankID: the rank this stop draws, for the sets — + // and every demo set that ships part of its pipework — that declare no + // StopRank rows at all. The reference converters follow it; without it the + // stop draws and plays nothing. Gathered into `ranks` at load when it is + // safe to do so; see the loader for the one case where it is not. + Id hintPrimaryRankId = 0; }; // One edge of the key-flow graph: keys played on `sourceKeyboard` also reach @@ -420,8 +441,16 @@ struct Enclosure { Id enclosureId = 0; std::string name; Id continuousControlId = 0; + // The filter the shades impose, as the engine's one-cutoff-one-gain model + // takes it. Hauptwerk states these PER PIPE (EnclosurePipe's FiltParam...), + // relative to each pipe's own pitch, so there is no single enclosure-level + // number in the file: these are the medians of the pipes' values gathered at + // load, which is the representative figure for a bus-level filter. Set from + // the pipes when the set declares any; the numbers below are only the + // fallback for a box that ships none. double closedFilterHz = 800.0, openFilterHz = 12000.0; double closedAttnDb = -24.0, openAttnDb = 0.0; + bool filterParamsFromPipes = false; // Shade positions the ODF actually declares (EnclosurePipe rows). An // enclosure with none is inert — the validator reports it rather than // silently swallowing the swell pedal (query enclosure-without-shades). diff --git a/src/mp_ui/Settings.cpp b/src/mp_ui/Settings.cpp index 1140ff4..b2be01b 100644 --- a/src/mp_ui/Settings.cpp +++ b/src/mp_ui/Settings.cpp @@ -1,5 +1,7 @@ #include "Settings.h" +#include + #include "ManualDialog.h" namespace mp::ui { @@ -810,6 +812,8 @@ MidiPanel::MidiPanel(MasterpieceProcessor& p, juce::AudioDeviceManager& devices) styleLabel(inputsLabel_, "MIDI inputs"); addAndMakeVisible(outputsLabel_); styleLabel(outputsLabel_, "MIDI output"); + addAndMakeVisible(sharedNote_); + sharedNote_.setColour(juce::Label::textColourId, juce::Colours::lightgrey); addAndMakeVisible(keyboardsLabel_); styleLabel(keyboardsLabel_, "Keyboards"); @@ -894,6 +898,29 @@ MidiPanel::~MidiPanel() { proc_.setMidiOutput(nullptr); } +// Manuals that answer the same channel, named. Sharing one is a way of +// playing two divisions from one keyboard; doing it unawares is how a manual +// ends up sounding the wrong division, which is what this line prevents. +void MidiPanel::showSharedChannels() { + std::map byChannel; + for (const auto& b : proc_.channelAssignments()) { + if (b.channel <= 0) continue; + const auto it = proc_.organModel().keyboards.find(b.keyboardId); + byChannel[b.channel].add(it != proc_.organModel().keyboards.end() && + !it->second.name.empty() + ? juce::String(it->second.name) + : "keyboard " + juce::String((int) b.keyboardId)); + } + juce::String text; + for (const auto& [channel, names] : byChannel) { + if (names.size() < 2) continue; + text << (text.isEmpty() ? "" : " ") << "Channel " << channel << ": " + << names.joinIntoString(", "); + } + sharedNote_.setText(text.isEmpty() ? "" : "Shared -- " + text, + juce::dontSendNotification); +} + void MidiPanel::refresh() { inputs_.clear(); for (const auto& in : juce::MidiInput::getAvailableDevices()) { @@ -934,12 +961,14 @@ void MidiPanel::refresh() { box->setSelectedId(selected, juce::dontSendNotification); auto* raw = box.get(); box->onChange = [this, kb, raw] { - // Clear any previous claim first: two keyboards on one channel would - // make one of them unreachable, and the player would have no way to see - // which. + // Shared, not taken: a channel chosen here may already belong to another + // manual, and one keyboard playing two divisions is a coupler a player + // can build for themselves. The line under the row says who else is on + // it, so nothing is hidden. if (raw->getSelectedId() > 1) - proc_.setKeyboardForChannel(raw->getSelectedId() - 1, kb); + proc_.setKeyboardForChannel(raw->getSelectedId() - 1, kb, 0, false); proc_.saveMidiMap(); + showSharedChannels(); }; addAndMakeVisible(*box); keyboardChannels_.push_back(std::move(box)); @@ -1015,6 +1044,8 @@ void MidiPanel::resized() { r.removeFromTop(2); } + sharedNote_.setBounds(r.removeFromTop(kRow).reduced(12, 0)); + r.removeFromTop(kGap); auto row = r.removeFromTop(kRow); outputsLabel_.setBounds(row.removeFromLeft(120)); diff --git a/src/mp_ui/Settings.h b/src/mp_ui/Settings.h index c0baeb4..b65df3f 100644 --- a/src/mp_ui/Settings.h +++ b/src/mp_ui/Settings.h @@ -212,6 +212,11 @@ class MidiPanel : public juce::Component, private juce::Timer { // Everything the two boxes cannot say: key range, transpose, velocity // window, tracker action, short octave, debounce. std::vector> keyboardMore_; + // Which manuals share a channel, said out loud. Sharing one is allowed -- + // it is how a single keyboard plays two divisions -- but a player who did + // it by accident would otherwise hear two divisions and not know why. + juce::Label sharedNote_; + void showSharedChannels(); juce::ComboBox output_; juce::ToggleButton feedback_{"Send stop changes back to the console"}; juce::TextButton saveMap_{"Save mapping"}; diff --git a/tests/fixtures/m24.expression.Organ_Hauptwerk_xml b/tests/fixtures/m24.expression.Organ_Hauptwerk_xml index 1c5cf97..c79e51f 100644 --- a/tests/fixtures/m24.expression.Organ_Hauptwerk_xml +++ b/tests/fixtures/m24.expression.Organ_Hauptwerk_xml @@ -70,15 +70,16 @@ + 1 Swell Box 1 - 700 - 14000 - -20 - 0 2 @@ -91,8 +92,22 @@ routing has something to get wrong. Pipe 9102 named twice by different boxes would be an authoring error and is reported, not resolved. --> - 19101 - 19103 + 19101 + 20 + 700 + 7000 + 0 + 14000 + 20000 + + 19103 + 20 + 700 + 7000 + 0 + 14000 + 20000 + diff --git a/tests/test_core.cpp b/tests/test_core.cpp index 163a0d4..1d55c2c 100644 --- a/tests/test_core.cpp +++ b/tests/test_core.cpp @@ -243,6 +243,204 @@ class PalletSwitchTest final : public mp::test::Test { } }; +// Elements the corpus fills that the loader left unread (2026-09-20): +// Stop -> Rank through the hint, the swell box's per-pipe filter, the +// per-layer velocity response, and the per-layer tremulant trims. Each was +// verified present in shipped sets before it was parsed — see the field +// survey in the gap register's method — and each is silent when unread: the +// stop plays nothing, the box loses its real tone, every note is one level, +// every stop on a chest wobbles alike. +class LoaderMissingElementsTest final : public mp::test::Test { +public: + LoaderMissingElementsTest() + : Test("functional.odf.loader-missing-elements", Category::Functional) {} + + static std::string palletOrgan(bool pipeHasPallet) { + std::string odf = + "" + "<_General>" + "1" + "" + "" + "1Manual" + "61" + "36" + "1" + "" + "" + "1Great" + "" + "" + "1Principal 81" + "101" + "7" + "2Octave 41" + "8" + "" + "" + "28" + "36" + "61" + "" + "" + "7Principal" + "8Octave" + "" + "" + "717" + "60"; + if (pipeHasPallet) + odf += "555"; + odf += + "" + "818" + "60" + ""; + return odf; + } + + void run() override { + // --- the hint reaches a rank StopRank never named ------------------ + { + mp::OdfLoader l; + mp::OdfLoader::Options o; + mp::OrganModel m; + mp::OdfDiagnostics d; + MP_CHECK(l.loadFromXmlString(palletOrgan(false), "a.Organ_Hauptwerk_xml", + o, m, d), + "an organ whose stop names its rank by hint loads"); + const auto s1 = m.stops.find(1); + MP_CHECK(s1 != m.stops.end() && s1->second.ranks.size() == 1, + "the hintless stop gains a rank entry"); + MP_CHECK(s1 != m.stops.end() && s1->second.ranks[0].rankId == 7, + "and it names the hinted rank"); + MP_CHECK(s1 != m.stops.end() && + s1->second.ranks[0].firstMappedDivisionNote == 36 && + s1->second.ranks[0].numMappedNotes == 61, + "mapped over the division keyboard's own compass, not a guess"); + // A stop that DID declare StopRank rows keeps exactly those. + const auto s2 = m.stops.find(2); + MP_CHECK(s2 != m.stops.end() && s2->second.ranks.size() == 1 && + s2->second.ranks[0].rankId == 8, + "a stop with StopRank rows is untouched by the hint"); + } + + // --- the hint must not bypass pallet wiring ------------------------ + // On a pallet organ the switch network decides when the pipe speaks + // (key AND stop AND routing). A synthesized direct path would sound the + // rank whenever the key reached the division, whether the box's own + // wiring agrees or not. + { + mp::OdfLoader l; + mp::OdfLoader::Options o; + mp::OrganModel m; + mp::OdfDiagnostics d; + MP_CHECK(l.loadFromXmlString(palletOrgan(true), "a.Organ_Hauptwerk_xml", + o, m, d), + "a pallet-wired organ with a hint loads"); + const auto s1 = m.stops.find(1); + MP_CHECK(s1 != m.stops.end() && s1->second.ranks.empty(), + "a hinted rank reached by pallets keeps its wiring: no entry"); + } + + // --- the swell box's filter, from its pipes ------------------------ + { + const std::string odf = + "" + "<_General>" + "1" + "" + "" + "1Swell" + "9" + "" + "" + "9Swell shoe" + "" + "" + "111" + "10" + "400" + "0" + "4000" + "" + "112" + "10" + "800" + "0" + "8000" + "" + "113" + "10" + "1200" + "0" + "12000" + ""; + mp::OdfLoader l; + mp::OdfLoader::Options o; + mp::OrganModel m; + mp::OdfDiagnostics d; + MP_CHECK(l.loadFromXmlString(odf, "a.Organ_Hauptwerk_xml", o, m, d), + "an enclosure whose filter is stated on its pipes loads"); + const auto e = m.enclosures.find(1); + MP_CHECK(e != m.enclosures.end() && e->second.filterParamsFromPipes, + "the box knows its filter came from the pipes"); + // The figures are stated against each pipe's own pitch, so the box takes + // the upper quartile: the median would describe a pipe lower than most + // of what is heard, and an "open" box would sound permanently closed. + MP_CHECK(e != m.enclosures.end() && e->second.closedFilterHz == 1200.0, + "the closed cutoff is the upper quartile of the pipes' maxima"); + MP_CHECK(e != m.enclosures.end() && e->second.openFilterHz == 12000.0, + "the open cutoff is the upper quartile too"); + MP_CHECK(e != m.enclosures.end() && e->second.closedAttnDb == -10.0, + "the closed attenuation is insertion loss plus the extra at min"); + MP_CHECK(e != m.enclosures.end() && e->second.openAttnDb == 0.0, + "an open box takes the insertion loss off"); + } + + // --- the per-layer velocity response and tremulant trims ----------- + { + const std::string odf = + "" + "<_General>" + "1" + "" + "1" + "R" + "" + "101" + "60" + "" + "" + "10010" + "-12.5" + "" + "Y" + "-3" + "" + "50" + "" + ""; + mp::OdfLoader l; + mp::OdfLoader::Options o; + mp::OrganModel m; + mp::OdfDiagnostics d; + MP_CHECK(l.loadFromXmlString(odf, "a.Organ_Hauptwerk_xml", o, m, d), + "a layer with a velocity response loads"); + const auto r = m.ranks.find(1); + MP_CHECK(r != m.ranks.end() && r->second.pipes.size() == 1 && + r->second.pipes[0].layers.size() == 1, + "the layer is there"); + const mp::PipeLayer& lay = r->second.pipes[0].layers[0]; + MP_CHECK(lay.velSensMaxAttenDb == -12.5, + "the velocity ceiling is read raw — the sign is the file's"); + MP_CHECK(lay.invertVelocitySens, "the inversion flag is read"); + MP_CHECK(lay.tremAmpDepthAdjustDb == -3.0, "the tremulant amp trim is read"); + MP_CHECK(lay.tremPitchDepthAdjustPct == 50.0, "the tremulant pitch trim is read"); + } + } +}; + #ifdef MP_TEST_HAS_AUDIO #include "../src/mp_ui/BmpImage.h" @@ -6226,14 +6424,16 @@ class MidiChannelExclusiveTest final : public mp::test::Test { } }; -// The mapping found on a real machine, verbatim: three manuals on channel 1. -// Loaded as it was, two manuals were unplayable and every manual sounded the -// pedal. It must come back repaired, with the organ's own channels in charge. +// Manuals sharing a MIDI channel. Up to 0.5.3 every binding in such a +// mapping was dropped on load, which is what a rig with one keyboard and +// three manuals looks like -- and reported as #29: only two manuals could be +// mapped to one channel because the third undid them. Sharing is now kept: +// one keyboard playing several divisions is a coupler a player can build. class MidiMapRepairTest final : public mp::test::Test { public: MidiMapRepairTest() : Test("functional.midi.repair-saved-map", Category::Functional) {} void run() override { - const std::string corrupt = + const std::string shared = "# Masterpiece MIDI map\n" "# \n" "manual 3 1 0 127 0 1 127 0 0 0 any\n" @@ -6242,12 +6442,16 @@ class MidiMapRepairTest final : public mp::test::Test { const std::vector playable{1, 2, 3, 4}; mp::MidiMap map; - MP_CHECK(map.fromText(corrupt), "the damaged file still parses"); + MP_CHECK(map.fromText(shared), "the file parses"); MP_CHECK(map.keyboardBindings().size() == 3, "all three claims were read"); - MP_CHECK(map.repairKeyboardBindings(playable) == 3, - "every manual in the collision gives way"); - MP_CHECK(map.keyboardBindingsEmpty(), - "nothing ambiguous survives: the organ's own channels apply"); + MP_CHECK(map.repairKeyboardBindings(playable) == 0, + "three manuals on one channel is a choice, not a fault"); + MP_CHECK(map.keyboardBindings().size() == 3, "all three survive the load"); + + // And they all sound: one note on channel 1 reaches every one of them. + std::vector hits; + map.matchKeyboards(0, 1, 60, 100, 0.0, hits); + MP_CHECK(hits.size() == 3, "one key press plays all three manuals"); // A sound mapping is left exactly alone. mp::MidiMap good; @@ -6256,12 +6460,20 @@ class MidiMapRepairTest final : public mp::test::Test { MP_CHECK(good.repairKeyboardBindings(playable) == 0, "a good mapping is kept"); MP_CHECK(good.keyboardBindings().size() == 2, "both assignments survive"); - // A split keyboard -- one manual bound twice -- is legitimate. + // A split keyboard -- one manual bound twice over two halves. mp::MidiMap split; split.fromText("manual 2 1 36 60 0 1 127 0 0 0 any\n" "manual 2 1 61 96 0 1 127 0 0 0 any\n"); MP_CHECK(split.repairKeyboardBindings(playable) == 0, - "one manual bound twice is not a collision"); + "one manual bound over two halves is kept"); + + // The same binding listed twice only doubles the work of every note. + mp::MidiMap twice; + twice.fromText("manual 2 1 0 127 0 1 127 0 0 0 any\n" + "manual 2 1 0 127 0 1 127 0 0 0 any\n"); + MP_CHECK(twice.repairKeyboardBindings(playable) == 1, + "an identical duplicate goes"); + MP_CHECK(twice.keyboardBindings().size() == 1, "one copy stays"); // A manual this organ does not have is stale, whatever else is right. mp::MidiMap stale; @@ -6272,11 +6484,12 @@ class MidiMapRepairTest final : public mp::test::Test { stale.keyboardBindings().front().keyboardId == 2, "the valid one stays"); - // Round trip: the repaired map written out and read back needs no repair. + // Round trip: what was loaded is what is written back. mp::MidiMap again; again.fromText(map.toText()); MP_CHECK(again.repairKeyboardBindings(playable) == 0, - "a repaired file stays repaired"); + "a shared mapping survives being saved and read again"); + MP_CHECK(again.keyboardBindings().size() == 3, "all three come back"); } }; @@ -7422,6 +7635,7 @@ static LoaderRejectsUnknownTest g_rejectUnknown; static LoaderToleranceTest g_tolerance; static LoaderEmptyTableTest g_emptyTable; static PalletSwitchTest g_palletSwitch; +static LoaderMissingElementsTest g_loaderMissing; #ifdef MP_TEST_HAS_AUDIO static BmpImageTest g_bmpImage; #endif From 898b5494750c967b966b994cd75eb1a3fbf4e61d Mon Sep 17 00:00:00 2001 From: Bonni Date: Tue, 22 Sep 2026 20:58:51 -0300 Subject: [PATCH 2/3] Match an organ to a known library when its path leads nowhere Reported again in #12 after 0.5.3: both standard Hauptwerk folders were links, OrganDefinitions into Dropbox and the packages onto an external disk. The definition's real path shares no parent with its audio, so no search of the path can find it. Masterpiece now keeps a list of sample libraries -- folders holding an OrganInstallationPackages directory. It is seeded with the standard Hauptwerk location when that exists, and every load that works adds the folder it used. When a definition's own path leads to no packages, each known library is asked whether it holds the packages the definition names, and the first that does is used. Checking the package ids is what makes this safe: a library holding other organs is passed over rather than reporting every sample missing. The per-organ folder setting still overrides it. The matching lives in the core so it can be tested without a processor writing to real settings. The test builds two unrelated locations, as the reporter suggested, and needs no symlinks, so it runs everywhere. --- src/mp_audio/MasterpieceProcessor.cpp | 60 +++++++++++++++++++++++++++ src/mp_audio/MasterpieceProcessor.h | 14 +++++++ src/mp_core/OdfLoader.cpp | 31 ++++++++++++++ src/mp_core/OdfLoader.h | 9 ++++ tests/test_core.cpp | 58 ++++++++++++++++++++++++++ 5 files changed, 172 insertions(+) diff --git a/src/mp_audio/MasterpieceProcessor.cpp b/src/mp_audio/MasterpieceProcessor.cpp index 658ba4f..45edd27 100644 --- a/src/mp_audio/MasterpieceProcessor.cpp +++ b/src/mp_audio/MasterpieceProcessor.cpp @@ -1185,6 +1185,8 @@ bool MasterpieceProcessor::writeGlobalFile() const { text << "reopenlast " << (reopenLastOrgan_ ? 1 : 0) << "\n"; text << "loadticks " << (loadTicks_.load(std::memory_order_acquire) ? 1 : 0) << "\n"; + for (const auto& lib : libraries_) + text << "library " << lib.getFullPathName() << "\n"; if (cacheDir_.getFullPathName().isNotEmpty()) text << "cachedir " << cacheDir_.getFullPathName() << "\n"; if (lastOrgan_.getFullPathName().isNotEmpty()) @@ -1229,6 +1231,11 @@ bool MasterpieceProcessor::loadGlobalDefaults() { reopenLastOrgan_ = val.getIntValue() != 0; } else if (key == "loadticks") { loadTicks_.store(val.getIntValue() != 0, std::memory_order_release); + } else if (key == "library") { + const juce::File dir(val); + if (val.isNotEmpty() && + std::find(libraries_.begin(), libraries_.end(), dir) == libraries_.end()) + libraries_.push_back(dir); } else if (key == "cachedir") { // A path, taken whole: the sample cache can be gigabytes, and a player // with a small fast disk and a large slow one wants to choose which of @@ -1304,6 +1311,38 @@ void MasterpieceProcessor::setReopenLastOrgan(bool on) { writeGlobalFile(); } +// Does one of the known libraries hold the packages this organ names? The +// matching itself lives in the core, where it can be tested without a +// processor writing to anyone's settings. +juce::File MasterpieceProcessor::libraryHolding(const OrganModel& model) const { + std::vector roots; + for (const auto& lib : libraries_) roots.push_back(lib.getFullPathName().toStdString()); + const std::string found = mp::findLibraryHolding(roots, model); + return found.empty() ? juce::File() : juce::File(found); +} + +// The place a Hauptwerk installation keeps its libraries, so the first load +// after installing Masterpiece already knows where to look. Added only if it +// is really there. +void MasterpieceProcessor::seedSampleLibraries() { + const auto standard = + juce::File::getSpecialLocation(juce::File::userHomeDirectory) + .getChildFile("Hauptwerk") + .getChildFile("HauptwerkSampleSetsAndComponents"); + if (standard.getChildFile("OrganInstallationPackages").isDirectory() && + std::find(libraries_.begin(), libraries_.end(), standard) == libraries_.end()) + libraries_.push_back(standard); +} + +void MasterpieceProcessor::rememberSampleLibrary(const juce::File& root) { + if (!root.isDirectory()) return; + if (!root.getChildFile("OrganInstallationPackages").isDirectory()) return; + if (std::find(libraries_.begin(), libraries_.end(), root) != libraries_.end()) + return; + libraries_.push_back(root); + writeGlobalFile(); +} + juce::File MasterpieceProcessor::defaultCacheDirectory() { return juce::File::getSpecialLocation(juce::File::userApplicationDataDirectory) .getChildFile("Masterpiece") @@ -2517,6 +2556,24 @@ MasterpieceProcessor::LoadResult MasterpieceProcessor::loadOrgan( // Publish the model before the audio, so a note-on during loading resolves // pipes that simply have no sound yet rather than reading a half-built map. + // The definition parsed, so its package ids are known. If the root worked + // out from the path does not hold them -- both standard folders linked to + // unrelated drives is the reported case, and no path can bridge that -- ask + // the libraries this machine knows about. + if (!organRootOverride_.isDirectory()) { + const juce::File derived(opts.organRootDir); + const auto packages = derived.getChildFile("OrganInstallationPackages"); + if (!packages.isDirectory()) { + seedSampleLibraries(); + const juce::File lib = libraryHolding(loaded); + if (lib.isDirectory()) { + opts.organRootDir = lib.getFullPathName().toStdString(); + juce::Logger::writeToLog("load: packages found in a known library: " + + lib.getFullPathName()); + } + } + } + model_ = std::move(loaded); organRootDir_ = opts.organRootDir; loadedOdf_ = odfFile; @@ -2915,6 +2972,9 @@ MasterpieceProcessor::LoadResult MasterpieceProcessor::loadOrgan( // Only now, having got this far: an organ that failed to load is not one // worth reopening on the next start. setLastOrgan(odfFile); + // And the library it came from, so a definition moved away from its audio + // later can still be matched to it. + if (!graphicsOnly) rememberSampleLibrary(juce::File(organRootDir_)); result.stopsEngaged = 0; // Only now: starting the organ above moves switches on this thread, and diff --git a/src/mp_audio/MasterpieceProcessor.h b/src/mp_audio/MasterpieceProcessor.h index b53308e..6d2e9cc 100644 --- a/src/mp_audio/MasterpieceProcessor.h +++ b/src/mp_audio/MasterpieceProcessor.h @@ -682,6 +682,16 @@ class MasterpieceProcessor : public juce::AudioProcessor { // instance. Empty means work it out from the path, which is the usual case. // Set before loading; saved with the organ's other settings. juce::File organRootOverride() const { return organRootOverride_; } + + // The folders this machine keeps sample libraries in: anything holding an + // OrganInstallationPackages directory. A definition whose own path leads + // nowhere near its audio -- both standard folders linked to unrelated + // drives, which is what a Hauptwerk installation reorganised by hand looks + // like -- is found by asking each of these whether it holds the packages + // the definition names. Seeded with the standard location, added to by + // every load that works, and saved with the other machine-wide settings. + const std::vector& sampleLibraries() const { return libraries_; } + void rememberSampleLibrary(const juce::File& root); void setOrganRootOverride(const juce::File& dir) { organRootOverride_ = dir; } // Where the engine gets sample audio. Injected rather than owned, so the @@ -858,6 +868,10 @@ class MasterpieceProcessor : public juce::AudioProcessor { std::unordered_set engagedSwitches_; std::string organRootDir_; juce::File organRootOverride_; + std::vector libraries_; + // The root that holds the packages this model names, or an empty file. + juce::File libraryHolding(const OrganModel& model) const; + void seedSampleLibraries(); // A drawstop on the console IS a switch; clicking it must draw the stop, not // merely animate the picture. Built at load so the audio thread never // searches for it. diff --git a/src/mp_core/OdfLoader.cpp b/src/mp_core/OdfLoader.cpp index 04a597f..3e24e15 100644 --- a/src/mp_core/OdfLoader.cpp +++ b/src/mp_core/OdfLoader.cpp @@ -2021,6 +2021,37 @@ bool hasInstallationPackages(const std::filesystem::path& root) { } // namespace +std::string findLibraryHolding(const std::vector& roots, + const OrganModel& model) { + std::vector wanted; + for (const auto& [id, ref] : model.samples) { + (void)id; + if (ref.installationPackageId > 0 && + std::find(wanted.begin(), wanted.end(), ref.installationPackageId) == wanted.end()) + wanted.push_back(ref.installationPackageId); + if (wanted.size() >= 4) break; // four is plenty to tell libraries apart + } + if (wanted.empty()) return {}; + + std::error_code ec; + for (const auto& root : roots) { + const std::filesystem::path packages = + std::filesystem::path(root) / "OrganInstallationPackages"; + if (!std::filesystem::is_directory(packages, ec)) continue; + bool all = true; + for (Id id : wanted) { + std::string digits = std::to_string(id); + if (digits.size() < 6) digits.insert(0, 6 - digits.size(), '0'); + if (!std::filesystem::is_directory(packages / digits, ec)) { + all = false; + break; + } + } + if (all) return root; + } + return {}; +} + std::string deriveOrganRoot(const std::string& odfPath) { const std::filesystem::path odf(odfPath); const std::filesystem::path logicalRoot = organRootFrom(odf); diff --git a/src/mp_core/OdfLoader.h b/src/mp_core/OdfLoader.h index fe7c3ee..4eaaf1d 100644 --- a/src/mp_core/OdfLoader.h +++ b/src/mp_core/OdfLoader.h @@ -106,4 +106,13 @@ class OdfLoader { // packages at all is unaffected. std::string deriveOrganRoot(const std::string& odfPath); +// Of these library roots, the first whose OrganInstallationPackages holds the +// packages this model names, or an empty string. For a definition whose path +// cannot lead to its audio: both standard folders linked to unrelated drives +// leave no shared parent to find. Checking the package ids the definition +// names is what makes the answer right rather than a guess -- a library that +// holds other organs is passed over. +std::string findLibraryHolding(const std::vector& roots, + const OrganModel& model); + } // namespace mp diff --git a/tests/test_core.cpp b/tests/test_core.cpp index 1d55c2c..1a6b253 100644 --- a/tests/test_core.cpp +++ b/tests/test_core.cpp @@ -548,6 +548,63 @@ class BmpImageTest final : public mp::test::Test { #endif // MP_TEST_HAS_AUDIO +// Reported in #12 after 0.5.3: both standard Hauptwerk folders linked to +// two unrelated drives -- OrganDefinitions into Dropbox, the packages onto +// an external disk. The definition's real path leads nowhere near its audio, +// and no walk up from it can, so the organ is matched to a library this +// machine already knows by the package ids it names. +class LibraryMatchTest final : public mp::test::Test { +public: + LibraryMatchTest() : Test("functional.loader.library-match", Category::Functional) {} + void run() override { + namespace fs = std::filesystem; + std::error_code ec; + const fs::path base = fs::temp_directory_path(ec) / "mp_library_test_7e21"; + if (ec) return; + fs::remove_all(base, ec); + struct Cleanup { fs::path p; ~Cleanup(){ std::error_code e; std::filesystem::remove_all(p,e);} } cleanup{base}; + + // Two libraries: one holds this organ's package, the other someone else's. + const fs::path other = base / "SomeOtherDrive"; + const fs::path mine = base / "SanDisk"; + fs::create_directories(other / "OrganInstallationPackages" / "000009", ec); + fs::create_directories(mine / "OrganInstallationPackages" / "002213", ec); + // And the definition somewhere unrelated to both. + const fs::path defs = base / "Dropbox" / "Hauptwerk" / "OrganDefinitions"; + fs::create_directories(defs, ec); + MP_CHECK(!ec, "the layout can be built"); + + mp::OrganModel m; + mp::SampleRef s; + s.sampleId = 1; + s.installationPackageId = 2213; + s.fileName = "Pipe/036-C.wav"; + m.samples[1] = s; + + // Nothing in the definition's own path leads to the audio. + const std::string derived = + mp::deriveOrganRoot((defs / "Friesach.Organ_Hauptwerk_xml").string()); + MP_CHECK(!fs::is_directory(fs::path(derived) / "OrganInstallationPackages", ec), + "the definition's path genuinely leads to no packages"); + + // The library that holds its package is found; the other is passed over, + // whichever order they are listed in. + const std::string a = mp::findLibraryHolding({other.string(), mine.string()}, m); + const std::string b = mp::findLibraryHolding({mine.string(), other.string()}, m); + MP_CHECK(fs::equivalent(a, mine, ec) && fs::equivalent(b, mine, ec), + "the library holding the named package is the one chosen"); + + // A library that holds other organs only is never chosen. + MP_CHECK(mp::findLibraryHolding({other.string()}, m).empty(), + "no library is chosen when none holds the package"); + + // A definition that names no package cannot be matched by guesswork. + mp::OrganModel bare; + MP_CHECK(mp::findLibraryHolding({mine.string()}, bare).empty(), + "a definition naming no package matches nothing"); + } +}; + class EncryptedDetectionTest final : public mp::test::Test { public: EncryptedDetectionTest() @@ -7635,6 +7692,7 @@ static LoaderRejectsUnknownTest g_rejectUnknown; static LoaderToleranceTest g_tolerance; static LoaderEmptyTableTest g_emptyTable; static PalletSwitchTest g_palletSwitch; +static LibraryMatchTest g_libraryMatch; static LoaderMissingElementsTest g_loaderMissing; #ifdef MP_TEST_HAS_AUDIO static BmpImageTest g_bmpImage; From f2dd0a336a45b60ca7d45c2707597099c2343e1b Mon Sep 17 00:00:00 2001 From: Bonni Date: Tue, 22 Sep 2026 21:34:24 -0300 Subject: [PATCH 3/3] Make the room reverb affordable at small buffers, and truly stereo Reported in #29: Augustine's impulse responses gave "just scratching sound" through a Focusrite on ASIO. Rendered offline, the same IR is clean; timed, it is not. At a 64-sample block the convolution took the engine from 33 to 2.5 times real time, because JUCE's convolution partitioned a two-and-a-half-second room at the audio block size. At the 32-sample buffers ASIO users choose, with a full registration or a slower machine, that falls behind real time, and live that is a crackle. The convolution is now non-uniform: a short zero-latency head, the tail in large partitions. Measured on the Lemmer room at 32-sample blocks it runs at 6.7 times real time -- with twice the work, because of the next change. Hauptwerk impulse responses are four-channel true stereo: left to left, left to right, right to left, right to right. They were read as stereo, which kept the first two paths and dropped the other two. Each input now runs through its own pair, and the four are summed. A Hauptwerk IR package also ships one file per sample rate; the one matching the device is now used, instead of resampling another, which changes the apparent size of the room. And the IR is normalised by its energy across all four paths with one gain, so the room keeps its balance and sits at the level it did. --- src/mp_audio/Convolver.cpp | 126 ++++++++++++++++++++++++++++++------- src/mp_audio/Convolver.h | 20 +++++- tests/test_core.cpp | 37 +++++++++++ 3 files changed, 160 insertions(+), 23 deletions(-) diff --git a/src/mp_audio/Convolver.cpp b/src/mp_audio/Convolver.cpp index 6e2433e..ce159c7 100644 --- a/src/mp_audio/Convolver.cpp +++ b/src/mp_audio/Convolver.cpp @@ -1,38 +1,102 @@ #include "Convolver.h" +#include + namespace mp { void Convolver::prepare(const juce::dsp::ProcessSpec& spec) { - convolution_.prepare(spec); + fromLeft_.prepare(spec); + fromRight_.prepare(spec); + sampleRate_ = spec.sampleRate; // The dry path has to be kept whole so the mix is a real crossfade rather - // than "wet plus whatever survived". + // than "wet plus whatever survived". The second buffer carries the right + // input through its own engine for a true-stereo IR. dry_.setSize(static_cast(spec.numChannels), static_cast(spec.maximumBlockSize), false, true, true); + right_.setSize(2, static_cast(spec.maximumBlockSize), false, true, true); prepared_ = true; } -void Convolver::reset() { convolution_.reset(); } +void Convolver::reset() { + fromLeft_.reset(); + fromRight_.reset(); +} + +juce::File Convolver::fileForRate(const juce::File& irFile, double rate) { + if (rate <= 0.0) return irFile; + const juce::String name = irFile.getFileNameWithoutExtension(); + // "-48000Hz": replace the rate, keep everything before it. + const int dash = name.lastIndexOfChar('-'); + if (dash < 0 || !name.endsWithIgnoreCase("Hz")) return irFile; + const juce::String stem = name.substring(0, dash); + const auto sibling = irFile.getSiblingFile( + stem + "-" + juce::String(juce::roundToInt(rate)) + "Hz" + + irFile.getFileExtension()); + return sibling.existsAsFile() ? sibling : irFile; +} + +bool Convolver::loadImpulseResponse(const juce::File& chosen) { + if (!chosen.existsAsFile()) return false; + const juce::File irFile = fileForRate(chosen, sampleRate_); + + juce::AudioFormatManager formats; + formats.registerBasicFormats(); + std::unique_ptr reader(formats.createReaderFor(irFile)); + if (reader == nullptr || reader->lengthInSamples <= 0) return false; -bool Convolver::loadImpulseResponse(const juce::File& irFile) { - if (!irFile.existsAsFile()) return false; + const int channels = static_cast(reader->numChannels); + const int length = static_cast(reader->lengthInSamples); + juce::AudioBuffer all(channels, length); + reader->read(&all, 0, length, 0, true, true); - convolution_.loadImpulseResponse( - irFile, - // Keep the IR's own sample rate handling to JUCE; resampling a room - // impulse badly is audible as a change of room size. - juce::dsp::Convolution::Stereo::yes, - juce::dsp::Convolution::Trim::yes, - 0, // 0 = use the whole file - juce::dsp::Convolution::Normalise::yes); + // One gain for every path, so a true-stereo room keeps its own balance + // between them; normalising each engine separately would not. By energy, + // not by peak: a room's loudness is the whole of its tail, and a two-second + // tail normalised to its peak came out more than 20 dB too hot. The scale + // is the one JUCE's own normalisation uses, so a room sits where it did. + double energy = 0.0; + for (int ch = 0; ch < channels; ++ch) { + const float* p = all.getReadPointer(ch); + double e = 0.0; + for (int i = 0; i < length; ++i) e += static_cast(p[i]) * p[i]; + energy = juce::jmax(energy, e); + } + const float gain = energy > 0.0 ? static_cast(0.125 / std::sqrt(energy)) : 1.0f; + + auto pair = [&](int a, int b) { + juce::AudioBuffer ir(2, length); + ir.copyFrom(0, 0, all, juce::jmin(a, channels - 1), 0, length); + ir.copyFrom(1, 0, all, juce::jmin(b, channels - 1), 0, length); + ir.applyGain(gain); + return ir; + }; + + // Four channels is true stereo, in Hauptwerk's order: left input to the + // left and right outputs, then right input to the left and right outputs. + // Two is an ordinary stereo IR; one is mono, used for both sides. + trueStereo_ = channels >= 4; + const double rate = reader->sampleRate; + using C = juce::dsp::Convolution; + if (trueStereo_) { + fromLeft_.loadImpulseResponse(pair(0, 1), rate, C::Stereo::yes, C::Trim::yes, + C::Normalise::no); + fromRight_.loadImpulseResponse(pair(2, 3), rate, C::Stereo::yes, C::Trim::yes, + C::Normalise::no); + } else { + fromLeft_.loadImpulseResponse(pair(0, channels > 1 ? 1 : 0), rate, C::Stereo::yes, + C::Trim::yes, C::Normalise::no); + } - irName_ = irFile.getFileNameWithoutExtension(); + irName_ = chosen.getFileNameWithoutExtension(); loaded_ = true; return true; } void Convolver::clear() { - convolution_.reset(); + fromLeft_.reset(); + fromRight_.reset(); loaded_ = false; + trueStereo_ = false; irName_ = {}; } @@ -44,17 +108,35 @@ void Convolver::process(juce::AudioBuffer& buffer) { if (numCh <= 0 || numSamples <= 0) return; if (mix_ <= 0.0f) return; - // Hold the dry signal aside. dry_ was sized at prepare(); a host handing us - // a larger block than it promised falls back to bypass rather than - // allocating on the audio thread. - if (dry_.getNumChannels() < numCh || dry_.getNumSamples() < numSamples) + // Hold the dry signal aside. The buffers were sized at prepare(); a host + // handing us a larger block than it promised falls back to bypass rather + // than allocating on the audio thread. + if (dry_.getNumChannels() < numCh || dry_.getNumSamples() < numSamples || + right_.getNumSamples() < numSamples) return; for (int ch = 0; ch < numCh; ++ch) dry_.copyFrom(ch, 0, buffer, ch, 0, numSamples); - juce::dsp::AudioBlock block(buffer); - juce::dsp::ProcessContextReplacing ctx(block); - convolution_.process(ctx); + if (trueStereo_ && numCh >= 2) { + // The right input, doubled, through its own engine gives right-to-left + // and right-to-right. The left input, doubled, through the other gives + // left-to-left and left-to-right. Their sum is the room. + right_.copyFrom(0, 0, buffer, 1, 0, numSamples); + right_.copyFrom(1, 0, buffer, 1, 0, numSamples); + buffer.copyFrom(1, 0, buffer, 0, 0, numSamples); + + juce::dsp::AudioBlock leftBlock(buffer.getArrayOfWritePointers(), 2, + static_cast(numSamples)); + fromLeft_.process(juce::dsp::ProcessContextReplacing(leftBlock)); + juce::dsp::AudioBlock rightBlock(right_.getArrayOfWritePointers(), 2, + static_cast(numSamples)); + fromRight_.process(juce::dsp::ProcessContextReplacing(rightBlock)); + buffer.addFrom(0, 0, right_, 0, 0, numSamples); + buffer.addFrom(1, 0, right_, 1, 0, numSamples); + } else { + juce::dsp::AudioBlock block(buffer); + fromLeft_.process(juce::dsp::ProcessContextReplacing(block)); + } // Equal-gain crossfade. The wet signal is the same material through a room, // so it correlates with the dry: equal-power would push the level up. diff --git a/src/mp_audio/Convolver.h b/src/mp_audio/Convolver.h index 3e3dd4e..893a1b6 100644 --- a/src/mp_audio/Convolver.h +++ b/src/mp_audio/Convolver.h @@ -41,8 +41,26 @@ class Convolver { // so it is safe to call unconditionally from processBlock. void process(juce::AudioBuffer& buffer); + // A Hauptwerk impulse-response package ships the same room once per + // sample rate, as "-44100Hz.wav", "-48000Hz.wav" and so on. Given any + // one of them, the sibling recorded at `rate`, or the file itself if there + // is none. Resampling a room is audible as a change of its size. + static juce::File fileForRate(const juce::File& irFile, double rate); + private: - juce::dsp::Convolution convolution_; + // Non-uniform partitioning: a short head at the audio block size keeps the + // reverb at zero latency, and the long tail is done in large partitions. + // Uniform partitioning at an ASIO-sized block (32 or 64 samples) spent + // most of a core on a two-second room, and live that is a crackle -- + // reported as "just scratching sound" in #29. + static constexpr int kHeadSize = 256; + // Two engines, for a true-stereo IR: one carries the left input to both + // outputs, the other the right. A two-channel IR uses only the first. + juce::dsp::Convolution fromLeft_{juce::dsp::Convolution::NonUniform{kHeadSize}}; + juce::dsp::Convolution fromRight_{juce::dsp::Convolution::NonUniform{kHeadSize}}; + bool trueStereo_ = false; + double sampleRate_ = 0.0; + juce::AudioBuffer right_; juce::AudioBuffer dry_; bool enabled_ = false; bool loaded_ = false; diff --git a/tests/test_core.cpp b/tests/test_core.cpp index 1a6b253..8172026 100644 --- a/tests/test_core.cpp +++ b/tests/test_core.cpp @@ -443,6 +443,42 @@ class LoaderMissingElementsTest final : public mp::test::Test { #ifdef MP_TEST_HAS_AUDIO #include "../src/mp_ui/BmpImage.h" +#include "../src/mp_audio/Convolver.h" + +// A Hauptwerk impulse-response package ships one room at several sample +// rates. Picking the file recorded at the device's rate avoids resampling the +// room, which is audible as a change of its size. +class IrRateTest final : public mp::test::Test { +public: + IrRateTest() : Test("functional.dsp.ir-rate-sibling", Category::Functional) {} + void run() override { + namespace fs = std::filesystem; + std::error_code ec; + const fs::path dir = fs::temp_directory_path(ec) / "mp_ir_rate_test_3b9d"; + if (ec) return; + fs::remove_all(dir, ec); + fs::create_directories(dir, ec); + if (ec) return; + struct Cleanup { fs::path p; ~Cleanup(){ std::error_code e; std::filesystem::remove_all(p,e);} } cleanup{dir}; + for (const char* rate : {"44100", "48000", "96000"}) { + std::ofstream f(dir / ("Room, omni {id}-" + std::string(rate) + "Hz.wav")); + f << "x"; + } + const juce::File given((dir / "Room, omni {id}-44100Hz.wav").string()); + MP_CHECK(mp::Convolver::fileForRate(given, 48000.0).getFileName() == + "Room, omni {id}-48000Hz.wav", + "the sibling at the device's rate is chosen"); + MP_CHECK(mp::Convolver::fileForRate(given, 96000.0).getFileName() == + "Room, omni {id}-96000Hz.wav", + "and at 96 kHz"); + MP_CHECK(mp::Convolver::fileForRate(given, 88200.0) == given, + "with no file at that rate, the one chosen is kept"); + + const juce::File plain((dir / "plain.wav").string()); + MP_CHECK(mp::Convolver::fileForRate(plain, 48000.0) == plain, + "a file not named by rate is used as it is"); + } +}; // Console artwork in BMP. JUCE reads PNG, JPEG and GIF; the older Hauptwerk // sets paint their consoles in BMP, and those came out black (issue #24). @@ -7696,6 +7732,7 @@ static LibraryMatchTest g_libraryMatch; static LoaderMissingElementsTest g_loaderMissing; #ifdef MP_TEST_HAS_AUDIO static BmpImageTest g_bmpImage; +static IrRateTest g_irRate; #endif static ConditionSenseTest g_conditionSense; static EncryptedDetectionTest g_encrypted;