From 9bc9468baadb1370a08ebeef89d28600f9a75c16 Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Mon, 25 Apr 2022 22:27:21 +1000
Subject: [PATCH 01/24] Start GYB import, also fix uf allocation leak and other
 bank loader cleanup

---
 src/engine/engine.h       |   1 +
 src/engine/fileOpsIns.cpp | 106 ++++++++++++++++++++++++++++++--------
 src/gui/gui.cpp           |   4 +-
 3 files changed, 87 insertions(+), 24 deletions(-)

diff --git a/src/engine/engine.h b/src/engine/engine.h
index 4b5614a6f..8c968ea2a 100644
--- a/src/engine/engine.h
+++ b/src/engine/engine.h
@@ -394,6 +394,7 @@ class DivEngine {
   void loadOPNI(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath);
   void loadY12(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath);
   void loadBNK(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath);
+  void loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath);
   void loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath);
   void loadFF(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath);
 
diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index 521ab4249..01f17c6e3 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -34,6 +34,7 @@ enum DivInsFormats {
   DIV_INSFORMAT_OPLI,
   DIV_INSFORMAT_OPNI,
   DIV_INSFORMAT_BNK,
+  DIV_INSFORMAT_GYB,
   DIV_INSFORMAT_OPM,
   DIV_INSFORMAT_FF,
 };
@@ -855,6 +856,7 @@ void DivEngine::loadBNK(SafeReader& reader, std::vector<DivInstrument*>& ret, St
   bool is_adlib = ((header>>8) == 0x2d42494c444100L);
   bool is_failed = false;
   int readCount = 0;
+  int insCount = 0;
 
   if (is_adlib) {
     try {
@@ -868,7 +870,7 @@ void DivEngine::loadBNK(SafeReader& reader, std::vector<DivInstrument*>& ret, St
       while (reader.tell() < data_offset) {
         reader.seek(3, SEEK_CUR);
         instNames.push_back(new String(reader.readString(9)));
-        ++readCount;
+        ++insCount;
       }
 
       // Seek to BNK data
@@ -877,10 +879,9 @@ void DivEngine::loadBNK(SafeReader& reader, std::vector<DivInstrument*>& ret, St
       };
 
       // Read until EOF
-      for (int i = 0; i < readCount; ++i) {
+      for (int i = 0; i < insCount; ++i) {
         bnktimbre_t timbre;
-        insList.push_back(new DivInstrument);
-        auto& ins = insList[i];
+        DivInstrument *ins = new DivInstrument;
 
         ins->type = DIV_INS_OPL;
         ins->fm.ops = 2;
@@ -921,6 +922,9 @@ void DivEngine::loadBNK(SafeReader& reader, std::vector<DivInstrument*>& ret, St
         ins->fm.op[0].ws = reader.readC();
         ins->fm.op[1].ws = reader.readC();
         ins->name = instNames[i]->length() > 0 ? (*instNames[i]) : fmt::sprintf("%s[%d]", stripPath, i);
+
+        insList.push_back(ins);
+        ++readCount;
       }
       reader.seek(0, SEEK_END);
 
@@ -1023,6 +1027,57 @@ void DivEngine::loadFF(SafeReader& reader, std::vector<DivInstrument*>& ret, Str
   }
 }
 
+void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath) {
+  std::vector<DivInstrument*> insList;
+
+  int readCount = 0;
+  bool is_failed = false;
+  auto readInstOperator = [&](SafeReader& reader, DivInstrumentFM::Operator& op, bool skipRegB4) {
+  };
+
+  try {
+    reader.seek(0, SEEK_SET);
+    uint16_t header = reader.readS();
+    uint8_t insMelodyCount, insDrumCount;
+
+    if (header == 0x0C1A) { // 26 12 in decimal bytes
+      uint8_t version = reader.readC();
+      if ((version^3)>0) { // GYBv1/2
+        insMelodyCount = reader.readC();
+        insDrumCount = reader.readC();
+        if (!reader.seek(0x100, SEEK_CUR)) { // skip MIDI instrument mapping
+          throw EndOfFileException(&reader, reader.tell() + 0x100);
+        }
+
+        if (version == 2) {
+          reader.readC(); // skip LFO speed since global
+        }
+
+        for (int i = 0; i < (insMelodyCount+insDrumCount); ++i) {
+          // Note: melody and drum patches are interleaved...
+
+        }
+
+      } else { // GYBv3+
+
+      }
+    }
+    
+  } catch (EndOfFileException& e) {
+    lastError = "premature end of file";
+    logE("premature end of file");
+    for (int i = readCount - 1; i >= 0; --i) {
+      delete insList[i];
+    }
+    is_failed = true;
+  }
+
+  if (!is_failed) {
+    for (int i = 0; i < readCount; ++i) {
+      ret.push_back(insList[i]);
+    }
+  }
+}
 void DivEngine::loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath) {
   std::vector<DivInstrument*> insList;
 
@@ -1039,14 +1094,14 @@ void DivEngine::loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, St
 
   DivInstrument* newPatch = NULL;
   
-  auto completePatchRead = [&]() {
+  auto completePatchRead = [&]() -> bool {
     return patchNameRead && lfoRead && characteristicRead && m1Read && c1Read && m2Read && c2Read;
   };
   auto resetPatchRead = [&]() {
     patchNameRead = lfoRead = characteristicRead = m1Read = c1Read = m2Read = c2Read = false;
     newPatch = NULL;
   };
-  auto readIntStrWithinRange = [](String&& input, int limitLow, int limitHigh) {
+  auto readIntStrWithinRange = [](String&& input, int limitLow, int limitHigh) -> int {
     int x = std::stoi(input.c_str());
     if (x > limitHigh || x < limitLow) {
       throw std::invalid_argument(fmt::sprintf("%s is out of bounds of range [%d..%d]", input, limitLow, limitHigh));
@@ -1156,6 +1211,7 @@ void DivEngine::loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, St
       addWarning("Last OPM patch read was incomplete and therefore not imported.");
       logW("Last OPM patch read was incomplete and therefore not imported.");
       delete newPatch;
+      newPatch = NULL;
     }
 
     for (int i = 0; i < readCount; ++i) {
@@ -1297,31 +1353,33 @@ std::vector<DivInstrument*> DivEngine::instrumentFromFile(const char* path) {
         }
         extS+=i;
       }
-      if (extS==String(".dmp")) {
+      if (extS==".dmp") {
         format=DIV_INSFORMAT_DMP;
-      } else if (extS==String(".tfi")) {
+      } else if (extS==".tfi") {
         format=DIV_INSFORMAT_TFI;
-      } else if (extS==String(".vgi")) {
+      } else if (extS==".vgi") {
         format=DIV_INSFORMAT_VGI;
-      } else if (extS==String(".fti")) {
+      } else if (extS==".fti") {
         format=DIV_INSFORMAT_FTI;
-      } else if (extS==String(".bti")) {
+      } else if (extS==".bti") {
         format=DIV_INSFORMAT_BTI;
-      } else if (extS==String(".s3i")) {
+      } else if (extS==".s3i") {
         format=DIV_INSFORMAT_S3I;
-      } else if (extS==String(".sbi")) {
+      } else if (extS==".sbi") {
         format=DIV_INSFORMAT_SBI;
-      } else if (extS==String(".opli")) {
+      } else if (extS==".opli") {
         format=DIV_INSFORMAT_OPLI;
-      } else if (extS==String(".opni")) {
+      } else if (extS==".opni") {
         format=DIV_INSFORMAT_OPNI;;
-      } else if (extS==String(".y12")) {
+      } else if (extS==".y12") {
         format=DIV_INSFORMAT_Y12;
-      } else if (extS==String(".bnk")) {
+      } else if (extS==".bnk") {
         format=DIV_INSFORMAT_BNK;
-      } else if (extS==String(".opm")) {
+      } else if (extS==".gyb") {
+        format=DIV_INSFORMAT_GYB;
+      } else if (extS==".opm") {
         format=DIV_INSFORMAT_OPM;
-      } else if (extS==String(".ff")) {
+      } else if (extS==".ff") {
         format=DIV_INSFORMAT_FF;
       }
     }
@@ -1351,19 +1409,22 @@ std::vector<DivInstrument*> DivEngine::instrumentFromFile(const char* path) {
         loadOPLI(reader,ret,stripPath);
         break;
       case DIV_INSFORMAT_OPNI:
-        loadOPNI(reader, ret, stripPath);
+        loadOPNI(reader,ret,stripPath);
         break;
       case DIV_INSFORMAT_Y12:
         loadY12(reader,ret,stripPath);
         break;
       case DIV_INSFORMAT_BNK:
-        loadBNK(reader, ret, stripPath);
+        loadBNK(reader,ret,stripPath);
         break;
       case DIV_INSFORMAT_FF:
         loadFF(reader,ret,stripPath);
         break;
+      case DIV_INSFORMAT_GYB:
+        loadGYB(reader,ret,stripPath);
+        break;
       case DIV_INSFORMAT_OPM:
-        loadOPM(reader, ret, stripPath);
+        loadOPM(reader,ret,stripPath);
         break;
     }
 
@@ -1374,5 +1435,6 @@ std::vector<DivInstrument*> DivEngine::instrumentFromFile(const char* path) {
     }
   }
 
+  delete[] buf;
   return ret;
 }
diff --git a/src/gui/gui.cpp b/src/gui/gui.cpp
index edcac7fc5..c572af6c0 100644
--- a/src/gui/gui.cpp
+++ b/src/gui/gui.cpp
@@ -1305,9 +1305,9 @@ void FurnaceGUI::openFileDialog(FurnaceGUIFileDialogs type) {
       if (!dirExists(workingDirIns)) workingDirIns=getHomeDir();
       hasOpened=fileDialog->openLoad(
         "Load Instrument",
-        {"compatible files", "*.fui *.dmp *.tfi *.vgi *.s3i *.sbi *.opli *.opni *.y12 *.bnk *.ff *.opm",
+        {"compatible files", "*.fui *.dmp *.tfi *.vgi *.s3i *.sbi *.opli *.opni *.y12 *.bnk *.ff *.gyb *.opm",
          "all files", ".*"},
-        "compatible files{.fui,.dmp,.tfi,.vgi,.s3i,.sbi,.opli,.opni,.y12,.bnk,.ff,.opm},.*",
+        "compatible files{.fui,.dmp,.tfi,.vgi,.s3i,.sbi,.opli,.opni,.y12,.bnk,.ff,.gyb,.opm},.*",
         workingDirIns,
         dpiScale,
         [this](const char* path) {

From 744735cca858ca088808c9fefd3e7078b9b04a91 Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Mon, 25 Apr 2022 23:57:15 +1000
Subject: [PATCH 02/24] GYB progress, need to verify read ordering

---
 src/engine/fileOpsIns.cpp | 162 ++++++++++++++++++++++++++++++--------
 1 file changed, 128 insertions(+), 34 deletions(-)

diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index 01f17c6e3..e81a09f16 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -760,7 +760,9 @@ void DivEngine::loadOPNI(SafeReader& reader, std::vector<DivInstrument*>& ret, S
 
       String insName = reader.readString(32);
       ins->name = (insName.size() > 0) ? insName : stripPath;
-      reader.seek(3, SEEK_CUR);  // skip MIDI params
+      if (!reader.seek(3, SEEK_CUR)) {  // skip MIDI params
+        throw EndOfFileException(&reader, reader.tell() + 3);
+      }
       uint8_t feedAlgo = reader.readC();
       ins->fm.alg = (feedAlgo & 0x7);
       ins->fm.fb = ((feedAlgo>>3) & 0x7);
@@ -1032,7 +1034,58 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
 
   int readCount = 0;
   bool is_failed = false;
-  auto readInstOperator = [&](SafeReader& reader, DivInstrumentFM::Operator& op, bool skipRegB4) {
+  auto readInstrument = [](SafeReader& reader, bool readRegB4) -> DivInstrument* {
+    const int opOrder[] = {0,2,1,3};
+    DivInstrument* ins = new DivInstrument;
+    ins->type = DIV_INS_FM;
+    ins->fm.ops = 4;
+
+    // TODO see https://plutiedev.com/ym2612-registers 
+    // and https://github.com/Wohlstand/OPN2BankEditor/blob/master/Specifications/GYB-file-specification.txt
+
+    uint8_t tmp;
+    
+    for (int i : opOrder) {
+      tmp = reader.readC(); // MUL/DT
+      ins->fm.op[i].mult = tmp & 0xF;
+      ins->fm.op[i].dt = ((tmp>>4) & 0x7);
+    }
+    for (int i : opOrder) {
+      tmp = reader.readC(); // TL
+      ins->fm.op[i].tl = tmp & 0x7F;
+    }
+    for (int i : opOrder) {
+      tmp = reader.readC(); // AR/RS
+      ins->fm.op[i].ar = tmp & 0x1F;
+      ins->fm.op[i].rs = ((tmp>>6) & 0x3);
+    }
+    for (int i : opOrder) {
+      tmp = reader.readC(); // DR/AM-ENA
+      ins->fm.op[i].dr = tmp & 0x1F;
+      ins->fm.op[i].am = ((tmp>>7) & 0x1);
+    }
+    for (int i : opOrder) {
+      tmp = reader.readC(); // SR (D2R)
+      ins->fm.op[i].d2r = tmp & 0x1F;
+    }
+    for (int i : opOrder) {
+      tmp = reader.readC(); // RR/SL
+      ins->fm.op[i].rr = tmp & 0xF;
+      ins->fm.op[i].sl = ((tmp>>4) & 0xF);
+    }
+    for (int i : opOrder) {
+      tmp = reader.readC(); // SSG-EG
+      ins->fm.op[i].ssgEnv = tmp & 0xF;
+    }
+    // ALG/FB
+    tmp = reader.readC();
+    ins->fm.alg = tmp & 0x3;
+    ins->fm.fb = ((tmp>>3) & 0x3);
+
+    if (!readRegB4) { // PAN
+      reader.readC(); // skip as not kept in Furnace instrument
+    }
+    return ins;
   };
 
   try {
@@ -1045,21 +1098,74 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
       if ((version^3)>0) { // GYBv1/2
         insMelodyCount = reader.readC();
         insDrumCount = reader.readC();
+
         if (!reader.seek(0x100, SEEK_CUR)) { // skip MIDI instrument mapping
           throw EndOfFileException(&reader, reader.tell() + 0x100);
         }
 
         if (version == 2) {
-          reader.readC(); // skip LFO speed since global
+          reader.readC(); // skip LFO speed (chip-global)
         }
 
+        // Instrument data
         for (int i = 0; i < (insMelodyCount+insDrumCount); ++i) {
           // Note: melody and drum patches are interleaved...
-
+          DivInstrument* newIns = readInstrument(reader, (version==2));
+          reader.readC();  // skip transpose
+          if (version == 2) {
+            reader.readC();  // skip padding
+          }
+          insList.push_back(newIns);
+          ++readCount;
         }
 
-      } else { // GYBv3+
+        // Instrument name
+        for (int i = 0; i < (insMelodyCount+insDrumCount); ++i) {
+          uint8_t nameLen = reader.readC();
+          String insName = (nameLen > 0) ? reader.readString(nameLen) : fmt::sprintf("%s [%d]", stripPath, readCount);
 
+          // TODO determine if a patch is 'empty' -> ignore and move on
+          if (insList[i] == NULL) {
+            continue;
+          }
+          insList[i]->name = insName;
+        }
+      } else { // GYBv3+
+        reader.readC();  // skip LFO speed (chip-global)
+        uint32_t fileSize = reader.readI();
+        uint32_t bankOffset = reader.readI();
+        uint32_t mapOffset = reader.readI();
+
+        if (reader.size() != fileSize || bankOffset > fileSize || mapOffset > fileSize) {
+          lastError = "GYBv3 file appears to have invalid data offsets.";
+          logE("GYBv3 file appears to have invalid data offsets.");
+          return;
+        }
+
+        if (!reader.seek(bankOffset, SEEK_SET)) {
+          throw EndOfFileException(&reader, bankOffset);
+        }
+        uint16_t insCount = reader.readS();
+
+        for (int i = 0; i < insCount; ++i) {
+          DivInstrument* newIns = readInstrument(reader, true);
+          reader.readC(); // skip transpose
+          uint8_t additionalDataFlags = reader.readC() & 0x1; // skip additional data bitfield
+          // TODO if chord notes attached, skip this
+          if ((additionalDataFlags&1) > 0) {
+            uint8_t notes = reader.readC();
+            for (int j = 0; j < notes; ++j) {
+              reader.readC();
+            }
+          }
+          uint8_t nameLen = reader.readC();
+          String insName = (nameLen > 0) ? reader.readString(nameLen) : fmt::sprintf("%s [%d]", stripPath, readCount++);
+          // TODO determine if a patch is 'empty' -> ignore and move on
+          newIns->name = insName;
+          insList.push_back(newIns);
+        }
+
+        reader.seek(0, SEEK_END);
       }
     }
     
@@ -1074,7 +1180,9 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
 
   if (!is_failed) {
     for (int i = 0; i < readCount; ++i) {
-      ret.push_back(insList[i]);
+      if (insList[i] != NULL) {
+        ret.push_back(insList[i]);
+      }
     }
   }
 }
@@ -1106,8 +1214,7 @@ void DivEngine::loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, St
     if (x > limitHigh || x < limitLow) {
       throw std::invalid_argument(fmt::sprintf("%s is out of bounds of range [%d..%d]", input, limitLow, limitHigh));
     }
-    return (x>limitHigh) ? limitHigh :
-           (x<limitLow) ? limitLow : x;
+    return x;
   };
   auto readOpmOperator = [&](SafeReader& reader, DivInstrumentFM::Operator& op) {
     op.ar = readIntStrWithinRange(reader.readStringToken(), 0, 31);
@@ -1370,7 +1477,7 @@ std::vector<DivInstrument*> DivEngine::instrumentFromFile(const char* path) {
       } else if (extS==".opli") {
         format=DIV_INSFORMAT_OPLI;
       } else if (extS==".opni") {
-        format=DIV_INSFORMAT_OPNI;;
+        format=DIV_INSFORMAT_OPNI;
       } else if (extS==".y12") {
         format=DIV_INSFORMAT_Y12;
       } else if (extS==".bnk") {
@@ -1385,46 +1492,33 @@ std::vector<DivInstrument*> DivEngine::instrumentFromFile(const char* path) {
     }
 
     switch (format) {
-      case DIV_INSFORMAT_DMP: {
-        loadDMP(reader,ret,stripPath);
+      case DIV_INSFORMAT_DMP: loadDMP(reader,ret,stripPath);
         break;
-      }
-      case DIV_INSFORMAT_TFI:
-        loadTFI(reader,ret,stripPath);
+      case DIV_INSFORMAT_TFI: loadTFI(reader,ret,stripPath);
         break;
-      case DIV_INSFORMAT_VGI:
-        loadVGI(reader,ret,stripPath);
+      case DIV_INSFORMAT_VGI: loadVGI(reader,ret,stripPath);
         break;
       case DIV_INSFORMAT_FTI: // TODO
         break;
       case DIV_INSFORMAT_BTI: // TODO
         break;
-      case DIV_INSFORMAT_S3I:
-        loadS3I(reader,ret,stripPath);
+      case DIV_INSFORMAT_S3I: loadS3I(reader,ret,stripPath);
         break;
-      case DIV_INSFORMAT_SBI:
-        loadSBI(reader,ret,stripPath);
+      case DIV_INSFORMAT_SBI: loadSBI(reader,ret,stripPath);
         break;
-      case DIV_INSFORMAT_OPLI:
-        loadOPLI(reader,ret,stripPath);
+      case DIV_INSFORMAT_OPLI: loadOPLI(reader,ret,stripPath);
         break;
-      case DIV_INSFORMAT_OPNI:
-        loadOPNI(reader,ret,stripPath);
+      case DIV_INSFORMAT_OPNI: loadOPNI(reader,ret,stripPath);
         break;
-      case DIV_INSFORMAT_Y12:
-        loadY12(reader,ret,stripPath);
+      case DIV_INSFORMAT_Y12: loadY12(reader,ret,stripPath);
         break;
-      case DIV_INSFORMAT_BNK:
-        loadBNK(reader,ret,stripPath);
+      case DIV_INSFORMAT_BNK: loadBNK(reader,ret,stripPath);
         break;
-      case DIV_INSFORMAT_FF:
-        loadFF(reader,ret,stripPath);
+      case DIV_INSFORMAT_FF: loadFF(reader,ret,stripPath);
         break;
-      case DIV_INSFORMAT_GYB:
-        loadGYB(reader,ret,stripPath);
+      case DIV_INSFORMAT_GYB: loadGYB(reader,ret,stripPath);
         break;
-      case DIV_INSFORMAT_OPM:
-        loadOPM(reader,ret,stripPath);
+      case DIV_INSFORMAT_OPM: loadOPM(reader,ret,stripPath);
         break;
     }
 

From df1cb0e7fa29f99bdb22f00b2e58e562cb60fd8d Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Sat, 30 Apr 2022 14:58:21 +1000
Subject: [PATCH 03/24] HOTFIX: Better token handling and simplify clear string
 comparisons

---
 src/engine/fileOpsIns.cpp | 15 +++++++--------
 src/engine/safeReader.cpp |  6 +++---
 src/engine/safeReader.h   |  2 +-
 3 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index e81a09f16..6a163d7c7 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -1040,11 +1040,10 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
     ins->type = DIV_INS_FM;
     ins->fm.ops = 4;
 
-    // TODO see https://plutiedev.com/ym2612-registers 
+    // see https://plutiedev.com/ym2612-registers 
     // and https://github.com/Wohlstand/OPN2BankEditor/blob/master/Specifications/GYB-file-specification.txt
 
     uint8_t tmp;
-    
     for (int i : opOrder) {
       tmp = reader.readC(); // MUL/DT
       ins->fm.op[i].mult = tmp & 0xF;
@@ -1254,7 +1253,7 @@ void DivEngine::loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, St
       }
 
       // Read each line for their respective params. They may not be written in the same LINE order but they 
-      // must absolutely be properly grouped per patch! Line prefixes must be separated by a space! (see inline comments)
+      // must absolutely be properly grouped per patch! Line prefixes MUST be separated by a space! (see inline comments)
       
       if (token.size() >= 2) {
         if (token[0] == '@') {
@@ -1264,7 +1263,7 @@ void DivEngine::loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, St
           newPatch->name = newPatch->name.size() > 0 ? newPatch->name : fmt::sprintf("%s[%d]", stripPath, readCount);
           patchNameRead = true;
 
-        } else if (token.compare(0,3,"CH:") == 0) {
+        } else if (token == "CH:") {
           // CH: PAN FL CON AMS PMS SLOT NE
           reader.readStringToken(); // skip PAN
           newPatch->fm.fb = readIntStrWithinRange(reader.readStringToken(), 0, 7);
@@ -1275,22 +1274,22 @@ void DivEngine::loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, St
           reader.readStringToken(); // skip NE
           characteristicRead = true;
 
-        } else if (token.compare(0,3,"C1:") == 0) {
+        } else if (token == "C1:") {
           // C1: AR D1R D2R RR D1L TL KS MUL DT1 DT2 AMS-EN
           readOpmOperator(reader, newPatch->fm.op[2]);
           c1Read = true;
 
-        } else if (token.compare(0,3,"C2:") == 0) {
+        } else if (token == "C2:") {
           // C2: AR D1R D2R RR D1L TL KS MUL DT1 DT2 AMS-EN
           readOpmOperator(reader, newPatch->fm.op[3]);
           c2Read = true;
 
-        } else if (token.compare(0,3,"M1:") == 0) {
+        } else if (token == "M1:") {
           // M1: AR D1R D2R RR D1L TL KS MUL DT1 DT2 AMS-EN
           readOpmOperator(reader, newPatch->fm.op[0]);
           m1Read = true;
 
-        } else if (token.compare(0,3,"M2:") == 0) {
+        } else if (token == "M2:") {
           // M2: AR D1R D2R RR D1L TL KS MUL DT1 DT2 AMS-EN
           readOpmOperator(reader, newPatch->fm.op[1]);
           m2Read = true;
diff --git a/src/engine/safeReader.cpp b/src/engine/safeReader.cpp
index 36d6097d6..ebbb55ad9 100644
--- a/src/engine/safeReader.cpp
+++ b/src/engine/safeReader.cpp
@@ -179,7 +179,7 @@ String SafeReader::readStringLine() {
   return ret;
 }
 
-String SafeReader::readStringToken(unsigned char delim) {
+String SafeReader::readStringToken(unsigned char delim, bool stripContiguous) {
   String ret;
   unsigned char c;
   if (isEOF()) throw EndOfFileException(this, len);
@@ -189,7 +189,7 @@ String SafeReader::readStringToken(unsigned char delim) {
       break;
     }
     if (c == delim) {
-      if (ret.length() == 0) {
+      if (ret.length() == 0 && stripContiguous) {
         continue;
       }
       break;
@@ -200,5 +200,5 @@ String SafeReader::readStringToken(unsigned char delim) {
 }
 
 String SafeReader::readStringToken() {
-  return readStringToken(' ');
+  return readStringToken(' ', true);
 }
diff --git a/src/engine/safeReader.h b/src/engine/safeReader.h
index e21311f98..1f7149834 100644
--- a/src/engine/safeReader.h
+++ b/src/engine/safeReader.h
@@ -67,7 +67,7 @@ class SafeReader {
     String readString();
     String readString(size_t len);
     String readStringLine();
-    String readStringToken(unsigned char delim);
+    String readStringToken(unsigned char delim, bool stripContiguous);
     String readStringToken();
     inline bool isEOF() { return curSeek >= len; };
 

From f720e0af4798cd6a5334321abd77f86aca2e1dc8 Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Sat, 30 Apr 2022 16:00:29 +1000
Subject: [PATCH 04/24] GYBv1/2 working

---
 src/engine/fileOpsIns.cpp | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index 6a163d7c7..4fe9be3c5 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -1081,8 +1081,10 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
     ins->fm.alg = tmp & 0x3;
     ins->fm.fb = ((tmp>>3) & 0x3);
 
-    if (!readRegB4) { // PAN
-      reader.readC(); // skip as not kept in Furnace instrument
+    if (!readRegB4) { // PAN / PMS / AMS
+      tmp = reader.readC();
+      ins->fm.fms = tmp & 0x7;
+      ins->fm.ams = ((tmp>>4) & 0x3);
     }
     return ins;
   };
@@ -1098,6 +1100,10 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
         insMelodyCount = reader.readC();
         insDrumCount = reader.readC();
 
+        if (insMelodyCount > 128 || insDrumCount > 128) {
+          throw std::invalid_argument(fmt::sprintf("GYBv1/2 patch count is out of bounds."));
+        }
+
         if (!reader.seek(0x100, SEEK_CUR)) { // skip MIDI instrument mapping
           throw EndOfFileException(&reader, reader.tell() + 0x100);
         }
@@ -1108,27 +1114,24 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
 
         // Instrument data
         for (int i = 0; i < (insMelodyCount+insDrumCount); ++i) {
-          // Note: melody and drum patches are interleaved...
+          // TODO determine if a patch is 'empty' -> ignore and move on
+          bool isDrum = (i >= insMelodyCount);
           DivInstrument* newIns = readInstrument(reader, (version==2));
           reader.readC();  // skip transpose
           if (version == 2) {
             reader.readC();  // skip padding
           }
+
+          uint8_t nameLen = reader.readC();
+          String insName = (nameLen > 0) ? reader.readString(nameLen) : fmt::sprintf("%s [%d]", stripPath, readCount);
+          
+          newIns->name = insName;
           insList.push_back(newIns);
           ++readCount;
         }
 
-        // Instrument name
-        for (int i = 0; i < (insMelodyCount+insDrumCount); ++i) {
-          uint8_t nameLen = reader.readC();
-          String insName = (nameLen > 0) ? reader.readString(nameLen) : fmt::sprintf("%s [%d]", stripPath, readCount);
+        // Map to note assignment currently not supported.
 
-          // TODO determine if a patch is 'empty' -> ignore and move on
-          if (insList[i] == NULL) {
-            continue;
-          }
-          insList[i]->name = insName;
-        }
       } else { // GYBv3+
         reader.readC();  // skip LFO speed (chip-global)
         uint32_t fileSize = reader.readI();
@@ -1175,6 +1178,12 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
       delete insList[i];
     }
     is_failed = true;
+
+  } catch (std::invalid_argument& e) {
+    lastError = fmt::sprintf("Invalid value found in patch file. %s", e.what());
+    logE("Invalid value found in patch file.");
+    logE(e.what());
+    is_failed = true;
   }
 
   if (!is_failed) {

From c9f289e44a17594ab0ee058ae4414de7a10bf92a Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Sat, 30 Apr 2022 16:11:27 +1000
Subject: [PATCH 05/24] Fix GYBv1/2 instrument name read

---
 src/engine/fileOpsIns.cpp | 43 ++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index 4fe9be3c5..74623d185 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -1024,9 +1024,9 @@ void DivEngine::loadFF(SafeReader& reader, std::vector<DivInstrument*>& ret, Str
     return;
   }
 
-  for (unsigned int i = 0; i < insCount; ++i) {
-    ret.push_back(insList[i]);
-  }
+for (unsigned int i = 0; i < insCount; ++i) {
+  ret.push_back(insList[i]);
+}
 }
 
 void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath) {
@@ -1035,7 +1035,7 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
   int readCount = 0;
   bool is_failed = false;
   auto readInstrument = [](SafeReader& reader, bool readRegB4) -> DivInstrument* {
-    const int opOrder[] = {0,2,1,3};
+    const int opOrder[] = { 0,2,1,3 };
     DivInstrument* ins = new DivInstrument;
     ins->type = DIV_INS_FM;
     ins->fm.ops = 4;
@@ -1047,7 +1047,7 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
     for (int i : opOrder) {
       tmp = reader.readC(); // MUL/DT
       ins->fm.op[i].mult = tmp & 0xF;
-      ins->fm.op[i].dt = ((tmp>>4) & 0x7);
+      ins->fm.op[i].dt = ((tmp >> 4) & 0x7);
     }
     for (int i : opOrder) {
       tmp = reader.readC(); // TL
@@ -1056,12 +1056,12 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
     for (int i : opOrder) {
       tmp = reader.readC(); // AR/RS
       ins->fm.op[i].ar = tmp & 0x1F;
-      ins->fm.op[i].rs = ((tmp>>6) & 0x3);
+      ins->fm.op[i].rs = ((tmp >> 6) & 0x3);
     }
     for (int i : opOrder) {
       tmp = reader.readC(); // DR/AM-ENA
       ins->fm.op[i].dr = tmp & 0x1F;
-      ins->fm.op[i].am = ((tmp>>7) & 0x1);
+      ins->fm.op[i].am = ((tmp >> 7) & 0x1);
     }
     for (int i : opOrder) {
       tmp = reader.readC(); // SR (D2R)
@@ -1070,7 +1070,7 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
     for (int i : opOrder) {
       tmp = reader.readC(); // RR/SL
       ins->fm.op[i].rr = tmp & 0xF;
-      ins->fm.op[i].sl = ((tmp>>4) & 0xF);
+      ins->fm.op[i].sl = ((tmp >> 4) & 0xF);
     }
     for (int i : opOrder) {
       tmp = reader.readC(); // SSG-EG
@@ -1079,12 +1079,12 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
     // ALG/FB
     tmp = reader.readC();
     ins->fm.alg = tmp & 0x3;
-    ins->fm.fb = ((tmp>>3) & 0x3);
+    ins->fm.fb = ((tmp >> 3) & 0x3);
 
     if (!readRegB4) { // PAN / PMS / AMS
       tmp = reader.readC();
       ins->fm.fms = tmp & 0x7;
-      ins->fm.ams = ((tmp>>4) & 0x3);
+      ins->fm.ams = ((tmp >> 4) & 0x3);
     }
     return ins;
   };
@@ -1096,7 +1096,7 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
 
     if (header == 0x0C1A) { // 26 12 in decimal bytes
       uint8_t version = reader.readC();
-      if ((version^3)>0) { // GYBv1/2
+      if ((version ^ 3) > 0) { // GYBv1/2
         insMelodyCount = reader.readC();
         insDrumCount = reader.readC();
 
@@ -1113,23 +1113,25 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
         }
 
         // Instrument data
-        for (int i = 0; i < (insMelodyCount+insDrumCount); ++i) {
-          // TODO determine if a patch is 'empty' -> ignore and move on
+        for (int i = 0; i < (insMelodyCount + insDrumCount); ++i) {
           bool isDrum = (i >= insMelodyCount);
-          DivInstrument* newIns = readInstrument(reader, (version==2));
+          DivInstrument* newIns = readInstrument(reader, (version == 2));
           reader.readC();  // skip transpose
           if (version == 2) {
             reader.readC();  // skip padding
           }
-
-          uint8_t nameLen = reader.readC();
-          String insName = (nameLen > 0) ? reader.readString(nameLen) : fmt::sprintf("%s [%d]", stripPath, readCount);
-          
-          newIns->name = insName;
+          reader.readC();
           insList.push_back(newIns);
           ++readCount;
         }
 
+        // Instrument name
+        for (int i = 0; i < (insMelodyCount + insDrumCount); ++i) {
+          uint8_t nameLen = reader.readC();
+          String insName = (nameLen > 0) ? reader.readString(nameLen) : fmt::sprintf("%s [%d]", stripPath, readCount);
+          insList[i]->name = insName;
+        }
+
         // Map to note assignment currently not supported.
 
       } else { // GYBv3+
@@ -1166,9 +1168,8 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
           newIns->name = insName;
           insList.push_back(newIns);
         }
-
-        reader.seek(0, SEEK_END);
       }
+      reader.seek(0, SEEK_END);
     }
     
   } catch (EndOfFileException& e) {

From 60d2f54ac06e0ae3169f05109119aa34a30c3593 Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Mon, 25 Apr 2022 22:27:21 +1000
Subject: [PATCH 06/24] Start GYB import, also fix uf allocation leak and other
 bank loader cleanup

---
 src/engine/fileOpsIns.cpp | 53 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 52 insertions(+), 1 deletion(-)

diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index 74623d185..4a2f49143 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -1195,6 +1195,57 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
     }
   }
 }
+void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath) {
+  std::vector<DivInstrument*> insList;
+
+  int readCount = 0;
+  bool is_failed = false;
+  auto readInstOperator = [&](SafeReader& reader, DivInstrumentFM::Operator& op, bool skipRegB4) {
+  };
+
+  try {
+    reader.seek(0, SEEK_SET);
+    uint16_t header = reader.readS();
+    uint8_t insMelodyCount, insDrumCount;
+
+    if (header == 0x0C1A) { // 26 12 in decimal bytes
+      uint8_t version = reader.readC();
+      if ((version^3)>0) { // GYBv1/2
+        insMelodyCount = reader.readC();
+        insDrumCount = reader.readC();
+        if (!reader.seek(0x100, SEEK_CUR)) { // skip MIDI instrument mapping
+          throw EndOfFileException(&reader, reader.tell() + 0x100);
+        }
+
+        if (version == 2) {
+          reader.readC(); // skip LFO speed since global
+        }
+
+        for (int i = 0; i < (insMelodyCount+insDrumCount); ++i) {
+          // Note: melody and drum patches are interleaved...
+
+        }
+
+      } else { // GYBv3+
+
+      }
+    }
+    
+  } catch (EndOfFileException& e) {
+    lastError = "premature end of file";
+    logE("premature end of file");
+    for (int i = readCount - 1; i >= 0; --i) {
+      delete insList[i];
+    }
+    is_failed = true;
+  }
+
+  if (!is_failed) {
+    for (int i = 0; i < readCount; ++i) {
+      ret.push_back(insList[i]);
+    }
+  }
+}
 void DivEngine::loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath) {
   std::vector<DivInstrument*> insList;
 
@@ -1486,7 +1537,7 @@ std::vector<DivInstrument*> DivEngine::instrumentFromFile(const char* path) {
       } else if (extS==".opli") {
         format=DIV_INSFORMAT_OPLI;
       } else if (extS==".opni") {
-        format=DIV_INSFORMAT_OPNI;
+        format=DIV_INSFORMAT_OPNI;;
       } else if (extS==".y12") {
         format=DIV_INSFORMAT_Y12;
       } else if (extS==".bnk") {

From b9e51ab97f108ec5347bef82a46b43f7fe2d8944 Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Mon, 25 Apr 2022 23:57:15 +1000
Subject: [PATCH 07/24] GYB progress, need to verify read ordering

---
 src/engine/fileOpsIns.cpp | 65 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 60 insertions(+), 5 deletions(-)

diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index 4a2f49143..357211458 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -1213,21 +1213,74 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
       if ((version^3)>0) { // GYBv1/2
         insMelodyCount = reader.readC();
         insDrumCount = reader.readC();
+
         if (!reader.seek(0x100, SEEK_CUR)) { // skip MIDI instrument mapping
           throw EndOfFileException(&reader, reader.tell() + 0x100);
         }
 
         if (version == 2) {
-          reader.readC(); // skip LFO speed since global
+          reader.readC(); // skip LFO speed (chip-global)
         }
 
+        // Instrument data
         for (int i = 0; i < (insMelodyCount+insDrumCount); ++i) {
           // Note: melody and drum patches are interleaved...
-
+          DivInstrument* newIns = readInstrument(reader, (version==2));
+          reader.readC();  // skip transpose
+          if (version == 2) {
+            reader.readC();  // skip padding
+          }
+          insList.push_back(newIns);
+          ++readCount;
         }
 
-      } else { // GYBv3+
+        // Instrument name
+        for (int i = 0; i < (insMelodyCount+insDrumCount); ++i) {
+          uint8_t nameLen = reader.readC();
+          String insName = (nameLen > 0) ? reader.readString(nameLen) : fmt::sprintf("%s [%d]", stripPath, readCount);
 
+          // TODO determine if a patch is 'empty' -> ignore and move on
+          if (insList[i] == NULL) {
+            continue;
+          }
+          insList[i]->name = insName;
+        }
+      } else { // GYBv3+
+        reader.readC();  // skip LFO speed (chip-global)
+        uint32_t fileSize = reader.readI();
+        uint32_t bankOffset = reader.readI();
+        uint32_t mapOffset = reader.readI();
+
+        if (reader.size() != fileSize || bankOffset > fileSize || mapOffset > fileSize) {
+          lastError = "GYBv3 file appears to have invalid data offsets.";
+          logE("GYBv3 file appears to have invalid data offsets.");
+          return;
+        }
+
+        if (!reader.seek(bankOffset, SEEK_SET)) {
+          throw EndOfFileException(&reader, bankOffset);
+        }
+        uint16_t insCount = reader.readS();
+
+        for (int i = 0; i < insCount; ++i) {
+          DivInstrument* newIns = readInstrument(reader, true);
+          reader.readC(); // skip transpose
+          uint8_t additionalDataFlags = reader.readC() & 0x1; // skip additional data bitfield
+          // TODO if chord notes attached, skip this
+          if ((additionalDataFlags&1) > 0) {
+            uint8_t notes = reader.readC();
+            for (int j = 0; j < notes; ++j) {
+              reader.readC();
+            }
+          }
+          uint8_t nameLen = reader.readC();
+          String insName = (nameLen > 0) ? reader.readString(nameLen) : fmt::sprintf("%s [%d]", stripPath, readCount++);
+          // TODO determine if a patch is 'empty' -> ignore and move on
+          newIns->name = insName;
+          insList.push_back(newIns);
+        }
+
+        reader.seek(0, SEEK_END);
       }
     }
     
@@ -1242,7 +1295,9 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
 
   if (!is_failed) {
     for (int i = 0; i < readCount; ++i) {
-      ret.push_back(insList[i]);
+      if (insList[i] != NULL) {
+        ret.push_back(insList[i]);
+      }
     }
   }
 }
@@ -1537,7 +1592,7 @@ std::vector<DivInstrument*> DivEngine::instrumentFromFile(const char* path) {
       } else if (extS==".opli") {
         format=DIV_INSFORMAT_OPLI;
       } else if (extS==".opni") {
-        format=DIV_INSFORMAT_OPNI;;
+        format=DIV_INSFORMAT_OPNI;
       } else if (extS==".y12") {
         format=DIV_INSFORMAT_Y12;
       } else if (extS==".bnk") {

From 66b05899593dd87cc9ed1883b73b36c19e51258d Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Sat, 30 Apr 2022 16:00:29 +1000
Subject: [PATCH 08/24] GYBv1/2 working

---
 src/engine/fileOpsIns.cpp | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index 357211458..8afbf8417 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -1214,6 +1214,10 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
         insMelodyCount = reader.readC();
         insDrumCount = reader.readC();
 
+        if (insMelodyCount > 128 || insDrumCount > 128) {
+          throw std::invalid_argument(fmt::sprintf("GYBv1/2 patch count is out of bounds."));
+        }
+
         if (!reader.seek(0x100, SEEK_CUR)) { // skip MIDI instrument mapping
           throw EndOfFileException(&reader, reader.tell() + 0x100);
         }
@@ -1224,27 +1228,24 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
 
         // Instrument data
         for (int i = 0; i < (insMelodyCount+insDrumCount); ++i) {
-          // Note: melody and drum patches are interleaved...
+          // TODO determine if a patch is 'empty' -> ignore and move on
+          bool isDrum = (i >= insMelodyCount);
           DivInstrument* newIns = readInstrument(reader, (version==2));
           reader.readC();  // skip transpose
           if (version == 2) {
             reader.readC();  // skip padding
           }
+
+          uint8_t nameLen = reader.readC();
+          String insName = (nameLen > 0) ? reader.readString(nameLen) : fmt::sprintf("%s [%d]", stripPath, readCount);
+          
+          newIns->name = insName;
           insList.push_back(newIns);
           ++readCount;
         }
 
-        // Instrument name
-        for (int i = 0; i < (insMelodyCount+insDrumCount); ++i) {
-          uint8_t nameLen = reader.readC();
-          String insName = (nameLen > 0) ? reader.readString(nameLen) : fmt::sprintf("%s [%d]", stripPath, readCount);
+        // Map to note assignment currently not supported.
 
-          // TODO determine if a patch is 'empty' -> ignore and move on
-          if (insList[i] == NULL) {
-            continue;
-          }
-          insList[i]->name = insName;
-        }
       } else { // GYBv3+
         reader.readC();  // skip LFO speed (chip-global)
         uint32_t fileSize = reader.readI();
@@ -1291,6 +1292,12 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
       delete insList[i];
     }
     is_failed = true;
+
+  } catch (std::invalid_argument& e) {
+    lastError = fmt::sprintf("Invalid value found in patch file. %s", e.what());
+    logE("Invalid value found in patch file.");
+    logE(e.what());
+    is_failed = true;
   }
 
   if (!is_failed) {

From 7cd0d728b203bb5d6288eea186f65f3ce1cb2f3a Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Sat, 30 Apr 2022 16:11:27 +1000
Subject: [PATCH 09/24] Fix GYBv1/2 instrument name read

---
 src/engine/fileOpsIns.cpp | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index 8afbf8417..8168ff0af 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -1227,23 +1227,25 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
         }
 
         // Instrument data
-        for (int i = 0; i < (insMelodyCount+insDrumCount); ++i) {
-          // TODO determine if a patch is 'empty' -> ignore and move on
+        for (int i = 0; i < (insMelodyCount + insDrumCount); ++i) {
           bool isDrum = (i >= insMelodyCount);
-          DivInstrument* newIns = readInstrument(reader, (version==2));
+          DivInstrument* newIns = readInstrument(reader, (version == 2));
           reader.readC();  // skip transpose
           if (version == 2) {
             reader.readC();  // skip padding
           }
-
-          uint8_t nameLen = reader.readC();
-          String insName = (nameLen > 0) ? reader.readString(nameLen) : fmt::sprintf("%s [%d]", stripPath, readCount);
-          
-          newIns->name = insName;
+          reader.readC();
           insList.push_back(newIns);
           ++readCount;
         }
 
+        // Instrument name
+        for (int i = 0; i < (insMelodyCount + insDrumCount); ++i) {
+          uint8_t nameLen = reader.readC();
+          String insName = (nameLen > 0) ? reader.readString(nameLen) : fmt::sprintf("%s [%d]", stripPath, readCount);
+          insList[i]->name = insName;
+        }
+
         // Map to note assignment currently not supported.
 
       } else { // GYBv3+
@@ -1280,9 +1282,8 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
           newIns->name = insName;
           insList.push_back(newIns);
         }
-
-        reader.seek(0, SEEK_END);
       }
+      reader.seek(0, SEEK_END);
     }
     
   } catch (EndOfFileException& e) {

From 7ae72f0fe41080c94d0b971c110a0ffad1e802f3 Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Sun, 8 May 2022 14:50:18 +1000
Subject: [PATCH 10/24] Rebase fix and move reused fileIns lambdas into cpp
 static functions (since effectively private)

---
 src/engine/engine.cpp     |   4 +-
 src/engine/fileOpsIns.cpp | 208 +++++++++-----------------------------
 src/engine/safeReader.cpp |   2 +-
 3 files changed, 52 insertions(+), 162 deletions(-)

diff --git a/src/engine/engine.cpp b/src/engine/engine.cpp
index 94c138a94..e9bd3f10c 100644
--- a/src/engine/engine.cpp
+++ b/src/engine/engine.cpp
@@ -1749,13 +1749,13 @@ int DivEngine::addSampleFromFile(const char* path) {
       }
       extS+=i;
     }
-    if (extS==String(".dmc")) { // read as .dmc
+    if (extS==".dmc") { // read as .dmc
       size_t len=0;
       DivSample* sample=new DivSample;
       int sampleCount=(int)song.sample.size();
       sample->name=stripPath;
 
-      FILE* f=fopen(path,"rb");
+      FILE* f=ps_fopen(path,"rb");
       if (f==NULL) {
         BUSY_END;
         lastError=fmt::sprintf("could not open file! (%s)",strerror(errno));
diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index 8168ff0af..54b84d1bd 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -80,7 +80,7 @@ struct bnktimbre_t {
           wave1;
 };
 
-auto readSbiOpData = [](sbi_t& sbi, SafeReader& reader) {
+static void readSbiOpData(sbi_t& sbi, SafeReader& reader) {
   sbi.Mcharacteristics = reader.readC();
   sbi.Ccharacteristics = reader.readC();
   sbi.Mscaling_output = reader.readC();
@@ -92,6 +92,11 @@ auto readSbiOpData = [](sbi_t& sbi, SafeReader& reader) {
   sbi.Mwave = reader.readC();
   sbi.Cwave = reader.readC();
   sbi.FeedConnect = reader.readC();
+}
+
+// detune needs extra translation from register to furnace format
+static inline uint8_t fmDtRegisterToFurnace(uint8_t&& dtNative) {
+  return (dtNative>=4) ? (7-dtNative) : (dtNative+3);
 };
 
 void DivEngine::loadDMP(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath) {
@@ -471,7 +476,6 @@ void DivEngine::loadS3I(SafeReader& reader, std::vector<DivInstrument*>& ret, St
 
       // 12-byte opl value - identical to SBI format
       sbi_t s3i;
-
       readSbiOpData(s3i, reader);
       
       DivInstrumentFM::Operator& opM = ins->fm.op[0];
@@ -976,9 +980,7 @@ void DivEngine::loadFF(SafeReader& reader, std::vector<DivInstrument*>& ret, Str
       for (unsigned int j = 0; j < 4; j++) {
         buf = reader.readC();
         ins->fm.op[j].mult = buf & 0xf;
-        // detune needs extra translation from register to furnace format
-        const int dtNative = (buf >> 4) & 0x7;
-        ins->fm.op[j].dt = (dtNative >= 4) ? (7 - dtNative) : (dtNative + 3);
+        ins->fm.op[j].dt = fmDtRegisterToFurnace((buf >> 4) & 0x7);
         ins->fm.op[j].ssgEnv = (buf >> 4) & 0x8;
       }
       for (unsigned int j = 0; j < 4; j++) {
@@ -1018,15 +1020,15 @@ void DivEngine::loadFF(SafeReader& reader, std::vector<DivInstrument*>& ret, Str
   } catch (EndOfFileException& e) {
     lastError="premature end of file";
     logE("premature end of file");
-    for (int i = readCount - 1; i >= 0; --i) {
+    for (int i = readCount; i >= 0; --i) {
       delete insList[i];
     }
     return;
   }
 
-for (unsigned int i = 0; i < insCount; ++i) {
-  ret.push_back(insList[i]);
-}
+  for (unsigned int i = 0; i < insCount; ++i) {
+    ret.push_back(insList[i]);
+  }
 }
 
 void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath) {
@@ -1035,7 +1037,7 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
   int readCount = 0;
   bool is_failed = false;
   auto readInstrument = [](SafeReader& reader, bool readRegB4) -> DivInstrument* {
-    const int opOrder[] = { 0,2,1,3 };
+    const int opOrder[] = { 0,1,2,3 };
     DivInstrument* ins = new DivInstrument;
     ins->type = DIV_INS_FM;
     ins->fm.ops = 4;
@@ -1043,48 +1045,48 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
     // see https://plutiedev.com/ym2612-registers 
     // and https://github.com/Wohlstand/OPN2BankEditor/blob/master/Specifications/GYB-file-specification.txt
 
-    uint8_t tmp;
+    uint8_t reg;
     for (int i : opOrder) {
-      tmp = reader.readC(); // MUL/DT
-      ins->fm.op[i].mult = tmp & 0xF;
-      ins->fm.op[i].dt = ((tmp >> 4) & 0x7);
+      reg = reader.readC(); // MUL/DT
+      ins->fm.op[i].mult = reg & 0xF;
+      ins->fm.op[i].dt = fmDtRegisterToFurnace((reg >> 4) & 0x7);
     }
     for (int i : opOrder) {
-      tmp = reader.readC(); // TL
-      ins->fm.op[i].tl = tmp & 0x7F;
+      reg = reader.readC(); // TL
+      ins->fm.op[i].tl = reg & 0x7F;
     }
     for (int i : opOrder) {
-      tmp = reader.readC(); // AR/RS
-      ins->fm.op[i].ar = tmp & 0x1F;
-      ins->fm.op[i].rs = ((tmp >> 6) & 0x3);
+      reg = reader.readC(); // AR/RS
+      ins->fm.op[i].ar = reg & 0x1F;
+      ins->fm.op[i].rs = ((reg >> 6) & 0x3);
     }
     for (int i : opOrder) {
-      tmp = reader.readC(); // DR/AM-ENA
-      ins->fm.op[i].dr = tmp & 0x1F;
-      ins->fm.op[i].am = ((tmp >> 7) & 0x1);
+      reg = reader.readC(); // DR/AM-ENA
+      ins->fm.op[i].dr = reg & 0x1F;
+      ins->fm.op[i].am = ((reg >> 7) & 0x1);
     }
     for (int i : opOrder) {
-      tmp = reader.readC(); // SR (D2R)
-      ins->fm.op[i].d2r = tmp & 0x1F;
+      reg = reader.readC(); // SR (D2R)
+      ins->fm.op[i].d2r = reg & 0x1F;
     }
     for (int i : opOrder) {
-      tmp = reader.readC(); // RR/SL
-      ins->fm.op[i].rr = tmp & 0xF;
-      ins->fm.op[i].sl = ((tmp >> 4) & 0xF);
+      reg = reader.readC(); // RR/SL
+      ins->fm.op[i].rr = reg & 0xF;
+      ins->fm.op[i].sl = ((reg >> 4) & 0xF);
     }
     for (int i : opOrder) {
-      tmp = reader.readC(); // SSG-EG
-      ins->fm.op[i].ssgEnv = tmp & 0xF;
+      reg = reader.readC(); // SSG-EG
+      ins->fm.op[i].ssgEnv = reg & 0xF;
     }
     // ALG/FB
-    tmp = reader.readC();
-    ins->fm.alg = tmp & 0x3;
-    ins->fm.fb = ((tmp >> 3) & 0x3);
+    reg = reader.readC();
+    ins->fm.alg = reg & 0x7;
+    ins->fm.fb = ((reg >> 3) & 0x7);
 
-    if (!readRegB4) { // PAN / PMS / AMS
-      tmp = reader.readC();
-      ins->fm.fms = tmp & 0x7;
-      ins->fm.ams = ((tmp >> 4) & 0x3);
+    if (readRegB4) { // PAN / PMS / AMS
+      reg = reader.readC();
+      ins->fm.fms = reg & 0x7;
+      ins->fm.ams = ((reg >> 4) & 0x3);
     }
     return ins;
   };
@@ -1096,12 +1098,14 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
 
     if (header == 0x0C1A) { // 26 12 in decimal bytes
       uint8_t version = reader.readC();
-      if ((version ^ 3) > 0) { // GYBv1/2
+
+      if ((version ^ 3) > 0) {
+        // GYBv1/2
         insMelodyCount = reader.readC();
         insDrumCount = reader.readC();
 
         if (insMelodyCount > 128 || insDrumCount > 128) {
-          throw std::invalid_argument(fmt::sprintf("GYBv1/2 patch count is out of bounds."));
+          throw std::invalid_argument("GYBv1/2 patch count is out of bounds.");
         }
 
         if (!reader.seek(0x100, SEEK_CUR)) { // skip MIDI instrument mapping
@@ -1120,7 +1124,6 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
           if (version == 2) {
             reader.readC();  // skip padding
           }
-          reader.readC();
           insList.push_back(newIns);
           ++readCount;
         }
@@ -1134,7 +1137,8 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
 
         // Map to note assignment currently not supported.
 
-      } else { // GYBv3+
+      } else {
+        // GYBv3+
         reader.readC();  // skip LFO speed (chip-global)
         uint32_t fileSize = reader.readI();
         uint32_t bankOffset = reader.readI();
@@ -1164,7 +1168,6 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
           }
           uint8_t nameLen = reader.readC();
           String insName = (nameLen > 0) ? reader.readString(nameLen) : fmt::sprintf("%s [%d]", stripPath, readCount++);
-          // TODO determine if a patch is 'empty' -> ignore and move on
           newIns->name = insName;
           insList.push_back(newIns);
         }
@@ -1195,120 +1198,7 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
     }
   }
 }
-void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath) {
-  std::vector<DivInstrument*> insList;
 
-  int readCount = 0;
-  bool is_failed = false;
-  auto readInstOperator = [&](SafeReader& reader, DivInstrumentFM::Operator& op, bool skipRegB4) {
-  };
-
-  try {
-    reader.seek(0, SEEK_SET);
-    uint16_t header = reader.readS();
-    uint8_t insMelodyCount, insDrumCount;
-
-    if (header == 0x0C1A) { // 26 12 in decimal bytes
-      uint8_t version = reader.readC();
-      if ((version^3)>0) { // GYBv1/2
-        insMelodyCount = reader.readC();
-        insDrumCount = reader.readC();
-
-        if (insMelodyCount > 128 || insDrumCount > 128) {
-          throw std::invalid_argument(fmt::sprintf("GYBv1/2 patch count is out of bounds."));
-        }
-
-        if (!reader.seek(0x100, SEEK_CUR)) { // skip MIDI instrument mapping
-          throw EndOfFileException(&reader, reader.tell() + 0x100);
-        }
-
-        if (version == 2) {
-          reader.readC(); // skip LFO speed (chip-global)
-        }
-
-        // Instrument data
-        for (int i = 0; i < (insMelodyCount + insDrumCount); ++i) {
-          bool isDrum = (i >= insMelodyCount);
-          DivInstrument* newIns = readInstrument(reader, (version == 2));
-          reader.readC();  // skip transpose
-          if (version == 2) {
-            reader.readC();  // skip padding
-          }
-          reader.readC();
-          insList.push_back(newIns);
-          ++readCount;
-        }
-
-        // Instrument name
-        for (int i = 0; i < (insMelodyCount + insDrumCount); ++i) {
-          uint8_t nameLen = reader.readC();
-          String insName = (nameLen > 0) ? reader.readString(nameLen) : fmt::sprintf("%s [%d]", stripPath, readCount);
-          insList[i]->name = insName;
-        }
-
-        // Map to note assignment currently not supported.
-
-      } else { // GYBv3+
-        reader.readC();  // skip LFO speed (chip-global)
-        uint32_t fileSize = reader.readI();
-        uint32_t bankOffset = reader.readI();
-        uint32_t mapOffset = reader.readI();
-
-        if (reader.size() != fileSize || bankOffset > fileSize || mapOffset > fileSize) {
-          lastError = "GYBv3 file appears to have invalid data offsets.";
-          logE("GYBv3 file appears to have invalid data offsets.");
-          return;
-        }
-
-        if (!reader.seek(bankOffset, SEEK_SET)) {
-          throw EndOfFileException(&reader, bankOffset);
-        }
-        uint16_t insCount = reader.readS();
-
-        for (int i = 0; i < insCount; ++i) {
-          DivInstrument* newIns = readInstrument(reader, true);
-          reader.readC(); // skip transpose
-          uint8_t additionalDataFlags = reader.readC() & 0x1; // skip additional data bitfield
-          // TODO if chord notes attached, skip this
-          if ((additionalDataFlags&1) > 0) {
-            uint8_t notes = reader.readC();
-            for (int j = 0; j < notes; ++j) {
-              reader.readC();
-            }
-          }
-          uint8_t nameLen = reader.readC();
-          String insName = (nameLen > 0) ? reader.readString(nameLen) : fmt::sprintf("%s [%d]", stripPath, readCount++);
-          // TODO determine if a patch is 'empty' -> ignore and move on
-          newIns->name = insName;
-          insList.push_back(newIns);
-        }
-      }
-      reader.seek(0, SEEK_END);
-    }
-    
-  } catch (EndOfFileException& e) {
-    lastError = "premature end of file";
-    logE("premature end of file");
-    for (int i = readCount - 1; i >= 0; --i) {
-      delete insList[i];
-    }
-    is_failed = true;
-
-  } catch (std::invalid_argument& e) {
-    lastError = fmt::sprintf("Invalid value found in patch file. %s", e.what());
-    logE("Invalid value found in patch file.");
-    logE(e.what());
-    is_failed = true;
-  }
-
-  if (!is_failed) {
-    for (int i = 0; i < readCount; ++i) {
-      if (insList[i] != NULL) {
-        ret.push_back(insList[i]);
-      }
-    }
-  }
-}
 void DivEngine::loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath) {
   std::vector<DivInstrument*> insList;
 
@@ -1348,8 +1238,7 @@ void DivEngine::loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, St
     op.tl = readIntStrWithinRange(reader.readStringToken(), 0, 127);
     op.rs = readIntStrWithinRange(reader.readStringToken(), 0, 3);;
     op.mult = readIntStrWithinRange(reader.readStringToken(), 0, 15);
-    op.dt = readIntStrWithinRange(reader.readStringToken(), 0, 7);
-    op.dt = (op.dt >= 4) ? (7 - op.dt) : (op.dt + 3);
+    op.dt = fmDtRegisterToFurnace(readIntStrWithinRange(reader.readStringToken(), 0, 7));
     op.dt2 = readIntStrWithinRange(reader.readStringToken(), 0, 3);
     op.am = readIntStrWithinRange(reader.readStringToken(), 0, 1);
   };
@@ -1377,7 +1266,8 @@ void DivEngine::loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, St
       }
 
       // Read each line for their respective params. They may not be written in the same LINE order but they 
-      // must absolutely be properly grouped per patch! Line prefixes MUST be separated by a space! (see inline comments)
+      // must absolutely be properly grouped per patch! Line prefixes MUST be separated by a space!
+      // (see inline comments indicating line structure examples)
       
       if (token.size() >= 2) {
         if (token[0] == '@') {
@@ -1419,7 +1309,7 @@ void DivEngine::loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, St
           m2Read = true;
 
         } else if (token.compare(0,4,"LFO:") == 0) {
-          // LFO: LFRQ AMD PMD WF NFRQ
+          // LFO:LFRQ AMD PMD WF NFRQ
           // Furnace patches do not store these as they are chip-global.
           reader.readStringLine();
           lfoRead = true;
@@ -1652,6 +1542,6 @@ std::vector<DivInstrument*> DivEngine::instrumentFromFile(const char* path) {
     }
   }
 
-  delete[] buf;
+  delete[] buf; // since we're done with this buffer
   return ret;
 }
diff --git a/src/engine/safeReader.cpp b/src/engine/safeReader.cpp
index ebbb55ad9..f7dc7ba24 100644
--- a/src/engine/safeReader.cpp
+++ b/src/engine/safeReader.cpp
@@ -170,7 +170,7 @@ String SafeReader::readStringLine() {
   unsigned char c;
   if (isEOF()) throw EndOfFileException(this, len);
 
-  while (!isEOF() && (c = readC()) != 0) {
+  while (!isEOF() && (c=readC())!=0) {
     if (c=='\r'||c=='\n') {
       break;
     }

From e289ba652d21e0efa0abb8e43bc5d599d1367265 Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Sun, 8 May 2022 15:49:33 +1000
Subject: [PATCH 11/24] GYBv3 sorta loading... just need to fix up some read
 issues

---
 src/engine/fileOpsIns.cpp | 94 ++++++++++++++++++++++-----------------
 1 file changed, 52 insertions(+), 42 deletions(-)

diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index 54b84d1bd..631901a7c 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -1045,50 +1045,57 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
     // see https://plutiedev.com/ym2612-registers 
     // and https://github.com/Wohlstand/OPN2BankEditor/blob/master/Specifications/GYB-file-specification.txt
 
-    uint8_t reg;
-    for (int i : opOrder) {
-      reg = reader.readC(); // MUL/DT
-      ins->fm.op[i].mult = reg & 0xF;
-      ins->fm.op[i].dt = fmDtRegisterToFurnace((reg >> 4) & 0x7);
-    }
-    for (int i : opOrder) {
-      reg = reader.readC(); // TL
-      ins->fm.op[i].tl = reg & 0x7F;
-    }
-    for (int i : opOrder) {
-      reg = reader.readC(); // AR/RS
-      ins->fm.op[i].ar = reg & 0x1F;
-      ins->fm.op[i].rs = ((reg >> 6) & 0x3);
-    }
-    for (int i : opOrder) {
-      reg = reader.readC(); // DR/AM-ENA
-      ins->fm.op[i].dr = reg & 0x1F;
-      ins->fm.op[i].am = ((reg >> 7) & 0x1);
-    }
-    for (int i : opOrder) {
-      reg = reader.readC(); // SR (D2R)
-      ins->fm.op[i].d2r = reg & 0x1F;
-    }
-    for (int i : opOrder) {
-      reg = reader.readC(); // RR/SL
-      ins->fm.op[i].rr = reg & 0xF;
-      ins->fm.op[i].sl = ((reg >> 4) & 0xF);
-    }
-    for (int i : opOrder) {
-      reg = reader.readC(); // SSG-EG
-      ins->fm.op[i].ssgEnv = reg & 0xF;
-    }
-    // ALG/FB
-    reg = reader.readC();
-    ins->fm.alg = reg & 0x7;
-    ins->fm.fb = ((reg >> 3) & 0x7);
-
-    if (readRegB4) { // PAN / PMS / AMS
+    try {
+      uint8_t reg;
+      for (int i : opOrder) {
+        reg = reader.readC(); // MUL/DT
+        ins->fm.op[i].mult = reg & 0xF;
+        ins->fm.op[i].dt = fmDtRegisterToFurnace((reg >> 4) & 0x7);
+      }
+      for (int i : opOrder) {
+        reg = reader.readC(); // TL
+        ins->fm.op[i].tl = reg & 0x7F;
+      }
+      for (int i : opOrder) {
+        reg = reader.readC(); // AR/RS
+        ins->fm.op[i].ar = reg & 0x1F;
+        ins->fm.op[i].rs = ((reg >> 6) & 0x3);
+      }
+      for (int i : opOrder) {
+        reg = reader.readC(); // DR/AM-ENA
+        ins->fm.op[i].dr = reg & 0x1F;
+        ins->fm.op[i].am = ((reg >> 7) & 0x1);
+      }
+      for (int i : opOrder) {
+        reg = reader.readC(); // SR (D2R)
+        ins->fm.op[i].d2r = reg & 0x1F;
+      }
+      for (int i : opOrder) {
+        reg = reader.readC(); // RR/SL
+        ins->fm.op[i].rr = reg & 0xF;
+        ins->fm.op[i].sl = ((reg >> 4) & 0xF);
+      }
+      for (int i : opOrder) {
+        reg = reader.readC(); // SSG-EG
+        ins->fm.op[i].ssgEnv = reg & 0xF;
+      }
+      // ALG/FB
       reg = reader.readC();
-      ins->fm.fms = reg & 0x7;
-      ins->fm.ams = ((reg >> 4) & 0x3);
+      ins->fm.alg = reg & 0x7;
+      ins->fm.fb = ((reg >> 3) & 0x7);
+
+      if (readRegB4) { // PAN / PMS / AMS
+        reg = reader.readC();
+        ins->fm.fms = reg & 0x7;
+        ins->fm.ams = ((reg >> 4) & 0x3);
+      }
+      return ins;
+
+    } catch (...) {
+      // Deallocate and rethrow to outer handler
+      delete ins;
+      throw;
     }
-    return ins;
   };
 
   try {
@@ -1166,10 +1173,13 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
               reader.readC();
             }
           }
+          reader.readC(); // padding
+          reader.readC(); // padding
           uint8_t nameLen = reader.readC();
           String insName = (nameLen > 0) ? reader.readString(nameLen) : fmt::sprintf("%s [%d]", stripPath, readCount++);
           newIns->name = insName;
           insList.push_back(newIns);
+          ++readCount;
         }
       }
       reader.seek(0, SEEK_END);

From 1c88e20fa70f18f302bdef31ad84e4a5fca2aa57 Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Mon, 9 May 2022 12:56:57 +1000
Subject: [PATCH 12/24] OPM parser tidyup, GYBv1/2/3 foundation support done,
 BNK parser cleanup

---
 src/engine/fileOpsIns.cpp | 123 +++++++++++++++++++++-----------------
 src/engine/safeReader.cpp |   1 +
 2 files changed, 68 insertions(+), 56 deletions(-)

diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index 631901a7c..efc66cef9 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -97,7 +97,7 @@ static void readSbiOpData(sbi_t& sbi, SafeReader& reader) {
 // detune needs extra translation from register to furnace format
 static inline uint8_t fmDtRegisterToFurnace(uint8_t&& dtNative) {
   return (dtNative>=4) ? (7-dtNative) : (dtNative+3);
-};
+}
 
 void DivEngine::loadDMP(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath) {
   DivInstrument* ins=new DivInstrument;
@@ -886,44 +886,40 @@ void DivEngine::loadBNK(SafeReader& reader, std::vector<DivInstrument*>& ret, St
 
       // Read until EOF
       for (int i = 0; i < insCount; ++i) {
-        bnktimbre_t timbre;
         DivInstrument *ins = new DivInstrument;
 
         ins->type = DIV_INS_OPL;
         ins->fm.ops = 2;
 
-        timbre.mode = reader.readC();
-        timbre.percVoice = reader.readC();
-        if (timbre.mode == 1) {
+        uint8_t timbreMode = reader.readC();
+        uint8_t timbrePercVoice = reader.readC();
+        if (timbreMode == 1) {
           ins->fm.opllPreset = (uint8_t)(1<<4);
         }
-        ins->fm.op[0].ksl = reader.readC();
-        ins->fm.op[0].mult = reader.readC();
-        ins->fm.fb = reader.readC();
-        ins->fm.op[0].ar = reader.readC();
-        ins->fm.op[0].sl = reader.readC();
-        ins->fm.op[0].sus = (reader.readC() != 0) ? 1 : 0;
-        ins->fm.op[0].dr = reader.readC();
-        ins->fm.op[0].rr = reader.readC();
-        ins->fm.op[0].tl = reader.readC();
-        ins->fm.op[0].am = reader.readC();
-        ins->fm.op[0].vib = reader.readC();
-        ins->fm.op[0].ksr = reader.readC();
-        ins->fm.alg = (reader.readC() == 0) ? 1 : 0;
 
-        ins->fm.op[1].ksl = reader.readC();
-        ins->fm.op[1].mult = reader.readC();
-        reader.readC(); // skip
-        ins->fm.op[1].ar = reader.readC();
-        ins->fm.op[1].sl = reader.readC();
-        ins->fm.op[1].sus = (reader.readC() != 0) ? 1 : 0;
-        ins->fm.op[1].dr = reader.readC();
-        ins->fm.op[1].rr = reader.readC();
-        ins->fm.op[1].tl = reader.readC();
-        ins->fm.op[1].am = reader.readC();
-        ins->fm.op[1].vib = reader.readC();
-        ins->fm.op[1].ksr = reader.readC();
-        reader.readC(); // skip
+        for (int i = 0; i < 2; ++i) {
+          ins->fm.op[i].ksl = reader.readC();
+          ins->fm.op[i].mult = reader.readC();
+          if (i==0) {
+            ins->fm.fb = reader.readC();
+          } else {
+            reader.readC(); // skip
+          }
+          ins->fm.op[i].ar = reader.readC();
+          ins->fm.op[i].sl = reader.readC();
+          ins->fm.op[i].sus = (reader.readC() != 0) ? 1 : 0;
+          ins->fm.op[i].dr = reader.readC();
+          ins->fm.op[i].rr = reader.readC();
+          ins->fm.op[i].tl = reader.readC();
+          ins->fm.op[i].am = reader.readC();
+          ins->fm.op[i].vib = reader.readC();
+          ins->fm.op[i].ksr = reader.readC();
+          if (i==0) {
+            ins->fm.alg = (reader.readC() == 0) ? 1 : 0;
+          } else {
+            reader.readC(); // skip
+          }
+        }
 
         ins->fm.op[0].ws = reader.readC();
         ins->fm.op[1].ws = reader.readC();
@@ -1033,9 +1029,9 @@ void DivEngine::loadFF(SafeReader& reader, std::vector<DivInstrument*>& ret, Str
 
 void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath) {
   std::vector<DivInstrument*> insList;
-
   int readCount = 0;
   bool is_failed = false;
+
   auto readInstrument = [](SafeReader& reader, bool readRegB4) -> DivInstrument* {
     const int opOrder[] = { 0,1,2,3 };
     DivInstrument* ins = new DivInstrument;
@@ -1124,19 +1120,20 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
         }
 
         // Instrument data
-        for (int i = 0; i < (insMelodyCount + insDrumCount); ++i) {
+        for (int i = 0; i < (insMelodyCount+insDrumCount); ++i) {
           bool isDrum = (i >= insMelodyCount);
           DivInstrument* newIns = readInstrument(reader, (version == 2));
+          insList.push_back(newIns);
+          ++readCount;
+
           reader.readC();  // skip transpose
           if (version == 2) {
             reader.readC();  // skip padding
           }
-          insList.push_back(newIns);
-          ++readCount;
         }
 
         // Instrument name
-        for (int i = 0; i < (insMelodyCount + insDrumCount); ++i) {
+        for (int i = 0; i < (insMelodyCount+insDrumCount); ++i) {
           uint8_t nameLen = reader.readC();
           String insName = (nameLen > 0) ? reader.readString(nameLen) : fmt::sprintf("%s [%d]", stripPath, readCount);
           insList[i]->name = insName;
@@ -1154,7 +1151,6 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
         if (reader.size() != fileSize || bankOffset > fileSize || mapOffset > fileSize) {
           lastError = "GYBv3 file appears to have invalid data offsets.";
           logE("GYBv3 file appears to have invalid data offsets.");
-          return;
         }
 
         if (!reader.seek(bankOffset, SEEK_SET)) {
@@ -1163,7 +1159,13 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
         uint16_t insCount = reader.readS();
 
         for (int i = 0; i < insCount; ++i) {
+          uint16_t patchPos = reader.tell();
+          uint16_t patchSize = reader.readS();
+
           DivInstrument* newIns = readInstrument(reader, true);
+          insList.push_back(newIns);
+          ++readCount;
+
           reader.readC(); // skip transpose
           uint8_t additionalDataFlags = reader.readC() & 0x1; // skip additional data bitfield
           // TODO if chord notes attached, skip this
@@ -1173,13 +1175,9 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
               reader.readC();
             }
           }
-          reader.readC(); // padding
-          reader.readC(); // padding
           uint8_t nameLen = reader.readC();
           String insName = (nameLen > 0) ? reader.readString(nameLen) : fmt::sprintf("%s [%d]", stripPath, readCount++);
           newIns->name = insName;
-          insList.push_back(newIns);
-          ++readCount;
         }
       }
       reader.seek(0, SEEK_END);
@@ -1252,10 +1250,18 @@ void DivEngine::loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, St
     op.dt2 = readIntStrWithinRange(reader.readStringToken(), 0, 3);
     op.am = readIntStrWithinRange(reader.readStringToken(), 0, 1);
   };
+  auto seekGroupValStart = [](SafeReader& reader, int pos) {
+    // Seek to position then move to next ':' character
+    if (!reader.seek(pos, SEEK_SET)) {
+      throw EndOfFileException(&reader, pos);
+    }
+    reader.readStringToken(':', false);
+  };
 
   try {
     reader.seek(0, SEEK_SET);
     while (!reader.isEOF()) {
+      size_t linePos = reader.tell();
       String token = reader.readStringToken();
       if (token.size() == 0) {
         continue;
@@ -1276,58 +1282,63 @@ void DivEngine::loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, St
       }
 
       // Read each line for their respective params. They may not be written in the same LINE order but they 
-      // must absolutely be properly grouped per patch! Line prefixes MUST be separated by a space!
-      // (see inline comments indicating line structure examples)
+      // must absolutely be properly grouped per patch! See inline comments indicating line structure examples.
       
       if (token.size() >= 2) {
+        // Checking line prefixes since they sometimes may not have a space after the ':'
         if (token[0] == '@') {
           // @:123 Name of patch
-          // Note: Fallback to bank filename and current patch number in _file_ order (not @n order)
+          seekGroupValStart(reader, linePos);
+          // Note: Fallback to bank filename and current patch number specified by @n
+          String opmPatchNum = reader.readStringToken();
           newPatch->name = reader.readStringLine();
-          newPatch->name = newPatch->name.size() > 0 ? newPatch->name : fmt::sprintf("%s[%d]", stripPath, readCount);
+          newPatch->name = newPatch->name.size() > 0 ? newPatch->name : fmt::sprintf("%s @%s", stripPath, opmPatchNum);
           patchNameRead = true;
 
-        } else if (token == "CH:") {
+        } else if (token.compare(0,3,"CH:") == 0) {
           // CH: PAN FL CON AMS PMS SLOT NE
+          seekGroupValStart(reader, linePos);
           reader.readStringToken(); // skip PAN
           newPatch->fm.fb = readIntStrWithinRange(reader.readStringToken(), 0, 7);
           newPatch->fm.alg = readIntStrWithinRange(reader.readStringToken(), 0, 7);
           newPatch->fm.ams = readIntStrWithinRange(reader.readStringToken(), 0, 4);
           newPatch->fm.fms = readIntStrWithinRange(reader.readStringToken(), 0, 7);
-          reader.readStringToken(); // skip SLOT
-          reader.readStringToken(); // skip NE
+          reader.readStringToken(); // skip SLOT (no furnace equivalent...yet?)
+          reader.readStringToken(); // skip NE   (^^^)
           characteristicRead = true;
 
-        } else if (token == "C1:") {
+        } else if (token.compare(0,3,"C1:") == 0) {
           // C1: AR D1R D2R RR D1L TL KS MUL DT1 DT2 AMS-EN
+          seekGroupValStart(reader, linePos);
           readOpmOperator(reader, newPatch->fm.op[2]);
           c1Read = true;
 
-        } else if (token == "C2:") {
+        } else if (token.compare(0,3,"C2:") == 0) {
           // C2: AR D1R D2R RR D1L TL KS MUL DT1 DT2 AMS-EN
+          seekGroupValStart(reader, linePos);
           readOpmOperator(reader, newPatch->fm.op[3]);
           c2Read = true;
 
-        } else if (token == "M1:") {
+        } else if (token.compare(0,3,"M1:") == 0) {
           // M1: AR D1R D2R RR D1L TL KS MUL DT1 DT2 AMS-EN
+          seekGroupValStart(reader, linePos);
           readOpmOperator(reader, newPatch->fm.op[0]);
           m1Read = true;
 
-        } else if (token == "M2:") {
+        } else if (token.compare(0,3,"M2:") == 0) {
           // M2: AR D1R D2R RR D1L TL KS MUL DT1 DT2 AMS-EN
+          seekGroupValStart(reader, linePos);
           readOpmOperator(reader, newPatch->fm.op[1]);
           m2Read = true;
 
         } else if (token.compare(0,4,"LFO:") == 0) {
           // LFO:LFRQ AMD PMD WF NFRQ
+          seekGroupValStart(reader, linePos);
           // Furnace patches do not store these as they are chip-global.
           reader.readStringLine();
           lfoRead = true;
-
-        } else {
-          // other unsupported lines ignored.
-          reader.readStringLine();
         }
+        // other unsupported lines ignored.
       }
 
       if (completePatchRead()) {
diff --git a/src/engine/safeReader.cpp b/src/engine/safeReader.cpp
index f7dc7ba24..145be333c 100644
--- a/src/engine/safeReader.cpp
+++ b/src/engine/safeReader.cpp
@@ -200,5 +200,6 @@ String SafeReader::readStringToken(unsigned char delim, bool stripContiguous) {
 }
 
 String SafeReader::readStringToken() {
+  // This will strip LHS whitespace and only return contents after it.
   return readStringToken(' ', true);
 }

From 77af4fda2a404416fd31101908850f8d46adb833 Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Mon, 9 May 2022 13:53:09 +1000
Subject: [PATCH 13/24] Remove unused/duplicate code, add stringNotBlank(str)
 checks

---
 src/engine/fileOpsIns.cpp | 131 ++++++++++++++++++--------------------
 1 file changed, 62 insertions(+), 69 deletions(-)

diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index efc66cef9..33ca61eeb 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -39,7 +39,7 @@ enum DivInsFormats {
   DIV_INSFORMAT_FF,
 };
 
-// Patch data structures
+// Reused patch data structures
 
 // SBI and some other OPL containers
 struct sbi_t {
@@ -56,30 +56,6 @@ struct sbi_t {
           FeedConnect;
 };
 
-// Adlib Visual Composer BNK
-struct bnkop_t {
-  uint8_t ksl,
-          multiple,
-          feedback, // op1 only
-          attack,
-          sustain,
-          eg,
-          decay,
-          releaseRate,
-          totalLevel,
-          am,
-          vib,
-          ksr,
-          con; // op1 only
-};
-struct bnktimbre_t {
-  uint8_t mode,
-          percVoice;
-  bnkop_t op[2];
-  uint8_t wave0,
-          wave1;
-};
-
 static void readSbiOpData(sbi_t& sbi, SafeReader& reader) {
   sbi.Mcharacteristics = reader.readC();
   sbi.Ccharacteristics = reader.readC();
@@ -99,6 +75,10 @@ static inline uint8_t fmDtRegisterToFurnace(uint8_t&& dtNative) {
   return (dtNative>=4) ? (7-dtNative) : (dtNative+3);
 }
 
+static inline bool stringNotBlank(String& str) {
+  return str.size() > 0 && str.find_first_not_of(' ') != String::npos;
+}
+
 void DivEngine::loadDMP(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath) {
   DivInstrument* ins=new DivInstrument;
   // this is a ridiculous mess
@@ -516,8 +496,8 @@ void DivEngine::loadS3I(SafeReader& reader, std::vector<DivInstrument*>& ret, St
       lastError="S3I PCM samples currently not supported.";
       logE("S3I PCM samples currently not supported.");
     }
-    ins->name = reader.readString(28);
-    ins->name = (ins->name.size() == 0) ? stripPath : ins->name;
+    String insName = reader.readString(28);
+    ins->name = stringNotBlank(insName) ? insName : stripPath;
 
     int s3i_signature = reader.readI();
 
@@ -548,8 +528,8 @@ void DivEngine::loadSBI(SafeReader& reader, std::vector<DivInstrument*>& ret, St
     bool is_6op = (sbi_header == 0x1A504F36); // 6OP\x1A - Freq Monster 801-specific
 
     // 32-byte null terminated instrument name
-    String patchName = reader.readString(32);
-    patchName = (patchName.size() == 0) ? stripPath : patchName;
+    String insName = reader.readString(32);
+    insName = stringNotBlank(insName) ? insName : stripPath;
 
     auto writeOp = [](sbi_t& sbi, DivInstrumentFM::Operator& opM, DivInstrumentFM::Operator& opC) {
       opM.mult = sbi.Mcharacteristics & 0xF;
@@ -588,7 +568,7 @@ void DivEngine::loadSBI(SafeReader& reader, std::vector<DivInstrument*>& ret, St
       DivInstrumentFM::Operator& opM = ins->fm.op[0];
       DivInstrumentFM::Operator& opC = ins->fm.op[1];
       ins->fm.ops = 2;
-      ins->name = patchName;
+      ins->name = insName;
       writeOp(sbi_op12, opM, opC);
       ins->fm.alg = (sbi_op12.FeedConnect & 0x1);
       ins->fm.fb = ((sbi_op12.FeedConnect >> 1) & 0x7);
@@ -614,7 +594,7 @@ void DivEngine::loadSBI(SafeReader& reader, std::vector<DivInstrument*>& ret, St
       DivInstrumentFM::Operator& opM4 = ins->fm.op[1];
       DivInstrumentFM::Operator& opC4 = ins->fm.op[3];
       ins->fm.ops = 4;
-      ins->name = patchName;
+      ins->name = insName;
       ins->fm.alg = (sbi_op12.FeedConnect & 0x1) | ((sbi_op34.FeedConnect & 0x1) << 1);
       ins->fm.fb = ((sbi_op34.FeedConnect >> 1) & 0x7);
       writeOp(sbi_op12, opM, opC);
@@ -633,7 +613,7 @@ void DivEngine::loadSBI(SafeReader& reader, std::vector<DivInstrument*>& ret, St
         DivInstrumentFM::Operator& opC6 = ins->fm.op[1];
         ins->type = DIV_INS_OPL;
         ins->fm.ops = 2;
-        ins->name = fmt::sprintf("%s (2op)", patchName);
+        ins->name = fmt::sprintf("%s (2op)", insName);
         writeOp(sbi_op12, opM6, opC6);
         ins->fm.alg = (sbi_op12.FeedConnect & 0x1);
         ins->fm.fb = ((sbi_op12.FeedConnect >> 1) & 0x7);
@@ -649,7 +629,9 @@ void DivEngine::loadSBI(SafeReader& reader, std::vector<DivInstrument*>& ret, St
   } catch (EndOfFileException& e) {
     lastError="premature end of file";
     logE("premature end of file");
-    delete ins;
+    if (ins != NULL) {
+      delete ins;
+    }
   }
 }
 
@@ -669,7 +651,7 @@ void DivEngine::loadOPLI(SafeReader& reader, std::vector<DivInstrument*>& ret, S
 
       ins->type = DIV_INS_OPL;
       String insName = reader.readString(32);
-      insName = (insName.size() > 0) ? insName : stripPath;
+      insName = stringNotBlank(insName) ? insName : stripPath;
       ins->name = insName;
       reader.seek(7, SEEK_CUR);  // skip MIDI params
       uint8_t instTypeFlags = reader.readC();  // [0EEEDCBA] - see WOPL/OPLI spec
@@ -740,7 +722,9 @@ void DivEngine::loadOPLI(SafeReader& reader, std::vector<DivInstrument*>& ret, S
   } catch (EndOfFileException& e) {
     lastError="premature end of file";
     logE("premature end of file");
-    delete ins;
+    if (ins != NULL) {
+      delete ins;
+    }
   }
 }
 
@@ -763,7 +747,7 @@ void DivEngine::loadOPNI(SafeReader& reader, std::vector<DivInstrument*>& ret, S
       ins->fm.ops = 4;
 
       String insName = reader.readString(32);
-      ins->name = (insName.size() > 0) ? insName : stripPath;
+      ins->name = stringNotBlank(insName) ? insName : stripPath;
       if (!reader.seek(3, SEEK_CUR)) {  // skip MIDI params
         throw EndOfFileException(&reader, reader.tell() + 3);
       }
@@ -805,7 +789,9 @@ void DivEngine::loadOPNI(SafeReader& reader, std::vector<DivInstrument*>& ret, S
   } catch (EndOfFileException& e) {
     lastError="premature end of file";
     logE("premature end of file");
-    delete ins;
+    if (ins != NULL) {
+      delete ins;
+    }
   }
 }
 
@@ -848,7 +834,9 @@ void DivEngine::loadY12(SafeReader& reader, std::vector<DivInstrument*>& ret, St
   } catch (EndOfFileException& e) {
     lastError="premature end of file";
     logE("premature end of file");
-    delete ins;
+    if (ins != NULL) {
+      delete ins;
+    }
   }
 }
 
@@ -884,7 +872,7 @@ void DivEngine::loadBNK(SafeReader& reader, std::vector<DivInstrument*>& ret, St
         throw EndOfFileException(&reader, data_offset);
       };
 
-      // Read until EOF
+      // Read until all patches have been accounted for.
       for (int i = 0; i < insCount; ++i) {
         DivInstrument *ins = new DivInstrument;
 
@@ -900,10 +888,9 @@ void DivEngine::loadBNK(SafeReader& reader, std::vector<DivInstrument*>& ret, St
         for (int i = 0; i < 2; ++i) {
           ins->fm.op[i].ksl = reader.readC();
           ins->fm.op[i].mult = reader.readC();
+          uint8_t fb = reader.readC();
           if (i==0) {
-            ins->fm.fb = reader.readC();
-          } else {
-            reader.readC(); // skip
+            ins->fm.fb = fb;
           }
           ins->fm.op[i].ar = reader.readC();
           ins->fm.op[i].sl = reader.readC();
@@ -914,26 +901,25 @@ void DivEngine::loadBNK(SafeReader& reader, std::vector<DivInstrument*>& ret, St
           ins->fm.op[i].am = reader.readC();
           ins->fm.op[i].vib = reader.readC();
           ins->fm.op[i].ksr = reader.readC();
+          uint8_t alg = (reader.readC() == 0) ? 1 : 0;
           if (i==0) {
-            ins->fm.alg = (reader.readC() == 0) ? 1 : 0;
-          } else {
-            reader.readC(); // skip
+            ins->fm.alg = alg;
           }
         }
-
         ins->fm.op[0].ws = reader.readC();
         ins->fm.op[1].ws = reader.readC();
-        ins->name = instNames[i]->length() > 0 ? (*instNames[i]) : fmt::sprintf("%s[%d]", stripPath, i);
+        ins->name = stringNotBlank(*instNames[i]) ? (*instNames[i]) : fmt::sprintf("%s[%d]", stripPath, i);
 
         insList.push_back(ins);
         ++readCount;
       }
+      // All data read, don't care about the rest.
       reader.seek(0, SEEK_END);
 
     } catch (EndOfFileException& e) {
       lastError="premature end of file";
       logE("premature end of file");
-      for (int i = readCount - 1; i >= 0; --i) {
+      for (int i = 0; i < readCount; ++i) {
         delete insList[i];
       }
       is_failed = true;
@@ -951,7 +937,7 @@ void DivEngine::loadBNK(SafeReader& reader, std::vector<DivInstrument*>& ret, St
     }
   }
 
-  for (auto& name : instNames) {
+  for (String* name : instNames) {
     delete name;
   }
 }
@@ -1016,6 +1002,7 @@ void DivEngine::loadFF(SafeReader& reader, std::vector<DivInstrument*>& ret, Str
   } catch (EndOfFileException& e) {
     lastError="premature end of file";
     logE("premature end of file");
+    // Include incomplete entry in deletion.
     for (int i = readCount; i >= 0; --i) {
       delete insList[i];
     }
@@ -1032,7 +1019,7 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
   int readCount = 0;
   bool is_failed = false;
 
-  auto readInstrument = [](SafeReader& reader, bool readRegB4) -> DivInstrument* {
+  auto readInstrument = [&](SafeReader& reader, bool readRegB4) -> DivInstrument* {
     const int opOrder[] = { 0,1,2,3 };
     DivInstrument* ins = new DivInstrument;
     ins->type = DIV_INS_FM;
@@ -1085,6 +1072,8 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
         ins->fm.fms = reg & 0x7;
         ins->fm.ams = ((reg >> 4) & 0x3);
       }
+      insList.push_back(ins);
+      ++readCount;
       return ins;
 
     } catch (...) {
@@ -1093,6 +1082,11 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
       throw;
     }
   };
+  auto readInstrumentName = [&](SafeReader& reader, DivInstrument* ins) {
+    uint8_t nameLen = reader.readC();
+    String insName = (nameLen>0) ? reader.readString(nameLen) : "";
+    ins->name = stringNotBlank(insName) ? insName : fmt::sprintf("%s [%d]", stripPath, readCount - 1);
+  };
 
   try {
     reader.seek(0, SEEK_SET);
@@ -1123,9 +1117,8 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
         for (int i = 0; i < (insMelodyCount+insDrumCount); ++i) {
           bool isDrum = (i >= insMelodyCount);
           DivInstrument* newIns = readInstrument(reader, (version == 2));
-          insList.push_back(newIns);
-          ++readCount;
 
+          // Additional data
           reader.readC();  // skip transpose
           if (version == 2) {
             reader.readC();  // skip padding
@@ -1134,9 +1127,7 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
 
         // Instrument name
         for (int i = 0; i < (insMelodyCount+insDrumCount); ++i) {
-          uint8_t nameLen = reader.readC();
-          String insName = (nameLen > 0) ? reader.readString(nameLen) : fmt::sprintf("%s [%d]", stripPath, readCount);
-          insList[i]->name = insName;
+          readInstrumentName(reader, insList[i]);
         }
 
         // Map to note assignment currently not supported.
@@ -1161,23 +1152,22 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
         for (int i = 0; i < insCount; ++i) {
           uint16_t patchPos = reader.tell();
           uint16_t patchSize = reader.readS();
-
           DivInstrument* newIns = readInstrument(reader, true);
-          insList.push_back(newIns);
-          ++readCount;
 
+          // Additional data
           reader.readC(); // skip transpose
           uint8_t additionalDataFlags = reader.readC() & 0x1; // skip additional data bitfield
-          // TODO if chord notes attached, skip this
+          
+          // if chord notes attached, skip this
           if ((additionalDataFlags&1) > 0) {
             uint8_t notes = reader.readC();
             for (int j = 0; j < notes; ++j) {
               reader.readC();
             }
           }
-          uint8_t nameLen = reader.readC();
-          String insName = (nameLen > 0) ? reader.readString(nameLen) : fmt::sprintf("%s [%d]", stripPath, readCount++);
-          newIns->name = insName;
+
+          // Instrument Name
+          readInstrumentName(reader, insList[i]);
         }
       }
       reader.seek(0, SEEK_END);
@@ -1186,9 +1176,6 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
   } catch (EndOfFileException& e) {
     lastError = "premature end of file";
     logE("premature end of file");
-    for (int i = readCount - 1; i >= 0; --i) {
-      delete insList[i];
-    }
     is_failed = true;
 
   } catch (std::invalid_argument& e) {
@@ -1204,6 +1191,10 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
         ret.push_back(insList[i]);
       }
     }
+  } else {
+    for (int i = 0; i < readCount; ++i) {
+      delete insList[i];
+    }
   }
 }
 
@@ -1261,6 +1252,7 @@ void DivEngine::loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, St
   try {
     reader.seek(0, SEEK_SET);
     while (!reader.isEOF()) {
+      // Checking line prefixes since they sometimes may not have a space after the ':'
       size_t linePos = reader.tell();
       String token = reader.readStringToken();
       if (token.size() == 0) {
@@ -1285,14 +1277,13 @@ void DivEngine::loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, St
       // must absolutely be properly grouped per patch! See inline comments indicating line structure examples.
       
       if (token.size() >= 2) {
-        // Checking line prefixes since they sometimes may not have a space after the ':'
         if (token[0] == '@') {
           // @:123 Name of patch
           seekGroupValStart(reader, linePos);
           // Note: Fallback to bank filename and current patch number specified by @n
           String opmPatchNum = reader.readStringToken();
-          newPatch->name = reader.readStringLine();
-          newPatch->name = newPatch->name.size() > 0 ? newPatch->name : fmt::sprintf("%s @%s", stripPath, opmPatchNum);
+          String insName = reader.readStringLine();
+          newPatch->name = stringNotBlank(insName) ? insName : fmt::sprintf("%s @%s", stripPath, opmPatchNum);
           patchNameRead = true;
 
         } else if (token.compare(0,3,"CH:") == 0) {
@@ -1337,8 +1328,10 @@ void DivEngine::loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, St
           // Furnace patches do not store these as they are chip-global.
           reader.readStringLine();
           lfoRead = true;
+        } else {
+          // other unsupported lines ignored.
+          reader.readStringLine();
         }
-        // other unsupported lines ignored.
       }
 
       if (completePatchRead()) {

From 25abf4c7333df60181b2b7871b47a7971afdeffc Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Mon, 9 May 2022 17:49:56 +1000
Subject: [PATCH 14/24] WOPN progress

---
 src/engine/engine.h       |   2 +
 src/engine/fileOpsIns.cpp | 268 +++++++++++++++++++++++++++++++++++++-
 src/gui/gui.cpp           |  22 +++-
 3 files changed, 282 insertions(+), 10 deletions(-)

diff --git a/src/engine/engine.h b/src/engine/engine.h
index 8c968ea2a..728927ae8 100644
--- a/src/engine/engine.h
+++ b/src/engine/engine.h
@@ -397,6 +397,8 @@ class DivEngine {
   void loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath);
   void loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath);
   void loadFF(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath);
+  void loadWOPL(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath);
+  void loadWOPN(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath);
 
   bool initAudioBackend();
   bool deinitAudioBackend();
diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index 33ca61eeb..91490f07d 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -36,6 +36,8 @@ enum DivInsFormats {
   DIV_INSFORMAT_BNK,
   DIV_INSFORMAT_GYB,
   DIV_INSFORMAT_OPM,
+  DIV_INSFORMAT_WOPL,
+  DIV_INSFORMAT_WOPN,
   DIV_INSFORMAT_FF,
 };
 
@@ -56,6 +58,13 @@ struct sbi_t {
           FeedConnect;
 };
 
+// MIDI-related
+struct midibank_t {
+  String name;
+  uint8_t bankMsb,
+          bankLsb;
+};
+
 static void readSbiOpData(sbi_t& sbi, SafeReader& reader) {
   sbi.Mcharacteristics = reader.readC();
   sbi.Ccharacteristics = reader.readC();
@@ -75,7 +84,7 @@ static inline uint8_t fmDtRegisterToFurnace(uint8_t&& dtNative) {
   return (dtNative>=4) ? (7-dtNative) : (dtNative+3);
 }
 
-static inline bool stringNotBlank(String& str) {
+static bool stringNotBlank(String& str) {
   return str.size() > 0 && str.find_first_not_of(' ') != String::npos;
 }
 
@@ -643,9 +652,6 @@ void DivEngine::loadOPLI(SafeReader& reader, std::vector<DivInstrument*>& ret, S
     String header = reader.readString(11);
     if (header == "WOPL3-INST") {
       uint16_t version = reader.readS();
-      if (version > 3) {
-        logW("Unknown OPLI version.");
-      }
 
       reader.readC();  // skip isPerc field
 
@@ -1085,7 +1091,9 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
   auto readInstrumentName = [&](SafeReader& reader, DivInstrument* ins) {
     uint8_t nameLen = reader.readC();
     String insName = (nameLen>0) ? reader.readString(nameLen) : "";
-    ins->name = stringNotBlank(insName) ? insName : fmt::sprintf("%s [%d]", stripPath, readCount - 1);
+    ins->name = stringNotBlank(insName) 
+      ? insName 
+      : fmt::sprintf("%s [%d]", stripPath, readCount - 1);
   };
 
   try {
@@ -1283,7 +1291,9 @@ void DivEngine::loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, St
           // Note: Fallback to bank filename and current patch number specified by @n
           String opmPatchNum = reader.readStringToken();
           String insName = reader.readStringLine();
-          newPatch->name = stringNotBlank(insName) ? insName : fmt::sprintf("%s @%s", stripPath, opmPatchNum);
+          newPatch->name = stringNotBlank(insName) 
+            ? insName 
+            : fmt::sprintf("%s @%s", stripPath, opmPatchNum);
           patchNameRead = true;
 
         } else if (token.compare(0,3,"CH:") == 0) {
@@ -1372,6 +1382,242 @@ void DivEngine::loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, St
   }
 }
 
+
+void DivEngine::loadWOPL(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath) {
+  DivInstrument* ins = new DivInstrument;
+
+  try {
+    reader.seek(0, SEEK_SET);
+    String header = reader.readString(11);
+    if (header == "WOPL3-BANK") {
+      uint16_t version = reader.readS();
+
+      reader.readC();  // skip isPerc field
+
+      ins->type = DIV_INS_OPL;
+      String insName = reader.readString(32);
+      insName = stringNotBlank(insName) ? insName : stripPath;
+      ins->name = insName;
+      reader.seek(7, SEEK_CUR);  // skip MIDI params
+      uint8_t instTypeFlags = reader.readC();  // [0EEEDCBA] - see WOPL/OPLI spec
+
+      bool is_4op = ((instTypeFlags & 0x1) == 1);
+      bool is_2x2op = (((instTypeFlags >> 1) & 0x1) == 1);
+      bool is_rhythm = (((instTypeFlags >> 4) & 0x7) > 0);
+
+      auto readOpliOp = [](SafeReader& reader, DivInstrumentFM::Operator& op) {
+        uint8_t characteristics = reader.readC();
+        uint8_t keyScaleLevel = reader.readC();
+        uint8_t attackDecay = reader.readC();
+        uint8_t sustainRelease = reader.readC();
+        uint8_t waveSelect = reader.readC();
+
+        op.mult = characteristics & 0xF;
+        op.ksr = ((characteristics >> 4) & 0x1);
+        op.sus = ((characteristics >> 5) & 0x1);
+        op.vib = ((characteristics >> 6) & 0x1);
+        op.am = ((characteristics >> 7) & 0x1);
+        op.tl = keyScaleLevel & 0x3F;
+        op.ksl = ((keyScaleLevel >> 6) & 0x3);
+        op.ar = ((attackDecay >> 4) & 0xF);
+        op.dr = attackDecay & 0xF;
+        op.rr = sustainRelease & 0xF;
+        op.sl = ((sustainRelease >> 4) & 0xF);
+        op.ws = waveSelect;
+      };
+
+      uint8_t feedConnect = reader.readC();
+      uint8_t feedConnect2nd = reader.readC();
+
+      ins->fm.alg = (feedConnect & 0x1);
+      ins->fm.fb = ((feedConnect >> 1) & 0xF);
+
+      if (is_4op && !is_2x2op) {
+        ins->fm.ops = 4;
+        ins->fm.alg = (feedConnect & 0x1) | ((feedConnect2nd & 0x1) << 1);
+        for (int i : {2, 0, 3, 1}) { // omfg >_<
+          readOpliOp(reader, ins->fm.op[i]);
+        }
+      }
+      else {
+        ins->fm.ops = 2;
+        for (int i : {1, 0}) {
+          readOpliOp(reader, ins->fm.op[i]);
+        }
+        if (is_rhythm) {
+          ins->fm.opllPreset = (uint8_t)(1 << 4);
+
+        }
+        else if (is_2x2op) {
+          // Note: Pair detuning offset not mappable. Use E5xx effect :P
+          ins->name = fmt::sprintf("%s (1)", insName);
+          ret.push_back(ins);
+
+          ins = new DivInstrument;
+          ins->type = DIV_INS_OPL;
+          ins->name = fmt::sprintf("%s (2)", insName);
+          for (int i : {1, 0}) {
+            readOpliOp(reader, ins->fm.op[i]);
+          }
+        }
+      }
+
+      // Skip rest of file
+      reader.seek(0, SEEK_END);
+      ret.push_back(ins);
+    }
+  }
+  catch (EndOfFileException& e) {
+    lastError = "premature end of file";
+    logE("premature end of file");
+    if (ins != NULL) {
+      delete ins;
+    }
+  }
+}
+
+void DivEngine::loadWOPN(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath) {
+  std::vector<DivInstrument*> insList;
+  int readCount = 0;
+
+  uint16_t version;
+  uint16_t meloBankCount;
+  uint16_t percBankCount;
+  std::vector<midibank_t*> meloMetadata;
+  std::vector<midibank_t*> percMetadata;
+
+  auto readWopnOp = [](SafeReader& reader, DivInstrumentFM::Operator& op) {
+    uint8_t dtMul = reader.readC();
+    uint8_t totalLevel = reader.readC();
+    uint8_t arRateScale = reader.readC();
+    uint8_t drAmpEnable = reader.readC();
+    uint8_t d2r = reader.readC();
+    uint8_t susRelease = reader.readC();
+    uint8_t ssgEg = reader.readC();
+    int total = 0;
+
+    total += (op.mult = dtMul & 0xF);
+    total += (op.dt = ((dtMul >> 4) & 0x7));
+    total += (op.tl = totalLevel & 0x3F);
+    total += (op.rs = ((arRateScale >> 6) & 0x3));
+    total += (op.ar = arRateScale & 0x1F);
+    total += (op.dr = drAmpEnable & 0x1F);
+    total += (op.am = ((drAmpEnable >> 7) & 0x1));
+    total += (op.d2r = d2r & 0x1F);
+    total += (op.rr = susRelease & 0xF);
+    total += (op.sl = ((susRelease >> 4) & 0xF));
+    total += (op.ssgEnv = ssgEg);
+    return total;
+  };
+  auto doParseWopnInstrument = [&](midibank_t*& metadata,  int patchNum) {
+    DivInstrument* ins = new DivInstrument;
+    try {
+      int patchTotal = 0;
+      ins->type = DIV_INS_FM;
+      ins->fm.ops = 4;
+
+      // Establish if it is a blank instrument.
+      String insName = reader.readString(32);
+      patchTotal += insName.size();
+
+      if (!reader.seek(3, SEEK_CUR)) {  // skip MIDI params
+        throw EndOfFileException(&reader, reader.tell() + 3);
+      }
+      uint8_t feedAlgo = reader.readC();
+      patchTotal += feedAlgo;
+      ins->fm.alg = (feedAlgo & 0x7);
+      ins->fm.fb = ((feedAlgo >> 3) & 0x7);
+      patchTotal += reader.readC();  // Skip global bank flags - see WOPN/OPNI spec
+
+      for (int i = 0; i < 4; ++i) {
+        patchTotal += readWopnOp(reader, ins->fm.op[i]);
+      }
+
+      if (version >= 2) {
+        patchTotal += reader.readS_BE(); // skip keyon delay
+        patchTotal += reader.readS_BE(); // skip keyoff delay
+      }
+
+      if (patchTotal > 0) {
+        // Write instrument          
+        ins->name = stringNotBlank(insName) 
+          ? insName 
+          : fmt::sprintf("%s[%d/%d] Patch %d", stripPath, metadata->bankMsb, metadata->bankLsb, patchNum);
+        ret.push_back(ins);
+        ++readCount;
+      } else {
+        // Empty instrument
+        delete ins;
+      }
+    } catch (...) {
+      // Deallocate and allow outer handler to do the rest.
+      delete ins;
+      throw;
+    }
+  };
+
+  try {
+    reader.seek(0, SEEK_SET);
+
+    String header = reader.readString(11);
+    if (header == "WOPN2-BANK" || header == "WOPN2-B2NK") {  // omfg >_<
+      version = reader.readS();
+      if (!(version >= 2) || version > 0xF) {
+        // version 1 doesn't have a version field........
+        reader.seek(-2, SEEK_CUR);
+      }
+
+      if (version >= 2) {
+        meloBankCount = reader.readS_BE();
+        percBankCount = reader.readS_BE();
+        reader.readC(); // skip chip-global LFO
+
+        for (int i = 0; i < meloBankCount; ++i) {
+          meloMetadata.push_back(new midibank_t);
+          String bankName = reader.readString(32);
+          meloMetadata[i]->bankLsb = reader.readC();
+          meloMetadata[i]->bankMsb = reader.readC();
+          meloMetadata[i]->name = stringNotBlank(bankName) 
+            ? bankName
+            : fmt::sprintf("Bank[%d/%d]", meloMetadata[i]->bankMsb, meloMetadata[i]->bankLsb);
+        }
+
+        for (int i = 0; i < percBankCount; ++i) {
+          percMetadata.push_back(new midibank_t);
+          String bankName = reader.readString(32);
+          percMetadata[i]->bankLsb = reader.readC();
+          percMetadata[i]->bankMsb = reader.readC();
+          percMetadata[i]->name = stringNotBlank(bankName)
+            ? bankName
+            : fmt::sprintf("Bank[%d/%d]", percMetadata[i]->bankMsb, percMetadata[i]->bankLsb);
+        }
+
+        for (int i = 0; i < meloBankCount; ++i) {
+          for (int j = 0; j < 128; ++j) {
+            doParseWopnInstrument(meloMetadata[i], j);
+          }
+        }
+        for (int i = 0; i < percBankCount; ++i) {
+          for (int j = 0; j < 128; ++j) {
+            doParseWopnInstrument(percMetadata[i], j);
+          }
+        }
+      }
+    }
+  }
+  catch (EndOfFileException& e) {
+    lastError = "premature end of file";
+    logE("premature end of file");
+  }
+
+  for (midibank_t* m : meloMetadata) {
+    delete m;
+  }
+  for (midibank_t* m : percMetadata) {
+    delete m;
+  }
+}
+
 std::vector<DivInstrument*> DivEngine::instrumentFromFile(const char* path) {
   std::vector<DivInstrument*> ret;
   warnings="";
@@ -1515,7 +1761,11 @@ std::vector<DivInstrument*> DivEngine::instrumentFromFile(const char* path) {
         format=DIV_INSFORMAT_OPM;
       } else if (extS==".ff") {
         format=DIV_INSFORMAT_FF;
-      }
+      } else if (extS==".wopl") {
+        format=DIV_INSFORMAT_WOPL;
+      } else if (extS==".wopn") {
+        format=DIV_INSFORMAT_WOPN;
+      } 
     }
 
     switch (format) {
@@ -1547,6 +1797,10 @@ std::vector<DivInstrument*> DivEngine::instrumentFromFile(const char* path) {
         break;
       case DIV_INSFORMAT_OPM: loadOPM(reader,ret,stripPath);
         break;
+      case DIV_INSFORMAT_WOPL: loadWOPL(reader,ret,stripPath);
+        break;
+      case DIV_INSFORMAT_WOPN: loadWOPN(reader,ret,stripPath);
+        break;
     }
 
     if (reader.tell()<reader.size()) {
diff --git a/src/gui/gui.cpp b/src/gui/gui.cpp
index c572af6c0..327f65ff6 100644
--- a/src/gui/gui.cpp
+++ b/src/gui/gui.cpp
@@ -1305,9 +1305,25 @@ void FurnaceGUI::openFileDialog(FurnaceGUIFileDialogs type) {
       if (!dirExists(workingDirIns)) workingDirIns=getHomeDir();
       hasOpened=fileDialog->openLoad(
         "Load Instrument",
-        {"compatible files", "*.fui *.dmp *.tfi *.vgi *.s3i *.sbi *.opli *.opni *.y12 *.bnk *.ff *.gyb *.opm",
-         "all files", ".*"},
-        "compatible files{.fui,.dmp,.tfi,.vgi,.s3i,.sbi,.opli,.opni,.y12,.bnk,.ff,.gyb,.opm},.*",
+        // TODO supply loadable formats in a dynamic, scalable, "DRY" way.
+        {"All compatible files", "*.fui *.dmp *.tfi *.vgi *.s3i *.sbi *.opli *.opni *.y12 *.bnk *.ff *.gyb *.opm *.wopl *.wopn",
+         "Furnace Instrument", "*.fui",
+         "Deflemask Preset", "*.dmp",
+         "TFM Music Maker Instrument", "*.tfi",
+         "VGM Music Maker Instrument", "*.vgi",
+         "Scream Tracker 3 Instrument", "*.s3i",
+         "SoundBlaster Instrument", "*.sbi",
+         "Wohlstand OPL Instrument", "*.opli",
+         "Wohlstand OPN Instrument", "*.opni",
+         "Gens KMod Patch Dump", "*.y12",
+         "BNK File (Adlib)", "*.bnk",
+         "FF Preset Bank", "*.ff",
+         "2612edit GYB Preset Bank", "*.gyb",
+         "VOPM Preset Bank", "*.opm",
+         "Wohlstand WOPL Bank", "*.wopl",
+         "Wohlstand WOPN Bank", "*.wopn",
+         "All files", ".*"},
+        "All compatible files{.fui,.dmp,.tfi,.vgi,.s3i,.sbi,.opli,.opni,.y12,.bnk,.ff,.gyb,.opm,.wopl,.wopn},.*",
         workingDirIns,
         dpiScale,
         [this](const char* path) {

From f8ffe04869624c8bea28393a22bc3f97d53b8b36 Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Mon, 9 May 2022 19:54:53 +1000
Subject: [PATCH 15/24] WOPN complete

---
 src/engine/fileOpsIns.cpp | 63 +++++++++++++++++++++++++--------------
 1 file changed, 40 insertions(+), 23 deletions(-)

diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index 91490f07d..80368cb7d 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -659,6 +659,7 @@ void DivEngine::loadOPLI(SafeReader& reader, std::vector<DivInstrument*>& ret, S
       String insName = reader.readString(32);
       insName = stringNotBlank(insName) ? insName : stripPath;
       ins->name = insName;
+      // TODO adapt MIDI key offset to transpose?
       reader.seek(7, SEEK_CUR);  // skip MIDI params
       uint8_t instTypeFlags = reader.readC();  // [0EEEDCBA] - see WOPL/OPLI spec
 
@@ -746,6 +747,7 @@ void DivEngine::loadOPNI(SafeReader& reader, std::vector<DivInstrument*>& ret, S
       if (!(version >= 2) || version > 0xF) {
         // version 1 doesn't have a version field........
         reader.seek(-2, SEEK_CUR);
+        version = 1;
       }
 
       reader.readC(); // skip isPerc
@@ -754,6 +756,7 @@ void DivEngine::loadOPNI(SafeReader& reader, std::vector<DivInstrument*>& ret, S
 
       String insName = reader.readString(32);
       ins->name = stringNotBlank(insName) ? insName : stripPath;
+      // TODO adapt MIDI key offset to transpose?
       if (!reader.seek(3, SEEK_CUR)) {  // skip MIDI params
         throw EndOfFileException(&reader, reader.tell() + 3);
       }
@@ -1398,6 +1401,7 @@ void DivEngine::loadWOPL(SafeReader& reader, std::vector<DivInstrument*>& ret, S
       String insName = reader.readString(32);
       insName = stringNotBlank(insName) ? insName : stripPath;
       ins->name = insName;
+      // TODO adapt MIDI key offset to transpose?
       reader.seek(7, SEEK_CUR);  // skip MIDI params
       uint8_t instTypeFlags = reader.readC();  // [0EEEDCBA] - see WOPL/OPLI spec
 
@@ -1509,10 +1513,10 @@ void DivEngine::loadWOPN(SafeReader& reader, std::vector<DivInstrument*>& ret, S
     total += (op.ssgEnv = ssgEg);
     return total;
   };
-  auto doParseWopnInstrument = [&](midibank_t*& metadata,  int patchNum) {
+  auto doParseWopnInstrument = [&](bool isPerc, midibank_t*& metadata, int patchNum) {
     DivInstrument* ins = new DivInstrument;
     try {
-      int patchTotal = 0;
+      long patchTotal = 0;
       ins->type = DIV_INS_FM;
       ins->fm.ops = 4;
 
@@ -1520,6 +1524,7 @@ void DivEngine::loadWOPN(SafeReader& reader, std::vector<DivInstrument*>& ret, S
       String insName = reader.readString(32);
       patchTotal += insName.size();
 
+      // TODO adapt MIDI key offset to transpose?
       if (!reader.seek(3, SEEK_CUR)) {  // skip MIDI params
         throw EndOfFileException(&reader, reader.tell() + 3);
       }
@@ -1534,15 +1539,17 @@ void DivEngine::loadWOPN(SafeReader& reader, std::vector<DivInstrument*>& ret, S
       }
 
       if (version >= 2) {
-        patchTotal += reader.readS_BE(); // skip keyon delay
-        patchTotal += reader.readS_BE(); // skip keyoff delay
+        reader.readS_BE(); // skip keyon delay
+        reader.readS_BE(); // skip keyoff delay
       }
 
       if (patchTotal > 0) {
-        // Write instrument          
+        // Write instrument
+        // TODO: OPN2BankEditor hardcodes GM1 Melodic patch names which are not included in the bank file......
         ins->name = stringNotBlank(insName) 
           ? insName 
-          : fmt::sprintf("%s[%d/%d] Patch %d", stripPath, metadata->bankMsb, metadata->bankLsb, patchNum);
+          : fmt::sprintf("%s[%s] %s Patch %d", 
+            stripPath, metadata->name, (isPerc) ? "Drum" : "Melodic", patchNum);
         ret.push_back(ins);
         ++readCount;
       } else {
@@ -1565,21 +1572,22 @@ void DivEngine::loadWOPN(SafeReader& reader, std::vector<DivInstrument*>& ret, S
       if (!(version >= 2) || version > 0xF) {
         // version 1 doesn't have a version field........
         reader.seek(-2, SEEK_CUR);
+        version = 1;
       }
 
-      if (version >= 2) {
-        meloBankCount = reader.readS_BE();
-        percBankCount = reader.readS_BE();
-        reader.readC(); // skip chip-global LFO
+      meloBankCount = reader.readS_BE();
+      percBankCount = reader.readS_BE();
+      reader.readC(); // skip chip-global LFO
 
+      if (version >= 2) {
         for (int i = 0; i < meloBankCount; ++i) {
           meloMetadata.push_back(new midibank_t);
           String bankName = reader.readString(32);
           meloMetadata[i]->bankLsb = reader.readC();
           meloMetadata[i]->bankMsb = reader.readC();
-          meloMetadata[i]->name = stringNotBlank(bankName) 
+          meloMetadata[i]->name = stringNotBlank(bankName)
             ? bankName
-            : fmt::sprintf("Bank[%d/%d]", meloMetadata[i]->bankMsb, meloMetadata[i]->bankLsb);
+            : fmt::sprintf("%d/%d", meloMetadata[i]->bankMsb, meloMetadata[i]->bankLsb);
         }
 
         for (int i = 0; i < percBankCount; ++i) {
@@ -1589,23 +1597,32 @@ void DivEngine::loadWOPN(SafeReader& reader, std::vector<DivInstrument*>& ret, S
           percMetadata[i]->bankMsb = reader.readC();
           percMetadata[i]->name = stringNotBlank(bankName)
             ? bankName
-            : fmt::sprintf("Bank[%d/%d]", percMetadata[i]->bankMsb, percMetadata[i]->bankLsb);
+            : fmt::sprintf("%d/%d", percMetadata[i]->bankMsb, percMetadata[i]->bankLsb);
         }
+      } else {
+        // TODO do version 1 multibank sets even exist?
+        meloMetadata.push_back(new midibank_t);
+        meloMetadata[0]->bankLsb = 0;
+        meloMetadata[0]->bankMsb = 0;
+        meloMetadata[0]->name = "0/0";
+        percMetadata.push_back(new midibank_t);
+        percMetadata[0]->bankLsb = 0;
+        percMetadata[0]->bankMsb = 0;
+        percMetadata[0]->name = "0/0";
+      }
 
-        for (int i = 0; i < meloBankCount; ++i) {
-          for (int j = 0; j < 128; ++j) {
-            doParseWopnInstrument(meloMetadata[i], j);
-          }
+      for (int i = 0; i < meloBankCount; ++i) {
+        for (int j = 0; j < 128; ++j) {
+          doParseWopnInstrument(false, meloMetadata[i], j);
         }
-        for (int i = 0; i < percBankCount; ++i) {
-          for (int j = 0; j < 128; ++j) {
-            doParseWopnInstrument(percMetadata[i], j);
-          }
+      }
+      for (int i = 0; i < percBankCount; ++i) {
+        for (int j = 0; j < 128; ++j) {
+          doParseWopnInstrument(true, percMetadata[i], j);
         }
       }
     }
-  }
-  catch (EndOfFileException& e) {
+  } catch (EndOfFileException& e) {
     lastError = "premature end of file";
     logE("premature end of file");
   }

From 158163312ba52bdc7dd375c0d7c585fcfb2958f2 Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Mon, 9 May 2022 20:06:09 +1000
Subject: [PATCH 16/24] WOPN redpath

---
 src/engine/fileOpsIns.cpp | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index 80368cb7d..468a871e1 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -1442,8 +1442,7 @@ void DivEngine::loadWOPL(SafeReader& reader, std::vector<DivInstrument*>& ret, S
         for (int i : {2, 0, 3, 1}) { // omfg >_<
           readOpliOp(reader, ins->fm.op[i]);
         }
-      }
-      else {
+      } else {
         ins->fm.ops = 2;
         for (int i : {1, 0}) {
           readOpliOp(reader, ins->fm.op[i]);
@@ -1470,8 +1469,7 @@ void DivEngine::loadWOPL(SafeReader& reader, std::vector<DivInstrument*>& ret, S
       reader.seek(0, SEEK_END);
       ret.push_back(ins);
     }
-  }
-  catch (EndOfFileException& e) {
+  } catch (EndOfFileException& e) {
     lastError = "premature end of file";
     logE("premature end of file");
     if (ins != NULL) {
@@ -1483,6 +1481,7 @@ void DivEngine::loadWOPL(SafeReader& reader, std::vector<DivInstrument*>& ret, S
 void DivEngine::loadWOPN(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath) {
   std::vector<DivInstrument*> insList;
   int readCount = 0;
+  bool is_failed = false;
 
   uint16_t version;
   uint16_t meloBankCount;
@@ -1625,6 +1624,7 @@ void DivEngine::loadWOPN(SafeReader& reader, std::vector<DivInstrument*>& ret, S
   } catch (EndOfFileException& e) {
     lastError = "premature end of file";
     logE("premature end of file");
+    is_failed = true;
   }
 
   for (midibank_t* m : meloMetadata) {
@@ -1633,6 +1633,12 @@ void DivEngine::loadWOPN(SafeReader& reader, std::vector<DivInstrument*>& ret, S
   for (midibank_t* m : percMetadata) {
     delete m;
   }
+
+  if (is_failed) {
+    for (DivInstrument* p : insList) {
+      delete p;
+    }
+  }
 }
 
 std::vector<DivInstrument*> DivEngine::instrumentFromFile(const char* path) {

From ef4a60e09554cb1181b9e67444ae47e74793dac2 Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Mon, 9 May 2022 21:05:43 +1000
Subject: [PATCH 17/24] WOPL progress!

---
 src/engine/fileOpsIns.cpp | 267 ++++++++++++++++++++++++++++----------
 1 file changed, 198 insertions(+), 69 deletions(-)

diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index 468a871e1..a8e1b126f 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -525,6 +525,7 @@ void DivEngine::loadS3I(SafeReader& reader, std::vector<DivInstrument*>& ret, St
 }
 
 void DivEngine::loadSBI(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath) {
+  std::vector<DivInstrument*> insList;
   DivInstrument* ins=new DivInstrument;
   try {
     reader.seek(0, SEEK_SET);
@@ -590,7 +591,7 @@ void DivEngine::loadSBI(SafeReader& reader, std::vector<DivInstrument*>& ret, St
 
       // Ignore rest of file - rest is 'reserved padding'.
       reader.seek(4, SEEK_CUR);
-      ret.push_back(ins);
+      insList.push_back(ins);
 
     } else if (is_4op || is_6op) {
       readSbiOpData(sbi_op34, reader);
@@ -613,7 +614,7 @@ void DivEngine::loadSBI(SafeReader& reader, std::vector<DivInstrument*>& ret, St
         // Freq Monster 801 6op SBIs use a 4+2op layout
         // Save the 4op portion before reading the 2op part
         ins->name = fmt::sprintf("%s (4op)", ins->name);
-        ret.push_back(ins);
+        insList.push_back(ins);
 
         readSbiOpData(sbi_op12, reader);
 
@@ -632,7 +633,7 @@ void DivEngine::loadSBI(SafeReader& reader, std::vector<DivInstrument*>& ret, St
       // Note: Freq Monster 801 adds a ton of other additional fields irrelevant to chip registers.
       //       If instrument transpose is ever supported, we can read it in maybe?
       reader.seek(0, SEEK_END);
-      ret.push_back(ins);
+      insList.push_back(ins);
     }
 
   } catch (EndOfFileException& e) {
@@ -641,18 +642,27 @@ void DivEngine::loadSBI(SafeReader& reader, std::vector<DivInstrument*>& ret, St
     if (ins != NULL) {
       delete ins;
     }
+    for (DivInstrument* p : insList) {
+      delete p;
+    }
+    return;
+  }
+
+  for (DivInstrument* p : insList) {
+    ret.push_back(p);
   }
 }
 
 void DivEngine::loadOPLI(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath) {
+  std::vector<DivInstrument*> insList;
   DivInstrument* ins = new DivInstrument;
 
   try {
     reader.seek(0, SEEK_SET);
     String header = reader.readString(11);
+
     if (header == "WOPL3-INST") {
       uint16_t version = reader.readS();
-
       reader.readC();  // skip isPerc field
 
       ins->type = DIV_INS_OPL;
@@ -711,7 +721,7 @@ void DivEngine::loadOPLI(SafeReader& reader, std::vector<DivInstrument*>& ret, S
         } else if (is_2x2op) {
           // Note: Pair detuning offset not mappable. Use E5xx effect :P
           ins->name = fmt::sprintf("%s (1)", insName);
-          ret.push_back(ins);
+          insList.push_back(ins);
 
           ins = new DivInstrument;
           ins->type = DIV_INS_OPL;
@@ -724,7 +734,7 @@ void DivEngine::loadOPLI(SafeReader& reader, std::vector<DivInstrument*>& ret, S
 
       // Skip rest of file
       reader.seek(0, SEEK_END);
-      ret.push_back(ins);
+      insList.push_back(ins);
     }
   } catch (EndOfFileException& e) {
     lastError="premature end of file";
@@ -732,6 +742,14 @@ void DivEngine::loadOPLI(SafeReader& reader, std::vector<DivInstrument*>& ret, S
     if (ins != NULL) {
       delete ins;
     }
+    for (DivInstrument* p : insList) {
+      delete p;
+    }
+    return;
+  }
+
+  for (DivInstrument* p : insList) {
+    ret.push_back(p);
   }
 }
 
@@ -1387,100 +1405,208 @@ void DivEngine::loadOPM(SafeReader& reader, std::vector<DivInstrument*>& ret, St
 
 
 void DivEngine::loadWOPL(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath) {
-  DivInstrument* ins = new DivInstrument;
+  std::vector<DivInstrument*> insList;
+  bool is_failed = false;
 
-  try {
-    reader.seek(0, SEEK_SET);
-    String header = reader.readString(11);
-    if (header == "WOPL3-BANK") {
-      uint16_t version = reader.readS();
+  uint16_t version;
+  uint16_t meloBankCount;
+  uint16_t percBankCount;
+  std::vector<midibank_t*> meloMetadata;
+  std::vector<midibank_t*> percMetadata;
 
-      reader.readC();  // skip isPerc field
+  auto readWoplOp = [](SafeReader& reader, DivInstrumentFM::Operator& op) {
+    uint8_t characteristics = reader.readC();
+    uint8_t keyScaleLevel = reader.readC();
+    uint8_t attackDecay = reader.readC();
+    uint8_t sustainRelease = reader.readC();
+    uint8_t waveSelect = reader.readC();
+    int total = 0;
 
+    total += (op.mult = characteristics & 0xF);
+    total += (op.ksr = ((characteristics >> 4) & 0x1));
+    total += (op.sus = ((characteristics >> 5) & 0x1));
+    total += (op.vib = ((characteristics >> 6) & 0x1));
+    total += (op.am = ((characteristics >> 7) & 0x1));
+    total += (op.tl = keyScaleLevel & 0x3F);
+    total += (op.ksl = ((keyScaleLevel >> 6) & 0x3));
+    total += (op.ar = ((attackDecay >> 4) & 0xF));
+    total += (op.dr = attackDecay & 0xF);
+    total += (op.rr = sustainRelease & 0xF);
+    total += (op.sl = ((sustainRelease >> 4) & 0xF));
+    total += (op.ws = waveSelect);
+    return total;
+  };
+
+  auto doParseWoplInstrument = [&](bool isPerc, midibank_t*& metadata, int patchNum) {
+    DivInstrument* ins = new DivInstrument;
+    DivInstrument* insPair = NULL;
+    try {
+      long patchSum = 0;
       ins->type = DIV_INS_OPL;
+
+      // Establish if it is a blank instrument.
       String insName = reader.readString(32);
-      insName = stringNotBlank(insName) ? insName : stripPath;
-      ins->name = insName;
+      patchSum += insName.size();
+      
       // TODO adapt MIDI key offset to transpose?
       reader.seek(7, SEEK_CUR);  // skip MIDI params
       uint8_t instTypeFlags = reader.readC();  // [0EEEDCBA] - see WOPL/OPLI spec
 
       bool is_4op = ((instTypeFlags & 0x1) == 1);
-      bool is_2x2op = (((instTypeFlags >> 1) & 0x1) == 1);
-      bool is_rhythm = (((instTypeFlags >> 4) & 0x7) > 0);
-
-      auto readOpliOp = [](SafeReader& reader, DivInstrumentFM::Operator& op) {
-        uint8_t characteristics = reader.readC();
-        uint8_t keyScaleLevel = reader.readC();
-        uint8_t attackDecay = reader.readC();
-        uint8_t sustainRelease = reader.readC();
-        uint8_t waveSelect = reader.readC();
-
-        op.mult = characteristics & 0xF;
-        op.ksr = ((characteristics >> 4) & 0x1);
-        op.sus = ((characteristics >> 5) & 0x1);
-        op.vib = ((characteristics >> 6) & 0x1);
-        op.am = ((characteristics >> 7) & 0x1);
-        op.tl = keyScaleLevel & 0x3F;
-        op.ksl = ((keyScaleLevel >> 6) & 0x3);
-        op.ar = ((attackDecay >> 4) & 0xF);
-        op.dr = attackDecay & 0xF;
-        op.rr = sustainRelease & 0xF;
-        op.sl = ((sustainRelease >> 4) & 0xF);
-        op.ws = waveSelect;
-      };
+      bool is_2x2op = (((instTypeFlags>>1) & 0x1) == 1);
+      bool is_rhythm = (((instTypeFlags>>4) & 0x7) > 0);
 
       uint8_t feedConnect = reader.readC();
       uint8_t feedConnect2nd = reader.readC();
 
       ins->fm.alg = (feedConnect & 0x1);
-      ins->fm.fb = ((feedConnect >> 1) & 0xF);
+      ins->fm.fb = ((feedConnect>>1) & 0xF);
 
       if (is_4op && !is_2x2op) {
         ins->fm.ops = 4;
         ins->fm.alg = (feedConnect & 0x1) | ((feedConnect2nd & 0x1) << 1);
-        for (int i : {2, 0, 3, 1}) { // omfg >_<
-          readOpliOp(reader, ins->fm.op[i]);
+        for (int i : {2,0,3,1}) { // omfg >_<
+          patchSum += readWoplOp(reader, ins->fm.op[i]);
         }
       } else {
         ins->fm.ops = 2;
-        for (int i : {1, 0}) {
-          readOpliOp(reader, ins->fm.op[i]);
+        for (int i : {1,0}) {
+          patchSum += readWoplOp(reader, ins->fm.op[i]);
         }
         if (is_rhythm) {
-          ins->fm.opllPreset = (uint8_t)(1 << 4);
-
-        }
-        else if (is_2x2op) {
+          ins->fm.opllPreset = (uint8_t)(1<<4);
+        } else if (is_2x2op) {
           // Note: Pair detuning offset not mappable. Use E5xx effect :P
-          ins->name = fmt::sprintf("%s (1)", insName);
-          ret.push_back(ins);
-
-          ins = new DivInstrument;
-          ins->type = DIV_INS_OPL;
-          ins->name = fmt::sprintf("%s (2)", insName);
-          for (int i : {1, 0}) {
-            readOpliOp(reader, ins->fm.op[i]);
+          ins->name = stringNotBlank(insName)
+            ? fmt::sprintf("%s (1)", insName)
+            : fmt::sprintf("%s[%s] %s Patch %d (1)",
+              stripPath, metadata->name, (isPerc) ? "Drum" : "Melodic", patchNum);
+          insList.push_back(ins);
+          patchSum = 0;
+          insPair = new DivInstrument;
+          insPair->type = DIV_INS_OPL;
+          insPair->name = fmt::sprintf("%s (2)", insName);
+          for (int i : {1,0}) {
+            patchSum += readWoplOp(reader, ins->fm.op[i]);
           }
         }
+
+        if (!is_2x2op) {
+          reader.seek(10, SEEK_CUR); // skip unused operator pair
+        }
       }
 
-      // Skip rest of file
-      reader.seek(0, SEEK_END);
-      ret.push_back(ins);
+      if (version >= 3) {
+        reader.readS_BE(); // skip keyon delay
+        reader.readS_BE(); // skip keyoff delay
+      }
+
+      if (patchSum > 0) {
+        // Write instrument
+        // TODO: OPL3BankEditor hardcodes GM1 Melodic patch names which are not included in the bank file......
+        if (is_2x2op) {
+          ins->name = stringNotBlank(insName)
+            ? fmt::sprintf("%s (2)", insName)
+            : fmt::sprintf("%s[%s] %s Patch %d (2)",
+              stripPath, metadata->name, (isPerc) ? "Drum" : "Melodic", patchNum);
+        } else {
+          ins->name = stringNotBlank(insName)
+            ? insName
+            : fmt::sprintf("%s[%s] %s Patch %d",
+              stripPath, metadata->name, (isPerc) ? "Drum" : "Melodic", patchNum);
+        }
+        insList.push_back(ins);
+      } else {
+        // Empty instrument
+        delete ins;
+      }
+    } catch (...) {
+      // Deallocate and allow outer handler to do the rest.
+      delete ins;
+      throw;
+    }
+  };
+
+  try {
+    reader.seek(0, SEEK_SET);
+
+    String header = reader.readString(11);
+    if (header == "WOPL3-BANK") {
+      version = reader.readS();
+      meloBankCount = reader.readS_BE();
+      percBankCount = reader.readS_BE();
+      reader.readC(); // skip chip-global LFO
+      reader.readC(); // skip additional flags
+
+      if (version >= 2) {
+        for (int i = 0; i < meloBankCount; ++i) {
+          meloMetadata.push_back(new midibank_t);
+          String bankName = reader.readString(32);
+          meloMetadata[i]->bankLsb = reader.readC();
+          meloMetadata[i]->bankMsb = reader.readC();
+          meloMetadata[i]->name = stringNotBlank(bankName)
+            ? bankName
+            : fmt::sprintf("%d/%d", meloMetadata[i]->bankMsb, meloMetadata[i]->bankLsb);
+        }
+
+        for (int i = 0; i < percBankCount; ++i) {
+          percMetadata.push_back(new midibank_t);
+          String bankName = reader.readString(32);
+          percMetadata[i]->bankLsb = reader.readC();
+          percMetadata[i]->bankMsb = reader.readC();
+          percMetadata[i]->name = stringNotBlank(bankName)
+            ? bankName
+            : fmt::sprintf("%d/%d", percMetadata[i]->bankMsb, percMetadata[i]->bankLsb);
+        }
+      } else {
+        // TODO do version 1 multibank sets even exist?
+        meloMetadata.push_back(new midibank_t);
+        meloMetadata[0]->bankLsb = 0;
+        meloMetadata[0]->bankMsb = 0;
+        meloMetadata[0]->name = "0/0";
+        percMetadata.push_back(new midibank_t);
+        percMetadata[0]->bankLsb = 0;
+        percMetadata[0]->bankMsb = 0;
+        percMetadata[0]->name = "0/0";
+      }
+
+      for (int i = 0; i < meloBankCount; ++i) {
+        for (int j = 0; j < 128; ++j) {
+          doParseWoplInstrument(false, meloMetadata[i], j);
+        }
+      }
+      for (int i = 0; i < percBankCount; ++i) {
+        for (int j = 0; j < 128; ++j) {
+          doParseWoplInstrument(true, percMetadata[i], j);
+        }
+      }
     }
   } catch (EndOfFileException& e) {
     lastError = "premature end of file";
     logE("premature end of file");
-    if (ins != NULL) {
-      delete ins;
+    is_failed = true;
+  }
+
+  for (midibank_t* m : meloMetadata) {
+    delete m;
+  }
+  for (midibank_t* m : percMetadata) {
+    delete m;
+  }
+
+  if (is_failed) {
+    for (DivInstrument* p : insList) {
+      delete p;
+    }
+  } else {
+    for (DivInstrument* p : insList) {
+      ret.push_back(p);
     }
   }
 }
 
 void DivEngine::loadWOPN(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath) {
   std::vector<DivInstrument*> insList;
-  int readCount = 0;
   bool is_failed = false;
 
   uint16_t version;
@@ -1515,26 +1641,26 @@ void DivEngine::loadWOPN(SafeReader& reader, std::vector<DivInstrument*>& ret, S
   auto doParseWopnInstrument = [&](bool isPerc, midibank_t*& metadata, int patchNum) {
     DivInstrument* ins = new DivInstrument;
     try {
-      long patchTotal = 0;
+      long patchSum = 0;
       ins->type = DIV_INS_FM;
       ins->fm.ops = 4;
 
       // Establish if it is a blank instrument.
       String insName = reader.readString(32);
-      patchTotal += insName.size();
+      patchSum += insName.size();
 
       // TODO adapt MIDI key offset to transpose?
       if (!reader.seek(3, SEEK_CUR)) {  // skip MIDI params
         throw EndOfFileException(&reader, reader.tell() + 3);
       }
       uint8_t feedAlgo = reader.readC();
-      patchTotal += feedAlgo;
+      patchSum += feedAlgo;
       ins->fm.alg = (feedAlgo & 0x7);
       ins->fm.fb = ((feedAlgo >> 3) & 0x7);
-      patchTotal += reader.readC();  // Skip global bank flags - see WOPN/OPNI spec
+      patchSum += reader.readC();  // Skip global bank flags - see WOPN/OPNI spec
 
       for (int i = 0; i < 4; ++i) {
-        patchTotal += readWopnOp(reader, ins->fm.op[i]);
+        patchSum += readWopnOp(reader, ins->fm.op[i]);
       }
 
       if (version >= 2) {
@@ -1542,15 +1668,14 @@ void DivEngine::loadWOPN(SafeReader& reader, std::vector<DivInstrument*>& ret, S
         reader.readS_BE(); // skip keyoff delay
       }
 
-      if (patchTotal > 0) {
+      if (patchSum > 0) {
         // Write instrument
         // TODO: OPN2BankEditor hardcodes GM1 Melodic patch names which are not included in the bank file......
         ins->name = stringNotBlank(insName) 
           ? insName 
           : fmt::sprintf("%s[%s] %s Patch %d", 
             stripPath, metadata->name, (isPerc) ? "Drum" : "Melodic", patchNum);
-        ret.push_back(ins);
-        ++readCount;
+        insList.push_back(ins);
       } else {
         // Empty instrument
         delete ins;
@@ -1638,6 +1763,10 @@ void DivEngine::loadWOPN(SafeReader& reader, std::vector<DivInstrument*>& ret, S
     for (DivInstrument* p : insList) {
       delete p;
     }
+  } else {
+    for (DivInstrument* p : insList) {
+      ret.push_back(p);
+    }
   }
 }
 

From 87ffcf2b2730735396cb3faed0073aab60dc1814 Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Mon, 9 May 2022 21:15:26 +1000
Subject: [PATCH 18/24] Adjust seek-ends since exact data reads should be
 correct.

---
 src/engine/fileOpsIns.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index a8e1b126f..491807fac 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -730,10 +730,12 @@ void DivEngine::loadOPLI(SafeReader& reader, std::vector<DivInstrument*>& ret, S
             readOpliOp(reader, ins->fm.op[i]);
           }
         }
+
+        if (!is_2x2op) {
+          reader.seek(10, SEEK_CUR); // skip unused operator pair
+        }
       }
 
-      // Skip rest of file
-      reader.seek(0, SEEK_END);
       insList.push_back(ins);
     }
   } catch (EndOfFileException& e) {
@@ -809,8 +811,6 @@ void DivEngine::loadOPNI(SafeReader& reader, std::vector<DivInstrument*>& ret, S
         readOpniOp(reader, ins->fm.op[i]);
       }
 
-      // Skip rest of file
-      reader.seek(0, SEEK_END);
       ret.push_back(ins);
     }
   } catch (EndOfFileException& e) {

From da6a6f514c95b1964c9571fcfea292b6b831456e Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Mon, 9 May 2022 21:20:14 +1000
Subject: [PATCH 19/24] Fix 2x2op WOPL load

---
 src/engine/fileOpsIns.cpp | 53 +++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 27 deletions(-)

diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index 491807fac..e5582b4aa 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -525,7 +525,7 @@ void DivEngine::loadS3I(SafeReader& reader, std::vector<DivInstrument*>& ret, St
 }
 
 void DivEngine::loadSBI(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath) {
-  std::vector<DivInstrument*> insList;
+  std::vector<DivInstrument*> insList; // in case 2x2op
   DivInstrument* ins=new DivInstrument;
   try {
     reader.seek(0, SEEK_SET);
@@ -654,9 +654,30 @@ void DivEngine::loadSBI(SafeReader& reader, std::vector<DivInstrument*>& ret, St
 }
 
 void DivEngine::loadOPLI(SafeReader& reader, std::vector<DivInstrument*>& ret, String& stripPath) {
-  std::vector<DivInstrument*> insList;
+  std::vector<DivInstrument*> insList; // in case 2x2op
   DivInstrument* ins = new DivInstrument;
 
+  auto readOpliOp = [](SafeReader& reader, DivInstrumentFM::Operator& op) {
+    uint8_t characteristics = reader.readC();
+    uint8_t keyScaleLevel = reader.readC();
+    uint8_t attackDecay = reader.readC();
+    uint8_t sustainRelease = reader.readC();
+    uint8_t waveSelect = reader.readC();
+
+    op.mult = characteristics & 0xF;
+    op.ksr = ((characteristics >> 4) & 0x1);
+    op.sus = ((characteristics >> 5) & 0x1);
+    op.vib = ((characteristics >> 6) & 0x1);
+    op.am = ((characteristics >> 7) & 0x1);
+    op.tl = keyScaleLevel & 0x3F;
+    op.ksl = ((keyScaleLevel >> 6) & 0x3);
+    op.ar = ((attackDecay >> 4) & 0xF);
+    op.dr = attackDecay & 0xF;
+    op.rr = sustainRelease & 0xF;
+    op.sl = ((sustainRelease >> 4) & 0xF);
+    op.ws = waveSelect;
+  };
+
   try {
     reader.seek(0, SEEK_SET);
     String header = reader.readString(11);
@@ -677,27 +698,6 @@ void DivEngine::loadOPLI(SafeReader& reader, std::vector<DivInstrument*>& ret, S
       bool is_2x2op = (((instTypeFlags>>1) & 0x1) == 1);
       bool is_rhythm = (((instTypeFlags>>4) & 0x7) > 0);
 
-      auto readOpliOp = [](SafeReader& reader, DivInstrumentFM::Operator& op) {
-        uint8_t characteristics = reader.readC();
-        uint8_t keyScaleLevel = reader.readC();
-        uint8_t attackDecay = reader.readC();
-        uint8_t sustainRelease = reader.readC();
-        uint8_t waveSelect = reader.readC();
-
-        op.mult = characteristics & 0xF;
-        op.ksr = ((characteristics >> 4) & 0x1);
-        op.sus = ((characteristics >> 5) & 0x1);
-        op.vib = ((characteristics >> 6) & 0x1);
-        op.am = ((characteristics >> 7) & 0x1);
-        op.tl = keyScaleLevel & 0x3F;
-        op.ksl = ((keyScaleLevel >> 6) & 0x3);
-        op.ar = ((attackDecay >> 4) & 0xF);
-        op.dr = attackDecay & 0xF;
-        op.rr = sustainRelease & 0xF;
-        op.sl = ((sustainRelease >> 4) & 0xF);
-        op.ws = waveSelect;
-      };
-
       uint8_t feedConnect = reader.readC();
       uint8_t feedConnect2nd = reader.readC();
 
@@ -1439,7 +1439,6 @@ void DivEngine::loadWOPL(SafeReader& reader, std::vector<DivInstrument*>& ret, S
 
   auto doParseWoplInstrument = [&](bool isPerc, midibank_t*& metadata, int patchNum) {
     DivInstrument* ins = new DivInstrument;
-    DivInstrument* insPair = NULL;
     try {
       long patchSum = 0;
       ins->type = DIV_INS_OPL;
@@ -1483,9 +1482,9 @@ void DivEngine::loadWOPL(SafeReader& reader, std::vector<DivInstrument*>& ret, S
               stripPath, metadata->name, (isPerc) ? "Drum" : "Melodic", patchNum);
           insList.push_back(ins);
           patchSum = 0;
-          insPair = new DivInstrument;
-          insPair->type = DIV_INS_OPL;
-          insPair->name = fmt::sprintf("%s (2)", insName);
+          ins = new DivInstrument;
+          ins->type = DIV_INS_OPL;
+          ins->name = fmt::sprintf("%s (2)", insName);
           for (int i : {1,0}) {
             patchSum += readWoplOp(reader, ins->fm.op[i]);
           }

From 0e077432fcd78e3a8433ca248796adc1f679d985 Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Mon, 9 May 2022 21:43:03 +1000
Subject: [PATCH 20/24] =?UTF-8?q?Compilerschei=C3=9Fe=20pedantisch=20uberp?=
 =?UTF-8?q?rufen?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 src/engine/fileOpsIns.cpp | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index e5582b4aa..c43eb5b3d 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -683,7 +683,7 @@ void DivEngine::loadOPLI(SafeReader& reader, std::vector<DivInstrument*>& ret, S
     String header = reader.readString(11);
 
     if (header == "WOPL3-INST") {
-      uint16_t version = reader.readS();
+      reader.readS();  // skip version (presently no difference here)
       reader.readC();  // skip isPerc field
 
       ins->type = DIV_INS_OPL;
@@ -907,7 +907,7 @@ void DivEngine::loadBNK(SafeReader& reader, std::vector<DivInstrument*>& ret, St
         ins->fm.ops = 2;
 
         uint8_t timbreMode = reader.readC();
-        uint8_t timbrePercVoice = reader.readC();
+        reader.readC();  // skip timbre perc voice
         if (timbreMode == 1) {
           ins->fm.opllPreset = (uint8_t)(1<<4);
         }
@@ -1144,8 +1144,7 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
 
         // Instrument data
         for (int i = 0; i < (insMelodyCount+insDrumCount); ++i) {
-          bool isDrum = (i >= insMelodyCount);
-          DivInstrument* newIns = readInstrument(reader, (version == 2));
+          readInstrument(reader, (version == 2));
 
           // Additional data
           reader.readC();  // skip transpose
@@ -1179,9 +1178,9 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
         uint16_t insCount = reader.readS();
 
         for (int i = 0; i < insCount; ++i) {
-          uint16_t patchPos = reader.tell();
-          uint16_t patchSize = reader.readS();
-          DivInstrument* newIns = readInstrument(reader, true);
+          reader.tell();  // skip patchPosOffset
+          reader.readS(); // skip patchSize
+          readInstrument(reader, true);
 
           // Additional data
           reader.readC(); // skip transpose

From fbcd84d79b3857a0b0d288c51b0d37a2f2fa6059 Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Thu, 12 May 2022 10:29:45 +1000
Subject: [PATCH 21/24] Address review comments

---
 src/gui/gui.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gui/gui.cpp b/src/gui/gui.cpp
index f77955049..2ea1c4270 100644
--- a/src/gui/gui.cpp
+++ b/src/gui/gui.cpp
@@ -1308,7 +1308,7 @@ void FurnaceGUI::openFileDialog(FurnaceGUIFileDialogs type) {
         // TODO supply loadable formats in a dynamic, scalable, "DRY" way.
         {"All compatible files", "*.fui *.dmp *.tfi *.vgi *.s3i *.sbi *.opli *.opni *.y12 *.bnk *.ff *.gyb *.opm *.wopl *.wopn",
          "Furnace Instrument", "*.fui",
-         "Deflemask Preset", "*.dmp",
+         "DefleMask Preset", "*.dmp",
          "TFM Music Maker Instrument", "*.tfi",
          "VGM Music Maker Instrument", "*.vgi",
          "Scream Tracker 3 Instrument", "*.s3i",

From 63d2e12c2788d1c3c70c52ef2157bd0c126a0557 Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Thu, 12 May 2022 10:55:47 +1000
Subject: [PATCH 22/24] Address review comments (missed these)

---
 src/engine/fileOpsIns.cpp | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index c43eb5b3d..b26cc67d6 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -1167,7 +1167,7 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
         uint32_t bankOffset = reader.readI();
         uint32_t mapOffset = reader.readI();
 
-        if (reader.size() != fileSize || bankOffset > fileSize || mapOffset > fileSize) {
+        if (bankOffset > fileSize || mapOffset > fileSize) {
           lastError = "GYBv3 file appears to have invalid data offsets.";
           logE("GYBv3 file appears to have invalid data offsets.");
         }
@@ -1177,9 +1177,9 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
         }
         uint16_t insCount = reader.readS();
 
+        size_t patchPosOffset = reader.tell();
         for (int i = 0; i < insCount; ++i) {
-          reader.tell();  // skip patchPosOffset
-          reader.readS(); // skip patchSize
+          uint16_t patchSize = reader.readS();
           readInstrument(reader, true);
 
           // Additional data
@@ -1196,6 +1196,11 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
 
           // Instrument Name
           readInstrumentName(reader, insList[i]);
+
+          // Retrieve next patch
+          if (!reader.seek(patchPosOffset + patchSize, SEEK_SET)) {
+            throw EndOfFileException(&reader, patchPosOffset + patchSize);
+          }
         }
       }
       reader.seek(0, SEEK_END);

From 1254aec80505bc95b6cd7f525683056a6ab2f49a Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Thu, 12 May 2022 11:00:25 +1000
Subject: [PATCH 23/24] Fix GYBv3 load with safer offset load

---
 src/engine/fileOpsIns.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index b26cc67d6..4f08fea62 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -1201,6 +1201,7 @@ void DivEngine::loadGYB(SafeReader& reader, std::vector<DivInstrument*>& ret, St
           if (!reader.seek(patchPosOffset + patchSize, SEEK_SET)) {
             throw EndOfFileException(&reader, patchPosOffset + patchSize);
           }
+          patchPosOffset = reader.tell();
         }
       }
       reader.seek(0, SEEK_END);

From fe11ec387709fb7f6c030058d69e7aee0b3cfbfa Mon Sep 17 00:00:00 2001
From: James Alan Nguyen <djtubig.malicex@gmail.com>
Date: Tue, 17 May 2022 11:42:10 +1000
Subject: [PATCH 24/24] pick nits

---
 src/engine/fileOpsIns.cpp | 42 ++++++++++++++++++++++++++-------------
 src/engine/safeReader.cpp |  8 ++++----
 2 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/src/engine/fileOpsIns.cpp b/src/engine/fileOpsIns.cpp
index 4f08fea62..bf83a2dcf 100644
--- a/src/engine/fileOpsIns.cpp
+++ b/src/engine/fileOpsIns.cpp
@@ -1925,37 +1925,51 @@ std::vector<DivInstrument*> DivEngine::instrumentFromFile(const char* path) {
     }
 
     switch (format) {
-      case DIV_INSFORMAT_DMP: loadDMP(reader,ret,stripPath);
+      case DIV_INSFORMAT_DMP:
+        loadDMP(reader,ret,stripPath);
         break;
-      case DIV_INSFORMAT_TFI: loadTFI(reader,ret,stripPath);
+      case DIV_INSFORMAT_TFI:
+        loadTFI(reader,ret,stripPath);
         break;
-      case DIV_INSFORMAT_VGI: loadVGI(reader,ret,stripPath);
+      case DIV_INSFORMAT_VGI:
+        loadVGI(reader,ret,stripPath);
         break;
       case DIV_INSFORMAT_FTI: // TODO
         break;
       case DIV_INSFORMAT_BTI: // TODO
         break;
-      case DIV_INSFORMAT_S3I: loadS3I(reader,ret,stripPath);
+      case DIV_INSFORMAT_S3I:
+        loadS3I(reader,ret,stripPath);
         break;
-      case DIV_INSFORMAT_SBI: loadSBI(reader,ret,stripPath);
+      case DIV_INSFORMAT_SBI:
+        loadSBI(reader,ret,stripPath);
         break;
-      case DIV_INSFORMAT_OPLI: loadOPLI(reader,ret,stripPath);
+      case DIV_INSFORMAT_OPLI:
+        loadOPLI(reader,ret,stripPath);
         break;
-      case DIV_INSFORMAT_OPNI: loadOPNI(reader,ret,stripPath);
+      case DIV_INSFORMAT_OPNI:
+        loadOPNI(reader,ret,stripPath);
         break;
-      case DIV_INSFORMAT_Y12: loadY12(reader,ret,stripPath);
+      case DIV_INSFORMAT_Y12:
+        loadY12(reader,ret,stripPath);
         break;
-      case DIV_INSFORMAT_BNK: loadBNK(reader,ret,stripPath);
+      case DIV_INSFORMAT_BNK:
+        loadBNK(reader,ret,stripPath);
         break;
-      case DIV_INSFORMAT_FF: loadFF(reader,ret,stripPath);
+      case DIV_INSFORMAT_FF:
+        loadFF(reader,ret,stripPath);
         break;
-      case DIV_INSFORMAT_GYB: loadGYB(reader,ret,stripPath);
+      case DIV_INSFORMAT_GYB:
+        loadGYB(reader,ret,stripPath);
         break;
-      case DIV_INSFORMAT_OPM: loadOPM(reader,ret,stripPath);
+      case DIV_INSFORMAT_OPM:
+        loadOPM(reader,ret,stripPath);
         break;
-      case DIV_INSFORMAT_WOPL: loadWOPL(reader,ret,stripPath);
+      case DIV_INSFORMAT_WOPL:
+        loadWOPL(reader,ret,stripPath);
         break;
-      case DIV_INSFORMAT_WOPN: loadWOPN(reader,ret,stripPath);
+      case DIV_INSFORMAT_WOPN:
+        loadWOPN(reader,ret,stripPath);
         break;
     }
 
diff --git a/src/engine/safeReader.cpp b/src/engine/safeReader.cpp
index 145be333c..173ffd235 100644
--- a/src/engine/safeReader.cpp
+++ b/src/engine/safeReader.cpp
@@ -171,7 +171,7 @@ String SafeReader::readStringLine() {
   if (isEOF()) throw EndOfFileException(this, len);
 
   while (!isEOF() && (c=readC())!=0) {
-    if (c=='\r'||c=='\n') {
+    if (c=='\r' || c=='\n') {
       break;
     }
     ret.push_back(c);
@@ -185,11 +185,11 @@ String SafeReader::readStringToken(unsigned char delim, bool stripContiguous) {
   if (isEOF()) throw EndOfFileException(this, len);
 
   while (!isEOF() && (c=readC())!=0) {
-    if (c == '\r' || c == '\n') {
+    if (c=='\r' || c=='\n') {
       break;
     }
-    if (c == delim) {
-      if (ret.length() == 0 && stripContiguous) {
+    if (c==delim) {
+      if (ret.length()==0 && stripContiguous) {
         continue;
       }
       break;
