Forum Rule: Always post complete source code & details to reproduce any issue!
Results 1 to 17 of 17

Thread: Audio stops working when repeatedly creating/disconnecting AudioConnections

  1. #1
    Member
    Join Date
    Jun 2018
    Location
    Oxford, UK
    Posts
    23

    Audio stops working when repeatedly creating/disconnecting AudioConnections

    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/co...udioStream.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!

  2. #2
    Senior Member+ Theremingenieur's Avatar
    Join Date
    Feb 2014
    Location
    Colmar, France
    Posts
    2,164
    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 by Theremingenieur; 01-02-2019 at 02:21 PM.

  3. #3
    Member
    Join Date
    Jun 2018
    Location
    Oxford, UK
    Posts
    23
    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-polypho...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?

  4. #4
    Senior Member+ Theremingenieur's Avatar
    Join Date
    Feb 2014
    Location
    Colmar, France
    Posts
    2,164
    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.

  5. #5
    Member
    Join Date
    Jun 2018
    Location
    Oxford, UK
    Posts
    23
    Quote Originally Posted by Theremingenieur View Post
    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.

    Quote Originally Posted by Theremingenieur View Post
    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.

    Quote Originally Posted by Theremingenieur View Post
    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

  6. #6
    Member
    Join Date
    Jun 2018
    Location
    Oxford, UK
    Posts
    23
    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);
    }

  7. #7
    Senior Member PaulStoffregen's Avatar
    Join Date
    Nov 2012
    Posts
    19,219
    Quote Originally Posted by mattybrad View Post
    Here's the code I've just tried:
    Ah, now that's a much better test case.

    Sorry, I can't look at this quickly, but I'm definitely putting it on my list of bugs to investigate & fix.

  8. #8
    Member
    Join Date
    Jun 2018
    Location
    Oxford, UK
    Posts
    23
    Quote Originally Posted by PaulStoffregen View Post
    Sorry, I can't look at this quickly, but I'm definitely putting it on my list of bugs to investigate & fix.
    No worries, thanks for having a look!

  9. #9
    Senior Member
    Join Date
    Nov 2012
    Posts
    1,002
    @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

  10. #10
    Senior Member
    Join Date
    Nov 2012
    Posts
    1,002
    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

  11. #11
    Senior Member PaulStoffregen's Avatar
    Join Date
    Nov 2012
    Posts
    19,219
    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).

  12. #12
    Senior Member
    Join Date
    Nov 2012
    Posts
    1,002
    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

  13. #13
    Senior Member
    Join Date
    Nov 2012
    Posts
    1,002
    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

  14. #14
    Senior Member+ Frank B's Avatar
    Join Date
    Apr 2014
    Location
    Germany NRW
    Posts
    5,245
    Quote Originally Posted by el_supremo View Post
    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.

  15. #15
    Member
    Join Date
    Jun 2018
    Location
    Oxford, UK
    Posts
    23
    Quote Originally Posted by el_supremo View Post
    I have a fix. The disconnect() was not properly releasing a used audio buffer.
    Thank you Pete, that's great! This forum is brilliant Within a few hours of reporting my issue I had both a great temporary workaround and a long-term fix - incredible!

  16. #16
    Senior Member
    Join Date
    Nov 2012
    Posts
    1,002
    @Frank B: Thanks Frank. I didn't know that. I'll give it a try sometime today.

    @mattybrad: You're welcome

    Pete

  17. #17
    Senior Member
    Join Date
    Nov 2012
    Posts
    1,002
    PR created. Thanks Frank.

    Pete

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •