Dynamic Audio Connection Bug? A new one!

Status
Not open for further replies.

jkoffman

Well-known member
Hi all,

I'm back playing with some dynamic audio connections. The ones I used to allow swapping left/right channels work great, and are in fact working well within this very project. I've tried to add another one and...it's just not working.

Code:
  pMixLTCANA1_0_ampLTCANAO1_0 = new AudioConnection(mixer_LTC_ANA_1, amp_LTC_AnaOut1_0); // regular LTC 
  pMixLTCANA1_0_ampLTCANAO1_0 -> disconnect();  

  pMixAUDANA1_0_ampLTCANAO1_0 = new AudioConnection(mixer_AUD_ANA_1, amp_LTC_AnaOut1_0); // Audio   
  pMixAUDANA1_0_ampLTCANAO1_0 -> disconnect();

This code should create a patch, disconnect it, then do it again for a different source to the same input on an amp block. It's not working as I'm expecting it to though.

I expect that at the end of the these 4 lines there should be two dynamic connections that are inactive. What actually happens is the first one (with the LTC comment) is active, even though it's been disconnected. Swapping the order in which these are connected (Audio first, then LTC) results in no change, LTC is still active. Commenting things out for testing shows that both connections work individually, but that nothing seems to disconnect them.

I'm stumped. I have working dynamic connections in this very project, but these ones aren't working properly at all.
 
Your can try using
Code:
delete pMixLTCANA1_0_ampLTCANAO1_0;
Instead of disconnect
The delete calls the AudioConnection deconstructor
Which internally also calls disconnect
 
Maybe you've found a bug in the audio library? Or maybe it's just a mistake or misunderstanding with your code?

First, please make sure you're using Teensyduino 1.53 or 1.54-beta. In Arduino, click Help > About to check the version.

If this is a previously unknown audio library bug, I can assure you absolutely no investigation is going to happen unless you provide a small but complete test program which demonstrates the problem. The 4 lines of code you've shown aren't enough. A complete program which can be copied into Arduino and uploaded onto a Teensy to reproduce the problem is required.

Please, do not just give me a link to a huge project. Create a relatively small test program which just demonstrates the problem.
 
Your can try using
Code:
delete pMixLTCANA1_0_ampLTCANAO1_0;
Instead of disconnect
The delete calls the AudioConnection deconstructor
Which internally also calls disconnect

Thanks for the reply! Back when I was looking at this last time, I remember there was a thread that talked of a bug that deleting and recreating connections would eventually crash the system. Naturally I can't find that original thread anymore.

Nevertheless, I tried using delete and it failed as well.

Luckily, while trying to find that thread I found my original thread talking about this. I feel like a bit of an idiot, I should have remembered what I had seen in the past. Here's the link: https://forum.pjrc.com/threads/62166-Dynamic-Audio-Connection-Bug

Basically, the issue is that you can't disconnect or delete a dynamic connection when it shares a node with a connection that isn't being disconnected or deleted. In my current case, the output node of mixer_LTC_ANA_1 goes to several places. Since I'm only trying to modify one of them, it just...doesn't seem to do anything. This is consistent with what I was seeing earlier in the year, and something I should have remembered. My apologies.

Paul - I do still think this is a bug. I will try to come up with a simple test program that demonstrates the issue easily so you can test. I hope the basic description here makes some sense so you can at least start to understand what I'm seeing.

Thanks!
 
Ok! I have come up with a test file that appears to show the issue. I don't think I can attach it directly, so here's a link to the .ino:

https://www.dropbox.com/s/nbu17ni5niwdpg0/DynamicPatchTest_v1.ino?dl=0

I've set up a simple system with I2S input, output, a mixer, and a sine wave generator.

There are a few options you can comment/uncomment to test and cause the issue to appear. Surprisingly, I found that things weren't exactly as I thought they were. Start by uncommenting patchcords 1 and 2, just to check your hardware is working. Then comment them out, and you'll hear the left output toggling on and off while the right output is just off. So far this all makes sense. Now the issue:

Uncomment patchcord 3. This lets the right side of the I2S input into a mixer, which goes into the right side of the I2S output. Suddenly the audio is constant output on both channels. I believe the audio is correctly following the left/right inputs (ie not being combined somehow).

Comment patchcord 3 and uncomment patchcord 4. This feeds a sine wave into the same input on the mixer. Suddenly the audio is back to toggling. Just beware the sine wave noise especially if you're using headphones.

So there we go. Please let me know if I can offer any help, further info, or anything that might let other recreate this issue!
 
Hi all,

Just curious if anyone has been able to replicate what I'm seeing with these patches.

Thanks!
 
