Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch numbers in the Library don't correspond to the ones in the patch (for non consecutive patch numbers in dump - e.g. Behringer Pro 800) #259

Open
Andy2No opened this issue Jul 28, 2023 · 7 comments
Labels
bug Something isn't working Ready for release Will be part of the next release

Comments

@Andy2No
Copy link

Andy2No commented Jul 28, 2023

I started trying to make a partial adaptation for the Behringer Pro 800, initially intended to allow patches to be named, which can't be done from the synth. Most of the sysex details have been kept secret, so far, so it can't do everything that an adaptation should. Patches can only be exported from the synth by asking for a full dump, from the front panel Global Settings menu.

I found that the patch numbers shown in the Library tab don't correspond to the ones in the patch, and can't because friendlyProgramName(program) is only ever passed an arbitrary number during the import from file process, which is just a count of where the import is up to, not actually a real patch number. Blank / init patches are not included in the dump, so the count is wrong as soon as there's a gap.

What I would need, to make this work, is a way of being passed the whole patch (message), not just the potentially wrong number, so the python code can extract the actual patch number. Perhaps the best way to approach this would be to allow an optional function to do that, which is called in preference to friendlyProgramName() if defined, e.g.

def friendlyProgramNameFromPatch(message)

I started this as a discussion, and there are more details there - #258

@Andy2No Andy2No changed the title Patch numbers in the Library don't correspond to the ones in the patch (Behringer Pro 800) Patch numbers in the Library don't correspond to the ones in the patch (for non consecutive patch numbers in dump - e.g. Behringer Pro 800) Jul 28, 2023
@christofmuc
Copy link
Owner

I have to check - normally, when you have implemented numberFromDump() it should take precedence over the running number during an import. Can you post your adaptation and maybe a dump file so I can give it a try?

@christofmuc
Copy link
Owner

I think you're on to something bigger here :-)

Some intermediate results:

  • getProgramNumber() of the ProgramDumpCapability is currently unused
  • The Patch class has a patchNumber() call, but these are only used for the C++ Synth implementations, not for the adaptations
  • The PatchHolder class also has a patchNumber() method, but this is only a getter of the number put in on construction.

What different type of numbers do we have?

  • The position of a patch in a bank (new with KnobKraft 2.x)
  • The number of a patch during the import.
    1. For these, we have the case that the Synth has numberFromDump(), and then we should always use that
    2. When the synth has no numberFromDump(), some kind of counting should be done during import.

We have different import methods, which count differently as well.

To fix this mess, we need to find all places where the PatchHolder class is created (this is the class that determines what is stored in the database).

@christofmuc christofmuc added the bug Something isn't working label Aug 3, 2023
@Andy2No
Copy link
Author

Andy2No commented Aug 3, 2023

Thanks. I only just read that, as I was about to add the dump and adaptation you asked for... Maybe I should put those in a discussion thread instead, since it's a universal behaviour, not specific to what I've done - just to any situation where there isn't a full set of consecutively numbered patches from the start up to the final one in a dump.

I think this is also probably going to apply to the Waldorf Pulse II, if I ever get any further with that one. At best, I expect patches missing where the init ones are. On mine that's a block of 80 or 90, towards the end of the range of patch numbers, with some non-blank ones at the high end.

I'll start a new discussion thread for the Pro-800 adaptation (such as it is) soon - I'm a bit tired now.

@christofmuc
Copy link
Owner

Wasp's nest. But I finally need to clean this up:

  1. The old C++ adaptations (well, before there were adaptations actually) did split the patch data that is location independent from the patch location, So e.g. for the Virus in the database there are only the parameter values, plus a position determined at import time. The Orm generates edit buffers or program dumps from these parameter values as it needs them, which is now only when a patch is stored in a bank at a certain position.
  2. The Python adaptation are "simpler" and rather store the data from the synth as they received it, as one or more MIDI messages. This naturally includes a program position. This can lead to ambiguity when putting a patch in a Synth bank, because now there are two numbers.
  3. To add to the complexity, we also have a running number used during the import. This was mainly built for synths that have no stored patch number, but as you discovered this currently takes precedence, which is a bug.

The fun really starts when looking at the names as well:

  1. When the synth has no name storage, it calculates the name via friendlyProgramName. This is based on the patch number. But when I move e.g. Patch 371 from the Matrix 1000 into a new bank 2 at position 11, it becomes Patch 211.
  2. The number is also used to initialize a patch name for those synths, but the name can be changed. So you could change the name of the patch 211 back to 371 (as text).
  3. When the synth has name storage as well, the program number is not used for the name. Still you can switch the UI to display patch numbers. Patch numbers are generally extracted via numberFromDump() which is part of the ProgramDumpCapability

What is the expected behavior?

  1. The stored program number is shown everywhere, if it is available. In case a patch is placed in a bank, so new number should be shown.
  2. A patch can actually have multiple numbers, but those are related to its position in a list. But not all lists have positions:
    a) Imports. Currently imports are implemented differently from user lists, and the original idea of retaining all imports in which a patch had been encountered is not fully implemented. Once imports are migrated into lists, we could easily add a patch into all imports in which it had been discovered. But that is a different feature.
    b) User lists. User lists cannot be sent to the synth, and they are made for collecting patches from muliple synths (e.g. to use in a song). So a position in a list needs to be stored, but it is not a storage place.
    c) Finally, synth and user banks. Here the position in the list is the same as the program place. The complexity here is that not all program numbers also contain the bank, some synths only store a position inside a bank, but the bank is unknown by the patch.

Uff.

@Andy2No
Copy link
Author

Andy2No commented Aug 6, 2023

I'd suggest let the adaptation decide how to display it, whenever that's an option. If it's in a list which doesn't display a number, then there's no need to ask the adaptation what to do, but otherwise I think it's best to pass the full "message" - as much of the database entry as an adaptation function can be shown, plus a context flag, to determine different cases.

For example, if friendlyProgramName() was passed the same "message" as numberFromDump(message), I would be able to show the actual bank and program slot number as appropriate for the synth - or any other. If it needs to display different things in different circumstances, that's where the context flag comes in - e.g. an enumerated type (or Python equivalent) or a short string constant or single character.

If the adaptation doesn't care about the context, then maybe it should be allowed to always show the same thing, if it wants to - if "message" is a program dump or an edit buffer dump then the adaptation will know what it wants to show, with the option of changing that behaviour based on the context.

Since existing adaptations already use friendlyProgramName(program), keep that as it is, but keep it optional and add another optional function with more flexibility, e.g.

def flexibleProgramName(program, message, context)

  • which then has the option of using program (the position number, presumably), and/or the full "message" (e.g. a program dump), and optionally returning different results depending on the value of "context".

As for some synths having a program number within a bank, but no bank name / number, I think it's better for the adaptation to be able to decide what to do for that particular synth - e.g. just show the program number, without a bank.

So, to summarise, give the adaptation programmer the ability to do what he or she wants, for that particular adaptation, where appropriate. If there are circumstances where the Orm needs to override that, then fair enough, do that instead.

@christofmuc christofmuc reopened this Aug 6, 2023
@christofmuc christofmuc added the Ready for release Will be part of the next release label Aug 6, 2023
@christofmuc
Copy link
Owner

@Andy2No Good idea with the new program function taking the message. This is a bigger change - I tried to fix the original problem that for adaptations numberFromDump() was never called in the 2.0.6 release. Please give it a spin and see if it makes a difference for you!

@Andy2No
Copy link
Author

Andy2No commented Aug 7, 2023

Fantastic! It worked first time, without any modification to my adaptation code :)

The ones that have blank names here are supposed to be like that - that's how an edited patch looks when you import the dump. All the patches are correctly numbered. A0-99 are the stock ones, but can all be overwritten. I might add a leading zero to the single digit ones, to match the display on the synth, otherwise it does all it needs to do, for now, for that synth. Renaming still works too.

Knobkraft Orm 2 0 6 test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Ready for release Will be part of the next release
Projects
None yet
Development

No branches or pull requests

2 participants