Skip to content

Commit

Permalink
Merge pull request #2593 from goddisignz/LP1855321_zero_playposition_…
Browse files Browse the repository at this point in the history
…jump_to_cue

Fix LP1855321 - Prevent jumping to main cue
  • Loading branch information
Holzhaus authored Apr 9, 2020
2 parents 7342351 + 40b0b79 commit b94350b
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 34 deletions.
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 @@ -1678,10 +1700,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 @@ -713,6 +713,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

0 comments on commit b94350b

Please sign in to comment.