ParameterQuality change in 2.3.0

@Vortico I was wondering if you could explain the implications and motivations of this change:

Make ParamQuantity::set/getValue() set/get the Param’s target value of the Engine’s per-sample smoothing algorithm instead of the Param’s immediate value. Add ParamQuantity::set/getImmediateValue(). Deprecate ParamQuantity::set/getSmoothValue().

2 Likes

I remember on super collider if there was no smoothing on bi-quads then horribleness would emanate from the speakers. Maybe it’s linked.

Everyone knows that of your module is going to modulate a biquad it needs to do something about the gunk. And smoothing a param doesn’t help with a CV the isn’t smoothed that modulates the same biquad. So I bout this is the reason.

Maybe it’s a GUI thread detach, so that the frame rate doesn’t alias/ cross modulate stepping noise?

On a quick look I think it’s just changing which method sounds like the default. Docs say:

image

So looks like what was previously set/getValue() becomes set/getImmediateValue() and what was previously set/getSmoothValue() becomes set/getValue(). I could definitely be wrong!

Yes, that is fairly clear. But what impact does that have? I’ve never investigated set/getSmoothValue, so I have no idea how that might impact behavior of a module. If the difference is obvious to the ears, then it seems this could “break” existing patches (meaning the patch result would be different / not give the expected result after modules get the new behavior).

But I say all this in ignorance. I too would love to hear from Andrew about the reasoning, and what to expect.

I’m also curious, do existing modules in the library automatically get this new behavior? Or do they need to be re-compiled. When would recompilation take place?

1 Like

Here are the 3 commits that were made in relation to this, just to add them to the conversation.

3 Likes

Thanks Steve. The description for that last one seems important to me.

Rename ParamQuantity::set/getDirectValue() to set/getImmediateValue().

Use setImmediateValue() when appropriate in ParamQuantity and SwitchQuantity, such as in reset(), randomize(), and setDisplayValue(). Add doc comments to ParamQuantity.

My interpretation is that the code for (most?) modules that use reset(), randomize(), and/or setDisplayValue() should be updated. I’m a little worried about that. Maybe I am wrong.

EDIT
Strike that. The SDK (or whatever) will typically take care of setting the param values properly with reset() and randomize(), no need for the developer code to change. That leaves setDisplayValue(), and I don’t have a sense of whether that is likely to be an issue that most developers need to worry about.

1 Like

This change resulted in a conversation Andrew and I had about a bug-like-behavior I found in surge.

First, remember there’s params and paramQuantities and this change only impacts ParamQuantity objects which (roughly) act as a description-of- and proxy-to the underlying param which the DSP uses.

The bug I ran into was this

  1. Knob::onDragMove called PQ::setSmoothed
  2. The engine moved PQ::setSmoothed → the param and param quantity value in engine_step.
  3. I had written a set of custom widgets that used paramQuantity->getValue to render and invalidate and so on (which seems reasonable since I wanted to draw the value but was wrong)

this meant (and still means) if you drag out a surge EGxVCA with no engine running (so no audio out), the sliders “dont work” and that “set/get Smoothed” was the api you needed to use for all param quantity changes.

I asked andrew “is this right - should I change to get/set ParamSmoothed everywhere” (and suggested also a rejected idea that the Ui thread know if the engine is running or not and move smoothed → value absent an engine which wasn’t right).

But it’s confusing. “Use set/getSmoothed everywhere and set/getValue rarely even if smoothing is off and so on” so andrew decided that making set/getValue == set/getSmoothedValue and introducing an explicit direct API was less bug prone. If you don’t call setDirect in a type-in or randomize the only change is that you will get the small smoothing behavior (which may be desirable)

Now the API is out I’m going to fix up surge for 2.1.5 to use this consistently in surge.

Hope that context helps some.

8 Likes

Tremendous thanks Paul - I don’t quite understand exactly what is going on, but with your writeup I feel I could grasp all the concepts if I invested time and experimentation. Especially the difference between Param and ParamQuantity - I’m still a bit confused, but you have provided a doorway to understanding.

