Only input faders use `interp_time` while others use number of steps as set in default constructor

[nickbailey]

Would you mind running the just-pushed test and veryifing that the debug output contains a lot of lines like:

JackBackend::getOutVolume(QString  "out_1"  ).
        num_steps =  2206 ; target= 0.250899
JackBackend::getOutVolume(QString  "out_2"  ).
        num_steps =  2206 ; target= 0.210432

(the target should be 1 before you move the output fader, and the number of steps will obviously change depending on your jack settings)

[nickbailey]

I changed the logic in the fader interpolation code in the latest commit. It looked to me like the cur_step variable was being tested for completion and, if so, set to 0 within the loop which is broken behaviour now the if ( !qFuzzyCompare(fs.target, fs.current)) test has been moved outside the loop.

The debugging output suggests the new logic works. I see for example:

In/Out fader. Target  0.181662 :  2401  steps, now  0
In/Out fader. Target  0.181662 :  2401  steps, now  512
In/Out fader. Target  0.181662 :  2401  steps, now  1024
In/Out fader. Target  0.181662 :  2401  steps, now  1536
In/Out fader. Target  0.181662 :  2401  steps, now  2048
In/Out fader reached target at step  2401
Matrix fader. Target  1.03659 :  2401  steps, now  0
Matrix fader. Target  1.03659 :  2401  steps, now  512
Matrix fader. Target  1.03659 :  2401  steps, now  1024
Matrix fader. Target  1.03659 :  2401  steps, now  1536
Matrix fader. Target  1.03659 :  2401  steps, now  2048
Matrix fader reached target at step  2401

[dsheeler]

I agree your changes make the code clearer, but I think the old code had the same behavior, because the first time that test finds the number of steps is reached, it sets the target and current to be equal, and the remaining iterations will find vol = current + step_num * (target - current)/num_steps = current, regardless of step_num, which gets reset and then incremented, but that's irrelevant because target - current = 0.

[nickbailey]

Yes, you're right. I should've spotted that. If you don't mind I'm going to leave it as writ because I find it more obvious, and it gets rid of a conditional in the loop. Call me The Early Optimiser :) Wondering if the issue is still extant as I'm having trouble reproducing it here. The debug output should print stuff like the above when faders are moved, and be similar for any fader, if things are OK.

[dsheeler]

Yes, it's better the way you wrote it. I'll tell you how I produced the issue:

gitdiff.txt

[nickbailey]

Just discovered the problem arises when a patch gets loaded. Thanks for your help, @dsheeler : I would never have spotted that!

EDIT: Exploding and recombining the fader doesn't help. Note to self: look at how faders are constructed in the XML-reading code.

[nickbailey]

It is to do with default constructors. Qt pain. I'll just live with it I guess.

I've imposed an extra duty on the backend authors to fix it by detecting in the getVolume methods are trying to us an incompletely initialised fader (num_steps will be 0 if so) and as they have access to all the backend info, substitute a properly formed one.

Good catch. I don't need interpolation so I'd never have spotted it!

[nickbailey]

Found the offending failure to register input and output channels. I'll close this issue unless anyone stops me.