Skip to content
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

Trying to merge back changes from SuperDough fork #303

Open
wants to merge 34 commits into
base: develop
Choose a base branch
from

Conversation

telephon
Copy link
Contributor

@telephon telephon commented Sep 5, 2024

See #302

There are lots of changes, so it is a little hard to tell what would impact what. But given this merge request, we can see the diff better.

@telephon telephon mentioned this pull request Sep 5, 2024
@geikha
Copy link
Contributor

geikha commented Sep 7, 2024

let's also consider that we could extend further the first handshake between Tidal/Strudel to SuperDirt, so that SuperDirt knows which one is connected and maybe interpret differently the messages (majorly on breaking changes)

@@ -250,20 +250,22 @@
//emulation of juno 60 chorus
SynthDef("strudel_chorus" ++ numChannels, { |out, mix, depth, speed|
var input = In.ar(out, numChannels);
var l;
var r;
var l = input[0];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

input may have any number of channels, and if someone chooses to run it mono, it will not be an array. You could use the technique we use in dirt_leslie.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did not know that it was not always an array, Ill fix that!

@daslyfe
Copy link

daslyfe commented Sep 13, 2024

let's also consider that we could extend further the first handshake between Tidal/Strudel to SuperDirt, so that SuperDirt knows which one is connected and maybe interpret differently the messages (majorly on breaking changes)

good callout, hopefully overtime tidal and strudel can be brought closer together in terms of things like note scaling but this is possibly a good short term solution

@telephon
Copy link
Contributor Author

let's also consider that we could extend further the first handshake between Tidal/Strudel to SuperDirt, so that SuperDirt knows which one is connected and maybe interpret differently the messages (majorly on breaking changes)

What differences would need to be handled? Wouldn't it be much easier to just keep things the same? We are a really small community!

@@ -67,6 +83,7 @@ DirtEvent {
var accelerate = ~accelerate.value;
var avgSpeed, endSpeed;
var useUnit;
~release = ~release.value ? 0.01;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default values are usually given in the default event. Unless you need to caclulate something with ~release here, also the .value will be called automatically.

@@ -102,24 +119,38 @@ DirtEvent {
};

sustain = ~sustain.value;

sustain = sustain ?? {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes below: are you sure strudel couldn't translate its logic to the way tidalcycles works? It will be very confusing to change all that …

@@ -129,6 +160,7 @@ DirtEvent {
~channel !? { ~pan = ~pan.value + (~channel.value / ~numChannels) };
~pan = ~pan * 2 - 1; // convert unipolar (0..1) range into bipolar one (-1...1)
~delayAmp = ~delay ? 0.0; // for clarity
~z1 = ~z1 ? ~n;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you explain what ~z1 is used for?

@@ -108,15 +109,15 @@ DirtEventTypes {

if (hasNote) {
freq = ~freq.value;
~midinote = (freq.cpsmidi).round(1).asInteger;
~midinote = ((freq.cpsmidi).round(1) ? 0).asInteger;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the default is not necessary and can be dropped. freq.cpsmidi).round(1) will never be nil.

// Assume aftertouch means no noteOn, for now..
if(~polyTouch.notNil) {
val = ~polyTouch;
note = ~midinote;
schedmidi.value({ midiout.polyTouch(chan, note, val) })
} {
// match dirt_gate SynthDef amplitude scaling
~amp = ~amp.value * pow(~gain.min(2) + ~overgain, 4);
~amp = ~amp.value * StrudelUtils.gainCurve(~gain + ~overgain, 4);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's introduce gainCurve to SuperDirt. Should it be global or orbit-wise?

~n = \none; // sample number or note
~octave = 5;
~midinote = #{ ~note ? ~n + (~octave * 12) };
~n = nil;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should find a way around setting this to nil. Why can't it be \none?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically, you can just directly replace the default if you need, no need for StrudelUtils:

~dirt.set(\n, 42); // whatever you want

~fadeTime = 0.001;
~delaytime = 0.1875;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not 100% sure, but I think this means that there is always delay on. This can be set for each piece separately.

@@ -179,7 +203,9 @@ DirtSoundLibrary {
};

files = pathMatch(folderPath.standardizePath +/+ "*"); // dependent on operating system
if(sortFiles) { files.sort };
if(sortFiles) {
files.sort;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in sclang, there is no ; inside the last statement of a function. It is not wrong, but the change is not needed.

@@ -298,8 +327,7 @@ DirtSoundLibrary {
^event

} {
// the index may be \none (a Symbol), but this converts it to 0
event = allEvents.wrapAt(index.asInteger);
event = allEvents.wrapAt((index ? 0).asInteger);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be revisited in SuperDirt.

@@ -320,8 +348,9 @@ DirtSoundLibrary {
}

makeEventForBuffer { |buffer, metaData|
var baseFreq = 60.midicps;
var baseFreq = StrudelUtils.baseNote().midicps;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I intentionally kept separate DirtSoundLibrary and SuperDirt. This StrudelUtils.baseNote(). couples both to one system. MAybe we find a way around this?

@@ -53,10 +54,14 @@ SuperDirt {

init {
soundLibrary = DirtSoundLibrary(server, numChannels);
outputvolume = server.volume;
outputvolume.setVolumeRange(-90, 0);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to be able to use SuperDirt with other libraries – we shouldn't mess with the server volume, I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am sure there is another way to set volume? Normally we are in the range [min = -90, max = 6] so here you have found a reason why this should be less?

@@ -74,7 +75,7 @@ Pdef(\x,
\s, Pn(Pshuf([\bd, \hh, \cp, \imp], 5)),
\begin, Pn(Pseries(0, 0.002, 20)),
\dur, 0.1,
\legato, Pwhite(0.01, 0.4)
\clip, Pwhite(0.01, 0.4)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is clip doing?

@@ -220,6 +225,7 @@
sound = MoogFF.ar(sound, SinOsc.ar(basefreq/32*rate, 0).range(lfof1,lfof2), resonance*4);
sound = MoogFF.ar(sound, min(env2*lfof2*1.1, 22000), 3);
sound = sound.tanh*5;
sound = sound * StrudelUtils.synthGain;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to hardcode the volume adjustments, you can just do that by setting the ~amp parameter in the input. So all the below can easily be avoided.

hresonance: ~hresonance,
out: ~out
])
// ~dirt.addModule('hpf',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you remove hpf and lpf – are there problems with them?

if(\SwitchDelay.asClass.notNil) {

SynthDef("dirt_delay" ++ numChannels, { |dryBus, effectBus, gate = 1, delaytime, delayfeedback, delaySend = 1, delayAmp = 1, lock = 0, cps = 1|
SynthDef("dirt_shimmer" ++ numChannels, { |dryBus, effectBus, gate = 1, delaytime, delayfeedback, shimmer = 0, lock = 1, cps = 1|
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's move shimmer to SuperDirt ...

var signal = In.ar(out, numChannels);
var env = EnvGen.ar(Env([0, 1, 1, 0], [fadeInTime, sustain, fadeTime], \sin));
var env = EnvGen.ar(Env([0, 1, 1, 0], [fadeInTime, totalDuration - (fadeInTime + fadeTime), fadeTime], \sin));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, in supercollider generally, sustain means totalDuration – this is a bit unusual, but the standard …

@telephon
Copy link
Contributor Author

@daslyfe I've left a lot of comments for you so it hopefully makes it easier to separate those things that are really needed and those that just happened due to a lack of documentation in superdirt.

In the meantime, since superdirt is live hackable, many of the changes could just be moved to a file that is evaluated at startup. Like all the SynthDefs etc.

Maybe you could check and see if you can separate things a little? That would be immensly helpful.

@daslyfe
Copy link

daslyfe commented Sep 19, 2024

@daslyfe I've left a lot of comments for you so it hopefully makes it easier to separate those things that are really needed and those that just happened due to a lack of documentation in superdirt.

In the meantime, since superdirt is live hackable, many of the changes could just be moved to a file that is evaluated at startup. Like all the SynthDefs etc.

Maybe you could check and see if you can separate things a little? That would be immensly helpful.

Thanks for taking the time to comb through all of this! It helps to know how things can be better separated. I am still in a “hacking stuff together” phase of making superdirt strudel compatible. I intend to break changes into smaller much more manageable PRs

@telephon
Copy link
Contributor Author

that is good! You could take my comments as things to mind to be able to do it with less effort / changes.

@telephon
Copy link
Contributor Author

@daslyfe I just say that you changed the monitor and added wet and dry.

GlobalDirtEffect(\dirt_monitor, [\limitertype, \outgain, \dry, \wet]).alwaysRun_(true),
and
SynthDef("dirt_monitor" ++ numChannels, { |dryBus, effectBus, outBus, gate = 1, limitertype = 1, outgain = 1, dry = 1, wet = 1|

Is there a reason why the local wet / dry controls of each effect are not sufficient?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants