Thoughts and suggestions

[miguelfreitas]

Hi Urs,

I'd like to thank you for this great project! The components for my PCB have not yet arrived and I'm still manufacturing the board but I really appreciate the time you've spent designing, documenting, providing the kits and stuff.

In the meantime I've been reading the code and I've a couple of comments:

1) I think we would benefit from using LDAC (latch) for real. The current scheme seems IMHO specially worrisome due to the uncertainty introduced by these positive/negative multiplications

https://github.com/GaudiLabs/OpenTheremin_V3/blob/master/Open_Theremin_V3/ihandlers.cpp#L154

This conditional code path (and maybe others) adds "jitter" to the DAC sampling instant. Jitter on ADC/DAC is well known source of performance degradation.

What I had in mind was something pretty simple: just pulse LDAC on the beginning of the INT handler so it would get well tied to the external clock with minimum jitter. (delaying value by one sample won't make any difference imho)

2) Why haven't you used the hardware's SPI? It seems that the pins are correctly mapped to the SPI port but I see all the bit-banging done by software. Why is that?

IMHO we could save some processor cycles by sparing the bit-banging work to hardware. From your comments in code this seems like a non-negligible time (9.6 us) that we may use for something else...

So why am I bothering anyway? Because I'd like to experiment with generating a chord on opentheremin, which would require advancing, reading and summing different pointers over the wavetables. Since the current code seems quite tight on available CPU cycles I will probably try optimizing it first. So i'd like to ask you first if there are any design issues I'm not aware that could explain (1) or (2).

Thanks!

[GaudiLabs]

Hi @miguelfreitas Thank you for your suggestions. I think they both are totally legit and I guess I just did not do it for a lack of programming skills - as you see from the hardware side it was meant like this :-). I hope you get your components soon and I would be happy if you can implement it.

[miguelfreitas]

Great! I'll give it a try. You should hear from me again soon ;-)

[miguelfreitas]

Progress notes: I've been playing with the emulator only (since I still have no hardware to try) and I was able to reduce ISR (INT1_vect) total execution time from 18 to 11us! :-)

https://github.com/miguelfreitas/OpenTheremin_V3

As commited (USE_HW_SPI defined without USE_SPI_INTERRUPT) the execution time is only 2us faster (from 18us to 16us) but shouldn't require any changes to ihandlers.cpp except for pulsing the DAC Latch.

Anyway it is not ready for use. This is just to keep you updated.

[miguelfreitas]

I believe I've figured out why you've needed that switch(vWavetableSelector) inside the INT1 handler: the problem was the wavetables[] declaration that wrongly placed the vector into PROGMEM (instead of a vector of PROGMEM pointers as intended). So removing the switch and indexing the vector is actually faster.

https://github.com/miguelfreitas/OpenTheremin_V3/commit/cb2736b84b2ef8c806d41dfadc0450385af4e183

[Saijin-Naib]

Miguel, if you are able to enable SPI and cut down some loop times, do you think there would be a way to squeeze more "steps" out of the LUTs to enable finer-grained sound steps?

[miguelfreitas]

@Saijin-Naib You meant increasing the sample rate? Yes, I think it is possible.

But if you have in mind the rate the pitch/volume are acquired to actually changing the sound, I don't know.

[Saijin-Naib]

@Saijin-Naib You meant increasing the sample rate? Yes, I think it is possible. But if you have in mind the rate the pitch/volume are acquired to actually changing the sound, I don't know.

I was thinking more the wavetables themselves, which seem to have 1024 pitch values with a range of -2048 - 2048 for amplitude.

What would increasing the sample rate as you've stated do?

[Theremingenieur]

This pull request takes your suggestions into account, thank you @miguelfreitas! I didn’t use the AVR emulator, but a true Open.Theremin, which allowed me to test every little step also by ear while playing. I wrote my own little hardware SPI library for that, to reduce the overhead. Everything which is now sent to the DAC2 (pitch and volume tuning) is latched immediately after CS2 going up. For the DAC1 (audio), I implemented the latch pulse at the very beginning of each audio interrupt. That causes a 32us delay for the audio which is not noticeable but the effect of the jitter free sound can be immediately heard. Since I was already on that, I also modified the register switch code, so that there are not longer 4, but 3 positions which allow to play the usual (default) pitch range in center position and to transpose a clean octave up or down from that by turning right or left. This transposing algorithm has the advantage that the tone spacing and the pitch tuning is not longer affected by the position of the register pot. And finally, I upgraded the volume control from 8 to 16 bit by taking the 8bit volume value and applying a simple approximated e-function y = x^2 + 2x which maps the initial 0 to 255 range exactly to a 0 to 65535 range in an ear friendly way which allows a much more expressive playing. The audio interrupt takes now by default about 14us if not disturbed by another interrupt. The worst case I could see with timer interrupts occurring at the same time, was 22us. If you take into consideration that we are now doing a signed 16 x 16 multiplication instead of an unsigned 16 x 8 with an if/else to handle the sign, we might (for the moment) be satisfied.

