`mod_source` not modifying correctly

[bwhitman]

In C, this:

struct event a = amy_default_event();
a.osc = 1;
a.wave = SINE;
a.freq_coefs[COEF_CONST] = .25;
a.amp_coefs[COEF_CONST] = 1;
amy_add_event(a);

struct event e = amy_default_event();
e.osc = 0;
e.wave = PULSE;
e.freq_coefs[COEF_CONST] = 440;
e.mod_source = 1;
e.duty_coefs[COEF_MOD] = 1;
e.velocity = 1;
amy_add_event(e);

Should make a 0.25Hz duty cycle LFO on a 440Hz pulse wave. But it plays a constant pulse wave instead.

Written in Python, it "just works":

amy.reset()
amy.send(osc=1,wave=amy.SINE,freq=0.25,amp=1)
amy.send(osc=0,wave=amy.PULSE,mod_source=1, freq=440, duty="0,0,0,0,0,1",vel=1)

Something is up with how we fill in X_coefs in Python that makes assumptions we're not getting in C, I think

[bwhitman]

A clue: explicitly setting x_coefs to 0 makes it equivalent:

struct event a = amy_default_event();
a.osc = 1;
a.wave = SINE;
a.freq_coefs[0] = 0.25;
a.freq_coefs[1] = 0;
a.freq_coefs[2] = 0;
a.freq_coefs[3] = 0;
a.freq_coefs[4] = 0;
a.freq_coefs[5] = 0;
a.freq_coefs[6] = 0;

a.amp_coefs[0] = 1;
a.amp_coefs[1] = 0;
a.amp_coefs[2] = 0;
a.amp_coefs[3] = 0;
a.amp_coefs[4] = 0;
a.amp_coefs[5] = 0;
a.amp_coefs[6] = 0;

amy_add_event(a);

struct event e = amy_default_event();
e.osc = 0;
e.wave = PULSE;
e.freq_coefs[0] = 440;
e.freq_coefs[1] = 0;
e.freq_coefs[2] = 0;
e.freq_coefs[3] = 0;
e.freq_coefs[4] = 0;
e.freq_coefs[5] = 0;
e.freq_coefs[6] = 0;

e.mod_source = 1;

e.duty_coefs[0] = 0;
e.duty_coefs[1] = 0;
e.duty_coefs[2] = 0;
e.duty_coefs[3] = 0;
e.duty_coefs[4] = 0;
e.duty_coefs[5] = 1;
e.duty_coefs[6] = 0;
e.velocity = 1;
amy_add_event(e);

I think we see the problem. amy_default_event() UNSETs each coefficient. The Python string gets parsed by amy_parse_message() which counts the parameters and sets the rest to 0 (which is critically different than an UNSET.)

I think the right thing to do is to set coefs to 0 on amy_default_event(), not UNSET. But I'll wait for @dpwe to weigh in.

[bwhitman]

