All parameters are in the vector now

and we store the positions for possible
future checks

src/sfizz/Opcode.cpp

@@ -15,28 +15,56 @@ sfz::Opcode::Opcode(absl::string_view inputOpcode, absl::string_view inputValue)
     trimInPlace(value);
     trimInPlace(opcode);
     size_t nextCharIndex { 0 };
+    int parameterPosition { 0 };
     auto nextNumIndex = opcode.find_first_of("1234567890");
     while (nextNumIndex != opcode.npos) {
-        lettersOnlyHash = hash(opcode.substr(nextCharIndex, nextNumIndex - nextCharIndex), lettersOnlyHash);
+        const auto numLetters = nextNumIndex - nextCharIndex;
+        parameterPosition += numLetters;
+        lettersOnlyHash = hash(opcode.substr(nextCharIndex, numLetters), lettersOnlyHash);
         nextCharIndex = opcode.find_first_not_of("1234567890", nextNumIndex);
 
         uint32_t returnedValue;
-        if (nextCharIndex == absl::string_view::npos) {
+        hasBackParameter = (nextCharIndex == opcode.npos);
-            if (absl::SimpleAtoi(opcode.substr(nextNumIndex), &returnedValue)) {
+        const auto numDigits = hasBackParameter ? opcode.npos : nextCharIndex - nextNumIndex;
-                ASSERT(returnedValue < std::numeric_limits<uint8_t>::max());
+        if (absl::SimpleAtoi(opcode.substr(nextNumIndex, numDigits), &returnedValue)) {
-                backParameter = static_cast<uint8_t>(returnedValue);
+            ASSERT(returnedValue < std::numeric_limits<uint8_t>::max());
-                break;
+            parameterPositions.push_back(parameterPosition);
-            }
+            parameters.push_back(returnedValue);
-        } else {
-            if (absl::SimpleAtoi(opcode.substr(nextNumIndex, nextCharIndex - nextNumIndex), &returnedValue)) {
-                ASSERT(returnedValue < std::numeric_limits<uint8_t>::max());
-                parameters.push_back(static_cast<uint8_t>(returnedValue));
-            }
         }
+
         nextNumIndex = opcode.find_first_of("1234567890", nextCharIndex);
     }
 
     if (nextCharIndex != opcode.npos)
         lettersOnlyHash = hash(opcode.substr(nextCharIndex), lettersOnlyHash);
-
+}
+
+absl::optional<uint8_t> sfz::Opcode::backParameter() const noexcept
+{
+    if (hasBackParameter && !parameters.empty())
+        return parameters.back();
+
+    return {};
+}
+
+absl::optional<uint8_t> sfz::Opcode::firstParameter() const noexcept
+{
+    if (!hasBackParameter && !parameters.empty())
+        return parameters.front();
+
+    if (hasBackParameter && parameters.size() > 1)
+        return parameters.front();
+
+    return {};
+}
+
+absl::optional<uint8_t> sfz::Opcode::middleParameter() const noexcept
+{
+    if (!hasBackParameter && parameters.size() > 1)
+        return parameters[1];
+
+    if (hasBackParameter && parameters.size() > 2)
+        return parameters[1];
+
+    return {};
 }

src/sfizz/Opcode.h

