Skip to content

Commit

Permalink
Remove problematic conditional program list population.
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
kramlie committed Feb 3, 2025
1 parent 67e6133 commit 7d65ea1
Showing 1 changed file with 29 additions and 39 deletions.
68 changes: 29 additions & 39 deletions src/LV2_Plugin/YoshimiLV2Plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -571,46 +571,27 @@ LV2_State_Status YoshimiLV2Plugin::stateRestore(LV2_State_Retrieve_Function retr

LV2_Program_Descriptor const * YoshimiLV2Plugin::getProgram(uint32_t index)
{
if (runtime().enableProgChange and flatbankprgs.empty())
if (flatbankprgs.empty())
{
int rootCC = runtime().midi_bank_root;
int bankCC = runtime().midi_bank_C;
// If both are set to the same CC, just ignore root.
if (rootCC != 128 and rootCC == bankCC)
rootCC = 128;
for (auto& [rootID, root] : synth.bank.getRoots())
{
if (rootCC == 128 and rootID != runtime().currentRoot)
continue;
BankEntryMap const& banks{synth.bank.getBanks(rootID)};
for (auto& [bankID,bank] : banks)
{
if (bankCC == 128 and bankID != runtime().currentBank)
if (bankID >= 128 or bank.dirname.empty())
continue;
if (bankID < 128 and not bank.dirname.empty())

for (auto& [instrumentID,instrument] : bank.instruments)
{
for (auto& [instrumentID,instrument] : bank.instruments)
{
if (not instrument.name.empty())
{
uint32_t banknum {0};
if (rootCC == 0)
banknum |= rootID << 7;
else if (rootCC == 32)
banknum |= rootID;
if (bankCC == 0)
banknum |= bankID << 7;
else if (bankCC == 32)
banknum |= bankID;

LV2Bank entry;
entry.bank = banknum;
entry.program = instrumentID;
entry.display = bank.dirname + " -> " + instrument.name;
entry.name = entry.display.c_str();
flatbankprgs.push_back(std::move(entry));
}
}
if (instrument.name.empty())
continue;

LV2Bank entry;
entry.bank = (rootID << 7) | bankID;
entry.program = instrumentID;
entry.display = bank.dirname + " -> " + instrument.name;
entry.name = entry.display.c_str();
flatbankprgs.push_back(std::move(entry));
}
}
}
Expand All @@ -622,18 +603,27 @@ LV2_Program_Descriptor const * YoshimiLV2Plugin::getProgram(uint32_t index)

void YoshimiLV2Plugin::selectProgramNew(unsigned char channel, uint32_t bank, uint32_t program)
{
if (runtime().midi_bank_root != 128
and runtime().midi_bank_root != runtime().midi_bank_C)
auto rootnum = bank >> 7;
auto banknum = bank & 127;

if (runtime().midi_bank_root != 128)
synth.mididecode.setMidiBankOrRootDir(rootnum, true, true);
else
{
short banknum = (runtime().midi_bank_root == 32) ? (bank & 127) : (bank >> 7);
synth.mididecode.setMidiBankOrRootDir(banknum, true, true);
if (runtime().currentRoot != rootnum)
return;
}

if (runtime().midi_bank_C != 128)
{
short banknum = (runtime().midi_bank_C == 0) ? (bank >> 7) : (bank & 127);
synth.mididecode.setMidiBankOrRootDir(banknum, true, false);
else
{
if (runtime().currentBank != banknum)
return;
}
synth.mididecode.setMidiProgram(channel, program, true);

if (runtime().enableProgChange)
synth.mididecode.setMidiProgram(channel, program, true);
}


Expand Down

0 comments on commit 7d65ea1

Please sign in to comment.