Issues with tonesweep in audio library

[Pio]

Hello Everyone!
My first post, and a bug + solution report
I have been working on a Teensy3.1 + Audio board based test signal generator oriented towards building music instruments and various effects. You can see and here the first concept testing here:

While doing this i encountered two bugs in the synth_tonesweep.cpp component.

The first one is allowing the t_time variable to be zero, which results in divide by zero operation and makes the sweep hang - it never stops.

line #57 in synth_tonesweep.cpp

if(t_time < 0)return false;

should be changed to

if(t_time <= 0)return false;

The second one is related to negative sweeps, when the tone_hi is less than tone_lo.
The problem shows up when the sweep is quite fast, resulting in a higher tone_incr value, which might end up being more than the low threshold (or start frequency).
It may lead to tone_freq underflow, as it is an unsigned type variable and never meeting the (tmp < tone_hi) stop condition.

Here is my proposed solution:
starting from line #107 in synth_tonesweep.cpp

      if(tone_sign > 0) {
        if(tmp > tone_hi) {
          sweep_busy = 0;
          break;
        }
        tone_freq += tone_incr;
      } else {
          if(tmp < tone_hi || tone_freq < tone_incr){        //we need to check if the actual current frequency
                                                             //is not less than the freq step, too
              sweep_busy = 0;                                //otherwise tone_freq will underflow...
              break;
          }
          tone_freq -= tone_incr;                            //...right here
      }

The last one is not an issue, but an upgrade.
I wanted to display the currently generated frequency. For that i added a short function getFreq().

In synth_tonesweep.h add

int getFreq(void);

in the public section.
The body of the function is placed in synth_tonesweep.cpp

int AudioSynthToneSweep::getFreq(void)
{
    return(tone_freq>>32);
}

Ie, having the start and stop frequencies + the current sweep position it is possible to display it as a nice bargraph.

Cheers!
--
Piotr

 

[PaulStoffregen]

Any chance you could post a couple small test programs? That'd save me some time having to cobble them together myself to test before & after applying these changes.

 

[Pio]

Here it is. Just comment/uncomment the #defines to start a test for zero time or a quick reverse sweep test.
It is just a tonesweep connected to both DACs on the codec chip. The loop tests if isPlaying is set and prints it via 115200baud uart.
The 2nd test is clearly audible, too. The sweep bounces back and forth instead of ending at the lower threshold frequency.

Hope that helps!

#include <Audio.h>
#include <Wire.h>
#include <SPI.h>
#include <SD.h>
#include <SerialFlash.h>

// comment/uncomment to perform the test
// Teensy3.1/3.2 + Audio adaptor board - SGTL5000

#define TEST_ZERO_TIME
//#define TEST_REV_SWEEP

// GUItool: begin automatically generated code
AudioSynthToneSweep      tonesweep;     //xy=944,253
AudioOutputI2S           i2s;           //xy=1124,256
AudioConnection          patchCord1(tonesweep, 0, i2s, 0);
AudioConnection          patchCord2(tonesweep, 0, i2s, 1);
AudioControlSGTL5000     sgtl5000_1;     //xy=1123,310
// GUItool: end automatically generated code

void setup() 
{
    Serial.begin(115200);
    delay(100);
    AudioMemory(10);
    sgtl5000_1.enable();
    sgtl5000_1.volume(0.5);

//test using time_ms value 0
#ifdef TEST_ZERO_TIME
    tonesweep.play( 1, 20, 20000, 0);    
#endif

// test using a quick reversed sweep
#ifdef TEST_REV_SWEEP
  tonesweep.play( 1, 20000, 20, 0.1);  
#endif


}

void loop() 
{

  if (tonesweep.isPlaying())
  {
    Serial.println("Sweep still going...");
    delay(500);
  }
}

### EDIT ###
Found another one. Compared to the synth_waveform component, the tonesweep produces only half of the output level when the amplitude is set to one.
synth_tonesweep.cpp line #62

tone_amp = t_amp * 32767.0;

tone_amp is a 15 bit value, but when applied to the DDS output signal, the right shift is 16bit, resulting in 50% output level max:
(line #102)

 *bp++ = (short)(( (short)(arm_sin_q31((uint32_t)((tone_phase >> 15)&0x7fffffff))>>16) *tone_amp) >> 16);

Changing the last 16 to 15 brings it back to the full scale.

 

[PaulStoffregen]

I've committed these improvements. Thanks.

https://github.com/PaulStoffregen/Audio/commit/88535e1164a290dcb5133c417e4b58991d4a7db7