I need help optimizing my code for a 2 oscillator synth

235 views
Skip to first unread message

Gianmaria Vasino

unread,
Mar 6, 2021, 1:04:38 PM3/6/21
to Mozzi-users
I've been experimenting with the mozzi library to create a 2 oscillator synth with a single lfo.

All works fine, but there is a clicking noise when playing.
After some research and experiments I found out it's because my Arduino UNO can't keep up with calculations.
Is there a way to optimize my code? Especially the keyboard input part that to me looks messy. I don't know of another way of making sure the buttons will work when held down.

To summarize my hardware setup:
  • x12 buttons (keyboard), assigned from pin 13 to pin 10 and from pin 8 to pin 1
  • x1 button (to switch the lfo function) assigned to pin 0
  • x5 potentiometer, assigned from pin A0 to pin A4. In order to control: octaves, waveform of aOsc, waveform of bOsc, volume mix between aOsc and bOsc and lfo frequency.
This is my code:

#include <MozziGuts.h>
#include <Oscil.h>
#include <PinChangeInt.h>
#include <tables/saw2048_int8.h>
#include <tables/triangle_dist_cubed_2048_int8.h>
#include <tables/square_no_alias_2048_int8.h>
#include <tables/sin256_int8.h>

#define CONTROL_RATE 128

Oscil <2048, AUDIO_RATE> aOsc(TRIANGLE_DIST_CUBED_2048_DATA);
Oscil <2048, AUDIO_RATE> bOsc(TRIANGLE_DIST_CUBED_2048_DATA);
Oscil <256, CONTROL_RATE> cOsc(SIN256_DATA); //lfo

#define C 523.25
#define Db 554.37
#define D 587.33
#define Eb 622.25
#define E 659.26
#define F 698.46
#define Gb 739.99
#define G 783.99
#define Ab 830.61
#define A 880
#define Bb 932.33
#define B 987.77

volatile int timer = 0;
volatile int timer2 = 0;

int button=1;

float freq;
int oct=1;
int waveA;
int waveB;
int lfoFreq = 0;
float detune = 0;
float ampMod = 1;

float volumeA, volumeB;
int sum;
int out;

int knob1;
int knob2;
int knob3;
int knob4;
int knob5;

volatile bool sw = false;

void setup()
  pinMode(13, INPUT_PULLUP);
  pinMode(12, INPUT_PULLUP);
  pinMode(11, INPUT_PULLUP);
  pinMode(10, INPUT_PULLUP);
  pinMode(8, INPUT_PULLUP);
  pinMode(7, INPUT_PULLUP);
  pinMode(6, INPUT_PULLUP);
  pinMode(5, INPUT_PULLUP);
  pinMode(4, INPUT_PULLUP);
  pinMode(3, INPUT_PULLUP);
  pinMode(2, INPUT_PULLUP);
  pinMode(1, INPUT_PULLUP);

  pinMode(0, INPUT_PULLUP);

  pinMode(A0, INPUT);
  pinMode(A1, INPUT);
  pinMode(A2, INPUT);
  pinMode(A3, INPUT);
  pinMode(A4, INPUT);

  PCintPort::attachInterrupt(0, lfoSwitch, RISING);

  PCintPort::attachInterrupt(13, lfoReset, RISING);
  PCintPort::attachInterrupt(12, lfoReset, RISING);
  PCintPort::attachInterrupt(11, lfoReset, RISING);
  PCintPort::attachInterrupt(10, lfoReset, RISING);
  PCintPort::attachInterrupt(8, lfoReset, RISING);
  PCintPort::attachInterrupt(7, lfoReset, RISING);
  PCintPort::attachInterrupt(6, lfoReset, RISING);
  PCintPort::attachInterrupt(5, lfoReset, RISING);
  PCintPort::attachInterrupt(4, lfoReset, RISING);
  PCintPort::attachInterrupt(3, lfoReset, RISING);
  PCintPort::attachInterrupt(2, lfoReset, RISING);
  PCintPort::attachInterrupt(1, lfoReset, RISING);

  startMozzi(CONTROL_RATE);
}

