EngineEffectsDelay: effects chain delay handling

CMakeLists.txt

@@ -628,6 +628,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL
   src/engine/controls/ratecontrol.cpp
   src/engine/effects/engineeffect.cpp
   src/engine/effects/engineeffectchain.cpp
+  src/engine/effects/engineeffectsdelay.cpp
   src/engine/effects/engineeffectsmanager.cpp
   src/engine/enginebuffer.cpp
   src/engine/enginedelay.cpp
@@ -1645,6 +1646,7 @@ add_executable(mixxx-test
   #src/test/effectchainslottest.cpp
   src/test/enginebufferscalelineartest.cpp
   src/test/enginebuffertest.cpp
+  src/test/engineeffectsdelay_test.cpp
   src/test/enginefilterbiquadtest.cpp
   src/test/enginemastertest.cpp
   src/test/enginemicrophonetest.cpp

src/effects/backends/builtin/pitchshifteffect.cpp

@@ -97,7 +97,6 @@ void PitchShiftEffect::processChannel(
             pInput,
             engineParameters.framesPerBuffer());
     pState->m_pRubberBand->process(
-            //static_cast<const float* const*>(pState->m_retrieveBuffer),
             pState->m_retrieveBuffer,
             engineParameters.framesPerBuffer(),
             false);
@@ -107,7 +106,6 @@ void PitchShiftEffect::processChannel(
             framesAvailable,
             engineParameters.framesPerBuffer());
     SINT receivedFrames = pState->m_pRubberBand->retrieve(
-            //static_cast<float* const*>(pState->m_retrieveBuffer),
             pState->m_retrieveBuffer,
             framesToRead);
 

src/effects/backends/builtin/pitchshifteffect.h

@@ -10,6 +10,7 @@
 #include "util/class.h"
 #include "util/defs.h"
 #include "util/sample.h"
+#include "util/types.h"
 
 namespace RubberBand {
 class RubberBandStretcher;

src/effects/backends/effectprocessor.h

@@ -98,6 +98,18 @@ class EffectProcessor {
             const mixxx::EngineParameters& engineParameters,
             const EffectEnableState enableState,
             const GroupFeatureState& groupFeatures) = 0;
+
+    /// This method is used for obtaining the delay of the output buffer
+    /// compared to the input buffer based on the internal effect processing.
+    /// The method returns the number of frames by which the dry signal
+    /// needs to be delayed so that buffers for the dry and wet signal (output
+    /// of the effect) overlap. The return value represents the current effect
+    /// latency. The value is used in the EngineEffectChain::process method
+    /// to calculate the resulting latency of the effect chain. Based
+    /// on the sum of the delay value of every effect in the effect chain,
+    /// the dry signal is delayed to overlap with the output wet signal
+    /// after processing all effects in the effects chain.
+    virtual SINT getGroupDelayFrames() = 0;
 };
 
 /// EffectProcessorImpl manages a separate EffectState for every combination of
@@ -149,6 +161,12 @@ class EffectProcessorImpl : public EffectProcessor {
             const EffectEnableState enableState,
             const GroupFeatureState& groupFeatures) = 0;
 
+    /// By default, the group delay for every effect is zero. The effect implementation
+    /// can override this method and set actual number of frames for the effect delay.
+    virtual SINT getGroupDelayFrames() override {
+        return 0;
+    }
+
     void process(const ChannelHandle& inputHandle,
             const ChannelHandle& outputHandle,
             const CSAMPLE* pInput,

src/engine/effects/engineeffect.h

@@ -15,6 +15,7 @@
 #include "engine/effects/groupfeaturestate.h"
 #include "engine/effects/message.h"
 #include "util/memory.h"
+#include "util/types.h"
 
 /// EngineEffect is a generic wrapper around an EffectProcessor which intermediates
 /// between an EffectSlot and the EffectProcessor. It implements the logic to handle
@@ -63,6 +64,10 @@ class EngineEffect final : public EffectsRequestHandler {
         return m_pManifest->name();
     }
 
+    SINT getGroupDelayFrames() {
+        return m_pProcessor->getGroupDelayFrames();
+    }
+
   private:
     QString debugString() const {
         return QString("EngineEffect(%1)").arg(m_pManifest->name());

src/engine/effects/engineeffectchain.cpp

@@ -265,6 +265,7 @@ bool EngineEffectChain::process(const ChannelHandle& inputHandle,
         // requires that the input buffer does not get modified.
         CSAMPLE* pIntermediateInput = pIn;
         CSAMPLE* pIntermediateOutput;
+        SINT effectChainGroupDelayFrames = 0;
         bool firstAddDryToWetEffectProcessed = false;
 
         for (EngineEffect* pEffect : qAsConst(m_effects)) {
@@ -308,12 +309,17 @@ bool EngineEffectChain::process(const ChannelHandle& inputHandle,
                     }
 
                     processingOccured = true;
+                    effectChainGroupDelayFrames += pEffect->getGroupDelayFrames();
+
                     // Output of this effect becomes the input of the next effect
                     pIntermediateInput = pIntermediateOutput;
                 }
             }
         }
 
+        m_effectsDelay.setDelayFrames(effectChainGroupDelayFrames);
+        m_effectsDelay.process(pIn, numSamples);
+
         if (processingOccured) {
             // pIntermediateInput is the output of the last processed effect. It would be the
             // intermediate input of the next effect if there was one.

src/engine/effects/engineeffectchain.h

@@ -4,6 +4,7 @@
 #include <QString>
 
 #include "engine/channelhandle.h"
+#include "engine/effects/engineeffectsdelay.h"
 #include "engine/effects/groupfeaturestate.h"
 #include "engine/effects/message.h"
 #include "util/class.h"
@@ -81,6 +82,7 @@ class EngineEffectChain final : public EffectsRequestHandler {
     mixxx::SampleBuffer m_buffer1;
     mixxx::SampleBuffer m_buffer2;
     ChannelHandleMap<ChannelHandleMap<ChannelStatus>> m_chainStatusForChannelMatrix;
+    EngineEffectsDelay m_effectsDelay;
 
     DISALLOW_COPY_AND_ASSIGN(EngineEffectChain);
 };

src/engine/effects/engineeffectsdelay.cpp

@@ -0,0 +1,79 @@
+#include "engine/effects/engineeffectsdelay.h"
+
+#include "util/rampingvalue.h"
+#include "util/sample.h"
+
+EngineEffectsDelay::EngineEffectsDelay()
+        : m_currentDelaySamples(0),
+          m_prevDelaySamples(0),
+          m_delayBufferWritePos(0) {
+    m_pDelayBuffer = SampleUtil::alloc(kDelayBufferSize);
+    SampleUtil::clear(m_pDelayBuffer, kDelayBufferSize);
+}
+
+void EngineEffectsDelay::process(CSAMPLE* pInOut,
+        const int iBufferSize) {
+    if (m_prevDelaySamples == 0 && m_currentDelaySamples == 0) {
+        for (int i = 0; i < iBufferSize; ++i) {
+            // Put samples into delay buffer.
+            m_pDelayBuffer[m_delayBufferWritePos] = pInOut[i];
+            m_delayBufferWritePos = (m_delayBufferWritePos + 1) % kDelayBufferSize;
+        }
+
+        return;
+    }
+
+    // The "+ kDelayBufferSize" addition ensures positive values for the modulo calculation.
+    // From a mathematical point of view, this addition can be removed. Anyway,
+    // from the cpp point of view, the modulo operator for negative values
+    // (for example, x % y, where x is a negative value) produces negative results
+    // (but in math the result value is positive).
+    int delaySourcePos =
+            (m_delayBufferWritePos + kDelayBufferSize - m_currentDelaySamples) %
+            kDelayBufferSize;
+
+    if (m_prevDelaySamples == m_currentDelaySamples) {
+        for (int i = 0; i < iBufferSize; ++i) {
+            // Put samples into delay buffer.
+            m_pDelayBuffer[m_delayBufferWritePos] = pInOut[i];
+            m_delayBufferWritePos = (m_delayBufferWritePos + 1) % kDelayBufferSize;
+
+            // Take a delayed sample from the delay buffer
+            // and copy it to the destination buffer.
+            pInOut[i] = m_pDelayBuffer[delaySourcePos];
+            delaySourcePos = (delaySourcePos + 1) % kDelayBufferSize;
+        }
+
+    } else {
+        // The "+ kDelayBufferSize" addition ensures positive values for the modulo calculation.
+        // From a mathematical point of view, this addition can be removed. Anyway,
+        // from the cpp point of view, the modulo operator for negative values
+        // (for example, x % y, where x is a negative value) produces negative results
+        // (but in math the result value is positive).
+        int oldDelaySourcePos =
+                (m_delayBufferWritePos + kDelayBufferSize - m_prevDelaySamples) %
+                kDelayBufferSize;
+
+        const RampingValue<CSAMPLE_GAIN> delayChangeRamped(0.0f, 1.0f, iBufferSize);
+
+        for (int i = 0; i < iBufferSize; ++i) {
+            // Put samples into delay buffer.
+            m_pDelayBuffer[m_delayBufferWritePos] = pInOut[i];
+            m_delayBufferWritePos = (m_delayBufferWritePos + 1) % kDelayBufferSize;
+
+            // Take delayed samples from the delay buffer
+            // and with the use of ramping (cross-fading),
+            // calculate the result sample value
+            // and put it into the dest buffer.
+            CSAMPLE_GAIN crossMix = delayChangeRamped.getNth(i);
+
+            pInOut[i] = m_pDelayBuffer[oldDelaySourcePos] * (1.0f - crossMix);
+            pInOut[i] += m_pDelayBuffer[delaySourcePos] * crossMix;
+
+            oldDelaySourcePos = (oldDelaySourcePos + 1) % kDelayBufferSize;
+            delaySourcePos = (delaySourcePos + 1) % kDelayBufferSize;
+        }
+
+        m_prevDelaySamples = m_currentDelaySamples;
+    }
+}

src/engine/effects/engineeffectsdelay.h

@@ -0,0 +1,81 @@
+#pragma once
+
+#include "engine/engine.h"
+#include "engine/engineobject.h"
+#include "util/assert.h"
+#include "util/sample.h"
+#include "util/types.h"
+
+namespace {
+static constexpr int kMaxDelayFrames =
+        mixxx::audio::SampleRate::kValueMax - 1;
+static constexpr int kDelayBufferSize =
+        mixxx::audio::SampleRate::kValueMax * mixxx::kEngineChannelCount;
+} // anonymous namespace
+
+/// The effect can produce the output signal with a specific delay caused
+/// by the inner effect processing. Based on that the signal on the input
+/// of the effect does not overlap with the signal on the effect output.
+/// For using the effect in the Dry/Wet or Dry+Wet mode the dry signal
+/// and wet signal do not overlap and the effect latency is recognizable.
+///
+/// The EngineEffectsDelay offers a solution to this situation by delaying
+/// the input dry signal by a specific amount of delay. The signal delaying
+/// handles the class method EngineEffectsDelay::process. The method
+/// can be used for the one specific effect same as the effect chain if
+/// the set delay is the delay of the whole effect chain (sum of the delay
+/// of all effects in the effect chain).
+///
+/// After delaying the non-delayed signal, both signals (delayed
+/// and non-delayed) can be mixed and used together.
+class EngineEffectsDelay final : public EngineObject {
+  public:
+    EngineEffectsDelay();
+
+    virtual ~EngineEffectsDelay(){};
+
+    /// Called from the audio thread
+
+    /// The method sets the number of frames of which will
+    /// the EngineEffectsDelay::process method delays the input signal.
+    /// By default, the EngineEffectsDelay::process method works with
+    /// a zero delay. When is the delay set, the EngineEffectsDelay::process
+    /// method works with this set delay value until the value is changed.
+    void setDelayFrames(SINT delayFrames) {
+        VERIFY_OR_DEBUG_ASSERT(delayFrames >= 0) {
+            delayFrames = 0;
+        }
+        VERIFY_OR_DEBUG_ASSERT(delayFrames <= kMaxDelayFrames) {
+            delayFrames = kMaxDelayFrames;
+        }
+
+        // Delay is reported from the effect chain by a number of frames
+        // to aware problems with a number of channels. The inner
+        // EngineEffectsDelay structure works with delay samples, so the value
+        // is recalculated for the EngineEffectsDelay usage.
+        m_currentDelaySamples = delayFrames * mixxx::kEngineChannelCount;
+    }
+
+    /// The method delays the input buffer by the set number of samples
+    /// and returns the result in the output buffer. The input buffer
+    /// is not changed. For zero delay the input buffer is copied into
+    /// the output buffer. For non-zero delay, the output signal is returned
+    /// with the delay.
+    ///
+    /// If the number of delay samples hasn't changed, between two
+    /// EngineEffectsDelay::process method calls, the delayed signal buffers
+    /// directly follow each other as an input buffer, however, with a delay.
+    ///
+    /// If the number of delayed samples has changed between two
+    /// EngineEffectsDelay::process method calls, the new delay value is set
+    /// as actual and the output buffer is filled using cross-fading
+    /// of the presumed output buffer for the previous delay value
+    /// and of the output buffer created using the new delay value.
+    void process(CSAMPLE* pInOut, const int iBufferSize) override;
+
+  private:
+    SINT m_currentDelaySamples;
+    SINT m_prevDelaySamples;
+    SINT m_delayBufferWritePos;
+    CSAMPLE* m_pDelayBuffer;
+};