Skip to content

Commit 4a268ce

Browse files
committed
Remove problematic conditional program list population.
Turns out that the conditional program list population introduced in 7ce151b is problematic. The issue was discovered while working on making program list changes dynamic (inform the host of list changes [1]). Take for example this scenario: 1. In the `~/.config/yoshimi/yoshimi-LV2.instance` file, `ignore_program_changes` is true. 2. In the LV2 state data, the same option is set to false (so Program Changes are enabled). 3. Because programs are queried before the state is loaded, this results in an empty list initially, because of point 1. 4. Then the state is loaded, which causes the list to be populated again. The problem is that some hosts have trouble with the window between step 3 and 4, and will often forget which program was set while the list is empty. Then they will set it to a default one once the list is populated again, instead of the one which they had saved earlier. This was only tested on Carla, but it's reasonable to assume that other hosts may have similar problems. Therefore, always populate the list with all entries, so that it's stable. However, we will only respond to program changes that can be made using the current combination of root, bank and program change options. The root and bank LSB/MSB settings are not significant, but they do need to be on to have any effect. Programs that do not match this "options filter" are ignored. [1] Implementing dynamic program list changes was abandoned because host support seems quite poor (at least in Carla), and in fact introduced more problems than it seemingly solved: * Carla deadlocks when programs are changed (see falkTX/Carla#1968) * Its program selection is nonsensical when programs change (see releadPrograms in Carla's CarlaPluginLV2.cpp). * Carla doesn't even save the program you have selected, so it's kind of useless other than to just select one as a one-off. Signed-off-by: Kristian Amlie <[email protected]>
1 parent 18b010b commit 4a268ce

File tree

1 file changed

+29
-39
lines changed

1 file changed

+29
-39
lines changed

src/LV2_Plugin/YoshimiLV2Plugin.cpp

Lines changed: 29 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -571,46 +571,27 @@ LV2_State_Status YoshimiLV2Plugin::stateRestore(LV2_State_Retrieve_Function retr
571571

572572
LV2_Program_Descriptor const * YoshimiLV2Plugin::getProgram(uint32_t index)
573573
{
574-
if (runtime().enableProgChange and flatbankprgs.empty())
574+
if (flatbankprgs.empty())
575575
{
576-
int rootCC = runtime().midi_bank_root;
577-
int bankCC = runtime().midi_bank_C;
578-
// If both are set to the same CC, just ignore root.
579-
if (rootCC != 128 and rootCC == bankCC)
580-
rootCC = 128;
581576
for (auto& [rootID, root] : synth.bank.getRoots())
582577
{
583-
if (rootCC == 128 and rootID != runtime().currentRoot)
584-
continue;
585578
BankEntryMap const& banks{synth.bank.getBanks(rootID)};
586579
for (auto& [bankID,bank] : banks)
587580
{
588-
if (bankCC == 128 and bankID != runtime().currentBank)
581+
if (bankID >= 128 or bank.dirname.empty())
589582
continue;
590-
if (bankID < 128 and not bank.dirname.empty())
583+
584+
for (auto& [instrumentID,instrument] : bank.instruments)
591585
{
592-
for (auto& [instrumentID,instrument] : bank.instruments)
593-
{
594-
if (not instrument.name.empty())
595-
{
596-
uint32_t banknum {0};
597-
if (rootCC == 0)
598-
banknum |= rootID << 7;
599-
else if (rootCC == 32)
600-
banknum |= rootID;
601-
if (bankCC == 0)
602-
banknum |= bankID << 7;
603-
else if (bankCC == 32)
604-
banknum |= bankID;
605-
606-
LV2Bank entry;
607-
entry.bank = banknum;
608-
entry.program = instrumentID;
609-
entry.display = bank.dirname + " -> " + instrument.name;
610-
entry.name = entry.display.c_str();
611-
flatbankprgs.push_back(std::move(entry));
612-
}
613-
}
586+
if (instrument.name.empty())
587+
continue;
588+
589+
LV2Bank entry;
590+
entry.bank = (rootID << 7) | bankID;
591+
entry.program = instrumentID;
592+
entry.display = bank.dirname + " -> " + instrument.name;
593+
entry.name = entry.display.c_str();
594+
flatbankprgs.push_back(std::move(entry));
614595
}
615596
}
616597
}
@@ -622,18 +603,27 @@ LV2_Program_Descriptor const * YoshimiLV2Plugin::getProgram(uint32_t index)
622603

623604
void YoshimiLV2Plugin::selectProgramNew(unsigned char channel, uint32_t bank, uint32_t program)
624605
{
625-
if (runtime().midi_bank_root != 128
626-
and runtime().midi_bank_root != runtime().midi_bank_C)
606+
auto rootnum = bank >> 7;
607+
auto banknum = bank & 127;
608+
609+
if (runtime().midi_bank_root != 128)
610+
synth.mididecode.setMidiBankOrRootDir(rootnum, true, true);
611+
else
627612
{
628-
short banknum = (runtime().midi_bank_root == 32) ? (bank & 127) : (bank >> 7);
629-
synth.mididecode.setMidiBankOrRootDir(banknum, true, true);
613+
if (runtime().currentRoot != rootnum)
614+
return;
630615
}
616+
631617
if (runtime().midi_bank_C != 128)
632-
{
633-
short banknum = (runtime().midi_bank_C == 0) ? (bank >> 7) : (bank & 127);
634618
synth.mididecode.setMidiBankOrRootDir(banknum, true, false);
619+
else
620+
{
621+
if (runtime().currentBank != banknum)
622+
return;
635623
}
636-
synth.mididecode.setMidiProgram(channel, program, true);
624+
625+
if (runtime().enableProgChange)
626+
synth.mididecode.setMidiProgram(channel, program, true);
637627
}
638628

639629

0 commit comments

Comments
 (0)