Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions core/include/orbit_delay_c_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ bool orbit_set_feedback(OrbitDelayHandle* handle, float value);
bool orbit_set_feedback_drive(OrbitDelayHandle* handle, float value);
bool orbit_set_feedback_nonlinear_amount(OrbitDelayHandle* handle, float value);
bool orbit_set_feedback_comp_threshold(OrbitDelayHandle* handle, float value);
bool orbit_set_feedback_highpass_hz(OrbitDelayHandle* handle, float value);
bool orbit_set_mix(OrbitDelayHandle* handle, float value);
bool orbit_set_input_gain(OrbitDelayHandle* handle, float value);
bool orbit_set_output_gain(OrbitDelayHandle* handle, float value);
Expand Down
6 changes: 6 additions & 0 deletions core/include/orbit_delay_core.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class OrbitDelayCore {
void setFeedbackDrive(float value);
void setFeedbackNonlinearAmount(float value);
void setFeedbackCompThreshold(float value);
void setFeedbackHighpassHz(float value);
void setMix(float value);
void setInputGain(float value);
void setOutputGain(float value);
Expand Down Expand Up @@ -109,6 +110,8 @@ class OrbitDelayCore {
static constexpr float kDiffuserUpdateDelta = 0.01f;
static constexpr float kDiffuserCadenceUpdateDelta = 0.03f;
static constexpr float kHighpassCutoffHz = 30.0f;
static constexpr float kMinFeedbackHighpassHz = 20.0f;
static constexpr float kMaxFeedbackHighpassHz = 2000.0f;
static constexpr float kOrbitReadGuardMinSamples = 16.0f;
static constexpr float kOrbitReadGuardMaxSamples = 64.0f;
static constexpr float kHighpassUpdateDeltaHz = 1.0f;
Expand Down Expand Up @@ -172,6 +175,7 @@ class OrbitDelayCore {
float feedbackDrive_ = 1.0f;
float feedbackNonlinearAmount_ = 0.0f;
float feedbackCompThreshold_ = 1.0f;
float feedbackHighpassHz_ = kHighpassCutoffHz;
float mix_ = 0.35f;
float toneHz_ = 8000.0f;
float smear_ = 0.0f;
Expand Down Expand Up @@ -202,6 +206,7 @@ class OrbitDelayCore {
LinearSmoother feedbackDriveSm_;
LinearSmoother feedbackNonlinearAmountSm_;
LinearSmoother feedbackCompThresholdSm_;
LinearSmoother feedbackHighpassHzSm_;
LinearSmoother toneSm_;
LinearSmoother orbitSm_;
LinearSmoother offsetSm_;
Expand All @@ -227,6 +232,7 @@ class OrbitDelayCore {
std::atomic<float> pendingFeedbackDrive_{1.0f};
std::atomic<float> pendingFeedbackNonlinearAmount_{0.0f};
std::atomic<float> pendingFeedbackCompThreshold_{1.0f};
std::atomic<float> pendingFeedbackHighpassHz_{kHighpassCutoffHz};
std::atomic<float> pendingMix_{0.35f};
std::atomic<float> pendingInputGain_{1.0f};
std::atomic<float> pendingOutputGain_{1.0f};
Expand Down
8 changes: 8 additions & 0 deletions core/src/orbit_delay_c_api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,14 @@ bool orbit_set_feedback_comp_threshold(OrbitDelayHandle* handle, float value) {
return true;
}

bool orbit_set_feedback_highpass_hz(OrbitDelayHandle* handle, float value) {
if (handle == nullptr) {
return false;
}
handle->core.setFeedbackHighpassHz(value);
return true;
}

bool orbit_set_mix(OrbitDelayHandle* handle, float value) {
if (handle == nullptr) {
return false;
Expand Down
29 changes: 24 additions & 5 deletions core/src/orbit_delay_core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ void OrbitDelayCore::applyPendingParamsIfNeeded() {
const float nextFeedbackDrive = pendingFeedbackDrive_.load(std::memory_order_relaxed);
const float nextFeedbackNonlinearAmount = pendingFeedbackNonlinearAmount_.load(std::memory_order_relaxed);
const float nextFeedbackCompThreshold = pendingFeedbackCompThreshold_.load(std::memory_order_relaxed);
const float nextFeedbackHighpassHz = pendingFeedbackHighpassHz_.load(std::memory_order_relaxed);
const float nextMix = pendingMix_.load(std::memory_order_relaxed);
const float nextInputGain = pendingInputGain_.load(std::memory_order_relaxed);
const float nextOutputGain = pendingOutputGain_.load(std::memory_order_relaxed);
Expand Down Expand Up @@ -198,6 +199,12 @@ void OrbitDelayCore::applyPendingParamsIfNeeded() {
feedbackCompThreshold_ = clampedFeedbackCompThreshold;
smoothTargetsChanged = true;
}
const float clampedFeedbackHighpassHz =
clampf(sanitizeFinite(nextFeedbackHighpassHz, feedbackHighpassHz_), kMinFeedbackHighpassHz, kMaxFeedbackHighpassHz);
if (feedbackHighpassHz_ != clampedFeedbackHighpassHz) {
feedbackHighpassHz_ = clampedFeedbackHighpassHz;
smoothTargetsChanged = true;
}
const float clampedMix = clampf(sanitizeFinite(nextMix, mix_), 0.0f, 1.0f);
if (mix_ != clampedMix) {
mix_ = clampedMix;
Expand Down Expand Up @@ -278,6 +285,7 @@ void OrbitDelayCore::syncDspParams() {
feedbackDriveSm_.configure(sampleRate_, kSmoothFeedbackNonlinearMs);
feedbackNonlinearAmountSm_.configure(sampleRate_, kSmoothFeedbackNonlinearMs);
feedbackCompThresholdSm_.configure(sampleRate_, kSmoothFeedbackNonlinearMs);
feedbackHighpassHzSm_.configure(sampleRate_, kSmoothToneMs);
feedbackLimiterL_.configure(sampleRate_);
feedbackLimiterR_.configure(sampleRate_);
toneSm_.configure(sampleRate_, kSmoothToneMs);
Expand All @@ -301,9 +309,10 @@ void OrbitDelayCore::syncDspParams() {
const float lowpassQ = feedbackLowpassQ();
lowpassL_.setParams(nyquistLimited, lowpassQ);
lowpassR_.setParams(nyquistLimited, lowpassQ);
highpassL_.setParams(kHighpassCutoffHz, kDefaultFeedbackLowpassQ);
highpassR_.setParams(kHighpassCutoffHz, kDefaultFeedbackLowpassQ);
appliedHighpassHz_ = kHighpassCutoffHz;
const float clampedHighpassHz = clampf(feedbackHighpassHz_, kMinFeedbackHighpassHz, clampf(0.49f * sampleRate_, kMinFeedbackHighpassHz, kMaxFeedbackHighpassHz));
highpassL_.setParams(clampedHighpassHz, kDefaultFeedbackLowpassQ);
highpassR_.setParams(clampedHighpassHz, kDefaultFeedbackLowpassQ);
appliedHighpassHz_ = clampedHighpassHz;
lowpassDirty_ = false;
}

Expand Down Expand Up @@ -331,6 +340,7 @@ void OrbitDelayCore::updateSmoothedTargetsIfDirty() {
feedbackDriveSm_.setTarget(feedbackDrive_);
feedbackNonlinearAmountSm_.setTarget(feedbackNonlinearAmount_);
feedbackCompThresholdSm_.setTarget(feedbackCompThreshold_);
feedbackHighpassHzSm_.setTarget(feedbackHighpassHz_);
mixSm_.setTarget(mix_);
inputGainSm_.setTarget(inputGain_);
outputGainSm_.setTarget(outputGain_);
Expand Down Expand Up @@ -360,7 +370,8 @@ OrbitDelayCore::SmoothedParams OrbitDelayCore::advanceSmoothers() {
heavyParamCadenceHit_ = advanceCadence();

maybeApplyLowpassCutoff(smoothedToneHz);
maybeApplyHighpassCutoff(kHighpassCutoffHz);
const float smoothedFeedbackHighpassHz = feedbackHighpassHzSm_.next();
maybeApplyHighpassCutoff(smoothedFeedbackHighpassHz);
maybeApplyDiffuserAmount(smoothedSmear);
return params;
}
Expand Down Expand Up @@ -406,7 +417,8 @@ void OrbitDelayCore::maybeApplyDiffuserAmount(float smoothedSmear) {
}

void OrbitDelayCore::maybeApplyHighpassCutoff(float targetHighpassHz) {
const float clampedHighpassHz = clampf(targetHighpassHz, 10.0f, clampf(0.49f * sampleRate_, 10.0f, 2000.0f));
const float clampedHighpassHz =
clampf(targetHighpassHz, kMinFeedbackHighpassHz, clampf(0.49f * sampleRate_, kMinFeedbackHighpassHz, kMaxFeedbackHighpassHz));
const float delta = std::fabs(clampedHighpassHz - appliedHighpassHz_);
if (lowpassDirty_ || (heavyParamCadenceHit_ && delta >= kHighpassUpdateDeltaHz)) {
highpassL_.setParams(clampedHighpassHz, kDefaultFeedbackLowpassQ);
Expand Down Expand Up @@ -438,6 +450,7 @@ void OrbitDelayCore::reset(float sampleRate) {
feedbackDriveSm_.reset(feedbackDrive_);
feedbackNonlinearAmountSm_.reset(feedbackNonlinearAmount_);
feedbackCompThresholdSm_.reset(feedbackCompThreshold_);
feedbackHighpassHzSm_.reset(feedbackHighpassHz_);
feedbackLimiterL_.reset();
feedbackLimiterR_.reset();
mixSm_.reset(mix_);
Expand Down Expand Up @@ -551,6 +564,12 @@ void OrbitDelayCore::setMix(float value) {
pendingParamVersion_.fetch_add(1u, std::memory_order_release);
}

void OrbitDelayCore::setFeedbackHighpassHz(float value) {
pendingFeedbackHighpassHz_.store(clampf(sanitizeFinite(value, feedbackHighpassHz_), kMinFeedbackHighpassHz, kMaxFeedbackHighpassHz),
std::memory_order_relaxed);
pendingParamVersion_.fetch_add(1u, std::memory_order_release);
}
Comment on lines +567 to +571

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Thread Safety / Data Race Issue

There is a potential data race on feedbackHighpassHz_. The function setFeedbackHighpassHz is called from the control/UI thread, but it reads feedbackHighpassHz_ (a non-atomic member variable) as a fallback value for sanitizeFinite. Meanwhile, the audio thread writes to feedbackHighpassHz_ in applyPendingParamsIfNeeded() on every processing block.

Reading a non-atomic variable that is concurrently written to by another thread is undefined behavior in C++.

Solution

Instead of falling back to the non-atomic feedbackHighpassHz_, you can safely fall back to the atomic pendingFeedbackHighpassHz_.load(std::memory_order_relaxed). This avoids the data race entirely and ensures thread safety.

Suggested change
void OrbitDelayCore::setFeedbackHighpassHz(float value) {
pendingFeedbackHighpassHz_.store(clampf(sanitizeFinite(value, feedbackHighpassHz_), kMinFeedbackHighpassHz, kMaxFeedbackHighpassHz),
std::memory_order_relaxed);
pendingParamVersion_.fetch_add(1u, std::memory_order_release);
}
void OrbitDelayCore::setFeedbackHighpassHz(float value) {
const float fallback = pendingFeedbackHighpassHz_.load(std::memory_order_relaxed);
pendingFeedbackHighpassHz_.store(clampf(sanitizeFinite(value, fallback), kMinFeedbackHighpassHz, kMaxFeedbackHighpassHz),
std::memory_order_relaxed);
pendingParamVersion_.fetch_add(1u, std::memory_order_release);
}


void OrbitDelayCore::setInputGain(float value) {
pendingInputGain_.store(clampf(sanitizeFinite(value, inputGain_), 0.0f, 4.0f), std::memory_order_relaxed);
pendingParamVersion_.fetch_add(1u, std::memory_order_release);
Expand Down
2 changes: 1 addition & 1 deletion targets/wasm/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,6 @@ target_link_options(orbit_delay_wasm PRIVATE
"-sEXPORT_ES6=1"
"-sENVIRONMENT=web,worker,node"
"-sALLOW_MEMORY_GROWTH=1"
"-sEXPORTED_FUNCTIONS=['_malloc','_free','_orbit_wasm_init','_orbit_wasm_free','_orbit_wasm_reset','_orbit_wasm_process_stereo','_orbit_wasm_set_orbit','_orbit_wasm_set_offset_samples','_orbit_wasm_set_tempo_bpm','_orbit_wasm_set_note_division','_orbit_wasm_set_stereo_spread','_orbit_wasm_set_lfo_rate_hz','_orbit_wasm_set_lfo_depth_samples','_orbit_wasm_set_lfo_stereo_phase_offset','_orbit_wasm_set_feedback','_orbit_wasm_set_mix','_orbit_wasm_set_input_gain','_orbit_wasm_set_output_gain','_orbit_wasm_set_tone_hz','_orbit_wasm_set_smear_amount','_orbit_wasm_set_shimmer_mode','_orbit_wasm_set_diffuser_stages','_orbit_wasm_set_dc_block_enabled','_orbit_wasm_set_read_mode','_orbit_wasm_set_feedback_tap_mixer_enabled','_orbit_wasm_set_feedback_tap_mixer_amount']"
"-sEXPORTED_FUNCTIONS=['_malloc','_free','_orbit_wasm_init','_orbit_wasm_free','_orbit_wasm_reset','_orbit_wasm_process_stereo','_orbit_wasm_set_orbit','_orbit_wasm_set_offset_samples','_orbit_wasm_set_tempo_bpm','_orbit_wasm_set_note_division','_orbit_wasm_set_stereo_spread','_orbit_wasm_set_lfo_rate_hz','_orbit_wasm_set_lfo_depth_samples','_orbit_wasm_set_lfo_stereo_phase_offset','_orbit_wasm_set_feedback','_orbit_wasm_set_feedback_highpass_hz','_orbit_wasm_set_mix','_orbit_wasm_set_input_gain','_orbit_wasm_set_output_gain','_orbit_wasm_set_tone_hz','_orbit_wasm_set_smear_amount','_orbit_wasm_set_shimmer_mode','_orbit_wasm_set_diffuser_stages','_orbit_wasm_set_dc_block_enabled','_orbit_wasm_set_read_mode','_orbit_wasm_set_feedback_tap_mixer_enabled','_orbit_wasm_set_feedback_tap_mixer_amount']"
"-sEXPORTED_RUNTIME_METHODS=['ccall','cwrap']"
)
1 change: 1 addition & 0 deletions targets/wasm/js/ui.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export function bindOrbitUiControls(api, root = document) {
['readMode', 'orbit_wasm_set_read_mode', (v) => Math.round(parseFloat(v))],
['stereoSpread', 'orbit_wasm_set_stereo_spread', parseFloat],
['feedback', 'orbit_wasm_set_feedback', parseFloat],
['feedbackHighpassHz', 'orbit_wasm_set_feedback_highpass_hz', parseFloat],
['mix', 'orbit_wasm_set_mix', parseFloat],
['inputGain', 'orbit_wasm_set_input_gain', parseFloat],
['outputGain', 'orbit_wasm_set_output_gain', parseFloat],
Expand Down
1 change: 1 addition & 0 deletions targets/wasm/orbit_delay_wasm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ bool orbit_wasm_set_lfo_rate_hz(float value) { return orbit_set_lfo_rate_hz(g_ct
bool orbit_wasm_set_lfo_depth_samples(float value) { return orbit_set_lfo_depth_samples(g_ctx.handle, value); }
bool orbit_wasm_set_lfo_stereo_phase_offset(float value) { return orbit_set_lfo_stereo_phase_offset(g_ctx.handle, value); }
bool orbit_wasm_set_feedback(float value) { return orbit_set_feedback(g_ctx.handle, value); }
bool orbit_wasm_set_feedback_highpass_hz(float value) { return orbit_set_feedback_highpass_hz(g_ctx.handle, value); }
bool orbit_wasm_set_mix(float value) { return orbit_set_mix(g_ctx.handle, value); }
bool orbit_wasm_set_input_gain(float value) { return orbit_set_input_gain(g_ctx.handle, value); }
bool orbit_wasm_set_output_gain(float value) { return orbit_set_output_gain(g_ctx.handle, value); }
Expand Down
Loading