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

Synths #214

Merged
merged 57 commits into from Aug 29, 2017

Conversation

Projects
None yet
3 participants
@jvntf
Copy link
Contributor

jvntf commented Aug 16, 2017

built on the work of @b2renger in the synths branch--

follows the original design...
adds an AudioVoice class, MonoSynth class that extends AudioVoice, and PolySynth class as a manager

PolySynth class is redesigned to use resources better, follow polyphony rules, etc

opened PR with master as opposed to synths because the synths branch is quite old and the amount of changes that show up in the diff make it confusing

b2renger and others added some commits Mar 7, 2016

update polyphonic synth example :
Now it uses the new audiovoice et polysynth classes
first revision
- rename filter to synth out.
- correct dc offset filter.
- adapt function names according to the env class.
- copy documentation from env class.
- update inline examples
- update example/polyphonic synth
Merge pull request #1 from therewasaguy/b2renger-master
AudioVoice --> MonoSynth, add scheduling args to synths
#47 polyphonic synth implementation tweaks :
- _setNote as private method.
- play and noteTrigger now accept a velocity parameter.
- update inline doc
- fix checking of existing note.
@jvntf

This comment has been minimized.

Copy link
Contributor Author

jvntf commented Aug 19, 2017

updated this branch to include commits from the old synths branch

@jvntf jvntf force-pushed the jvntf:synths branch from 51dadcd to 69a252a Aug 19, 2017

@jvntf jvntf force-pushed the jvntf:synths branch from 69a252a to 905a842 Aug 19, 2017

@jvntf jvntf force-pushed the jvntf:synths branch from a320c75 to 905a842 Aug 20, 2017

@therewasaguy

This comment has been minimized.

Copy link
Member

therewasaguy commented Aug 20, 2017

This is looking pretty good to me! @b2renger curious if you have any thoughts as this builds on what you started?

I just sent a PR (jvntf#4) that fixes an issue I had when running locally that you may not have had, @jvntf.

I want to see what the generated reference pages will look like to users, but I'm getting an error when compiling documentation (the error is unrelated to this PR so I'll look at that separate). And maybe we should tackle documentation separately.

We'll also want to clean up the examples related to synths in the /examples directory to distinguish between examples that use synths from the library, and examples that build their own (which I think are good to have as well?)

@jvntf jvntf referenced this pull request Aug 20, 2017

Merged

Preset library #215

@b2renger

This comment has been minimized.

Copy link
Member

b2renger commented Aug 21, 2017

@jvntf this is really cool :) @therewasaguy I'll try to look it up soon (today or tomorrow) and I'll try to look at the spatializer branch that was merged as well (sometime later on)

@therewasaguy
Copy link
Member

therewasaguy left a comment

I wonder if synths should play frequency, instead of MIDI? With one option we stay more open to microtonal music, with the other we make it easier to make nice music with plain numbers. But MIDI still has a bit of a learning curve and I think it'd make sense to utilize freqToMidi in examples so that users can learn

// set range of env (TO DO: allow this to be scheduled in advance)
var susTime = susTime || this.sustain;
this.susTime = susTime;
this.triggerAttack(note, veocity, secondsFromNow);

This comment has been minimized.

@therewasaguy

therewasaguy Aug 21, 2017

Member

veocity --> velocity

Running eslint should catch things like this