Regardless, at least now I better understand the rationale behind the decision, even if I don’t fully appreciate the significance.

1 Like

Basically with 2.3.0 and later “if you are writing a widget; use PQ::set/getValue; if you are writing a thing which sets a value explicitly like a setValueFromString or randomize use PQ::set/getDirectValue; and always use param.value not PQ::getValue in your dsp unless you know you really really are sure” may be a more concise summary?

1 Like

I was worried that the change in getValue’s behavior would affect my module’s realtime response to CV, when I combine a parameter’s value with CV input and attenuverter per sample. (I actually want audio-rate CV signals to work, even applied to something like a slider or knob.)

But then I realized this only affects the parameter (knob/slider) part of the value, and nobody is tweaking knobs and sliders with their mouse at audio frequencies! (Unless they truly are tweaking, ahem.)

Very few of us actually implement parameter widgets directly, or at least we don’t do anything other than cosmetic stuff to them. Sounds like the net result is, I don’t need to worry about this affecting someone’s generative patch. Behavior should be the same, if I understand this correctly.

Yes, I pointed out yesterday that the smoothing would have no effect on CV (a few posts up, on this thread). I don’t really know what “most people” do, but I have certainly written my own param widgets from scratch.

I read your post but I didn’t get it until now. I just re-read it and now it makes sense. Sometimes I have to work through something in my own mind before it clicks.

2 Likes

Thanks Paul. Seems like Andrew is using the major-API-version to signify a break in API backwards-compatibility and minor-API-version to signify a break in API forwards-compatibility, at least in this case where semantics/behaviour is concerned.

/rant-mode-on
I still really don’t understand why Andrew does not engage with the Rack developers on this forum which is made for it. It would be so much more natural and efficient for everyone, rather than the sillines of email messages back/forth to individual people, or random Discord messages. I just don’t get it. It might be eventual-open-source at the code level but it certainly isn’t open in all other respects.
/rant-mode-off

3 Likes

yes this is ParamQuantity, not Param

Remember your DSP uses params[MY_ENUM].getValue() to get values.

The information about your param is set up by configParam etc… which consumes an index (enum) and returns a ParamQuantity. That param quantity acts as the metadata about and UI proxy to the dsp param. The job of the engine is to take values from param quantity and apply them to the param which is why the engine is required for smoothing.

This change basically means “ParamQuantity:: get/set value do what you expect whether an engine is running or not” whereas the prior statement was “ParamQuaintity::get/set Smoothed worked but did update value”

Again the short version is, with this change: UI Code use ParamQuantity::get/set Value' and DSP code use Param::getValue` and the long version means you probably understand the long version anyway :slight_smile:

The Smoothed versions are just deprecated and so there should really be no noticeable effect to any plugin unless you were directly using the pre-smooth values in your DSP code or something nutty like that. So I wouldn’t say this is a ‘break’ as much as a ‘rename and newly deprecated tag on the old name’, mostly. And for modules which got this wrong (like surge did) it’s actually a fix!

Right, but there is something to be said for bug-compatibility and behavioural change. In my mind, if something changes behaviour it’s a “break”, even if it doesn’t break your patch but just makes it work/sound slightly different. Sometimes it doesn’t matter much, i.e. in an individual plugin, but at the level of the Rack API I think it does matter.

If you chose to define break that way then this is a break.

I suppose since this only really makes a difference absent an audio engine, it won’t change the sound of your patch though, if I want to be super pedantic :slight_smile: (I guess it will also change your sound if you relied on on-screen knob drags creating zipper sounds in mis-coded ui elements).

Also all code which compiled with 2.2 will compile with 2.3

But I’m just here explaining the change! Don’t want to argue.

2 Likes

Ok so some of my modules used internal parameters no knobs for some state, and yes these are now library submitted to be switches with “less tooltip”. Yow da perp of bad code I know, but should work. Wonky machine state assigns by .setValue.