From 8c07938b712c1be45b9f89896f5b1c0eb5d88ac7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 31 Oct 2017 22:47:45 +0100 Subject: [PATCH 01/35] Removed unused EngineControl::getTrigger() and added some missing override keywords --- src/engine/bpmcontrol.h | 6 +++--- src/engine/clockcontrol.h | 4 ++-- src/engine/cuecontrol.h | 4 ++-- src/engine/enginecontrol.cpp | 11 ----------- src/engine/enginecontrol.h | 7 +------ src/engine/keycontrol.h | 2 +- src/engine/loopingcontrol.cpp | 21 --------------------- src/engine/loopingcontrol.h | 21 +++++++-------------- src/engine/quantizecontrol.h | 6 +++--- src/engine/ratecontrol.h | 6 +++--- src/engine/sync/synccontrol.h | 2 +- src/test/readaheadmanager_test.cpp | 15 +-------------- 12 files changed, 24 insertions(+), 81 deletions(-) diff --git a/src/engine/bpmcontrol.h b/src/engine/bpmcontrol.h index 38a8541555c8..ae7b493178c9 100644 --- a/src/engine/bpmcontrol.h +++ b/src/engine/bpmcontrol.h @@ -20,7 +20,7 @@ class BpmControl : public EngineControl { public: BpmControl(QString group, UserSettingsPointer pConfig); - virtual ~BpmControl(); + ~BpmControl() override; double getBpm() const; double getLocalBpm() const { return m_pLocalBpm ? m_pLocalBpm->get() : 0.0; } @@ -38,11 +38,11 @@ class BpmControl : public EngineControl { double getBeatDistance(double dThisPosition) const; double getPreviousSample() const { return m_dPreviousSample; } - void setCurrentSample(const double dCurrentSample, const double dTotalSamples); + void setCurrentSample(const double dCurrentSample, const double dTotalSamples) override; double process(const double dRate, const double dCurrentSample, const double dTotalSamples, - const int iBufferSize); + const int iBufferSize) override; void setTargetBeatDistance(double beatDistance); void setInstantaneousBpm(double instantaneousBpm); void resetSyncAdjustment(); diff --git a/src/engine/clockcontrol.h b/src/engine/clockcontrol.h index 56ec15561635..614ad9a24e59 100644 --- a/src/engine/clockcontrol.h +++ b/src/engine/clockcontrol.h @@ -16,10 +16,10 @@ class ClockControl: public EngineControl { ClockControl(QString group, UserSettingsPointer pConfig); - virtual ~ClockControl(); + ~ClockControl() override; double process(const double dRate, const double currentSample, - const double totalSamples, const int iBufferSize); + const double totalSamples, const int iBufferSize) override; public slots: void trackLoaded(TrackPointer pNewTrack, TrackPointer pOldTrack) override; diff --git a/src/engine/cuecontrol.h b/src/engine/cuecontrol.h index b6356681fc87..e6d61ffe7d1a 100644 --- a/src/engine/cuecontrol.h +++ b/src/engine/cuecontrol.h @@ -86,9 +86,9 @@ class CueControl : public EngineControl { public: CueControl(QString group, UserSettingsPointer pConfig); - virtual ~CueControl(); + ~CueControl() override; - virtual void hintReader(HintVector* pHintList); + virtual void hintReader(HintVector* pHintList) override; bool updateIndicatorsAndModifyPlay(bool newPlay, bool playPossible); void updateIndicators(); bool isTrackAtCue(); diff --git a/src/engine/enginecontrol.cpp b/src/engine/enginecontrol.cpp index 045a987abd0e..9f35ec707e08 100644 --- a/src/engine/enginecontrol.cpp +++ b/src/engine/enginecontrol.cpp @@ -41,17 +41,6 @@ double EngineControl::nextTrigger(const double dRate, return kNoTrigger; } -double EngineControl::getTrigger(const double dRate, - const double currentSample, - const double totalSamples, - const int iBufferSize) { - Q_UNUSED(dRate); - Q_UNUSED(currentSample); - Q_UNUSED(totalSamples); - Q_UNUSED(iBufferSize); - return kNoTrigger; -} - void EngineControl::trackLoaded(TrackPointer pNewTrack, TrackPointer pOldTrack) { Q_UNUSED(pNewTrack); Q_UNUSED(pOldTrack); diff --git a/src/engine/enginecontrol.h b/src/engine/enginecontrol.h index a444be13945c..e207f4fef0d3 100644 --- a/src/engine/enginecontrol.h +++ b/src/engine/enginecontrol.h @@ -36,7 +36,7 @@ class EngineControl : public QObject { public: EngineControl(QString group, UserSettingsPointer pConfig); - virtual ~EngineControl(); + ~EngineControl() override; // Called by EngineBuffer::process every latency period. See the above // comments for information about guarantees that hold during this call. An @@ -54,11 +54,6 @@ class EngineControl : public QObject { const double dTotalSamples, const int iBufferSize); - virtual double getTrigger(const double dRate, - const double dCurrentSample, - const double dTotalSamples, - const int iBufferSize); - // hintReader allows the EngineControl to provide hints to the reader to // indicate that the given portion of a song is a potential imminent seek // target. diff --git a/src/engine/keycontrol.h b/src/engine/keycontrol.h index b3fc6dea0a85..b97f3567c240 100644 --- a/src/engine/keycontrol.h +++ b/src/engine/keycontrol.h @@ -25,7 +25,7 @@ class KeyControl : public EngineControl { }; KeyControl(QString group, UserSettingsPointer pConfig); - virtual ~KeyControl(); + ~KeyControl() override; // Returns a struct, with the results of the last pitch and tempo calculations KeyControl::PitchTempoRatio getPitchTempoRatio(); diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index 6c162fcbaf47..98d199ae5724 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -365,27 +365,6 @@ double LoopingControl::nextTrigger(const double dRate, return kNoTrigger; } -double LoopingControl::getTrigger(const double dRate, - const double currentSample, - const double totalSamples, - const int iBufferSize) { - Q_UNUSED(currentSample); - Q_UNUSED(totalSamples); - Q_UNUSED(iBufferSize); - bool bReverse = dRate < 0; - LoopSamples loopSamples = m_loopSamples.getValue(); - - if (m_bLoopingEnabled && !m_bReloopCatchUpcomingLoop && - !m_bAdjustingLoopIn && !m_bAdjustingLoopOut) { - if (bReverse) { - return loopSamples.end; - } else { - return loopSamples.start; - } - } - return kNoTrigger; -} - void LoopingControl::hintReader(HintVector* pHintList) { LoopSamples loopSamples = m_loopSamples.getValue(); Hint loop_hint; diff --git a/src/engine/loopingcontrol.h b/src/engine/loopingcontrol.h index 39aa977119bd..9436b1395c22 100644 --- a/src/engine/loopingcontrol.h +++ b/src/engine/loopingcontrol.h @@ -28,35 +28,28 @@ class LoopingControl : public EngineControl { static QList getBeatSizes(); LoopingControl(QString group, UserSettingsPointer pConfig); - virtual ~LoopingControl(); + ~LoopingControl() override; // process() updates the internal state of the LoopingControl to reflect the // correct current sample. If a loop should be taken LoopingControl returns // the sample that should be seeked to. Otherwise it returns currentSample. - virtual double process(const double dRate, + double process(const double dRate, const double currentSample, const double totalSamples, - const int iBufferSize); + const int iBufferSize) override; // nextTrigger returns the sample at which the engine will be triggered to // take a loop, given the value of currentSample and dRate. - virtual double nextTrigger(const double dRate, + double nextTrigger(const double dRate, const double currentSample, const double totalSamples, - const int iBufferSize); - - // getTrigger returns the sample that the engine will next be triggered to - // loop to, given the value of currentSample and dRate. - virtual double getTrigger(const double dRate, - const double currentSample, - const double totalSamples, - const int iBufferSize); + const int iBufferSize) override; // hintReader will add to hintList hints both the loop in and loop out // sample, if set. - virtual void hintReader(HintVector* pHintList); + void hintReader(HintVector* pHintList) override; - virtual void notifySeek(double dNewPlaypos); + void notifySeek(double dNewPlaypos) override; public slots: void slotLoopIn(double pressed); diff --git a/src/engine/quantizecontrol.h b/src/engine/quantizecontrol.h index a76321683d05..76baef63ebf0 100644 --- a/src/engine/quantizecontrol.h +++ b/src/engine/quantizecontrol.h @@ -16,10 +16,10 @@ class QuantizeControl : public EngineControl { Q_OBJECT public: QuantizeControl(QString group, UserSettingsPointer pConfig); - virtual ~QuantizeControl(); + ~QuantizeControl() override; - virtual void setCurrentSample(const double dCurrentSample, - const double dTotalSamples); + void setCurrentSample(const double dCurrentSample, + const double dTotalSamples) override; public slots: void trackLoaded(TrackPointer pNewTrack, TrackPointer pOldTrack) override; diff --git a/src/engine/ratecontrol.h b/src/engine/ratecontrol.h index 3341b7f51f79..85d30ecbf652 100644 --- a/src/engine/ratecontrol.h +++ b/src/engine/ratecontrol.h @@ -32,7 +32,7 @@ class RateControl : public EngineControl { Q_OBJECT public: RateControl(QString group, UserSettingsPointer pConfig); - virtual ~RateControl(); + ~RateControl() override; void setBpmControl(BpmControl* bpmcontrol); // Must be called during each callback of the audio thread so that @@ -40,7 +40,7 @@ class RateControl : public EngineControl { double process(const double dRate, const double currentSample, const double totalSamples, - const int bufferSamples); + const int bufferSamples) override; // Returns the current engine rate. "reportScratching" is used to tell // the caller that the user is currently scratching, and this is used to // disable keylock. @@ -61,7 +61,7 @@ class RateControl : public EngineControl { static void setRateRamp(bool); // Set Rate Ramp Sensitivity static void setRateRampSensitivity(int); - virtual void notifySeek(double dNewPlaypos); + void notifySeek(double dNewPlaypos) override; public slots: void slotReverseRollActivate(double); diff --git a/src/engine/sync/synccontrol.h b/src/engine/sync/synccontrol.h index 446ed9ea85c2..7af10a30489c 100644 --- a/src/engine/sync/synccontrol.h +++ b/src/engine/sync/synccontrol.h @@ -22,7 +22,7 @@ class SyncControl : public EngineControl, public Syncable { static const double kBpmDouble; SyncControl(const QString& group, UserSettingsPointer pConfig, EngineChannel* pChannel, SyncableListener* pEngineSync); - virtual ~SyncControl(); + ~SyncControl() override; const QString& getGroup() const { return m_sGroup; } EngineChannel* getChannel() const { return m_pChannel; } diff --git a/src/test/readaheadmanager_test.cpp b/src/test/readaheadmanager_test.cpp index afdb5dd90c23..7303c3c3ee54 100644 --- a/src/test/readaheadmanager_test.cpp +++ b/src/test/readaheadmanager_test.cpp @@ -63,25 +63,12 @@ class StubLoopControl : public LoopingControl { return m_processReturnValues.takeFirst(); } - // getTrigger returns the sample that the engine will next be triggered to - // loop to, given the value of currentSample and dRate. - double getTrigger(const double dRate, - const double currentSample, - const double totalSamples, - const int iBufferSize) override { - Q_UNUSED(dRate); - Q_UNUSED(currentSample); - Q_UNUSED(totalSamples); - Q_UNUSED(iBufferSize); - return kNoTrigger; - } - // hintReader has no effect in this stubbed class void hintReader(HintVector* pHintList) override { Q_UNUSED(pHintList); } - void notifySeek(double dNewPlaypos) { + void notifySeek(double dNewPlaypos) override { Q_UNUSED(dNewPlaypos); } From 137635287a137a1cc2713b25c43e1a3db38c4893 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 31 Oct 2017 22:57:46 +0100 Subject: [PATCH 02/35] removed EngineControl::getNextTrigger() since it is only implemented in LoopingControl --- src/engine/enginecontrol.cpp | 11 ----------- src/engine/enginecontrol.h | 5 ----- src/engine/loopingcontrol.h | 4 ++-- 3 files changed, 2 insertions(+), 18 deletions(-) diff --git a/src/engine/enginecontrol.cpp b/src/engine/enginecontrol.cpp index 9f35ec707e08..9593c31d178b 100644 --- a/src/engine/enginecontrol.cpp +++ b/src/engine/enginecontrol.cpp @@ -30,17 +30,6 @@ double EngineControl::process(const double dRate, return kNoTrigger; } -double EngineControl::nextTrigger(const double dRate, - const double currentSample, - const double totalSamples, - const int iBufferSize) { - Q_UNUSED(dRate); - Q_UNUSED(currentSample); - Q_UNUSED(totalSamples); - Q_UNUSED(iBufferSize); - return kNoTrigger; -} - void EngineControl::trackLoaded(TrackPointer pNewTrack, TrackPointer pOldTrack) { Q_UNUSED(pNewTrack); Q_UNUSED(pOldTrack); diff --git a/src/engine/enginecontrol.h b/src/engine/enginecontrol.h index e207f4fef0d3..e2dee10dbb40 100644 --- a/src/engine/enginecontrol.h +++ b/src/engine/enginecontrol.h @@ -49,11 +49,6 @@ class EngineControl : public QObject { const double dTotalSamples, const int iBufferSize); - virtual double nextTrigger(const double dRate, - const double dCurrentSample, - const double dTotalSamples, - const int iBufferSize); - // hintReader allows the EngineControl to provide hints to the reader to // indicate that the given portion of a song is a potential imminent seek // target. diff --git a/src/engine/loopingcontrol.h b/src/engine/loopingcontrol.h index 9436b1395c22..1f3cc1765985 100644 --- a/src/engine/loopingcontrol.h +++ b/src/engine/loopingcontrol.h @@ -40,10 +40,10 @@ class LoopingControl : public EngineControl { // nextTrigger returns the sample at which the engine will be triggered to // take a loop, given the value of currentSample and dRate. - double nextTrigger(const double dRate, + virtual double nextTrigger(const double dRate, const double currentSample, const double totalSamples, - const int iBufferSize) override; + const int iBufferSize); // hintReader will add to hintList hints both the loop in and loop out // sample, if set. From 34f4cb96dfecbc6ce4451c4059f9e44403d0d6e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 31 Oct 2017 23:23:10 +0100 Subject: [PATCH 03/35] removed m_bReloopCatchUpcomingLoop, after setting it always true to solve bug #1716897 --- src/engine/loopingcontrol.cpp | 32 ++++++++++++++------------------ src/engine/loopingcontrol.h | 1 - 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index 98d199ae5724..2b2c07d992d0 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -321,24 +321,23 @@ double LoopingControl::process(const double dRate, double retval = kNoTrigger; LoopSamples loopSamples = m_loopSamples.getValue(); - if (m_bLoopingEnabled && - loopSamples.start != kNoTrigger && - loopSamples.end != kNoTrigger) { - bool outsideLoop = currentSample >= loopSamples.end || - currentSample <= loopSamples.start; - if (outsideLoop) { - if (!m_bReloopCatchUpcomingLoop && !m_bAdjustingLoopIn && !m_bAdjustingLoopOut) { - retval = reverse ? loopSamples.end : loopSamples.start; - } - } else { - m_bReloopCatchUpcomingLoop = false; - } - } if (m_bAdjustingLoopIn) { setLoopInToCurrentPosition(); } else if (m_bAdjustingLoopOut) { setLoopOutToCurrentPosition(); + } else if (m_bLoopingEnabled && + loopSamples.start != kNoTrigger && + loopSamples.end != kNoTrigger) { + if (reverse) { + if (currentSample <= loopSamples.start) { + retval = loopSamples.end; + } + } else { + if (currentSample >= loopSamples.end) { + retval = loopSamples.start; + } + } } return retval; @@ -354,7 +353,7 @@ double LoopingControl::nextTrigger(const double dRate, bool bReverse = dRate < 0; LoopSamples loopSamples = m_loopSamples.getValue(); - if (m_bLoopingEnabled && !m_bReloopCatchUpcomingLoop && + if (m_bLoopingEnabled && !m_bAdjustingLoopIn && !m_bAdjustingLoopOut) { if (bReverse) { return loopSamples.start; @@ -629,13 +628,10 @@ void LoopingControl::slotReloopToggle(double val) { LoopSamples loopSamples = m_loopSamples.getValue(); if (loopSamples.start != kNoTrigger && loopSamples.end != kNoTrigger && loopSamples.start <= loopSamples.end) { - if (getCurrentSample() < loopSamples.start) { - m_bReloopCatchUpcomingLoop = true; - } setLoopingEnabled(true); // If we're not playing, jump to the loop in point so the waveform // shows where it will play from when playback resumes. - if (!m_pPlayButton->toBool() && !m_bReloopCatchUpcomingLoop) { + if (!m_pPlayButton->toBool() && getCurrentSample() > loopSamples.start) { slotLoopInGoto(1); } } diff --git a/src/engine/loopingcontrol.h b/src/engine/loopingcontrol.h index 1f3cc1765985..5d3de6377f28 100644 --- a/src/engine/loopingcontrol.h +++ b/src/engine/loopingcontrol.h @@ -128,7 +128,6 @@ class LoopingControl : public EngineControl { bool m_bLoopingEnabled = false; bool m_bLoopRollActive = false; bool m_bLoopManualTogglePressedToExitLoop = false; - bool m_bReloopCatchUpcomingLoop = false; bool m_bAdjustingLoopIn = false; bool m_bAdjustingLoopOut = false; bool m_bLoopOutPressedWhileLoopDisabled = false; From f862b39fedd277da78afa6f4949cc4e522591a16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 31 Oct 2017 23:29:20 +0100 Subject: [PATCH 04/35] move some member variables to initaizer list --- src/engine/loopingcontrol.cpp | 8 +++++++- src/engine/loopingcontrol.h | 12 ++++++------ 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index 2b2c07d992d0..ce65115cebe9 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -39,7 +39,13 @@ QList LoopingControl::getBeatSizes() { LoopingControl::LoopingControl(QString group, UserSettingsPointer pConfig) - : EngineControl(group, pConfig) { + : EngineControl(group, pConfig), + m_bLoopingEnabled(false), + m_bLoopRollActive(false), + m_bLoopManualTogglePressedToExitLoop(false), + m_bAdjustingLoopIn(false), + m_bAdjustingLoopOut(false), + m_bLoopOutPressedWhileLoopDisabled(false) { LoopSamples loopSamples = { kNoTrigger, kNoTrigger }; m_loopSamples.setValue(loopSamples); m_iCurrentSample = 0; diff --git a/src/engine/loopingcontrol.h b/src/engine/loopingcontrol.h index 5d3de6377f28..bcf74ea8b0d9 100644 --- a/src/engine/loopingcontrol.h +++ b/src/engine/loopingcontrol.h @@ -125,12 +125,12 @@ class LoopingControl : public EngineControl { ControlObject* m_pSlipEnabled; ControlObject* m_pPlayButton; - bool m_bLoopingEnabled = false; - bool m_bLoopRollActive = false; - bool m_bLoopManualTogglePressedToExitLoop = false; - bool m_bAdjustingLoopIn = false; - bool m_bAdjustingLoopOut = false; - bool m_bLoopOutPressedWhileLoopDisabled = false; + bool m_bLoopingEnabled; + bool m_bLoopRollActive; + bool m_bLoopManualTogglePressedToExitLoop; + bool m_bAdjustingLoopIn; + bool m_bAdjustingLoopOut; + bool m_bLoopOutPressedWhileLoopDisabled; // TODO(DSC) Make the following values double ControlValueAtomic m_loopSamples; QAtomicInt m_iCurrentSample; From c4bf90b575033384c6d10fcb00cdffdcb7e40135 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Wed, 1 Nov 2017 22:31:42 +0100 Subject: [PATCH 05/35] Allow rounding issues when exit the loop by seek, attempt to fix bug 1669500 --- src/engine/loopingcontrol.cpp | 4 +++- src/engine/readaheadmanager.cpp | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index ce65115cebe9..bef93af5ceb4 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -728,7 +728,9 @@ void LoopingControl::notifySeek(double dNewPlaypos) { if (m_bLoopingEnabled) { // Disable loop when we jump after it, using hot cues or waveform overview // If we jump before, the loop it is kept enabled as catching loop - if (dNewPlaypos > loopSamples.end) { + // + 2 because the new playposition can be rounded to loop end because of + // playing not at rate 1. + if (dNewPlaypos > loopSamples.end + 2) { setLoopingEnabled(false); } } diff --git a/src/engine/readaheadmanager.cpp b/src/engine/readaheadmanager.cpp index 915329cae480..26167ac44113 100644 --- a/src/engine/readaheadmanager.cpp +++ b/src/engine/readaheadmanager.cpp @@ -216,8 +216,9 @@ void ReadAheadManager::addReadLogEntry(double virtualPlaypositionStart, } // Not thread-save, call from engine thread only -double ReadAheadManager::getFilePlaypositionFromLog(double currentFilePlayposition, - double numConsumedSamples) { +double ReadAheadManager::getFilePlaypositionFromLog( + double currentFilePlayposition, double numConsumedSamples) { + if (numConsumedSamples == 0) { return currentFilePlayposition; } From 42f2b79b455a5979d7007e6c230d76b3b8010e81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Thu, 2 Nov 2017 22:21:53 +0100 Subject: [PATCH 06/35] rmoved unused m_bLoopManualTogglePressedToExitLoop --- src/engine/loopingcontrol.cpp | 1 - src/engine/loopingcontrol.h | 1 - src/engine/readaheadmanager.cpp | 9 +++++---- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index bef93af5ceb4..6da99a421347 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -42,7 +42,6 @@ LoopingControl::LoopingControl(QString group, : EngineControl(group, pConfig), m_bLoopingEnabled(false), m_bLoopRollActive(false), - m_bLoopManualTogglePressedToExitLoop(false), m_bAdjustingLoopIn(false), m_bAdjustingLoopOut(false), m_bLoopOutPressedWhileLoopDisabled(false) { diff --git a/src/engine/loopingcontrol.h b/src/engine/loopingcontrol.h index bcf74ea8b0d9..e4a7d5b66870 100644 --- a/src/engine/loopingcontrol.h +++ b/src/engine/loopingcontrol.h @@ -127,7 +127,6 @@ class LoopingControl : public EngineControl { bool m_bLoopingEnabled; bool m_bLoopRollActive; - bool m_bLoopManualTogglePressedToExitLoop; bool m_bAdjustingLoopIn; bool m_bAdjustingLoopOut; bool m_bLoopOutPressedWhileLoopDisabled; diff --git a/src/engine/readaheadmanager.cpp b/src/engine/readaheadmanager.cpp index 26167ac44113..a3d6f9c51b9a 100644 --- a/src/engine/readaheadmanager.cpp +++ b/src/engine/readaheadmanager.cpp @@ -66,8 +66,8 @@ SINT ReadAheadManager::getNextSamples(double dRate, CSAMPLE* pOutput, } else { // We can only read whole frames from the reader. // Use ceil here, to be sure to reach the loop trigger. - preloop_samples = SampleUtil::ceilPlayPosToFrameStart(samplesToLoopTrigger, - kNumChannels); + preloop_samples = SampleUtil::ceilPlayPosToFrameStart( + samplesToLoopTrigger, kNumChannels); // clamp requested samples from the caller to the loop trigger point samples_from_reader = math_clamp(requested_samples, static_cast(0), preloop_samples); @@ -103,12 +103,13 @@ SINT ReadAheadManager::getNextSamples(double dRate, CSAMPLE* pOutput, // Activate on this trigger if necessary if (loop_active) { - // LoopingControl makes the decision about whether we should loop or - // not. + // LoopingControl makes the decision about whether we should jump to + // the other end of the loop or not const double loop_target = m_pLoopingControl->process( dRate, m_currentPosition, 0, 0); if (loop_target != kNoTrigger) { + // Jump to other end of loop. m_currentPosition = loop_target; if (preloop_samples > 0) { // we are up to one frame ahead of the loop trigger From 3b4a6ab6e0b35e88e1a0e3eb43778a429083ecc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Fri, 3 Nov 2017 00:01:45 +0100 Subject: [PATCH 07/35] split process function for only one call per callback --- src/engine/loopingcontrol.cpp | 20 +++++++++++++++----- src/engine/loopingcontrol.h | 4 ++++ src/engine/readaheadmanager.cpp | 4 ++-- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index 6da99a421347..13ad09f38aa4 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -315,6 +315,7 @@ double LoopingControl::process(const double dRate, const int iBufferSize) { Q_UNUSED(totalSamples); Q_UNUSED(iBufferSize); + Q_UNUSED(dRate); int currentSampleEven = static_cast(currentSample); if (!even(currentSampleEven)) { @@ -322,16 +323,25 @@ double LoopingControl::process(const double dRate, } m_iCurrentSample = currentSampleEven; + if (m_bAdjustingLoopIn) { + setLoopInToCurrentPosition(); + } else if (m_bAdjustingLoopOut) { + setLoopOutToCurrentPosition(); + } + + return kNoTrigger; +} + +double LoopingControl::getLoopTarget( + const double dRate, const double currentSample) { bool reverse = dRate < 0; double retval = kNoTrigger; LoopSamples loopSamples = m_loopSamples.getValue(); - if (m_bAdjustingLoopIn) { - setLoopInToCurrentPosition(); - } else if (m_bAdjustingLoopOut) { - setLoopOutToCurrentPosition(); - } else if (m_bLoopingEnabled && + if (!m_bAdjustingLoopIn && + !m_bAdjustingLoopOut && + m_bLoopingEnabled && loopSamples.start != kNoTrigger && loopSamples.end != kNoTrigger) { if (reverse) { diff --git a/src/engine/loopingcontrol.h b/src/engine/loopingcontrol.h index e4a7d5b66870..c0f277e33df3 100644 --- a/src/engine/loopingcontrol.h +++ b/src/engine/loopingcontrol.h @@ -38,6 +38,9 @@ class LoopingControl : public EngineControl { const double totalSamples, const int iBufferSize) override; + double getLoopTarget( + const double dRate, const double currentSample); + // nextTrigger returns the sample at which the engine will be triggered to // take a loop, given the value of currentSample and dRate. virtual double nextTrigger(const double dRate, @@ -126,6 +129,7 @@ class LoopingControl : public EngineControl { ControlObject* m_pPlayButton; bool m_bLoopingEnabled; + bool m_bLoopSamplesChanged; bool m_bLoopRollActive; bool m_bAdjustingLoopIn; bool m_bAdjustingLoopOut; diff --git a/src/engine/readaheadmanager.cpp b/src/engine/readaheadmanager.cpp index a3d6f9c51b9a..98848ad045fa 100644 --- a/src/engine/readaheadmanager.cpp +++ b/src/engine/readaheadmanager.cpp @@ -105,8 +105,8 @@ SINT ReadAheadManager::getNextSamples(double dRate, CSAMPLE* pOutput, if (loop_active) { // LoopingControl makes the decision about whether we should jump to // the other end of the loop or not - const double loop_target = m_pLoopingControl->process( - dRate, m_currentPosition, 0, 0); + const double loop_target = m_pLoopingControl->getLoopTarget( + dRate, m_currentPosition); if (loop_target != kNoTrigger) { // Jump to other end of loop. From 4c4ae2097dd62414e630da1b2c2a4f389e42e5ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Fri, 3 Nov 2017 00:03:53 +0100 Subject: [PATCH 08/35] disable loop when jumping out of it in reverse direction --- src/engine/loopingcontrol.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index 13ad09f38aa4..b2ee596a66f3 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -732,14 +732,14 @@ void LoopingControl::slotLoopEndPos(double pos) { m_loopSamples.setValue(loopSamples); } +// This is called from the engine thread void LoopingControl::notifySeek(double dNewPlaypos) { LoopSamples loopSamples = m_loopSamples.getValue(); if (m_bLoopingEnabled) { - // Disable loop when we jump after it, using hot cues or waveform overview - // If we jump before, the loop it is kept enabled as catching loop + // Disable loop when we jumping out, using hot cues or waveform overview. // + 2 because the new playposition can be rounded to loop end because of // playing not at rate 1. - if (dNewPlaypos > loopSamples.end + 2) { + if (dNewPlaypos < loopSamples.start - 2 || dNewPlaypos > loopSamples.end + 2) { setLoopingEnabled(false); } } From d7a6e92aec69c14ce04176088ab88e45e26a543b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sat, 4 Nov 2017 23:27:15 +0100 Subject: [PATCH 09/35] Allow scratching in front of an enabled loop --- src/engine/loopingcontrol.cpp | 20 ++++++++++++++++---- src/engine/loopingcontrol.h | 2 +- src/engine/readaheadmanager.cpp | 7 ++----- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index b2ee596a66f3..c83e330d21bf 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -45,8 +45,8 @@ LoopingControl::LoopingControl(QString group, m_bAdjustingLoopIn(false), m_bAdjustingLoopOut(false), m_bLoopOutPressedWhileLoopDisabled(false) { - LoopSamples loopSamples = { kNoTrigger, kNoTrigger }; - m_loopSamples.setValue(loopSamples); + m_oldLoopSamples = { kNoTrigger, kNoTrigger }; + m_loopSamples.setValue(m_oldLoopSamples); m_iCurrentSample = 0; m_pActiveBeatLoop = NULL; @@ -332,24 +332,36 @@ double LoopingControl::process(const double dRate, return kNoTrigger; } +// This must be called only once from the engine thread double LoopingControl::getLoopTarget( const double dRate, const double currentSample) { bool reverse = dRate < 0; double retval = kNoTrigger; + bool loopMoved = false; LoopSamples loopSamples = m_loopSamples.getValue(); + if (loopSamples.start != m_oldLoopSamples.start || + loopSamples.end != m_oldLoopSamples.end) { + loopMoved = true; + m_oldLoopSamples = loopSamples; + } if (!m_bAdjustingLoopIn && !m_bAdjustingLoopOut && m_bLoopingEnabled && loopSamples.start != kNoTrigger && loopSamples.end != kNoTrigger) { + // We jump the loop, if we are passing the end, or if the loop was moved + // out of play position + // The 2 is for rounding. if (reverse) { - if (currentSample <= loopSamples.start) { + if (currentSample <= loopSamples.start && + (loopMoved || currentSample >= loopSamples.start - 2)) { retval = loopSamples.end; } } else { - if (currentSample >= loopSamples.end) { + if (currentSample >= loopSamples.end && + (loopMoved || currentSample <= loopSamples.end + 2)) { retval = loopSamples.start; } } diff --git a/src/engine/loopingcontrol.h b/src/engine/loopingcontrol.h index c0f277e33df3..c24d7a3b8e8c 100644 --- a/src/engine/loopingcontrol.h +++ b/src/engine/loopingcontrol.h @@ -129,13 +129,13 @@ class LoopingControl : public EngineControl { ControlObject* m_pPlayButton; bool m_bLoopingEnabled; - bool m_bLoopSamplesChanged; bool m_bLoopRollActive; bool m_bAdjustingLoopIn; bool m_bAdjustingLoopOut; bool m_bLoopOutPressedWhileLoopDisabled; // TODO(DSC) Make the following values double ControlValueAtomic m_loopSamples; + LoopSamples m_oldLoopSamples; QAtomicInt m_iCurrentSample; ControlObject* m_pQuantizeEnabled; ControlObject* m_pNextBeat; diff --git a/src/engine/readaheadmanager.cpp b/src/engine/readaheadmanager.cpp index 98848ad045fa..ce7dc96a5fdc 100644 --- a/src/engine/readaheadmanager.cpp +++ b/src/engine/readaheadmanager.cpp @@ -60,10 +60,7 @@ SINT ReadAheadManager::getNextSamples(double dRate, CSAMPLE* pOutput, samplesToLoopTrigger = in_reverse ? m_currentPosition - loop_trigger : loop_trigger - m_currentPosition; - if (samplesToLoopTrigger < 0) { - // We have already passed the loop trigger - samples_from_reader = 0; - } else { + if (samplesToLoopTrigger > 0) { // We can only read whole frames from the reader. // Use ceil here, to be sure to reach the loop trigger. preloop_samples = SampleUtil::ceilPlayPosToFrameStart( @@ -238,8 +235,8 @@ double ReadAheadManager::getFilePlaypositionFromLog( // Notify EngineControls that we have taken a seek. // Every new entry start with a seek + // (Not looping control) if (shouldNotifySeek) { - m_pLoopingControl->notifySeek(entry.virtualPlaypositionStart); if (m_pRateControl) { m_pRateControl->notifySeek(entry.virtualPlaypositionStart); } From d9c69070c6ee0e44daa63e964c2ed3800deca059 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sat, 4 Nov 2017 23:47:17 +0100 Subject: [PATCH 10/35] Only disable the loop when actually jumping out of one. --- src/engine/loopingcontrol.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index c83e330d21bf..794b28b3711d 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -751,7 +751,9 @@ void LoopingControl::notifySeek(double dNewPlaypos) { // Disable loop when we jumping out, using hot cues or waveform overview. // + 2 because the new playposition can be rounded to loop end because of // playing not at rate 1. - if (dNewPlaypos < loopSamples.start - 2 || dNewPlaypos > loopSamples.end + 2) { + if (m_iCurrentSample >= loopSamples.start - 2 && + m_iCurrentSample <= loopSamples.end + 2 && + (dNewPlaypos < loopSamples.start - 2 || dNewPlaypos > loopSamples.end + 2)) { setLoopingEnabled(false); } } From b064901d641898d7b63f77e5557872c571d60464 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sun, 5 Nov 2017 00:06:48 +0100 Subject: [PATCH 11/35] Allow to enable a stored loop --- src/engine/loopingcontrol.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index 794b28b3711d..d8aec43f3cd9 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -676,9 +676,7 @@ void LoopingControl::slotReloopAndStop(double pressed) { } void LoopingControl::slotLoopStartPos(double pos) { - if (!m_pTrack) { - return; - } + // This slot is called before trackLoaded() for a new Track int newpos = pos; if (newpos != kNoTrigger && !even(newpos)) { @@ -711,9 +709,7 @@ void LoopingControl::slotLoopStartPos(double pos) { } void LoopingControl::slotLoopEndPos(double pos) { - if (!m_pTrack) { - return; - } + // This slot is called before trackLoaded() for a new Track int newpos = pos; if (newpos != -1 && !even(newpos)) { From 719314e020fd3a4b6bcc5ca08f694b7bf0852ef8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Mon, 6 Nov 2017 22:09:30 +0100 Subject: [PATCH 12/35] fix reloop toggle behind a loop --- src/engine/loopingcontrol.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index d8aec43f3cd9..c32e48045479 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -656,9 +656,7 @@ void LoopingControl::slotReloopToggle(double val) { if (loopSamples.start != kNoTrigger && loopSamples.end != kNoTrigger && loopSamples.start <= loopSamples.end) { setLoopingEnabled(true); - // If we're not playing, jump to the loop in point so the waveform - // shows where it will play from when playback resumes. - if (!m_pPlayButton->toBool() && getCurrentSample() > loopSamples.start) { + if (getCurrentSample() > loopSamples.end) { slotLoopInGoto(1); } } From ae5f20a334b6ed6a0eaada47fa644bf9480386a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Mon, 6 Nov 2017 23:12:47 +0100 Subject: [PATCH 13/35] Fix one test and remove two obolete tests --- src/engine/loopingcontrol.h | 2 +- src/test/readaheadmanager_test.cpp | 31 +++++++----------------------- 2 files changed, 8 insertions(+), 25 deletions(-) diff --git a/src/engine/loopingcontrol.h b/src/engine/loopingcontrol.h index c24d7a3b8e8c..367600b310d6 100644 --- a/src/engine/loopingcontrol.h +++ b/src/engine/loopingcontrol.h @@ -38,7 +38,7 @@ class LoopingControl : public EngineControl { const double totalSamples, const int iBufferSize) override; - double getLoopTarget( + virtual double getLoopTarget( const double dRate, const double currentSample); // nextTrigger returns the sample at which the engine will be triggered to diff --git a/src/test/readaheadmanager_test.cpp b/src/test/readaheadmanager_test.cpp index 7303c3c3ee54..78815df0c0fc 100644 --- a/src/test/readaheadmanager_test.cpp +++ b/src/test/readaheadmanager_test.cpp @@ -63,6 +63,13 @@ class StubLoopControl : public LoopingControl { return m_processReturnValues.takeFirst(); } + double getLoopTarget(const double dRate, + const double dCurrentSample) override { + Q_UNUSED(dRate); + RELEASE_ASSERT(!m_processReturnValues.isEmpty()); + return m_processReturnValues.takeFirst(); + } + // hintReader has no effect in this stubbed class void hintReader(HintVector* pHintList) override { Q_UNUSED(pHintList); @@ -101,30 +108,6 @@ class ReadAheadManagerTest : public MixxxTest { CSAMPLE* m_pBuffer; }; -TEST_F(ReadAheadManagerTest, LoopEnableSeekBackward) { - // If a loop is enabled and the current playposition is ahead of the loop, - // we should seek to the beginning of the loop. - m_pReadAheadManager->notifySeek(110); - // Trigger value means, the sample that triggers the loop (loop out) - m_pLoopControl->pushTriggerReturnValue(100); - // Process value is the sample we should seek to - m_pLoopControl->pushProcessReturnValue(10); - EXPECT_EQ(0, m_pReadAheadManager->getNextSamples(1.0, m_pBuffer, 100)); - EXPECT_EQ(10, m_pReadAheadManager->getPlaypos()); -} - -TEST_F(ReadAheadManagerTest, InReverseLoopEnableSeekForward) { - // If we are in reverse, a loop is enabled, and the current playposition - // is before of the loop, we should seek to the out point of the loop. - m_pReadAheadManager->notifySeek(1); - // Trigger value means, the sample that triggers the loop (loop in) - m_pLoopControl->pushTriggerReturnValue(10); - // Process value is the sample we should seek to. - m_pLoopControl->pushProcessReturnValue(100); - EXPECT_EQ(0, m_pReadAheadManager->getNextSamples(-1.0, m_pBuffer, 100)); - EXPECT_EQ(100, m_pReadAheadManager->getPlaypos()); -} - TEST_F(ReadAheadManagerTest, FractionalFrameLoop) { // If we are in reverse, a loop is enabled, and the current playposition // is before of the loop, we should seek to the out point of the loop. From e68383944bcc25b60c3a119bd4c63a86847d364a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 7 Nov 2017 22:30:00 +0100 Subject: [PATCH 14/35] Don't adjust playposition when moving a catching loop. --- src/engine/loopingcontrol.cpp | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index c32e48045479..52eca50ab82b 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -338,11 +338,14 @@ double LoopingControl::getLoopTarget( bool reverse = dRate < 0; double retval = kNoTrigger; - bool loopMoved = false; + bool loopMovedOut = false; LoopSamples loopSamples = m_loopSamples.getValue(); if (loopSamples.start != m_oldLoopSamples.start || loopSamples.end != m_oldLoopSamples.end) { - loopMoved = true; + if (currentSample >= loopSamples.start - 2 && + (currentSample <= loopSamples.end + 2)) { + loopMovedOut = true; + } m_oldLoopSamples = loopSamples; } @@ -356,12 +359,12 @@ double LoopingControl::getLoopTarget( // The 2 is for rounding. if (reverse) { if (currentSample <= loopSamples.start && - (loopMoved || currentSample >= loopSamples.start - 2)) { + (loopMovedOut || currentSample >= loopSamples.start - 2)) { retval = loopSamples.end; } } else { if (currentSample >= loopSamples.end && - (loopMoved || currentSample <= loopSamples.end + 2)) { + (loopMovedOut || currentSample <= loopSamples.end + 2)) { retval = loopSamples.start; } } @@ -1123,6 +1126,12 @@ void LoopingControl::seekInsideAdjustedLoop(int old_loop_in, int old_loop_out, // Copy on stack since m_iCurrentSample sample can change under us. int currentSample = m_iCurrentSample; if (currentSample >= new_loop_in && currentSample <= new_loop_out) { + // playposition already is inside the loop + return; + } + if (currentSample < old_loop_in - 2 && currentSample <= new_loop_out) { + // Playposition was before a catching loop and is still a catching loop + // nothing to do return; } @@ -1130,11 +1139,6 @@ void LoopingControl::seekInsideAdjustedLoop(int old_loop_in, int old_loop_out, if (!even(new_loop_size)) { --new_loop_size; } - if (new_loop_size > old_loop_out - old_loop_in) { - // Could this happen if the user grows a loop and then also shifts it? - qWarning() << "seekInsideAdjustedLoop called for loop that got larger -- ignoring"; - return; - } int adjusted_position = currentSample; while (adjusted_position > new_loop_out) { From 093e4eae1f96fdbdd5a6ba34ee10e341d0c12eb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 7 Nov 2017 22:44:05 +0100 Subject: [PATCH 15/35] Disable a Loop when jumping over it using hot-cues --- src/engine/loopingcontrol.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index 52eca50ab82b..43fe495ac923 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -745,12 +745,19 @@ void LoopingControl::slotLoopEndPos(double pos) { void LoopingControl::notifySeek(double dNewPlaypos) { LoopSamples loopSamples = m_loopSamples.getValue(); if (m_bLoopingEnabled) { - // Disable loop when we jumping out, using hot cues or waveform overview. + // Disable loop when we jumping out, or over a catching loop, + // using hot cues or waveform overview. // + 2 because the new playposition can be rounded to loop end because of // playing not at rate 1. if (m_iCurrentSample >= loopSamples.start - 2 && m_iCurrentSample <= loopSamples.end + 2 && - (dNewPlaypos < loopSamples.start - 2 || dNewPlaypos > loopSamples.end + 2)) { + dNewPlaypos < loopSamples.start - 2) { + // jumping out of loop in backwards + setLoopingEnabled(false); + } + if (m_iCurrentSample <= loopSamples.end + 2 && + dNewPlaypos > loopSamples.end + 2) { + // jumping out a loop or over a catching loop forward setLoopingEnabled(false); } } From 21fc021c977a563aecfab068623909cb4ad60e57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Wed, 8 Nov 2017 21:49:35 +0100 Subject: [PATCH 16/35] Move setting loop samples before juming into new loop --- src/engine/loopingcontrol.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index 43fe495ac923..7c352b7115a0 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -1013,6 +1013,10 @@ void LoopingControl::slotBeatLoop(double beats, bool keepStartPoint, bool enable return; } + m_loopSamples.setValue(newloopSamples); + m_pCOLoopStartPosition->set(newloopSamples.start); + m_pCOLoopEndPosition->set(newloopSamples.end); + // If resizing an inactive loop by changing beatloop_size, // do not seek to the adjusted loop. if (keepStartPoint && (enable || m_bLoopingEnabled)) { @@ -1020,9 +1024,6 @@ void LoopingControl::slotBeatLoop(double beats, bool keepStartPoint, bool enable newloopSamples.start, newloopSamples.end); } - m_loopSamples.setValue(newloopSamples); - m_pCOLoopStartPosition->set(newloopSamples.start); - m_pCOLoopEndPosition->set(newloopSamples.end); if (enable) { setLoopingEnabled(true); } @@ -1114,10 +1115,10 @@ void LoopingControl::slotLoopMove(double beats) { } loopSamples.start = new_loop_in; - m_pCOLoopStartPosition->set(new_loop_in); loopSamples.end = new_loop_out; - m_pCOLoopEndPosition->set(new_loop_out); m_loopSamples.setValue(loopSamples); + m_pCOLoopStartPosition->set(new_loop_in); + m_pCOLoopEndPosition->set(new_loop_out); // If we are looping make sure that the play head does not leave the // loop as a result of our adjustment. From 92028d0f0ca5061313cd6e55d3db482c135036be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Wed, 8 Nov 2017 22:24:20 +0100 Subject: [PATCH 17/35] simplified some loopingcontrol functions --- src/engine/loopingcontrol.cpp | 16 +++++----------- src/engine/loopingcontrol.h | 8 +++----- src/engine/readaheadmanager.cpp | 4 ++-- src/test/readaheadmanager_test.cpp | 15 ++++++--------- 4 files changed, 16 insertions(+), 27 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index 7c352b7115a0..752ef9cfce8a 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -334,9 +334,7 @@ double LoopingControl::process(const double dRate, // This must be called only once from the engine thread double LoopingControl::getLoopTarget( - const double dRate, const double currentSample) { - bool reverse = dRate < 0; - + bool reverse, const double currentSample) { double retval = kNoTrigger; bool loopMovedOut = false; LoopSamples loopSamples = m_loopSamples.getValue(); @@ -373,19 +371,15 @@ double LoopingControl::getLoopTarget( return retval; } -double LoopingControl::nextTrigger(const double dRate, - const double currentSample, - const double totalSamples, - const int iBufferSize) { +double LoopingControl::nextTrigger(bool reverse, + const double currentSample) { Q_UNUSED(currentSample); - Q_UNUSED(totalSamples); - Q_UNUSED(iBufferSize); - bool bReverse = dRate < 0; + LoopSamples loopSamples = m_loopSamples.getValue(); if (m_bLoopingEnabled && !m_bAdjustingLoopIn && !m_bAdjustingLoopOut) { - if (bReverse) { + if (reverse) { return loopSamples.start; } else { return loopSamples.end; diff --git a/src/engine/loopingcontrol.h b/src/engine/loopingcontrol.h index 367600b310d6..f979e26f40e5 100644 --- a/src/engine/loopingcontrol.h +++ b/src/engine/loopingcontrol.h @@ -39,14 +39,12 @@ class LoopingControl : public EngineControl { const int iBufferSize) override; virtual double getLoopTarget( - const double dRate, const double currentSample); + bool reverse, const double currentSample); // nextTrigger returns the sample at which the engine will be triggered to // take a loop, given the value of currentSample and dRate. - virtual double nextTrigger(const double dRate, - const double currentSample, - const double totalSamples, - const int iBufferSize); + virtual double nextTrigger(bool reverse, + const double currentSample); // hintReader will add to hintList hints both the loop in and loop out // sample, if set. diff --git a/src/engine/readaheadmanager.cpp b/src/engine/readaheadmanager.cpp index ce7dc96a5fdc..9652354d761b 100644 --- a/src/engine/readaheadmanager.cpp +++ b/src/engine/readaheadmanager.cpp @@ -50,7 +50,7 @@ SINT ReadAheadManager::getNextSamples(double dRate, CSAMPLE* pOutput, // A loop will only limit the amount we can read in one shot. const double loop_trigger = m_pLoopingControl->nextTrigger( - dRate, m_currentPosition, 0, 0); + in_reverse, m_currentPosition); bool loop_active = loop_trigger != kNoTrigger; SINT preloop_samples = 0; double samplesToLoopTrigger = 0.0; @@ -103,7 +103,7 @@ SINT ReadAheadManager::getNextSamples(double dRate, CSAMPLE* pOutput, // LoopingControl makes the decision about whether we should jump to // the other end of the loop or not const double loop_target = m_pLoopingControl->getLoopTarget( - dRate, m_currentPosition); + in_reverse, m_currentPosition); if (loop_target != kNoTrigger) { // Jump to other end of loop. diff --git a/src/test/readaheadmanager_test.cpp b/src/test/readaheadmanager_test.cpp index 78815df0c0fc..0089ae58f87f 100644 --- a/src/test/readaheadmanager_test.cpp +++ b/src/test/readaheadmanager_test.cpp @@ -39,14 +39,10 @@ class StubLoopControl : public LoopingControl { m_processReturnValues.push_back(value); } - double nextTrigger(const double dRate, - const double currentSample, - const double totalSamples, - const int iBufferSize) override { - Q_UNUSED(dRate); + double nextTrigger(bool reverse, + const double currentSample) override { + Q_UNUSED(reverse); Q_UNUSED(currentSample); - Q_UNUSED(totalSamples); - Q_UNUSED(iBufferSize); RELEASE_ASSERT(!m_triggerReturnValues.isEmpty()); return m_triggerReturnValues.takeFirst(); } @@ -63,9 +59,10 @@ class StubLoopControl : public LoopingControl { return m_processReturnValues.takeFirst(); } - double getLoopTarget(const double dRate, + double getLoopTarget(bool reverse, const double dCurrentSample) override { - Q_UNUSED(dRate); + Q_UNUSED(reverse); + Q_UNUSED(dCurrentSample); RELEASE_ASSERT(!m_processReturnValues.isEmpty()); return m_processReturnValues.takeFirst(); } From 6ecc23fc45943ea154de7d12f830f315833aabbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Thu, 9 Nov 2017 23:19:58 +0100 Subject: [PATCH 18/35] store current samples in m_engineLoopSamples to avoid a change in between --- src/engine/loopingcontrol.cpp | 4 +++- src/engine/loopingcontrol.h | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index 752ef9cfce8a..45a6c7bb6374 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -337,7 +337,7 @@ double LoopingControl::getLoopTarget( bool reverse, const double currentSample) { double retval = kNoTrigger; bool loopMovedOut = false; - LoopSamples loopSamples = m_loopSamples.getValue(); + LoopSamples loopSamples = m_engineLoopSamples; if (loopSamples.start != m_oldLoopSamples.start || loopSamples.end != m_oldLoopSamples.end) { if (currentSample >= loopSamples.start - 2 && @@ -376,6 +376,8 @@ double LoopingControl::nextTrigger(bool reverse, Q_UNUSED(currentSample); LoopSamples loopSamples = m_loopSamples.getValue(); + // Store loop samples for later getLoopTarget() + m_engineLoopSamples = loopSamples; if (m_bLoopingEnabled && !m_bAdjustingLoopIn && !m_bAdjustingLoopOut) { diff --git a/src/engine/loopingcontrol.h b/src/engine/loopingcontrol.h index f979e26f40e5..0661efe2e8e5 100644 --- a/src/engine/loopingcontrol.h +++ b/src/engine/loopingcontrol.h @@ -134,6 +134,7 @@ class LoopingControl : public EngineControl { // TODO(DSC) Make the following values double ControlValueAtomic m_loopSamples; LoopSamples m_oldLoopSamples; + LoopSamples m_engineLoopSamples; QAtomicInt m_iCurrentSample; ControlObject* m_pQuantizeEnabled; ControlObject* m_pNextBeat; From bc0ef8428ae75f92c241fb4e82a0db975a0c9fc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sat, 11 Nov 2017 15:36:51 +0100 Subject: [PATCH 19/35] Calculate the seek and the jump target inside a loop in a single nextTrigger() call from the engine. This simpifies the control flow a lot and fixes races, between scheduled seeks and loop jumps. --- src/engine/loopingcontrol.cpp | 121 ++++++++++++----------------- src/engine/loopingcontrol.h | 11 ++- src/engine/readaheadmanager.cpp | 91 +++++++++++----------- src/test/looping_control_test.cpp | 30 +++++-- src/test/readaheadmanager_test.cpp | 44 ++++------- 5 files changed, 137 insertions(+), 160 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index 45a6c7bb6374..b46e26b883c7 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -45,7 +45,7 @@ LoopingControl::LoopingControl(QString group, m_bAdjustingLoopIn(false), m_bAdjustingLoopOut(false), m_bLoopOutPressedWhileLoopDisabled(false) { - m_oldLoopSamples = { kNoTrigger, kNoTrigger }; + m_oldLoopSamples = { kNoTrigger, kNoTrigger, false }; m_loopSamples.setValue(m_oldLoopSamples); m_iCurrentSample = 0; m_pActiveBeatLoop = NULL; @@ -247,7 +247,6 @@ void LoopingControl::slotLoopScale(double scaleFactor) { return; } int loop_length = loopSamples.end - loopSamples.start; - int old_loop_end = loopSamples.end; int samples = m_pTrackSamples->get(); loop_length *= scaleFactor; @@ -280,17 +279,13 @@ void LoopingControl::slotLoopScale(double scaleFactor) { loopSamples.end = samples; } + // Reseek if the loop shrank out from under the playposition. + loopSamples.seek = (m_bLoopingEnabled && scaleFactor < 1.0); + m_loopSamples.setValue(loopSamples); // Update CO for loop end marker m_pCOLoopEndPosition->set(loopSamples.end); - - // Reseek if the loop shrank out from under the playposition. - if (m_bLoopingEnabled && scaleFactor < 1.0) { - seekInsideAdjustedLoop( - loopSamples.start, old_loop_end, - loopSamples.start, loopSamples.end); - } } void LoopingControl::slotLoopHalve(double pressed) { @@ -332,58 +327,37 @@ double LoopingControl::process(const double dRate, return kNoTrigger; } -// This must be called only once from the engine thread -double LoopingControl::getLoopTarget( - bool reverse, const double currentSample) { - double retval = kNoTrigger; - bool loopMovedOut = false; - LoopSamples loopSamples = m_engineLoopSamples; - if (loopSamples.start != m_oldLoopSamples.start || - loopSamples.end != m_oldLoopSamples.end) { - if (currentSample >= loopSamples.start - 2 && - (currentSample <= loopSamples.end + 2)) { - loopMovedOut = true; - } - m_oldLoopSamples = loopSamples; - } - - if (!m_bAdjustingLoopIn && - !m_bAdjustingLoopOut && - m_bLoopingEnabled && - loopSamples.start != kNoTrigger && - loopSamples.end != kNoTrigger) { - // We jump the loop, if we are passing the end, or if the loop was moved - // out of play position - // The 2 is for rounding. - if (reverse) { - if (currentSample <= loopSamples.start && - (loopMovedOut || currentSample >= loopSamples.start - 2)) { - retval = loopSamples.end; - } - } else { - if (currentSample >= loopSamples.end && - (loopMovedOut || currentSample <= loopSamples.end + 2)) { - retval = loopSamples.start; - } - } - } - - return retval; -} - double LoopingControl::nextTrigger(bool reverse, - const double currentSample) { + const double currentSample, + double *pTarget) { Q_UNUSED(currentSample); + *pTarget = kNoTrigger; + LoopSamples loopSamples = m_loopSamples.getValue(); - // Store loop samples for later getLoopTarget() + // Store loop samples for subsequent getLoopTarget() call m_engineLoopSamples = loopSamples; + if (loopSamples.seek) { + // here the loop has changed and the play position + // should be moved with it + *pTarget = seekInsideAdjustedLoop(currentSample, + m_oldLoopSamples.start, loopSamples.start, loopSamples.end); + if (*pTarget != kNoTrigger) { + // jump immediately + return currentSample; + } + } + if (m_bLoopingEnabled && - !m_bAdjustingLoopIn && !m_bAdjustingLoopOut) { + !m_bAdjustingLoopIn && !m_bAdjustingLoopOut && + loopSamples.start != kNoTrigger && + loopSamples.end != kNoTrigger) { if (reverse) { + *pTarget = loopSamples.end; return loopSamples.start; } else { + *pTarget = loopSamples.start; return loopSamples.end; } } @@ -472,6 +446,8 @@ void LoopingControl::setLoopInToCurrentPosition() { } loopSamples.start = pos; + loopSamples.seek = false; + m_pCOLoopStartPosition->set(loopSamples.start); if (m_pQuantizeEnabled->toBool() @@ -565,6 +541,8 @@ void LoopingControl::setLoopOutToCurrentPosition() { // set loop out position loopSamples.end = pos; + loopSamples.seek = false; + m_pCOLoopEndPosition->set(loopSamples.end); m_loopSamples.setValue(loopSamples); @@ -693,6 +671,7 @@ void LoopingControl::slotLoopStartPos(double pos) { setLoopingEnabled(false); } + loopSamples.seek = false; loopSamples.start = newpos; m_pCOLoopStartPosition->set(newpos); @@ -733,6 +712,7 @@ void LoopingControl::slotLoopEndPos(double pos) { setLoopingEnabled(false); } loopSamples.end = newpos; + loopSamples.seek = false; m_pCOLoopEndPosition->set(newpos); m_loopSamples.setValue(loopSamples); } @@ -908,7 +888,7 @@ void LoopingControl::slotBeatLoop(double beats, bool keepStartPoint, bool enable // Calculate the new loop start and end samples // give start and end defaults so we can detect problems - LoopSamples newloopSamples = {kNoTrigger, kNoTrigger}; + LoopSamples newloopSamples = {kNoTrigger, kNoTrigger, false}; LoopSamples loopSamples = m_loopSamples.getValue(); // Start from the current position/closest beat and @@ -1009,17 +989,14 @@ void LoopingControl::slotBeatLoop(double beats, bool keepStartPoint, bool enable return; } + // If resizing an inactive loop by changing beatloop_size, + // do not seek to the adjusted loop. + newloopSamples.seek = (keepStartPoint && (enable || m_bLoopingEnabled)); + m_loopSamples.setValue(newloopSamples); m_pCOLoopStartPosition->set(newloopSamples.start); m_pCOLoopEndPosition->set(newloopSamples.end); - // If resizing an inactive loop by changing beatloop_size, - // do not seek to the adjusted loop. - if (keepStartPoint && (enable || m_bLoopingEnabled)) { - seekInsideAdjustedLoop(loopSamples.start, loopSamples.end, - newloopSamples.start, newloopSamples.end); - } - if (enable) { setLoopingEnabled(true); } @@ -1110,33 +1087,31 @@ void LoopingControl::slotLoopMove(double beats) { --new_loop_out; } + // If we are looping make sure that the play head does not leave the + // loop as a result of our adjustment. + loopSamples.seek = m_bLoopingEnabled; + loopSamples.start = new_loop_in; loopSamples.end = new_loop_out; m_loopSamples.setValue(loopSamples); m_pCOLoopStartPosition->set(new_loop_in); m_pCOLoopEndPosition->set(new_loop_out); - - // If we are looping make sure that the play head does not leave the - // loop as a result of our adjustment. - if (m_bLoopingEnabled) { - seekInsideAdjustedLoop(old_loop_in, old_loop_out, - new_loop_in, new_loop_out); - } } } -void LoopingControl::seekInsideAdjustedLoop(int old_loop_in, int old_loop_out, - int new_loop_in, int new_loop_out) { +// Must be called from the engine thread only +int LoopingControl::seekInsideAdjustedLoop( + int currentSample, int old_loop_in, + int new_loop_in, int new_loop_out) { // Copy on stack since m_iCurrentSample sample can change under us. - int currentSample = m_iCurrentSample; if (currentSample >= new_loop_in && currentSample <= new_loop_out) { // playposition already is inside the loop - return; + return kNoTrigger; } if (currentSample < old_loop_in - 2 && currentSample <= new_loop_out) { // Playposition was before a catching loop and is still a catching loop // nothing to do - return; + return kNoTrigger; } int new_loop_size = new_loop_out - new_loop_in; @@ -1167,7 +1142,9 @@ void LoopingControl::seekInsideAdjustedLoop(int old_loop_in, int old_loop_out, } if (adjusted_position != currentSample) { m_iCurrentSample = adjusted_position; - seekAbs(static_cast(adjusted_position)); + return adjusted_position; + } else { + return kNoTrigger; } } diff --git a/src/engine/loopingcontrol.h b/src/engine/loopingcontrol.h index 0661efe2e8e5..8d605fc7340e 100644 --- a/src/engine/loopingcontrol.h +++ b/src/engine/loopingcontrol.h @@ -38,13 +38,11 @@ class LoopingControl : public EngineControl { const double totalSamples, const int iBufferSize) override; - virtual double getLoopTarget( - bool reverse, const double currentSample); - // nextTrigger returns the sample at which the engine will be triggered to // take a loop, given the value of currentSample and dRate. virtual double nextTrigger(bool reverse, - const double currentSample); + const double currentSample, + double *pTarget); // hintReader will add to hintList hints both the loop in and loop out // sample, if set. @@ -93,6 +91,7 @@ class LoopingControl : public EngineControl { struct LoopSamples { int start; int end; + bool seek; }; void setLoopingEnabled(bool enabled); @@ -105,8 +104,8 @@ class LoopingControl : public EngineControl { // When a loop changes size such that the playposition is outside of the loop, // we can figure out the best place in the new loop to seek to maintain // the beat. It will even keep multi-bar phrasing correct with 4/4 tracks. - void seekInsideAdjustedLoop(int old_loop_in, int old_loop_out, - int new_loop_in, int new_loop_out); + int seekInsideAdjustedLoop(int currentSample, + int old_loop_in, int new_loop_in, int new_loop_out); ControlPushButton* m_pCOBeatLoopActivate; ControlPushButton* m_pCOBeatLoopRollActivate; diff --git a/src/engine/readaheadmanager.cpp b/src/engine/readaheadmanager.cpp index 9652354d761b..e10ae22e8b74 100644 --- a/src/engine/readaheadmanager.cpp +++ b/src/engine/readaheadmanager.cpp @@ -47,27 +47,31 @@ SINT ReadAheadManager::getNextSamples(double dRate, CSAMPLE* pOutput, //qDebug() << "start" << start_sample << requested_samples; - + double target; // A loop will only limit the amount we can read in one shot. const double loop_trigger = m_pLoopingControl->nextTrigger( - in_reverse, m_currentPosition); - bool loop_active = loop_trigger != kNoTrigger; + in_reverse, m_currentPosition, &target); + SINT preloop_samples = 0; double samplesToLoopTrigger = 0.0; + bool reachedTrigger = false; + SINT samples_from_reader = requested_samples; - if (loop_active) { + if (loop_trigger != kNoTrigger) { samplesToLoopTrigger = in_reverse ? m_currentPosition - loop_trigger : loop_trigger - m_currentPosition; - if (samplesToLoopTrigger > 0) { + if (samplesToLoopTrigger >= 0.0) { // We can only read whole frames from the reader. // Use ceil here, to be sure to reach the loop trigger. preloop_samples = SampleUtil::ceilPlayPosToFrameStart( samplesToLoopTrigger, kNumChannels); // clamp requested samples from the caller to the loop trigger point - samples_from_reader = math_clamp(requested_samples, - static_cast(0), preloop_samples); + if (preloop_samples <= requested_samples) { + reachedTrigger = true; + samples_from_reader = preloop_samples; + } } } @@ -99,49 +103,44 @@ SINT ReadAheadManager::getNextSamples(double dRate, CSAMPLE* pOutput, } // Activate on this trigger if necessary - if (loop_active) { - // LoopingControl makes the decision about whether we should jump to - // the other end of the loop or not - const double loop_target = m_pLoopingControl->getLoopTarget( - in_reverse, m_currentPosition); - - if (loop_target != kNoTrigger) { - // Jump to other end of loop. - m_currentPosition = loop_target; - if (preloop_samples > 0) { - // we are up to one frame ahead of the loop trigger - double overshoot = preloop_samples - samplesToLoopTrigger; - // start the loop later accordingly to be sure the loop length is as desired - // e.g. exactly one bar. - m_currentPosition += overshoot; - - // Example in frames; - // loop start 1.1 loop end 3.3 loop length 2.2 - // m_currentPosition samplesToLoopTrigger preloop_samples - // 2.0 1.3 2 - // 1.8 1.5 2 - // 1.6 1.7 2 - // 1.4 1.9 2 - // 1.2 2.1 3 - // Average preloop_samples = 2.2 - } + if (reachedTrigger) { + DEBUG_ASSERT(target != kNoTrigger); + + // Jump to other end of loop. + m_currentPosition = target; + if (preloop_samples > 0) { + // we are up to one frame ahead of the loop trigger + double overshoot = preloop_samples - samplesToLoopTrigger; + // start the loop later accordingly to be sure the loop length is as desired + // e.g. exactly one bar. + m_currentPosition += overshoot; + + // Example in frames; + // loop start 1.1 loop end 3.3 loop length 2.2 + // m_currentPosition samplesToLoopTrigger preloop_samples + // 2.0 1.3 2 + // 1.8 1.5 2 + // 1.6 1.7 2 + // 1.4 1.9 2 + // 1.2 2.1 3 + // Average preloop_samples = 2.2 + } - // start reading before the loop start point, to crossfade these samples - // with the samples we need to the loop end - int loop_read_position = SampleUtil::roundPlayPosToFrameStart( - m_currentPosition + (in_reverse ? preloop_samples : -preloop_samples), kNumChannels); + // start reading before the loop start point, to crossfade these samples + // with the samples we need to the loop end + int loop_read_position = SampleUtil::roundPlayPosToFrameStart( + m_currentPosition + (in_reverse ? preloop_samples : -preloop_samples), kNumChannels); - int looping_samples_read = m_pReader->read( - loop_read_position, samples_read, in_reverse, m_pCrossFadeBuffer); + int looping_samples_read = m_pReader->read( + loop_read_position, samples_read, in_reverse, m_pCrossFadeBuffer); - if (looping_samples_read != samples_read) { - qDebug() << "ERROR: Couldn't get all needed samples for crossfade."; - } + if (looping_samples_read != samples_read) { + qDebug() << "ERROR: Couldn't get all needed samples for crossfade."; + } - // do crossfade from the current buffer into the new loop beginning - if (samples_read != 0) { // avoid division by zero - SampleUtil::linearCrossfadeBuffers(pOutput, pOutput, m_pCrossFadeBuffer, samples_read); - } + // do crossfade from the current buffer into the new loop beginning + if (samples_read != 0) { // avoid division by zero + SampleUtil::linearCrossfadeBuffers(pOutput, pOutput, m_pCrossFadeBuffer, samples_read); } } diff --git a/src/test/looping_control_test.cpp b/src/test/looping_control_test.cpp index 526bf1a6eb6f..d013efc57ee0 100644 --- a/src/test/looping_control_test.cpp +++ b/src/test/looping_control_test.cpp @@ -390,7 +390,11 @@ TEST_F(LoopingControlTest, LoopScale_HalvesLoop) { EXPECT_EQ(500, m_pLoopEndPoint->get()); // Since the current sample was out of range of the new loop, // the current sample should reseek based on the new loop size. - EXPECT_EQ(300, m_pChannel1->getEngineBuffer()->m_pLoopingControl->getCurrentSample()); + double target; + double trigger = m_pChannel1->getEngineBuffer()->m_pLoopingControl->nextTrigger( + false, 1800, &target); + EXPECT_EQ(300, target); + EXPECT_EQ(1800, trigger); } TEST_F(LoopingControlTest, LoopDoubleButton_IgnoresPastTrackEnd) { @@ -513,7 +517,11 @@ TEST_F(LoopingControlTest, LoopMoveTest) { EXPECT_EQ(44100, m_pLoopStartPoint->get()); EXPECT_EQ(44400, m_pLoopEndPoint->get()); // Should seek to the corresponding offset within the moved loop - EXPECT_EQ(44110, m_pChannel1->getEngineBuffer()->m_pLoopingControl->getCurrentSample()); + double target; + double trigger = m_pChannel1->getEngineBuffer()->m_pLoopingControl->nextTrigger( + false, 10, &target); + EXPECT_EQ(44110, target); + EXPECT_EQ(10, trigger); // Move backward so that the current position is outside the new location of the loop m_pChannel1->getEngineBuffer()->queueNewPlaypos(44300, EngineBuffer::SEEK_STANDARD); @@ -523,9 +531,12 @@ TEST_F(LoopingControlTest, LoopMoveTest) { ProcessBuffer(); EXPECT_EQ(0, m_pLoopStartPoint->get()); EXPECT_EQ(300, m_pLoopEndPoint->get()); - EXPECT_EQ(200, m_pChannel1->getEngineBuffer()->m_pLoopingControl->getCurrentSample()); + trigger = m_pChannel1->getEngineBuffer()->m_pLoopingControl->nextTrigger( + false, 44300, &target); + EXPECT_EQ(200, target); + EXPECT_EQ(44300, trigger); - // Now repeat the test with looping disabled (should not affect the + // Now repeat the test with looping disabled (should not affect the // playhead). m_pButtonReloopToggle->slotSet(1); EXPECT_FALSE(isLoopEnabled()); @@ -537,7 +548,10 @@ TEST_F(LoopingControlTest, LoopMoveTest) { EXPECT_EQ(44100, m_pLoopStartPoint->get()); EXPECT_EQ(44400, m_pLoopEndPoint->get()); // Should not seek inside the moved loop when the loop is disabled - EXPECT_EQ(200, m_pChannel1->getEngineBuffer()->m_pLoopingControl->getCurrentSample()); + trigger = m_pChannel1->getEngineBuffer()->m_pLoopingControl->nextTrigger( + false, 200, &target); + EXPECT_EQ(-1, target); + EXPECT_EQ(-1, trigger); // Move backward so that the current position is outside the new location of the loop m_pChannel1->getEngineBuffer()->queueNewPlaypos(500, EngineBuffer::SEEK_STANDARD); @@ -577,7 +591,11 @@ TEST_F(LoopingControlTest, LoopResizeSeek) { // loop. EXPECT_EQ(0, m_pLoopStartPoint->get()); EXPECT_EQ(450, m_pLoopEndPoint->get()); - EXPECT_EQ(50, m_pChannel1->getEngineBuffer()->m_pLoopingControl->getCurrentSample()); + double target; + double trigger = m_pChannel1->getEngineBuffer()->m_pLoopingControl->nextTrigger( + false, 500, &target); + EXPECT_EQ(50, target); + EXPECT_EQ(500, trigger); // But if looping is not enabled, no warping occurs. m_pLoopStartPoint->slotSet(0); diff --git a/src/test/readaheadmanager_test.cpp b/src/test/readaheadmanager_test.cpp index 0089ae58f87f..be03d6d8a8f8 100644 --- a/src/test/readaheadmanager_test.cpp +++ b/src/test/readaheadmanager_test.cpp @@ -35,38 +35,22 @@ class StubLoopControl : public LoopingControl { m_triggerReturnValues.push_back(value); } - void pushProcessReturnValue(double value) { - m_processReturnValues.push_back(value); + void pushTargetReturnValue(double value) { + m_targetReturnValues.push_back(value); } double nextTrigger(bool reverse, - const double currentSample) override { + const double currentSample, + double* pTarget) override { Q_UNUSED(reverse); Q_UNUSED(currentSample); + Q_UNUSED(pTarget); + RELEASE_ASSERT(!m_targetReturnValues.isEmpty()); + *pTarget = m_targetReturnValues.takeFirst(); RELEASE_ASSERT(!m_triggerReturnValues.isEmpty()); return m_triggerReturnValues.takeFirst(); } - double process(const double dRate, - const double dCurrentSample, - const double dTotalSamples, - const int iBufferSize) override { - Q_UNUSED(dRate); - Q_UNUSED(dCurrentSample); - Q_UNUSED(dTotalSamples); - Q_UNUSED(iBufferSize); - RELEASE_ASSERT(!m_processReturnValues.isEmpty()); - return m_processReturnValues.takeFirst(); - } - - double getLoopTarget(bool reverse, - const double dCurrentSample) override { - Q_UNUSED(reverse); - Q_UNUSED(dCurrentSample); - RELEASE_ASSERT(!m_processReturnValues.isEmpty()); - return m_processReturnValues.takeFirst(); - } - // hintReader has no effect in this stubbed class void hintReader(HintVector* pHintList) override { Q_UNUSED(pHintList); @@ -84,7 +68,7 @@ class StubLoopControl : public LoopingControl { protected: QList m_triggerReturnValues; - QList m_processReturnValues; + QList m_targetReturnValues; }; class ReadAheadManagerTest : public MixxxTest { @@ -117,12 +101,12 @@ TEST_F(ReadAheadManagerTest, FractionalFrameLoop) { m_pLoopControl->pushTriggerReturnValue(20.2); m_pLoopControl->pushTriggerReturnValue(20.2); // Process value is the sample we should seek to. - m_pLoopControl->pushProcessReturnValue(3.3); - m_pLoopControl->pushProcessReturnValue(3.3); - m_pLoopControl->pushProcessReturnValue(3.3); - m_pLoopControl->pushProcessReturnValue(3.3); - m_pLoopControl->pushProcessReturnValue(3.3); - m_pLoopControl->pushProcessReturnValue(kNoTrigger); + m_pLoopControl->pushTargetReturnValue(3.3); + m_pLoopControl->pushTargetReturnValue(3.3); + m_pLoopControl->pushTargetReturnValue(3.3); + m_pLoopControl->pushTargetReturnValue(3.3); + m_pLoopControl->pushTargetReturnValue(3.3); + m_pLoopControl->pushTargetReturnValue(kNoTrigger); // read from start to loop trigger, overshoot 0.3 EXPECT_EQ(20, m_pReadAheadManager->getNextSamples(1.0, m_pBuffer, 100)); // read loop From f545378a08c070746fca36d140f5ae51549a118c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sat, 11 Nov 2017 16:14:31 +0100 Subject: [PATCH 20/35] removed unused return value from EngineControl::process() --- src/engine/bpmcontrol.cpp | 11 ----------- src/engine/bpmcontrol.h | 4 ---- src/engine/clockcontrol.cpp | 4 +--- src/engine/clockcontrol.h | 2 +- src/engine/enginecontrol.cpp | 3 +-- src/engine/enginecontrol.h | 6 ++---- src/engine/loopingcontrol.cpp | 4 +--- src/engine/loopingcontrol.h | 2 +- src/engine/ratecontrol.cpp | 6 ++---- src/engine/ratecontrol.h | 2 +- 10 files changed, 10 insertions(+), 34 deletions(-) diff --git a/src/engine/bpmcontrol.cpp b/src/engine/bpmcontrol.cpp index a8666c8b16fd..ae2167e81ad3 100644 --- a/src/engine/bpmcontrol.cpp +++ b/src/engine/bpmcontrol.cpp @@ -792,17 +792,6 @@ void BpmControl::setCurrentSample(const double dCurrentSample, const double dTot EngineControl::setCurrentSample(dCurrentSample, dTotalSamples); } -double BpmControl::process(const double dRate, - const double dCurrentSample, - const double dTotalSamples, - const int iBufferSize) { - Q_UNUSED(dRate); - Q_UNUSED(dCurrentSample); - Q_UNUSED(dTotalSamples); - Q_UNUSED(iBufferSize); - return kNoTrigger; -} - double BpmControl::updateLocalBpm() { double prev_local_bpm = m_pLocalBpm->get(); double local_bpm = 0; diff --git a/src/engine/bpmcontrol.h b/src/engine/bpmcontrol.h index ae7b493178c9..7c1ed553dbdf 100644 --- a/src/engine/bpmcontrol.h +++ b/src/engine/bpmcontrol.h @@ -39,10 +39,6 @@ class BpmControl : public EngineControl { double getPreviousSample() const { return m_dPreviousSample; } void setCurrentSample(const double dCurrentSample, const double dTotalSamples) override; - double process(const double dRate, - const double dCurrentSample, - const double dTotalSamples, - const int iBufferSize) override; void setTargetBeatDistance(double beatDistance); void setInstantaneousBpm(double instantaneousBpm); void resetSyncAdjustment(); diff --git a/src/engine/clockcontrol.cpp b/src/engine/clockcontrol.cpp index b6f03f5bfe6c..841b4662159a 100644 --- a/src/engine/clockcontrol.cpp +++ b/src/engine/clockcontrol.cpp @@ -46,7 +46,7 @@ void ClockControl::slotBeatsUpdated() { } } -double ClockControl::process(const double dRate, +void ClockControl::process(const double dRate, const double currentSample, const double totalSamples, const int iBuffersize) { @@ -66,6 +66,4 @@ double ClockControl::process(const double dRate, double distanceToClosestBeat = fabs(currentSample - closestBeat); m_pCOBeatActive->set(distanceToClosestBeat < blinkIntervalSamples / 2.0); } - - return kNoTrigger; } diff --git a/src/engine/clockcontrol.h b/src/engine/clockcontrol.h index 614ad9a24e59..3aa83a87d663 100644 --- a/src/engine/clockcontrol.h +++ b/src/engine/clockcontrol.h @@ -18,7 +18,7 @@ class ClockControl: public EngineControl { ~ClockControl() override; - double process(const double dRate, const double currentSample, + void process(const double dRate, const double currentSample, const double totalSamples, const int iBufferSize) override; public slots: diff --git a/src/engine/enginecontrol.cpp b/src/engine/enginecontrol.cpp index 9593c31d178b..9e8f1e9f28c1 100644 --- a/src/engine/enginecontrol.cpp +++ b/src/engine/enginecontrol.cpp @@ -19,7 +19,7 @@ EngineControl::EngineControl(QString group, EngineControl::~EngineControl() { } -double EngineControl::process(const double dRate, +void EngineControl::process(const double dRate, const double dCurrentSample, const double dTotalSamples, const int iBufferSize) { @@ -27,7 +27,6 @@ double EngineControl::process(const double dRate, Q_UNUSED(dCurrentSample); Q_UNUSED(dTotalSamples); Q_UNUSED(iBufferSize); - return kNoTrigger; } void EngineControl::trackLoaded(TrackPointer pNewTrack, TrackPointer pOldTrack) { diff --git a/src/engine/enginecontrol.h b/src/engine/enginecontrol.h index e2dee10dbb40..550f16094bc7 100644 --- a/src/engine/enginecontrol.h +++ b/src/engine/enginecontrol.h @@ -41,10 +41,8 @@ class EngineControl : public QObject { // Called by EngineBuffer::process every latency period. See the above // comments for information about guarantees that hold during this call. An // EngineControl can perform any upkeep operations that are necessary during - // this call. If the EngineControl would like to request the playback - // position to be altered, it should return the sample to seek to from this - // method. Otherwise it should return kNoTrigger. - virtual double process(const double dRate, + // this call. + virtual void process(const double dRate, const double dCurrentSample, const double dTotalSamples, const int iBufferSize); diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index b46e26b883c7..a3d40872fed3 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -304,7 +304,7 @@ void LoopingControl::slotLoopDouble(double pressed) { slotBeatLoop(m_pCOBeatLoopSize->get() * 2.0, true, false); } -double LoopingControl::process(const double dRate, +void LoopingControl::process(const double dRate, const double currentSample, const double totalSamples, const int iBufferSize) { @@ -323,8 +323,6 @@ double LoopingControl::process(const double dRate, } else if (m_bAdjustingLoopOut) { setLoopOutToCurrentPosition(); } - - return kNoTrigger; } double LoopingControl::nextTrigger(bool reverse, diff --git a/src/engine/loopingcontrol.h b/src/engine/loopingcontrol.h index 8d605fc7340e..2dbcbd4f79a3 100644 --- a/src/engine/loopingcontrol.h +++ b/src/engine/loopingcontrol.h @@ -33,7 +33,7 @@ class LoopingControl : public EngineControl { // process() updates the internal state of the LoopingControl to reflect the // correct current sample. If a loop should be taken LoopingControl returns // the sample that should be seeked to. Otherwise it returns currentSample. - double process(const double dRate, + void process(const double dRate, const double currentSample, const double totalSamples, const int iBufferSize) override; diff --git a/src/engine/ratecontrol.cpp b/src/engine/ratecontrol.cpp index 7fe644353a55..c5ddefb466cf 100644 --- a/src/engine/ratecontrol.cpp +++ b/src/engine/ratecontrol.cpp @@ -512,7 +512,7 @@ double RateControl::calculateSpeed(double baserate, double speed, bool paused, return rate; } -double RateControl::process(const double rate, +void RateControl::process(const double rate, const double currentSample, const double totalSamples, const int bufferSamples) @@ -547,7 +547,7 @@ double RateControl::process(const double rate, // Avoid Division by Zero if (range == 0) { qDebug() << "Avoiding a Division by Zero in RATERAMP_STEP code"; - return kNoTrigger; + return; } double change = m_pRateDir->get() * m_dTemp / @@ -611,8 +611,6 @@ double RateControl::process(const double rate, resetRateTemp(); } } - - return kNoTrigger; } double RateControl::getTempRate() { diff --git a/src/engine/ratecontrol.h b/src/engine/ratecontrol.h index 85d30ecbf652..99f6d0d4a03c 100644 --- a/src/engine/ratecontrol.h +++ b/src/engine/ratecontrol.h @@ -37,7 +37,7 @@ class RateControl : public EngineControl { void setBpmControl(BpmControl* bpmcontrol); // Must be called during each callback of the audio thread so that // RateControl has a chance to update itself. - double process(const double dRate, + void process(const double dRate, const double currentSample, const double totalSamples, const int bufferSamples) override; From e680975b36fc897af7170ba5113c873e5bdb58cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sun, 12 Nov 2017 15:02:59 +0100 Subject: [PATCH 21/35] restore lost loop enbled condition --- src/engine/loopingcontrol.cpp | 37 ++++++++++++++++++++++------------- src/engine/loopingcontrol.h | 1 - 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index a3d40872fed3..9d47ac964a58 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -333,24 +333,28 @@ double LoopingControl::nextTrigger(bool reverse, *pTarget = kNoTrigger; LoopSamples loopSamples = m_loopSamples.getValue(); - // Store loop samples for subsequent getLoopTarget() call - m_engineLoopSamples = loopSamples; - - if (loopSamples.seek) { - // here the loop has changed and the play position - // should be moved with it - *pTarget = seekInsideAdjustedLoop(currentSample, - m_oldLoopSamples.start, loopSamples.start, loopSamples.end); - if (*pTarget != kNoTrigger) { - // jump immediately - return currentSample; - } - } if (m_bLoopingEnabled && !m_bAdjustingLoopIn && !m_bAdjustingLoopOut && loopSamples.start != kNoTrigger && loopSamples.end != kNoTrigger) { + + if (loopSamples.start != m_oldLoopSamples.start || + loopSamples.end != m_oldLoopSamples.end) { + // bool seek is only valid after the loop has changed + if (loopSamples.seek) { + // here the loop has changed and the play position + // should be moved with it + *pTarget = seekInsideAdjustedLoop(currentSample, + m_oldLoopSamples.start, loopSamples.start, loopSamples.end); + if (*pTarget != kNoTrigger) { + // jump immediately + return currentSample; + } + } + m_oldLoopSamples = loopSamples; + } + if (reverse) { *pTarget = loopSamples.end; return loopSamples.start; @@ -1044,7 +1048,12 @@ void LoopingControl::slotBeatJump(double beats) { return; } - if (m_bLoopingEnabled && !m_bAdjustingLoopIn && !m_bAdjustingLoopOut) { + LoopSamples loopSamples = m_loopSamples.getValue(); + int currentSample = m_iCurrentSample; + + if (m_bLoopingEnabled && !m_bAdjustingLoopIn && !m_bAdjustingLoopOut && + loopSamples.start < currentSample && loopSamples.end > currentSample) { + // If inside an active loop, move loop slotLoopMove(beats); } else { seekAbs(m_pBeats->findNBeatsFromSample(getCurrentSample(), beats)); diff --git a/src/engine/loopingcontrol.h b/src/engine/loopingcontrol.h index 2dbcbd4f79a3..9434c9824928 100644 --- a/src/engine/loopingcontrol.h +++ b/src/engine/loopingcontrol.h @@ -133,7 +133,6 @@ class LoopingControl : public EngineControl { // TODO(DSC) Make the following values double ControlValueAtomic m_loopSamples; LoopSamples m_oldLoopSamples; - LoopSamples m_engineLoopSamples; QAtomicInt m_iCurrentSample; ControlObject* m_pQuantizeEnabled; ControlObject* m_pNextBeat; From c013ce6034a34c7514a4438e222b4e2a4ed86c26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sun, 12 Nov 2017 19:32:39 +0100 Subject: [PATCH 22/35] do loops seeks when paused --- src/engine/loopingcontrol.cpp | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index 9d47ac964a58..278223996e4a 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -316,7 +316,18 @@ void LoopingControl::process(const double dRate, if (!even(currentSampleEven)) { currentSampleEven--; } - m_iCurrentSample = currentSampleEven; + + if (m_iCurrentSample != currentSampleEven) { + m_iCurrentSample = currentSampleEven; + } else { + // no transport, so we have to do scheduled seeks here + double target; + const double loop_trigger = nextTrigger( + false, m_iCurrentSample, &target); + if (loop_trigger == m_iCurrentSample) { + seekAbs(target); + } + } if (m_bAdjustingLoopIn) { setLoopInToCurrentPosition(); @@ -347,12 +358,12 @@ double LoopingControl::nextTrigger(bool reverse, // should be moved with it *pTarget = seekInsideAdjustedLoop(currentSample, m_oldLoopSamples.start, loopSamples.start, loopSamples.end); - if (*pTarget != kNoTrigger) { - // jump immediately - return currentSample; - } } m_oldLoopSamples = loopSamples; + if (*pTarget != kNoTrigger) { + // jump immediately + return currentSample; + } } if (reverse) { @@ -1052,7 +1063,8 @@ void LoopingControl::slotBeatJump(double beats) { int currentSample = m_iCurrentSample; if (m_bLoopingEnabled && !m_bAdjustingLoopIn && !m_bAdjustingLoopOut && - loopSamples.start < currentSample && loopSamples.end > currentSample) { + loopSamples.start <= currentSample + 2 && + loopSamples.end >= currentSample - 2) { // If inside an active loop, move loop slotLoopMove(beats); } else { @@ -1148,7 +1160,6 @@ int LoopingControl::seekInsideAdjustedLoop( } } if (adjusted_position != currentSample) { - m_iCurrentSample = adjusted_position; return adjusted_position; } else { return kNoTrigger; From ca48a4ccc66deb1a0256bde94d779b0492b99786 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sun, 12 Nov 2017 21:09:39 +0100 Subject: [PATCH 23/35] added LoopingControlTest.LoopEscape --- src/test/looping_control_test.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/test/looping_control_test.cpp b/src/test/looping_control_test.cpp index d013efc57ee0..b3e4ecb88975 100644 --- a/src/test/looping_control_test.cpp +++ b/src/test/looping_control_test.cpp @@ -848,3 +848,23 @@ TEST_F(LoopingControlTest, Beatjump_MovesLoopBoundaries) { EXPECT_EQ(beatLength, m_pLoopStartPoint->get()); EXPECT_EQ(beatLength * 2, m_pLoopEndPoint->get()); } + +TEST_F(LoopingControlTest, LoopEscape) { + m_pLoopStartPoint->slotSet(100); + m_pLoopEndPoint->slotSet(200); + m_pButtonReloopToggle->set(1.0); + m_pButtonReloopToggle->set(0.0); + ProcessBuffer(); + EXPECT_TRUE(isLoopEnabled()); + // seek outside a loop schould disable it + seekToSampleAndProcess(300); + EXPECT_FALSE(isLoopEnabled()); + + m_pButtonReloopToggle->set(1.0); + m_pButtonReloopToggle->set(0.0); + ProcessBuffer(); + EXPECT_TRUE(isLoopEnabled()); + // seek outside a loop schould disable it + seekToSampleAndProcess(50); + EXPECT_FALSE(isLoopEnabled()); +} From 091f6ac82b8765ed85327bdcc16953ce0841e846 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sun, 12 Nov 2017 22:08:23 +0100 Subject: [PATCH 24/35] fix failing tests --- src/test/looping_control_test.cpp | 32 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/src/test/looping_control_test.cpp b/src/test/looping_control_test.cpp index b3e4ecb88975..f3c54a1bc676 100644 --- a/src/test/looping_control_test.cpp +++ b/src/test/looping_control_test.cpp @@ -516,12 +516,9 @@ TEST_F(LoopingControlTest, LoopMoveTest) { ProcessBuffer(); EXPECT_EQ(44100, m_pLoopStartPoint->get()); EXPECT_EQ(44400, m_pLoopEndPoint->get()); + ProcessBuffer(); // Should seek to the corresponding offset within the moved loop - double target; - double trigger = m_pChannel1->getEngineBuffer()->m_pLoopingControl->nextTrigger( - false, 10, &target); - EXPECT_EQ(44110, target); - EXPECT_EQ(10, trigger); + EXPECT_EQ(44110, m_pChannel1->getEngineBuffer()->m_pLoopingControl->getCurrentSample()); // Move backward so that the current position is outside the new location of the loop m_pChannel1->getEngineBuffer()->queueNewPlaypos(44300, EngineBuffer::SEEK_STANDARD); @@ -531,10 +528,8 @@ TEST_F(LoopingControlTest, LoopMoveTest) { ProcessBuffer(); EXPECT_EQ(0, m_pLoopStartPoint->get()); EXPECT_EQ(300, m_pLoopEndPoint->get()); - trigger = m_pChannel1->getEngineBuffer()->m_pLoopingControl->nextTrigger( - false, 44300, &target); - EXPECT_EQ(200, target); - EXPECT_EQ(44300, trigger); + ProcessBuffer(); + EXPECT_EQ(200, m_pChannel1->getEngineBuffer()->m_pLoopingControl->getCurrentSample()); // Now repeat the test with looping disabled (should not affect the // playhead). @@ -548,10 +543,7 @@ TEST_F(LoopingControlTest, LoopMoveTest) { EXPECT_EQ(44100, m_pLoopStartPoint->get()); EXPECT_EQ(44400, m_pLoopEndPoint->get()); // Should not seek inside the moved loop when the loop is disabled - trigger = m_pChannel1->getEngineBuffer()->m_pLoopingControl->nextTrigger( - false, 200, &target); - EXPECT_EQ(-1, target); - EXPECT_EQ(-1, trigger); + EXPECT_EQ(200, m_pChannel1->getEngineBuffer()->m_pLoopingControl->getCurrentSample()); // Move backward so that the current position is outside the new location of the loop m_pChannel1->getEngineBuffer()->queueNewPlaypos(500, EngineBuffer::SEEK_STANDARD); @@ -591,11 +583,8 @@ TEST_F(LoopingControlTest, LoopResizeSeek) { // loop. EXPECT_EQ(0, m_pLoopStartPoint->get()); EXPECT_EQ(450, m_pLoopEndPoint->get()); - double target; - double trigger = m_pChannel1->getEngineBuffer()->m_pLoopingControl->nextTrigger( - false, 500, &target); - EXPECT_EQ(50, target); - EXPECT_EQ(500, trigger); + ProcessBuffer(); + EXPECT_EQ(50, m_pChannel1->getEngineBuffer()->m_pLoopingControl->getCurrentSample()); // But if looping is not enabled, no warping occurs. m_pLoopStartPoint->slotSet(0); @@ -812,6 +801,13 @@ TEST_F(LoopingControlTest, Beatjump_MovesActiveLoop) { EXPECT_EQ(beatLength, m_pLoopStartPoint->get()); EXPECT_EQ(beatLength * 5, m_pLoopEndPoint->get()); + // jump backward with playposition outside the loop should not move the loop + m_pButtonBeatJumpBackward->set(1.0); + m_pButtonBeatJumpBackward->set(0.0); + EXPECT_EQ(beatLength, m_pLoopStartPoint->get()); + EXPECT_EQ(beatLength * 5, m_pLoopEndPoint->get()); + + seekToSampleAndProcess(beatLength); m_pButtonBeatJumpBackward->set(1.0); m_pButtonBeatJumpBackward->set(0.0); EXPECT_EQ(0, m_pLoopStartPoint->get()); From 3968faa5df3c86823803f040b54bd1bc471de2f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 14 Nov 2017 00:01:49 +0100 Subject: [PATCH 25/35] Add elipsis in currentLoopMatchesBeatloopSize() --- src/engine/loopingcontrol.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index 278223996e4a..d6d1bf866377 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -847,13 +847,11 @@ bool LoopingControl::currentLoopMatchesBeatloopSize() { LoopSamples loopSamples = m_loopSamples.getValue(); // Calculate where the loop out point would be if it is a beatloop - int beatLoopOutPoint = + double beatLoopOutPoint = m_pBeats->findNBeatsFromSample(loopSamples.start, m_pCOBeatLoopSize->get()); - if (!even(beatLoopOutPoint)) { - beatLoopOutPoint--; - } - return loopSamples.end == beatLoopOutPoint; + return loopSamples.end > beatLoopOutPoint - 2 && + loopSamples.end < beatLoopOutPoint + 2; } void LoopingControl::updateBeatLoopingControls() { From c5e6f454b8c354c8abcf30211524bd2d037158c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 14 Nov 2017 00:28:55 +0100 Subject: [PATCH 26/35] Enable double precision for loop positions --- src/engine/loopingcontrol.cpp | 92 +++++++++---------------------- src/engine/loopingcontrol.h | 8 +-- src/test/looping_control_test.cpp | 6 +- 3 files changed, 33 insertions(+), 73 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index d6d1bf866377..06181762c5e9 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -246,22 +246,18 @@ void LoopingControl::slotLoopScale(double scaleFactor) { if (loopSamples.start == kNoTrigger || loopSamples.end == kNoTrigger) { return; } - int loop_length = loopSamples.end - loopSamples.start; - int samples = m_pTrackSamples->get(); + double loop_length = loopSamples.end - loopSamples.start; + int trackSamples = m_pTrackSamples->get(); loop_length *= scaleFactor; // Abandon loops that are too short of extend beyond the end of the file. if (loop_length < MINIMUM_AUDIBLE_LOOP_SIZE || - loopSamples.start + loop_length > samples) { + loopSamples.start + loop_length > trackSamples) { return; } loopSamples.end = loopSamples.start + loop_length; - if (!even(loopSamples.end)) { - loopSamples.end--; - } - // TODO(XXX) we could be smarter about taking the active beatloop, scaling // it by the desired amount and trying to find another beatloop that matches // it, but for now we just clear the active beat loop if somebody scales. @@ -269,14 +265,14 @@ void LoopingControl::slotLoopScale(double scaleFactor) { // Don't allow 0 samples loop, so one can still manipulate it if (loopSamples.end == loopSamples.start) { - if ((loopSamples.end + 2) >= samples) + if ((loopSamples.end + 2) >= trackSamples) loopSamples.start -= 2; else loopSamples.end += 2; } // Do not allow loops to go past the end of the song - else if (loopSamples.end > samples) { - loopSamples.end = samples; + else if (loopSamples.end > trackSamples) { + loopSamples.end = trackSamples; } // Reseek if the loop shrank out from under the playposition. @@ -414,7 +410,7 @@ void LoopingControl::setLoopInToCurrentPosition() { // set loop-in position LoopSamples loopSamples = m_loopSamples.getValue(); double quantizedBeat = -1; - int pos = m_iCurrentSample; + double pos = m_iCurrentSample; if (m_pQuantizeEnabled->toBool() && m_pBeats != nullptr) { if (m_bAdjustingLoopIn) { double closestBeat = m_pClosestBeat->get(); @@ -427,14 +423,10 @@ void LoopingControl::setLoopInToCurrentPosition() { quantizedBeat = m_pClosestBeat->get(); } if (quantizedBeat != -1) { - pos = static_cast(floor(quantizedBeat)); + pos = quantizedBeat; } } - if (pos != -1 && !even(pos)) { - pos--; - } - // Reset the loop out position if it is before the loop in so that loops // cannot be inverted. if (loopSamples.end != kNoTrigger && @@ -449,7 +441,7 @@ void LoopingControl::setLoopInToCurrentPosition() { if (loopSamples.end != kNoTrigger && (loopSamples.end - pos) < MINIMUM_AUDIBLE_LOOP_SIZE) { if (quantizedBeat != -1 && m_pBeats) { - pos = static_cast(floor(m_pBeats->findNthBeat(quantizedBeat, -2))); + pos = m_pBeats->findNthBeat(quantizedBeat, -2); if (pos == -1 || (loopSamples.end - pos) < MINIMUM_AUDIBLE_LOOP_SIZE) { pos = loopSamples.end - MINIMUM_AUDIBLE_LOOP_SIZE; } @@ -524,14 +516,10 @@ void LoopingControl::setLoopOutToCurrentPosition() { quantizedBeat = m_pClosestBeat->get(); } if (quantizedBeat != -1) { - pos = static_cast(floor(quantizedBeat)); + pos = quantizedBeat; } } - if (pos != -1 && !even(pos)) { - pos++; // Increment to avoid shortening too-short loops - } - // If the user is trying to set a loop-out before the loop in or without // having a loop-in, then ignore it. if (loopSamples.start == kNoTrigger || pos < loopSamples.start) { @@ -665,15 +653,9 @@ void LoopingControl::slotReloopAndStop(double pressed) { void LoopingControl::slotLoopStartPos(double pos) { // This slot is called before trackLoaded() for a new Track - - int newpos = pos; - if (newpos != kNoTrigger && !even(newpos)) { - newpos--; - } - LoopSamples loopSamples = m_loopSamples.getValue(); - if (loopSamples.start == newpos) { + if (loopSamples.start == pos) { //nothing to do return; } @@ -685,8 +667,8 @@ void LoopingControl::slotLoopStartPos(double pos) { } loopSamples.seek = false; - loopSamples.start = newpos; - m_pCOLoopStartPosition->set(newpos); + loopSamples.start = pos; + m_pCOLoopStartPosition->set(pos); if (loopSamples.end != kNoTrigger && loopSamples.end <= loopSamples.start) { @@ -700,13 +682,8 @@ void LoopingControl::slotLoopStartPos(double pos) { void LoopingControl::slotLoopEndPos(double pos) { // This slot is called before trackLoaded() for a new Track - int newpos = pos; - if (newpos != -1 && !even(newpos)) { - newpos--; - } - LoopSamples loopSamples = m_loopSamples.getValue(); - if (loopSamples.end == newpos) { + if (loopSamples.end == pos) { //nothing to do return; } @@ -714,7 +691,7 @@ void LoopingControl::slotLoopEndPos(double pos) { // Reject if the loop-in is not set, or if the new position is before the // start point (but not -1). if (loopSamples.start == kNoTrigger || - (newpos != kNoTrigger && newpos <= loopSamples.start)) { + (pos != kNoTrigger && pos <= loopSamples.start)) { m_pCOLoopEndPosition->set(loopSamples.end); return; } @@ -724,9 +701,9 @@ void LoopingControl::slotLoopEndPos(double pos) { if (pos == -1.0) { setLoopingEnabled(false); } - loopSamples.end = newpos; + loopSamples.end = pos; loopSamples.seek = false; - m_pCOLoopEndPosition->set(newpos); + m_pCOLoopEndPosition->set(pos); m_loopSamples.setValue(loopSamples); } @@ -940,18 +917,11 @@ void LoopingControl::slotBeatLoop(double beats, bool keepStartPoint, bool enable } } else { - newloopSamples.start = floor(cur_pos); + newloopSamples.start = cur_pos; } } - if (!even(newloopSamples.start)) { - newloopSamples.start--; - } - newloopSamples.end = m_pBeats->findNBeatsFromSample(newloopSamples.start, beats); - if (!even(newloopSamples.end)) { - newloopSamples.end--; - } if (newloopSamples.start == newloopSamples.end) { if ((newloopSamples.end + 2) > samples) { @@ -1093,16 +1063,10 @@ void LoopingControl::slotLoopMove(double beats) { if (BpmControl::getBeatContext(m_pBeats, getCurrentSample(), nullptr, nullptr, nullptr, nullptr)) { - int old_loop_in = loopSamples.start; - int old_loop_out = loopSamples.end; - int new_loop_in = m_pBeats->findNBeatsFromSample(old_loop_in, beats); - int new_loop_out = m_pBeats->findNBeatsFromSample(old_loop_out, beats); - if (!even(new_loop_in)) { - --new_loop_in; - } - if (!even(new_loop_out)) { - --new_loop_out; - } + double old_loop_in = loopSamples.start; + double old_loop_out = loopSamples.end; + double new_loop_in = m_pBeats->findNBeatsFromSample(old_loop_in, beats); + double new_loop_out = m_pBeats->findNBeatsFromSample(old_loop_out, beats); // If we are looping make sure that the play head does not leave the // loop as a result of our adjustment. @@ -1118,8 +1082,8 @@ void LoopingControl::slotLoopMove(double beats) { // Must be called from the engine thread only int LoopingControl::seekInsideAdjustedLoop( - int currentSample, int old_loop_in, - int new_loop_in, int new_loop_out) { + double currentSample, double old_loop_in, + double new_loop_in, double new_loop_out) { // Copy on stack since m_iCurrentSample sample can change under us. if (currentSample >= new_loop_in && currentSample <= new_loop_out) { // playposition already is inside the loop @@ -1131,12 +1095,8 @@ int LoopingControl::seekInsideAdjustedLoop( return kNoTrigger; } - int new_loop_size = new_loop_out - new_loop_in; - if (!even(new_loop_size)) { - --new_loop_size; - } - - int adjusted_position = currentSample; + double new_loop_size = new_loop_out - new_loop_in; + double adjusted_position = currentSample; while (adjusted_position > new_loop_out) { adjusted_position -= new_loop_size; VERIFY_OR_DEBUG_ASSERT(adjusted_position > new_loop_in) { diff --git a/src/engine/loopingcontrol.h b/src/engine/loopingcontrol.h index 9434c9824928..ea89363e3d6b 100644 --- a/src/engine/loopingcontrol.h +++ b/src/engine/loopingcontrol.h @@ -89,8 +89,8 @@ class LoopingControl : public EngineControl { private: struct LoopSamples { - int start; - int end; + double start; + double end; bool seek; }; @@ -104,8 +104,8 @@ class LoopingControl : public EngineControl { // When a loop changes size such that the playposition is outside of the loop, // we can figure out the best place in the new loop to seek to maintain // the beat. It will even keep multi-bar phrasing correct with 4/4 tracks. - int seekInsideAdjustedLoop(int currentSample, - int old_loop_in, int new_loop_in, int new_loop_out); + int seekInsideAdjustedLoop(double currentSample, + double old_loop_in, double new_loop_in, double new_loop_out); ControlPushButton* m_pCOBeatLoopActivate; ControlPushButton* m_pCOBeatLoopRollActivate; diff --git a/src/test/looping_control_test.cpp b/src/test/looping_control_test.cpp index f3c54a1bc676..76ad0280a648 100644 --- a/src/test/looping_control_test.cpp +++ b/src/test/looping_control_test.cpp @@ -110,10 +110,10 @@ TEST_F(LoopingControlTest, LoopSet) { TEST_F(LoopingControlTest, LoopSetOddSamples) { m_pLoopStartPoint->slotSet(1); - m_pLoopEndPoint->slotSet(101); + m_pLoopEndPoint->slotSet(101.5); seekToSampleAndProcess(50); - EXPECT_EQ(0, m_pLoopStartPoint->get()); - EXPECT_EQ(100, m_pLoopEndPoint->get()); + EXPECT_EQ(1, m_pLoopStartPoint->get()); + EXPECT_EQ(101.5, m_pLoopEndPoint->get()); } TEST_F(LoopingControlTest, LoopInSetInsideLoopContinues) { From 05e3d496b5a3ab437831caa03456fcd332b875fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Wed, 15 Nov 2017 00:49:04 +0100 Subject: [PATCH 27/35] Fix Beats::findNBeatsFromSample() precision --- build/depends.py | 1 + src/engine/loopingcontrol.cpp | 12 ++++---- src/track/beats.cpp | 55 +++++++++++++++++++++++++++++++++++ src/track/beats.h | 39 ++----------------------- 4 files changed, 64 insertions(+), 43 deletions(-) create mode 100644 src/track/beats.cpp diff --git a/build/depends.py b/build/depends.py index 178e10f8c9b4..73982b505bf7 100644 --- a/build/depends.py +++ b/build/depends.py @@ -1062,6 +1062,7 @@ def sources(self, build): "track/beatgrid.cpp", "track/beatmap.cpp", "track/beatutils.cpp", + "track/beats.cpp", "track/bpm.cpp", "track/keyfactory.cpp", "track/keys.cpp", diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index 06181762c5e9..2f897b7f10b1 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -335,8 +335,6 @@ void LoopingControl::process(const double dRate, double LoopingControl::nextTrigger(bool reverse, const double currentSample, double *pTarget) { - Q_UNUSED(currentSample); - *pTarget = kNoTrigger; LoopSamples loopSamples = m_loopSamples.getValue(); @@ -938,8 +936,8 @@ void LoopingControl::slotBeatLoop(double beats, bool keepStartPoint, bool enable // the end of the track, let beatloop_size be set to // a smaller size, but not get larger. double previousBeatloopSize = m_pCOBeatLoopSize->get(); - double previousBeatloopOutPoint = - m_pBeats->findNBeatsFromSample(newloopSamples.start, previousBeatloopSize); + double previousBeatloopOutPoint = m_pBeats->findNBeatsFromSample( + newloopSamples.start, previousBeatloopSize); if (previousBeatloopOutPoint < newloopSamples.start && beats < previousBeatloopSize) { m_pCOBeatLoopSize->setAndConfirm(beats); @@ -951,9 +949,9 @@ void LoopingControl::slotBeatLoop(double beats, bool keepStartPoint, bool enable // do not resize the existing loop until beatloop_size matches // the size of the existing loop. // Do not return immediately so beatloop_size can be updated. - bool avoidResize = false; + bool omitResize = false; if (!currentLoopMatchesBeatloopSize() && !enable) { - avoidResize = true; + omitResize = true; } if (m_pCOBeatLoopSize->get() != beats) { @@ -966,7 +964,7 @@ void LoopingControl::slotBeatLoop(double beats, bool keepStartPoint, bool enable return; } - if (avoidResize) { + if (omitResize) { return; } diff --git a/src/track/beats.cpp b/src/track/beats.cpp new file mode 100644 index 000000000000..80510c4e5da2 --- /dev/null +++ b/src/track/beats.cpp @@ -0,0 +1,55 @@ + +#include "track/beats.h" + + + +int Beats::numBeatsInRange(double dStartSample, double dEndSample) { + double dLastCountedBeat = 0.0; + int iBeatsCounter; + for (iBeatsCounter = 1; dLastCountedBeat < dEndSample; iBeatsCounter++) { + dLastCountedBeat = findNthBeat(dStartSample, iBeatsCounter); + if (dLastCountedBeat == -1) { + break; + } + } + return iBeatsCounter - 2; +}; + +double Beats::findNBeatsFromSample(double fromSample, double beats) const { + double nthBeat; + double prevBeat; + double nextBeat; + + if (!findPrevNextBeats(fromSample, &prevBeat, &nextBeat)) { + return fromSample; + } + double fromFractionBeats = (fromSample - prevBeat) / (nextBeat - prevBeat); + double beatsFromPrevBeat = fromFractionBeats + beats; + + int fullBeats = static_cast(beatsFromPrevBeat); + double fractionBeats = beatsFromPrevBeat - fullBeats; + + // Add the length between this beat and the fullbeats'th beat + // to the end position + if (fullBeats > 0) { + nthBeat = findNthBeat(nextBeat, fullBeats); + } else { + nthBeat = findNthBeat(prevBeat, fullBeats - 1); + } + + if (nthBeat == -1) { + return fromSample; + } + + // Add the fraction of the beat + if (fractionBeats != 0) { + nextBeat = findNthBeat(nthBeat, 2); + if (nextBeat == -1) { + return fromSample; + } + nthBeat += (nextBeat - nthBeat) * fractionBeats; + } + + return nthBeat; +}; + diff --git a/src/track/beats.h b/src/track/beats.h index 112dd9861946..ecca3b5036db 100644 --- a/src/track/beats.h +++ b/src/track/beats.h @@ -104,45 +104,12 @@ class Beats { // then dSamples is returned. If no beat can be found, returns -1. virtual double findNthBeat(double dSamples, int n) const = 0; - inline int numBeatsInRange(double dStartSample, double dEndSample) { - double dLastCountedBeat = 0.0; - int iBeatsCounter; - for (iBeatsCounter = 1; dLastCountedBeat < dEndSample; iBeatsCounter++) { - dLastCountedBeat = findNthBeat(dStartSample, iBeatsCounter); - if (dLastCountedBeat == -1) { - break; - } - } - return iBeatsCounter - 2; - }; + int numBeatsInRange(double dStartSample, double dEndSample); // Find the sample N beats away from dSample. The number of beats may be // negative and does not need to be an integer. - inline double findNBeatsFromSample(double dSample, double beats) const { - double endSample = dSample; - double thisBeat = 0.0, nthBeat = 0.0; - int fullBeats = static_cast(beats); - // Add the length between this beat and the fullbeats'th beat - // to the end position - if (beats > 0) { - thisBeat = findNthBeat(dSample, 1); - nthBeat = findNthBeat(dSample, fullBeats + 1); - } else if (beats < 0) { - thisBeat = findNthBeat(dSample, -1); - nthBeat = findNthBeat(dSample, fullBeats - 1); - } - endSample += nthBeat - thisBeat; - - // Add the fraction of the beat - double fractionBeats = beats - static_cast(fullBeats); - if (fractionBeats != 0) { - double endNextBeat, endPrevBeat; - findPrevNextBeats(endSample, &endPrevBeat, &endNextBeat); - endSample += (endNextBeat - endPrevBeat) * fractionBeats; - } - - return endSample; - }; + double findNBeatsFromSample(double fromSample, double beats) const; + // Adds to pBeatsList the position in samples of every beat occuring between // startPosition and endPosition. BeatIterator must be iterated while From 1a74fd6a9c4f918ea6224ec7e3f77ee8d80da33c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Wed, 15 Nov 2017 22:26:14 +0100 Subject: [PATCH 28/35] Avoid escaping loop after manually adjust --- src/engine/loopingcontrol.cpp | 2 +- src/engine/readaheadmanager.cpp | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index 2f897b7f10b1..0f716714d850 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -501,7 +501,7 @@ void LoopingControl::slotLoopInGoto(double pressed) { void LoopingControl::setLoopOutToCurrentPosition() { LoopSamples loopSamples = m_loopSamples.getValue(); double quantizedBeat = -1; - int pos = m_iCurrentSample; + int pos = m_iCurrentSample + 2; if (m_pQuantizeEnabled->toBool() && m_pBeats != nullptr) { if (m_bAdjustingLoopOut) { double closestBeat = m_pClosestBeat->get(); diff --git a/src/engine/readaheadmanager.cpp b/src/engine/readaheadmanager.cpp index e10ae22e8b74..edad047e2696 100644 --- a/src/engine/readaheadmanager.cpp +++ b/src/engine/readaheadmanager.cpp @@ -62,7 +62,7 @@ SINT ReadAheadManager::getNextSamples(double dRate, CSAMPLE* pOutput, samplesToLoopTrigger = in_reverse ? m_currentPosition - loop_trigger : loop_trigger - m_currentPosition; - if (samplesToLoopTrigger >= 0.0) { + if (samplesToLoopTrigger > 0.0) { // We can only read whole frames from the reader. // Use ceil here, to be sure to reach the loop trigger. preloop_samples = SampleUtil::ceilPlayPosToFrameStart( @@ -72,6 +72,9 @@ SINT ReadAheadManager::getNextSamples(double dRate, CSAMPLE* pOutput, reachedTrigger = true; samples_from_reader = preloop_samples; } + } else { + reachedTrigger = true; + samples_from_reader = 0; } } From 7cf32d0316e51da8df34295e4c2b991e31b8f02b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Thu, 16 Nov 2017 20:58:50 +0100 Subject: [PATCH 29/35] Avoid escaping loop after manually adjust, a second way --- src/engine/loopingcontrol.cpp | 20 ++++++++++++++++++++ src/engine/loopingcontrol.h | 2 ++ src/engine/readaheadmanager.cpp | 5 +---- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index 0f716714d850..3ce3ad03d913 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -44,6 +44,8 @@ LoopingControl::LoopingControl(QString group, m_bLoopRollActive(false), m_bAdjustingLoopIn(false), m_bAdjustingLoopOut(false), + m_bAdjustingLoopInOld(false), + m_bAdjustingLoopOutOld(false), m_bLoopOutPressedWhileLoopDisabled(false) { m_oldLoopSamples = { kNoTrigger, kNoTrigger, false }; m_loopSamples.setValue(m_oldLoopSamples); @@ -339,6 +341,24 @@ double LoopingControl::nextTrigger(bool reverse, LoopSamples loopSamples = m_loopSamples.getValue(); + if (m_bAdjustingLoopInOld != m_bAdjustingLoopIn) { + m_bAdjustingLoopInOld = m_bAdjustingLoopIn; + if (reverse && !m_bAdjustingLoopIn) { + m_oldLoopSamples = loopSamples; + *pTarget = loopSamples.end; + return currentSample; + } + } + + if (m_bAdjustingLoopOutOld != m_bAdjustingLoopOut) { + m_bAdjustingLoopOutOld = m_bAdjustingLoopOut; + if (!reverse && !m_bAdjustingLoopOut) { + m_oldLoopSamples = loopSamples; + *pTarget = loopSamples.start; + return currentSample; + } + } + if (m_bLoopingEnabled && !m_bAdjustingLoopIn && !m_bAdjustingLoopOut && loopSamples.start != kNoTrigger && diff --git a/src/engine/loopingcontrol.h b/src/engine/loopingcontrol.h index ea89363e3d6b..fe053f444dcd 100644 --- a/src/engine/loopingcontrol.h +++ b/src/engine/loopingcontrol.h @@ -129,6 +129,8 @@ class LoopingControl : public EngineControl { bool m_bLoopRollActive; bool m_bAdjustingLoopIn; bool m_bAdjustingLoopOut; + bool m_bAdjustingLoopInOld; + bool m_bAdjustingLoopOutOld; bool m_bLoopOutPressedWhileLoopDisabled; // TODO(DSC) Make the following values double ControlValueAtomic m_loopSamples; diff --git a/src/engine/readaheadmanager.cpp b/src/engine/readaheadmanager.cpp index edad047e2696..e10ae22e8b74 100644 --- a/src/engine/readaheadmanager.cpp +++ b/src/engine/readaheadmanager.cpp @@ -62,7 +62,7 @@ SINT ReadAheadManager::getNextSamples(double dRate, CSAMPLE* pOutput, samplesToLoopTrigger = in_reverse ? m_currentPosition - loop_trigger : loop_trigger - m_currentPosition; - if (samplesToLoopTrigger > 0.0) { + if (samplesToLoopTrigger >= 0.0) { // We can only read whole frames from the reader. // Use ceil here, to be sure to reach the loop trigger. preloop_samples = SampleUtil::ceilPlayPosToFrameStart( @@ -72,9 +72,6 @@ SINT ReadAheadManager::getNextSamples(double dRate, CSAMPLE* pOutput, reachedTrigger = true; samples_from_reader = preloop_samples; } - } else { - reachedTrigger = true; - samples_from_reader = 0; } } From 39d67f64d505ab5c0c52a4d51af0f695a9996efe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Thu, 16 Nov 2017 21:39:28 +0100 Subject: [PATCH 30/35] Optimize beat jumpfor non cons beat grids --- src/engine/loopingcontrol.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index 3ce3ad03d913..f29777b64e4e 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -1081,10 +1081,10 @@ void LoopingControl::slotLoopMove(double beats) { if (BpmControl::getBeatContext(m_pBeats, getCurrentSample(), nullptr, nullptr, nullptr, nullptr)) { - double old_loop_in = loopSamples.start; - double old_loop_out = loopSamples.end; - double new_loop_in = m_pBeats->findNBeatsFromSample(old_loop_in, beats); - double new_loop_out = m_pBeats->findNBeatsFromSample(old_loop_out, beats); + double new_loop_in = m_pBeats->findNBeatsFromSample(loopSamples.start, beats); + double new_loop_out = currentLoopMatchesBeatloopSize() ? + m_pBeats->findNBeatsFromSample(new_loop_in, m_pCOBeatLoopSize->get()) : + m_pBeats->findNBeatsFromSample(loopSamples.start, beats); // If we are looping make sure that the play head does not leave the // loop as a result of our adjustment. From f822cb8a2bde1dcb4895319e652df34be11be335 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Wed, 6 Dec 2017 21:14:25 +0100 Subject: [PATCH 31/35] Enable loop wen moving loop in marker in the same way we do for the loop out marker --- src/engine/loopingcontrol.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index f29777b64e4e..338ea3bd035e 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -473,6 +473,12 @@ void LoopingControl::setLoopInToCurrentPosition() { m_pCOLoopStartPosition->set(loopSamples.start); + // start looping + if (loopSamples.start != kNoTrigger && + loopSamples.end != kNoTrigger) { + setLoopingEnabled(true); + } + if (m_pQuantizeEnabled->toBool() && loopSamples.start < loopSamples.end && m_pBeats != nullptr) { From 8299de41d5282671766fda49de61c7e43d00ebc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Thu, 7 Dec 2017 23:06:17 +0100 Subject: [PATCH 32/35] Fix typo that causes an infinite loop --- src/engine/loopingcontrol.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index 338ea3bd035e..98cd06968e52 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -1090,7 +1090,7 @@ void LoopingControl::slotLoopMove(double beats) { double new_loop_in = m_pBeats->findNBeatsFromSample(loopSamples.start, beats); double new_loop_out = currentLoopMatchesBeatloopSize() ? m_pBeats->findNBeatsFromSample(new_loop_in, m_pCOBeatLoopSize->get()) : - m_pBeats->findNBeatsFromSample(loopSamples.start, beats); + m_pBeats->findNBeatsFromSample(loopSamples.end, beats); // If we are looping make sure that the play head does not leave the // loop as a result of our adjustment. @@ -1120,6 +1120,7 @@ int LoopingControl::seekInsideAdjustedLoop( } double new_loop_size = new_loop_out - new_loop_in; + DEBUG_ASSERT(new_loop_size > 0); double adjusted_position = currentSample; while (adjusted_position > new_loop_out) { adjusted_position -= new_loop_size; From ab694d03e0f8b0c19c6220aec52c7f44e8fef0df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Fri, 8 Dec 2017 14:30:32 +0100 Subject: [PATCH 33/35] Get rid of rounding issues by fully moving to double precision looping. --- src/engine/loopingcontrol.cpp | 75 ++++++++++++++++++++++------------- src/engine/loopingcontrol.h | 4 +- 2 files changed, 49 insertions(+), 30 deletions(-) diff --git a/src/engine/loopingcontrol.cpp b/src/engine/loopingcontrol.cpp index 98cd06968e52..af981f6fe3e1 100644 --- a/src/engine/loopingcontrol.cpp +++ b/src/engine/loopingcontrol.cpp @@ -49,7 +49,7 @@ LoopingControl::LoopingControl(QString group, m_bLoopOutPressedWhileLoopDisabled(false) { m_oldLoopSamples = { kNoTrigger, kNoTrigger, false }; m_loopSamples.setValue(m_oldLoopSamples); - m_iCurrentSample = 0; + m_currentSample.setValue(0.0); m_pActiveBeatLoop = NULL; //Create loop-in, loop-out, loop-exit, and reloop/exit ControlObjects @@ -310,20 +310,33 @@ void LoopingControl::process(const double dRate, Q_UNUSED(iBufferSize); Q_UNUSED(dRate); - int currentSampleEven = static_cast(currentSample); - if (!even(currentSampleEven)) { - currentSampleEven--; - } + double oldCurrentSample = m_currentSample.getValue(); - if (m_iCurrentSample != currentSampleEven) { - m_iCurrentSample = currentSampleEven; + if (oldCurrentSample != currentSample) { + m_currentSample.setValue(currentSample); } else { // no transport, so we have to do scheduled seeks here - double target; - const double loop_trigger = nextTrigger( - false, m_iCurrentSample, &target); - if (loop_trigger == m_iCurrentSample) { - seekAbs(target); + LoopSamples loopSamples = m_loopSamples.getValue(); + if (m_bLoopingEnabled && + !m_bAdjustingLoopIn && !m_bAdjustingLoopOut && + loopSamples.start != kNoTrigger && + loopSamples.end != kNoTrigger) { + + if (loopSamples.start != m_oldLoopSamples.start || + loopSamples.end != m_oldLoopSamples.end) { + // bool seek is only valid after the loop has changed + if (loopSamples.seek) { + // here the loop has changed and the play position + // should be moved with it + double target = seekInsideAdjustedLoop(currentSample, + m_oldLoopSamples.start, loopSamples.start, loopSamples.end); + if (target != kNoTrigger) { + // jump immediately + seekAbs(target); + } + } + m_oldLoopSamples = loopSamples; + } } } @@ -376,6 +389,8 @@ double LoopingControl::nextTrigger(bool reverse, m_oldLoopSamples = loopSamples; if (*pTarget != kNoTrigger) { // jump immediately + qDebug() << currentSample << + m_oldLoopSamples.start << loopSamples.start << loopSamples.end; return currentSample; } } @@ -428,7 +443,7 @@ void LoopingControl::setLoopInToCurrentPosition() { // set loop-in position LoopSamples loopSamples = m_loopSamples.getValue(); double quantizedBeat = -1; - double pos = m_iCurrentSample; + double pos = m_currentSample.getValue(); if (m_pQuantizeEnabled->toBool() && m_pBeats != nullptr) { if (m_bAdjustingLoopIn) { double closestBeat = m_pClosestBeat->get(); @@ -469,7 +484,6 @@ void LoopingControl::setLoopInToCurrentPosition() { } loopSamples.start = pos; - loopSamples.seek = false; m_pCOLoopStartPosition->set(loopSamples.start); @@ -477,6 +491,9 @@ void LoopingControl::setLoopInToCurrentPosition() { if (loopSamples.start != kNoTrigger && loopSamples.end != kNoTrigger) { setLoopingEnabled(true); + loopSamples.seek = true; + } else { + loopSamples.seek = false; } if (m_pQuantizeEnabled->toBool() @@ -527,7 +544,7 @@ void LoopingControl::slotLoopInGoto(double pressed) { void LoopingControl::setLoopOutToCurrentPosition() { LoopSamples loopSamples = m_loopSamples.getValue(); double quantizedBeat = -1; - int pos = m_iCurrentSample + 2; + int pos = m_currentSample.getValue(); if (m_pQuantizeEnabled->toBool() && m_pBeats != nullptr) { if (m_bAdjustingLoopOut) { double closestBeat = m_pClosestBeat->get(); @@ -566,15 +583,16 @@ void LoopingControl::setLoopOutToCurrentPosition() { // set loop out position loopSamples.end = pos; - loopSamples.seek = false; m_pCOLoopEndPosition->set(loopSamples.end); - m_loopSamples.setValue(loopSamples); // start looping if (loopSamples.start != kNoTrigger && loopSamples.end != kNoTrigger) { setLoopingEnabled(true); + loopSamples.seek = true; + } else { + loopSamples.seek = false; } if (m_pQuantizeEnabled->toBool() && m_pBeats != nullptr) { @@ -585,6 +603,8 @@ void LoopingControl::setLoopOutToCurrentPosition() { clearActiveBeatLoop(); } //qDebug() << "set loop_out to " << loopSamples.end; + + m_loopSamples.setValue(loopSamples); } void LoopingControl::slotLoopOut(double pressed) { @@ -734,19 +754,18 @@ void LoopingControl::slotLoopEndPos(double pos) { // This is called from the engine thread void LoopingControl::notifySeek(double dNewPlaypos) { LoopSamples loopSamples = m_loopSamples.getValue(); + double currentSample = m_currentSample.getValue(); if (m_bLoopingEnabled) { // Disable loop when we jumping out, or over a catching loop, // using hot cues or waveform overview. - // + 2 because the new playposition can be rounded to loop end because of - // playing not at rate 1. - if (m_iCurrentSample >= loopSamples.start - 2 && - m_iCurrentSample <= loopSamples.end + 2 && - dNewPlaypos < loopSamples.start - 2) { + if (currentSample >= loopSamples.start && + currentSample <= loopSamples.end && + dNewPlaypos < loopSamples.start) { // jumping out of loop in backwards setLoopingEnabled(false); } - if (m_iCurrentSample <= loopSamples.end + 2 && - dNewPlaypos > loopSamples.end + 2) { + if (currentSample <= loopSamples.end && + dNewPlaypos > loopSamples.end) { // jumping out a loop or over a catching loop forward setLoopingEnabled(false); } @@ -1052,11 +1071,11 @@ void LoopingControl::slotBeatJump(double beats) { } LoopSamples loopSamples = m_loopSamples.getValue(); - int currentSample = m_iCurrentSample; + double currentSample = m_currentSample.getValue(); if (m_bLoopingEnabled && !m_bAdjustingLoopIn && !m_bAdjustingLoopOut && - loopSamples.start <= currentSample + 2 && - loopSamples.end >= currentSample - 2) { + loopSamples.start <= currentSample && + loopSamples.end >= currentSample) { // If inside an active loop, move loop slotLoopMove(beats); } else { @@ -1113,7 +1132,7 @@ int LoopingControl::seekInsideAdjustedLoop( // playposition already is inside the loop return kNoTrigger; } - if (currentSample < old_loop_in - 2 && currentSample <= new_loop_out) { + if (currentSample < old_loop_in && currentSample <= new_loop_out) { // Playposition was before a catching loop and is still a catching loop // nothing to do return kNoTrigger; diff --git a/src/engine/loopingcontrol.h b/src/engine/loopingcontrol.h index fe053f444dcd..42141abde17f 100644 --- a/src/engine/loopingcontrol.h +++ b/src/engine/loopingcontrol.h @@ -91,7 +91,7 @@ class LoopingControl : public EngineControl { struct LoopSamples { double start; double end; - bool seek; + bool seek; // force the playposition to be inside the loop after adjusting it. }; void setLoopingEnabled(bool enabled); @@ -135,7 +135,7 @@ class LoopingControl : public EngineControl { // TODO(DSC) Make the following values double ControlValueAtomic m_loopSamples; LoopSamples m_oldLoopSamples; - QAtomicInt m_iCurrentSample; + ControlValueAtomic m_currentSample; ControlObject* m_pQuantizeEnabled; ControlObject* m_pNextBeat; ControlObject* m_pPreviousBeat; From d85d3b31c51db7788ee502802fb6418083e5d9a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sat, 9 Dec 2017 21:32:08 +0100 Subject: [PATCH 34/35] Try to fix test on gcc 7 using EXPECT_NEAR --- src/test/looping_control_test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/looping_control_test.cpp b/src/test/looping_control_test.cpp index 76ad0280a648..5bc6e73c339f 100644 --- a/src/test/looping_control_test.cpp +++ b/src/test/looping_control_test.cpp @@ -527,7 +527,7 @@ TEST_F(LoopingControlTest, LoopMoveTest) { m_pButtonBeatMoveBackward->set(0.0); ProcessBuffer(); EXPECT_EQ(0, m_pLoopStartPoint->get()); - EXPECT_EQ(300, m_pLoopEndPoint->get()); + EXPECT_NEAR(300, m_pLoopEndPoint->get(), 0.000000001); ProcessBuffer(); EXPECT_EQ(200, m_pChannel1->getEngineBuffer()->m_pLoopingControl->getCurrentSample()); @@ -552,7 +552,7 @@ TEST_F(LoopingControlTest, LoopMoveTest) { m_pButtonBeatMoveBackward->set(0.0); ProcessBuffer(); EXPECT_EQ(0, m_pLoopStartPoint->get()); - EXPECT_EQ(300, m_pLoopEndPoint->get()); + EXPECT_NEAR(300, m_pLoopEndPoint->get(), 0.000000001); EXPECT_EQ(500, m_pChannel1->getEngineBuffer()->m_pLoopingControl->getCurrentSample()); } From e877dee154c335429c6b9cf8c87df18094ef8c85 Mon Sep 17 00:00:00 2001 From: Uwe Klotz Date: Sun, 10 Dec 2017 09:37:06 +0100 Subject: [PATCH 35/35] Keep the test code maintainable (#24) Even if it is only a test the rules for maintainable code still apply. --- src/test/looping_control_test.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/test/looping_control_test.cpp b/src/test/looping_control_test.cpp index 5bc6e73c339f..5515b5733490 100644 --- a/src/test/looping_control_test.cpp +++ b/src/test/looping_control_test.cpp @@ -11,6 +11,10 @@ #include "test/mockedenginebackendtest.h" #include "util/memory.h" +// Due to rounding errors loop positions should be compared with EXPECT_NEAR instead of EXPECT_EQ. +// NOTE(uklotzde, 2017-12-10): The rounding errors currently only appeared with GCC 7.2.1. +constexpr double kLoopPositionMaxAbsError = 0.000000001; + class LoopingControlTest : public MockedEngineBackendTest { public: LoopingControlTest() @@ -527,7 +531,7 @@ TEST_F(LoopingControlTest, LoopMoveTest) { m_pButtonBeatMoveBackward->set(0.0); ProcessBuffer(); EXPECT_EQ(0, m_pLoopStartPoint->get()); - EXPECT_NEAR(300, m_pLoopEndPoint->get(), 0.000000001); + EXPECT_NEAR(300, m_pLoopEndPoint->get(), kLoopPositionMaxAbsError); ProcessBuffer(); EXPECT_EQ(200, m_pChannel1->getEngineBuffer()->m_pLoopingControl->getCurrentSample()); @@ -552,7 +556,7 @@ TEST_F(LoopingControlTest, LoopMoveTest) { m_pButtonBeatMoveBackward->set(0.0); ProcessBuffer(); EXPECT_EQ(0, m_pLoopStartPoint->get()); - EXPECT_NEAR(300, m_pLoopEndPoint->get(), 0.000000001); + EXPECT_NEAR(300, m_pLoopEndPoint->get(), kLoopPositionMaxAbsError); EXPECT_EQ(500, m_pChannel1->getEngineBuffer()->m_pLoopingControl->getCurrentSample()); }