void updateControl()
{
  knob1 = mozziAnalogRead(A0);
  oct  = (knob1/204)+1;
  if(oct%2!=0 && oct>1)
  {
    oct++;
  }

   waveA = mozziAnalogRead(A1);
 
 if(waveA<=511)
  {
    aOsc.setTable(TRIANGLE_DIST_CUBED_2048_DATA);
  }
 else if(waveA>511)
  {
    aOsc.setTable(SQUARE_NO_ALIAS_2048_DATA);
  }

  waveB = mozziAnalogRead(A2);
  
  if(waveB<=511)
  {
    bOsc.setTable(TRIANGLE_DIST_CUBED_2048_DATA);
  }
 else if(waveB>511)
  {
    bOsc.setTable(SAW2048_DATA);
  }

    knob4 = mozziAnalogRead(A3);
    volumeB = (float)knob4/1023;
    volumeA = 1 - volumeB;

    knob5 = mozziAnalogRead(A4);
    lfoFreq = ((float)knob5/51);
    cOsc.setFreq(lfoFreq);    //lfo
    
    button=1;
    if(digitalRead(13) == 0)
    {
      freq=C;
      button  =  0;
    }
    else if(digitalRead(12) == 0)
    {
      freq=Db;
      button =  0;
    }
    else if(digitalRead(11) == 0)
    {
      freq=D;
      button =  0;
    }
    else if(digitalRead(10) == 0)
    {
      freq=Eb;
      button =  0;
    }
    else if(digitalRead(8) == 0)
    {
      freq=E;
      button =  0;
    }
    else if(digitalRead(7) == 0)
    {
      freq=F;
      button =  0;
    }
    else if(digitalRead(6) == 0)
    {
      freq=Gb;
      button =  0;
    }
    else if(digitalRead(5) == 0)
    {
      freq=G;
      button =  0;
    }
    else if(digitalRead(4) == 0)
    {
      freq=Ab;
      button =  0;
    }
    else if(digitalRead(3) == 0)
    {
      freq=A;
      button =  0;
    }
    else if(digitalRead(2) == 0)
    {
      freq=Bb;
      button =  0;
    }
    else if(digitalRead(1) == 0)
    {
      freq=B;
      button =  0;
    }
    else if (button!=0)
    {
      freq=0;
    }

    //lfo selector
    if(!sw)
    {
      detune = ((float)cOsc.next()/(128))*freq;//for pitch control with lfo
      if(lfoFreq<=0)
      {
        detune = 0;
      }
       ampMod=1;
    }
   if(sw)
    {
      detune = 0;
      ampMod = 1 - ((float)cOsc.next()/(128));
      if(lfoFreq<=0)
      {
        ampMod = 1;
      }
    }
  
  aOsc.setFreq(freq*oct+detune);
  bOsc.setFreq((freq*oct-detune)/2);
}

int updateAudio(){
  out = aOsc.next()*volumeA+bOsc.next()*volumeB;
  return out*ampMod;
}

void loop(){
  audioHook();
}

void lfoSwitch()
{
  if((millis()-timer)>200)
  {
    sw = !sw;
    timer = millis();
  }
}

void lfoReset()
{
  if((millis()-timer2)>200)
  {
    cOsc.setPhase(0);
    timer2 = millis();
  }
}

Tim Barrass

unread,
Mar 7, 2021, 7:01:43 AM3/7/21
to mozzi...@googlegroups.com
Hi Gianmaria,

a couple of general things in your code that can be optimised for speed are:
- floats: try to avoid them where you can... try to have no floats at all in updateAudio().  If the only thing you do is change the float calculations to integer arithmetic in updateAudio(), you could make a big difference to processor load.  Use integer arithmetic and bit shifting.  So volume can be expressed as 0-255, then shifted >>8 after multiplying, like in many of the Mozzi examples.  There is some support for fixed-point arithmetic in Mozzi, which is useful if you really need it.
- try to only do things once when the state of a knob or switch or conditional changes, not every time updateControl() runs or every time an analog input is read eg. no need to set tables every time unless the selection has changed.


