From f0f4c34ad8e52f2624b7db608d87c158d38aaee2 Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Sun, 17 May 2020 21:42:57 +0200 Subject: [PATCH 1/8] Numbering of regions and voices --- src/sfizz/Region.h | 14 ++++++++++-- src/sfizz/Synth.cpp | 5 +++-- src/sfizz/Voice.cpp | 4 ++-- src/sfizz/Voice.h | 12 +++++++++- tests/RegionActivationT.cpp | 2 +- tests/RegionT.cpp | 4 ++-- tests/RegionTriggersT.cpp | 4 ++-- tests/RegionValueComputationsT.cpp | 36 +++++++++++++++--------------- 8 files changed, 51 insertions(+), 30 deletions(-) diff --git a/src/sfizz/Region.h b/src/sfizz/Region.h index 5ce49ff8..9b6e01b2 100644 --- a/src/sfizz/Region.h +++ b/src/sfizz/Region.h @@ -35,8 +35,8 @@ namespace sfz { * */ struct Region { - Region(const MidiState& midiState, absl::string_view defaultPath = "") - : midiState(midiState), defaultPath(std::move(defaultPath)) + Region(int regionNumber, const MidiState& midiState, absl::string_view defaultPath = "") + : regionNumber(regionNumber), midiState(midiState), defaultPath(std::move(defaultPath)) { ccSwitched.set(); @@ -46,6 +46,14 @@ struct Region { Region(const Region&) = default; ~Region() = default; + /** + * @brief Get the number which identifies this region, also its index + */ + int getIdNumber() const noexcept + { + return regionNumber; + } + /** * @brief Triggers on release? * @@ -238,6 +246,8 @@ struct Region { */ float getGainToEffectBus(unsigned number) const noexcept; + const int regionNumber {}; + // Sound source: sample playback FileId sampleId {}; // Sample int sampleQuality { Default::sampleQuality }; diff --git a/src/sfizz/Synth.cpp b/src/sfizz/Synth.cpp index 2718a1e1..b46a18b6 100644 --- a/src/sfizz/Synth.cpp +++ b/src/sfizz/Synth.cpp @@ -95,7 +95,8 @@ void sfz::Synth::onParseWarning(const SourceRange& range, const std::string& mes void sfz::Synth::buildRegion(const std::vector& regionOpcodes) { - auto lastRegion = absl::make_unique(resources.midiState, defaultPath); + int regionNumber = static_cast(regions.size()); + auto lastRegion = absl::make_unique(regionNumber, resources.midiState, defaultPath); auto parseOpcodes = [&](const std::vector& opcodes) { for (auto& opcode : opcodes) { @@ -1094,7 +1095,7 @@ void sfz::Synth::resetVoices(int numVoices) voices.reserve(numVoices); for (int i = 0; i < numVoices; ++i) - voices.push_back(absl::make_unique(resources)); + voices.push_back(absl::make_unique(i, resources)); voiceViewArray.clear(); voiceViewArray.reserve(numVoices); diff --git a/src/sfizz/Voice.cpp b/src/sfizz/Voice.cpp index 6ae90db2..2f3ebe05 100644 --- a/src/sfizz/Voice.cpp +++ b/src/sfizz/Voice.cpp @@ -14,8 +14,8 @@ #include "Interpolators.h" #include "absl/algorithm/container.h" -sfz::Voice::Voice(sfz::Resources& resources) -: resources(resources) +sfz::Voice::Voice(int voiceNumber, sfz::Resources& resources) +: voiceNumber(voiceNumber), resources(resources) { filters.reserve(config::filtersPerVoice); equalizers.reserve(config::eqsPerVoice); diff --git a/src/sfizz/Voice.h b/src/sfizz/Voice.h index a709eb58..0c7937c7 100644 --- a/src/sfizz/Voice.h +++ b/src/sfizz/Voice.h @@ -32,14 +32,22 @@ public: /** * @brief Construct a new voice with the midistate singleton * + * @param voiceNumber * @param midiState */ - Voice(Resources& resources); + Voice(int voiceNumber, Resources& resources); enum class TriggerType { NoteOn, NoteOff, CC }; + /** + * @brief Get the number which identifies this voice, its index + */ + int getIdNumber() const noexcept + { + return voiceNumber; + } /** * @brief Change the sample rate of the voice. This is used to compute all * pitch related transformations so it needs to be propagated from the synth @@ -270,6 +278,8 @@ private: void setupOscillatorUnison(); void updateChannelPowers(AudioSpan buffer); + const int voiceNumber {}; + Region* region { nullptr }; enum class State { diff --git a/tests/RegionActivationT.cpp b/tests/RegionActivationT.cpp index e65934b7..41990934 100644 --- a/tests/RegionActivationT.cpp +++ b/tests/RegionActivationT.cpp @@ -13,7 +13,7 @@ using namespace sfz::literals; TEST_CASE("Region activation", "Region tests") { sfz::MidiState midiState; - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "sample", "*sine" }); SECTION("Basic state") diff --git a/tests/RegionT.cpp b/tests/RegionT.cpp index db2c9511..c683cd96 100644 --- a/tests/RegionT.cpp +++ b/tests/RegionT.cpp @@ -14,7 +14,7 @@ using namespace sfz::literals; TEST_CASE("[Region] Parsing opcodes") { sfz::MidiState midiState; - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; SECTION("sample") { @@ -1672,7 +1672,7 @@ TEST_CASE("[Region] Parsing opcodes") TEST_CASE("[Region] Non-conforming floating point values in integer opcodes") { sfz::MidiState midiState; - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "offset", "2014.5" }); REQUIRE(region.offset == 2014); region.parseOpcode({ "pitch_keytrack", "-2.1" }); diff --git a/tests/RegionTriggersT.cpp b/tests/RegionTriggersT.cpp index 75f5d9b5..4ee56cde 100644 --- a/tests/RegionTriggersT.cpp +++ b/tests/RegionTriggersT.cpp @@ -13,7 +13,7 @@ using namespace sfz::literals; TEST_CASE("Basic triggers", "Region triggers") { sfz::MidiState midiState; - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "sample", "*sine" }); SECTION("key") @@ -130,7 +130,7 @@ TEST_CASE("Basic triggers", "Region triggers") TEST_CASE("Legato triggers", "Region triggers") { sfz::MidiState midiState; - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "sample", "*sine" }); SECTION("First note playing") { diff --git a/tests/RegionValueComputationsT.cpp b/tests/RegionValueComputationsT.cpp index e6547b01..50deb231 100644 --- a/tests/RegionValueComputationsT.cpp +++ b/tests/RegionValueComputationsT.cpp @@ -17,7 +17,7 @@ constexpr int numRandomTests { 64 }; TEST_CASE("[Region] Crossfade in on key") { sfz::MidiState midiState; - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "sample", "*sine" }); region.parseOpcode({ "xfin_lokey", "1" }); region.parseOpcode({ "xfin_hikey", "3" }); @@ -29,7 +29,7 @@ TEST_CASE("[Region] Crossfade in on key") TEST_CASE("[Region] Crossfade in on key - 2") { sfz::MidiState midiState; - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "sample", "*sine" }); region.parseOpcode({ "xfin_lokey", "1" }); region.parseOpcode({ "xfin_hikey", "5" }); @@ -44,7 +44,7 @@ TEST_CASE("[Region] Crossfade in on key - 2") TEST_CASE("[Region] Crossfade in on key - gain") { sfz::MidiState midiState; - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "sample", "*sine" }); region.parseOpcode({ "xfin_lokey", "1" }); region.parseOpcode({ "xfin_hikey", "5" }); @@ -59,7 +59,7 @@ TEST_CASE("[Region] Crossfade in on key - gain") TEST_CASE("[Region] Crossfade out on key") { sfz::MidiState midiState; - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "sample", "*sine" }); region.parseOpcode({ "xfout_lokey", "51" }); region.parseOpcode({ "xfout_hikey", "55" }); @@ -75,7 +75,7 @@ TEST_CASE("[Region] Crossfade out on key") TEST_CASE("[Region] Crossfade out on key - gain") { sfz::MidiState midiState; - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "sample", "*sine" }); region.parseOpcode({ "xfout_lokey", "51" }); region.parseOpcode({ "xfout_hikey", "55" }); @@ -92,7 +92,7 @@ TEST_CASE("[Region] Crossfade out on key - gain") TEST_CASE("[Region] Crossfade in on velocity") { sfz::MidiState midiState; - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "sample", "*sine" }); region.parseOpcode({ "xfin_lovel", "20" }); region.parseOpcode({ "xfin_hivel", "24" }); @@ -109,7 +109,7 @@ TEST_CASE("[Region] Crossfade in on velocity") TEST_CASE("[Region] Crossfade in on vel - gain") { sfz::MidiState midiState; - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "sample", "*sine" }); region.parseOpcode({ "xfin_lovel", "20" }); region.parseOpcode({ "xfin_hivel", "24" }); @@ -127,7 +127,7 @@ TEST_CASE("[Region] Crossfade in on vel - gain") TEST_CASE("[Region] Crossfade out on vel") { sfz::MidiState midiState; - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "sample", "*sine" }); region.parseOpcode({ "xfout_lovel", "51" }); region.parseOpcode({ "xfout_hivel", "55" }); @@ -144,7 +144,7 @@ TEST_CASE("[Region] Crossfade out on vel") TEST_CASE("[Region] Crossfade out on vel - gain") { sfz::MidiState midiState; - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "sample", "*sine" }); region.parseOpcode({ "xfout_lovel", "51" }); region.parseOpcode({ "xfout_hivel", "55" }); @@ -162,7 +162,7 @@ TEST_CASE("[Region] Crossfade out on vel - gain") TEST_CASE("[Region] Crossfade in on CC") { sfz::MidiState midiState; - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "sample", "*sine" }); region.parseOpcode({ "xfin_locc24", "20" }); region.parseOpcode({ "xfin_hicc24", "24" }); @@ -186,7 +186,7 @@ TEST_CASE("[Region] Crossfade in on CC") TEST_CASE("[Region] Crossfade in on CC - gain") { sfz::MidiState midiState; - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "sample", "*sine" }); region.parseOpcode({ "xfin_locc24", "20" }); region.parseOpcode({ "xfin_hicc24", "24" }); @@ -210,7 +210,7 @@ TEST_CASE("[Region] Crossfade in on CC - gain") TEST_CASE("[Region] Crossfade out on CC") { sfz::MidiState midiState; - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "sample", "*sine" }); region.parseOpcode({ "xfout_locc24", "20" }); region.parseOpcode({ "xfout_hicc24", "24" }); @@ -234,7 +234,7 @@ TEST_CASE("[Region] Crossfade out on CC") TEST_CASE("[Region] Crossfade out on CC - gain") { sfz::MidiState midiState; - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "sample", "*sine" }); region.parseOpcode({ "xfout_locc24", "20" }); region.parseOpcode({ "xfout_hicc24", "24" }); @@ -259,7 +259,7 @@ TEST_CASE("[Region] Crossfade out on CC - gain") TEST_CASE("[Region] Velocity bug for extreme values - veltrack at 0") { sfz::MidiState midiState; - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "sample", "*sine" }); region.parseOpcode({ "amp_veltrack", "0" }); REQUIRE(region.getNoteGain(64, 127_norm) == 1.0_a); @@ -270,7 +270,7 @@ TEST_CASE("[Region] Velocity bug for extreme values - veltrack at 0") TEST_CASE("[Region] Velocity bug for extreme values - positive veltrack") { sfz::MidiState midiState; - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "sample", "*sine" }); region.parseOpcode({ "amp_veltrack", "100" }); REQUIRE(region.getNoteGain(64, 127_norm) == 1.0_a); @@ -280,7 +280,7 @@ TEST_CASE("[Region] Velocity bug for extreme values - positive veltrack") TEST_CASE("[Region] Velocity bug for extreme values - negative veltrack") { sfz::MidiState midiState; - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "sample", "*sine" }); region.parseOpcode({ "amp_veltrack", "-100" }); REQUIRE(region.getNoteGain(64, 127_norm) == Approx(0.0).margin(0.0001)); @@ -291,7 +291,7 @@ TEST_CASE("[Region] rt_decay") { sfz::MidiState midiState; midiState.setSampleRate(1000); - sfz::Region region { midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "sample", "*sine" }); region.parseOpcode({ "trigger", "release" }); region.parseOpcode({ "rt_decay", "10" }); @@ -311,7 +311,7 @@ TEST_CASE("[Region] rt_decay") TEST_CASE("[Region] Base delay") { sfz::MidiState midiState; - sfz::Region region{ midiState }; + sfz::Region region { 0, midiState }; region.parseOpcode({ "sample", "*sine" }); region.parseOpcode({ "delay", "10" }); REQUIRE( region.getDelay() == 10.0f ); From 2c3314d0adbc0e5acb0fad65fb0c5d5f5a569898 Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Mon, 18 May 2020 11:59:31 +0200 Subject: [PATCH 2/8] Add a listener of voice state changes --- src/sfizz/Synth.cpp | 14 ++++++++++++-- src/sfizz/Synth.h | 8 +++++++- src/sfizz/Voice.cpp | 16 +++++++++++++--- src/sfizz/Voice.h | 26 ++++++++++++++++++++++---- 4 files changed, 54 insertions(+), 10 deletions(-) diff --git a/src/sfizz/Synth.cpp b/src/sfizz/Synth.cpp index b46a18b6..2c889bf0 100644 --- a/src/sfizz/Synth.cpp +++ b/src/sfizz/Synth.cpp @@ -44,6 +44,13 @@ sfz::Synth::~Synth() resources.filePool.emptyFileLoadingQueues(); } +void sfz::Synth::onVoiceStateChanged(int idNumber, Voice::State state) +{ + (void)idNumber; + (void)state; + DBG("Voice " << idNumber << ": state " << static_cast(state)); +} + void sfz::Synth::onParseFullBlock(const std::string& header, const std::vector& members) { switch (hash(header)) { @@ -1094,8 +1101,11 @@ void sfz::Synth::resetVoices(int numVoices) voices.clear(); voices.reserve(numVoices); - for (int i = 0; i < numVoices; ++i) - voices.push_back(absl::make_unique(i, resources)); + for (int i = 0; i < numVoices; ++i) { + auto voice = absl::make_unique(i, resources); + voice->setStateListener(this); + voices.emplace_back(std::move(voice)); + } voiceViewArray.clear(); voiceViewArray.reserve(numVoices); diff --git a/src/sfizz/Synth.h b/src/sfizz/Synth.h index 0819880e..883fedf4 100644 --- a/src/sfizz/Synth.h +++ b/src/sfizz/Synth.h @@ -60,7 +60,7 @@ namespace sfz { * The jack_client.cpp file contains examples of the most classical usage of the * synth and can be used as a reference. */ -class Synth final : public Parser::Listener { +class Synth final : public Voice::StateListener, public Parser::Listener { public: /** * @brief Construct a new Synth object with no voices. If you want sound @@ -493,6 +493,12 @@ public: */ const std::vector& getCCLabels() const noexcept { return ccLabels; } +protected: + /** + * @brief The voice callback which is called during a change of state. + */ + void onVoiceStateChanged(int idNumber, Voice::State state) override; + protected: /** * @brief The parser callback; this is called by the parent object each time diff --git a/src/sfizz/Voice.cpp b/src/sfizz/Voice.cpp index 2f3ebe05..fc65892c 100644 --- a/src/sfizz/Voice.cpp +++ b/src/sfizz/Voice.cpp @@ -15,7 +15,7 @@ #include "absl/algorithm/container.h" sfz::Voice::Voice(int voiceNumber, sfz::Resources& resources) -: voiceNumber(voiceNumber), resources(resources) +: voiceNumber(voiceNumber), stateListener(nullptr), resources(resources) { filters.reserve(config::filtersPerVoice); equalizers.reserve(config::eqsPerVoice); @@ -36,7 +36,7 @@ void sfz::Voice::startVoice(Region* region, int delay, int number, float value, triggerValue = value; this->region = region; - state = State::playing; + switchState(State::playing); ASSERT(delay >= 0); if (delay < 0) @@ -647,7 +647,7 @@ bool sfz::Voice::checkOffGroup(int delay, uint32_t group) noexcept void sfz::Voice::reset() noexcept { - state = State::idle; + switchState(State::idle); region = nullptr; currentPromise.reset(); sourcePosition = 0; @@ -766,3 +766,13 @@ void sfz::Voice::updateChannelPowers(AudioSpan buffer) channelEnvelopeFilters[i].tickLowpass(std::abs(input[s])); } } + + +void sfz::Voice::switchState(State s) +{ + if (s != state) { + state = s; + if (stateListener) + stateListener->onVoiceStateChanged(voiceNumber, s); + } +} diff --git a/src/sfizz/Voice.h b/src/sfizz/Voice.h index 0c7937c7..a9b347d2 100644 --- a/src/sfizz/Voice.h +++ b/src/sfizz/Voice.h @@ -48,6 +48,22 @@ public: { return voiceNumber; } + + enum class State { + idle, + playing + }; + + class StateListener { + public: + virtual void onVoiceStateChanged(int /*idNumber*/, State /*state*/) {} + }; + + /** + * @brief Sets the listener which is called when the voice state changes. + */ + void setStateListener(StateListener *l) noexcept { stateListener = l; } + /** * @brief Change the sample rate of the voice. This is used to compute all * pitch related transformations so it needs to be propagated from the synth @@ -278,14 +294,16 @@ private: void setupOscillatorUnison(); void updateChannelPowers(AudioSpan buffer); + /** + * @brief Modify the voice state and notify any listeners. + */ + void switchState(State s); + const int voiceNumber {}; + StateListener* stateListener = nullptr; Region* region { nullptr }; - enum class State { - idle, - playing - }; State state { State::idle }; bool noteIsOff { false }; From c239629be1079a2d74b9c6ac83ac3783ad581df7 Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Sat, 30 May 2020 17:54:21 +0200 Subject: [PATCH 3/8] Initialize id number to -1, do not consider it as index --- src/sfizz/Region.h | 4 ++-- src/sfizz/Voice.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sfizz/Region.h b/src/sfizz/Region.h index 9b6e01b2..b18b7bd5 100644 --- a/src/sfizz/Region.h +++ b/src/sfizz/Region.h @@ -47,7 +47,7 @@ struct Region { ~Region() = default; /** - * @brief Get the number which identifies this region, also its index + * @brief Get the number which identifies this region */ int getIdNumber() const noexcept { @@ -246,7 +246,7 @@ struct Region { */ float getGainToEffectBus(unsigned number) const noexcept; - const int regionNumber {}; + const int regionNumber { -1 }; // Sound source: sample playback FileId sampleId {}; // Sample diff --git a/src/sfizz/Voice.h b/src/sfizz/Voice.h index a9b347d2..59058c59 100644 --- a/src/sfizz/Voice.h +++ b/src/sfizz/Voice.h @@ -42,7 +42,7 @@ public: CC }; /** - * @brief Get the number which identifies this voice, its index + * @brief Get the number which identifies this voice */ int getIdNumber() const noexcept { @@ -299,7 +299,7 @@ private: */ void switchState(State s); - const int voiceNumber {}; + const int voiceNumber { -1 }; StateListener* stateListener = nullptr; Region* region { nullptr }; From e07912f7ce5b01aa6c1df525a11994599e61b790 Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Thu, 11 Jun 2020 23:50:41 +0200 Subject: [PATCH 4/8] Implement the identifier in a better and more safe way --- src/sfizz/NumericId.h | 31 +++++++++++++++++++++++++++++++ src/sfizz/Region.h | 9 +++++---- src/sfizz/Synth.cpp | 6 +++--- src/sfizz/Synth.h | 2 +- src/sfizz/Voice.cpp | 4 ++-- src/sfizz/Voice.h | 12 +++++++----- 6 files changed, 49 insertions(+), 15 deletions(-) create mode 100644 src/sfizz/NumericId.h diff --git a/src/sfizz/NumericId.h b/src/sfizz/NumericId.h new file mode 100644 index 00000000..1d975ec0 --- /dev/null +++ b/src/sfizz/NumericId.h @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: BSD-2-Clause + +// This code is part of the sfizz library and is licensed under a BSD 2-clause +// license. You should have receive a LICENSE.md file along with the code. +// If not, contact the sfizz maintainers at https://github.com/sfztools/sfizz + +#pragma once + +/** + * @brief Numeric identifier + * + * It is a generic numeric identifier. The template wrapper serves to enforce a + * stronger compile-time check, such that one kind of identifier can't be + * mistaken for another kind, or for an unrelated integer such as an index. + */ +template +struct NumericId { + constexpr NumericId() = default; + + explicit constexpr NumericId(int number) + : number(number) + { + } + + constexpr bool valid() const noexcept + { + return number != -1; + } + + const int number = -1; +}; diff --git a/src/sfizz/Region.h b/src/sfizz/Region.h index b18b7bd5..ad81f97e 100644 --- a/src/sfizz/Region.h +++ b/src/sfizz/Region.h @@ -16,6 +16,7 @@ #include "AudioBuffer.h" #include "MidiState.h" #include "FileId.h" +#include "NumericId.h" #include "absl/types/optional.h" #include #include @@ -36,7 +37,7 @@ namespace sfz { */ struct Region { Region(int regionNumber, const MidiState& midiState, absl::string_view defaultPath = "") - : regionNumber(regionNumber), midiState(midiState), defaultPath(std::move(defaultPath)) + : id{regionNumber}, midiState(midiState), defaultPath(std::move(defaultPath)) { ccSwitched.set(); @@ -49,9 +50,9 @@ struct Region { /** * @brief Get the number which identifies this region */ - int getIdNumber() const noexcept + NumericId getId() const noexcept { - return regionNumber; + return id; } /** @@ -246,7 +247,7 @@ struct Region { */ float getGainToEffectBus(unsigned number) const noexcept; - const int regionNumber { -1 }; + const NumericId id; // Sound source: sample playback FileId sampleId {}; // Sample diff --git a/src/sfizz/Synth.cpp b/src/sfizz/Synth.cpp index 2c889bf0..c609d3c5 100644 --- a/src/sfizz/Synth.cpp +++ b/src/sfizz/Synth.cpp @@ -44,11 +44,11 @@ sfz::Synth::~Synth() resources.filePool.emptyFileLoadingQueues(); } -void sfz::Synth::onVoiceStateChanged(int idNumber, Voice::State state) +void sfz::Synth::onVoiceStateChanged(NumericId id, Voice::State state) { - (void)idNumber; + (void)id; (void)state; - DBG("Voice " << idNumber << ": state " << static_cast(state)); + DBG("Voice " << id.number << ": state " << static_cast(state)); } void sfz::Synth::onParseFullBlock(const std::string& header, const std::vector& members) diff --git a/src/sfizz/Synth.h b/src/sfizz/Synth.h index 883fedf4..d04a5c96 100644 --- a/src/sfizz/Synth.h +++ b/src/sfizz/Synth.h @@ -497,7 +497,7 @@ protected: /** * @brief The voice callback which is called during a change of state. */ - void onVoiceStateChanged(int idNumber, Voice::State state) override; + void onVoiceStateChanged(NumericId idNumber, Voice::State state) override; protected: /** diff --git a/src/sfizz/Voice.cpp b/src/sfizz/Voice.cpp index fc65892c..37dafdde 100644 --- a/src/sfizz/Voice.cpp +++ b/src/sfizz/Voice.cpp @@ -15,7 +15,7 @@ #include "absl/algorithm/container.h" sfz::Voice::Voice(int voiceNumber, sfz::Resources& resources) -: voiceNumber(voiceNumber), stateListener(nullptr), resources(resources) +: id{voiceNumber}, stateListener(nullptr), resources(resources) { filters.reserve(config::filtersPerVoice); equalizers.reserve(config::eqsPerVoice); @@ -773,6 +773,6 @@ void sfz::Voice::switchState(State s) if (s != state) { state = s; if (stateListener) - stateListener->onVoiceStateChanged(voiceNumber, s); + stateListener->onVoiceStateChanged(id, s); } } diff --git a/src/sfizz/Voice.h b/src/sfizz/Voice.h index 59058c59..515feb12 100644 --- a/src/sfizz/Voice.h +++ b/src/sfizz/Voice.h @@ -14,6 +14,7 @@ #include "AudioSpan.h" #include "LeakDetector.h" #include "OnePoleFilter.h" +#include "NumericId.h" #include "absl/types/span.h" #include #include @@ -41,12 +42,13 @@ public: NoteOff, CC }; + /** - * @brief Get the number which identifies this voice + * @brief Get the unique identifier of this voice in a synth */ - int getIdNumber() const noexcept + NumericId getId() const noexcept { - return voiceNumber; + return id; } enum class State { @@ -56,7 +58,7 @@ public: class StateListener { public: - virtual void onVoiceStateChanged(int /*idNumber*/, State /*state*/) {} + virtual void onVoiceStateChanged(NumericId /*id*/, State /*state*/) {} }; /** @@ -299,7 +301,7 @@ private: */ void switchState(State s); - const int voiceNumber { -1 }; + const NumericId id; StateListener* stateListener = nullptr; Region* region { nullptr }; From 37ef2b99429d6721d9e7f5872346066df1512d1d Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Thu, 11 Jun 2020 23:52:17 +0200 Subject: [PATCH 5/8] Update benchmarks --- benchmarks/BM_ADSR.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmarks/BM_ADSR.cpp b/benchmarks/BM_ADSR.cpp index e2d077c0..c17a1a31 100644 --- a/benchmarks/BM_ADSR.cpp +++ b/benchmarks/BM_ADSR.cpp @@ -35,7 +35,7 @@ public: } sfz::MidiState midiState; - sfz::Region region{midiState}; + sfz::Region region{0, midiState}; sfz::ADSREnvelope envelope; std::vector output; }; From 2c10173356c42707612ebfa93fefd0a0fc8658fb Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Fri, 12 Jun 2020 00:26:02 +0200 Subject: [PATCH 6/8] Add the lookup functions by identifier --- src/sfizz/NumericId.h | 10 ++++++++++ src/sfizz/Synth.cpp | 34 ++++++++++++++++++++++++++++++++++ src/sfizz/Synth.h | 14 ++++++++++++++ 3 files changed, 58 insertions(+) diff --git a/src/sfizz/NumericId.h b/src/sfizz/NumericId.h index 1d975ec0..b2fed501 100644 --- a/src/sfizz/NumericId.h +++ b/src/sfizz/NumericId.h @@ -27,5 +27,15 @@ struct NumericId { return number != -1; } + constexpr bool operator==(NumericId other) const noexcept + { + return number == other.number; + } + + constexpr bool operator!=(NumericId other) const noexcept + { + return number != other.number; + } + const int number = -1; }; diff --git a/src/sfizz/Synth.cpp b/src/sfizz/Synth.cpp index c609d3c5..001dae48 100644 --- a/src/sfizz/Synth.cpp +++ b/src/sfizz/Synth.cpp @@ -1056,6 +1056,40 @@ const sfz::EffectBus* sfz::Synth::getEffectBusView(int idx) const noexcept return (size_t)idx < effectBuses.size() ? effectBuses[idx].get() : nullptr; } +const sfz::Region* sfz::Synth::getRegionById(NumericId id) const noexcept +{ + const size_t size = regions.size(); + + if (size == 0 || !id.valid()) + return nullptr; + + // search a sequence of ordered identifiers with potential gaps + size_t index = static_cast(id.number); + index = std::min(index, size - 1); + + while (index > 0 && regions[index]->getId().number > id.number) + --index; + + return (regions[index]->getId() == id) ? regions[index].get() : nullptr; +} + +const sfz::Voice* sfz::Synth::getVoiceById(NumericId id) const noexcept +{ + const size_t size = voices.size(); + + if (size == 0 || !id.valid()) + return nullptr; + + // search a sequence of ordered identifiers with potential gaps + size_t index = static_cast(id.number); + index = std::min(index, size - 1); + + while (index > 0 && voices[index]->getId().number > id.number) + --index; + + return (voices[index]->getId() == id) ? voices[index].get() : nullptr; +} + const sfz::Voice* sfz::Synth::getVoiceView(int idx) const noexcept { return (size_t)idx < voices.size() ? voices[idx].get() : nullptr; diff --git a/src/sfizz/Synth.h b/src/sfizz/Synth.h index d04a5c96..9c5420e9 100644 --- a/src/sfizz/Synth.h +++ b/src/sfizz/Synth.h @@ -178,6 +178,20 @@ public: * @brief Export a MIDI Name document describing the loaded instrument */ std::string exportMidnam(absl::string_view model = {}) const; + /** + * @brief Find the region which is associated with the given identifier. + * + * @param id + * @return const Region* + */ + const Region* getRegionById(NumericId id) const noexcept; + /** + * @brief Find the voice which is associated with the given identifier. + * + * @param id + * @return const Voice* + */ + const Voice* getVoiceById(NumericId id) const noexcept; /** * @brief Get a raw view into a specific region. This is mostly used * for testing. From 215077deaace5171b4723aa875631556759eaad1 Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Fri, 12 Jun 2020 00:53:22 +0200 Subject: [PATCH 7/8] Make region removal order-preserving --- src/sfizz/Synth.cpp | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/sfizz/Synth.cpp b/src/sfizz/Synth.cpp index 001dae48..7c535aa6 100644 --- a/src/sfizz/Synth.cpp +++ b/src/sfizz/Synth.cpp @@ -361,22 +361,20 @@ void sfz::Synth::finalizeSfzLoad() { resources.filePool.setRootDirectory(parser.originalDirectory()); - auto currentRegion = regions.begin(); - auto lastRegion = regions.rbegin(); - auto removeCurrentRegion = [¤tRegion, &lastRegion]() { - if (currentRegion->get() == nullptr) - return; + size_t currentRegionIndex = 0; + size_t currentRegionCount = regions.size(); - DBG("Removing the region with sample " << currentRegion->get()->sampleId); - std::iter_swap(currentRegion, lastRegion); - ++lastRegion; + auto removeCurrentRegion = [this, ¤tRegionIndex, ¤tRegionCount]() { + DBG("Removing the region with sample " << regions[currentRegionIndex]->sampleId); + regions.erase(regions.begin() + currentRegionIndex); + --currentRegionCount; }; size_t maxFilters { 0 }; size_t maxEQs { 0 }; - while (currentRegion < lastRegion.base()) { - auto region = currentRegion->get(); + while (currentRegionIndex < currentRegionCount) { + auto region = regions[currentRegionIndex].get(); if (!region->oscillator && !region->isGenerator()) { if (!resources.filePool.checkSampleId(region->sampleId)) { @@ -481,11 +479,10 @@ void sfz::Synth::finalizeSfzLoad() maxFilters = max(maxFilters, region->filters.size()); maxEQs = max(maxEQs, region->equalizers.size()); - ++currentRegion; + ++currentRegionIndex; } - const auto remainingRegions = std::distance(regions.begin(), lastRegion.base()); - DBG("Removing " << (regions.size() - remainingRegions) << " out of " << regions.size() << " regions"); - regions.resize(remainingRegions); + DBG("Removing " << (regions.size() - currentRegionCount) << " out of " << regions.size() << " regions"); + regions.resize(currentRegionCount); modificationTime = checkModificationTime(); for (auto& voice : voices) { From 0bbb1e930f1d0c889b032f673242297fb636339e Mon Sep 17 00:00:00 2001 From: Jean Pierre Cimalando Date: Fri, 12 Jun 2020 00:53:36 +0200 Subject: [PATCH 8/8] Add test --- tests/SynthT.cpp | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/SynthT.cpp b/tests/SynthT.cpp index e59df54f..112acb0d 100644 --- a/tests/SynthT.cpp +++ b/tests/SynthT.cpp @@ -6,6 +6,7 @@ #include "sfizz/Synth.h" #include "sfizz/SfzHelpers.h" +#include "sfizz/NumericId.h" #include "catch2/catch.hpp" using namespace Catch::literals; using namespace sfz::literals; @@ -462,3 +463,26 @@ TEST_CASE("[Synth] velcurve") REQUIRE( synth.getRegionView(1)->velocityCurve(96_norm) == Approx(0.5f).margin(1e-2) ); REQUIRE( synth.getRegionView(1)->velocityCurve(127_norm) == 0.0_a ); } + +TEST_CASE("[Synth] Region by identifier") +{ + sfz::Synth synth; + synth.loadSfzString("regionByIdentifier.sfz", R"( +sample=*sine +sample=*sine +sample=doesNotExist.wav +sample=*sine +sample=doesNotExist.wav +sample=*sine +)"); + + REQUIRE(synth.getNumRegions() == 4); + REQUIRE(synth.getRegionView(0) == synth.getRegionById(NumericId{0})); + REQUIRE(synth.getRegionView(1) == synth.getRegionById(NumericId{1})); + REQUIRE(nullptr == synth.getRegionById(NumericId{2})); + REQUIRE(synth.getRegionView(2) == synth.getRegionById(NumericId{3})); + REQUIRE(nullptr == synth.getRegionById(NumericId{4})); + REQUIRE(synth.getRegionView(3) == synth.getRegionById(NumericId{5})); + REQUIRE(nullptr == synth.getRegionById(NumericId{6})); + REQUIRE(nullptr == synth.getRegionById(NumericId{})); +}