Renaming issues (REV2)

[salzaverde]

Sorry if this is a pilot error - I'm having some trouble renaming REV2 patches.

Use Case:

• Load bank from REV2
• Rename patches
• Send bank to REV2

Behaviour using the "Current Patch" tab on the right hand side:

Patch can be renamed and shows correctly in the "Synth Bank" list and in the library view - however, on "send to synth" nothing happens and on "import again", too. Seems like the name is not considered as a changed patch?

Behaviour using "Bulk rename patches": Nothing happens, no changes in either the library, nor the "Synth Bank", nor the synth itself.

Further things I noted down the road, should I create bugs for those, too?

• "Add new user bank" using "fill" didn't work ("error DATABASE ERROR in putPatchList: SQL Exception FOREIGN KEY constraint failed"). "Add new List" always worked. At some point is mysteriously started to work but when creating a new database, it's broken again.
• Deleted user banks are still visible in the UI but gone after restart

Tested on Mac (M2, Sonoma 14.4.1) and Linux

[christofmuc]

Thanks for reporting! That sounds indeed like a problem (or actually more than one).

Just to check - are you on at least version 2.4.1, there was already one bug fix regarding the renaming.

What might be going on here:

• A patch with no changes other than the name is not considered a new patch, so the send bank command might decide that the patch does not need to be sent.
• Retrieving a bank where just a name is changed might filter out name changes on reimport.

I will look into it and come up with a plan for fixing, clearly this is a use case that should be supported. I think retrieving the bank would work if the send had worked, because there is the notion that if you import a patch again with a name different from the one stored in the database, it is considered to be a "better name", and should overwrite the name already in the DB.

This whole thing is a bit more complex than expected, but the main reason for all the effort is the deduplication of patches, which makes the software think it is the same patch if all but the name are equal.

The two other items are individual bugs, I will split out issues to track them separately.

[christofmuc]

Confirmed - when I rename a patch, the synth bank containing that patch does not flag the patch as "dirty" (red frame), and thus clicking on send to synth does not send the bank to the synth. This is true for all synths where patches are sent one by one, and only those which are "changed" with a red frame.

Clearest solution would be to flag those patches "dirty" which got a name change?

[christofmuc]

On the issue with the renaming: I can confirm that the patch is actually renamed by exporting the bank into a syx file, and then using a new database to import that syx file. The new name is imported.

So it seems the problem is that the new patch is not sent - @salzaverde did you test the "In synth" bank type, or the user bank? The send logic is differnent, and the In synth bank send should always send all patches, with renames. If that doesn't work either, I need to search in a different place for the bug.

[salzaverde]

Cool, thanks for looking into this already!

I tried versions 2.3, 2.4.1 and 2.4.2 with the "In Synth" bank type.

Just curious: Is there a strong use case for automatically removing duplicate patches which don't have the same name? Otherwise, less intrusive would be a filter to show patches with the same settings (and different names).

Let me know if I can help fix something (some hints to where the magic happens would be helpful).

[salzaverde]

Actually I'm not sure if that occured with the "In Synth" bank type or a user bank. Will test this tonight!

[christofmuc]

@salzaverde Knobkraft started as a Library tool to sieve through large amounts of potentially duplicated legacy sysex files, so the deduplication was the primary goal. Bank management came later, and as you can see is not fully functional yet.

Each synth adaption can choose which bits in the sysex are considered relevant and which should be igored when calculating the "fingerprint". Most choose to ignore the name so two patches of the same name are considered to be the same patch. When you reimport a patch, you overwrite the existing patch name unless the name is detected to be a "default name", i.e. something like "Basic Program A" or whatever the synth generates when no patch name is entered.

The main problem is when you rename a patch in an adaptation that considers the name as part of the fingerprint, the patch is duplicated because the primary key in the database is the fingerprint. The renamed patch is put into the database, but it doesn't overwrite the original patch because now the key is different.

We so have the opposite filter: You can search for patches that are different despite having the same name...

All in all quite a complex situation... Lists are meant to be able to contain a patch multiple times, and banks are special lists. So what is missing really is that the imports are creating proper lists. Then it is easy to create new banks from the import. Note that many imports do not create banks, there are many files out there which just contain a number of patches, or even just one patch per file.

If you want to help, probably the design and thinking right now is best - you can see there are quite some design challenges involved!

[salzaverde]

I see, thanks for the insight. Yes that's an interesting problem, I'll give it some thought!

Came around to checking the bank type in the meantime - it's definitely the "In synth" bank. Steps to reproduce:

• Select "In synth - U1"bank on the left pane
• Choose patch on the right
• Current Patch Tab -> rename
• Send to synth

[salzaverde]

It's interesting - my use cases are really the exact opposite, I explicitly need duplicate patches :)

Playing in a live band I need the patches to be named according to the song title and patches from the same song must be arranged in a sequence for me to quickly switch to the next patch using a foot pedal. I might need my goto "Rhodes" sound in multiple songs, so the same patch will exist as "Song 1 - Rhodes" and "Song 2 - Rhodes".

[salzaverde]

There is a popular saying in german: "Sometimes a pull request sais more than a thousand words" ;)

Those PRs fix the issue and make things a little cleaner, too, I hope:

MidiKraft PR Orm PR

Changes:

• UserBanks don't sync directly to the synth but to the corresponding ActiveSynthBank first
  • No need to send always all patches in case of user banks (former option "ignoreDirty"/"fullBank"), only patches are sent that have actually changed
