Bug in PlaySynthMusic demo

h4yn0nnym0u5e

Well-known member
This code doesn't end cleanly: the last note off command is read, but before it gets acted on the stop command is prematurely fetched and acted on, rather than waiting for the next "note" (command, really) to be loaded.
Code:
  // read the next note from the table
  c = [COLOR="#0000FF"][B]*sp++[/B][/COLOR];
  opcode = c & 0xF0;
  chan = c & 0x0F;

  if(c < 0x80) {
    // Delay
    d_time = (c << 8) | *sp++;
    delay(d_time);
    return;
  }
  if([COLOR="#FF0000"][B]*sp[/B][/COLOR] == CMD_STOP) {
    for (chan=0; chan<10; chan++) {
      envs[chan]->noteOff();
      waves[chan]->amplitude(0);
    }
    Serial.println("DONE");
    while(1);
  }
The last line of the score contains:
Code:
0x81, 0x88, 0,12, 0x82,[COLOR="#0000FF"] 0x89[/COLOR], [COLOR="#FF0000"]0xf0[/COLOR]};
Inserting a delay after the last note off would cause the stop command to be entirely missed and score decoding would run off the end of the array...

This demo is nearly a good test platform, but it also suffers from:
  • Stop command enters a while (1) loop, so analysis of correct shutdown after the last note is hindered
  • pointless return statements in the other command-decoding if() blocks, which need to be removed if any analysis is to be done at intermediate points in the loop()
Cheers

Jonathan
 
The line
Code:
for (chan=0; chan<10; chan++) {
should be
Code:
for (chan=0; chan<0x10; chan++) {
or
Code:
for (chan=0; chan<16; chan++) {
:p
I knew that... :D

Except ... it should really be neither of those things. The hex constant may be what the original programmer intended, but forgot the 0x; but why should it be a hex constant, it's just a count. And 16 is just a guess, really: if someone came along and edited the array sizes (two more instances of the magic number 16, which have to match for correct operation...) but didn't notice this instance, the loop would still be wrong.

A slightly better piece of code would be to have:
Code:
#define MAX_CHAN 15 // MIDI channels run from 0 to 15
#define COUNT_OF(a) (sizeof a / sizeof a[0]) // minimal implementation: better versions exist
...
AudioSynthWaveform *waves[MAX_CHAN+1] = {...}
AudioSynthWaveform *envs[MAX_CHAN+1] = {...}
...

for (int i = 0; i<COUNT_OF(envs);i++)
{
...
}
This still might (depending on the code in the for() loop's body) make the assumption that COUNT_OF(envs) is the same as COUNT_OF(waves), which it would not be if someone decided to tweak the demo to make a two-oscillator version, but it's a bit better.

Part of the problem is that while the Audio library is good, it's not terribly helpful when it comes to robust implementation of a polyphonic synth, even a super-simple one like this...

Cheers

Jonathan
 
Jonathan:

...if someone decided to tweak the demo to make a two-oscillator version...

I would think that one obvious approach would be to add another dimension to the envs array (logical), rather than just mutiplying the number of envs (very confusing !!) when transitioning from one voice to multiple.

Part of the problem is that while the Audio library is good, it's not terribly helpful when it comes to robust implementation of a polyphonic synth, even a super-simple one like this...

I can't find the specific post right now, but somewhere in another post, @PaulStoffregen made the comment (paraphrasing) that the Teensy Audio Layout GUI & associated Audio library are intended simply for creating an audio path. And (in my [kd5rxt-mark] opinion) it does that very well. Responsibility for creating the logic for making use of that path is always on the designer/coder. Robust implementation of anything is even more so always on the designer/coder !!

Speaking from personal experience (creating my TeensyMIDIPolySynth - a 3-voice 16-poly mixed-multi-modulated-filtered-waveform synthesizer !!), without the Teensy Audio Layout GUI & the associated Audio library, I could *never* have created such a complex & capable synthesizer on my own. Creating the 16 audio paths, each with 3 voices & multiple weveforms, each individually selectable & adjustable, each with filtering & modulation on top of that would have been a coding nightmare, way beyond my understanding & capabilities !! The Teensy Audio Layout GUI & associated Audio library made it very easy & very understandable, facilitating the ability to start simple (monophonic, single voice, etc.) & adding/building as I went. I am extremely happy with the end result.

The house builder shouldn't blame his hammer if/when the completed house frame is not square !!

You might take a look at manicksan's implementation of an alternate Audio Layout GUI which is referenced <here>. He developed it specifically with the intent of aiding in the implementation of polyphonic capability. However, no matter which tool(s) you use, you are still responsible for all of the coding logic.

Mark J Culross
KD5RXT
 
Can't argue that the Audio library and GUI are great to get started from, and kudos to Paul for putting the effort in. He's got a business to run, and I wouldn't expect any more attention from him than to accept any bug-fixes supplied by the community.

But I would argue that we've been supplied with the hammer and left to build the house with it: no spades, rulers or set squares... (that's very unfair of me, but you get the drift). I can't look at your code (Google Drive wants me to compromise my 3rd-party cookies - nope), but another project I have looked at has a 15-element voice core, with about 25 global elements as sources, and about 8 downstream for final mix and effects. For a 12-voice synth, the definition file runs to nearly 1000 lines: to expand it to 16 voices, I'd estimate it would required 300 additional carefully-crafted lines just in the definition file. The code looks fine, there's a definition for how many voices there are (in another file with no guarantee it matches the audio definition file) and that's used throughout. But there's still line after line after line of code dedicated to just one synth topology: it would be a massive undertaking to add just one oscillator per voice, let alone change it to an FM synth, or make it multi-timbral.

I've looked at manicksan's improved GUI, and had some brief dialogue with him - definitely looks to be an improvement. I aim to continue that discussion when it's a bit clearer to me what might be of most use.

Part of the reason I looked at PlaySynthMusic was as a torture test for an upgrade to the Audio core, with a view to enabling construction and destruction of voices of different topologies at run time. Assuming I can get it robust, the above debate may become more relevant. Currently distracted trying to get the miditones utility to emit one MIDI channel per voice.

At the moment I 100% agree with you that it's possible to get some truly awesome results, with a very reasonable learning curve, and 110% agree it's the programmer's responsibility to get it right!

But better tools would be a great help.

Cheers

Jonathan
 
Back
Top