Merge pull request #420 from jpcima/midnam-fixes

Midnam fixes

src/sfizz/SfzHelpers.h

@@ -7,6 +7,7 @@
 #pragma once
 #include <absl/types/optional.h>
 #include <absl/strings/string_view.h>
+#include <absl/algorithm/container.h>
 //#include <string>
 #include <array>
 #include <cmath>
@@ -202,6 +203,34 @@ inline CXX14_CONSTEXPR Type vaGain(Type cutoff, Type sampleRate)
     return std::tan(cutoff / sampleRate * pi<Type>());
 }
 
+/**
+ * @brief Insert an item uniquely into a vector of pairs.
+ *
+ * @param pairVector the vector of pairs
+ * @param key the unique key
+ * @param value the value
+ * @param replace whether to replace the value if the key is already present
+ * @return whether the item was inserted
+ */
+template <class P, class T, class U>
+bool insertPairUniquely(std::vector<P>& pairVector, const T& key, U value, bool replace = true)
+{
+    bool result = false;
+    auto it = absl::c_find_if(
+        pairVector, [&key](const P& pair) { return pair.first == key; });
+    if (it != pairVector.end()) {
+        if (replace) {
+            it->second = std::move(value);
+            result = true;
+        }
+    }
+    else {
+        pairVector.emplace_back(key, std::move(value));
+        result = true;
+    }
+    return result;
+}
+
 /**
  * @brief From a source view, find the next sfz header and its members and
  *          return them, while updating the source by removing this header

src/sfizz/Synth.cpp

@@ -231,6 +231,10 @@ void sfz::Synth::clear()
     // set default controllers
     cc(0, 7, 100);     // volume
     hdcc(0, 10, 0.5f); // pan
+
+    // set default controller labels
+    insertPairUniquely(ccLabels, 7, "Volume");
+    insertPairUniquely(ccLabels, 10, "Pan");
 }
 
 void sfz::Synth::handleMasterOpcodes(const std::vector<Opcode>& members)
@@ -332,12 +336,12 @@ void sfz::Synth::handleControlOpcodes(const std::vector<Opcode>& members)
             break;
         case hash("label_cc&"):
             if (Default::ccNumberRange.containsWithEnd(member.parameters.back()))
-                ccLabels.emplace_back(member.parameters.back(), std::string(member.value));
+                insertPairUniquely(ccLabels, member.parameters.back(), std::string(member.value));
             break;
         case hash("label_key&"):
             if (member.parameters.back() <= Default::keyRange.getEnd()) {
                 const auto noteNumber = static_cast<uint8_t>(member.parameters.back());
-                keyLabels.emplace_back(noteNumber, std::string(member.value));
+                insertPairUniquely(keyLabels, noteNumber, std::string(member.value));
             }
             break;
         case hash("default_path"):
@@ -541,7 +545,7 @@ void sfz::Synth::finalizeSfzLoad()
         }
 
         if (region->keyswitchLabel && region->keyswitch)
-            keyswitchLabels.push_back({ *region->keyswitch, *region->keyswitchLabel });
+            insertPairUniquely(keyswitchLabels, *region->keyswitch, *region->keyswitchLabel);
 
         // Some regions had group number but no "group-level" opcodes handled the polyphony
         while (polyphonyGroups.size() <= region->group) {
@@ -1257,13 +1261,15 @@ std::string sfz::Synth::exportMidnam(absl::string_view model) const
         cns.append_attribute("Name").set_value("Controls");
         for (const auto& pair : ccLabels) {
             anonymousCCs.set(pair.first, false);
-            pugi::xml_node cn = cns.append_child("Control");
+            if (pair.first < 128) {
-            cn.append_attribute("Type").set_value("7bit");
+                pugi::xml_node cn = cns.append_child("Control");
-            cn.append_attribute("Number").set_value(std::to_string(pair.first).c_str());
+                cn.append_attribute("Type").set_value("7bit");
-            cn.append_attribute("Name").set_value(pair.second.c_str());
+                cn.append_attribute("Number").set_value(std::to_string(pair.first).c_str());
+                cn.append_attribute("Name").set_value(pair.second.c_str());
+            }
         }
 
-        for (unsigned i = 0; i < anonymousCCs.size(); ++i) {
+        for (unsigned i = 0, n = std::min<unsigned>(128, anonymousCCs.size()); i < n; ++i) {
             if (anonymousCCs[i]) {
                 pugi::xml_node cn = cns.append_child("Control");
                 cn.append_attribute("Type").set_value("7bit");

tests/FilesT.cpp

@@ -591,11 +591,9 @@ TEST_CASE("[Files] Labels")
     REQUIRE( keyLabels[0].second == "Cymbals" );
     REQUIRE( keyLabels[1].first == 65 );
     REQUIRE( keyLabels[1].second == "Crash" );
-    REQUIRE( ccLabels.size() == 2);
+    REQUIRE( ccLabels.size() >= 2);
-    REQUIRE( ccLabels[0].first == 54 );
+    REQUIRE( absl::c_find(ccLabels, CCNamePair { 54, "Gain" }) != ccLabels.end() );
-    REQUIRE( ccLabels[0].second == "Gain" );
+    REQUIRE( absl::c_find(ccLabels, CCNamePair { 2, "Other" }) != ccLabels.end() );
-    REQUIRE( ccLabels[1].first == 2 );
-    REQUIRE( ccLabels[1].second == "Other" );
     const std::string xmlMidnam = synth.exportMidnam();
     REQUIRE(xmlMidnam.find("<Note Number=\"12\" Name=\"Cymbals\" />") != xmlMidnam.npos);
     REQUIRE(xmlMidnam.find("<Note Number=\"65\" Name=\"Crash\" />") != xmlMidnam.npos);
@@ -612,3 +610,24 @@ TEST_CASE("[Files] Switch labels")
     REQUIRE(xmlMidnam.find("<Note Number=\"38\" Name=\"Triangle\" />") != xmlMidnam.npos);
     REQUIRE(xmlMidnam.find("<Note Number=\"40\" Name=\"Saw\" />") != xmlMidnam.npos);
 }
+
+TEST_CASE("[Files] Duplicate labels")
+{
+    sfz::Synth synth;
+    synth.loadSfzString(
+        fs::current_path() / "tests/TestFiles/labels.sfz",
+        R"(<control> label_key60=Baz label_key60=Quux
+<control> label_cc20=Foo label_cc20=Bar
+<region> sample=*sine)");
+
+    auto keyLabels = synth.getKeyLabels();
+    auto ccLabels = synth.getCCLabels();
+    REQUIRE( keyLabels.size() == 1);
+    REQUIRE( keyLabels[0].first == 60 );
+    REQUIRE( keyLabels[0].second == "Quux" );
+    REQUIRE( ccLabels.size() >= 1);
+    REQUIRE( absl::c_find(ccLabels, CCNamePair { 20, "Bar" }) != ccLabels.end() );
+    const std::string xmlMidnam = synth.exportMidnam();
+    REQUIRE(xmlMidnam.find("<Note Number=\"60\" Name=\"Quux\" />") != xmlMidnam.npos);
+    REQUIRE(xmlMidnam.find("<Control Type=\"7bit\" Number=\"20\" Name=\"Bar\" />") != xmlMidnam.npos);
+}