• Synthbanks set patches "dirty" in case of name changes

The MidiKraft PR contains two commits, one for the user bank update scheme and one for considering names for dirty states.

The Orm PR contains two commits, the first fixes an unrelated crash - Sorry for not using a separate PR for this, it's my first time to create PRs from a fork like this so I was a little confused.

[Andy2No]

  * No need to send always all patches in case of user banks (former option "ignoreDirty"/"fullBank"), only patches are sent that have actually changed

Does this complicate the process of using a User bank to reorganise the patches in a bank using the Orm?

Suppose I have a bank on the synth which contains patches I'm happy with, but I want them in a different order. The intuitive thing to do would be to copy them into a new User bank in the order I want them, then write that bank to the synth. Wouldn't this complicate that process?

[salzaverde]

Hi @Andy2No,

there should be no difference for users, besides sending a user bank would be sometimes faster.

Let's say you would want to switch the order of patches "6" and "7" through creating a user bank. Currently, all patches would be sent to the device, even if only patches "6" and "7" have changed - with the suggested changes, only patches "6" and "7" would be updated.

[Andy2No]

Hi @salzaverde,

That seems like a good idea, especially as I suspect some synths keep patches in Flash, which has a limited number of write cycles per block. I wasn't aware the Orm kept a synced version of patches on the synth to compare to though. If I start it up, having previously imported a bank from a synth, does it still have that bank to do the comparison with? Does the fact that it would only have imported unique patches (e.g. not duplicates present elsewhere in the database or in that bank) create any problems?

[salzaverde]

You are right, I tested that - sending a user bank without having loaded the corresponding synth bank first, fails (It doesn't crash at least so that's good :)).

I think it would be good to always have a virtual representation of the synth's state before being able to change it. We could either load all writable synth banks when the synth is connected or just don't allow sending user banks without the corresponding synth bank being loaded.

Thoughts?

[Andy2No]

I can only think of two approaches. Either just send the user bank as is, overwriting whatever's there, or do a read-modify-write operation on the bank in the synth - before writing the user bank, read the bank from the synth as the first step of the process. I think it would also be necessary to suppress the skipping of duplicates, or things would just get messy, leading to mistakes. If a patch is on the synth twice, whether or not both patches have the same name, keep both of them for the comparison, if only because that makes it easier to keep the patches in the correct place in the bank, and removes some of the ways of getting it horribly wrong.

On the whole, I think it would be less confusing if the Orm didn't skip duplicates at all, when reading from a synth. Just marking them as duplicates in some way would be better. When I tried importing patches from my Zoom MS-70CDR I ended up with less than a full bank and thought it had failed to read the one I specifically named myself, shortly before. It turns out it was a duplicate of another one, other than the patch name - which is useful to know, if it's pointed out, but it just made it seem broken. Since I did want to back up that patch, with that name, I now have to try to figure out what it's a duplicate of, so I can delete or edit the other one. It's just extra complication, to me. I'd have been fine with just getting both patches and not knowing they were the same too. If someone wants to reorder patches in a bank, they want to start with a full copy of the bank, in the same order. I appreciate that it goes against the original design philosophy, but I personally think skipping patches goes too far. You then end up with a bank with patches in different slots or locations, because some of them were missed out. You might also end up with a bank where one patch you particularly wanted, and remember by name, didn't get copied into the database.

[christofmuc]

I agree the deduplication is counter intuitive with the bank management.

You can sort of disable deduplication if the fingerprint implementation uses all bytes of a patch, including program position and name. But importing the bank a second time would still show an empty list. Why am I so focused on deduplication? Because when sorting patches, you want to assign patch type and mark favorites. If you have the same patches under a different name or program position, you would be wasting a lot of time.

I think the correct way is to:

• Fix the problem that renamed patches are not sent to the synth (which was the original issue here)
• Always import full banks and files, to stop confusing users from getting less patches in an import than were there (I have been confused by this already as well)
• Allow patches to appear multiple times in a list (this is actually possible) and under different names (which is not possible yet).
• Automatically create banks for imports where applicable (not easy)

The original database structure patches becomes the repository for the data structure of the patch, i.e. the parameters. They might or might not contain program positions and names.

The "PatchInList" structure in addition to the new program position also gets a name allowing to record the name of the patch in this list - and this name is unique to the list, and is written into the patch data only before it is sent to the synth. This should allow for @salzaverde's completely valid use case to add the song number into the patch name.

Thanks @salzaverde for the pull requests, i will look at them at next opportunity! And then I need to make a plan on the changes required for this!

@Andy2No Thanks as always for your input, appreciated!

[markusschloesser]

To chime in as well. Personally I don't care about banks and names, but I do care a lot about deduplication. I use knobkraft as the DB for all my sounds and do not rely anymore at all on synth internal preset management (because mostly Imho they are crap). But I do understand that there are different needs out there 😇

[salzaverde]

Agreed and we should absolutely support all our use cases. The challenge is to find an architecture which can do that in a clean way. I'll continue thinking about this, too, and might draw up some UMLish use case and component diagrams for discussion. Might be better suited in "discussions" then?

The bug at hand would be fixed with this commit - @christofmuc we might drop the rest of those PR's for now until we have a more solid idea of how the architecture could adapt.

[christofmuc]

Let's continue the discussion in #362, as the original problem has been solved (plus 3 or 4 other issues which came up, and got their own issue numbers).

I'll close this issue even if there is more good info in here, we can still refer to it.