-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Synthio envelope #7862
Synthio envelope #7862
Conversation
adsr = np.concatenate(
(
np.linspace(3277, 32767, num=40, dtype=np.int16, endpoint=False),
np.linspace(32767, 0, num=20, dtype=np.int16, endpoint=True),
)
)
#...
synth = synthio.Synthesizer(sample_rate=48000, waveform=waveform, envelope=adsr, envelope_hold_index=42) A basic linear volume ramp, then slight decay, followed by a sustain at about 80% of full scale. At 48kHz, it takes about 0.2s to ramp to full volume. The envelope can in principle be changed dynamically, same, as waveform, but I didn't test this. now tested on pygamer with a variant of my nunchuk instrument program from #7825 |
5151e82
to
9221756
Compare
looks like the ci docs failure is due to weblate being down for maintenance. |
@jedgarpark @todbot in case this is of interest, I'm working on adding ADSR-style envelope to synthio. I think it's ready for somebody to try. However, I'm on vacation 😀 so I won't necessarily act on feedback in a timely fashion. The changes also give another bump in loudness, because I improved (I hope) how the overall envelope of all playing notes is bounded. |
@jepler that's great, I'd love to try it out. |
@jepler it works, thanks so much! Going to see if i can figure out how to adjust the ADSR values. |
This frickin' rules. Thank you @jepler! |
|
@jepler this seems to be an ASR envelope, and is working very nicely (or maybe I haven't found the decay time control?) Here's my guess at what the adjustable values are: attack_time = 40
sustain_level = 32767
release_time = 140
asr = np.concatenate(
(
np.linspace(3277, sustain_level, num=attack_time, dtype=np.int16, endpoint=False),
np.linspace(sustain_level, 0, num=release_time, dtype=np.int16, endpoint=True),
)
)
synth = synthio.Synthesizer(
sample_rate=48000,
waveform=waveform,
envelope=asr,
envelope_hold_index=42
) |
I think this should be a fully configurable ADSR (instead of jepler's original where the decay rate is the same as the release rate), but perceptually the sound seems to "bounce" down-then-up, after the decay. Weird. attack_time = 100
decay_time = 50
release_time = 300
attack_level = 32767
sustain_level = 5767
adsr = np.concatenate( (
np.linspace(2, attack_level, num=attack_time, dtype=np.int16, endpoint=False),
np.linspace(attack_level, sustain_level, num=decay_time, dtype=np.int16, endpoint=False),
# sustain_level should be at envelope_hold_index = attack_time + decay_time
np.linspace(sustain_level, 0, num=release_time, dtype=np.int16, endpoint=True),
) )
print("adsr:", adsr[attack_time+decay_time], list(adsr) )
synth = synthio.Synthesizer(
sample_rate=48000,
waveform=waveform,
envelope=adsr,
envelope_hold_index=attack_time+decay_time
) This is really cool! My questions thus far:
|
# Time is in units of 256 samples, e.g., ~5ms at 48kHz
attack_time = 40
peak_level = 30000
decay_time = 40
sustain_level = 20000
release_time = 140
adsr = np.concatenate(
(
np.linspace(
peak_level / attack_time,
peak_level,
num=attack_time,
dtype=np.int16,
endpoint=False,
),
np.linspace(
peak_level, sustain_level, num=decay_time, dtype=np.int16, endpoint=False
),
np.linspace(sustain_level, 0, num=release_time, dtype=np.int16, endpoint=True),
)
)
sustain_index = attack_time+decay_time |
renamed the parameter to |
2f208b5
to
2f4e8fc
Compare
Thanks @jepler, that makes sense. This is a lot of fun to play with and already so very useful. Some comments from trying it out:
I've been putting my experiments up as gists. Thank you for putting yours up their too. I loved the wind chime one! |
Yes, right now releasing just jumps from whatever spot in the envelope to the first index after the sustain index. Given the underlying table-based approach I chose, what else would be sensible to do? Or should I abandon the table-based approach? (it's not too late for that) I'd like to directly support velocity (as relative note volume), but I don't know what the API would look like. Notes would either become objects with multiple properties, or all the 'parts' would be packed together into a single number like e.g., 24-bit rgb values are. I'd like to pick something that would work when doing multiple 'channels' (independent waveforms & envelopes). Are the NoteOn and NoteOff classes from adafruit_midi something to follow here? Or should I literally move closer to parsing a midi steam, like with a literal |
Oh, if the API is still fluid, I have opinions! 😀 The current waveform approach is interesting and allows for complex envelopes, so it'd be great for LFOs, but I don't see how to make it work easily as an ADSR envelope, where the envelope can constantly change based on user playing. Most of the synth APIs I have come across have something akin to "setLevels(v1,v2,v3,v4)" and "setRates(r1,r2,r3,r4)" methods on an envelope. The envelope system lerps between the levels according to the rates. So the envelope system tracks only these 8 values and the one or two internal values for lerping. Generally the above is wrapped up in an "Envelope" object and it's the Envelope that contains a "noteOn()" method to start the envelope attack and a "noteOff()" method to switch to the release phase. (There is a separate "Oscillator" object that is always running and contains a "setFrequency(hz)" method. This is so one can devote multiple detuned oscillators per played note, a common trick to make more complex sounds, something I think we can emulate with wavetables) Pseudo-code using such a design handling MIDI messages might look like:
For a fairly easy-to-read example of this kind of API, see Mozzi's ADSR.h and Oscil.h. I can imagine a
I think this kinda sounds like the two buffer params I was imagining too. I wish there was a way to modularize this a bit more so that
I would follow neither, honestly. The Also standard MIDI assumes a Western 12-tone equal temperament scale, great for most uses, but some people might want to do just temperament or microtonal scales. So if possible, I would recommend the ability to set frequency directly in Hz. |
f654f82
to
9d2f7c6
Compare
I'd prefer 8 separate parameters with better names. indices are hard to understand. |
Me too. But for some reason I was assuming we need to stick to ReadableBuffers to allow real-time control of these params. I am probably wrong about that. |
Maybe have a separate data class that holds config params, or use a namedtuple? |
I think there's no reason that If it's what y'all want I can abandon the arbitrary-buffer-based way of this PR as it stands, and switch to something more like the mozzi's envelope generator. switching synthesis to be based on frequency in (possibly floating point) Hz instead of MIDI notes would be a topic for a future PR. |
9d2f7c6
to
362fe10
Compare
@todbot in the mozzy way what parameters define a 'plucked' note's envelope? |
Here's what I tentatively have (only this draft documentation) for a more mozzy-like envelope (and the Synthesizer object will have a runtime-assignable
does this make sense / sound like something that is better than the current table-based approach? |
I'm ok with a buffer if it is a shape. I'd avoid a buffer if you'd want each index to mean something else. In that case, it should be named values. (I leave the synth decisions to folks who've done it.) |
This is the problem with ADSR envelopes in general. Many synths solve the problem by defining a curve shape (linear, exp, inv exp) to the rates. But the synth libraries I know only do linear rates for their ADSR (e.g. Mozzi, TeensyAL, DaisySP are linear). The DaisySP library has an separate AD envelope w/ a curve setting that is meant for percussive sounds like drums and plucks. Mozzi has something similar with an "EAD" envelope. If it's easy to bake in a slight exponential-like decay, I think that would be the most musically useful without needing to provide yet-another API parameter. Most sounds decay like that so it's what humans expect. |
Makes a lot of sense to me. And maybe opens the door for potentially other kinds of envelopes that people will want. |
OK, I'll re-work this PR so that it uses a parameter-based envelope instead of a table-based one. Thank you for the input. |
Thanks @jepler! I am astounded that we can get polyphonic synth functionality in CircuitPython. |
This is updated! Now an envelope can be used similar to this, from my testing code:
|
90e053d
to
c99af58
Compare
@jedgarpark @todbot I think this is ready for another round of testing. |
99d0c4c
to
e869ed0
Compare
This works for me (tested playing midi to raw files on host computer, as well as a variant of the nunchuk instrument on pygamer) it has to re-factor how/when MIDI reading occurs, because reasons. endorse new test results .. and allow `-1` to specify a note with no sustain (plucked)
this allows to test how the midi synthesizer is working, without access to hardware. Run `micropython-coverage midi2wav.py` and it will create `tune.wav` as an output.
e869ed0
to
38fd6ae
Compare
Hi @jepler! A few observations:
Here's a gist with a demo video showing what I've been testing with. |
I want to tackle that in the next round of improvements, so that a note can be associate with an envelope rather than having just one envelope for the whole Synthesizer object.
The current implementation of Envelope objects makes all properties read-only. I think this may have to stay as it is, because otherwise it gets complicated to decide what notes would be affected (need something re-calculated internally) when assigning a property. I'm open to ways around this so I can lift the restriction in the future.
Yes. This is true. I remain open to finding some different way to avoid clipping (or, I guess, permission to say that it's the user's fault when they get clipping). The number of simultaneous voices is fixed at compile time, but a particular user may need fewer. Maybe just one. Another user may want all 12, or even wish for more. Furthemore, the 1-voice user will want that voice to be able to play at full loudness, not 1/12 loudness. So, the synthesizer implements a very aggressive form of dynamic range compression by taking the sum of all the current notes' envelope loudness, and dividing the overall loudness by |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor things. Very cool that you have tests for this and can run it from the Unix port.
Testing envelope now -- this is working really nicely, sounds great! I think the envelope parameters make a lot of sense now (to people familiar with typical synth methods at least). |
Co-authored-by: Scott Shawcroft <scott@tannewt.org>
Co-authored-by: Scott Shawcroft <scott@tannewt.org>
@microdev1 this is failing in a weird way:
please let me know if there's something I can do to resolve it |
.. instead of just printing the unraiseable error on the repl
@microdev1 the CI problem seems to have healed so that's good I guess |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One nitpick that you can do in your next PR if you like.
@@ -28,122 +28,118 @@ | |||
#include "shared-bindings/synthio/MidiTrack.h" | |||
|
|||
|
|||
STATIC NORETURN void raise_midi_stream_error(uint32_t pos) { | |||
mp_raise_ValueError_varg(translate("Error in MIDI stream at position %d"), pos); | |||
STATIC void print_midi_stream_error(synthio_miditrack_obj_t *self) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This no longer prints. Maybe set
, record
or report
?
This adds a parametric A/D/S/R envelope to synthio. The same envelope is applied to all notes, but can be changed dynamically. the final approach is parametric, though some initial comments describe the original buffer-based approach