Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix LP1855321 - Prevent jumping to main cue #2593

Merged
merged 6 commits into from
Apr 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/analyzer/analyzerthread.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
78 changes: 48 additions & 30 deletions src/engine/controls/cuecontrol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,8 @@ 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);
Expand Down Expand Up @@ -410,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.
Expand All @@ -424,20 +420,28 @@ 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:
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) {
Expand Down Expand Up @@ -547,17 +551,15 @@ void CueControl::loadCuesFromTrack() {
}
}

void CueControl::reloadCuesFromTrack() {
if (!m_pLoadedTrack)
void CueControl::trackAnalyzed() {
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();
// 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();
Expand All @@ -567,28 +569,48 @@ 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);
}
}
}

void CueControl::trackCuesUpdated() {
reloadCuesFromTrack();
loadCuesFromTrack();
}

void CueControl::trackBeatsUpdated() {
reloadCuesFromTrack();
loadCuesFromTrack();
}

void CueControl::quantizeChanged(double v) {
Q_UNUSED(v);

reloadCuesFromTrack();
// check if we were at the cue point before
bool wasTrackAtCue = getTrackAt() == TrackAt::Cue;
bool wasTrackAtIntro = isTrackAtIntroCue();

loadCuesFromTrack();

// 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) {
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) {
Expand Down Expand Up @@ -1674,10 +1696,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);
}
Expand Down
3 changes: 1 addition & 2 deletions src/engine/controls/cuecontrol.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -138,6 +137,7 @@ class CueControl : public EngineControl {
void quantizeChanged(double v);

void cueUpdated();
void trackAnalyzed();
void trackCuesUpdated();
void trackBeatsUpdated();
void hotcueSet(HotcueControl* pControl, double v);
Expand Down Expand Up @@ -190,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;
Expand Down
46 changes: 44 additions & 2 deletions src/test/cuecontrol_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -292,14 +293,55 @@ 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<int>(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();

Expand Down
4 changes: 4 additions & 0 deletions src/track/track.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
3 changes: 3 additions & 0 deletions src/track/track.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down