NTS-1 mkII: unit_set_param_value() calls after unit_init() and before the first call of unit_render() destroy the parameter values

[boochow]

A series of unit_set_param_value() calls occur just after unit_init() and before the first call of unit_render() have wrong parameter values(zero in most cases.) and they eventually destroy parameter values initialized in unit_init().
The parameter values of these unit_set_param_value() calls don't fit the value ranges defined in header.c and should be removed.
As a workaround, I use a flag variable to indicate whether the first call of unit_render() has occurred and ignore all unit_set_param_value() calls before the first call of unit_render().

    inline int8_t Init(const unit_runtime_desc_t * desc) {
        started_ = false;
    .
    .
    fast_inline void Process(const float * in, float * out, size_t frames) {
        started_ = true;
    .
    .
    inline void setParameter(uint8_t index, int32_t value) {
        if (!started_) {
            return;
	}

[AndrewCapon]

After factory reset and install I see all unit_set_param_value() calls have value of 0, at this point I would think they should all be the default value of the parameter.

Once a param has been set by the user and the unit is reloaded then I get the values the user set, this also works after a power cycle.

So we are in a bit of a bind, how do we know we have just been installed and we are getting incorrect 0 values rather than the actual values set by the user and saved away somewhere?

So the bug is really that instead of copying the default values of the params to the storage on install, it is instead just zeroing them.

[boochow]

So it seems that

(1) When the power goes down, NTS-1 mkII keeps only the last state of the user's settings for all the parameters, including the oscillator selection.
(2) After the power is on, the last selected oscillator is selected again, initialized, and passed the previous settings by a series of unit_set_param_value() calls.
(3) When the user selects another oscillator, NTS-1 mkII calls its unit_init() then unit_set_param_value() with the values of zero.

Since we can set the parameters to their default values in unit_init(), I think this (3) is not only useless but harmful.

Note:
The workaround I wrote in the first post even ignores the values the user set at the last time, so it is not a good one.

[AndrewCapon]

I have done some more testing, with two user OSCs:

First param A and Param B always get sent the same value as the encoders are set to.

The other 8 values do seem to be saved independently for each osc:

So for the following examples I have set up both Sup1 and Sup2 with some values, when I switch between them:

For Sup1 I get:

Sup1
unit_set_param_value(0, 100)
unit_set_param_value(1, 20)
unit_set_param_value(2, 28)
unit_set_param_value(3, 240)
unit_set_param_value(4, 100)
unit_set_param_value(5, 100)
unit_set_param_value(6, 100)
unit_set_param_value(7, 11)
unit_set_param_value(8, 10)
unit_set_param_value(9, 12)

For Sup2 I get:

Sup2
unit_set_param_value(0, 100)
unit_set_param_value(1, 20)
unit_set_param_value(2, 6)
unit_set_param_value(3, -228)
unit_set_param_value(4, 1)
unit_set_param_value(5, 4)
unit_set_param_value(6, 10)
unit_set_param_value(7, 2)
unit_set_param_value(8, 2)

So good stuff there at least.

Now when I turn it off with Sup2 selected, and turn back on I get:

Sup2
unit_set_param_value(0, 100)
unit_set_param_value(1, 100)
unit_set_param_value(2, 6)
unit_set_param_value(3, -228)
unit_set_param_value(4, 1)
unit_set_param_value(5, 4)
unit_set_param_value(6, 10)
unit_set_param_value(7, 2)
unit_set_param_value(8, 2)
unit_set_param_value(9, 2)

So that's good again, if I now select Sup1 though it is all 0:

unit_set_param_value(0, 100)
unit_set_param_value(1, 100)
unit_set_param_value(2, 0)
unit_set_param_value(3, 0)
unit_set_param_value(4, 0)
unit_set_param_value(5, 0)
unit_set_param_value(6, 0)
unit_set_param_value(7, 0)
unit_set_param_value(8, 0)
unit_set_param_value(9, 0)

So I'm guessing that it can store multiple oscs params somewhere in ram, you can change between oscs and you get the right params.

When you power cycle it though, only the last selected OSC gets it saved params, the others get zeros!

[dukesrg]

So all in all, is it working as intended:

1. user init values
2. set param values restored only for the active oscillator on power on
3. 1st render call
   ???

[AndrewCapon]

Nope, I think it is buggy as hell!

1. The default parameter values are never ever used, in the drumlogue these are used to init the module when it is first selected.

2. When turning the unit off and on, only the params for the currently selected user osc are saved.

So when the unit is turned on, they either need to load its params properly, or use the default values set in the params. Not just set everything to 0!

[AndrewCapon]

Actually, looking at my old drumlogue code it looks like they don't set the params from the defaults as I found code doing it in my Init()!

So I guess the only way around the zeros at the moment is as @boochow says, just ignore the initial set of params being sent.

[AndrewCapon]

So, this hack code seems to work.

If "valid" parameters are found (at least one not 0) then it updates the real params, otherwise it thows them away so the ones set in init() don't get overridden.

__unit_callback void unit_set_param_value(uint8_t id, int32_t value) {
	static bool bInitialParams = true;
	static bool bAnySet = false;

	static int32_t uValues[10];
	if(bInitialParams)
	{
		uValues[id] = value;

		if(id > 1 && value != 0)
			bAnySet = true;

		if(id == 9)
		{
			bInitialParams = false;
			if (bAnySet)
			{
				for(uint8_t u=0; u<10; u++)
					s_synth_instance.setParameter(u, uValues[u]);
			}
		}
	}
	else
		s_synth_instance.setParameter(id, value);
}

Less readable but maybe better:

__unit_callback void unit_set_param_value(uint8_t id, int32_t value) {
	static bool bInitialParams = true;
	static bool bAnySet = false;

	static int32_t uValues[10];
	if(bInitialParams)
	{
		uValues[id] = value;

		if(id > 1 && value != 0)
			bAnySet = true;

		bInitialParams = !(bAnySet || id==9);

		if(bAnySet)
		{
			for(uint8_t u=0; u<=id; u++)
				s_synth_instance.setParameter(u, uValues[u]);
		}
	}
	else
		s_synth_instance.setParameter(id, value);
}

If the number of params !=10 probably needs adjusting.

[dukesrg]

Drumlogue has an option to recall the program - on that action the parameter values are restored.Prette the same mk2 should restore the values for the last oscillator on power on as it is expected to produce the same sound it was last used.

[AndrewCapon]

There is another issue.

If your unit is the last in the list and the user turns the TYPE encoder to select the next one that doesn't exist unit_set_param_value() is called for every param with a value of 0!

Init() is not called so we can't even attempt to catch and mend this one.

[AndrewCapon]

OK, another hack that "fixes" both issues with the zeros:

__unit_callback void unit_set_param_value(uint8_t id, int32_t value) {
	// there are two states where we can get params as 0
	// 1. When the nts1 is turned off it seems to save the params of the currently selected OSC
	//    If then the user changes to this OSC, Param 0 and 1 will have the current pot value, 2-9 will all be 0s
	// 2. When this osc is the last in the list and the user turns the encoder to the right, all params are set to 0

	static int32_t uNextIndexToCheck = 0;
	static int32_t uValues[10];


	// save params, if ids values are 0 then advance index check, otherwise reset
	if(uNextIndexToCheck == id)
	{
		uValues[id] = unit_get_param_value(id);
		if(id < 2 || value == 0.0f)
			uNextIndexToCheck++;
	}
	else
		uNextIndexToCheck = 0; // reset

	// set the value
	s_synth_instance.setParameter(id, value);

	// If we get to param 10 we need to revert the params
	if(uNextIndexToCheck == 10)
	{
		// Params 2-9 are 0 so revert all params
		for(uint8_t u=0; u<10; u++)
			s_synth_instance.setParameter(u, uValues[u]);

		// reset index check
		uNextIndexToCheck = 0;
	}
}

[AndrewCapon]

And another issue:

The Korg librarian when receiving "programs" from the NTS1 doesn't call unit_get_param_value() for each parameter, it just retrieves the values for params that the NTS1 used when it called unit_set_param_value() previously.

So if the NTS1 called unit_set_param_value() with 0, that is what you get in the program retrieved. So unless the user changes every parameter manually then the "programs" are not correct. Any params you set to default values are lost!

[boochow]

I tested firmware v1.2 and can confirm that this issue has been fixed.