From e07af9ebe45ee3d528ebfb69e0881f029f9cd0f5 Mon Sep 17 00:00:00 2001 From: Philip Gottschling Date: Wed, 25 Mar 2020 14:30:16 +0100 Subject: [PATCH 1/5] Fix LP1855321 - Prevent undesired jumping to main cue --- src/analyzer/analyzerthread.cpp | 1 + src/engine/controls/cuecontrol.cpp | 34 ++++++++++++++++----- src/engine/controls/cuecontrol.h | 1 + src/test/cuecontrol_test.cpp | 48 ++++++++++++++++++++++++++++-- src/track/track.cpp | 4 +++ src/track/track.h | 3 ++ 6 files changed, 82 insertions(+), 9 deletions(-) diff --git a/src/analyzer/analyzerthread.cpp b/src/analyzer/analyzerthread.cpp index a6f7f0802ea3..9affe5f545ad 100644 --- a/src/analyzer/analyzerthread.cpp +++ b/src/analyzer/analyzerthread.cpp @@ -348,6 +348,7 @@ void AnalyzerThread::emitDoneProgress(AnalyzerProgress doneProgress) { // thread that might trigger database actions! The TrackAnalysisScheduler // must store a TrackPointer until receiving the Done signal. TrackId trackId = m_currentTrack->getId(); + m_currentTrack->analysisFinished(); m_currentTrack.reset(); emitProgress(AnalyzerThreadState::Done, trackId, doneProgress); } diff --git a/src/engine/controls/cuecontrol.cpp b/src/engine/controls/cuecontrol.cpp index 59bfd428f48f..35e53669736a 100644 --- a/src/engine/controls/cuecontrol.cpp +++ b/src/engine/controls/cuecontrol.cpp @@ -363,6 +363,10 @@ void CueControl::trackLoaded(TrackPointer pNewTrack) { } m_pLoadedTrack = pNewTrack; + connect(m_pLoadedTrack.get(), &Track::analyzed, + this, &CueControl::trackAnalyzed, + Qt::DirectConnection); + connect(m_pLoadedTrack.get(), &Track::cuesUpdated, this, &CueControl::trackCuesUpdated, Qt::DirectConnection); @@ -551,13 +555,14 @@ void CueControl::reloadCuesFromTrack() { if (!m_pLoadedTrack) return; - // Determine current playing position of the track. - TrackAt trackAt = getTrackAt(); - bool wasTrackAtZeroPos = isTrackAtZeroPos(); - bool wasTrackAtIntroCue = isTrackAtIntroCue(); - // Update COs with cues from track. loadCuesFromTrack(); +} + +void CueControl::trackAnalyzed() { + if (!m_pLoadedTrack) { + return; + } // Retrieve current position of cues from COs. double cue = m_pCuePoint->get(); @@ -567,11 +572,11 @@ void CueControl::reloadCuesFromTrack() { SeekOnLoadMode seekOnLoadMode = getSeekOnLoadPreference(); if (seekOnLoadMode == SeekOnLoadMode::MainCue) { - if ((trackAt == TrackAt::Cue || wasTrackAtZeroPos) && cue != Cue::kNoPosition) { + if (cue != Cue::kNoPosition) { seekExact(cue); } } else if (seekOnLoadMode == SeekOnLoadMode::IntroStart) { - if ((wasTrackAtIntroCue || wasTrackAtZeroPos) && intro != Cue::kNoPosition) { + if (intro != Cue::kNoPosition) { seekExact(intro); } } @@ -588,7 +593,22 @@ void CueControl::trackBeatsUpdated() { void CueControl::quantizeChanged(double v) { Q_UNUSED(v); + // check if we were at the cue point before + bool wasTrackAtCue = getTrackAt() == TrackAt::Cue; + bool wasTrackAtIntro = isTrackAtIntroCue(); + reloadCuesFromTrack(); + + // Retrieve new cue pos and follow + double cue = m_pCuePoint->get(); + if (wasTrackAtCue && cue != Cue::kNoPosition) { + seekExact(cue); + } + // Retrieve new intro start pos and follow + double intro = m_pIntroStartPosition->get(); + if(wasTrackAtIntro && intro != Cue::kNoPosition) { + seekExact(intro); + } } void CueControl::hotcueSet(HotcueControl* pControl, double v) { diff --git a/src/engine/controls/cuecontrol.h b/src/engine/controls/cuecontrol.h index 2d406a7d439e..b1fdad21cac5 100644 --- a/src/engine/controls/cuecontrol.h +++ b/src/engine/controls/cuecontrol.h @@ -138,6 +138,7 @@ class CueControl : public EngineControl { void quantizeChanged(double v); void cueUpdated(); + void trackAnalyzed(); void trackCuesUpdated(); void trackBeatsUpdated(); void hotcueSet(HotcueControl* pControl, double v); diff --git a/src/test/cuecontrol_test.cpp b/src/test/cuecontrol_test.cpp index de18a0eb64f8..f2932ffc8b65 100644 --- a/src/test/cuecontrol_test.cpp +++ b/src/test/cuecontrol_test.cpp @@ -273,8 +273,9 @@ TEST_F(CueControlTest, SeekOnLoadMainCue) { EXPECT_DOUBLE_EQ(100.0, m_pCuePoint->get()); EXPECT_DOUBLE_EQ(100.0, getCurrentSample()); - // Move cue and check if track is following it. + // Move cue like silence analysis does and check if track is following it pTrack->setCuePoint(CuePosition(200.0)); + pTrack->analysisFinished(); ProcessBuffer(); EXPECT_DOUBLE_EQ(200.0, m_pCuePoint->get()); @@ -292,14 +293,57 @@ TEST_F(CueControlTest, SeekOnLoadDefault_CueInPreroll) { EXPECT_DOUBLE_EQ(-100.0, m_pCuePoint->get()); EXPECT_DOUBLE_EQ(-100.0, getCurrentSample()); - // Move cue and check if track is following it. + // Move cue like silence analysis does and check if track is following it pTrack->setCuePoint(CuePosition(-200.0)); + pTrack->analysisFinished(); ProcessBuffer(); EXPECT_DOUBLE_EQ(-200.0, m_pCuePoint->get()); EXPECT_DOUBLE_EQ(-200.0, getCurrentSample()); } +TEST_F(CueControlTest, FollowCueOnQuantize) { + config()->set(ConfigKey("[Controls]", "CueRecall"), + ConfigValue(static_cast(SeekOnLoadMode::MainCue))); + TrackPointer pTrack = createTestTrack(); + pTrack->setSampleRate(44100); + pTrack->setBpm(120.0); + + const int frameSize = 2; + const int sampleRate = pTrack->getSampleRate(); + const double bpm = pTrack->getBpm(); + const double beatLength = (60.0 * sampleRate / bpm) * frameSize; + double cuePos = 1.8*beatLength; + double quantizedCuePos = 2.0*beatLength; + pTrack->setCuePoint(cuePos); + + loadTrack(pTrack); + + EXPECT_DOUBLE_EQ(cuePos, m_pCuePoint->get()); + EXPECT_DOUBLE_EQ(cuePos, getCurrentSample()); + + // enable quantization and expect current position to follow + m_pQuantizeEnabled->set(1); + ProcessBuffer(); + EXPECT_DOUBLE_EQ(quantizedCuePos, m_pCuePoint->get()); + EXPECT_DOUBLE_EQ(quantizedCuePos, getCurrentSample()); + + // move current position to track start + m_pQuantizeEnabled->set(0); + ProcessBuffer(); + setCurrentSample(0.0); + ProcessBuffer(); + EXPECT_DOUBLE_EQ(0.0, getCurrentSample()); + + // enable quantization again and expect play position to stay at track start + m_pQuantizeEnabled->set(1); + ProcessBuffer(); + EXPECT_DOUBLE_EQ(quantizedCuePos, m_pCuePoint->get()); + EXPECT_DOUBLE_EQ(0.0, getCurrentSample()); + + +} + TEST_F(CueControlTest, IntroCue_SetStartEnd_ClearStartEnd) { TrackPointer pTrack = createAndLoadFakeTrack(); diff --git a/src/track/track.cpp b/src/track/track.cpp index 27e04c8282ff..9aa425c01244 100644 --- a/src/track/track.cpp +++ b/src/track/track.cpp @@ -702,6 +702,10 @@ void Track::setCuePoint(CuePosition cue) { emit cuesUpdated(); } +void Track::analysisFinished() { + emit analyzed(); +} + CuePosition Track::getCuePoint() const { QMutexLocker lock(&m_qMutex); return m_record.getCuePoint(); diff --git a/src/track/track.h b/src/track/track.h index 78965cf65dfa..87de8eba241d 100644 --- a/src/track/track.h +++ b/src/track/track.h @@ -248,6 +248,8 @@ class Track : public QObject { CuePosition getCuePoint() const; // Set the track's main cue point void setCuePoint(CuePosition cue); + // Call when analysis is done. + void analysisFinished(); // Calls for managing the track's cue points CuePointer createAndAddCue(); @@ -325,6 +327,7 @@ class Track : public QObject { void keysUpdated(); void ReplayGainUpdated(mixxx::ReplayGain replayGain); void cuesUpdated(); + void analyzed(); void changed(TrackId trackId); void dirty(TrackId trackId); From 97fc29860900e51bdd29234677d76ac3ae66ec4f Mon Sep 17 00:00:00 2001 From: Philip Gottschling Date: Wed, 25 Mar 2020 15:06:12 +0100 Subject: [PATCH 2/5] do not follow cue when playing --- src/engine/controls/cuecontrol.cpp | 16 ++++++++++++---- src/test/cuecontrol_test.cpp | 6 ++---- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/engine/controls/cuecontrol.cpp b/src/engine/controls/cuecontrol.cpp index 35e53669736a..bb5f322a0e73 100644 --- a/src/engine/controls/cuecontrol.cpp +++ b/src/engine/controls/cuecontrol.cpp @@ -363,9 +363,7 @@ void CueControl::trackLoaded(TrackPointer pNewTrack) { } m_pLoadedTrack = pNewTrack; - connect(m_pLoadedTrack.get(), &Track::analyzed, - this, &CueControl::trackAnalyzed, - Qt::DirectConnection); + connect(m_pLoadedTrack.get(), &Track::analyzed, this, &CueControl::trackAnalyzed, Qt::DirectConnection); connect(m_pLoadedTrack.get(), &Track::cuesUpdated, this, &CueControl::trackCuesUpdated, @@ -564,6 +562,11 @@ void CueControl::trackAnalyzed() { return; } + // if we are playing (no matter what reason for) do not seek + if (m_pPlay->toBool()) { + return; + } + // Retrieve current position of cues from COs. double cue = m_pCuePoint->get(); double intro = m_pIntroStartPosition->get(); @@ -599,6 +602,11 @@ void CueControl::quantizeChanged(double v) { reloadCuesFromTrack(); + // if we are playing (no matter what reason for) do not seek + if (m_pPlay->toBool()) { + return; + } + // Retrieve new cue pos and follow double cue = m_pCuePoint->get(); if (wasTrackAtCue && cue != Cue::kNoPosition) { @@ -606,7 +614,7 @@ void CueControl::quantizeChanged(double v) { } // Retrieve new intro start pos and follow double intro = m_pIntroStartPosition->get(); - if(wasTrackAtIntro && intro != Cue::kNoPosition) { + if (wasTrackAtIntro && intro != Cue::kNoPosition) { seekExact(intro); } } diff --git a/src/test/cuecontrol_test.cpp b/src/test/cuecontrol_test.cpp index f2932ffc8b65..ed21f7473da9 100644 --- a/src/test/cuecontrol_test.cpp +++ b/src/test/cuecontrol_test.cpp @@ -313,8 +313,8 @@ TEST_F(CueControlTest, FollowCueOnQuantize) { const int sampleRate = pTrack->getSampleRate(); const double bpm = pTrack->getBpm(); const double beatLength = (60.0 * sampleRate / bpm) * frameSize; - double cuePos = 1.8*beatLength; - double quantizedCuePos = 2.0*beatLength; + double cuePos = 1.8 * beatLength; + double quantizedCuePos = 2.0 * beatLength; pTrack->setCuePoint(cuePos); loadTrack(pTrack); @@ -340,8 +340,6 @@ TEST_F(CueControlTest, FollowCueOnQuantize) { ProcessBuffer(); EXPECT_DOUBLE_EQ(quantizedCuePos, m_pCuePoint->get()); EXPECT_DOUBLE_EQ(0.0, getCurrentSample()); - - } TEST_F(CueControlTest, IntroCue_SetStartEnd_ClearStartEnd) { From e96c90e1ee665e928c8a8452d354870b01c6903e Mon Sep 17 00:00:00 2001 From: Philip Gottschling Date: Sun, 29 Mar 2020 14:15:35 +0200 Subject: [PATCH 3/5] removed unused and unnecessary methods --- src/engine/controls/cuecontrol.cpp | 18 +++--------------- src/engine/controls/cuecontrol.h | 2 -- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/src/engine/controls/cuecontrol.cpp b/src/engine/controls/cuecontrol.cpp index bb5f322a0e73..b0b9a06372db 100644 --- a/src/engine/controls/cuecontrol.cpp +++ b/src/engine/controls/cuecontrol.cpp @@ -549,14 +549,6 @@ void CueControl::loadCuesFromTrack() { } } -void CueControl::reloadCuesFromTrack() { - if (!m_pLoadedTrack) - return; - - // Update COs with cues from track. - loadCuesFromTrack(); -} - void CueControl::trackAnalyzed() { if (!m_pLoadedTrack) { return; @@ -586,11 +578,11 @@ void CueControl::trackAnalyzed() { } void CueControl::trackCuesUpdated() { - reloadCuesFromTrack(); + loadCuesFromTrack(); } void CueControl::trackBeatsUpdated() { - reloadCuesFromTrack(); + loadCuesFromTrack(); } void CueControl::quantizeChanged(double v) { @@ -600,7 +592,7 @@ void CueControl::quantizeChanged(double v) { bool wasTrackAtCue = getTrackAt() == TrackAt::Cue; bool wasTrackAtIntro = isTrackAtIntroCue(); - reloadCuesFromTrack(); + loadCuesFromTrack(); // if we are playing (no matter what reason for) do not seek if (m_pPlay->toBool()) { @@ -1702,10 +1694,6 @@ double CueControl::quantizeCuePoint(double cuePos) { return cuePos; } -bool CueControl::isTrackAtZeroPos() { - return (fabs(getSampleOfTrack().current) < 1.0f); -} - bool CueControl::isTrackAtIntroCue() { return (fabs(getSampleOfTrack().current - m_pIntroStartPosition->get()) < 1.0f); } diff --git a/src/engine/controls/cuecontrol.h b/src/engine/controls/cuecontrol.h index b1fdad21cac5..517a511126c5 100644 --- a/src/engine/controls/cuecontrol.h +++ b/src/engine/controls/cuecontrol.h @@ -126,7 +126,6 @@ class CueControl : public EngineControl { void hintReader(HintVector* pHintList) override; bool updateIndicatorsAndModifyPlay(bool newPlay, bool playPossible); void updateIndicators(); - bool isTrackAtZeroPos(); bool isTrackAtIntroCue(); void resetIndicators(); bool isPlayingByPlayButton(); @@ -191,7 +190,6 @@ class CueControl : public EngineControl { void attachCue(CuePointer pCue, HotcueControl* pControl); void detachCue(HotcueControl* pControl); void loadCuesFromTrack(); - void reloadCuesFromTrack(); double quantizeCuePoint(double position); double getQuantizedCurrentPosition(); TrackAt getTrackAt() const; From 131de5f4a3ab43b3a737ed93fde8754a27f673fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nino=20Mi=C5=A1ki=C4=87-Pletenac?= Date: Sun, 29 Mar 2020 03:15:20 +0200 Subject: [PATCH 4/5] Prevent jump when track analysis finishes while quantization is enabled --- src/engine/controls/cuecontrol.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/engine/controls/cuecontrol.cpp b/src/engine/controls/cuecontrol.cpp index b0b9a06372db..0b638185af4d 100644 --- a/src/engine/controls/cuecontrol.cpp +++ b/src/engine/controls/cuecontrol.cpp @@ -433,13 +433,19 @@ void CueControl::trackLoaded(TrackPointer pNewTrack) { seekExact(0.0); } break; - case SeekOnLoadMode::MainCue: - if (mainCuePoint.getPosition() != Cue::kNoPosition) { - seekExact(mainCuePoint.getPosition()); + case SeekOnLoadMode::MainCue: { + // Take main cue position from CO instead of cue point list because + // value in CO will be quantized if quantization is enabled + // while value in cue point list will never be quantized. + // This prevents jumps when track analysis finishes while quantization is enabled. + double cuePoint = m_pCuePoint->get(); + if (cuePoint != Cue::kNoPosition) { + seekExact(cuePoint); } else { seekExact(0.0); } break; + } case SeekOnLoadMode::IntroStart: { double introStart = m_pIntroStartPosition->get(); if (introStart != Cue::kNoPosition) { From efe411f0bcb95f6cb710fbeb3a60cf7bccbee2ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nino=20Mi=C5=A1ki=C4=87-Pletenac?= Date: Sun, 29 Mar 2020 18:24:00 +0200 Subject: [PATCH 5/5] Fetch audible start position only when it is needed --- src/engine/controls/cuecontrol.cpp | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/engine/controls/cuecontrol.cpp b/src/engine/controls/cuecontrol.cpp index 0b638185af4d..a870b8444ad8 100644 --- a/src/engine/controls/cuecontrol.cpp +++ b/src/engine/controls/cuecontrol.cpp @@ -412,12 +412,6 @@ void CueControl::trackLoaded(TrackPointer pNewTrack) { // Seek track according to SeekOnLoadMode. SeekOnLoadMode seekOnLoadMode = getSeekOnLoadPreference(); - CuePointer pAudibleSound = pNewTrack->findCueByType(mixxx::CueType::AudibleSound); - double firstSound = Cue::kNoPosition; - if (pAudibleSound) { - firstSound = pAudibleSound->getPosition(); - } - switch (seekOnLoadMode) { case SeekOnLoadMode::Beginning: // This allows users to load tracks and have the needle-drop be maintained. @@ -426,13 +420,15 @@ void CueControl::trackLoaded(TrackPointer pNewTrack) { seekExact(0.0); } break; - case SeekOnLoadMode::FirstSound: - if (firstSound != Cue::kNoPosition) { - seekExact(firstSound); + case SeekOnLoadMode::FirstSound: { + CuePointer pAudibleSound = pNewTrack->findCueByType(mixxx::CueType::AudibleSound); + if (pAudibleSound && pAudibleSound->getPosition() != Cue::kNoPosition) { + seekExact(pAudibleSound->getPosition()); } else { seekExact(0.0); } break; + } case SeekOnLoadMode::MainCue: { // Take main cue position from CO instead of cue point list because // value in CO will be quantized if quantization is enabled