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

Portamento is borked #7301

Open
Andreya-Autumn opened this issue Nov 10, 2023 · 12 comments · Fixed by #7318
Open

Portamento is borked #7301

Andreya-Autumn opened this issue Nov 10, 2023 · 12 comments · Fixed by #7318
Labels
Bug Report Item submitted using the Bug Report template DSP Issues and feature requests related to sound generation in the synth

Comments

@Andreya-Autumn
Copy link
Collaborator

Andreya-Autumn commented Nov 10, 2023

An exciting 3 part series!

Episode 1 - "0 = unmodulatable"

Load Init Sine and make it mono
Assign a Macro to portamento at full strength
Turn the macro up almost to full
Play a few overlapping notes and notice that they aren't gliding
...but if we make the baseline porta value just slightly >0 they do glide!

Pretty bonkered right? Stay tuned, it gets weirder!

Episode 2 - "if (value + mod) > max then bork!"

Starting at the end of episode 1, turn the baseline porta value up to where the macro would bring it past max.

The glide is now gone again, and in its place is the following wonderful feature:
The first note you play sounds the correct frequency, the next note sounds the same frequency again no matter what key you press, after that each keypress will sound the frequency of the previously pressed key. :)

Episode 3 - "Was it always like this?"

Un-assign the macro now, and keep the porta value high
Press and hold C
Then press and release any other note a few times in a row (holding the original C).
The porta to the second note initiates the first time, but after that it's gone.
It'll skip to the second note immediately but still glides back to C.

If the distance to the other note is big and the porta is slow, you can hear that the problem begins already if you release the second note while the glide is traveling there. It skips up instantly and starts gliding down.

@Andreya-Autumn Andreya-Autumn added DSP Issues and feature requests related to sound generation in the synth Bug Report Item submitted using the Bug Report template labels Nov 10, 2023
@Andreya-Autumn Andreya-Autumn added this to the Surge XT 1.3 milestone Nov 10, 2023
@baconpaul
Copy link
Collaborator

The first is a modulation order problem which happens all over the place. We define it as correct behavior in that you can’t modulate porta time once the glide is over and at 0 the glide is over before the end of the first block

the second is a bug needing a clamp though

@Andreya-Autumn
Copy link
Collaborator Author

Ah right. I remember that now, makes sense.

Any ideas about 3?

@baconpaul
Copy link
Collaborator

I have not been at a computer with surge since I read your issue! So no idea on 3 yet. I can easily imagine how an aborted porta glide goes wrong though

@Andreya-Autumn Andreya-Autumn changed the title Portamento is borked: Portamento is borked Nov 11, 2023
@baconpaul
Copy link
Collaborator

I dont think 2 is incorrect behavior per se but it is caused by the portanemto time being 2^x

and so what's happening is with ports near max and modulated to max you get a 4 hour glide. If when you play the second note with it held you move the macro back you hear the ports start again

I will cap portatime at something reasonable

@Andreya-Autumn
Copy link
Collaborator Author

Hahaha yeah, In the highly unlikely event someone wants a 4 hour glide they can use pitch automation I guess. So yeah, clamping it seems good.

It's still weird that on subsequent overlapping keypresses it does insta-shift (to the previous notes frequency). I'm now inclined to diagnose both that aspect of 2 and 3 as the same bug and I would call it "new notes mid-portamento don't interrupt correctly." Or something.

@baconpaul
Copy link
Collaborator

problem 3 i a really nasty problem with the non-single-trigger modes (MONO and MONO FP). I don't know if I can fix it without a breaking change.

Here's a simpler case to number 3 (and the back half of your number 2)

  • Init Sine in MONO or MONO FP
  • Set portamento to a long time.
  • Press Middle C.
  • Press the G above. While the glide is still ongoing release
  • Hear that the release initiates at the G not at the current porta state

Mono ST and ST + FP don't do this; it's only the retriggering modes which do

baconpaul added a commit to baconpaul/surge that referenced this issue Nov 20, 2023
Max porta time was 2 (aka "4 seconds") but was modulatable across
the entire range up to about 128 seconds I think. That's the same
as stuck. So add an internal clamp which means even in extreme off
the end modulation, you cap out at 16 seconds.

Addresses case 2 of surge-synthesizer#7301
baconpaul added a commit that referenced this issue Nov 20, 2023
Max porta time was 2 (aka "4 seconds") but was modulatable across
the entire range up to about 128 seconds I think. That's the same
as stuck. So add an internal clamp which means even in extreme off
the end modulation, you cap out at 16 seconds.

Addresses case 2 of #7301
baconpaul added a commit to baconpaul/surge that referenced this issue Nov 20, 2023
In Mono and Mono FP the portamento would work incorrectly
on a held release with a partial slide. that is, set portamento
to a long time, Press C, press G wait until you get to about E
in the slide, release the G. The porta-slide-back would start from
G.

This makes it start from E with the appropriate phasing and time.

It is a change in bheaviour but a correct behaviour so there
is no bailout option right now.

Closes surge-synthesizer#7301
baconpaul added a commit that referenced this issue Nov 20, 2023
In Mono and Mono FP the portamento would work incorrectly
on a held release with a partial slide. that is, set portamento
to a long time, Press C, press G wait until you get to about E
in the slide, release the G. The porta-slide-back would start from
G.

This makes it start from E with the appropriate phasing and time.

It is a change in bheaviour but a correct behaviour so there
is no bailout option right now.

Closes #7301
@Andreya-Autumn
Copy link
Collaborator Author

We're almost there but not quite there:

Init Sine or whatever
Long release and porta.

As long as at least one note is always held, you get the new behavior, which is great: The note is always sliding towards the last key you pressed. But if you release each note before tapping the next one, you still get something like the old weirdness where interrupting the porta makes it skip.

@Andreya-Autumn
Copy link
Collaborator Author

if (wasGated && pkeyToReuse > 0)

Taking a guess that WasGated being part of the condition here is causing that issue? "Uber_release" to the rescue?

@baconpaul
Copy link
Collaborator

yeah i think that's right

@Andreya-Autumn
Copy link
Collaborator Author

On second thought, I don't think that's it. That line is in the mono and mono_fp case, but the behavior happens in every case including poly!

@baconpaul
Copy link
Collaborator

ugh what a mess. it is just wrong

I'm tempted to back down this change altogether and ship 1.3 == 1.2.3 since i'm now getting very worried about side effects in the small porta case. Your thoughts?

baconpaul added a commit to baconpaul/surge that referenced this issue Nov 22, 2023
Partly reverting the change we made addressing surge-synthesizer#7301, we change
the attempt-to-make-mono-fp-with-long-porta work fix, since it
ran away and didn't fix many cases.
baconpaul added a commit that referenced this issue Nov 22, 2023
Partly reverting the change we made addressing #7301, we change
the attempt-to-make-mono-fp-with-long-porta work fix, since it
ran away and didn't fix many cases.
@Andreya-Autumn
Copy link
Collaborator Author

Longer comment on Discord. But long story short: This has been borked since pre-XT times. It's sort of the correct behavior for poly (non-piano) mode, but mono would need a make-over. Totally agree that's a process that needs to not be stressed. So removing from milestone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Report Item submitted using the Bug Report template DSP Issues and feature requests related to sound generation in the synth
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants