-
Notifications
You must be signed in to change notification settings - Fork 11
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
mod_source
not modifying correctly
#163
Comments
A clue: explicitly setting 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. I think the right thing to do is to set |
(Unfortunately just making that fix, i.e. setting every |
In order to have sane default behavior, the various ControlCoef vectors are initialized with some nonzero values: See the setup in Specifically, If you're setting the 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 |
In general, I'm OK with the Python interface having semi-magical behavior ("Oh look! Setting |
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;
} |
I don't see any difference here C vs Python -- we're not directly manipulating the oscillators (we're not changing |
I agree. Resetting Amy is important in both Python and C. |
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. |
The specific problem of mod_osc's being silenced because of the default dependence of amplitude on note velocity is fixed by e9f5fc8 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. |
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. |
In C, this:
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":
Something is up with how we fill in
X_coefs
in Python that makes assumptions we're not getting in C, I thinkThe text was updated successfully, but these errors were encountered: