surge-synthesizer / surge

Synthesizer plug-in (previously released as Vember Audio Surge)
https://surge-synthesizer.github.io/
GNU General Public License v3.0
3.02k stars 386 forks source link

Velocity is backwards #389

Open sense-amr opened 5 years ago

sense-amr commented 5 years ago

Velocity .. its a tool many musicians use to "express" the note in more ways than just ON/OFF... it typically allows a musician to vary the volume of the note .. and most Daw's support the function of allowing a note velocity .. to which the Synth then responds via .. whats commonly refered to as "Velocity Sensitivity" ..

In surge this is functioned by the Slider next to Gain in the AMP EG section .. as pictured below .. it is labeled "Vel"

vel <---

Typically 100% ie, Vel slider being all the way up referencing in the case of surge 0.00dB .. would mean the user has the Highest amount and thusly MOST velocity sensitivty from a note in a DAW sending velocity information ..

In the case of surge as referenced from the manual we have .. a difference case.. It's backwards..

velmanual

As mentioned above and also noted (pardon the pun) .. by several users on the Slack .. It is expected that velocity = 100% sensitivity rather than "Neutral" .. at Full Vertical deflection of the Vel slider..

I propose that we have this ammended.. in the interest of very easy and functional use of the Velocity response capabilities that i know Surge .. truly has ..

NB: I have provided some testing Proof that the velocity response is infact backwards to what people would normally expect.. in the following bitwig file .. for your own testing purposes..

sense_surge_VelocityTest.zip

esaruoho commented 5 years ago

@baconpaul imho, any change like this should be configurable in the settings menu. Because, there will be people who have built presets and whole songs based on Surge working one way, and if this, second way, is implemented, all their tracks will break.

sense-amr commented 5 years ago

incidentally the Patch reference for this i believe .. is

in the FXP file ..
sense-amr commented 5 years ago

I have to say i think this is a reasonably important thing to address as it has already been noticed by several casual users of Surge .. and although i would highly doubt many of the existing preset included patches even vary this value <a_vca_velsense type="2" value="0.000000" .. there might be some who do and who want to use Velocity sensitivity to add expression to their patches.. and for those .. who do it should be allowed in a normal way that is typically 100% Vel = 100% Velocity sensitivity.

kzantow commented 5 years ago

@esaruoho there would be a migration so existing patches sound the same, just with the updated behavior

sense-amr commented 5 years ago

@kzantow which is pretty easy right ? running a script on existing patches with regard to

and shipping further builds once Velocity is ammended in the code.. with those ammended patches too.. right? thusly removing any need for any fear of patches breaking that are "old" which are now .. not old..

esaruoho commented 5 years ago

@esaruoho there would be a migration so existing patches sound the same, just with the updated behavior

i don't see how this doesn't ruin songs made with user-defined Surge sounds saved into the song and sounding a specific way..

And let's not forget, we have no control over Surge preset packs that were made with Surge 1.52 on mind. what happens when you load those preset packs in, and the patch does not sound like it should?

sense-amr commented 5 years ago
that doesnt exist in a FXP on surge 1.52 presets?
sense-amr commented 5 years ago

and it kinda makes Zero sense that you cant "access" Surge 1.52 presets @esaruoho because all you need to do is load it then save it again and you have an FXP..

and if someone wants to furnish me with a list of such "Surge 1.52" patches.. ill happily "convert" them to 1.6 patches

esaruoho commented 5 years ago

and it kinda makes Zero sense that you cant "access" Surge 1.52 presets @esaruoho because all you need to do is load it then save it again and you have an FXP..

and if someone wants to furnish me with a list of such "Surge 1.52" patches.. ill happily "convert" them to 1.6 patches

Tell me how you will "convert" to 1.6 patches this package that is being sold, and is on a server that you have no access to. https://www.mysteryislands-music.com/product/vember-audio-surge-residue-soundset/

You will do no such thing. Because you don't have access to each and every preset pack downloaded by each and every downloader of each and every preset pack that exists for Surge.

We're talking about basically ruining people's songs when they switch from 1.52 to Surge 1.6. This is not something to be spoken about lightly and in a jokey manner.

If the plugin breaks songs, users of Surge 1.52 who used it years ago, will not want to upgrade to a Surge 1.6.

sense-amr commented 5 years ago

Neither is Velocity 100% = 0

sense-amr commented 5 years ago

oh more insider information.. i had no idea surge packs for a now FREE OPEN SOURCE SYNTH will be SOLD.. nor did i know theres some hidden super special STASH of "1.52" Surge patches! ..

