Skip to content

sync phaser period to tempo#1261

Merged
daschuer merged 7 commits intomixxxdj:masterfrom
Be-ing:tempo_sync_phaser
Oct 8, 2017
Merged

sync phaser period to tempo#1261
daschuer merged 7 commits intomixxxdj:masterfrom
Be-ing:tempo_sync_phaser

Conversation

@Be-ing
Copy link
Copy Markdown
Contributor

@Be-ing Be-ing commented May 15, 2017

Tempo sync all the temporal effects!

Depends on:

@Be-ing Be-ing mentioned this pull request May 15, 2017
1 task
Comment thread src/effects/native/phasereffect.cpp Outdated
pOutput[i] = pInput[i] * (1.0 - 0.5 * depth) + left * depth * 0.5;
pOutput[i + 1] = pInput[i + 1] * (1.0 - 0.5 * depth) + right * depth * 0.5;
pOutput[i] = pInput[i] + left * depth * 0.5;
pOutput[i + 1] = pInput[i + 1] + right * depth * 0.5;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Unlike Reverb, the Phaser can be used as an insert effect. Since it has no delay component, you can turn it into a kind of insert effect by using the dry wet knob only. After this change, this is not possible any-more.

How about issue the "not loud enough" problem by a special scale for depth:
-> left half, new scale: up to 100 % dry + 50 % phaser
-> right half: up to 0% dry + 100 % phaser

Copy link
Copy Markdown
Contributor Author

@Be-ing Be-ing May 16, 2017

Choose a reason for hiding this comment

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

I do not understand the issue nor your proposal here. I have added a new commit turning this into a send effect. I think it works well. Please test this approach.

Copy link
Copy Markdown
Contributor Author

@Be-ing Be-ing Jul 4, 2017

Choose a reason for hiding this comment

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

I implemented a new way to mix the input and output signals. I think it (somewhat?) matches your proposal. The depth knob acts similar to a dry/wet knob, but because the wet signal is formed by adding two samples, it is divided by 2 before mixing with the dry signal. With the new formula, the input signal is unaffected when the depth knob is all the way down and the volume does not increase when the depth knob is all the way right, but the sound of the effect is clearly audible. However, it is a bit difficult to hear in the middle. I'll play with this more and if it works well I'll try implementing it in #1257 for the Flanger effect too.

Comment thread src/effects/native/phasereffect.cpp Outdated
} else {
// Period is a number of seconds
// TODO: remove assumption of stereo signal
period = period * sampleRate * 2;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My tracks have a much higher average BPM. Did you consider to pick 120 bpm to have similar scales?
It looks like this decision is a common one among all beat synced effects.
Can we define this somewhere central?

How about pick a reasonable clock in this case, like the other deck bpm or the master clock?
This is probably out of the focus of this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Picking any BPM would be totally arbitrary and inappropriate for a lot of music. Using the master clock would make sense, but the effects system does not provide that information currently. Only information about the channel being processed is available to the effect.

@daschuer
Copy link
Copy Markdown
Member

daschuer commented May 16, 2017 via email

@Be-ing
Copy link
Copy Markdown
Contributor Author

Be-ing commented May 16, 2017

If you turn to the second half it may turn to a 100 % dry effect.

I don't understand why you want that. The effect's sound comes from mixing the dry signal with the phase shifted wet signal to create interference. Without the dry signal there is no audible change, unless the dry signal is reintroduced by turning down the chain's dry/wet knob. Changing how the effect interacts with the dry/wet knob half way through the metaknob is counterintuitive and inconsistent with how other effects work, which would be strange in a chain.

@ronso0 ronso0 mentioned this pull request Jun 9, 2017
2 tasks
Comment thread src/effects/native/phasereffect.cpp Outdated
// the dry and computed sample, so to avoid making the output louder, divide
// the wet signal by 2.
pOutput[i] = pInput[i] * (1.0f - depth)
+ (pInput[i] + left) / 2.0f * depth;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this works quite well except that it is difficult to hear before the depth knob is up half way. I think changing the curve of the parameter to some sort of logarithmic curve could solve that issue, but I don't quite understand the math behind ControlLogPotmeterBehavior or ControlAudioTaperPotBehavior. @daschuer do you have any ideas for how to improve the scaling?

@Be-ing Be-ing force-pushed the tempo_sync_phaser branch from 8e61cda to 5c79a9c Compare July 31, 2017 06:13
@Be-ing
Copy link
Copy Markdown
Contributor Author

Be-ing commented Jul 31, 2017

I rebased this on master to remove the commits related to the Depth parameter and keep this PR focused on syncing the period to the tempo. Work on the Depth parameter continues in #1326.

@Be-ing
Copy link
Copy Markdown
Contributor Author

Be-ing commented Sep 26, 2017

I have added a Triplet parameter and adjusted the parameter defaults, ranges, and ordering a bit. I tried adding a Quantize parameter like Echo, but unlike Echo, I don't think there is a use case for an unquantized Phaser period.

Comment thread src/effects/native/phasereffect.cpp Outdated
* sampleRate * kChannels;
} else {
// period is a number of seconds
period = std::max(period, 1/4.0) * sampleRate * kChannels;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

remove kChannels here, it is applied twice

Comment thread src/effects/native/phasereffect.cpp Outdated
period /= 3.0;
}
period = period * groupFeatures.beat_length_sec
* sampleRate * kChannels;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

remove kChannels here, it is applied twice

Comment thread src/effects/native/phasereffect.cpp Outdated
period->setUnitsHint(EffectManifestParameter::UnitsHint::BEATS);
period->setMinimum(0.0);
period->setMaximum(4.0);
period->setDefault(0.5);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this should be 1

@Be-ing
Copy link
Copy Markdown
Contributor Author

Be-ing commented Oct 8, 2017

Notes addressed.

@daschuer
Copy link
Copy Markdown
Member

daschuer commented Oct 8, 2017

Thank you!

@daschuer daschuer merged commit 335f24d into mixxxdj:master Oct 8, 2017
@Be-ing Be-ing deleted the tempo_sync_phaser branch October 8, 2017 21:30
@mixxxbot mixxxbot mentioned this pull request Aug 22, 2022
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.

2 participants