From bce4fe96d047ae5f47cc6ee010805483ff2cbaf8 Mon Sep 17 00:00:00 2001 From: Paul Fd Date: Thu, 5 Mar 2020 01:38:04 +0100 Subject: [PATCH 1/8] Replace the underlying structure of CCMap with a smallish vector --- src/sfizz/CCMap.h | 43 ++++++++++++++++++++++--------------------- src/sfizz/Region.cpp | 2 +- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/sfizz/CCMap.h b/src/sfizz/CCMap.h index 34bf5b7c..6da8da03 100644 --- a/src/sfizz/CCMap.h +++ b/src/sfizz/CCMap.h @@ -6,7 +6,8 @@ #pragma once #include "LeakDetector.h" -#include +#include +#include namespace sfz { /** @@ -42,7 +43,7 @@ public: */ const ValueType& getWithDefault(int index) const noexcept { - auto it = container.find(index); + auto it = absl::c_find_if(container, [&](auto&& pair){ return pair.first == index; }); if (it == container.end()) { return defaultValue; } else { @@ -51,16 +52,20 @@ public: } /** - * @brief Get the value at index key or emplace a new one if not present + * @brief Get the value at index or emplace a new one if not present * - * @param key the index of the element + * @param index the index of the element * @return ValueType& */ - ValueType& operator[](const int& key) noexcept + ValueType& operator[](const int& index) noexcept { - if (!contains(key)) - container.emplace(key, defaultValue); - return container.operator[](key); + auto it = absl::c_find_if(container, [&](auto&& pair){ return pair.first == index; }); + if (it == container.end()) { + container.emplace_back(index, defaultValue); + return container.back().second; + } else { + return it->second; + } } /** @@ -70,13 +75,6 @@ public: * @return false */ inline bool empty() const { return container.empty(); } - /** - * @brief Returns the value at index with bounds checking (and possibly exceptions) - * - * @param index - * @return const ValueType& - */ - const ValueType& at(int index) const { return container.at(index); } /** * @brief Returns true if the container containers an element at index * @@ -84,14 +82,17 @@ public: * @return true * @return false */ - bool contains(int index) const noexcept { return container.find(index) != container.end(); } - typename std::map::iterator begin() { return container.begin(); } - typename std::map::const_iterator begin() const { return container.cbegin(); } - typename std::map::iterator end() { return container.end(); } - typename std::map::const_iterator end() const { return container.cend(); } + bool contains(int index) const noexcept + { + return absl::c_find_if(container, [&](auto&& pair){ return pair.first == index; }) != container.end(); + } + typename std::vector>::iterator begin() { return container.begin(); } + typename std::vector>::const_iterator begin() const { return container.cbegin(); } + typename std::vector>::iterator end() { return container.end(); } + typename std::vector>::const_iterator end() const { return container.cend(); } private: const ValueType defaultValue; - std::map container; + std::vector> container; LEAK_DETECTOR(CCMap); }; } diff --git a/src/sfizz/Region.cpp b/src/sfizz/Region.cpp index 94d0218e..9e2c162e 100644 --- a/src/sfizz/Region.cpp +++ b/src/sfizz/Region.cpp @@ -854,7 +854,7 @@ bool sfz::Region::registerCC(int ccNumber, uint8_t ccValue) noexcept if (!triggerOnCC) return false; - if (ccTriggers.contains(ccNumber) && ccTriggers.at(ccNumber).containsWithEnd(ccValue)) + if (ccTriggers.contains(ccNumber) && ccTriggers[ccNumber].containsWithEnd(ccValue)) return true; else return false; From 8becafcae00a1eec6f5d18db2a5bdca9ab634ba5 Mon Sep 17 00:00:00 2001 From: Paul Fd Date: Thu, 5 Mar 2020 16:38:19 +0100 Subject: [PATCH 2/8] Insert sorted into the vector, and use only const iterators in CCMap --- src/sfizz/CCMap.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/sfizz/CCMap.h b/src/sfizz/CCMap.h index 6da8da03..c025f3e3 100644 --- a/src/sfizz/CCMap.h +++ b/src/sfizz/CCMap.h @@ -61,8 +61,9 @@ public: { auto it = absl::c_find_if(container, [&](auto&& pair){ return pair.first == index; }); if (it == container.end()) { - container.emplace_back(index, defaultValue); - return container.back().second; + auto newElement = std::make_pair(index, defaultValue); + auto inserted = container.insert(absl::c_upper_bound(container, newElement, [](auto& lhs, auto& rhs) { return lhs.first < rhs.first; }), newElement); + return inserted->second; } else { return it->second; } @@ -86,11 +87,11 @@ public: { return absl::c_find_if(container, [&](auto&& pair){ return pair.first == index; }) != container.end(); } - typename std::vector>::iterator begin() { return container.begin(); } typename std::vector>::const_iterator begin() const { return container.cbegin(); } - typename std::vector>::iterator end() { return container.end(); } typename std::vector>::const_iterator end() const { return container.cend(); } private: + // typename std::vector>::iterator begin() { return container.begin(); } + // typename std::vector>::iterator end() { return container.end(); } const ValueType defaultValue; std::vector> container; LEAK_DETECTOR(CCMap); From d10cd27ceb392b90c0c20ff61f7f4c1f6cf78c2c Mon Sep 17 00:00:00 2001 From: Paul Fd Date: Thu, 5 Mar 2020 16:38:31 +0100 Subject: [PATCH 3/8] Clean up the Range class --- src/sfizz/Range.h | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/src/sfizz/Range.h b/src/sfizz/Range.h index acb88c66..35e8e1b4 100644 --- a/src/sfizz/Range.h +++ b/src/sfizz/Range.h @@ -22,27 +22,12 @@ class Range { public: constexpr Range() = default; - // constexpr Range(std::initializer_list list) - // { - // switch(list.size()) - // { - // case 0: - // break; - // case 1: - // _start = *list.begin(); - // _end = _start; - // break; - // default: - // _start = *list.begin(); - // _end = *(list.begin() + 1); - // } - // } constexpr Range(Type start, Type end) noexcept : _start(start) , _end(std::max(start, end)) { + } - ~Range() = default; Type getStart() const noexcept { return _start; } Type getEnd() const noexcept { return _end; } /** @@ -51,8 +36,6 @@ public: * @return std::pair */ std::pair getPair() const noexcept { return std::make_pair(_start, _end); } - Range(const Range& range) = default; - Range(Range&& range) = default; constexpr Type length() const { return _end - _start; } void setStart(Type start) noexcept { From f70b042da6c7f60101989352097a731566dc2675 Mon Sep 17 00:00:00 2001 From: Paul Fd Date: Sat, 7 Mar 2020 12:35:48 +0100 Subject: [PATCH 4/8] Change CCValuePair type alias and use it in CCMap Also use lower bound and binary search in the CCMap vector --- src/sfizz/CCMap.h | 27 ++++++++++++++------------- src/sfizz/EGDescription.h | 14 +++++++------- src/sfizz/MidiState.h | 2 +- src/sfizz/Opcode.h | 4 ++-- src/sfizz/Region.cpp | 8 ++++---- src/sfizz/Region.h | 10 +++++----- src/sfizz/SfzHelpers.h | 29 ++++++++++++++++++++++++++--- src/sfizz/Voice.cpp | 30 +++++++++++++++--------------- 8 files changed, 74 insertions(+), 50 deletions(-) diff --git a/src/sfizz/CCMap.h b/src/sfizz/CCMap.h index c025f3e3..a71ba2f4 100644 --- a/src/sfizz/CCMap.h +++ b/src/sfizz/CCMap.h @@ -6,6 +6,7 @@ #pragma once #include "LeakDetector.h" +#include "SfzHelpers.h" #include #include @@ -43,11 +44,11 @@ public: */ const ValueType& getWithDefault(int index) const noexcept { - auto it = absl::c_find_if(container, [&](auto&& pair){ return pair.first == index; }); - if (it == container.end()) { + auto it = absl::c_lower_bound(container, index, CompareCC{}); + if (it == container.end() || it->cc != index) { return defaultValue; } else { - return it->second; + return it->value; } } @@ -59,13 +60,12 @@ public: */ ValueType& operator[](const int& index) noexcept { - auto it = absl::c_find_if(container, [&](auto&& pair){ return pair.first == index; }); - if (it == container.end()) { - auto newElement = std::make_pair(index, defaultValue); - auto inserted = container.insert(absl::c_upper_bound(container, newElement, [](auto& lhs, auto& rhs) { return lhs.first < rhs.first; }), newElement); - return inserted->second; + auto it = absl::c_lower_bound(container, index, CompareCC{}); + if (it == container.end() || it->cc != index) { + auto inserted = container.insert(it, { index, defaultValue }); + return inserted->value; } else { - return it->second; + return it->value; } } @@ -85,15 +85,16 @@ public: */ bool contains(int index) const noexcept { - return absl::c_find_if(container, [&](auto&& pair){ return pair.first == index; }) != container.end(); + return absl::c_binary_search(container, index, CompareCC{}); } - typename std::vector>::const_iterator begin() const { return container.cbegin(); } - typename std::vector>::const_iterator end() const { return container.cend(); } + typename std::vector>::const_iterator begin() const { return container.cbegin(); } + typename std::vector>::const_iterator end() const { return container.cend(); } private: // typename std::vector>::iterator begin() { return container.begin(); } // typename std::vector>::iterator end() { return container.end(); } + const ValueType defaultValue; - std::vector> container; + std::vector> container; LEAK_DETECTOR(CCMap); }; } diff --git a/src/sfizz/EGDescription.h b/src/sfizz/EGDescription.h index cc655511..afaa32c8 100644 --- a/src/sfizz/EGDescription.h +++ b/src/sfizz/EGDescription.h @@ -64,13 +64,13 @@ struct EGDescription float vel2sustain { Default::vel2sustain }; int vel2depth { Default::depth }; - absl::optional ccAttack; - absl::optional ccDecay; - absl::optional ccDelay; - absl::optional ccHold; - absl::optional ccRelease; - absl::optional ccStart; - absl::optional ccSustain; + absl::optional> ccAttack; + absl::optional> ccDecay; + absl::optional> ccDelay; + absl::optional> ccHold; + absl::optional> ccRelease; + absl::optional> ccStart; + absl::optional> ccSustain; /** * @brief Get the attack with possibly a CC modifier and a velocity modifier diff --git a/src/sfizz/MidiState.h b/src/sfizz/MidiState.h index afba37c8..ad8238fd 100644 --- a/src/sfizz/MidiState.h +++ b/src/sfizz/MidiState.h @@ -120,7 +120,7 @@ public: T modulate(T value, const CCMap& modifiers, const Range& validRange, const modFunction& lambda = addToBase) const noexcept { for (auto& mod: modifiers) { - lambda(value, normalizeCC(getCCValue(mod.first)) * mod.second); + lambda(value, normalizeCC(getCCValue(mod.cc)) * mod.value); } return validRange.clamp(value); } diff --git a/src/sfizz/Opcode.h b/src/sfizz/Opcode.h index 227c3d28..2ac43209 100644 --- a/src/sfizz/Opcode.h +++ b/src/sfizz/Opcode.h @@ -184,11 +184,11 @@ inline void setRangeStartFromOpcode(const Opcode& opcode, Range& targ * @param validRange the range of admitted values used to clamp the opcode */ template -inline void setCCPairFromOpcode(const Opcode& opcode, absl::optional& target, const Range& validRange) +inline void setCCPairFromOpcode(const Opcode& opcode, absl::optional>& target, const Range& validRange) { auto value = readOpcode(opcode.value, validRange); if (value && Default::ccNumberRange.containsWithEnd(opcode.parameters.back())) - target = std::make_pair(opcode.parameters.back(), *value); + target = { opcode.parameters.back(), *value }; else target = {}; } diff --git a/src/sfizz/Region.cpp b/src/sfizz/Region.cpp index 9e2c162e..004b5fe9 100644 --- a/src/sfizz/Region.cpp +++ b/src/sfizz/Region.cpp @@ -992,14 +992,14 @@ float sfz::Region::getCrossfadeGain(const sfz::SfzCCArray& ccState) noexcept // Crossfades due to CC states for (const auto& valuePair : crossfadeCCInRange) { - const auto ccValue = ccState[valuePair.first]; - const auto crossfadeRange = valuePair.second; + const auto ccValue = ccState[valuePair.cc]; + const auto crossfadeRange = valuePair.value; gain *= crossfadeIn(crossfadeRange, ccValue, crossfadeCCCurve); } for (const auto& valuePair : crossfadeCCOutRange) { - const auto ccValue = ccState[valuePair.first]; - const auto crossfadeRange = valuePair.second; + const auto ccValue = ccState[valuePair.cc]; + const auto crossfadeRange = valuePair.value; gain *= crossfadeOut(crossfadeRange, ccValue, crossfadeCCCurve); } diff --git a/src/sfizz/Region.h b/src/sfizz/Region.h index cf08657b..fa5ea786 100644 --- a/src/sfizz/Region.h +++ b/src/sfizz/Region.h @@ -264,11 +264,11 @@ struct Region { float pan { Default::pan }; // pan float width { Default::width }; // width float position { Default::position }; // position - absl::optional volumeCC; // volume_oncc - absl::optional amplitudeCC; // amplitude_oncc - absl::optional panCC; // pan_oncc - absl::optional widthCC; // width_oncc - absl::optional positionCC; // position_oncc + absl::optional> volumeCC; // volume_oncc + absl::optional> amplitudeCC; // amplitude_oncc + absl::optional> panCC; // pan_oncc + absl::optional> widthCC; // width_oncc + absl::optional> positionCC; // position_oncc uint8_t ampKeycenter { Default::ampKeycenter }; // amp_keycenter float ampKeytrack { Default::ampKeytrack }; // amp_keytrack float ampVeltrack { Default::ampVeltrack }; // amp_keytrack diff --git a/src/sfizz/SfzHelpers.h b/src/sfizz/SfzHelpers.h index 1e590d1b..eb05e6bc 100644 --- a/src/sfizz/SfzHelpers.h +++ b/src/sfizz/SfzHelpers.h @@ -16,9 +16,32 @@ namespace sfz { using SfzCCArray = std::array; -using CCValuePair = std::pair ; using CCNamePair = std::pair; +template +struct CCValuePair { + int cc; + ValueType value; +}; + +template +struct CompareCC { + bool operator()(const CCValuePair& valuePair, const int& cc) + { + return (valuePair.cc < cc); + } + + bool operator()(const int& cc, const CCValuePair& valuePair) + { + return (cc < valuePair.cc); + } + + bool operator()(const CCValuePair& lhs, const CCValuePair& rhs) + { + return (lhs.cc < rhs.cc); + } +}; + /** * @brief Converts cents to a pitch ratio * @@ -94,10 +117,10 @@ constexpr float normalizeBend(float bendValue) * @param value * @return float */ -inline float ccSwitchedValue(const SfzCCArray& ccValues, const absl::optional& ccSwitch, float value) noexcept +inline float ccSwitchedValue(const SfzCCArray& ccValues, const absl::optional>& ccSwitch, float value) noexcept { if (ccSwitch) - return value + ccSwitch->second * normalizeCC(ccValues[ccSwitch->first]); + return value + ccSwitch->value * normalizeCC(ccValues[ccSwitch->cc]); else return value; } diff --git a/src/sfizz/Voice.cpp b/src/sfizz/Voice.cpp index 64bac0f2..df3e3c10 100644 --- a/src/sfizz/Voice.cpp +++ b/src/sfizz/Voice.cpp @@ -47,7 +47,7 @@ void sfz::Voice::startVoice(Region* region, int delay, int number, uint8_t value baseVolumedB = region->getBaseVolumedB(number); auto volumedB { baseVolumedB }; if (region->volumeCC) - volumedB += normalizeCC(resources.midiState.getCCValue(region->volumeCC->first)) * region->volumeCC->second; + volumedB += normalizeCC(resources.midiState.getCCValue(region->volumeCC->cc)) * region->volumeCC->value; volumeEnvelope.reset(db2mag(Default::volumeRange.clamp(volumedB))); baseGain = region->getBaseGain(); @@ -56,7 +56,7 @@ void sfz::Voice::startVoice(Region* region, int delay, int number, uint8_t value float gain { baseGain }; if (region->amplitudeCC) - gain += normalizeCC(resources.midiState.getCCValue(region->amplitudeCC->first)) * normalizePercents(region->amplitudeCC->second); + gain += normalizeCC(resources.midiState.getCCValue(region->amplitudeCC->cc)) * normalizePercents(region->amplitudeCC->value); amplitudeEnvelope.reset(Default::normalizedRange.clamp(gain)); float crossfadeGain { region->getCrossfadeGain(resources.midiState.getCCArray()) }; @@ -65,19 +65,19 @@ void sfz::Voice::startVoice(Region* region, int delay, int number, uint8_t value basePan = normalizePercents(region->pan); auto pan { basePan }; if (region->panCC) - pan += normalizeCC(resources.midiState.getCCValue(region->panCC->first)) * normalizePercents(region->panCC->second); + pan += normalizeCC(resources.midiState.getCCValue(region->panCC->cc)) * normalizePercents(region->panCC->value); panEnvelope.reset(Default::symmetricNormalizedRange.clamp(pan)); basePosition = normalizePercents(region->position); auto position { basePosition }; if (region->positionCC) - position += normalizeCC(resources.midiState.getCCValue(region->positionCC->first)) * normalizePercents(region->positionCC->second); + position += normalizeCC(resources.midiState.getCCValue(region->positionCC->cc)) * normalizePercents(region->positionCC->value); positionEnvelope.reset(Default::symmetricNormalizedRange.clamp(position)); baseWidth = normalizePercents(region->width); auto width { baseWidth }; if (region->widthCC) - width += normalizeCC(resources.midiState.getCCValue(region->widthCC->first)) * normalizePercents(region->widthCC->second); + width += normalizeCC(resources.midiState.getCCValue(region->widthCC->cc)) * normalizePercents(region->widthCC->value); widthEnvelope.reset(Default::symmetricNormalizedRange.clamp(width)); pitchBendEnvelope.setFunction([region](float pitchValue){ @@ -168,28 +168,28 @@ void sfz::Voice::registerCC(int delay, int ccNumber, uint8_t ccValue) noexcept // TODO: this feels like a hack, revisit this along with the smoothed envelopes... delay = max(delay, minEnvelopeDelay); - if (region->amplitudeCC && ccNumber == region->amplitudeCC->first) { - const float newGain { baseGain + normalizeCC(ccValue) * normalizePercents(region->amplitudeCC->second) }; + if (region->amplitudeCC && ccNumber == region->amplitudeCC->cc) { + const float newGain { baseGain + normalizeCC(ccValue) * normalizePercents(region->amplitudeCC->value) }; amplitudeEnvelope.registerEvent(delay, Default::normalizedRange.clamp(newGain)); } - if (region->volumeCC && ccNumber == region->volumeCC->first) { - const float newVolumedB { baseVolumedB + normalizeCC(ccValue) * region->volumeCC->second }; + if (region->volumeCC && ccNumber == region->volumeCC->cc) { + const float newVolumedB { baseVolumedB + normalizeCC(ccValue) * region->volumeCC->value }; volumeEnvelope.registerEvent(delay, db2mag(Default::volumeRange.clamp(newVolumedB))); } - if (region->panCC && ccNumber == region->panCC->first) { - const float newPan { basePan + normalizeCC(ccValue) * normalizePercents(region->panCC->second) }; + if (region->panCC && ccNumber == region->panCC->cc) { + const float newPan { basePan + normalizeCC(ccValue) * normalizePercents(region->panCC->value) }; panEnvelope.registerEvent(delay, Default::symmetricNormalizedRange.clamp(newPan)); } - if (region->positionCC && ccNumber == region->positionCC->first) { - const float newPosition { basePosition + normalizeCC(ccValue) * normalizePercents(region->positionCC->second) }; + if (region->positionCC && ccNumber == region->positionCC->cc) { + const float newPosition { basePosition + normalizeCC(ccValue) * normalizePercents(region->positionCC->value) }; positionEnvelope.registerEvent(delay, Default::symmetricNormalizedRange.clamp(newPosition)); } - if (region->widthCC && ccNumber == region->widthCC->first) { - const float newWidth { baseWidth + normalizeCC(ccValue) * normalizePercents(region->widthCC->second) }; + if (region->widthCC && ccNumber == region->widthCC->cc) { + const float newWidth { baseWidth + normalizeCC(ccValue) * normalizePercents(region->widthCC->value) }; widthEnvelope.registerEvent(delay, Default::symmetricNormalizedRange.clamp(newWidth)); } From 4398f3bab469a4e68e530b766d73e224d265d614 Mon Sep 17 00:00:00 2001 From: Paul Fd Date: Sat, 7 Mar 2020 12:36:01 +0100 Subject: [PATCH 5/8] Update the tests for the new CCValuePair alias --- tests/FilesT.cpp | 4 +-- tests/RegionT.cpp | 68 +++++++++++++++++++++++------------------------ 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/tests/FilesT.cpp b/tests/FilesT.cpp index c8cd6c03..b397d154 100644 --- a/tests/FilesT.cpp +++ b/tests/FilesT.cpp @@ -306,8 +306,8 @@ TEST_CASE("[Files] wrong (overlapping) replacement for defines") REQUIRE( synth.getRegionView(1)->keyRange.getStart() == 57 ); REQUIRE( synth.getRegionView(1)->keyRange.getEnd() == 57 ); REQUIRE( synth.getRegionView(2)->amplitudeCC ); - REQUIRE( synth.getRegionView(2)->amplitudeCC->first == 10 ); - REQUIRE( synth.getRegionView(2)->amplitudeCC->second == 34.0f ); + REQUIRE( synth.getRegionView(2)->amplitudeCC->cc == 10 ); + REQUIRE( synth.getRegionView(2)->amplitudeCC->value == 34.0f ); } TEST_CASE("[Files] Specific bug: relative path with backslashes") diff --git a/tests/RegionT.cpp b/tests/RegionT.cpp index 71063e74..65bf133f 100644 --- a/tests/RegionT.cpp +++ b/tests/RegionT.cpp @@ -464,8 +464,8 @@ TEST_CASE("[Region] Parsing opcodes") REQUIRE(!region.panCC); region.parseOpcode({ "pan_oncc45", "4.2" }); REQUIRE(region.panCC); - REQUIRE(region.panCC->first == 45); - REQUIRE(region.panCC->second == 4.2f); + REQUIRE(region.panCC->cc == 45); + REQUIRE(region.panCC->value == 4.2f); } SECTION("width") @@ -486,8 +486,8 @@ TEST_CASE("[Region] Parsing opcodes") REQUIRE(!region.widthCC); region.parseOpcode({ "width_oncc45", "4.2" }); REQUIRE(region.widthCC); - REQUIRE(region.widthCC->first == 45); - REQUIRE(region.widthCC->second == 4.2f); + REQUIRE(region.widthCC->cc == 45); + REQUIRE(region.widthCC->value == 4.2f); } SECTION("position") @@ -508,8 +508,8 @@ TEST_CASE("[Region] Parsing opcodes") REQUIRE(!region.positionCC); region.parseOpcode({ "position_oncc45", "4.2" }); REQUIRE(region.positionCC); - REQUIRE(region.positionCC->first == 45); - REQUIRE(region.positionCC->second == 4.2f); + REQUIRE(region.positionCC->cc == 45); + REQUIRE(region.positionCC->value == 4.2f); } SECTION("amp_keycenter") @@ -964,20 +964,20 @@ TEST_CASE("[Region] Parsing opcodes") REQUIRE(region.amplitudeEG.ccRelease); REQUIRE(region.amplitudeEG.ccStart); REQUIRE(region.amplitudeEG.ccSustain); - REQUIRE(region.amplitudeEG.ccAttack->first == 1); - REQUIRE(region.amplitudeEG.ccDecay->first == 2); - REQUIRE(region.amplitudeEG.ccDelay->first == 3); - REQUIRE(region.amplitudeEG.ccHold->first == 4); - REQUIRE(region.amplitudeEG.ccRelease->first == 5); - REQUIRE(region.amplitudeEG.ccStart->first == 6); - REQUIRE(region.amplitudeEG.ccSustain->first == 7); - REQUIRE(region.amplitudeEG.ccAttack->second == 1.0f); - REQUIRE(region.amplitudeEG.ccDecay->second == 2.0f); - REQUIRE(region.amplitudeEG.ccDelay->second == 3.0f); - REQUIRE(region.amplitudeEG.ccHold->second == 4.0f); - REQUIRE(region.amplitudeEG.ccRelease->second == 5.0f); - REQUIRE(region.amplitudeEG.ccStart->second == 6.0f); - REQUIRE(region.amplitudeEG.ccSustain->second == 7.0f); + REQUIRE(region.amplitudeEG.ccAttack->cc == 1); + REQUIRE(region.amplitudeEG.ccDecay->cc == 2); + REQUIRE(region.amplitudeEG.ccDelay->cc == 3); + REQUIRE(region.amplitudeEG.ccHold->cc == 4); + REQUIRE(region.amplitudeEG.ccRelease->cc == 5); + REQUIRE(region.amplitudeEG.ccStart->cc == 6); + REQUIRE(region.amplitudeEG.ccSustain->cc == 7); + REQUIRE(region.amplitudeEG.ccAttack->value == 1.0f); + REQUIRE(region.amplitudeEG.ccDecay->value == 2.0f); + REQUIRE(region.amplitudeEG.ccDelay->value == 3.0f); + REQUIRE(region.amplitudeEG.ccHold->value == 4.0f); + REQUIRE(region.amplitudeEG.ccRelease->value == 5.0f); + REQUIRE(region.amplitudeEG.ccStart->value == 6.0f); + REQUIRE(region.amplitudeEG.ccSustain->value == 7.0f); // region.parseOpcode({ "ampeg_attack_oncc1", "101" }); region.parseOpcode({ "ampeg_decay_oncc2", "101" }); @@ -986,13 +986,13 @@ TEST_CASE("[Region] Parsing opcodes") region.parseOpcode({ "ampeg_release_oncc5", "101" }); region.parseOpcode({ "ampeg_start_oncc6", "101" }); region.parseOpcode({ "ampeg_sustain_oncc7", "101" }); - REQUIRE(region.amplitudeEG.ccAttack->second == 100.0f); - REQUIRE(region.amplitudeEG.ccDecay->second == 100.0f); - REQUIRE(region.amplitudeEG.ccDelay->second == 100.0f); - REQUIRE(region.amplitudeEG.ccHold->second == 100.0f); - REQUIRE(region.amplitudeEG.ccRelease->second == 100.0f); - REQUIRE(region.amplitudeEG.ccStart->second == 100.0f); - REQUIRE(region.amplitudeEG.ccSustain->second == 100.0f); + REQUIRE(region.amplitudeEG.ccAttack->value == 100.0f); + REQUIRE(region.amplitudeEG.ccDecay->value == 100.0f); + REQUIRE(region.amplitudeEG.ccDelay->value == 100.0f); + REQUIRE(region.amplitudeEG.ccHold->value == 100.0f); + REQUIRE(region.amplitudeEG.ccRelease->value == 100.0f); + REQUIRE(region.amplitudeEG.ccStart->value == 100.0f); + REQUIRE(region.amplitudeEG.ccSustain->value == 100.0f); // region.parseOpcode({ "ampeg_attack_oncc1", "-101" }); region.parseOpcode({ "ampeg_decay_oncc2", "-101" }); @@ -1001,13 +1001,13 @@ TEST_CASE("[Region] Parsing opcodes") region.parseOpcode({ "ampeg_release_oncc5", "-101" }); region.parseOpcode({ "ampeg_start_oncc6", "-101" }); region.parseOpcode({ "ampeg_sustain_oncc7", "-101" }); - REQUIRE(region.amplitudeEG.ccAttack->second == -100.0f); - REQUIRE(region.amplitudeEG.ccDecay->second == -100.0f); - REQUIRE(region.amplitudeEG.ccDelay->second == -100.0f); - REQUIRE(region.amplitudeEG.ccHold->second == -100.0f); - REQUIRE(region.amplitudeEG.ccRelease->second == -100.0f); - REQUIRE(region.amplitudeEG.ccStart->second == -100.0f); - REQUIRE(region.amplitudeEG.ccSustain->second == -100.0f); + REQUIRE(region.amplitudeEG.ccAttack->value == -100.0f); + REQUIRE(region.amplitudeEG.ccDecay->value == -100.0f); + REQUIRE(region.amplitudeEG.ccDelay->value == -100.0f); + REQUIRE(region.amplitudeEG.ccHold->value == -100.0f); + REQUIRE(region.amplitudeEG.ccRelease->value == -100.0f); + REQUIRE(region.amplitudeEG.ccStart->value == -100.0f); + REQUIRE(region.amplitudeEG.ccSustain->value == -100.0f); } SECTION("sustain_sw and sostenuto_sw") From 3046c2393f519a698b35c3faea6eb10c23164f6b Mon Sep 17 00:00:00 2001 From: Paul Fd Date: Sat, 7 Mar 2020 12:43:20 +0100 Subject: [PATCH 6/8] Added a template specialization to compare on value and not CC --- src/sfizz/CCMap.h | 6 +++--- src/sfizz/SfzHelpers.h | 22 ++++++++++++++++++++-- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/sfizz/CCMap.h b/src/sfizz/CCMap.h index a71ba2f4..09e8830c 100644 --- a/src/sfizz/CCMap.h +++ b/src/sfizz/CCMap.h @@ -44,7 +44,7 @@ public: */ const ValueType& getWithDefault(int index) const noexcept { - auto it = absl::c_lower_bound(container, index, CompareCC{}); + auto it = absl::c_lower_bound(container, index, CCValuePairComparator{}); if (it == container.end() || it->cc != index) { return defaultValue; } else { @@ -60,7 +60,7 @@ public: */ ValueType& operator[](const int& index) noexcept { - auto it = absl::c_lower_bound(container, index, CompareCC{}); + auto it = absl::c_lower_bound(container, index, CCValuePairComparator{}); if (it == container.end() || it->cc != index) { auto inserted = container.insert(it, { index, defaultValue }); return inserted->value; @@ -85,7 +85,7 @@ public: */ bool contains(int index) const noexcept { - return absl::c_binary_search(container, index, CompareCC{}); + return absl::c_binary_search(container, index, CCValuePairComparator{}); } typename std::vector>::const_iterator begin() const { return container.cbegin(); } typename std::vector>::const_iterator end() const { return container.cend(); } diff --git a/src/sfizz/SfzHelpers.h b/src/sfizz/SfzHelpers.h index eb05e6bc..4a69ba95 100644 --- a/src/sfizz/SfzHelpers.h +++ b/src/sfizz/SfzHelpers.h @@ -24,8 +24,8 @@ struct CCValuePair { ValueType value; }; -template -struct CompareCC { +template +struct CCValuePairComparator { bool operator()(const CCValuePair& valuePair, const int& cc) { return (valuePair.cc < cc); @@ -42,6 +42,24 @@ struct CompareCC { } }; +template +struct CCValuePairComparator { + bool operator()(const CCValuePair& valuePair, const ValueType& value) + { + return (valuePair.value < value); + } + + bool operator()(const ValueType& value, const CCValuePair& valuePair) + { + return (value < valuePair.value); + } + + bool operator()(const CCValuePair& lhs, const CCValuePair& rhs) + { + return (lhs.value < rhs.value); + } +}; + /** * @brief Converts cents to a pitch ratio * From 2e020cb21317fab1da06c0c333b5c7c520b018b5 Mon Sep 17 00:00:00 2001 From: Paul Fd Date: Sat, 7 Mar 2020 14:05:05 +0100 Subject: [PATCH 7/8] Added a benchmark for maps --- benchmarks/BM_maps.cpp | 350 ++++++++++++++++++++++++++++++++++++++ benchmarks/CMakeLists.txt | 2 + 2 files changed, 352 insertions(+) create mode 100644 benchmarks/BM_maps.cpp diff --git a/benchmarks/BM_maps.cpp b/benchmarks/BM_maps.cpp new file mode 100644 index 00000000..88edd380 --- /dev/null +++ b/benchmarks/BM_maps.cpp @@ -0,0 +1,350 @@ +// 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 + +#include +#include +#include +#include +#include "../src/sfizz/Range.h" +#include +#include + +constexpr int maxCC { 256 }; + +class MyFixture : public benchmark::Fixture { +public: + void SetUp(const ::benchmark::State& state) + { + std::random_device rd {}; + std::mt19937 gen { rd() }; + std::uniform_real_distribution distFloat { 0.1f, 1.0f }; + std::uniform_int_distribution distInt { 1, maxCC }; + floats = std::vector(state.range(0)); + ccs = std::vector(state.range(0)); + ranges = std::vector>(state.range(0)); + absl::c_generate(floats, [&]() { + return distFloat(gen); + }); + absl::c_generate(ccs, [&]() { + return distInt(gen); + }); + absl::c_generate(ranges, [&]() { + return sfz::Range(distInt(gen), distInt(gen)); + }); + } + + void TearDown(const ::benchmark::State& state [[maybe_unused]]) + { + } + + std::vector ccs; + std::vector> ranges; + std::vector floats; +}; + +template +struct CCValuePair { + int cc; + ValueType value; +}; + +template +struct CCValuePairComparator { + bool operator()(const CCValuePair& valuePair, const int& cc) + { + return (valuePair.cc < cc); + } + + bool operator()(const int& cc, const CCValuePair& valuePair) + { + return (cc < valuePair.cc); + } + + bool operator()(const CCValuePair& lhs, const CCValuePair& rhs) + { + return (lhs.cc < rhs.cc); + } +}; + +template +struct CCValuePairComparator { + bool operator()(const CCValuePair& valuePair, const ValueType& value) + { + return (valuePair.value < value); + } + + bool operator()(const ValueType& value, const CCValuePair& valuePair) + { + return (value < valuePair.value); + } + + bool operator()(const CCValuePair& lhs, const CCValuePair& rhs) + { + return (lhs.value < rhs.value); + } +}; + +template +class CCMap { +public: + CCMap() = delete; + /** + * @brief Construct a new CCMap object with the specified default value. + * + * @param defaultValue + */ + CCMap(const ValueType& defaultValue) + : defaultValue(defaultValue) + { + } + CCMap(CCMap&&) = default; + CCMap(const CCMap&) = default; + ~CCMap() = default; + + /** + * @brief Returns the held object at the index, or a default value if not present + * + * @param index + * @return const ValueType& + */ + const ValueType& getWithDefault(int index) const noexcept + { + auto it = absl::c_lower_bound(container, index, CCValuePairComparator{}); + if (it == container.end() || it->cc != index) { + return defaultValue; + } else { + return it->value; + } + } + + /** + * @brief Get the value at index or emplace a new one if not present + * + * @param index the index of the element + * @return ValueType& + */ + ValueType& operator[](const int& index) noexcept + { + auto it = absl::c_lower_bound(container, index, CCValuePairComparator{}); + if (it == container.end() || it->cc != index) { + auto inserted = container.insert(it, { index, defaultValue }); + return inserted->value; + } else { + return it->value; + } + } + + /** + * @brief Is the container empty + * + * @return true + * @return false + */ + inline bool empty() const { return container.empty(); } + /** + * @brief Returns true if the container containers an element at index + * + * @param index + * @return true + * @return false + */ + bool contains(int index) const noexcept + { + return absl::c_binary_search(container, index, CCValuePairComparator{}); + } + typename std::vector>::const_iterator begin() const { return container.cbegin(); } + typename std::vector>::const_iterator end() const { return container.cend(); } +private: + // typename std::vector>::iterator begin() { return container.begin(); } + // typename std::vector>::iterator end() { return container.end(); } + + const ValueType defaultValue; + std::vector> container; +}; + +BENCHMARK_DEFINE_F(MyFixture, FillVector_Float) +(benchmark::State& state) +{ + for (auto _ : state) { + CCMap map { 0 }; + for (int i = 0; i < state.range(0); ++i) + map[ccs[i]] = floats[i]; + } +} + +BENCHMARK_DEFINE_F(MyFixture, FillVector_Range) +(benchmark::State& state) +{ + for (auto _ : state) { + CCMap> map { sfz::Range(0, 127) }; + for (int i = 0; i < state.range(0); ++i) + map[ccs[i]] = ranges[i]; + } +} + +BENCHMARK_DEFINE_F(MyFixture, FillAbseilFlatHM_Float) +(benchmark::State& state) +{ + for (auto _ : state) { + absl::flat_hash_map map; + for (int i = 0; i < state.range(0); ++i) + map[ccs[i]] = floats[i]; + } +} + +BENCHMARK_DEFINE_F(MyFixture, FillAbseilFlatHM_Range) +(benchmark::State& state) +{ + for (auto _ : state) { + absl::flat_hash_map> map; + for (int i = 0; i < state.range(0); ++i) + map[ccs[i]] = ranges[i]; + } +} + +BENCHMARK_DEFINE_F(MyFixture, LookupBaseline_Float) +(benchmark::State& state) +{ + std::vector output; + output.resize(state.range(0)); + + std::vector map; + output.reserve(state.range(0)); + for (int i = 0; i < state.range(0); ++i) + map.push_back(floats[i]); + + for (auto _ : state) { + for (int i = 0; i < state.range(0); ++i) + output[i] = map[i]; + } +} + +BENCHMARK_DEFINE_F(MyFixture, LookupBaseline_Range) +(benchmark::State& state) +{ + std::vector> output; + output.resize(state.range(0)); + + std::vector> map; + output.reserve(state.range(0)); + for (int i = 0; i < state.range(0); ++i) + map.push_back(ranges[i]); + + for (auto _ : state) { + for (int i = 0; i < state.range(0); ++i) + output[i] = map[i]; + } +} + +BENCHMARK_DEFINE_F(MyFixture, LookupVector_Float) +(benchmark::State& state) +{ + std::vector output; + output.resize(state.range(0)); + + CCMap map { 0 }; + for (int i = 0; i < state.range(0); ++i) + map[ccs[i]] = floats[i]; + + for (auto _ : state) { + for (int i = 0; i < state.range(0); ++i) + output[i] = map[ccs[i]]; + } +} + +BENCHMARK_DEFINE_F(MyFixture, LookupVector_Range) +(benchmark::State& state) +{ + std::vector> output; + output.resize(state.range(0)); + + CCMap> map { sfz::Range(0, 127) }; + for (int i = 0; i < state.range(0); ++i) + map[ccs[i]] = ranges[i]; + + for (auto _ : state) { + for (int i = 0; i < state.range(0); ++i) + output[i] = map[ccs[i]]; + } +} + +BENCHMARK_DEFINE_F(MyFixture, LookupAbseilFlatHM_Float) +(benchmark::State& state) +{ + std::vector output; + output.resize(state.range(0)); + + absl::flat_hash_map map; + for (int i = 0; i < state.range(0); ++i) + map[ccs[i]] = floats[i]; + for (auto _ : state) { + for (int i = 0; i < state.range(0); ++i) + output[i] = map[ccs[i]]; + } +} + +BENCHMARK_DEFINE_F(MyFixture, LookupAbseilFlatHM_Range) +(benchmark::State& state) +{ + std::vector> output; + output.resize(state.range(0)); + + absl::flat_hash_map> map; + for (int i = 0; i < state.range(0); ++i) + map[ccs[i]] = ranges[i]; + + for (auto _ : state) { + for (int i = 0; i < state.range(0); ++i) + output[i] = map[ccs[i]]; + } +} + + +BENCHMARK_DEFINE_F(MyFixture, IterateVector_Float) +(benchmark::State& state) +{ + std::vector output; + output.reserve(maxCC); + + CCMap map { 0 }; + for (int i = 0; i < state.range(0); ++i) + map[ccs[i]] = floats[i]; + + for (auto _ : state) { + for (auto& pair: map) + output.push_back(pair.value); + } +} + +BENCHMARK_DEFINE_F(MyFixture, IterateAbseilFlatHM_Float) +(benchmark::State& state) +{ + std::vector output; + output.reserve(maxCC); + + absl::flat_hash_map map; + for (int i = 0; i < state.range(0); ++i) + map[ccs[i]] = floats[i]; + for (auto _ : state) { + for (auto& pair: map) + output.push_back(pair.second); + } +} + + +BENCHMARK_REGISTER_F(MyFixture, FillVector_Float)->RangeMultiplier(2)->Range(16, 512); +// BENCHMARK_REGISTER_F(MyFixture, FillVector_Range)->RangeMultiplier(2)->Range(16, 512); +BENCHMARK_REGISTER_F(MyFixture, FillAbseilFlatHM_Float)->RangeMultiplier(2)->Range(16, 512); +// BENCHMARK_REGISTER_F(MyFixture, FillAbseilFlatHM_Range)->RangeMultiplier(2)->Range(16, 512); +BENCHMARK_REGISTER_F(MyFixture, LookupBaseline_Float)->RangeMultiplier(2)->Range(16, 512); +// BENCHMARK_REGISTER_F(MyFixture, LookupBaseline_Range)->RangeMultiplier(2)->Range(16, 512); +BENCHMARK_REGISTER_F(MyFixture, LookupVector_Float)->RangeMultiplier(2)->Range(16, 512); +// BENCHMARK_REGISTER_F(MyFixture, LookupVector_Range)->RangeMultiplier(2)->Range(16, 512); +BENCHMARK_REGISTER_F(MyFixture, LookupAbseilFlatHM_Float)->RangeMultiplier(2)->Range(16, 512); +// BENCHMARK_REGISTER_F(MyFixture, LookupAbseilFlatHM_Range)->RangeMultiplier(2)->Range(16, 512); +BENCHMARK_REGISTER_F(MyFixture, IterateVector_Float)->Range(maxCC, maxCC); +BENCHMARK_REGISTER_F(MyFixture, IterateAbseilFlatHM_Float)->Range(maxCC, maxCC); +BENCHMARK_MAIN(); diff --git a/benchmarks/CMakeLists.txt b/benchmarks/CMakeLists.txt index b143ac5a..8931eca6 100644 --- a/benchmarks/CMakeLists.txt +++ b/benchmarks/CMakeLists.txt @@ -58,6 +58,8 @@ sfizz_add_benchmark(bm_diff BM_diff.cpp) sfizz_add_benchmark(bm_widthPos BM_widthPos.cpp) sfizz_add_benchmark(bm_interpolationCast BM_interpolationCast.cpp) sfizz_add_benchmark(bm_pointerIterationOrOffsets BM_pointerIterationOrOffsets.cpp) +sfizz_add_benchmark(bm_maps BM_maps.cpp) +target_link_libraries(bm_maps PRIVATE absl::flat_hash_map) sfizz_add_benchmark(bm_logger BM_logger.cpp) target_link_libraries(bm_logger PRIVATE sfizz::sfizz) From 57eadc7a93814c0d348b60ecaf8a9a0f44a19671 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Sat, 7 Mar 2020 14:18:47 +0100 Subject: [PATCH 8/8] Global definition of LIBATOMIC_FOUND for arm benchmarks --- benchmarks/CMakeLists.txt | 5 ++++- cmake/SfizzConfig.cmake | 5 +++++ src/CMakeLists.txt | 8 ++------ 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/benchmarks/CMakeLists.txt b/benchmarks/CMakeLists.txt index 8931eca6..0aa090ae 100644 --- a/benchmarks/CMakeLists.txt +++ b/benchmarks/CMakeLists.txt @@ -27,7 +27,10 @@ macro(sfizz_add_benchmark TARGET) target_link_libraries("${TARGET}" PRIVATE absl::span absl::algorithm PRIVATE benchmark::benchmark benchmark::benchmark_main - PRIVATE bm_simd bm_ftz) + PRIVATE bm_simd bm_ftz) + if (LIBATOMIC_FOUND) + target_link_libraries ("${TARGET}" PRIVATE atomic) + endif() target_include_directories("${TARGET}" PRIVATE ../src/sfizz ../src/external) endmacro() diff --git a/cmake/SfizzConfig.cmake b/cmake/SfizzConfig.cmake index 19215e3a..0f5fc728 100644 --- a/cmake/SfizzConfig.cmake +++ b/cmake/SfizzConfig.cmake @@ -66,6 +66,11 @@ if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND NOT ANDROID) endif() endif() +include (CheckLibraryExists) +if (UNIX AND NOT APPLE) + check_library_exists(atomic __atomic_load "" LIBATOMIC_FOUND) +endif() + # Don't show build information when building a different project function (show_build_info_if_needed) if (CMAKE_PROJECT_NAME STREQUAL "sfizz") diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index fcaeca79..2475fd65 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -1,5 +1,4 @@ include (GNUInstallDirs) -include (CheckLibraryExists) set (SFIZZ_SOURCES sfizz/Synth.cpp @@ -55,11 +54,8 @@ endif() add_library (sfizz::parser ALIAS sfizz_parser) add_library (sfizz::sfizz ALIAS sfizz_static) -if (UNIX AND NOT APPLE) - check_library_exists(atomic __atomic_load "" LIBATOMIC_FOUND) - if (LIBATOMIC_FOUND) - target_link_libraries (sfizz_static PRIVATE atomic) - endif() +if (LIBATOMIC_FOUND) + target_link_libraries (sfizz_static PRIVATE atomic) endif() # Shared library and installation target