(Unfortunately just making that fix, i.e. setting every coef to 0 on amy_default_event(), causes the AMY event queue to overload during make test. This is because it's creating an AMY delta for 7*5 = 35 parameters per event, even if that CtrlCoef isn't being used. The reason the Python parser doesn't do this is we only set the parameters to 0 when you set a single CtrlCoef. I think it may be a better use of our time to seamlessly handle UNSET CtrlCoefs when they are evaluated.

[dpwe]

In order to have sane default behavior, the various ControlCoef vectors are initialized with some nonzero values: See the setup in reset_osc() around line 581 of amy.c (also described briefly in the paragraph beginning "Actually" in README.md).

Specifically, amp_coefs is initialized to {0, 0, 1, 1, 0, 0, 0} (COEF_VEL and COEF_EG0, so the amplitude follows the note-on velocity modulated by envelope generator 0), and logfreq_coefs is initialized to {0, 1, 0, 0, 0, 0, 1} (COEF_NOTE and COEF_BEND, so the oscillator frequency follows the note-on pitch and the global pitch bend). In addition the COEF_CONST fields of duty_coefs and pan_coefs are 0.5, since these are the default, central, values.

If you're setting the _coefs vectors by hand and you don't want the defaults, you'll have to clear these values to zero, but that's normally not a problem. The most common exception is an LFO, which you typically don't want to depend on the note velocity (since you want it to free-run without a note-on event). I think that's why the original code doesn't achieve any LFO modulation. All it needs is a.amp_coefs[COEF_VEL] = 0 added when setting up osc 1. The other coefs being set to zero in the fixed code above are redundant with reset_osc() defaults.

Of course, another common gotcha with direct manipulation of oscs from C is not resetting the osc, and so inheriting possibly non-default settings from the previous use of the osc. It's probably good practice to call amy_reset_oscs() or send e.reset_osc = osc events before setting up oscs. This appears is several forms in src/examples.c.

[dpwe]

In general, I'm OK with the Python interface having semi-magical behavior ("Oh look! Setting amp=4 automatically clears the COEF_VEL parameter"), but having the C-interface be much more explicit and literal ("I really just wanted to set amp_coefs[COEF_CONST] to 4 without changing the value of amp_coefs[COEF_VEL]").

[bwhitman]

But sending any message with a CtrlCoef via Python or an AMY wire string will set the unspecified coefs to 0, not the defaults you specified. Is this a bug? This is the behavior I'm attempting to ape in #166 . If that's wrong, we should fix it here too.

void parse_coef_message(char *message, float *coefs) {
    int num_coefs = parse_float_list_message(message, coefs, NUM_COMBO_COEFS);
    // Clear the unspecified coefs to zero.
    for (int i = num_coefs; i < NUM_COMBO_COEFS; ++i)
        coefs[i] = 0;
}

[bwhitman]

Of course, another common gotcha with direct manipulation of oscs from C is not resetting the osc,

I don't see any difference here C vs Python -- we're not directly manipulating the oscillators (we're not changing synth[osc]), we're doing precisely what you'd do in Python/wire messages, creating an struct event which is parsed by amy_add_event and turned into deltas. There's no special reason to call for a reset in C that you wouldn't also want in Python.

[dpwe]

I agree. Resetting Amy is important in both Python and C.

[dpwe]

But sending any message with a CtrlCoef via Python or an AMY wire string will set the unspecified coefs to 0, not the defaults you specified. Is this a bug? This is the behavior I'm attempting to ape in #166 . If that's wrong, we should fix it here too.

It's intended behavior because the "magic" clearing of defaults is (sometimes) beneficial: if you're manually setting the CONST coefficient of a control vector via Python, you may well be doing some interactive exploration and you just want that value to take on an unmodulated constant value. I don't want to force you to type "freq=440,0,0,0,0,0,0" every time.

The "interactive investigation" scenario doesn't really apply to the C API.

One thing I haven't yet raised is the treatment of bare commas in coef strings (e.g. amp=",,,,,1"). In float lists, they result in the missing value being treated as zero, but in int lists (only algo_source) they result in UNSET (or some other passed-in special value). We could make bare commas mean UNSET for float lists too, and remove the implicit trailing zeros, so you could pick out individual values to set via the wire interface too. That might be the best solution.

[dpwe]

The specific problem of mod_osc's being silenced because of the default dependence of amplitude on note velocity is fixed by e9f5fc82cc70914b2c882eb48f74c4ad28a06411 which clears the amplitude-velocity link for the special case of oscillators designated modulators.

The larger problem of inconsistency between the Python API (ControlCoefficients set as entire vectors at once) and the C API (elements in ControlCoefficients set individually) remains to be resolved, the WIP is in branch pythons_coef_unset.

[dpwe]

The inconsistency between C API setting individual coefficients, and Python API setting whole vectors at once, was resolved in #172 by making the Python API also set individual coefficients.