Skip to content

AutoDJProcessorTest.FadeToDeck2_Long_Transition fix #34

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.

Sign up for GitHub

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

Merged
Be-ing merged 19 commits into from
Nov 7, 2019
Merged

AutoDJProcessorTest.FadeToDeck2_Long_Transition fix #34

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
c1010a6
make emitLoadTrackToPlayer and emitAutoDJStateChanged a protected non…
daschuer Oct 27, 2019
cf658a9
move no outro start fallback code into getOutroStartPosition()
daschuer Oct 27, 2019
df29633
consider current position of the from deck after chaning the transiti…
daschuer Oct 27, 2019
2b1f525
fix AutoDJProcessorTest.FadeToDeck2_Long_Transition test. This fixes …
daschuer Oct 27, 2019
5e6988e
Cache hasFadeTransition before possible override
daschuer Oct 28, 2019
74fdcb8
Adjust FadeToDeck2_Pause_Transition for no fading transitions
daschuer Oct 28, 2019
277f5da
improve comments
daschuer Oct 28, 2019
96a5a45
improve error handing in case of -1 cue positions.
daschuer Oct 28, 2019
873e313
improve crossfaderChanged()
daschuer Oct 29, 2019
d6e6cf4
Adjust short track workaround in outro start mode
daschuer Oct 30, 2019
70214df
Adjust intro and outro change slots
daschuer Oct 30, 2019
e1c45e5
don't bother with playing state when calculating a new transition
daschuer Oct 30, 2019
6dac3a6
Don't use non cons references in playerPositionChanged() and remove f…
daschuer Oct 31, 2019
fd36927
Don't use non const references in toggleAutoDJ()
daschuer Oct 31, 2019
2f88f54
Reset startPos, just in case calculateTransition() is not able to set…
daschuer Oct 31, 2019
7036b74
Set the just loaded deck as toDeck
daschuer Nov 3, 2019
9951daa
respect the rate slider when calculating auto-DJ transitions
daschuer Nov 4, 2019
db49654
Whatch rate changes
daschuer Nov 4, 2019
efa6c00
fix recursive positionChanged call. Fix omitting fist seek.
daschuer Nov 7, 2019
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
65 changes: 33 additions & 32 deletions src/library/autodj/autodjprocessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -622,14 +622,16 @@ void AutoDJProcessor::playerPositionChanged(DeckAttributes* pAttributes,
}

if (m_eState == ADJ_IDLE) {
if (!thisDeckPlaying) {
if (!thisDeckPlaying && thisPlayPosition < 1) {
// this is a cueing seek, recalculate the transition, from the
// new position.
// This can be our own seek to startPos or a random seek by a user.
// we need to call calculateTransition() because we are not sure.
// If using the full track mode with a transition time of 0,
// thisDeckPlaying will be false but the transition should not be
// recalculated here.
// Don't adjust transition when reaching the end. In this case it is
// always stopped.
calculateTransition(&otherDeck, &thisDeck, false);
} else if (thisDeck.isRepeat()) {
// repeat pauses auto DJ
Expand All @@ -644,6 +646,8 @@ void AutoDJProcessor::playerPositionChanged(DeckAttributes* pAttributes,
if (thisPlayPosition >= thisDeck.fadeBeginPos && thisDeck.isFromDeck) {
if (m_eState == ADJ_IDLE) {
if (thisDeckPlaying || thisPlayPosition >= 1.0) {
// cache this before calculating the new transition in otherDeck.play();
bool hasFadeTransition = thisDeck.fadeBeginPos < thisDeck.fadeEndPos;

@Be-ing Be-ing Oct 28, 2019

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When could this be false? It is not obvious. At first glance it seems like this should be a debug assert.

@daschuer daschuer Oct 28, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is always false, if you use a negative or zero transition time.
I will add a comment.

if (!otherDeckPlaying) {
// Re-cue the track if the user has seeked it to the very end
if (otherDeck.playPosition() >= otherDeck.fadeBeginPos) {
Expand All @@ -656,6 +660,12 @@ void AutoDJProcessor::playerPositionChanged(DeckAttributes* pAttributes,
// that was "on deck" from the top of the queue.
removeLoadedTrackFromTopOfQueue(otherDeck);

if (!hasFadeTransition) {
setCrossfader(thisDeck.isLeft() ? 1.0 : -1.0);
thisDeck.stop();
loadNextTrackFromQueue(thisDeck);
return;
}
m_transitionProgress = 0.0;
// Set the state as FADING.
m_eState = thisDeck.isLeft() ? ADJ_LEFT_FADING : ADJ_RIGHT_FADING;
Expand Down Expand Up @@ -685,24 +695,6 @@ void AutoDJProcessor::playerPositionChanged(DeckAttributes* pAttributes,
m_transitionProgress = 1.0;
} else {
// We are in Fading state.

// The math below will not work with a transition time of 0.
if (thisDeck.fadeBeginPos == thisDeck.fadeEndPos) {
setCrossfader(crossfaderTarget);
m_transitionProgress = 1.0;
// Usually code above will take care of these steps when the
// playposition updates again, however this does not occur when
// using the Full Track mode with a fade time of 0 because the
// toDeck stops.
if (thisPlayPosition >= 1) {
otherDeck.play();
loadNextTrackFromQueue(thisDeck);
m_eState = ADJ_IDLE;
emitAutoDJStateChanged(m_eState);
}
return;
}

// Calculate the current transitionProgress, the place between begin
// and end position and the step we have taken since the last call
double transitionProgress = (thisPlayPosition - thisDeck.fadeBeginPos) /
Expand Down Expand Up @@ -932,7 +924,15 @@ double AutoDJProcessor::getIntroEndPosition(DeckAttributes* pDeck) {
}

double AutoDJProcessor::getOutroStartPosition(DeckAttributes* pDeck) {
return samplePositionToSeconds(pDeck->outroStartPosition(), pDeck);
double outroStart = samplePositionToSeconds(pDeck->outroStartPosition(), pDeck);
if (outroStart < 0.0) {
// Assume a zero length outro if outroStartIsNot set.
// The outroEnd is automatically placed by AnalyzerSilence, so use
// that as a fallback if the user has not placed outroStart. If it has
// not been placed, getOutroEndPosition will return the end of the track.
outroStart = getOutroEndPosition(pDeck);
}
return outroStart;
}

double AutoDJProcessor::getOutroEndPosition(DeckAttributes* pDeck) {
Expand Down Expand Up @@ -1037,13 +1037,6 @@ void AutoDJProcessor::calculateTransition(DeckAttributes* pFromDeck,
double outroStart = getOutroStartPosition(pFromDeck);
double fromDeckPosition = pFromDeck->playPosition() * pFromDeck->duration();

if (outroStart <= 0.0) {
// Assume a zero length outro.
// The outroEnd is automatically placed by AnalyzerSilence, so use
// that as a fallback if the user has not placed outroStart. If it has
// not been placed, getOutroEndPosition will return the end of the track.
outroStart = outroEnd;
}
if (fromDeckPosition > outroStart) {
// We have already passed outroStart
// This can happen if we have just enabled auto DJ
Expand All @@ -1060,6 +1053,7 @@ void AutoDJProcessor::calculateTransition(DeckAttributes* pFromDeck,
// here it is done for FullIntroOutro and FadeAtOutroStart.
// It is adjusted below for the other modes.
pToDeck->fadeBeginPos = getOutroStartPosition(pToDeck);
pToDeck->fadeEndPos = getOutroEndPosition(pToDeck);

double introStart;
if (seekToStartPoint || toDeckPosition >= pToDeck->fadeBeginPos) {
Expand Down Expand Up @@ -1136,7 +1130,7 @@ void AutoDJProcessor::calculateTransition(DeckAttributes* pFromDeck,
pFromDeck->fadeEndPos = outroEnd;
pToDeck->startPos = introStart;
} else {
useFixedFadeTime(pFromDeck, pToDeck, outroEnd, introStart);
useFixedFadeTime(pFromDeck, pToDeck, fromDeckPosition, outroEnd, introStart);
}
break;
case TransitionMode::FadeAtOutroStart:
Expand Down Expand Up @@ -1191,7 +1185,7 @@ void AutoDJProcessor::calculateTransition(DeckAttributes* pFromDeck,
pFromDeck->fadeEndPos = outroEnd;
pToDeck->startPos = introStart;
} else {
useFixedFadeTime(pFromDeck, pToDeck, outroEnd, introStart);
useFixedFadeTime(pFromDeck, pToDeck, fromDeckPosition, outroEnd, introStart);
}
break;
case TransitionMode::FixedSkipSilence:
Expand All @@ -1208,6 +1202,7 @@ void AutoDJProcessor::calculateTransition(DeckAttributes* pFromDeck,
startPoint = toDeckPosition;
}
useFixedFadeTime(pFromDeck, pToDeck,
fromDeckPosition,
getLastSoundPosition(pFromDeck),
startPoint);
}
Expand All @@ -1227,6 +1222,7 @@ void AutoDJProcessor::calculateTransition(DeckAttributes* pFromDeck,
startPoint = toDeckPosition;
}
useFixedFadeTime(pFromDeck, pToDeck,
fromDeckPosition,
pFromDeck->duration(),
startPoint);
}
Expand All @@ -1237,6 +1233,7 @@ void AutoDJProcessor::calculateTransition(DeckAttributes* pFromDeck,
pFromDeck->fadeEndPos /= fromTrackDuration;
pToDeck->startPos /= toTrackDuration;
pToDeck->fadeBeginPos /= toTrackDuration;
pToDeck->fadeEndPos /= toTrackDuration;

pFromDeck->isFromDeck = true;
pToDeck->isFromDeck = false;
Expand All @@ -1251,11 +1248,15 @@ void AutoDJProcessor::calculateTransition(DeckAttributes* pFromDeck,
}

void AutoDJProcessor::useFixedFadeTime(DeckAttributes* pFromDeck,
DeckAttributes* pToDeck, double endPoint, double startPoint) {
DeckAttributes* pToDeck, double fromDeckPosition, double endPoint, double startPoint) {
if (m_transitionTime > 0.0) {
// Guard against the next track being too short. This transition must finish
// before the next transition starts.
double toDeckOutroStart = pToDeck->fadeBeginPos;
if (pToDeck->fadeBeginPos >= pToDeck->fadeEndPos) {
// no outro defined, the toDeck will also use the transition time
toDeckOutroStart -= m_transitionTime;
}
if (toDeckOutroStart <= startPoint) {
// we are already too late
// Check OutroEnd as alternative, which is for all transition mode
Expand All @@ -1275,12 +1276,12 @@ void AutoDJProcessor::useFixedFadeTime(DeckAttributes* pFromDeck,
double transitionTime = math_min(toDeckOutroStart - startPoint,
m_transitionTime);

pFromDeck->fadeBeginPos = endPoint - transitionTime;
pFromDeck->fadeBeginPos = math_max(endPoint - transitionTime, fromDeckPosition);
pFromDeck->fadeEndPos = endPoint;
pToDeck->startPos = startPoint;
} else {
pFromDeck->fadeBeginPos = endPoint;
pFromDeck->fadeEndPos = endPoint - m_transitionTime;
pFromDeck->fadeEndPos = endPoint;

@Be-ing Be-ing Oct 27, 2019

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks fading with a negative transition time. AutoDJ stops in this case.

@daschuer daschuer Oct 28, 2019

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was causes elsewhere, but fixed now.

pToDeck->startPos = startPoint + m_transitionTime;
}
}
Expand Down
21 changes: 11 additions & 10 deletions src/library/autodj/autodjprocessor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,15 +196,6 @@ class AutoDJProcessor : public QObject {
void fadeNow();
AutoDJError toggleAutoDJ(bool enable);

// The following virtual signal wrappers are used for testing
virtual void emitLoadTrackToPlayer(TrackPointer pTrack, QString group,
bool play) {
emit(loadTrackToPlayer(pTrack, group, play));
}
virtual void emitAutoDJStateChanged(AutoDJProcessor::AutoDJState state) {
emit(autoDJStateChanged(state));
}

signals:
void loadTrackToPlayer(TrackPointer pTrack, QString group,
bool play);
Expand All @@ -229,6 +220,16 @@ class AutoDJProcessor : public QObject {
void controlShuffle(double value);
void controlSkipNext(double value);

protected:
// The following virtual signal wrappers are used for testing
virtual void emitLoadTrackToPlayer(TrackPointer pTrack, QString group,
bool play) {
emit(loadTrackToPlayer(pTrack, group, play));
}
virtual void emitAutoDJStateChanged(AutoDJProcessor::AutoDJState state) {
emit(autoDJStateChanged(state));
}

private:
// Gets or sets the crossfader position while normalizing it so that -1 is
// all the way mixed to the left side and 1 is all the way mixed to the
Expand All @@ -254,7 +255,7 @@ class AutoDJProcessor : public QObject {
DeckAttributes* pToDeck,
bool seekToStartPoint);
void useFixedFadeTime(DeckAttributes* pFromDeck, DeckAttributes* pToDeck,
double endPoint, double startPoint);
double fromDeckPosition, double endPoint, double startPoint);
DeckAttributes* getOtherDeck(const DeckAttributes* pFromDeck,
bool playing = false);

Expand Down