Fix quantization in the effect engine (metronome effect)

src/effects/backends/builtin/autopaneffect.cpp

@@ -88,10 +88,10 @@ void AutoPanEffect::processChannel(
     double period = m_pPeriodParameter->value();
     const auto smoothing = static_cast<float>(0.5 - m_pSmoothingParameter->value());
 
-    if (groupFeatures.has_beat_length_sec) {
+    if (groupFeatures.beat_length_frames.has_value()) {
         // period is a number of beats
         double beats = std::max(roundToFraction(period, 2), 0.25);
-        period = beats * groupFeatures.beat_length_sec * engineParameters.sampleRate();
+        period = beats * groupFeatures.beat_length_frames.value();
 
         // TODO(xxx) sync phase
         //if (groupFeatures.has_beat_fraction) {

src/effects/backends/builtin/biquadfullkilleqeffect.cpp

@@ -380,7 +380,7 @@ void BiquadFullKillEQEffect::processChannel(
         pState->m_highKill->pauseFilter();
     }
 
-    if (activeFilters == 0) {
+    if (activeFilters == 0 && pOutput != pInput) {
         SampleUtil::copy(pOutput, pInput, engineParameters.samplesPerBuffer());
     }
 

src/effects/backends/builtin/distortioneffect.cpp

@@ -105,7 +105,9 @@ void DistortionEffect::processChannel(
     CSAMPLE driveParam = static_cast<CSAMPLE>(m_pDrive->value());
 
     if (driveParam < 0.01) {
-        SampleUtil::copy(pOutput, pInput, numSamples);
+        if (pOutput != pInput) {
+            SampleUtil::copy(pOutput, pInput, numSamples);
+        }
         return;
     }
 
@@ -122,7 +124,9 @@ void DistortionEffect::processChannel(
 
     default:
         // We should never enter here, but we act as a noop effect just in case.
-        SampleUtil::copy(pOutput, pInput, numSamples);
+        if (pOutput != pInput) {
+            SampleUtil::copy(pOutput, pInput, numSamples);
+        }
         return;
     }
 }

src/effects/backends/builtin/echoeffect.cpp

@@ -132,7 +132,7 @@ void EchoEffect::processChannel(
     const auto pingpong_frac = static_cast<CSAMPLE_GAIN>(m_pPingPongParameter->value());
 
     int delay_frames;
-    if (groupFeatures.has_beat_length_sec) {
+    if (groupFeatures.beat_length_frames.has_value()) {
         // period is a number of beats
         if (m_pQuantizeParameter->toBool()) {
             period = std::max(roundToFraction(period, 4), 1 / 8.0);
@@ -142,8 +142,7 @@ void EchoEffect::processChannel(
         } else if (period < 1 / 8.0) {
             period = 1 / 8.0;
         }
-        delay_frames = static_cast<int>(period * groupFeatures.beat_length_sec *
-                engineParameters.sampleRate());
+        delay_frames = static_cast<int>(period * groupFeatures.beat_length_frames.value());
     } else {
         // period is a number of seconds
         period = std::max(period, 1 / 8.0);

src/effects/backends/builtin/flangereffect.cpp

@@ -117,14 +117,13 @@ void FlangerEffect::processChannel(
         const GroupFeatureState& groupFeatures) {
     double lfoPeriodParameter = m_pSpeedParameter->value();
     double lfoPeriodFrames;
-    if (groupFeatures.has_beat_length_sec) {
+    if (groupFeatures.beat_length_frames.has_value()) {
         // lfoPeriodParameter is a number of beats
         lfoPeriodParameter = std::max(roundToFraction(lfoPeriodParameter, 2.0), kMinLfoBeats);
         if (m_pTripletParameter->toBool()) {
             lfoPeriodParameter /= 3.0;
         }
-        lfoPeriodFrames = lfoPeriodParameter * groupFeatures.beat_length_sec *
-                engineParameters.sampleRate();
+        lfoPeriodFrames = lfoPeriodParameter * groupFeatures.beat_length_frames.value();
     } else {
         // lfoPeriodParameter is a number of seconds
         lfoPeriodFrames = std::max(lfoPeriodParameter, kMinLfoBeats) *

src/effects/backends/builtin/loudnesscontoureffect.cpp

@@ -109,9 +109,9 @@ void LoudnessContourEffect::processChannel(
     auto gain = static_cast<CSAMPLE_GAIN>(pState->m_oldGain);
 
     if (enableState != EffectEnableState::Disabling) {
-        bool useGain = m_pUseGain->toBool() && groupFeatures.has_gain;
+        bool useGain = m_pUseGain->toBool() && groupFeatures.gain.has_value();
         double loudness = m_pLoudness->value();
-        double gainKnob = groupFeatures.gain;
+        double gainKnob = groupFeatures.gain.value();
 
         filterGainDb = loudness;
 
@@ -141,7 +141,9 @@ void LoudnessContourEffect::processChannel(
     if (filterGainDb == 0) {
         pState->m_low->pauseFilter();
         pState->m_high->pauseFilter();
-        SampleUtil::copy(pOutput, pInput, engineParameters.samplesPerBuffer());
+        if (pOutput != pInput) {
+            SampleUtil::copy(pOutput, pInput, engineParameters.samplesPerBuffer());
+        }
     } else {
         pState->m_low->process(pInput, pOutput, engineParameters.samplesPerBuffer());
         pState->m_high->process(pOutput, pState->m_pBuf, engineParameters.samplesPerBuffer());

src/effects/backends/builtin/metronomeeffect.cpp

@@ -59,13 +59,13 @@ void MetronomeEffect::processChannel(
         const GroupFeatureState& groupFeatures) {
     Q_UNUSED(pInput);
 
-    MetronomeGroupState* gs = pGroupState;
-
     if (enableState == EffectEnableState::Disabled) {
-        gs->m_framesSinceClickStart = 0;
+        // assume click is fully played
         return;
     }
 
+    MetronomeGroupState* gs = pGroupState;
+
     SINT clickSize = kClickSize44100;
     const CSAMPLE* click = kClick44100;
     if (engineParameters.sampleRate() >= 96000) {
@@ -76,52 +76,77 @@ void MetronomeEffect::processChannel(
         click = kClick48000;
     }
 
-    SINT maxFrames;
-    if (m_pSyncParameter->toBool() && groupFeatures.has_beat_length_sec) {
-        maxFrames = static_cast<SINT>(
-                engineParameters.sampleRate() * groupFeatures.beat_length_sec);
-        if (groupFeatures.has_beat_fraction) {
-            const auto currentFrame = static_cast<SINT>(
-                    maxFrames * groupFeatures.beat_fraction);
-            if (maxFrames > clickSize &&
-                    currentFrame > clickSize &&
-                    currentFrame < maxFrames - clickSize &&
-                    gs->m_framesSinceClickStart > clickSize) {
-                // plays a single click on low speed
-                gs->m_framesSinceClickStart = currentFrame;
-            }
-        }
-    } else {
-        maxFrames = static_cast<SINT>(
-                engineParameters.sampleRate() * 60 / m_pBpmParameter->value());
+    if (pOutput != pInput) {
+        SampleUtil::copy(pOutput, pInput, engineParameters.samplesPerBuffer());
     }
 
-    SampleUtil::copy(pOutput, pInput, engineParameters.samplesPerBuffer());
+    if (enableState == EffectEnableState::Enabling) {
+        if (m_pSyncParameter->toBool() && groupFeatures.beat_fraction_buffer_end.has_value()) {
+            // Skip first click and sync phase
+            gs->m_framesSinceClickStart = clickSize;
+        } else {
+            // click right away after enabling
+            gs->m_framesSinceClickStart = 0;
+        }
+    }
 
     if (gs->m_framesSinceClickStart < clickSize) {
-        // still in click region, write remaining click frames.
+        // In click region, write remaining click frames.
         const SINT copyFrames =
                 math_min(engineParameters.framesPerBuffer(),
                         clickSize - gs->m_framesSinceClickStart);
         SampleUtil::addMonoToStereo(pOutput, &click[gs->m_framesSinceClickStart], copyFrames);
     }
 
-    gs->m_framesSinceClickStart += engineParameters.framesPerBuffer();
+    double bufferEnd = gs->m_framesSinceClickStart + engineParameters.framesPerBuffer();
+
+    double nextClickStart = bufferEnd; // default to "no new click";
+    if (m_pSyncParameter->toBool() && groupFeatures.beat_length_frames.has_value()) {
+        if (groupFeatures.beat_fraction_buffer_end.has_value()) {
+            double beatLength = groupFeatures.beat_length_frames.value();
+            if (groupFeatures.scratch_rate.has_value()) {
+                if (groupFeatures.scratch_rate.value() != 0.0) {
+                    beatLength /= groupFeatures.scratch_rate.value();
+                } else {
+                    beatLength = 0;
+                }
+            }
+
+            double beatToBufferEnd;
+            if (beatLength > 0) {
+                beatToBufferEnd =
+                        beatLength *
+                        groupFeatures.beat_fraction_buffer_end.value();
+            } else {
+                beatToBufferEnd =
+                        beatLength * -1 *
+                        (1 - groupFeatures.beat_fraction_buffer_end.value());
+            }
 
-    if (gs->m_framesSinceClickStart > maxFrames) {
-        // overflow, all overflowed frames are the start of a new click sound
-        gs->m_framesSinceClickStart -= maxFrames;
-        while (gs->m_framesSinceClickStart > engineParameters.framesPerBuffer()) {
-            // loop into a valid region, this happens if the maxFrames was lowered
-            gs->m_framesSinceClickStart -= engineParameters.framesPerBuffer();
+            if (bufferEnd > beatToBufferEnd) {
+                // We have a new beat before the current buffer ends
+                nextClickStart = bufferEnd - beatToBufferEnd;
+            }
+        } else {
+            nextClickStart = groupFeatures.beat_length_frames.value();
         }
+    } else {
+        nextClickStart = engineParameters.sampleRate() * 60 / m_pBpmParameter->value();
+    }
 
-        // gs->m_framesSinceClickStart matches now already at the first frame
-        // of next pOutput.
-        const unsigned int outputOffset =
-                engineParameters.framesPerBuffer() - gs->m_framesSinceClickStart;
-        const unsigned int copyFrames =
-                math_min(gs->m_framesSinceClickStart, clickSize);
-        SampleUtil::addMonoToStereo(&pOutput[outputOffset * 2], click, copyFrames);
+    if (bufferEnd > nextClickStart) {
+        // We need to start a new click
+        SINT outputOffset = static_cast<SINT>(nextClickStart) - gs->m_framesSinceClickStart;
+        if (outputOffset > 0) {
+            if (outputOffset < engineParameters.framesPerBuffer()) {
+                const SINT copyFrames =
+                        math_min(engineParameters.framesPerBuffer() - outputOffset, clickSize);
+                SampleUtil::addMonoToStereo(&pOutput[outputOffset * 2], &click[0], copyFrames);
+                gs->m_framesSinceClickStart = -outputOffset;
+            }
+        }
+    }
+    if (gs->m_framesSinceClickStart < engineParameters.framesPerBuffer() + clickSize) {
+        gs->m_framesSinceClickStart += engineParameters.framesPerBuffer();
     }
 }

src/effects/backends/builtin/parametriceqeffect.cpp

@@ -187,7 +187,9 @@ void ParametricEQEffect::processChannel(
             pState->m_bands[1]->process(pInput, pOutput, engineParameters.samplesPerBuffer());
         } else {
             pState->m_bands[1]->pauseFilter();
-            SampleUtil::copy(pOutput, pInput, engineParameters.samplesPerBuffer());
+            if (pOutput != pInput) {
+                SampleUtil::copy(pOutput, pInput, engineParameters.samplesPerBuffer());
+            }
         }
     }
 

src/effects/backends/builtin/phasereffect.cpp

@@ -132,14 +132,13 @@ void PhaserEffect::processChannel(
 
     double periodParameter = m_pLFOPeriodParameter->value();
     double periodSamples;
-    if (groupFeatures.has_beat_length_sec) {
+    if (groupFeatures.beat_length_frames.has_value()) {
         // periodParameter is a number of beats
         periodParameter = std::max(roundToFraction(periodParameter, 2.0), 1 / 4.0);
         if (m_pTripletParameter->toBool()) {
             periodParameter /= 3.0;
         }
-        periodSamples = periodParameter * groupFeatures.beat_length_sec *
-                engineParameters.sampleRate();
+        periodSamples = periodParameter * groupFeatures.beat_length_frames.value();
     } else {
         // periodParameter is a number of seconds
         periodSamples = std::max(periodParameter, 1 / 4.0) * engineParameters.sampleRate();

src/effects/backends/builtin/threebandbiquadeqeffect.cpp

@@ -351,7 +351,9 @@ void ThreeBandBiquadEQEffect::processChannel(
     }
 
     if (activeFilters == 0) {
-        SampleUtil::copy(pOutput, pInput, engineParameters.samplesPerBuffer());
+        if (pOutput != pInput) {
+            SampleUtil::copy(pOutput, pInput, engineParameters.samplesPerBuffer());
+        }
     }
 
     if (enableState == EffectEnableState::Disabling) {

src/effects/backends/builtin/tremoloeffect.cpp

@@ -138,9 +138,9 @@ void TremoloEffect::processChannel(
     bool tripletDisabling = pState->tripletEnabled && !m_pTripletParameter->toBool();
 
     if (enableState == EffectEnableState::Enabling || quantizeEnabling || tripletDisabling) {
-        if (gf.has_beat_length_sec && gf.has_beat_fraction) {
-            currentFrame = static_cast<unsigned int>(gf.beat_fraction *
-                    gf.beat_length_sec * engineParameters.sampleRate());
+        if (gf.beat_length_frames.has_value() && gf.beat_fraction_buffer_end.has_value()) {
+            currentFrame = static_cast<unsigned int>(gf.beat_fraction_buffer_end.value() *
+                    gf.beat_length_frames.value());
         } else {
             currentFrame = 0;
         }
@@ -149,7 +149,7 @@ void TremoloEffect::processChannel(
 
     int framePerPeriod;
     double rate = m_pRateParameter->value();
-    if (gf.has_beat_length_sec && gf.has_beat_fraction) {
+    if (gf.beat_length_frames.has_value() && gf.beat_fraction_buffer_end.has_value()) {
         if (m_pQuantizeParameter->toBool()) {
             const auto divider = static_cast<int>(log2(rate));
             rate = pow(2, divider);
@@ -158,8 +158,7 @@ void TremoloEffect::processChannel(
                 rate *= 3.0;
             }
         }
-        const auto framePerBeat = static_cast<int>(
-                gf.beat_length_sec * engineParameters.sampleRate());
+        const auto framePerBeat = static_cast<int>(gf.beat_length_frames.value());
         framePerPeriod = static_cast<int>(framePerBeat / rate);
     } else {
         framePerPeriod = static_cast<int>(engineParameters.sampleRate() / rate);

src/engine/controls/bpmcontrol.cpp

@@ -1085,7 +1085,7 @@ void BpmControl::resetSyncAdjustment() {
     m_resetSyncAdjustment = true;
 }
 
-void BpmControl::collectFeatures(GroupFeatureState* pGroupFeatures) const {
+void BpmControl::collectFeatures(GroupFeatureState* pGroupFeatures, double speed) const {
     // Without a beatgrid we don't know any beat details.
     FrameInfo info = frameInfo();
     if (!info.sampleRate.isValid() || !m_pBeats) {
@@ -1106,16 +1106,12 @@ void BpmControl::collectFeatures(GroupFeatureState* pGroupFeatures) const {
                 nextBeatPosition,
                 &beatLengthFrames,
                 &beatFraction)) {
-        const double framesPerSecond = info.sampleRate * m_pRateRatio->get();
-        if (framesPerSecond != 0.0) {
-            pGroupFeatures->beat_length_sec = beatLengthFrames / framesPerSecond;
-            pGroupFeatures->has_beat_length_sec = true;
-        } else {
-            pGroupFeatures->has_beat_length_sec = false;
+        const double rateRatio = m_pRateRatio->get();
+        if (rateRatio != 0.0) {
+            pGroupFeatures->beat_length_frames = beatLengthFrames / rateRatio;
+            pGroupFeatures->scratch_rate = speed / rateRatio;
         }
-
-        pGroupFeatures->has_beat_fraction = true;
-        pGroupFeatures->beat_fraction = beatFraction;
+        pGroupFeatures->beat_fraction_buffer_end = beatFraction;
     }
 }
 

src/engine/controls/bpmcontrol.h

@@ -64,7 +64,7 @@ class BpmControl : public EngineControl {
     /// override is used for seeks.
     double updateBeatDistance(mixxx::audio::FramePos playpos);
 
-    void collectFeatures(GroupFeatureState* pGroupFeatures) const;
+    void collectFeatures(GroupFeatureState* pGroupFeatures, double speed) const;
 
     // Calculates contextual information about beats: the previous beat, the
     // next beat, the current beat length, and the beat ratio (how far dPosition

src/engine/controls/quantizecontrol.cpp

@@ -67,8 +67,6 @@ void QuantizeControl::playPosChanged(mixxx::audio::FramePos position) {
     // We only need to update the prev or next if the current sample is
     // out of range of the existing beat positions or if we've been forced to
     // do so.
-    // NOTE: This bypasses the epsilon calculation, but is there a way
-    //       that could actually cause a problem?
     const auto prevBeatPosition =
             mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid(
                     m_pCOPrevBeat->get());
@@ -88,7 +86,7 @@ void QuantizeControl::lookupBeatPositions(mixxx::audio::FramePos position) {
     if (pBeats) {
         mixxx::audio::FramePos prevBeatPosition;
         mixxx::audio::FramePos nextBeatPosition;
-        pBeats->findPrevNextBeats(position, &prevBeatPosition, &nextBeatPosition, true);
+        pBeats->findPrevNextBeats(position, &prevBeatPosition, &nextBeatPosition, false);
         // FIXME: -1.0 is a valid frame position, should we set the COs to NaN?
         m_pCOPrevBeat->set(prevBeatPosition.toEngineSamplePosMaybeInvalid());
         m_pCONextBeat->set(nextBeatPosition.toEngineSamplePosMaybeInvalid());