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

A Host of things are not stored in patch master issue for 1.6.2 #915

Closed
baconpaul opened this issue Jun 16, 2019 · 17 comments · Fixed by #1032
Closed

A Host of things are not stored in patch master issue for 1.6.2 #915

baconpaul opened this issue Jun 16, 2019 · 17 comments · Fixed by #1032
Milestone

Comments

@baconpaul
Copy link
Collaborator

baconpaul commented Jun 16, 2019

Some things should be stored in patch; some things should be stored in DAW state but not patch. This issue is a master issue for those as we change patch format

Stored in .fxp

Stored in daw-only patch

@baconpaul baconpaul added this to the 1.6.2 milestone Jun 16, 2019
@tank-trax
Copy link
Collaborator

recommendation/reminder: to have the alternate tuning files stored in a standard location, that opens by default when selecting

@baconpaul
Copy link
Collaborator Author

On mac my file picker opens in the last directory I opened pretty consistently. I am assuming the linux file picker doesn't do that?

To do that we need to implement the "suggested start path" API of the file open dialog too, which I haven't done.

@tank-trax
Copy link
Collaborator

with Zenity it opens up most recent files but oddly it's an old history and does not appear to add the latest folder as an option

@tank-trax
Copy link
Collaborator

tank-trax commented Jun 19, 2019

after further testing Zenity is a good option, it allows for Bookmarked folders and is certainly workable for a decent workflow, like many things, practice, trial and error... provides solutions

getting the tunings stored in a FXP file would be the next feature I am most interested in

@baconpaul
Copy link
Collaborator Author

Yup - that's on my list for post vacation. I will do all the streaming changes at once.

@baconpaul
Copy link
Collaborator Author

So here's my plan of attack
For the in-patch stuff it is obvious. Bump the version. etc.

But for the daw-only things like zoom we need a daw=true to cycle through save_patch which puts the extra stuff from the synth onto the patch appropriately; then use that in save_raw; and read it defensively in load_patch.

@mortfell
Copy link

mortfell commented Aug 7, 2019

Am adding it here because it seems on topic.

I have noticed the position of the 8 macro controls are saved with the DAW, but not with the VST preset. Is this something that could be saved with presets? In my experience saving macro positions with the preset is the usual behavior for plugins that support macro controls, but maybe there are reasons that Surge does not do this?

When I design sounds, I like to define macros as I work, and use them in sculpting the sounds. This method doesn't work if the macro value is not stored in the preset because the preset will not sound as intended when loaded.

Maybe this is something that could be considered with regards to changing patch format? Adding this functionality to presets wouldn't really effect anyone who doesn't use it, and since old presets don't seem to save macro positions saved, this presumably wouldn't break compatibility with any old patches.

@baconpaul
Copy link
Collaborator Author

Yeah this is the right spot. I’ll take a look when I do this.

Out of curiosity - does your daw send the midi cc messages for the controls giving the value or is the value saved through params?

@mortfell
Copy link

mortfell commented Aug 8, 2019

It seems to be saved through the "control 1-8" params themselves.....
I've tried with REAPER, Ableton Live 10 and Renoise, and any time I edit the controls with the song they come up right how I left them.

Just messing around here, and I've noticed some notable behavior, maybe a bug? Or maybe a design decision.

Surge actually DOES save the state of the macro controls "Controls 1-8" to "FXP" files, but it always zeroes out Controls 1-8 when loading from it's internal preset browser. If you use a DAWs own preset browser it seems to load the control settings as they were saved. I've tried this in all 3 of those daws and it always loads the controls correctly when using the DAW preset loading.

I can't find any presets from the factory library that don't just have macros all set to initialized values, but here is a simple preset with Control 1 mapped to a few parameters. When loading the FXP from the Surge browser, it's set to 0. When loading FXP from a DAW; control 1 is set at around 60 percent of the way up.

BS angry bass EMU.zip

@baconpaul
Copy link
Collaborator Author

Great. That's super useful. Oh and can you let me know - which OS and Plugin Flavor (AU, VSt2, VST3) do you use? They all take a similar code path of course. I agree this sounds like a bug. Save/switch/load should be a no-op.

@baconpaul
Copy link
Collaborator Author

Huh turns out this is a conscious choice in the code.

Look at this line

if (!is_preset && (p->QueryDoubleAttribute("v", &d) == TIXML_SUCCESS))

which is in the surge patch upstream from XML. The "is_preset" bool is set to true when loading an .fxp and false when streaming content from the daw.

There is no comment as to why that's what you want. And I can't think why it is. But this is a separate issue from adding to the patch format. (Although I'm fine to keep it here since it's all the same code once I open it up anyway).

I can't think why you wouldn't want to persist the value you saved at - you can change it with a new controller. I'll ask folks on slack for an opinion too but seems like a simple fix. Just remove that !is_preset

@mortfell
Copy link

mortfell commented Aug 8, 2019

haha Interesting.... yeah I wonder why you would want that??

Probably a moot point now LOL but This was tested VST on windows 10 64bit, but just double checked on my Mac OS sierra Machine. Same behaviour in VST3 AUi and VST.

@baconpaul
Copy link
Collaborator Author

Yeah the code is portable. If no-one has any convincing reason for why you would want that in the next 24 hours I'll change it. It is weird.

baconpaul added a commit to baconpaul/surge that referenced this issue Aug 9, 2019
As discussed in surge-synthesizer#915, for some reason the patch unstream doesn't
restore the value of the 8 controls whereas daw unstream does. This
is a conscious choice by the code for no reason any of us can see,
and is counterintuitive, so change it.
baconpaul added a commit that referenced this issue Aug 9, 2019
As discussed in #915, for some reason the patch unstream doesn't
restore the value of the 8 controls whereas daw unstream does. This
is a conscious choice by the code for no reason any of us can see,
and is counterintuitive, so change it.
@baconpaul
Copy link
Collaborator Author

No-one had any good explanation for that if. And I removed it and everything still worked and the controller restored. So I have taken it out and pushed. Just did the merge. Should be in the nightly this afternoon.

@mortfell
Copy link

mortfell commented Aug 9, 2019

Awesome look forward to testing it!

@mortfell
Copy link

mortfell commented Aug 9, 2019

Just installed the new update, works great! Thank you.

@baconpaul
Copy link
Collaborator Author

So it turns out that master volume, poly limit, and fx_bypass state are all explicitly removed from restoring in presets. I can kind of see why that's the case. So i think I'm just going to do zoom and .scl file in the 1.6.2 release.

baconpaul added a commit to baconpaul/surge that referenced this issue Aug 16, 2019
Some features of the synth - notably, zoom, MPE Enablement, and
Tuning - are features of the DAW environment you are working in and
were not persisted. This fixes that by adding a dawExtraState section
to the streaming protocol which is only populated and read at DAW time
not at general patch time.

Closes surge-synthesizer#890
Closes surge-synthesizer#914
CLoses surge-synthesizer#915
Mostly wraps up surge-synthesizer#828
baconpaul added a commit that referenced this issue Aug 16, 2019
Some features of the synth - notably, zoom, MPE Enablement, and
Tuning - are features of the DAW environment you are working in and
were not persisted. This fixes that by adding a dawExtraState section
to the streaming protocol which is only populated and read at DAW time
not at general patch time.

Closes #890
Closes #914
CLoses #915
Mostly wraps up #828
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants