Audio stops working when repeatedly creating/disconnecting AudioConnections

Status
Not open for further replies.

mattybrad

Member
Hi all, I've found some behaviour which is either a bug, something stupid I've done with pointers, or just me pushing the audio library past its limits!

I've been designing a modular synth where you can change the signal path between various Teensy audio modules during runtime. Rather than statically declaring AudioConnections at the start of the code (the way the online GUI handles AudioConnections), I have been creating new AudioConnections during runtime, then using the disconnect() function when they are no longer required. This has been working brilliantly on the Teensy 3.6, but I'm currently working on a stripped-down design for the Teensy 3.2 and I've realised that adding and removing several AudioConnection objects causes the audio to either go very weird or stop working completely.

Here is a test sketch to demonstrate the problem:

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

AudioControlSGTL5000 sgtl;
AudioOutputI2S mainOutput;
AudioSynthWaveform sineWave;
AudioConnection* connection;

void setup() {
  AudioMemory(25);
  sgtl.enable();
  sgtl.volume(0.2);
  sineWave.begin(0.5,440,WAVEFORM_SINE);
}

// ideally this loop should generate a sine wave which is switched rapidly on and off forever
// instead, it drops out after a few seconds
void loop() {
  // create connection between sine wave and main output  
  connection = new AudioConnection(sineWave,0,mainOutput,0);
  delay(100);
  
  // break the connection
  connection->disconnect(); 
  delay(100);
}

The audio output works as expected for a little while, then becomes glitchy/silent. My best guess is a memory problem, since increasing AudioMemory allows the sketch to work for longer (this is also why I didn't notice a problem on the Teensy 3.6, where I could allocate more memory). I've tried deleting/nullifying my connection pointer, but this didn't seem to do anything. I've also tried fiddling with the core Teensy libraries, but I'm out of my depth there - the connect() and disconnect() functions are on line 202 and 236 here https://github.com/PaulStoffregen/cores/blob/master/teensy3/AudioStream.cpp in case that helps speed up any detective work.

If anyone could possibly confirm this behaviour or suggest a possible solution, I would be extremely grateful - my synth project depends on it!
 
You are doing a weird/silly thing, which is creating a new AudioConnection (and eat up new memory) with each loop iteration until the RAM is full.

You should initialize connection = new AudioConnection(sineWave,0,mainOutput,0); within setup().

In the loop, you'd then use connection.connect() and connection.disconnect() without creating new connections and thus without allocating superfluous memory.

Real life example: Each time the ashtray of your car is full, do you put it aside and buy a new one? Or do you empty and re-use the old one? ;)
 
Last edited:
Thanks for the reply! Yes, that would solve the particular problem that I have shown in my example, but the problem is that I think I have to create new connections to do what I'm trying to do (which I may not have explained very well!). Here's a fuller explanation of what I'm trying to achieve:

I have a selection of audio objects, for instance: an oscillator, white noise, a filter effect, and a mixer. I want to be able to connect (and disconnect) the inputs and outputs of these audio objects in various combinations during runtime, according to user input. For instance, a user might connect the oscillator output to the filter for a few seconds, then decide to disconnect it and connect the oscillator to the mixer instead. Here's the project page, in case that makes it clearer: https://www.pjrc.com/polymod-polyphonic-digital-modular-synth/

The problem is, as I increase the number of objects, I can't initialise all the connections in the setup() function, because there are so many possible connections. So, I suppose my question really is: is there a way to dynamically create a connection, then later disconnect it and fully destroy the connection so that it doesn't take up any memory after it has been disconnected?
 
Creating (and destroying) objects dynamically during runtime is extremely dangerous on embedded systems with limited RAM. There is the theoretic opportunity du use delete for object pointers, but the memory would not be de-allocated until you have an explicit destructor function for that object which would care about freeing up the RAM.

That's why you will rarely find such functionality in Arduino, Teensy, and similar libraries. And, though, there are lots of people who mess up their memory while handling String objects instead of char arrays...

Instead of simulating real life patch cords with connect() and disconnect() you could use mixer objects as a kind of crossbar switches between your oscillators, effects, and so on and enable/disable signal routes simply by adjusting the corresponding mixer channel amplitude or by setting it to 0.

Or you fork the audio library and you add code to destroy AudioConnection objects in a clean way. Once tested and approved, I'm sure that Paul will then accept your pull request.
 
Creating (and destroying) objects dynamically during runtime is extremely dangerous on embedded systems with limited RAM.

Yes, sadly I come from the world of Javascript, and haven't managed to rid myself of those tendencies! I was aware while writing the code for my synth that RAM usage might be an issue, but the Teensy 3.6 never complained and just kept working, so I pushed on.

Instead of simulating real life patch cords with connect() and disconnect() you could use mixer objects as a kind of crossbar switches between your oscillators, effects, and so on and enable/disable signal routes simply by adjusting the corresponding mixer channel amplitude or by setting it to 0.

Thanks, that's a really interesting suggestion. It won't work for my main synth project (where even the types of audio object are unknown until runtime!) but it's a nice solution for my stripped-down version.

Or you fork the audio library and you add code to destroy AudioConnection objects in a clean way.

I was afraid this might be the answer! If anyone has an ideas on how to do this then I'd be very grateful, otherwise I guess I'm in for some serious coding :)
 
An interesting development! I've just tried changing the code so that the connection is declared statically, and the behaviour is exactly the same. Here's the code I've just tried:

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

AudioControlSGTL5000 sgtl;
AudioOutputI2S mainOutput;
AudioSynthWaveform sineWave;
AudioConnection connection(sineWave,0,mainOutput,0);;

void setup() {
  AudioMemory(25);
  sgtl.enable();
  sgtl.volume(0.2);
  sineWave.begin(0.5,440,WAVEFORM_SINE);
}

// ideally this loop should generate a sine wave which is switched rapidly on and off forever
// instead, it drops out after a few seconds
void loop() {
  connection.disconnect(); 
  delay(100);
  connection.connect();
  delay(100);
}
 
@Paul:
I've had a quick scan through the AudioConnection code and one thing that looks wrong is in disconnect(). It doesn't fix the bug, but shouldn't there be an __enable_IRQ before the return?
Code:
void AudioConnection::disconnect(void)
{
	AudioConnection *p;

	if (!isConnected) return;
	if (dest_index > dst.num_inputs) return;
	__disable_irq();
	// Remove destination from source list
	p = src.destination_list;
	if (p == NULL) {
          // >>> Shouldn't this be here? __enable_irq();
		return;

Pete
 
FWIW: The problem is directly related (somehow) to the number of audio buffers allocated with AudioMemory.
Change the two delays to 200 so that it slows the beeping down enough that they can be counted. If AudioMemory is set to N, the code will beep correctly N-1 times, then it will buzz once and then die.
Still digging :)

Pete
 
Kinda sounds like it's leaking one of the buffers.

This disconnect function was a contribution I accepted on github, but the only testing I did before clicking the Merge button was that it didn't break the existing examples (which don't use it).
 
I have a fix. The disconnect() was not properly releasing a used audio buffer.
In AudioStream.cpp change the disconnect() function to this:
Code:
void AudioConnection::disconnect(void)
{
	AudioConnection *p;

	if (!isConnected) return;
	if (dest_index > dst.num_inputs) return;
	__disable_irq();
	// Remove destination from source list
	p = src.destination_list;
	if (p == NULL) {
//>>> PAH re-enable the IRQ
		__enable_irq();
		return;
	}
	/*else <<< Don't need an else here */
	if (p == this) {
		if (p->next_dest) {
			src.destination_list = next_dest;
		} else {
			src.destination_list = NULL;
		}
	} else {
		while (p) {
			if (p == this) {
				if (p->next_dest) {
					p = next_dest;
					break;
				} else {
					p = NULL;
					break;
				}
			}
			p = p->next_dest;
		}
	}

//>>> PAH release the audio buffer properly
	//Remove possible pending src block from destination
	if(dst.inputQueue[dest_index] != NULL) {
		AudioStream::release(dst.inputQueue[dest_index]);
		// release() re-enables the IRQ. Need it to be off a little longer
		__disable_irq();
		dst.inputQueue[dest_index] = NULL;
	}
	//Check if the disconnected AudioStream objects should still be active
	src.numConnections--;
	if (src.numConnections == 0) {
		src.active = false;
	}

	dst.numConnections--;
	if (dst.numConnections == 0) {
		dst.active = false;
	}

	isConnected = false;

	__enable_irq();
}


I tested it with this code which allocates only 2 buffers and generates a 50ms tone in stereo.
Code:
// See https://forum.pjrc.com/threads/54760
#include <Audio.h>
#include <Wire.h>
#include <SPI.h>
#include <SD.h>
#include <SerialFlash.h>

AudioControlSGTL5000 sgtl;
AudioOutputI2S mainOutput;
AudioSynthWaveform sineWave;
AudioConnection connection1(sineWave,0,mainOutput,0);
AudioConnection connection2(sineWave,0,mainOutput,1);

void setup() {
  AudioMemory(2);
  sgtl.enable();
  sgtl.volume(0.2);
  sineWave.begin(0.5,440,WAVEFORM_SINE);
}

// This loop generates a stereo sine wave which is switched rapidly on and off forever

void loop() {
  connection1.disconnect();
  connection2.disconnect();
  delay(50);
  connection1.connect();
  connection2.connect();
  delay(50);
}

Pete
 
Was going to try setting this fix as a git PR but when I try to clone the Teensy core libraries I get a fatal SSL error:
Code:
$ git clone https://github.com/PaulStoffregen/cores.git
Cloning into 'cores'...
fatal: unable to access 'https://github.com/PaulStoffregen/cores.git/': SSL certificate problem: self signed certificate in certificate chain

This is on Windows 10 Pro.
Pete
 
Was going to try setting this fix as a git PR but when I try to clone the Teensy core libraries I get a fatal SSL error:
Code:
$ git clone https://github.com/PaulStoffregen/cores.git
Cloning into 'cores'...
fatal: unable to access 'https://github.com/PaulStoffregen/cores.git/': SSL certificate problem: self signed certificate in certificate chain

This is on Windows 10 Pro.
Pete

It's possible to edit the files (and add a PR) directly on the github webpage. I do this sometimes, it's the fastest way.
 
@Frank B: Thanks Frank. I didn't know that. I'll give it a try sometime today.

@mattybrad: You're welcome :)

Pete
 
Hi people ! Really interesting thread, I'm very new to Teensy and also interested in "dynamic patching" - So, if I understand well, thanks to Pete's fix, the disconnect() being ok we can now use new and delete (which calls disconnect) and safely create connections between effects or sound generators on the fly ?

Because it's exactly what I plan to do with my Teensy 3.6 + audio board : have several effects and disconnect / connect them on the fly
 
Status
Not open for further replies.
Back
Top