@@ -20,20 +20,23 @@
 
 namespace sfz {
 /**
- * @brief Opcode description class; should be very lightweight to use
+ * @brief Opcode description class. The class parses the parameters
- * and move around. The class parses the parameters of the opcode
+ * of the opcode on construction.
- * on construction.
  *
  */
 struct Opcode {
     Opcode() = delete;
+    absl::optional<uint8_t> backParameter() const noexcept;
+    absl::optional<uint8_t> firstParameter() const noexcept;
+    absl::optional<uint8_t> middleParameter() const noexcept;
     Opcode(absl::string_view inputOpcode, absl::string_view inputValue);
     absl::string_view opcode {};
     absl::string_view value {};
     uint64_t lettersOnlyHash { Fnv1aBasis };
     // This is to handle the integer parameters of some opcodes
     std::vector<uint8_t> parameters;
-    absl::optional<uint8_t> backParameter {};
+    std::vector<int> parameterPositions;
+    bool hasBackParameter { false };
     LEAK_DETECTOR(Opcode);
 };
 
@@ -189,8 +192,9 @@ template <class ValueType>
 inline void setCCPairFromOpcode(const Opcode& opcode, absl::optional<CCValuePair>& target, const Range<ValueType>& validRange)
 {
     auto value = readOpcode(opcode.value, validRange);
-    if (value && opcode.backParameter && Default::ccNumberRange.containsWithEnd(*opcode.backParameter))
+    const auto backParameter = opcode.backParameter();
-        target = std::make_pair(*opcode.backParameter, *value);
+    if (value && backParameter && Default::ccNumberRange.containsWithEnd(*backParameter))
+        target = std::make_pair(*backParameter, *value);
     else
         target = {};
 }

src/sfizz/Region.cpp

@@ -17,9 +17,10 @@
 
 bool sfz::Region::parseOpcode(const Opcode& opcode)
 {
+    const auto backParameter = opcode.backParameter();
     // Check that the parameter is well formed
-    if (opcode.backParameter && !sfz::Default::ccNumberRange.containsWithEnd(*opcode.backParameter)) {
+    if (backParameter && !sfz::Default::ccNumberRange.containsWithEnd(*backParameter)) {
-        DBG("Wrong parameter value (" << std::to_string(*opcode.backParameter) << ") for opcode " << opcode.opcode);
+        DBG("Wrong parameter value (" << std::to_string(*backParameter) << ") for opcode " << opcode.opcode);
         return false;
     }
 
@@ -135,13 +136,13 @@ bool sfz::Region::parseOpcode(const Opcode& opcode)
         setRangeEndFromOpcode(opcode, bendRange, Default::bendRange);
         break;
     case hash("locc"):
-        if (opcode.backParameter) {
+        if (backParameter) {
-            setRangeStartFromOpcode(opcode, ccConditions[*opcode.backParameter], Default::ccValueRange);
+            setRangeStartFromOpcode(opcode, ccConditions[*backParameter], Default::ccValueRange);
         }
         break;
     case hash("hicc"):
-        if (opcode.backParameter)
+        if (backParameter)
-            setRangeEndFromOpcode(opcode, ccConditions[*opcode.backParameter], Default::ccValueRange);
+            setRangeEndFromOpcode(opcode, ccConditions[*backParameter], Default::ccValueRange);
         break;
     case hash("sw_lokey"):
         setRangeStartFromOpcode(opcode, keyswitchRange, Default::keyRange);
@@ -233,13 +234,13 @@ bool sfz::Region::parseOpcode(const Opcode& opcode)
         break;
     case hash("on_locc"):
     case hash("start_locc"):
-        if (opcode.backParameter)
+        if (backParameter)
-            setRangeStartFromOpcode(opcode, ccTriggers[*opcode.backParameter], Default::ccTriggerValueRange);
+            setRangeStartFromOpcode(opcode, ccTriggers[*backParameter], Default::ccTriggerValueRange);
         break;
     case hash("on_hicc"):
     case hash("start_hicc"):
-        if (opcode.backParameter)
+        if (backParameter)
-            setRangeEndFromOpcode(opcode, ccTriggers[*opcode.backParameter], Default::ccTriggerValueRange);
+            setRangeEndFromOpcode(opcode, ccTriggers[*backParameter], Default::ccTriggerValueRange);
         break;
 
     // Performance parameters: amplifier
@@ -293,7 +294,7 @@ bool sfz::Region::parseOpcode(const Opcode& opcode)
         {
             auto value = readOpcode(opcode.value, Default::ampVelcurveRange);
             if (value)
-                velocityPoints.emplace_back(*opcode.backParameter, *value);
+                velocityPoints.emplace_back(*backParameter, *value);
         }
         break;
     case hash("xfin_lokey"):
@@ -345,23 +346,23 @@ bool sfz::Region::parseOpcode(const Opcode& opcode)
         }
         break;
     case hash("xfin_locc"):
-        if (opcode.backParameter) {
+        if (backParameter) {
-            setRangeStartFromOpcode(opcode, crossfadeCCInRange[*opcode.backParameter], Default::ccValueRange);
+            setRangeStartFromOpcode(opcode, crossfadeCCInRange[*backParameter], Default::ccValueRange);
         }
         break;
     case hash("xfin_hicc"):
-        if (opcode.backParameter) {
+        if (backParameter) {
-            setRangeEndFromOpcode(opcode, crossfadeCCInRange[*opcode.backParameter], Default::ccValueRange);
+            setRangeEndFromOpcode(opcode, crossfadeCCInRange[*backParameter], Default::ccValueRange);
         }
         break;
     case hash("xfout_locc"):
-        if (opcode.backParameter) {
+        if (backParameter) {
-            setRangeStartFromOpcode(opcode, crossfadeCCOutRange[*opcode.backParameter], Default::ccValueRange);
+            setRangeStartFromOpcode(opcode, crossfadeCCOutRange[*backParameter], Default::ccValueRange);
         }
         break;
     case hash("xfout_hicc"):
-        if (opcode.backParameter) {
+        if (backParameter) {
-            setRangeEndFromOpcode(opcode, crossfadeCCOutRange[*opcode.backParameter], Default::ccValueRange);
+            setRangeEndFromOpcode(opcode, crossfadeCCOutRange[*backParameter], Default::ccValueRange);
         }
         break;
     case hash("xf_cccurve"):

src/sfizz/Synth.cpp

@@ -152,20 +152,21 @@ void sfz::Synth::handleGlobalOpcodes(const std::vector<Opcode>& members)
 void sfz::Synth::handleControlOpcodes(const std::vector<Opcode>& members)
 {
     for (auto& member : members) {
+        const auto backParameter = member.backParameter();
         switch (member.lettersOnlyHash) {
         case hash("Set_cc"):
             [[fallthrough]];
         case hash("set_cc"):
-            if (member.backParameter && Default::ccNumberRange.containsWithEnd(*member.backParameter)) {
+            if (backParameter && Default::ccNumberRange.containsWithEnd(*backParameter)) {
                 const auto ccValue = readOpcode(member.value, Default::ccValueRange).value_or(0);
-                midiState.ccEvent(*member.backParameter, ccValue);
+                midiState.ccEvent(*backParameter, ccValue);
             }
             break;
         case hash("Label_cc"):
             [[fallthrough]];
         case hash("label_cc"):
-            if (member.backParameter && Default::ccNumberRange.containsWithEnd(*member.backParameter))
+            if (backParameter && Default::ccNumberRange.containsWithEnd(*backParameter))
-                ccNames.emplace_back(*member.backParameter, std::string(member.value));
+                ccNames.emplace_back(*backParameter, std::string(member.value));
             break;
         case hash("Default_path"):
             [[fallthrough]];

tests/OpcodeT.cpp

@@ -17,7 +17,9 @@ TEST_CASE("[Opcode] Construction")
         REQUIRE(opcode.lettersOnlyHash == hash("sample"));
         REQUIRE(opcode.parameters.empty());
         REQUIRE(opcode.value == "dummy");
-        REQUIRE(!opcode.backParameter);
+        REQUIRE(!opcode.backParameter());
+        REQUIRE(!opcode.firstParameter());
+        REQUIRE(!opcode.middleParameter());
     }
 
     SECTION("Normal construction with underscore")
@@ -27,7 +29,9 @@ TEST_CASE("[Opcode] Construction")
         REQUIRE(opcode.lettersOnlyHash == hash("sample_underscore"));
         REQUIRE(opcode.parameters.empty());
         REQUIRE(opcode.value == "dummy");
-        REQUIRE(!opcode.backParameter);
+        REQUIRE(!opcode.backParameter());
+        REQUIRE(!opcode.firstParameter());
+        REQUIRE(!opcode.middleParameter());
     }
 
     SECTION("Parameterized opcode")
@@ -36,9 +40,13 @@ TEST_CASE("[Opcode] Construction")
         REQUIRE(opcode.opcode == "sample123");
         REQUIRE(opcode.lettersOnlyHash == hash("sample"));
         REQUIRE(opcode.value == "dummy");
-        REQUIRE(opcode.parameters.empty());
+        REQUIRE(opcode.parameters.size() == 1);
-        REQUIRE(opcode.backParameter);
+        REQUIRE(opcode.parameters == std::vector<uint8_t>({ 123 }));
-        REQUIRE(*opcode.backParameter == 123);
+        REQUIRE(opcode.parameterPositions == std::vector<int>({ 6 }));
+        REQUIRE(opcode.backParameter());
+        REQUIRE(*opcode.backParameter() == 123);
+        REQUIRE(!opcode.firstParameter());
+        REQUIRE(!opcode.middleParameter());
     }
 
     SECTION("Parameterized opcode with underscore")
@@ -47,9 +55,10 @@ TEST_CASE("[Opcode] Construction")
         REQUIRE(opcode.opcode == "sample_underscore123");
         REQUIRE(opcode.lettersOnlyHash == hash("sample_underscore"));
         REQUIRE(opcode.value == "dummy");
-        REQUIRE(opcode.parameters.empty());
+        REQUIRE(opcode.parameters == std::vector<uint8_t>({ 123 }));
-        REQUIRE(opcode.backParameter);
+        REQUIRE(opcode.parameterPositions == std::vector<int>({ 17 }));
-        REQUIRE(*opcode.backParameter == 123);
+        REQUIRE(opcode.backParameter());
+        REQUIRE(*opcode.backParameter() == 123);
     }
 
     SECTION("Parameterized opcode within the opcode")
@@ -58,9 +67,11 @@ TEST_CASE("[Opcode] Construction")
         REQUIRE(opcode.opcode == "sample1_underscore");
         REQUIRE(opcode.lettersOnlyHash == hash("sample_underscore"));
         REQUIRE(opcode.value == "dummy");
-        REQUIRE(opcode.parameters.size() == 1);
+        REQUIRE(opcode.parameters == std::vector<uint8_t>({ 1 }));
-        REQUIRE(opcode.parameters[0] == 1);
+        REQUIRE(!opcode.backParameter());
-        REQUIRE(!opcode.backParameter);
+        REQUIRE(opcode.firstParameter());
+        REQUIRE(*opcode.firstParameter() == 1);
+        REQUIRE(!opcode.middleParameter());
     }
 
     SECTION("Parameterized opcode within the opcode")
@@ -71,7 +82,6 @@ TEST_CASE("[Opcode] Construction")
         REQUIRE(opcode.value == "dummy");
         REQUIRE(opcode.parameters.size() == 1);
         REQUIRE(opcode.parameters[0] == 123);
-        REQUIRE(!opcode.backParameter);
     }
 
     SECTION("Parameterized opcode within the opcode twice")
@@ -83,7 +93,13 @@ TEST_CASE("[Opcode] Construction")
         REQUIRE(opcode.parameters.size() == 2);
         REQUIRE(opcode.parameters[0] == 123);
         REQUIRE(opcode.parameters[1] == 44);
-        REQUIRE(!opcode.backParameter);
+        REQUIRE(opcode.parameters == std::vector<uint8_t>({ 123, 44 }));
+        REQUIRE(opcode.parameterPositions == std::vector<int>({ 6, 13 }));
+        REQUIRE(!opcode.backParameter());
+        REQUIRE(opcode.firstParameter());
+        REQUIRE(*opcode.firstParameter() == 123);
+        REQUIRE(opcode.middleParameter());
+        REQUIRE(*opcode.middleParameter() == 44);
     }
 
     SECTION("Parameterized opcode within the opcode twice, with a back parameter")
@@ -92,11 +108,15 @@ TEST_CASE("[Opcode] Construction")
         REQUIRE(opcode.opcode == "sample123_double44_underscore23");
         REQUIRE(opcode.lettersOnlyHash == hash("sample_double_underscore"));
         REQUIRE(opcode.value == "dummy");
-        REQUIRE(opcode.parameters.size() == 2);
+        REQUIRE(opcode.parameters.size() == 3);
-        REQUIRE(opcode.parameters[0] == 123);
+        REQUIRE(opcode.parameters == std::vector<uint8_t>({ 123, 44, 23 }));
-        REQUIRE(opcode.parameters[1] == 44);
+        REQUIRE(opcode.parameterPositions == std::vector<int>({ 6, 13, 24 }));
-        REQUIRE(opcode.backParameter);
+        REQUIRE(opcode.backParameter());
-        REQUIRE(*opcode.backParameter == 23);
+        REQUIRE(*opcode.backParameter() == 23);
+        REQUIRE(opcode.firstParameter());
+        REQUIRE(*opcode.firstParameter() == 123);
+        REQUIRE(opcode.middleParameter());
+        REQUIRE(*opcode.middleParameter() == 44);
     }
 }