This bug never made it onto my to-investigate list, probably because the Dropbox link wasn't working. I tried again just now and it's not cooperating with my browser.

If anyone has a Dropbox account, can you please download this code and post it here on the forum message?
 
Code:
#include <Audio.h>      // Include the Teensy Audio Library
#include <Wire.h>       // Include the Teensy I2C Library 
#include <Metro.h>      // Include the Metro library for triggering functions based on time

AudioInputI2S            audioIn;           //xy=178,263
AudioOutputI2S           audioOut;           //xy=539,261
AudioMixer4              mixer1;         //xy=407,264
AudioSynthWaveformSine   sine1; 

// Uncomment these to test the system (just a straight passthrough)
//AudioConnection          patchCord1(audioIn, 0, audioOut, 0);
//AudioConnection          patchCord2(audioIn, 1, audioOut, 1);

// Uncomment this and audio will be constant
//AudioConnection          patchCord3(audioIn, 1, mixer1, 1);

// Uncomment this and audio toggles (as expected - but beware the sine noise)
//AudioConnection          patchCord4(sine1, 0, mixer1, 1);

// Leave this one connected
AudioConnection          patchCord6(mixer1, 0, audioOut, 1);

// Dynamic patchcord
AudioConnection*     pANAI0_ANAO0;

AudioControlSGTL5000     sgtl5000_1;     //xy=340,426
// GUItool: end automatically generated code


#define LED_HB  5 // Pin with LED for heartbeat

// Variables
int LED_HB_State = LOW;     // Var to hold state of heartbeat LED


// Instantiate a Metro object
Metro Int500msMetro = Metro(500);  // Set 500msMetro to return true every half second


// Functions Below
// ---------------

void audio_in_setup()     
{
  pANAI0_ANAO0 = new AudioConnection(audioIn, 0, audioOut, 0);

// Both of the following options work (depending on the other things connected)
//  pANAI0_ANAO0 -> disconnect();
//  delete pANAI0_ANAO0; // -> disconnect();
}


// Ignore this function for now - Not used
//void audio_in_toggle(void)  
//{
//  if(pANAI0_ANAO0)
//  {    
//    pANAI0_ANAO0 -> disconnect();
//    Serial.println("Disconnected");
//  }
//  else
//  {
//    pANAI0_ANAO0 -> disconnect();    
//    Serial.println("Connected");
//  }
//}

void audio_in_disconnect(void)  
{
    pANAI0_ANAO0 -> disconnect();
    Serial.println("Disconnected");
}

void audio_in_connect(void)  
{
    pANAI0_ANAO0 -> connect();
    Serial.println("Connected");
}


void setup() {
  
  Serial.begin(115200);
  delay(500);
  Serial.printf("Starting setup...\n");


  AudioMemory(12);  // Audio memory for audio library

  sgtl5000_1.enable();  // Enable the audio shield
  sgtl5000_1.inputSelect(AUDIO_INPUT_LINEIN); // Set the input to line in
  sgtl5000_1.volume(0.5);   // Initial volume level for headphone out

  audio_in_setup();

  sine1.amplitude(0.25);
  sine1.frequency(500);
  
  Serial.printf("Setup complete.\n");


}

void loop() {
 // 500msMetro object - a pseudo half second interrupt
  if (Int500msMetro.check() == 1) // Check if the object has passed its interval
  {
    // Heartbeat LED
    if (LED_HB_State == LOW)
    {
      LED_HB_State = HIGH;
      audio_in_connect();
    }
    else
    {
      LED_HB_State = LOW;
      audio_in_disconnect();
    }

    digitalWrite(LED_HB, LED_HB_State);

//    audio_in_toggle();
  }

}

I copied and pasted that from the DropBox.

Thank you!!!
 
@Paul - Here is the file.
Filename is 'DynamicPatchTest_v1.ino

Code:
#include <Audio.h>      // Include the Teensy Audio Library
#include <Wire.h>       // Include the Teensy I2C Library 
#include <Metro.h>      // Include the Metro library for triggering functions based on time

AudioInputI2S            audioIn;           //xy=178,263
AudioOutputI2S           audioOut;           //xy=539,261
AudioMixer4              mixer1;         //xy=407,264
AudioSynthWaveformSine   sine1; 

// Uncomment these to test the system (just a straight passthrough)
//AudioConnection          patchCord1(audioIn, 0, audioOut, 0);
//AudioConnection          patchCord2(audioIn, 1, audioOut, 1);

// Uncomment this and audio will be constant
//AudioConnection          patchCord3(audioIn, 1, mixer1, 1);

// Uncomment this and audio toggles (as expected - but beware the sine noise)
//AudioConnection          patchCord4(sine1, 0, mixer1, 1);

