From 5e74f910c66ca426400aa13f303abae0f944e897 Mon Sep 17 00:00:00 2001 From: Paul Ferrand Date: Fri, 29 May 2020 23:30:53 +0200 Subject: [PATCH] Correct a bug with sfz v1 velcurve The velcurve logic now uses the same as sfz v2 curves --- src/sfizz/Curve.cpp | 29 ++++++++++++++++++++++++++ src/sfizz/Curve.h | 11 ++++++++++ src/sfizz/Region.cpp | 13 +++--------- src/sfizz/Region.h | 4 +++- src/sfizz/Synth.cpp | 24 +++------------------ tests/RegionT.cpp | 8 +++++-- tests/SynthT.cpp | 28 +++++++++++++++++++++++++ tests/TestFiles/velocity_endpoints.sfz | 2 ++ 8 files changed, 85 insertions(+), 34 deletions(-) create mode 100644 tests/TestFiles/velocity_endpoints.sfz diff --git a/src/sfizz/Curve.cpp b/src/sfizz/Curve.cpp index f349efad..eaa22cdf 100644 --- a/src/sfizz/Curve.cpp +++ b/src/sfizz/Curve.cpp @@ -57,6 +57,35 @@ Curve Curve::buildCurveFromHeader( return curve; } +Curve Curve::buildFromVelcurvePoints( + const std::vector>& points, + Interpolator itp, bool invert) +{ + Curve curve; + bool fillStatus[NumValues] = {}; + const Range fullRange { -HUGE_VALF, +HUGE_VALF }; + + auto setPoint = [&curve, &fillStatus](int i, float x) { + curve._points[i] = x; + fillStatus[i] = true; + }; + + if (invert) { + setPoint(0, 1.0); + setPoint(NumValues - 1, 0.0); + } else { + setPoint(0, 0.0); + setPoint(NumValues - 1, 1.0); + } + + for (const auto& point: points) { + setPoint(point.first, point.second); + } + + curve.fill(itp, fillStatus); + return curve; +} + Curve Curve::buildPredefinedCurve(int index) { Curve curve; diff --git a/src/sfizz/Curve.h b/src/sfizz/Curve.h index 829a2d8b..d75e1a67 100644 --- a/src/sfizz/Curve.h +++ b/src/sfizz/Curve.h @@ -67,6 +67,17 @@ public: absl::Span members, Interpolator itp = Interpolator::Linear, bool limit = false); + /** + * @brief Build a curve based on sfz v1 "amp_velcurve_&" points + * + * @param points the points vector + * @param itp kind of interpolator to fill between values + * @param invert whether to invert the curve + */ + static Curve buildFromVelcurvePoints( + const std::vector>& points, + Interpolator itp = Interpolator::Linear, bool invert = false); + /** * @brief Number of predefined curves */ diff --git a/src/sfizz/Region.cpp b/src/sfizz/Region.cpp index afd438bd..63fc5676 100644 --- a/src/sfizz/Region.cpp +++ b/src/sfizz/Region.cpp @@ -388,7 +388,7 @@ bool sfz::Region::parseOpcode(const Opcode& rawOpcode) return false; if (value) - velocityPoints.emplace_back(normalizeVelocity(opcode.parameters.back()), *value); + velocityPoints.emplace_back(opcode.parameters.back(), *value); } break; case hash("xfin_lokey"): @@ -1130,15 +1130,8 @@ float sfz::Region::velocityCurve(float velocity) const noexcept ASSERT(velocity >= 0.0f && velocity <= 1.0f); float gain { 1.0f }; - if (velocityPoints.size() > 0) { // Custom velocity curve - auto after = absl::c_find_if(velocityPoints, [velocity](const std::pair& val) { return val.first >= velocity; }); - auto before = after == velocityPoints.begin() ? velocityPoints.begin() : after - 1; - // Linear interpolation - float relativePositionInSegment { - (velocity - before->first) / (after->first - before->first) - }; - float segmentEndpoints { after->second - before->second }; - gain *= relativePositionInSegment * segmentEndpoints; + if (velCurve) { // Custom velocity curve + return velCurve->evalNormalized(velocity); } else { // Standard velocity curve // FIXME: Maybe there's a prettier way to check the boundaries? const float gaindB = [&]() { diff --git a/src/sfizz/Region.h b/src/sfizz/Region.h index b532f712..a3e8a096 100644 --- a/src/sfizz/Region.h +++ b/src/sfizz/Region.h @@ -6,6 +6,7 @@ #pragma once #include "CCMap.h" +#include "Curve.h" #include "LeakDetector.h" #include "Defaults.h" #include "EGDescription.h" @@ -304,7 +305,8 @@ struct Region { uint8_t ampKeycenter { Default::ampKeycenter }; // amp_keycenter float ampKeytrack { Default::ampKeytrack }; // amp_keytrack float ampVeltrack { Default::ampVeltrack }; // amp_keytrack - std::vector> velocityPoints; // amp_velcurve_N + std::vector> velocityPoints; // amp_velcurve_N + absl::optional velCurve {}; float ampRandom { Default::ampRandom }; // amp_random Range crossfadeKeyInRange { Default::crossfadeKeyInRange }; Range crossfadeKeyOutRange { Default::crossfadeKeyOutRange }; diff --git a/src/sfizz/Synth.cpp b/src/sfizz/Synth.cpp index 41f72086..4fd0743c 100644 --- a/src/sfizz/Synth.cpp +++ b/src/sfizz/Synth.cpp @@ -315,26 +315,6 @@ void sfz::Synth::handleEffectOpcodes(const std::vector& rawMembers) bus.addEffect(std::move(fx)); } -void addEndpointsToVelocityCurve(sfz::Region& region) -{ - if (region.velocityPoints.size() > 0) { - const auto velocityStart = sfz::Default::velocityRange.getStart(); - const auto velocityEnd = sfz::Default::velocityRange.getEnd(); - absl::c_sort(region.velocityPoints, [](const std::pair& lhs, const std::pair& rhs) { return lhs.first < rhs.first; }); - if (region.ampVeltrack > 0) { - if (region.velocityPoints.front().first != velocityStart) - region.velocityPoints.insert(region.velocityPoints.begin(), std::make_pair(velocityStart, velocityStart)); - if (region.velocityPoints.back().first != velocityEnd) - region.velocityPoints.push_back(std::make_pair(velocityEnd, velocityEnd)); - } else { - if (region.velocityPoints.front().first != velocityEnd) - region.velocityPoints.insert(region.velocityPoints.begin(), std::make_pair(velocityEnd, velocityStart)); - if (region.velocityPoints.back().first != velocityStart) - region.velocityPoints.push_back(std::make_pair(velocityStart, velocityEnd)); - } - } -} - bool sfz::Synth::loadSfzFile(const fs::path& file) { clear(); @@ -484,7 +464,9 @@ void sfz::Synth::finalizeSfzLoad() } } - addEndpointsToVelocityCurve(*region); + if (!region->velocityPoints.empty()) + region->velCurve = Curve::buildFromVelcurvePoints( + region->velocityPoints, Curve::Interpolator::Linear, region->ampVeltrack < 0.0f); region->registerPitchWheel(0); region->registerAftertouch(0); region->registerTempo(2.0f); diff --git a/tests/RegionT.cpp b/tests/RegionT.cpp index 4884274b..c18638cd 100644 --- a/tests/RegionT.cpp +++ b/tests/RegionT.cpp @@ -694,9 +694,13 @@ TEST_CASE("[Region] Parsing opcodes") SECTION("amp_velcurve") { region.parseOpcode({ "amp_velcurve_6", "0.4" }); - REQUIRE(region.velocityPoints.back() == std::make_pair(6_norm, 0.4f)); + REQUIRE(region.velocityPoints.back() == std::pair(6, 0.4f)); region.parseOpcode({ "amp_velcurve_127", "-1.0" }); - REQUIRE(region.velocityPoints.back() == std::make_pair(127_norm, 0.0f)); + REQUIRE(region.velocityPoints.back() == std::pair(127, 0.0f)); + region.parseOpcode({ "amp_velcurve_008", "0.3" }); + REQUIRE(region.velocityPoints.back() == std::pair(8, 0.3f)); + region.parseOpcode({ "amp_velcurve_064", "0.9" }); + REQUIRE(region.velocityPoints.back() == std::pair(64, 0.9f)); } SECTION("xfin_lokey, xfin_hikey") diff --git a/tests/SynthT.cpp b/tests/SynthT.cpp index e89ef65e..e59df54f 100644 --- a/tests/SynthT.cpp +++ b/tests/SynthT.cpp @@ -434,3 +434,31 @@ TEST_CASE("[Synth] Basic curves") // Default linear REQUIRE( curves.getCurve(16).evalCC7(63) == Approx(63_norm) ); } + +TEST_CASE("[Synth] Velocity points") +{ + sfz::Synth synth; + synth.loadSfzFile(fs::current_path() / "tests/TestFiles/velocity_endpoints.sfz"); + REQUIRE( !synth.getRegionView(0)->velocityPoints.empty()); + REQUIRE( synth.getRegionView(0)->velocityPoints[0].first == 64 ); + REQUIRE( synth.getRegionView(0)->velocityPoints[0].second == 1.0_a ); + REQUIRE( !synth.getRegionView(1)->velocityPoints.empty()); + REQUIRE( synth.getRegionView(1)->velocityPoints[0].first == 64 ); + REQUIRE( synth.getRegionView(1)->velocityPoints[0].second == 1.0_a ); +} + +TEST_CASE("[Synth] velcurve") +{ + sfz::Synth synth; + synth.loadSfzFile(fs::current_path() / "tests/TestFiles/velocity_endpoints.sfz"); + REQUIRE( synth.getRegionView(0)->velocityCurve(0_norm) == 0.0_a ); + REQUIRE( synth.getRegionView(0)->velocityCurve(32_norm) == Approx(0.5f).margin(1e-2) ); + REQUIRE( synth.getRegionView(0)->velocityCurve(64_norm) == 1.0_a ); + REQUIRE( synth.getRegionView(0)->velocityCurve(96_norm) == 1.0_a ); + REQUIRE( synth.getRegionView(0)->velocityCurve(127_norm) == 1.0_a ); + REQUIRE( synth.getRegionView(1)->velocityCurve(0_norm) == 1.0_a ); + REQUIRE( synth.getRegionView(1)->velocityCurve(32_norm) == 1.0_a ); + REQUIRE( synth.getRegionView(1)->velocityCurve(64_norm) == 1.0_a ); + REQUIRE( synth.getRegionView(1)->velocityCurve(96_norm) == Approx(0.5f).margin(1e-2) ); + REQUIRE( synth.getRegionView(1)->velocityCurve(127_norm) == 0.0_a ); +} diff --git a/tests/TestFiles/velocity_endpoints.sfz b/tests/TestFiles/velocity_endpoints.sfz new file mode 100644 index 00000000..dd3808d7 --- /dev/null +++ b/tests/TestFiles/velocity_endpoints.sfz @@ -0,0 +1,2 @@ + amp_velcurve_064=1 sample=*sine + amp_velcurve_064=1 amp_veltrack=-100 sample=*sine