But hey if there is .. if SOMEONE would like to furnish them to me.. ill be more than happy to CONVERT Them to 1.6 surge .. by loading them and saving them? oh thats right.. 1.52 patches PROBABLY wont work anyway will they ? since they were made with an OLDER version of surge .. and surge is in 2019 now.. with 1.6 ..

But hey if there is MAGIC afoot with for some unknown reason shipping 1.52 patches of Surge for 1.6 Surge .. that OBVIOUSLY means someone has already FIXED them to work .. so i dont really see what the problem is at all.. with then having THEIR modified in the FXP file ..?

Oh yeah that url references ONE pack .. https://www.mysteryislands-music.com/brand/vember-audio/surge/

with a total of 40 Presets.. not overly difficult to convert at all.. and btw i dont think its VEMBER anything anymore ..

sense-amr commented 5 years ago

why is a FOSS project supporting Pay for presets anyway ?

esaruoho commented 5 years ago

You seem to be purposefully ignoring what I am saying, so I will, once again, explain.

This change will break songs and presets. Songs that are on people's harddrives, presets that are on people's harddrives. I realise that you seem to be ignoring the magnitude of how much this matters.

If there are changes made into how presets, and, by extension, songs, are loaded/played, this will mean that songs, that worked with Surge 1.52, will break on Surge 1.6.

This means that the plugin is untrustworthy. It therefore will not work like it used to, and I realise that the common theme seems to be "screw them all, who cares about their music, let's break stuff", but I really don't think that's the way to go.

That's why I suggested, that this could be a menu switch for those that actually need this. Those that don't need it, will use the regular Surge, and their songs will not break.

This is about trustworthiness, nothing else.

esaruoho commented 5 years ago

Yes, people are selling, and giving away presets that were created with Surge 1.52. Yes, they should play like they should - i.e., if you have made a song with presets or self-made sounds, which played 1 way with Surge 1.52, they should play that same way with 1.6.

You have no worldwide control over the internet's .zip content of Surge presets. You never have, and you never will. So don't propose that you can just go, and tell people "yeah so like these presets you're selling, they don't work with Surge 1.6 so here's a fixed bunch of presets where they will work with 1.6".

Most people that use Surge daily, are still using Surge 1.52. They will move, when they can be sure that Surge 1.6 does not crash or break their songs. It would be correct to expect that a preset made with Surge 1.6 would play, currently, identically with 1.52. Same the other way.

If you make a preset with Surge 1.52, it should play the same way with Surge 1.6.

esaruoho commented 5 years ago

I gave you an example of a surge preset pack made for 1.52. There are multiple such packs. You seem to be hell-bent on having a problem with it being a paid patch pack.

There are numerous places all over the web that give away free Surge 1.52 presets, and they work with Surge 1.52.

They should also play the same way with Surge 1.6. For you to purposefully, repeatedly, ignore this, is beyond me.

sense-amr commented 5 years ago

ahh yeh ok .. lets just leave it like this then velmanual

and advise everyone that comes into #general asking "hey why doesnt velocity work?" .. Oh thats because intuitively you're meant to turn your "Vel" knob .. down to ZERO .. which means it has 100% velocity sensitivity ..

yeah..

sense-amr commented 5 years ago

its actually You that is missing .. and conflating THIS issue with your idea of me wanting to break surge .. totally absurge!

sense-amr commented 5 years ago

Disclaimer:

This ISSUE is specifically stated to be about the Vel slider @ AMP envelope and how it currently functions inside Surge.. any link about "breaking patches" is not intended nor considered at the time of making this issue..

esaruoho commented 5 years ago

i don't grok why we can't just have a setting in the settings menu for surge 1.52 compatibility Mode ON, where it is OFF by default? ya know?

baconpaul commented 5 years ago

This is a long thread which I haven’t fully read but I see I got tagged!

Anyway I don’t plan to work on it any time soon - bitmap arity, userdefaults and saved zoom, vst3 zoom, and mpe polyphony would all come ahead. So I think we have enough time to hear from @kurasu who designed it.

baconpaul commented 5 years ago

are you guys confusing the value and the display of value? Like if the slider was flipped and tooltip was different but values in internal engine were completely the same would this whole problem just be solved? Is it just “the value at the top of the slider should be the one at the bottom and vice versa?”

I agree with @esaruoho that patch compatabikit with 152 is a key goal

sense-amr commented 5 years ago

i hope you do plan on it before "release" .. because velocity 100% = 0 is a pretty significant issue for a new user of a Synth..

unless it just stays like this

velmanual