--
You received this message because you are subscribed to the Google Groups "Mozzi-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to mozzi-users...@googlegroups.com.
To view this discussion on the web, visit https://groups.google.com/d/msgid/mozzi-users/e098d49d-1eaf-423b-9416-693ccca40efcn%40googlegroups.com.

Gianmaria Vasino

unread,
Mar 7, 2021, 7:40:35 AM3/7/21
to Mozzi-users
Thank you for your answer, but I still have a doubt:
How would you rewrite the code to make things only happen once when a knob is turned or a switch is pressed?

Staffan Melin (Oscillator)

unread,
Mar 7, 2021, 8:33:59 AM3/7/21
to mozzi...@googlegroups.com
You save the old value and compare the new value with the old. If they differ, update the old value = new value and make the changes.

Hth Staffan

elby...@gmail.com

unread,
Mar 7, 2021, 1:19:03 PM3/7/21
to Mozzi-users
When you implement what Staffan suggested, you'll need to think about what the word "differ" means.  You do NOT want to use equality (==) on a value coming from a potentiometer returning an analog value in the range (0, 1023).  If you're not already familiar with the concept, I'd suggest Googling the term "hysteresis", probably in conjunction with some subset of {"arduino," "knob," "control,"  "potentiometer"} to discover why this is a problem, and what to do about it.

tomco...@live.fr

unread,
Mar 7, 2021, 1:35:33 PM3/7/21
to Mozzi-users
Also, you may consider using "switch case" instead of this bunch of "else if" instructions : it will increase clearness and will be faster (or, at worst same speed) as it might implement it as a lookup!
Hope it helps!

Gianmaria Vasino

unread,
Mar 7, 2021, 2:20:36 PM3/7/21
to Mozzi-users
I had thought about using switch case but the problem is that, from what I know, it's only possible to compare a single variable to a bunch of values. In my case I have different pins that can have different values. So I don't see how it would be possible to use it.

tomcombriat

unread,
Mar 7, 2021, 2:29:23 PM3/7/21
to gian...@gmail.com, mozzi...@googlegroups.com
you can, for instance, add all your digital read in one variable, with a well chosen multiplier, different for each.
For instance, a = digitalRead(a1) + digital Read(a2)*2 etc...
The value of a will be unique for each case. To optimise a lot you can do these multiplications by shifting.
Note that it also allows you to handle several button press if you want.
I hope I'm clear, not so easy to type a good answer on a phone :/..



tomcombriat@live.fr
Envoyé depuis mon téléphone realme
You received this message because you are subscribed to a topic in the Google Groups "Mozzi-users" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/mozzi-users/i5lmQb3BulU/unsubscribe.
To unsubscribe from this group and all its topics, send an email to mozzi-users...@googlegroups.com.
To view this discussion on the web, visit https://groups.google.com/d/msgid/mozzi-users/dabb42c0-4490-4f89-bb94-f24e8ae66680n%40googlegroups.com.

Gianmaria Vasino

unread,
Mar 8, 2021, 1:56:57 PM3/8/21
to Mozzi-users
except I can't find numbers that generate unique cases for the different button presses. I don't think it really works this way :/
The digitalRead gives 1 as default when the button is not pressed in my case, if I multiply all those digitalRead I can press multiple buttons and have it registered as the press of only one. I tried with several number groups and I can't seem to find a number set that when added up with other numbers of the same set never results in a number already present in the group. I am sure there are several sets of numbers like these but it's a pain coming up with 12 numbers like that and for them to be not weird numbers to work with.
Isn't there a simpler solution? Using interrupts is awkward and not ideal for a button that needs to be held down, at least I think so.

tomcombriat

unread,
Mar 8, 2021, 2:31:05 PM3/8/21
to gian...@gmail.com, mozzi...@googlegroups.com
Powers of two would work:
1, 2, 4 etc.
Binary algebra works this way, there will be only one solution to every combination. You only need to define the cases you need, and fallback on another behaviour if none of fulfilled.
As I said previously, this multiplying can also be done by left shifting, further optimising your code.
As a side note, reading all digital pins every time ensure a constant load on the MCU which is always a good thing when working in real time!



tomcombriat@live.fr
Envoyé depuis mon téléphone realme
Reply all
Reply to author
Forward
0 new messages