From 73a80a93cb3b560249469df01bb8b3a67365bcf3 Mon Sep 17 00:00:00 2001 From: Kevin <68612569+diyelectromusic@users.noreply.github.com> Date: Sat, 1 Apr 2023 16:04:47 +0100 Subject: [PATCH] Fix some of the issues with bank handling (#461) * Fix for Issue #457 - changed m_nNumLoadedBanks to be m_nNumHighestBank and updated functionality in menus accordingly. * Fix for Issue #460 implements MIDI Bank Select MSB/LSB based on PR #395 submitted by @abscisys * Fix for Issue #458 to not show blank banks in the menus. Also handles MIDI bank select messages and won't change banks if the final selected bank isn't loaded. * Firm up bank validity handling slightly. --- src/mididevice.cpp | 6 +++- src/minidexed.cpp | 46 +++++++++++++++++++++--- src/minidexed.h | 5 +++ src/sysexfileloader.cpp | 78 +++++++++++++++++++++++++++++++++++++---- src/sysexfileloader.h | 13 ++++--- src/uimenu.cpp | 17 +++------ 6 files changed, 137 insertions(+), 28 deletions(-) diff --git a/src/mididevice.cpp b/src/mididevice.cpp index 4e51056..64fc009 100644 --- a/src/mididevice.cpp +++ b/src/mididevice.cpp @@ -36,7 +36,7 @@ LOGMODULE ("mididevice"); #define MIDI_AFTERTOUCH 0b1010 // TODO #define MIDI_CHANNEL_AFTERTOUCH 0b1101 // right now Synth_Dexed just manage Channel Aftertouch not Polyphonic AT -> 0b1010 #define MIDI_CONTROL_CHANGE 0b1011 - #define MIDI_CC_BANK_SELECT_MSB 0 // TODO + #define MIDI_CC_BANK_SELECT_MSB 0 #define MIDI_CC_MODULATION 1 #define MIDI_CC_BREATH_CONTROLLER 2 #define MIDI_CC_FOOT_PEDAL 4 @@ -283,6 +283,10 @@ void CMIDIDevice::MIDIMessageHandler (const u8 *pMessage, size_t nLength, unsign m_pSynthesizer->SetPan (pMessage[2], nTG); break; + case MIDI_CC_BANK_SELECT_MSB: + m_pSynthesizer->BankSelectMSB (pMessage[2], nTG); + break; + case MIDI_CC_BANK_SELECT_LSB: m_pSynthesizer->BankSelectLSB (pMessage[2], nTG); break; diff --git a/src/minidexed.cpp b/src/minidexed.cpp index 6a3897e..66c6151 100644 --- a/src/minidexed.cpp +++ b/src/minidexed.cpp @@ -61,6 +61,7 @@ CMiniDexed::CMiniDexed (CConfig *pConfig, CInterruptSystem *pInterrupt, for (unsigned i = 0; i < CConfig::ToneGenerators; i++) { m_nVoiceBankID[i] = 0; + m_nVoiceBankIDMSB[i] = 0; m_nProgram[i] = 0; m_nVolume[i] = 100; m_nPan[i] = 64; @@ -365,14 +366,47 @@ CSysExFileLoader *CMiniDexed::GetSysExFileLoader (void) return &m_SysExFileLoader; } +void CMiniDexed::BankSelect (unsigned nBank, unsigned nTG) +{ + nBank=constrain((int)nBank,0,16383); + + assert (nTG < CConfig::ToneGenerators); + + if (GetSysExFileLoader ()->IsValidBank(nBank)) + { + // Only change if we have the bank loaded + m_nVoiceBankID[nTG] = nBank; + + m_UI.ParameterChanged (); + } +} + +void CMiniDexed::BankSelectMSB (unsigned nBankMSB, unsigned nTG) +{ + nBankMSB=constrain((int)nBankMSB,0,127); + + assert (nTG < CConfig::ToneGenerators); + // MIDI Spec 1.0 "BANK SELECT" states: + // "The transmitter must transmit the MSB and LSB as a pair, + // and the Program Change must be sent immediately after + // the Bank Select pair." + // + // So it isn't possible to validate the selected bank ID until + // we receive both MSB and LSB so just store the MSB for now. + m_nVoiceBankIDMSB[nTG] = nBankMSB; +} + void CMiniDexed::BankSelectLSB (unsigned nBankLSB, unsigned nTG) { nBankLSB=constrain((int)nBankLSB,0,127); assert (nTG < CConfig::ToneGenerators); - m_nVoiceBankID[nTG] = nBankLSB; + unsigned nBank = m_nVoiceBankID[nTG]; + unsigned nBankMSB = m_nVoiceBankIDMSB[nTG]; + nBank = (nBankMSB << 7) + nBankLSB; - m_UI.ParameterChanged (); + // Now should have both MSB and LSB so enable the BankSelect + BankSelect(nBank, nTG); } void CMiniDexed::ProgramChange (unsigned nProgram, unsigned nTG) @@ -717,7 +751,9 @@ void CMiniDexed::SetTGParameter (TTGParameter Parameter, int nValue, unsigned nT switch (Parameter) { - case TGParameterVoiceBank: BankSelectLSB (nValue, nTG); break; + case TGParameterVoiceBank: BankSelect (nValue, nTG); break; + case TGParameterVoiceBankMSB: BankSelectMSB (nValue, nTG); break; + case TGParameterVoiceBankLSB: BankSelectLSB (nValue, nTG); break; case TGParameterProgram: ProgramChange (nValue, nTG); break; case TGParameterVolume: SetVolume (nValue, nTG); break; case TGParameterPan: SetPan (nValue, nTG); break; @@ -771,6 +807,8 @@ int CMiniDexed::GetTGParameter (TTGParameter Parameter, unsigned nTG) switch (Parameter) { case TGParameterVoiceBank: return m_nVoiceBankID[nTG]; + case TGParameterVoiceBankMSB: return m_nVoiceBankID[nTG] >> 7; + case TGParameterVoiceBankLSB: return m_nVoiceBankID[nTG] & 0x7F; case TGParameterProgram: return m_nProgram[nTG]; case TGParameterVolume: return m_nVolume[nTG]; case TGParameterPan: return m_nPan[nTG]; @@ -1445,7 +1483,7 @@ void CMiniDexed::LoadPerformanceParameters(void) for (unsigned nTG = 0; nTG < CConfig::ToneGenerators; nTG++) { - BankSelectLSB (m_PerformanceConfig.GetBankNumber (nTG), nTG); + BankSelect (m_PerformanceConfig.GetBankNumber (nTG), nTG); ProgramChange (m_PerformanceConfig.GetVoiceNumber (nTG), nTG); SetMIDIChannel (m_PerformanceConfig.GetMIDIChannel (nTG), nTG); SetVolume (m_PerformanceConfig.GetVolume (nTG), nTG); diff --git a/src/minidexed.h b/src/minidexed.h index a74a04f..8e8deaf 100644 --- a/src/minidexed.h +++ b/src/minidexed.h @@ -63,6 +63,8 @@ public: CSysExFileLoader *GetSysExFileLoader (void); + void BankSelect (unsigned nBank, unsigned nTG); + void BankSelectMSB (unsigned nBankMSB, unsigned nTG); void BankSelectLSB (unsigned nBankLSB, unsigned nTG); void ProgramChange (unsigned nProgram, unsigned nTG); void SetVolume (unsigned nVolume, unsigned nTG); @@ -149,6 +151,8 @@ public: enum TTGParameter { TGParameterVoiceBank, + TGParameterVoiceBankMSB, + TGParameterVoiceBankLSB, TGParameterProgram, TGParameterVolume, TGParameterPan, @@ -227,6 +231,7 @@ private: CDexedAdapter *m_pTG[CConfig::ToneGenerators]; unsigned m_nVoiceBankID[CConfig::ToneGenerators]; + unsigned m_nVoiceBankIDMSB[CConfig::ToneGenerators]; unsigned m_nProgram[CConfig::ToneGenerators]; unsigned m_nVolume[CConfig::ToneGenerators]; unsigned m_nPan[CConfig::ToneGenerators]; diff --git a/src/sysexfileloader.cpp b/src/sysexfileloader.cpp index ffba8f5..d5159eb 100644 --- a/src/sysexfileloader.cpp +++ b/src/sysexfileloader.cpp @@ -65,7 +65,7 @@ CSysExFileLoader::CSysExFileLoader (const char *pDirName) : m_DirName (pDirName) { m_DirName += "/voice"; - m_nNumLoadedBanks = 0; + m_nNumHighestBank = 0; for (unsigned i = 0; i <= MaxVoiceBankID; i++) { @@ -83,7 +83,7 @@ CSysExFileLoader::~CSysExFileLoader (void) void CSysExFileLoader::Load (void) { - m_nNumLoadedBanks = 0; + m_nNumHighestBank = 0; DIR *pDirectory = opendir (m_DirName.c_str ()); if (!pDirectory) @@ -141,7 +141,11 @@ void CSysExFileLoader::Load (void) LOGDBG ("Bank #%u successfully loaded", nBank); m_BankFileName[nBank] = pEntry->d_name; - m_nNumLoadedBanks++; + if (nBank > m_nNumHighestBank) + { + // This is the bank ID of the highest loaded bank + m_nNumHighestBank = nBank; + } } else { @@ -188,9 +192,71 @@ std::string CSysExFileLoader::GetBankName (unsigned nBankID) return "NO NAME"; } -unsigned CSysExFileLoader::GetNumLoadedBanks (void) +unsigned CSysExFileLoader::GetNextBankUp (unsigned nBankID) { - return m_nNumLoadedBanks; + // Find the next loaded bank "up" from the provided bank ID + for (unsigned id=nBankID+1; id <= m_nNumHighestBank; id++) + { + if (IsValidBank(id)) + { + return id; + } + } + + // Handle wrap-around + for (unsigned id=0; id= 0; id--) + { + if (IsValidBank((unsigned)id)) + { + return id; + } + } + + // Handle wrap-around + for (unsigned id=m_nNumHighestBank; id>nBankID; id--) + { + if (IsValidBank(id)) + { + return id; + } + } + + // If we get here there are no other banks! + return nBankID; +} + +bool CSysExFileLoader::IsValidBank (unsigned nBankID) +{ + if (m_pVoiceBank[nBankID]) + { + // Use a valid "status start/end" as an indicator of a valid loaded bank + if ((m_pVoiceBank[nBankID]->StatusStart == 0xF0) && + (m_pVoiceBank[nBankID]->StatusEnd == 0xF7)) + { + return true; + } + } + return false; +} + +unsigned CSysExFileLoader::GetNumHighestBank (void) +{ + return m_nNumHighestBank; } void CSysExFileLoader::GetVoice (unsigned nBankID, unsigned nVoiceID, uint8_t *pVoiceData) @@ -198,7 +264,7 @@ void CSysExFileLoader::GetVoice (unsigned nBankID, unsigned nVoiceID, uint8_t *p if ( nBankID <= MaxVoiceBankID && nVoiceID <= VoicesPerBank) { - if (m_pVoiceBank[nBankID]) + if (IsValidBank(nBankID)) { DecodePackedVoice (m_pVoiceBank[nBankID]->Voice[nVoiceID], pVoiceData); diff --git a/src/sysexfileloader.h b/src/sysexfileloader.h index 5954bfe..3c20d0f 100644 --- a/src/sysexfileloader.h +++ b/src/sysexfileloader.h @@ -29,7 +29,7 @@ class CSysExFileLoader // Loader for DX7 .syx files { public: - static const unsigned MaxVoiceBankID = 16383; + static const unsigned MaxVoiceBankID = 16383; // i.e. 14-bit MSB/LSB value between 0 and 16383 static const unsigned VoicesPerBank = 32; static const size_t SizePackedVoice = 128; static const size_t SizeSingleVoice = 156; @@ -56,10 +56,13 @@ public: void Load (void); - std::string GetBankName (unsigned nBankID); // 0 .. 127 - unsigned GetNumLoadedBanks (); // 0 .. MaxVoiceBankID + std::string GetBankName (unsigned nBankID); // 0 .. MaxVoiceBankID + unsigned GetNumHighestBank (); // 0 .. MaxVoiceBankID + bool IsValidBank (unsigned nBankID); + unsigned GetNextBankUp (unsigned nBankID); + unsigned GetNextBankDown (unsigned nBankID); - void GetVoice (unsigned nBankID, // 0 .. 127 + void GetVoice (unsigned nBankID, // 0 .. MaxVoiceBankID unsigned nVoiceID, // 0 .. 31 uint8_t *pVoiceData); // returns unpacked format (156 bytes) @@ -69,7 +72,7 @@ private: private: std::string m_DirName; - unsigned m_nNumLoadedBanks; + unsigned m_nNumHighestBank; TVoiceBank *m_pVoiceBank[MaxVoiceBankID+1]; std::string m_BankFileName[MaxVoiceBankID+1]; diff --git a/src/uimenu.cpp b/src/uimenu.cpp index a1d44ce..bcea3e0 100644 --- a/src/uimenu.cpp +++ b/src/uimenu.cpp @@ -486,7 +486,6 @@ void CUIMenu::EditGlobalParameter (CUIMenu *pUIMenu, TMenuEvent Event) void CUIMenu::EditVoiceBankNumber (CUIMenu *pUIMenu, TMenuEvent Event) { unsigned nTG = pUIMenu->m_nMenuStackParameter[pUIMenu->m_nCurrentMenuDepth-1]; - int nLoadedBanks = pUIMenu->m_pMiniDexed->GetSysExFileLoader ()->GetNumLoadedBanks(); int nValue = pUIMenu->m_pMiniDexed->GetTGParameter (CMiniDexed::TGParameterVoiceBank, nTG); @@ -496,19 +495,13 @@ void CUIMenu::EditVoiceBankNumber (CUIMenu *pUIMenu, TMenuEvent Event) break; case MenuEventStepDown: - if (--nValue < 0) - { - nValue = 0; - } + nValue = pUIMenu->m_pMiniDexed->GetSysExFileLoader ()->GetNextBankDown(nValue); pUIMenu->m_pMiniDexed->SetTGParameter ( CMiniDexed::TGParameterVoiceBank, nValue, nTG); break; case MenuEventStepUp: - if (++nValue > (int) nLoadedBanks-1) - { - nValue = nLoadedBanks-1; - } + nValue = pUIMenu->m_pMiniDexed->GetSysExFileLoader ()->GetNextBankUp(nValue); pUIMenu->m_pMiniDexed->SetTGParameter ( CMiniDexed::TGParameterVoiceBank, nValue, nTG); break; @@ -537,7 +530,7 @@ void CUIMenu::EditVoiceBankNumber (CUIMenu *pUIMenu, TMenuEvent Event) void CUIMenu::EditProgramNumber (CUIMenu *pUIMenu, TMenuEvent Event) { unsigned nTG = pUIMenu->m_nMenuStackParameter[pUIMenu->m_nCurrentMenuDepth-1]; - int nLoadedBanks = pUIMenu->m_pMiniDexed->GetSysExFileLoader ()->GetNumLoadedBanks(); + int nHighestBank = pUIMenu->m_pMiniDexed->GetSysExFileLoader ()->GetNumHighestBank(); int nValue = pUIMenu->m_pMiniDexed->GetTGParameter (CMiniDexed::TGParameterProgram, nTG); @@ -555,7 +548,7 @@ void CUIMenu::EditProgramNumber (CUIMenu *pUIMenu, TMenuEvent Event) if (--nVB < 0) { // Wrap around to last loaded bank - nVB = nLoadedBanks-1; + nVB = nHighestBank; } pUIMenu->m_pMiniDexed->SetTGParameter (CMiniDexed::TGParameterVoiceBank, nVB, nTG); } @@ -568,7 +561,7 @@ void CUIMenu::EditProgramNumber (CUIMenu *pUIMenu, TMenuEvent Event) // Switch up a voice bank and reset to voice 0 nValue = 0; int nVB = pUIMenu->m_pMiniDexed->GetTGParameter(CMiniDexed::TGParameterVoiceBank, nTG); - if (++nVB > (int) nLoadedBanks-1) + if (++nVB > (int) nHighestBank) { // Wrap around to first bank nVB = 0;