baconpaul commented 5 years ago

Well either someone will work on it, someone will make a case that it is correct, or this issue will be open when we do a release, or there will never be a release. I know one of those four is true.

Don’t know if it will be me. I appreciate your hope for my plans tho...

I haven’t played with the slider to see what it does. Before I started making a case coding or proposing anything I would, as would, I hope, some other person who was making a case coding or proposing would too!

sense-amr commented 5 years ago

yeah i made a case.. and provided evidence..

sagantech commented 5 years ago

So I was conversing with sense about this and verified that this is an issue in 1.5.2 as well as 1.6. My idea is to simply change the display so it is inverted. No change to patches or DSP code needed.

sense-amr commented 5 years ago

lets also talk to @kurasu about this .. and see what he says .. but i think thats a great fix if it is to be changed @sagantech

sense-amr commented 5 years ago

note: i did read "Vel" to be a function of Velocity sensitivity control and i would imagine most new users of Surge would also think it i s..

baconpaul commented 5 years ago

@sagantech - thanks. If it really is just a display thing the fix is super easy. Won’t break anyone’s track or anything.

sense-amr commented 5 years ago

@baconpaul why don't you do a test yourself ? i provided the bitwig file..

sense-amr commented 5 years ago

Hmm having some discussions with @kzantow about this .. we make the point that "Velocity" is already a mod parameter in the synth .. that can be routed at will by the user to everything and anything they want.. In many ways this makes the "vel" reasonably redundant..

We make a tentative proposal to use that slider for something else.. perhaps a Pan?..

thoughts? .. harsh rebuttals? welcome..

baconpaul commented 5 years ago

When I get time to look at this issue in detail, testing it is my first step :) I’ll also scan all the presets to see which ones use this if any.

Re-using for pan seems a bit odd since you want pan per osc. And boy do I want pan per osc!

sense-amr commented 5 years ago

Thats ok ,. panning isnt really the thing for it .. i thought better to put a slider that enables colour change of the GUI then that can be mapped to an LFO for technicolour surge.. cos everyone wants technicolour surge

The testing has been done.. and im sure youll have EXACTLY the same result :) .. the only question really is does it need to be fixed for users or not .. who have incidentally said "why doesnt surge have velocity control?" .. and they said that precisely because they had the slider ALREADY at full position which it is normally if you didnt touch anything.. and full slider = 100% = NO velocity sensitivity .. which is the opposite from any other synth youve ever used im sure.. and yeah i dont really get why this is an issue .. that people need to argue against..

its basically a no brainer.. people want velocity sensitivity .. the synth suggests its there.. Its not there as expected..

image 3

Now they could actually always map The velocity modulator to any other parameter they want.. that will give them Velocity control .. ..

But the question is .. does the Slider need Fixing .. or removing.. It has been suggested that the slider could simply be inverted .. then that would give the user the ability to have slider = 0 .. = No vel sensitivity ..which most people would expect.. removing it would not remove the users ability .. as mentioned above .. for them to map velocity to anything and everything

sense-amr commented 5 years ago

closed this issue as it seems not to be important and people can't even be bothered to explain why ..

sense-amr commented 5 years ago

grok-the-world_0

baconpaul commented 5 years ago

Welcome back @sense-amr. I promise we will get to this issue!

sense-amr commented 5 years ago

@baconpaul thank you Paul , if it is indeed an issue .. i believe it should be addressed, my main concern with it was to asccertain whether it would be dealt with before the "release" of the upcomming release version .. it's affecting users ability to understand that Surge DOES indeed have the potential for velocity sensitivity .. not everyone is going to use the "velocity" modulator .. most people will opt to quickly access velocity sensitivity via the "vel" slider..

baconpaul commented 5 years ago

Yup. Unless we’ve tagged an issue future release it is my intent to at least work on it and fix it or make a conscious decision to not with concensus before we release for all issues.

baconpaul commented 5 years ago

And thanks for reopening it.

sense-amr commented 5 years ago

I welcome all positive discussion about it .. what i dont want is insinuations EVER that by raising an issue .. im BREAKING something, as we know that only happens when i attempt to CODE to fix something.. not the case here..

baconpaul commented 5 years ago

OK @sense-amr I finally had 10 minutes to try this and ha yes it is totally weird and I now totally understand what you are on about.

So I made a track in logic and crescendoed the notes from 0->127. With a piano they got louder obviously.

Then I pulled in surge init. With "vel" at the top default position of 100% there was NO velocity sensitivity and with the "vel" at the bottom position of 0% there was the same velocity sensitivity as a piano.