// Leave this one connected
AudioConnection          patchCord6(mixer1, 0, audioOut, 1);

// Dynamic patchcord
AudioConnection*     pANAI0_ANAO0;

AudioControlSGTL5000     sgtl5000_1;     //xy=340,426
// GUItool: end automatically generated code


#define LED_HB  5 // Pin with LED for heartbeat

// Variables
int LED_HB_State = LOW;     // Var to hold state of heartbeat LED


// Instantiate a Metro object
Metro Int500msMetro = Metro(500);  // Set 500msMetro to return true every half second


// Functions Below
// ---------------

void audio_in_setup()     
{
  pANAI0_ANAO0 = new AudioConnection(audioIn, 0, audioOut, 0);

// Both of the following options work (depending on the other things connected)
//  pANAI0_ANAO0 -> disconnect();
//  delete pANAI0_ANAO0; // -> disconnect();
}


// Ignore this function for now - Not used
//void audio_in_toggle(void)  
//{
//  if(pANAI0_ANAO0)
//  {    
//    pANAI0_ANAO0 -> disconnect();
//    Serial.println("Disconnected");
//  }
//  else
//  {
//    pANAI0_ANAO0 -> disconnect();    
//    Serial.println("Connected");
//  }
//}

void audio_in_disconnect(void)  
{
    pANAI0_ANAO0 -> disconnect();
    Serial.println("Disconnected");
}

void audio_in_connect(void)  
{
    pANAI0_ANAO0 -> connect();
    Serial.println("Connected");
}


void setup() {
  
  Serial.begin(115200);
  delay(500);
  Serial.printf("Starting setup...\n");


  AudioMemory(12);  // Audio memory for audio library

  sgtl5000_1.enable();  // Enable the audio shield
  sgtl5000_1.inputSelect(AUDIO_INPUT_LINEIN); // Set the input to line in
  sgtl5000_1.volume(0.5);   // Initial volume level for headphone out

  audio_in_setup();

  sine1.amplitude(0.25);
  sine1.frequency(500);
  
  Serial.printf("Setup complete.\n");


}

void loop() {
 // 500msMetro object - a pseudo half second interrupt
  if (Int500msMetro.check() == 1) // Check if the object has passed its interval
  {
    // Heartbeat LED
    if (LED_HB_State == LOW)
    {
      LED_HB_State = HIGH;
      audio_in_connect();
    }
    else
    {
      LED_HB_State = LOW;
      audio_in_disconnect();
    }

    digitalWrite(LED_HB, LED_HB_State);

//    audio_in_toggle();
  }

}

EDIT: He beat me to it:)
 
Ah I am so sorry Paul! I had no idea the link wasn't working for you!

I appreciate you looking into this when you have time! Happy to help in any way I can, I'm just not sure what to do.

Thanks!
 
fyi, this is behavior related to the subject I've noticed and just tested.
Code:
 AudioConnection*             patchCords[28];

Then I have 5 different patch cord arrangements, lets call them A,B,C,D,E.

I create the first patch cord arrangement, A, like this:

Code:
this->patchCords[0] = new AudioConnection(*this->envelope, 0, *this->mixer, 0);
.. repeat 27 times...

Then, when I want to switch arrangements, like to arrangement B, I do this:
Code:
for (int i = 0; i < 28; i++) {
    delete this->patchCords[i];    
  }
this->patchCords[0] = new AudioConnection(*this->amplifier, 0, *this->mixer, 0)
...repeat 27 times...

That tends to work fine, except, if I create the array of patch cords out of order. Example, if in arrangement C, I create the patches in this order [2], [5], [0], [1], [22] then I get some odd behavior. Arrangement C will work fine, but, if I switch from arrangement C to arrangement D, D will produces messed up audio, as if the patches are not arranged correctly. If I switch back to C, then back to D again, everything will be fine. It's only screwy the one time.

Anyway, I'm not asking for anything, just thought I'd share in case it's helpful to anyone else. I'm thinking about not using an array for patch cords and seeing if that makes a difference. But, as long as I keep the array of patch cords created in the same numeric order I have not had any issues.
 
There seems to be an error in the disconnect method, when unlinking an element from a linked list the pointer of the prevoius node most be shifted, so in AudioStream.cpp line 257

Code:
			if (p == this) {
				if (p->next_dest) {
					p = next_dest;
					break;
				} else {
					p = NULL;
					break;
				}
			}

should probably look like
Code:
			if (p->next_dest == this) {
 				p->next_dest = this->next_dest;
				break;
			}
 
