From 45493b94b298d554aa77e121450296c6abdcfe4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sun, 27 Feb 2022 11:47:48 +0100 Subject: [PATCH 1/7] AutoDJ: Added more debug and assert statements --- src/library/autodj/autodjprocessor.cpp | 29 +++++++++++++++----------- 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/library/autodj/autodjprocessor.cpp b/src/library/autodj/autodjprocessor.cpp index 87a9e7cf9c3..14c22b11e79 100644 --- a/src/library/autodj/autodjprocessor.cpp +++ b/src/library/autodj/autodjprocessor.cpp @@ -621,11 +621,6 @@ void AutoDJProcessor::crossfaderChanged(double value) { void AutoDJProcessor::playerPositionChanged(DeckAttributes* pAttributes, double thisPlayPosition) { - if (sDebug) { - //qDebug() << this << "playerPositionChanged" << pAttributes->group - // << thisPlayPosition; - } - // Auto-DJ needs at least two decks VERIFY_OR_DEBUG_ASSERT(m_decks.length() > 1) { return; @@ -778,6 +773,12 @@ void AutoDJProcessor::playerPositionChanged(DeckAttributes* pAttributes, // that was "on deck" from the top of the queue. // Note: This is a DB call and takes long. removeLoadedTrackFromTopOfQueue(*otherDeck); + } else { + if (sDebug) { + qDebug() << this << "playerPositionChanged()" + << pAttributes->group << thisPlayPosition + << "but not playing"; + } } } @@ -1137,23 +1138,23 @@ double AutoDJProcessor::samplePositionToSeconds(double samplePosition, DeckAttri void AutoDJProcessor::calculateTransition(DeckAttributes* pFromDeck, DeckAttributes* pToDeck, bool seekToStartPoint) { - if (pFromDeck == nullptr || pToDeck == nullptr) { + VERIFY_OR_DEBUG_ASSERT(pFromDeck && pToDeck) { return; } - if (pFromDeck->loading || pToDeck->loading) { + VERIFY_OR_DEBUG_ASSERT(!pFromDeck->loading && !pToDeck->loading) { // don't use halve new halve old data during // changing of tracks return; } - qDebug() << "player" << pFromDeck->group << "calculateTransition()"; - // We require ADJ_IDLE to prevent changing the thresholds in the middle of a // fade. VERIFY_OR_DEBUG_ASSERT(m_eState == ADJ_IDLE) { return; } + qDebug() << "player" << pFromDeck->group << "calculateTransition()"; + const double fromDeckEndPosition = getEndSecond(pFromDeck); const double toDeckEndPosition = getEndSecond(pToDeck); // Since the end position is measured in seconds from 0:00 it is also @@ -1169,7 +1170,7 @@ void AutoDJProcessor::calculateTransition(DeckAttributes* pFromDeck, pToDeck->startPos = kKeepPosition; return; } - if (toDeckDuration <= 0) { + VERIFY_OR_DEBUG_ASSERT(toDeckDuration > 0) { // Playing Track has no duration. This should not happen, because short // tracks are skipped after load. loadNextTrackFromQueue(*pToDeck, false); @@ -1459,6 +1460,10 @@ void AutoDJProcessor::playerTrackLoaded(DeckAttributes* pDeck, TrackPointer pTra // repeat a probably missed update playerPositionChanged(fromDeck, 1.0); } + } else { + if (sDebug) { + qDebug() << this << "playerTrackLoaded()" << pDeck->group << "but not a toDeck"; + } } } } @@ -1468,8 +1473,8 @@ void AutoDJProcessor::playerLoadingTrack(DeckAttributes* pDeck, TrackPointer pNewTrack, TrackPointer pOldTrack) { if (sDebug) { qDebug() << this << "playerLoadingTrack" << pDeck->group - << "new:"<< (pNewTrack ? pNewTrack->getLocation() : "(null)") - << "old:"<< (pOldTrack ? pOldTrack->getLocation() : "(null)"); + << "new:" << (pNewTrack ? pNewTrack->getLocation() : "(null)") + << "old:" << (pOldTrack ? pOldTrack->getLocation() : "(null)"); } pDeck->loading = true; From 49819ef33f7d359b89e30ad922cdd8a06d6160ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Wed, 2 Mar 2022 11:34:34 +0100 Subject: [PATCH 2/7] Improve comments in AutoDJProcessor::playerPlayChanged --- src/library/autodj/autodjprocessor.cpp | 43 ++++++++++++++------------ 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/src/library/autodj/autodjprocessor.cpp b/src/library/autodj/autodjprocessor.cpp index 14c22b11e79..6e7dda311c6 100644 --- a/src/library/autodj/autodjprocessor.cpp +++ b/src/library/autodj/autodjprocessor.cpp @@ -957,26 +957,29 @@ void AutoDJProcessor::playerPlayChanged(DeckAttributes* thisDeck, bool playing) return; } - if (playing && !otherDeck->isPlaying()) { - // In case both decks were stopped and now this one just started, make - // this deck the "from deck". - calculateTransition(thisDeck, getOtherDeck(thisDeck), false); - } - - // If the user manually pressed play on the "to deck" before fading, for - // example to adjust the intro/outro cues, and lets the deck play until the - // end, seek back to the start point instead of keeping the deck stopped at - // the end. - if (!playing && thisDeck->playPosition() >= 1.0 && !thisDeck->isFromDeck) { - // toDeck has stopped at the end. Recalculate the transition, because - // it has been done from a now irrelevant previous position. - // This forces the other deck to be the fromDeck. - thisDeck->startPos = kKeepPosition; - calculateTransition(otherDeck, thisDeck, true); - if (thisDeck->startPos != kKeepPosition) { - // Note: this seek will trigger the playerPositionChanged slot - // which may calls the calculateTransition() again without seek = true; - thisDeck->setPlayPosition(thisDeck->startPos); + if (playing) { + if (!otherDeck->isPlaying()) { + // In case both decks were stopped and now this one just started, make + // this deck the "from deck". + calculateTransition(thisDeck, getOtherDeck(thisDeck), false); + } + } else { + // Deck paused + // This may happen if the user has previously pressed play on the "to deck" + // before fading, for example to adjust the intro/outro cues, and lets the + // deck play until the end, seek back to the start point instead of keeping + // the deck stopped at the end. + if (thisDeck->playPosition() >= 1.0 && !thisDeck->isFromDeck) { + // toDeck has stopped at the end. Recalculate the transition, because + // it has been done from a now irrelevant previous position. + // This forces the other deck to be the fromDeck. + thisDeck->startPos = kKeepPosition; + calculateTransition(otherDeck, thisDeck, true); + if (thisDeck->startPos != kKeepPosition) { + // Note: this seek will trigger the playerPositionChanged slot + // which may calls the calculateTransition() again without seek = true; + thisDeck->setPlayPosition(thisDeck->startPos); + } } } } From c12cc2c6ab90e58cb5d22d2958f73080a59dbccc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Wed, 2 Mar 2022 12:10:04 +0100 Subject: [PATCH 3/7] Add debug message before every calculateTransition() call --- src/library/autodj/autodjprocessor.cpp | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/library/autodj/autodjprocessor.cpp b/src/library/autodj/autodjprocessor.cpp index 6e7dda311c6..d5398d54b28 100644 --- a/src/library/autodj/autodjprocessor.cpp +++ b/src/library/autodj/autodjprocessor.cpp @@ -692,6 +692,10 @@ void AutoDJProcessor::playerPositionChanged(DeckAttributes* pAttributes, } else { // At least right deck is playing // Set crossfade thresholds for right deck. + if (sDebug) { + qDebug() << this << "playerPositionChanged" + << "right deck playing"; + } calculateTransition(rightDeck, leftDeck, false); } emitAutoDJStateChanged(m_eState); @@ -738,6 +742,10 @@ void AutoDJProcessor::playerPositionChanged(DeckAttributes* pAttributes, // recalculated here. // Don't adjust transition when reaching the end. In this case it is // always stopped. + if (sDebug) { + qDebug() << this << "playerPositionChanged" + << "cueing seek"; + } calculateTransition(otherDeck, thisDeck, false); } else if (thisDeck->isRepeat()) { // repeat pauses auto DJ @@ -964,11 +972,10 @@ void AutoDJProcessor::playerPlayChanged(DeckAttributes* thisDeck, bool playing) calculateTransition(thisDeck, getOtherDeck(thisDeck), false); } } else { - // Deck paused - // This may happen if the user has previously pressed play on the "to deck" - // before fading, for example to adjust the intro/outro cues, and lets the - // deck play until the end, seek back to the start point instead of keeping - // the deck stopped at the end. + // Deck paused + // This may happen if the user has previously pressed play on the "to deck" + // before fading, for example to adjust the intro/outro cues, and lets the + // deck play until the end, seek back to the start point instead of keeping if (thisDeck->playPosition() >= 1.0 && !thisDeck->isFromDeck) { // toDeck has stopped at the end. Recalculate the transition, because // it has been done from a now irrelevant previous position. From 6bf68be972751d860a059230d6e19b487036b22d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Wed, 2 Mar 2022 12:41:33 +0100 Subject: [PATCH 4/7] Debug intro and outro length --- src/library/autodj/autodjprocessor.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/library/autodj/autodjprocessor.cpp b/src/library/autodj/autodjprocessor.cpp index d5398d54b28..aee5370c652 100644 --- a/src/library/autodj/autodjprocessor.cpp +++ b/src/library/autodj/autodjprocessor.cpp @@ -1236,6 +1236,12 @@ void AutoDJProcessor::calculateTransition(DeckAttributes* pFromDeck, } double introLength = introEnd - introStart; + + if (sDebug) { + qDebug() << this << "calculateTransition" + << "introLength" << introLength + << "outroLength" << outroLength; + } switch (m_transitionMode) { case TransitionMode::FullIntroOutro: { From 4cd4e33788c0e6d55eb7d9c7d8d2061d3f5e4a94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Fri, 4 Mar 2022 21:57:44 +0100 Subject: [PATCH 5/7] Fix unintendend jump-cuts in auto DJ This happened because the track is not able to seek exactly to the intro start point. In this case taking the start point as end point does not lead to a zero length intro. --- src/library/autodj/autodjprocessor.cpp | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/library/autodj/autodjprocessor.cpp b/src/library/autodj/autodjprocessor.cpp index aee5370c652..4efd3dc55da 100644 --- a/src/library/autodj/autodjprocessor.cpp +++ b/src/library/autodj/autodjprocessor.cpp @@ -1163,8 +1163,6 @@ void AutoDJProcessor::calculateTransition(DeckAttributes* pFromDeck, return; } - qDebug() << "player" << pFromDeck->group << "calculateTransition()"; - const double fromDeckEndPosition = getEndSecond(pFromDeck); const double toDeckEndPosition = getEndSecond(pToDeck); // Since the end position is measured in seconds from 0:00 it is also @@ -1227,16 +1225,15 @@ void AutoDJProcessor::calculateTransition(DeckAttributes* pFromDeck, introStart = toDeckPositionSeconds; } - double introEnd = getIntroEndSecond(pToDeck); - if (introEnd < introStart) { - // introEnd is invalid. Assume a zero length intro. - // The introStart is automatically placed by AnalyzerSilence, so use - // that as a fallback if the user has not placed introEnd. - introEnd = introStart; + double introLength = 0; + const double introEndSample = pToDeck->introEndPosition(); + if (introEndSample != Cue::kNoPosition) { + const double introEnd = samplePositionToSeconds(introEndSample, pToDeck); + if (introStart < introEnd) { + introLength = introEnd - introStart; + } } - double introLength = introEnd - introStart; - if (sDebug) { qDebug() << this << "calculateTransition" << "introLength" << introLength From d0e8a68c4bbf95772fbea0e78635d021d9398c32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sun, 13 Mar 2022 15:13:51 +0100 Subject: [PATCH 6/7] AutoDJ: make use of if constexpr --- src/library/autodj/autodjprocessor.cpp | 34 +++++++++++++------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/library/autodj/autodjprocessor.cpp b/src/library/autodj/autodjprocessor.cpp index 4efd3dc55da..0339c4e9e66 100644 --- a/src/library/autodj/autodjprocessor.cpp +++ b/src/library/autodj/autodjprocessor.cpp @@ -19,7 +19,7 @@ const double kKeepPosition = -1.0; const mixxx::audio::ChannelCount kChannelCount = mixxx::kEngineChannelCount; -static const bool sDebug = false; +constexpr bool sDebug = false; } // anonymous namespace DeckAttributes::DeckAttributes(int index, @@ -692,7 +692,7 @@ void AutoDJProcessor::playerPositionChanged(DeckAttributes* pAttributes, } else { // At least right deck is playing // Set crossfade thresholds for right deck. - if (sDebug) { + if constexpr (sDebug) { qDebug() << this << "playerPositionChanged" << "right deck playing"; } @@ -742,7 +742,7 @@ void AutoDJProcessor::playerPositionChanged(DeckAttributes* pAttributes, // recalculated here. // Don't adjust transition when reaching the end. In this case it is // always stopped. - if (sDebug) { + if constexpr (sDebug) { qDebug() << this << "playerPositionChanged" << "cueing seek"; } @@ -782,7 +782,7 @@ void AutoDJProcessor::playerPositionChanged(DeckAttributes* pAttributes, // Note: This is a DB call and takes long. removeLoadedTrackFromTopOfQueue(*otherDeck); } else { - if (sDebug) { + if constexpr (sDebug) { qDebug() << this << "playerPositionChanged()" << pAttributes->group << thisPlayPosition << "but not playing"; @@ -944,7 +944,7 @@ void AutoDJProcessor::maybeFillRandomTracks() { } void AutoDJProcessor::playerPlayChanged(DeckAttributes* thisDeck, bool playing) { - if (sDebug) { + if constexpr (sDebug) { qDebug() << this << "playerPlayChanged" << thisDeck->group << playing; } @@ -992,7 +992,7 @@ void AutoDJProcessor::playerPlayChanged(DeckAttributes* thisDeck, bool playing) } void AutoDJProcessor::playerIntroStartChanged(DeckAttributes* pAttributes, double position) { - if (sDebug) { + if constexpr (sDebug) { qDebug() << this << "playerIntroStartChanged" << pAttributes->group << position; } // nothing to do, because we want not to re-cue the toDeck and the from @@ -1000,7 +1000,7 @@ void AutoDJProcessor::playerIntroStartChanged(DeckAttributes* pAttributes, doubl } void AutoDJProcessor::playerIntroEndChanged(DeckAttributes* pAttributes, double position) { - if (sDebug) { + if constexpr (sDebug) { qDebug() << this << "playerIntroEndChanged" << pAttributes->group << position; } @@ -1021,7 +1021,7 @@ void AutoDJProcessor::playerIntroEndChanged(DeckAttributes* pAttributes, double } void AutoDJProcessor::playerOutroStartChanged(DeckAttributes* pAttributes, double position) { - if (sDebug) { + if constexpr (sDebug) { qDebug() << this << "playerOutroStartChanged" << pAttributes->group << position; } @@ -1038,7 +1038,7 @@ void AutoDJProcessor::playerOutroStartChanged(DeckAttributes* pAttributes, doubl } void AutoDJProcessor::playerOutroEndChanged(DeckAttributes* pAttributes, double position) { - if (sDebug) { + if constexpr (sDebug) { qDebug() << this << "playerOutroEndChanged" << pAttributes->group << position; } @@ -1234,7 +1234,7 @@ void AutoDJProcessor::calculateTransition(DeckAttributes* pFromDeck, } } - if (sDebug) { + if constexpr (sDebug) { qDebug() << this << "calculateTransition" << "introLength" << introLength << "outroLength" << outroLength; @@ -1384,7 +1384,7 @@ void AutoDJProcessor::calculateTransition(DeckAttributes* pFromDeck, pFromDeck->fadeBeginPos = 1; } - if (sDebug) { + if constexpr (sDebug) { qDebug() << this << "calculateTransition" << pFromDeck->group << pFromDeck->fadeBeginPos << pFromDeck->fadeEndPos << pToDeck->startPos; @@ -1435,7 +1435,7 @@ void AutoDJProcessor::useFixedFadeTime( } void AutoDJProcessor::playerTrackLoaded(DeckAttributes* pDeck, TrackPointer pTrack) { - if (sDebug) { + if constexpr (sDebug) { qDebug() << this << "playerTrackLoaded" << pDeck->group << (pTrack ? pTrack->getLocation() : "(null)"); } @@ -1474,7 +1474,7 @@ void AutoDJProcessor::playerTrackLoaded(DeckAttributes* pDeck, TrackPointer pTra playerPositionChanged(fromDeck, 1.0); } } else { - if (sDebug) { + if constexpr (sDebug) { qDebug() << this << "playerTrackLoaded()" << pDeck->group << "but not a toDeck"; } } @@ -1484,7 +1484,7 @@ void AutoDJProcessor::playerTrackLoaded(DeckAttributes* pDeck, TrackPointer pTra void AutoDJProcessor::playerLoadingTrack(DeckAttributes* pDeck, TrackPointer pNewTrack, TrackPointer pOldTrack) { - if (sDebug) { + if constexpr (sDebug) { qDebug() << this << "playerLoadingTrack" << pDeck->group << "new:" << (pNewTrack ? pNewTrack->getLocation() : "(null)") << "old:" << (pOldTrack ? pOldTrack->getLocation() : "(null)"); @@ -1513,7 +1513,7 @@ void AutoDJProcessor::playerLoadingTrack(DeckAttributes* pDeck, } void AutoDJProcessor::playerEmpty(DeckAttributes* pDeck) { - if (sDebug) { + if constexpr (sDebug) { qDebug() << this << "playerEmpty()" << pDeck->group; } @@ -1530,7 +1530,7 @@ void AutoDJProcessor::playerEmpty(DeckAttributes* pDeck) { } void AutoDJProcessor::playerRateChanged(DeckAttributes* pAttributes) { - if (sDebug) { + if constexpr (sDebug) { qDebug() << this << "playerRateChanged" << pAttributes->group; } @@ -1547,7 +1547,7 @@ void AutoDJProcessor::playerRateChanged(DeckAttributes* pAttributes) { } void AutoDJProcessor::setTransitionTime(int time) { - if (sDebug) { + if constexpr (sDebug) { qDebug() << this << "setTransitionTime" << time; } From 02811a9f6abafcf952ac2e26895751770f18754f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sun, 13 Mar 2022 22:39:22 +0100 Subject: [PATCH 7/7] AutoDJ: remove wrong assertion --- src/library/autodj/autodjprocessor.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/library/autodj/autodjprocessor.cpp b/src/library/autodj/autodjprocessor.cpp index 0339c4e9e66..8d76ee88b04 100644 --- a/src/library/autodj/autodjprocessor.cpp +++ b/src/library/autodj/autodjprocessor.cpp @@ -1151,7 +1151,7 @@ void AutoDJProcessor::calculateTransition(DeckAttributes* pFromDeck, VERIFY_OR_DEBUG_ASSERT(pFromDeck && pToDeck) { return; } - VERIFY_OR_DEBUG_ASSERT(!pFromDeck->loading && !pToDeck->loading) { + if (pFromDeck->loading || pToDeck->loading) { // don't use halve new halve old data during // changing of tracks return;