[Theremingenieur]

About the wave table length and sampling rate: Both are linked in some way. The sample rate determines the highest reproducible frequency which is exactly 1/2 of f_sample, see the Nyquist theorem. With the actual sampling rate of 31.250kHz, that means that we can reproduce frequencies up to 15625Hz. The highest playable note on this instrument is 4168Hz, this is the C which is 4 octaves above middle C and corresponds to the highest key of a grand piano. Even with such a high note which rarely appears in written music, there is still enough headroom for a few harmonics. Thus, I don’t see the need for a higher sampling frequency for the moment. Modern analog theremins like the Moog Etherwave Standard have cut-off filters at around 14000Hz to prevent the sound from becoming too screechy, older tube theremins cut already around 3500Hz which gives them a warm and mellow sound. The wave table length (the number of samples) is responsible for the lowest playable frequency which is f_sample/n = 31250 / 1024 = 30.5Hz. That is a little lower than the C which is 3 octaves below middle C and thus even a third lower than the lowest (E)-String of a double bass. So, in the actual configuration of the wave table size in combination with the register switch, you get a range of 7 playable octaves out of this tiny instrument which is more than most analog theremins can cover. Again, from my point of view (as an engineer and as a musician), there is actually no need for changes on that side.

[miguelfreitas]

This pull request takes your suggestions into account, thank you @miguelfreitas! I didn’t use the AVR emulator, but a true Open.Theremin, which allowed me to test every little step also by ear while playing. I wrote my own little hardware SPI library for that, to reduce the overhead. Everything which is now sent to the DAC2 (pitch and volume tuning) is latched immediately after CS2 going up. For the DAC1 (audio), I implemented the latch pulse at the very beginning of each audio interrupt. That causes a 32us delay for the audio which is not noticeable but the effect of the jitter free sound can be immediately heard.

Thanks @Theremingenieur! I'm glad it helped. I've guessed the jitter-free sound might be noticeable, so thanks for confirming :-)

Since I was already on that, I also modified the register switch code, so that there are not longer 4, but 3 positions which allow to play the usual (default) pitch range in center position and to transpose a clean octave up or down from that by turning right or left. This transposing algorithm has the advantage that the tone spacing and the pitch tuning is not longer affected by the position of the register pot.

Very interesting idea!

Since I'm not musician I've only been playing with a crazy idea I don't know if it is useful, that is, using the timbre switch for changing "chords" (defined as ratios between simultaneous tones). As the musician probably won't have a third arm for changing it on-the-fly, I don't know if stucking into a single "chord" is of any use for real music, but maybe just for strange sound generation...

And finally, I upgraded the volume control from 8 to 16 bit by taking the 8bit volume value and applying a simple approximated e-function y = x^2 + 2x which maps the initial 0 to 255 range exactly to a 0 to 65535 range in an ear friendly way which allows a much more expressive playing.

Very nice. Do you know if other Theremins usually map the volume/pitches linear or exponentially?

The audio interrupt takes now by default about 14us if not disturbed by another interrupt. The worst case I could see with timer interrupts occurring at the same time, was 22us. If you take into consideration that we are now doing a signed 16 x 16 multiplication instead of an unsigned 16 x 8 with an if/else to handle the sign, we might (for the moment) be satisfied.

+1:

[Saijin-Naib]

@Theremingenieur , thanks so much for helping me understand, and for that amazing PR based upon @miguelfreitas ideas!

So to be clear, with your new PR merged, we would have finer volume steps while playing, but not necessarily finer pitch steps between our current min/max notes, correct? What controls that aspect?

[Theremingenieur]

Since one octave is doubling the frequency and two octaves quadrupling and so forth, pitch control is always exponential. With a Theremin, that is almost by design, because the df/dx increases when you come closer to the pitch antenna and thus produce higher frequencies. In analog Theremin designs, you’ve even to make a technical effort to flatten somewhat that exponential curve, so that the tone spacing doesn’t shrink for higher notes. Volume or expression control should be exponential, too, because the human ear needs 10 times more sound pressure to perceive a subjective loudness doubling. That’s why in audio gear, all volume potentiometers have exponential curves (although they are traditionally called log-potentiometers). What I try to achieve with the current volume mod is to make the volume start (close to the antenna) smoother, it was somewhat snappy which is not ok when playing expressive music like for example the post-romantic stuff by Gabriel Fauré. That snappiness made the volume rise quickly, and above, you could make huge movements without noticing any volume change. Classical Thereminists are used to have a volume field which is 25 to 40cm in height, so that they can control all volume nuances over the full dynamic range.

[Saijin-Naib]

Thank you so much! I'm obviously very much no knowledgeable about the design specifics and technical aspects of these instruments, so this is very valuable information.

[Theremingenieur]

@Saijin-Naib : The pitch side was and is already very well optimized. After calibration and some fine tuning with the pitch potentiometer, you have a pitch field with almost equal tone spacing over 5 octaves which can be played like on an analog Theremin with the classical aerial fingering technique. Thus, switching between analog instruments and the Open.Theremin is without difficulty for an experienced player who will find a very similar feeling. What makes you think that it needs to be improved?

[Saijin-Naib]

What makes you think that it needs to be improved?

Nothing, just a misunderstanding of what the value range in the wavetables represented, and my thoughts that only having a range of -2048 to 2048 meant that there were only 4096 "steps" of pitch change, and I thought that finer might be better.

[Theremingenieur]

Man... the values from -2048 to +2047 give 4096 different amplitude steps. 4096 is 2^12, which means that this is exactly the number of steps which the 12-bit MCP4921 DAC can handle. Sending coarser steps would represent a loss of dynamic and details and sending finer steps wouldn’t improve the result but just represent more CPU load for the good old asthmatic Arduino UNO. 12bit resolution allows a dynamic range or signal to noise ratio of 73.6dB. This is not CD quality, I admit, but DACs with higher resolution can neither be controlled by an Arduino at the actual sampling rate, nor do they fit into a hobbyists budget. But the actual 73.6dB are fine, compared to most other electronic music instruments. The electromagnetic pickups used in guitars and basses produce so much hum, buzz, and hiss that the signal to noise ratio corresponds rather to a 10bit DAC with steps only from -512 to +511...

[Saijin-Naib]

And now I understand why the wavetables have that range. Makes sense, and thanks for the detailed explanation.

[MrDham]

Hi. Great upgrade ! I also start to takeit into account in the MIDI branch.

[MrDham]

All changes seem quite straightforward to implement, I'll take a couple of days to merge them...

Maybe just one question: A great improvement is the volume response. Reading the code, I understand that "audio volume (16bit) = pseudo-exp (hand position (8 bit))" Midi Volume message has 7bit definition, I think it should rather reflect hand position than volume, correct ?

[Theremingenieur]

Exponentiation should not be done when sending volume information over MIDI. This is normally done later in the VCA. Thus, I’d keep on sending the volume data only in the coarse 7bit format, take the 8bit hand position vScaledVolume >> 1. An improvement would be using the 14bit format by using the raw 12bit volume value and scale it up v_vol << 2, then sending the higher 7 bits over rod_midi_cc and the lower 7 bits over (rod_midi_cc +32) which would be the corresponding LSB control channel.

[MrDham]

Ok, let's keep it linear. That was also my intuition. Work in progress...

Looking at Open Theremin V3.1 code, I found something that looks like a small typo in "ihandlers.cpp" Line 130: "mcpDacSend(vPointerIncrement); ...."

Should I read "SPImcpDACsend(vPointerIncrement); ...." ?

Not a big concern, it is for experimental CV mode (non-active code), just for sake of completion.

[Theremingenieur]

I found something that looks like a small typo in "ihandlers.cpp" Line 130: "mcpDacSend(vPointerIncrement); ...."

Should I read "SPImcpDACsend(vPointerIncrement); ...." ?

Not a big concern, it is for experimental CV mode (non-active code), just for sake of completion.

You are right, thank you!

[MrDham]

About MIDI version: changes are implemented in a specific branch. I am checking stability. I'll merge to master branch in a day or two...

[MrDham]

MIDI open Theremin V3: done ! These changes are implemented as well.

BTW, I extended pitch bend range choice (from 5 to 8 options). 4 Octaves bends are now possible on synth suporting it (DIY software synths, pure data patch, ... )