So the good news is velocity sensitivity works. We don't have to change any DSP and don't have to change any patches. The only problem here really is that the slider is upside down.

So here's my proposal

  1. Don't change any patches. Leave them all with vel_sens=0. Don't even change init. Leave that with vel_sens=0
  2. Change the string display of the vel sens. So rather than going between -inf (which is 100% vel sens) and 0 (which is no vel sense) make the display go between "0 vel sens" and "100% vel sens"
  3. Flip the order of the slider so 0->1 and 1->0. This means in init the slider will be all the way down rather than all the way up (because remember in step 1 I'm not changing any patches).
  4. Update the manual to remove the text that says 'the velocity slider does a correct but counterintuitive thing' to instead have text that says 'the velocity slider does a correct intuitive thing'
  5. open another issue to have the long and exciting conversation about whether the default value for vel in the init patch should be different.

So nothing to break; no patches to scan.

The only bummer about this is putting a custom function in the slider so it reverse values is a teensy bit tricky. So it will take a little while to get to this. But it's much easier than feared if we actually wanted to change what the value meant.

sound good yeah?

baconpaul commented 5 years ago

Oh and I'm tagging this 'ui problem' since as outlined above the only change we need is to the display of the value, not the meaning or response to the value.

sense-amr commented 5 years ago

Well i'm glad you tested it .. and now can see whats going on with the issue.. I guess the question now is "is it actually an issue?" as in do you think it causes a problem for users, is it something that needs to "fixed" ..

There are essentially 2 issues here i think ..

  1. that the Slider responds in an unexpected way for the users functionality.. thus giving them the impression "Velocity sensitivity doesnt work" .. when in fact it does work... it's just upside down to what they would normally expect..

  2. The init patch comes with the slider at the Full 100% value ..

On further examination of many of the default packaged patches i found most users opted to use the "Velocity" modulator instead of the slider.. because it DID behave in a way they understood and expected.. so they probably just omitted the use of the Vel slider alltogether when creating their patches..

I was incidentally talking to Claes about this and his response was ..

lets think about that after 1.6, it's not that of a priority for now

so .. i guess if its EASY to fix .. it shouldnt be a problem but if its not really a priority it can stay on the back burner for now .. i would like to say tho its something i'd personally like to see rectified so that the new users of Surge which presumably we are wanting to target... are going to be able to have clearly functional "Vel" Sensitivity .. with out having to come to the conclusion that it doesnt work .. and look for a work around like the "Velocity" modulator ..

baconpaul commented 5 years ago

Right. Thenslider now is best described as “velocity compression”. At 100% all your velocities are squished to 127 and at 0% they map 1:1 onto input velocity. Knowing that makes it make sense

But having the default be full velocity compression is odd

I will ponder some also. Lots of options. I’m not sure which is best but I do know we can adjust it without breaking anything since it’s just a ui and into default thing

sense-amr commented 5 years ago

Yeah also the concept of having a Velocity sensitivity which Claes did describe it to me as i discussed it with him .. being a dB value instead of a % ... i think is a bit weird..

baconpaul commented 5 years ago

It does explain why I had a hard time making surge as expressive as equator. If I had set that parameter lower ....

Anyway I get it. Lemme think. I’ll also read the code one day to find out exactly the function which is applied

per99 commented 5 years ago

Hi guys, I tried to make Surge velocity sensitive by turning the vel.-slider down to -40. But my Surge still isn't velocity sensitive. I had to push the gain slider upwards to be able to hear anyting at all... But as I said - no sensitivity. I have the free Surge 32 bit, 1.6.0. , someone on the KVR forum converted the 64 bit a couple of months ago. Do you think this has got anything to do with it?

esaruoho commented 5 years ago

@per99 i highly recommend deleting the 32-bit version someone gave you months ago, and download the newest 32-bit build from http://surge-synthesizer.github.io . please check with that first to see if the issue still exists. :) if it does, then at least we're all working with the newest version and reporting the newest version's issues.

per99 commented 5 years ago

@esaruoho I will definitely follow your advise and get back to you :)

per99 commented 5 years ago

OK, I have downloaded and tested: The VST3 version has no vel sens, but the .dll version works as described above (I use Cubase Essential 5 and have Windows 10 proffessional).

baconpaul commented 5 years ago

Oh yeah so that's the VST3 midi information bug. https://github.com/surge-synthesizer/surge/issues/26

Midi information about things like velocity and pitch don't flow properly in the VST3 yet

The VST2 and the AU it works fine

Sigh - lots to fix. Lots to fix.