-
Notifications
You must be signed in to change notification settings - Fork 34
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
expose fade times for root activation/deactivation #52
Changes from 5 commits
7fb7947
aa44e01
743a879
b2de8e9
16668c7
f12b6a9
0ae49af
226c05b
0338960
4d81418
2a7d23c
d29f4fe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -354,7 +354,7 @@ namespace elem | |
auto ptr = std::dynamic_pointer_cast<RootNode<FloatType>>(it->second); | ||
if (ptr) | ||
{ | ||
ptr->activate(currentRoots.empty() ? FloatType(1) : FloatType(0)); | ||
ptr->setProperty("active", true); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to make sure I'm following here, even though this was maybe my idea anyways from our early convos– this line does change the default behavior of the initial render(), right? But our intention is that a user who wants to disable the fade on the first render should make their first render call with Just want to be clear on that so I can update the docs correctly when we push v4 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exactly! |
||
active.insert(nodeId); | ||
ELEM_DBG("[Success] Activated root: " << nodeIdToHex(nodeId)); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ | |
#include "../SingleWriterSingleReaderQueue.h" | ||
|
||
#include "helpers/Change.h" | ||
#include "helpers/GainFade.h" | ||
#include "helpers/RefCountedPool.h" | ||
|
||
|
||
|
@@ -19,28 +20,14 @@ namespace elem | |
return channelIndex.load(); | ||
} | ||
|
||
FloatType getTargetGain() | ||
{ | ||
return targetGain.load(); | ||
} | ||
|
||
bool active() | ||
{ | ||
return targetGain.load() > 0.0f; | ||
return fade.on(); | ||
} | ||
|
||
bool stillRunning() | ||
{ | ||
auto const t = targetGain.load(); | ||
auto const c = currentGain.load(); | ||
|
||
return (t >= 0.5 || (std::abs(c - t) >= 1e-6)); | ||
} | ||
|
||
void activate(FloatType initialGain = FloatType(0)) | ||
{ | ||
setProperty("active", true); | ||
currentGain.store(initialGain); | ||
return active() || !fade.settled(); | ||
} | ||
|
||
int setProperty(std::string const& key, js::Value const& val) override | ||
|
@@ -49,13 +36,30 @@ namespace elem | |
if (!val.isBool()) | ||
return ReturnCode::InvalidPropertyType(); | ||
|
||
targetGain.store(FloatType(val ? 1 : 0)); | ||
if (val) | ||
fade.fadeIn(); | ||
else | ||
fade.fadeOut(); | ||
} | ||
|
||
if (key == "channel") { | ||
channelIndex.store(static_cast<int>((js::Number) val)); | ||
} | ||
|
||
if (key == "fadeInMs") { | ||
if (!val.isNumber()) | ||
return ReturnCode::InvalidPropertyType(); | ||
|
||
fade.setFadeInTimeMs(GraphNode<FloatType>::getSampleRate(), static_cast<float>((js::Number) val)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Super nitpick but you don't need the |
||
} | ||
|
||
if (key == "fadeOutMs") { | ||
if (!val.isNumber()) | ||
return ReturnCode::InvalidPropertyType(); | ||
|
||
fade.setFadeOutTimeMs(GraphNode<FloatType>::getSampleRate(), static_cast<float>((js::Number) val)); | ||
} | ||
|
||
return GraphNode<FloatType>::setProperty(key, val); | ||
} | ||
|
||
|
@@ -70,22 +74,11 @@ namespace elem | |
if (numChannels < 1) | ||
return (void) std::fill_n(outputData, numSamples, FloatType(0)); | ||
|
||
auto const t = targetGain.load(); | ||
auto c = currentGain.load(); | ||
|
||
auto const direction = (t < c) ? FloatType(-1) : FloatType(1); | ||
auto const step = direction * FloatType(20) / FloatType(GraphNode<FloatType>::getSampleRate()); | ||
|
||
for (size_t i = 0; i < numSamples; ++i) { | ||
outputData[i] = inputData[0][i] * c; | ||
c = std::clamp(c + step, FloatType(0), FloatType(1)); | ||
} | ||
|
||
currentGain.store(c); | ||
fade.process(inputData[0], outputData, numSamples); | ||
} | ||
|
||
std::atomic<FloatType> targetGain = 1; | ||
std::atomic<FloatType> currentGain = 0; | ||
GainFade<FloatType> fade = {GraphNode<FloatType>::getSampleRate(), 20, 20, 0.0, 1.0}; | ||
|
||
std::atomic<int> channelIndex = -1; | ||
}; | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,62 +6,116 @@ | |
namespace elem | ||
{ | ||
|
||
namespace detail { | ||
inline double millisecondsToStep(double sampleRate, double ms) { | ||
return ms > 1e-6 ? 1.0 / (sampleRate * ms / 1000.0) : 1.0; | ||
} | ||
} | ||
|
||
template <typename FloatType> | ||
struct GainFade | ||
{ | ||
GainFade(double sampleRate, double fadeTimeMs) | ||
: step(FloatType(1.0 / (sampleRate * fadeTimeMs / 1000.0))) | ||
GainFade(double sampleRate, double fadeInTimeMs, double fadeOutTimeMs, FloatType current = 0.0, FloatType target = 0.0) | ||
: currentGain(current) | ||
, targetGain(target) | ||
{ | ||
setFadeInTimeMs(sampleRate, fadeInTimeMs); | ||
setFadeOutTimeMs(sampleRate, fadeOutTimeMs); | ||
} | ||
|
||
GainFade(GainFade const& other) | ||
: currentGain(other.currentGain), targetGain(other.targetGain), step(other.step) | ||
: currentGain(other.currentGain.load()) | ||
, targetGain(other.targetGain.load()) | ||
, step(other.step.load()) | ||
, inStep(other.inStep) | ||
, outStep(other.outStep) | ||
{ | ||
} | ||
|
||
void operator= (GainFade const& other) | ||
{ | ||
currentGain = other.currentGain; | ||
targetGain = other.targetGain; | ||
step = other.step; | ||
currentGain.store(other.currentGain.load()); | ||
targetGain.store(other.targetGain.load()); | ||
step.store(other.step.load()); | ||
inStep = other.inStep; | ||
outStep = other.outStep; | ||
} | ||
|
||
FloatType operator() (FloatType x) { | ||
if (currentGain == targetGain) | ||
return (currentGain * x); | ||
auto const _currentGain = currentGain.load(); | ||
if (_currentGain == targetGain.load()) | ||
return (_currentGain * x); | ||
|
||
auto y = x * currentGain; | ||
currentGain = std::clamp(currentGain + step, FloatType(0), FloatType(1)); | ||
auto y = x * _currentGain; | ||
currentGain.store(std::clamp(_currentGain + step.load(), FloatType(0), FloatType(1))); | ||
|
||
return y; | ||
} | ||
|
||
void setTargetGain (FloatType g) { | ||
targetGain = g; | ||
void process (const FloatType* input, FloatType* output, int numSamples) { | ||
auto const _currentGain = currentGain.load(); | ||
if (_currentGain == targetGain.load()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's use the |
||
for (int i = 0; i < numSamples; ++i) { | ||
output[i] = input[i] * _currentGain; | ||
} | ||
return; | ||
} | ||
|
||
if (targetGain < currentGain) { | ||
step = FloatType(-1) * std::abs(step); | ||
} else { | ||
step = std::abs(step); | ||
auto const _step = step.load(); | ||
for (int i = 0; i < numSamples; ++i) { | ||
output[i] = input[i] * std::clamp(_currentGain + _step * i, FloatType(0), FloatType(1)); | ||
} | ||
|
||
currentGain.store(std::clamp(_currentGain + _step * numSamples, FloatType(0), FloatType(1))); | ||
} | ||
|
||
void setFadeInTimeMs(double sampleRate, double fadeInTimeMs) { | ||
inStep = detail::millisecondsToStep(sampleRate, fadeInTimeMs); | ||
updateCurrentStep(); | ||
} | ||
|
||
void setFadeOutTimeMs(double sampleRate, double fadeOutTimeMs) { | ||
outStep = FloatType(-1) * detail::millisecondsToStep(sampleRate, fadeOutTimeMs); | ||
updateCurrentStep(); | ||
} | ||
|
||
void fadeIn() { | ||
targetGain.store(FloatType(1)); | ||
updateCurrentStep(); | ||
} | ||
|
||
void fadeOut() { | ||
targetGain.store(FloatType(0)); | ||
updateCurrentStep(); | ||
} | ||
|
||
void setCurrentGain(FloatType gain) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unused? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we use it a couple places in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah ty! |
||
currentGain.store(gain); | ||
} | ||
|
||
bool on() { | ||
return (targetGain > FloatType(0.5)); | ||
return (targetGain.load() > FloatType(0.5)); | ||
} | ||
|
||
bool settled() { | ||
return fpEqual(targetGain, currentGain); | ||
return fpEqual(targetGain.load(), currentGain.load()); | ||
} | ||
|
||
void reset() { | ||
currentGain = FloatType(0); | ||
targetGain = FloatType(0); | ||
currentGain.store(FloatType(0)); | ||
targetGain.store(FloatType(0)); | ||
} | ||
|
||
private: | ||
void updateCurrentStep() { | ||
step.store(currentGain.load() > targetGain.load() ? outStep : inStep); | ||
} | ||
|
||
FloatType currentGain = 0; | ||
FloatType targetGain = 0; | ||
FloatType step = 0; | ||
std::atomic<FloatType> currentGain = 0; | ||
std::atomic<FloatType> targetGain = 0; | ||
std::atomic<FloatType> step = 0; | ||
FloatType inStep = 0; | ||
FloatType outStep = 0; | ||
}; | ||
|
||
} // namespace elem |
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.
Just noticing this– I think this changes the semantics of the conditional. We want to return early if the root node is inactive, even if it's still fading out.
I have a ticket in my todo-list to revisit here to smoothly fade out the feedback taps here, but until we get to that I'd say let's preserve the existing behavior here