*/
p5.MonoSynth.prototype.triggerAttack = function (note, velocity, secondsFromNow) {
var secondsFromNow = secondsFromNow || 0;
var freq = p5.prototype.midiToFreq(note);

This comment has been minimized.

@therewasaguy

therewasaguy Aug 21, 2017

Member

I wonder if these methods should use frequency, and let users decide whether they want to work in midi by utilizing midiToFreq in the synth examples?

This comment has been minimized.

@jvntf

jvntf Aug 21, 2017

Author Contributor

hm... i agree that playing frequency is important, but i like the ease of playing midi notes. maybe similar format to the soundLoop of two modes differentiated by String and Number would be good. Frequency = Number, midi notes ("C4", "Eb3"...etc) = String ?

This comment has been minimized.

@b2renger

b2renger Aug 22, 2017

Member

Using midiToFreq is good but the two modes is probably the better option since it is used in the soundLoop class (that I have to check !) and allows both

@jvntf

This comment has been minimized.

Copy link
Contributor Author

jvntf commented Aug 21, 2017

@therewasaguy re: the \examples directory I am thinking
\examples\monosynth_basic\ is a nice basic example that should stay as is
\examples\polyphonic_synth\ is a very cool example that should stay as an example of how to extend and build your own synths (needs some updating due to API change)
\examples\polysynth_old\ is also cool but seems like it maybe can just be combined with \examples\polyphonic_synth\? (this example contains a few more extensions of AudioVoice class (needs some updating due to API change)
\examples\polyphonicSynth-Keyboard\ is a 15-key keyboard example using p5.PolySynth and p5.MonoSynth
i'm happy to do these updates / migrations

@b2renger

This comment has been minimized.

Copy link
Member

b2renger commented Aug 22, 2017

@jvntf @therewasaguy

All your work is great on this subject. I think it's ready to merge when the examples are done. This is awesome.

As for the example I agree with you @jvntf the wrappers from the psynth_old should be merged in the new example and maybe we could switch beetween extensions with keyboard interactions ?

About the keyboard example it sounds a bit weird when several keys are pressed together, is it because of the synth ? because it sound like we cannot play a simple 2-note or 3-note chord.

Lastly the granular_sampler_psynth can probably be deleted from the master branch, those classes don't have to deal with sample stuff.

@b2renger

This comment has been minimized.

Copy link
Member

b2renger commented Aug 23, 2017

@jvntf I was thinking about the example with the keyboard and the fact that everything was distorted when playing more than one voice.

And I remembered I had tried to reduce the env range according to the number of voices used in a polysynth like this : 4721470

maybe it could be a good idea to have this kind of mechanism here too

@jvntf

This comment has been minimized.

Copy link
Contributor Author

jvntf commented Aug 23, 2017

@b2renger when you say distorted, are you referring to the clipping that sometimes happens? I just added in a maxRange for the envelope per your suggestion! let me know what you think-- to me it seems that it sounds pretty good considering the current p5.MonoSynth is fairly minimal in terms of sound design

@jvntf jvntf changed the title Synths WIP Synths Aug 25, 2017

@jvntf

This comment has been minimized.

Copy link
Contributor Author

jvntf commented Aug 28, 2017

@b2renger ok now i am hearing what i think you are hearing. strangely it sounds very different on laptop speakers, stereo, and headphones... sounds like a clipping problem but I'm not sure what to make of all the different results

@jvntf jvntf force-pushed the jvntf:synths branch from ba38ef9 to 6e3f153 Aug 28, 2017

@therewasaguy

This comment has been minimized.

Copy link
Member

therewasaguy commented Aug 29, 2017

@jvntf I think this distortion results from playing multiple notes with an amplitude envelope that maxes out at 1.0. Especially when the notes are in a similar frequency range, some speakers just can't handle it.

@b2renger I like the approach of setting the envelope's attack range based on the number of voices.

Building on that idea, @jvntf has set us up to track _voicesInUse, and we could use that to modulate the polysynth's output amplitude based on the number of active voices. This way, if just one note is active, it can still be loud—we only need to turn it down once other voices join in.

I think we should go ahead and merge this and address the distortion in a separate PR. I was thinking to squash some of these commits down because there are so many, but it's tricky, especially with so many contributors, and I think the commit history can be valuable like the way @b2renger referenced 4721470. So let's just merge it with all the history.

Very excited about all of this!

@therewasaguy therewasaguy merged commit d893159 into processing:master Aug 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment