AudioParameterChoice::convertTo0to1 suggestion

Juce 4 looks great so far, funnily i implemented lots of functionality already in my own extensions classes ;-)

 

The way you convert a choice parameter to a float value, seems not save with rounding

For example if you have three choices, and use the second (1), the float value is 0.33333333333, if there is only a minor adjustment it turns to the first choice (0)

 

float AudioParameterChoice::convertTo0to1 (int v) const noexcept { return limitRange (v) / (float) choices.size(); }

instead it should be

float AudioParameterChoice::convertTo0to1 (int v) const noexcept { return limitRange (v+0.5f) / (float) choices.size(); }

or better (check if its the first or last step)

float AudioParameterChoice::convertTo0to1 (int v) const noexcept 

{

  if (v<=0) return 0.f;

  if (v>=choices.size()-1) return 1.f;

  return limitRange (v+0.5f) / (float) choices.size();

};

 

 

 

Huh? limitRange takes an int, so that code would add 0.5 to an int, then round the result down to an int, pass that into a function that returns another int.. ??? How would that make sense?

ah yes, i didn't checked that limitRange uses int (btw. its unnecessary)

float AudioParameterChoice::convertTo0to1 (int v) const noexcept 

{

  if (v<=0) return 0.f;

  if (v>=choices.size()-1) return 1.f;

  return (v+0.5f) / (float) choices.size();

};

I support this concern, however the proposed solution goes in the wrong direction in my opinion. The integer numbers of the choices are an subset of floating point numbers. So there is no problem to use the original implementation. The problem arises when the float number got truncated (e.g. when transformed into a string for saving) and is reduced afterwards to the choices.

The boundaries should not be at the calculated values but in between, what could be achieved by adding a small delta like:

int AudioParameterChoice::convertFrom0to1 (float v) const noexcept       { return limitRange ((int) (v * ((float) choices.size() + 0.1f))); }

 

Thanks, i still prefer my solution, the value is exact in the center of "range" of a certain integer value (exept first and last)

In most hosts you can use a slider, to change the value, its like switch, you would assume (as human), that a certain position is in the middle of range which represents the integer-value.

Ok, thanks guys, I'll take a look..

I look at the implementation again, I'm still not happy with getValue & setValue, i think the implementation is wrong also for the other classes.

float AudioParameterChoice::getValue() const { return convertTo0to1 (roundToInt (value)); }
void AudioParameterChoice::setValue (float newValue) { value = (float) convertFrom0to1 (newValue); }

That value should be only stored as float, exactly that float which the hosts commits. So if the hosts calls getValue you get the exact same value again.

Only when the other code needs the integer representation it should be converted. I think this is what the host expects as behavior. (To go one step further, the storing of the current value should be the job of the abstract base class).

Only the converting should be part of the sub-classes.

float AudioParameterChoice::getValue() const { return value }
void AudioParameterChoice::setValue (float newValue) { value = jlimit(0.f,1.f,newValue); }

int AudioParameterChoice::getCurrentStep(convertFrom0to1 (newValue);)

 

 

 

You are right, and mathematically I have to admit the idea with a fixed delta is wrong. It needs to calculate like:

return (v + 0.5 / choices.size()) * choices.size();

which is equivalent to:

return (int)((v * choices.size()) + 0.5);

And needs to check, that there is actually a choice -> choices.size() > 1.

int AudioParameterChoice::convertFrom0to1 (float v) const noexcept { 
    if (choices.size() > 1)
        return limitRange ((int) (v * ((float) choices.size() + 0.1f))); 
    return 0;
}

But I still think reading the value is the right place to fix it, as you don't know, if writing loses precision at all...

I just wanted to correct my error, from now I leave it to jules, as there are lots of ways to fix...

As long as it calculates correctly I shall be happy (which includes 0.333 * 3 should get choice one, which it does not, because there is only truncation done, as far as I can see).

When i started this topic the new parameter system was very fresh, i hoped that this would be fixed quickly, but now people using it. But for me its just a wrong design decision. I can’t use it.

The same goes for AudioProcessorValueTreeState::Parameter

  void setValue (float newValue) override
    {
        newValue = range.snapToLegalValue (range.convertFrom0to1 (newValue));

        if (value != newValue || listenersNeedCalling)
        {
            value = newValue;