Or even replace the while loop:
Code:
		while (p) {
			if (p == this) {
				if (p->next_dest) {
					p = next_dest;
					break;
				} else {
					p = NULL;
					break;
				}
			}
		}
by
Code:
		while (p->next_dest) {
			if (p->next_dest == this) {
 				p->next_dest = this->next_dest;
				break;
			}
			p = p->next_dest;
		}

[ the test for p being NULL has already been performed at this point ]
Personally I'd pull out the pointer chasing/manipulation into separate function(s) or method(s) for clarity
 
Personally I'd pull out the pointer chasing/manipulation into separate function(s) or method(s) for clarity
Agree - there's a lot of "whizz down lists doing stuff" that could do with cleaning up.

Also the red code here:
Code:
	if (p == NULL) {
//>>> PAH re-enable the IRQ
		__enable_irq();
		return;
	[COLOR="#FF0000"][B]} else if (p == this) {
		if (p->next_dest) {
			src.destination_list = next_dest;
		} else {
			src.destination_list = NULL;
		}[/B][/COLOR]
	} else {
		while (p) {
			if (p == this) {
				if (p->next_dest) {
					p = next_dest;
					break;
				} else {
					p = NULL;
					break;
				}
			}
			p = p->next_dest;
		}
	}
can simply be replaced by
Code:
[COLOR="#FF0000"][B]
} else if (p == this) {		
		src.destination_list = next_dest;
		}[/B][/COLOR]
 
Indeed yes, can be cleaned up, mostly to make the logic more clear, its not time critical code.
 
No worries - I hope this is just the start!

I just realised that I've actually changed the documented operation, whereby "If you attempt to connect many outputs to a single input, only the last connection will work.". With the new code, only the first connection will work. I hope it won't break existing applications, but it's easy to change it back to the documented behaviour, and (with a bit more effort) to make it disconnect the previous connection properly rather than leaving it "dangling".
 
Nice work


This got me thinkin, instead of using the "c++ template" audio Switch object that I made which adds extra latency,
It's now possible to create a Class that manages the different inputs/outputs in four arrays (could also use a struct to merge the object and the portindex):

AudioStream *output_objects[no];
int output_ports[no];
AudioStream *input_objects[ni];
int input_ports[ni];

That class and the array contents is then initialized using a special "Audio Design Tool" object.

Then when a connection should be made the class only need a function that takes two int parameters:
output wire index and input wire index

this would mean that predefined routings can be saved to a SD-card and then later simply be used.

The routing structure can then be edited in the special tool, this would save a lot of compile/upload(flash) times.

Here is a example that maybe reassembles the OPL-3 structure??
(note. the blue objects are called junctions and are only virtual objects and will not generate any extra code, they are used to clarify the design)

audioRouter.png

the arrays will then look like this:
Code:
output_objects[] = {mix0, mult0, waveformMod0, waveformMod1,waveformMod2,waveformMod3,mult1,mix1};
output_ports[] = {0,0,0,0,0,0,0,0};
input_objects[] = {waveformMod0,waveformMod1,mult0,mult0,mix0,mix0,mixOut,mixOut,mix1,mix1,mult1,mult1,waveformMod2,waveformMod3};
input_ports[] = {0,0,0,1,0,1,0,1,0,1,0,1,0,0};

which would mean that a audio connection can be made like this:
Code:
AudioConnection ac1(output_objects[2],output_ports[2],input_objects[6],input_ports[6]);
AudioConnection ac2(output_objects[3],output_ports[3],input_objects[7],input_ports[7]);
to route the waveformMod0 and waveformMod1 outputs to the mixOut input ports 0 and 1 respective.
 
I hope the basic AudioStream and Connection objects stays simple

+1 for that - it would break too much existing code to change the basics, hence my concern about "fixing" the documented misfeature: I don't know there isn't something out there that relies on it!

But there is definitely a use-case for saving settings to external storage and re-loading them without the need for re-compilation of the core application. The lowest level of this is making AudioStream and AudioConnection objects properly dynamic, as foreseen in Paul's roadmap page and raised in my https://forum.pjrc.com/threads/66840-Roadmap-quot-Dynamic-Updates-quot-any-effort-going-on thread.

@manicksan has clearly got right alongside the Design Tool and can export an audio system topology in a number of ways (including the one we already have), plus do a bunch of other stuff I haven't yet understood because I'm still getting my hands grubby at the lowest level of the code. Once I have it working "as now only better", then there's the opportunity to make the Design Tool interactive with a running Teensy, maybe figure out a way to "skin" the various control functions offered by the processing objects to make mapping to MIDI events, analogue inputs, quadrature encoders etc. a lot slicker than it it now, and so forth.

Cheers

Jonathan
 
Status
Not open for further replies.
Back
Top