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
6 changes: 6 additions & 0 deletions core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,9 @@ target_include_directories(orbit_delay_core
PUBLIC
${CMAKE_SOURCE_DIR}
)


target_compile_definitions(orbit_delay_core
PUBLIC
ORBIT_DELAY_ENABLE_HERMITE=1
)
4 changes: 4 additions & 0 deletions core/include/orbit_delay_core.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,12 @@ class OrbitDelayCore {
static constexpr float kMaxNoteDivision = 4.0f;
static constexpr uint32_t kHeavyParamCadenceSamples = 16u;
static constexpr float kLowpassUpdateDeltaHz = 20.0f;
static constexpr float kLowpassCadenceUpdateDeltaHz = 80.0f;
static constexpr float kDiffuserUpdateDelta = 0.01f;
static constexpr float kDiffuserCadenceUpdateDelta = 0.03f;
static constexpr float kHighpassCutoffHz = 30.0f;
static constexpr float kOrbitReadGuardMinSamples = 16.0f;
static constexpr float kOrbitReadGuardMaxSamples = 64.0f;
static constexpr float kHighpassUpdateDeltaHz = 1.0f;
static constexpr float kDefaultFeedbackLowpassQ = 0.707f;
static constexpr float kReverseLegacyFeedbackLowpassQ = 0.5f;
Expand Down
39 changes: 25 additions & 14 deletions core/src/orbit_delay_core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,14 @@ float computeTempoDelaySamples(float sampleRate, float tempoBpm, float noteDivis
float wrapUnitParam(float value, float fallback) {
return isFiniteSafe(value) ? wrapPosFloat(value, 1.0f, 1.0f) : fallback;
}

float readDelayWrapped(const DelayLine& delay, float readPos) {
#if defined(ORBIT_DELAY_ENABLE_HERMITE)
return delay.readAbsoluteHermiteWrapped(readPos);
#else
return delay.readAbsoluteLinearWrapped(readPos);
#endif
}
} // namespace

void OrbitDelayCore::applyPendingParamsIfNeeded() {
Expand Down Expand Up @@ -375,7 +383,8 @@ bool OrbitDelayCore::advanceCadence() {
void OrbitDelayCore::maybeApplyLowpassCutoff(float smoothedToneHz) {
const float clampedToneHz = clampf(smoothedToneHz, 300.0f, clampf(0.49f * sampleRate_, 300.0f, 12000.0f));
const float delta = std::fabs(clampedToneHz - appliedToneHz_);
if (lowpassDirty_ || heavyParamCadenceHit_ || delta >= kLowpassUpdateDeltaHz) {
const bool cadenceAndAudible = heavyParamCadenceHit_ && delta >= kLowpassCadenceUpdateDeltaHz;
if (lowpassDirty_ || delta >= kLowpassUpdateDeltaHz || cadenceAndAudible) {
Comment on lines +386 to +387

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

The logic for cadence-driven updates is redundant and likely inverted. Currently, kLowpassUpdateDeltaHz is 20.0 and kLowpassCadenceUpdateDeltaHz is 80.0. Since delta >= 80.0 implies delta >= 20.0, the cadenceAndAudible condition is dead code—the filter will update every sample as soon as the delta exceeds 20.0, regardless of the cadence hit.

Furthermore, this change prevents the filter from ever reaching its final target if the smoother stops moving while the delta is less than 20Hz (whereas previously it would sync on the next cadence hit). To achieve the goal of reducing updates while maintaining accuracy, you should update immediately for large changes and on cadence for smaller ones.

Suggested change
const bool cadenceAndAudible = heavyParamCadenceHit_ && delta >= kLowpassCadenceUpdateDeltaHz;
if (lowpassDirty_ || delta >= kLowpassUpdateDeltaHz || cadenceAndAudible) {
const bool cadenceAndAudible = heavyParamCadenceHit_ && delta >= kLowpassUpdateDeltaHz;
if (lowpassDirty_ || delta >= kLowpassCadenceUpdateDeltaHz || cadenceAndAudible) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve lowpass updates for sub-20 Hz tone moves

Reintroduce a cadence-based fallback for small tone deltas; after this change, if lowpassDirty_ is false and the tone change stays below 20 Hz, maybeApplyLowpassCutoff never applies the new cutoff (delta >= 20 is false and cadenceAndAudible requires delta >= 80). Tone parameter updates do not set lowpassDirty_, so small but intentional adjustments can be ignored indefinitely.

Useful? React with 👍 / 👎.

Comment on lines +386 to +387

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Cadence thresholds are currently ineffective due to condition dominance

Line 387 and Line 399 already trigger on lower deltas (kLowpassUpdateDeltaHz, kDiffuserUpdateDelta), so the new cadence checks never change behavior (80 >= 20, 0.03 >= 0.01). This defeats the intended cadence gating/perf win.

One concrete way to restore cadence-gated behavior
-    const bool cadenceAndAudible = heavyParamCadenceHit_ && delta >= kLowpassCadenceUpdateDeltaHz;
-    if (lowpassDirty_ || delta >= kLowpassUpdateDeltaHz || cadenceAndAudible) {
+    const bool cadenceAndAudible = heavyParamCadenceHit_ && delta >= kLowpassCadenceUpdateDeltaHz;
+    if (lowpassDirty_ || cadenceAndAudible) {
@@
-    const bool cadenceAndAudible = heavyParamCadenceHit_ && delta >= kDiffuserCadenceUpdateDelta;
-    if (diffuserDirty_ || delta >= kDiffuserUpdateDelta || cadenceAndAudible) {
+    const bool cadenceAndAudible = heavyParamCadenceHit_ && delta >= kDiffuserCadenceUpdateDelta;
+    if (diffuserDirty_ || cadenceAndAudible) {

Also applies to: 398-399

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/src/orbit_delay_core.cpp` around lines 386 - 387, The cadence check
(heavyParamCadenceHit_ && delta >= kLowpassCadenceUpdateDeltaHz) is currently
masked by the broader delta checks (delta >= kLowpassUpdateDeltaHz /
kDiffuserUpdateDelta), so cadence never changes behavior; modify the boolean
logic so cadence-triggered updates only fire when the regular update thresholds
would NOT already fire — for example, replace the current condition with
something like: if (lowpassDirty_ || delta >= kLowpassUpdateDeltaHz ||
(heavyParamCadenceHit_ && delta >= kLowpassCadenceUpdateDeltaHz && delta <
kLowpassUpdateDeltaHz)) and apply the analogous change for the diffuser branch
(use kDiffuserCadenceUpdateDelta and kDiffuserUpdateDelta / diffuserDirty_),
ensuring the cadence guard is combined with a check that delta is below the
broader update threshold so cadence gating actually takes effect.

const float lowpassQ = feedbackLowpassQ();
lowpassL_.setParams(clampedToneHz, lowpassQ);
lowpassR_.setParams(clampedToneHz, lowpassQ);
Expand All @@ -386,7 +395,8 @@ void OrbitDelayCore::maybeApplyLowpassCutoff(float smoothedToneHz) {

void OrbitDelayCore::maybeApplyDiffuserAmount(float smoothedSmear) {
const float delta = std::fabs(smoothedSmear - appliedSmear_);
if (diffuserDirty_ || heavyParamCadenceHit_ || delta >= kDiffuserUpdateDelta) {
const bool cadenceAndAudible = heavyParamCadenceHit_ && delta >= kDiffuserCadenceUpdateDelta;
if (diffuserDirty_ || delta >= kDiffuserUpdateDelta || cadenceAndAudible) {
Comment on lines +398 to +399

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

Similar to the lowpass logic, the cadence check here is redundant because kDiffuserCadenceUpdateDelta (0.03) is greater than kDiffuserUpdateDelta (0.01). The immediate update condition will always trigger before the cadence condition.

Suggested change
const bool cadenceAndAudible = heavyParamCadenceHit_ && delta >= kDiffuserCadenceUpdateDelta;
if (diffuserDirty_ || delta >= kDiffuserUpdateDelta || cadenceAndAudible) {
const bool cadenceAndAudible = heavyParamCadenceHit_ && delta >= kDiffuserUpdateDelta;
if (diffuserDirty_ || delta >= kDiffuserCadenceUpdateDelta || cadenceAndAudible) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve diffuser updates for sub-0.01 smear moves

Keep a cadence-triggered path for small smear adjustments; with this condition, once diffuserDirty_ is cleared, smear changes below 0.01 never propagate (delta >= 0.01 is false and the cadence branch now also requires delta >= 0.03). That creates dead zones for fine automation/knob moves that previously converged through cadence updates.

Useful? React with 👍 / 👎.

const float clamped = clampf(smoothedSmear, 0.0f, 1.0f);
diffuserL_.setAmount(clamped);
diffuserR_.setAmount(clamped);
Expand Down Expand Up @@ -618,21 +628,21 @@ float OrbitDelayCore::processChannelFast(float input,
if (readMode_ == ReadMode::AccidentalReverse) {
const float readPos = wrapPosFloat(static_cast<float>(delay.writePos) - params.tempoDelaySamples + spread + lfoSamples, delaySize, invDelaySize);
baseReadPos = readPos;
#if defined(ORBIT_DELAY_ENABLE_HERMITE)
wet = delay.readAbsoluteHermiteWrapped(readPos);
#else
wet = delay.readAbsoluteLinearWrapped(readPos);
#endif
wet = readDelayWrapped(delay, readPos);
} else {
float readPos = params.orbit * static_cast<float>(delay.writePos) + params.offsetSamples + spread + lfoSamples;
readPos = wrapPosFloat(readPos, delaySize, invDelaySize);
baseReadPos = readPos;

#if defined(ORBIT_DELAY_ENABLE_HERMITE)
wet = delay.readAbsoluteHermiteWrapped(readPos);
#else
wet = delay.readAbsoluteLinearWrapped(readPos);
#endif
const float writePos = static_cast<float>(delay.writePos);
const float maxGuardBySize = 0.25f * delaySize;
const float guardSamples = clampf(maxGuardBySize, kOrbitReadGuardMinSamples, kOrbitReadGuardMaxSamples);

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.

medium

The guardSamples calculation floors at kOrbitReadGuardMinSamples (16), which can exceed maxGuardBySize (25% of the buffer) for small delay lines. If guardSamples becomes too large relative to delaySize, the read pointer may become stuck or the guard condition may always be true. The clamp should ensure the guard does not exceed the available safety margin.

Suggested change
const float guardSamples = clampf(maxGuardBySize, kOrbitReadGuardMinSamples, kOrbitReadGuardMaxSamples);
const float guardSamples = clampf(kOrbitReadGuardMaxSamples, kOrbitReadGuardMinSamples, maxGuardBySize);

const float distanceForward = wrapPosFloat(readPos - writePos, delaySize, invDelaySize);
if (distanceForward < guardSamples || distanceForward > (delaySize - guardSamples)) {
Comment on lines +638 to +640

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Cap read guard relative to delay size

Clamp guardSamples so it cannot exceed half the buffer size; with the current fixed minimum of 16 samples, any attached delay buffer smaller than 32 samples has no valid read region (distanceForward < guard or > size-guard is always true), so reads are forced to writePos + guard every sample. Since attachBufferMono/attachBuffers explicitly accept sizes down to 4, this regresses valid configurations by collapsing orbit/offset modulation into a fixed tap.

Useful? React with 👍 / 👎.

readPos = wrapPosFloat(writePos + guardSamples, delaySize, invDelaySize);
}
Comment on lines +637 to +642

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard size can collapse reads onto/near write position for small delay buffers

With Line 638 clamped to a minimum of 16 samples, guardSamples can exceed delaySize for valid small buffers, making the clamp branch always trigger and potentially wrapping to the write head. Clamp guard by buffer-size feasibility before use.

Suggested fix
-        const float maxGuardBySize = 0.25f * delaySize;
-        const float guardSamples = clampf(maxGuardBySize, kOrbitReadGuardMinSamples, kOrbitReadGuardMaxSamples);
+        const float maxGuardBySize = 0.25f * delaySize;
+        const float requestedGuard = clampf(maxGuardBySize, kOrbitReadGuardMinSamples, kOrbitReadGuardMaxSamples);
+        const float maxSafeGuard = clampf(0.5f * (delaySize - 1.0f), 1.0f, kOrbitReadGuardMaxSamples);
+        const float guardSamples = clampf(requestedGuard, 1.0f, maxSafeGuard);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const float maxGuardBySize = 0.25f * delaySize;
const float guardSamples = clampf(maxGuardBySize, kOrbitReadGuardMinSamples, kOrbitReadGuardMaxSamples);
const float distanceForward = wrapPosFloat(readPos - writePos, delaySize, invDelaySize);
if (distanceForward < guardSamples || distanceForward > (delaySize - guardSamples)) {
readPos = wrapPosFloat(writePos + guardSamples, delaySize, invDelaySize);
}
const float maxGuardBySize = 0.25f * delaySize;
const float requestedGuard = clampf(maxGuardBySize, kOrbitReadGuardMinSamples, kOrbitReadGuardMaxSamples);
const float maxSafeGuard = clampf(0.5f * (delaySize - 1.0f), 1.0f, kOrbitReadGuardMaxSamples);
const float guardSamples = clampf(requestedGuard, 1.0f, maxSafeGuard);
const float distanceForward = wrapPosFloat(readPos - writePos, delaySize, invDelaySize);
if (distanceForward < guardSamples || distanceForward > (delaySize - guardSamples)) {
readPos = wrapPosFloat(writePos + guardSamples, delaySize, invDelaySize);
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@core/src/orbit_delay_core.cpp` around lines 637 - 642, The guardSamples value
may exceed the actual delaySize for small buffers (causing reads to collapse
onto the write head); ensure guardSamples is bounded by the buffer feasibility
before use by clamping it against delaySize (e.g., limit to at most delaySize *
0.5f or delaySize - 1 sample equivalent) after computing maxGuardBySize and
before comparing to distanceForward; update the logic around
maxGuardBySize/guardSamples (and the subsequent use with wrapPosFloat, readPos,
writePos, invDelaySize) so guardSamples cannot be larger than the delay buffer
allows.


baseReadPos = readPos;
wet = readDelayWrapped(delay, readPos);
}

if (!isFiniteSafe(wet)) {
Expand Down Expand Up @@ -682,7 +692,8 @@ float OrbitDelayCore::processChannelFast(float input,
}
}

delay.write(toBuffer);
const float toBufferClipped = softClipPolynomial(toBuffer);
delay.write(toBufferClipped);

const float out = (sanitizedInput * (1.0f - params.mix) + filteredWet * params.mix) * params.outputGain;
return isFiniteSafe(out) ? out : 0.0f;
Expand Down