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.

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

AutoDJ: don't use removed Intro end and outro start makers, use transition time instead #11830

Merged
merged 5 commits into from
Dec 19, 2023
Merged

AutoDJ: don't use removed Intro end and outro start makers, use transition time instead #11830

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
dd608bf
Auto-DJ: Don't adopt last sound as outro end, when user has explicit …
daschuer Aug 13, 2023
430a8f8
Auto-DJ: Don't adopt first sound as intro start, when user has explic…
daschuer Aug 13, 2023
8f892dc
Add some comments
daschuer Oct 5, 2023
8e1e8bb
Improve comments
daschuer Dec 18, 2023
c2a54a7
Allow to place an automatic set transition into the prerole
daschuer Dec 18, 2023
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
41 changes: 38 additions & 3 deletions src/library/autodj/autodjprocessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1093,8 +1093,25 @@ void AutoDJProcessor::playerOutroEndChanged(DeckAttributes* pAttributes, double
double AutoDJProcessor::getIntroStartSecond(DeckAttributes* pDeck) {
const mixxx::audio::FramePos trackEndPosition = pDeck->trackEndPosition();
const mixxx::audio::FramePos introStartPosition = pDeck->introStartPosition();
const mixxx::audio::FramePos introEndPosition = pDeck->introEndPosition();
if (!introStartPosition.isValid() || introStartPosition > trackEndPosition) {
return getFirstSoundSecond(pDeck);
double firstSoundSecond = getFirstSoundSecond(pDeck);
if (!introEndPosition.isValid() || introEndPosition > trackEndPosition) {
// No intro start and intro end set, use First Sound.
return firstSoundSecond;
}
double introEndSecond = framePositionToSeconds(introEndPosition, pDeck);
if (m_transitionTime >= 0 && firstSoundSecond < introEndSecond) {
double introStartFromTime = introEndSecond - m_transitionTime;
if (introStartFromTime > firstSoundSecond) {
Copy link
Member

Choose a reason for hiding this comment

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

This would exclude introStart in the preroll if m_transitionTime > introEndSecond.
Is this intended?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think originally it was intended. But now I see no strong reason for it. So let's allow it.

// The introStart is automatically placed by AnalyzerSilence at the first sound
// Here the user has removed it, but has placed an intro end.
// Use the transition time instead the dismissed first sound position.
daschuer marked this conversation as resolved.
Show resolved Hide resolved
return introStartFromTime;
}
return firstSoundSecond;
}
return introEndSecond;
}
return framePositionToSeconds(introStartPosition, pDeck);
}
Expand Down Expand Up @@ -1127,12 +1144,30 @@ double AutoDJProcessor::getOutroStartSecond(DeckAttributes* pDeck) {

double AutoDJProcessor::getOutroEndSecond(DeckAttributes* pDeck) {
const mixxx::audio::FramePos trackEndPosition = pDeck->trackEndPosition();
const mixxx::audio::FramePos outroStartPosition = pDeck->outroStartPosition();
const mixxx::audio::FramePos outroEndPosition = pDeck->outroEndPosition();
if (!outroEndPosition.isValid() || outroEndPosition > trackEndPosition) {
return getLastSoundSecond(pDeck);
double lastSoundSecond = getLastSoundSecond(pDeck);
DEBUG_ASSERT(lastSoundSecond <= framePositionToSeconds(trackEndPosition, pDeck));
if (!outroStartPosition.isValid() || outroStartPosition > trackEndPosition) {
// No outro start and outro end set, use Last Sound.
return lastSoundSecond;
Comment on lines +1146 to +1147
Copy link
Member

Choose a reason for hiding this comment

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

wasn't this dependent on transition mode?

Copy link
Member Author

Choose a reason for hiding this comment

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

Here we calculate a reliable OutroEnd position, as an input to the transition mode dependent code.
The change here takes place in case we have an OutroStart. Than we try to use the transition time to calulate a better OutroEnd than just the lastSoundSecond as the last resort.

}
// Try to find a better Outro End using Outro Start and transition time
double outroStartSecond = framePositionToSeconds(outroStartPosition, pDeck);
if (m_transitionTime >= 0 && lastSoundSecond > outroStartSecond) {
double outroEndFromTime = outroStartSecond + m_transitionTime;
if (outroEndFromTime < lastSoundSecond) {
Comment on lines +1152 to +1153
Copy link
Member

Choose a reason for hiding this comment

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

you'll also need to check for outroEndFromTime < trackEndPosition IIUC.

Copy link
Member Author

Choose a reason for hiding this comment

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

lastSoundSecond has been clamped to be <= trackEndPosition
I will add a note.

// The outroEnd is automatically placed by AnalyzerSilence at the last sound
// Here the user has removed it, but has placed a outro start.
// Use the transition time instead the dismissed last sound position.
daschuer marked this conversation as resolved.
Show resolved Hide resolved
return outroEndFromTime;
}
return lastSoundSecond;
}
return outroStartSecond;
}
return framePositionToSeconds(outroEndPosition, pDeck);
;
}

double AutoDJProcessor::getFirstSoundSecond(DeckAttributes* pDeck) {
Expand Down