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

Jump to bottom

Auto dj cross fader center #13628

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+25 −10
Open

Auto dj cross fader center #13628

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
abcd88e
AutoDj Crossfader to the center
davidlmorris Sep 1, 2024
e0c9c71
Update autodjprocessor.cpp
davidlmorris Sep 4, 2024
eee0af2
fix pre-commit
Swiftb0y Sep 6, 2024
c98107f
Update autodjprocessor.cpp
davidlmorris Sep 14, 2024
e52293c
New transition mode
davidlmorris Sep 29, 2024
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
40 changes: 30 additions & 10 deletions src/library/autodj/autodjprocessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,17 @@ AutoDJProcessor::AutoDJProcessor(
m_eState(ADJ_DISABLED),
m_transitionProgress(0.0),
m_transitionTime(kTransitionPreferenceDefault) {
m_pAutoDJTableModel = new PlaylistTableModel(this, pTrackCollectionManager,
"mixxx.db.model.autodj");
m_pAutoDJTableModel = new PlaylistTableModel(
this, pTrackCollectionManager, "mixxx.db.model.autodj");
m_pAutoDJTableModel->selectPlaylist(iAutoDJPlaylistId);
m_pAutoDJTableModel->select();

m_pShufflePlaylist = new ControlPushButton(
ConfigKey("[AutoDJ]", "shuffle_playlist"));
connect(m_pShufflePlaylist, &ControlPushButton::valueChanged,
this, &AutoDJProcessor::controlShuffle);
connect(m_pShufflePlaylist,
&ControlPushButton::valueChanged,
this,
&AutoDJProcessor::controlShuffle);

m_pSkipNext = new ControlPushButton(
ConfigKey("[AutoDJ]", "skip_next"));
Expand Down Expand Up @@ -566,6 +568,8 @@ AutoDJProcessor::AutoDJError AutoDJProcessor::toggleAutoDJ(bool enable) {
setCrossfader(1.0);
}
}
m_crossfaderStartCenter = m_pConfig->getValue<bool>(ConfigKey(kConfigKey,
Copy link
Member

Choose a reason for hiding this comment

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

m_crossfaderStartCenter should be also initialized in the constructor.
Currently we need to restart auto DJ to switch the modes. When it becomes a transition mode, it can be already changes on the fly.

Copy link
Author

Choose a reason for hiding this comment

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

As far as I can tell AutoDj is instantiated just once at the program start, so I've made the change even if I think is a tad redundant for completeness. "Currently we need to restart auto DJ to switch the modes.", is true, like a lot of changes made in options.

QStringLiteral("center_xfader_when_starting_xfade")));
emitAutoDJStateChanged(m_eState);
} else { // Disable Auto DJ
m_pEnabledAutoDJ->setAndConfirm(0.0);
Expand Down Expand Up @@ -832,6 +836,7 @@ void AutoDJProcessor::playerPositionChanged(DeckAttributes* pAttributes,
double crossfaderTarget;
if (m_eState == ADJ_LEFT_FADING) {
crossfaderTarget = 1.0;
Copy link
Member

Choose a reason for hiding this comment

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

Please revert this extra line


} else if (m_eState == ADJ_RIGHT_FADING) {
crossfaderTarget = -1.0;
} else {
Expand Down Expand Up @@ -859,17 +864,31 @@ void AutoDJProcessor::playerPositionChanged(DeckAttributes* pAttributes,
double transitionProgress = (thisPlayPosition - thisDeck->fadeBeginPos) /
(thisDeck->fadeEndPos - thisDeck->fadeBeginPos);
double transitionStep = transitionProgress - m_transitionProgress;

if ((transitionProgress == 0.0) &&
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure if the == 0.0 state is always reached, because of rounding issues.i think it is more reliable to move this in an else branch of the condition below.

Copy link
Member

Choose a reason for hiding this comment

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

Confirmed, this condition is never true and can be removed.

Copy link
Author

Choose a reason for hiding this comment

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

After some testing (and an assert), I couldn't find this state being reached either. So I agree it can be removed.

(m_crossfaderStartCenter)) {
setCrossfader(0.0); // So we start at the mid point before we make any fader moves.
}
if (transitionStep > 0.0) {
// We have made progress.
// Backward seeks pause the transitions; forward seeks speed up
// the transitions. If there has been a seek beyond endPos, end
// the transition immediately.
double remainingCrossfader = crossfaderTarget - currentCrossfader;
double adjustment = remainingCrossfader /
(1.0 - m_transitionProgress) * transitionStep;
// we move the crossfader linearly with
// movements in this track's play position.
setCrossfader(currentCrossfader + adjustment);
if ((m_crossfaderStartCenter) &&
Copy link
Member

Choose a reason for hiding this comment

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

Auto DJ should allow to adjust the Crossfader during a transition. So the code must be able to continue fading from any position. In this case I can imagine a emergency condition where the user realizes that he wants fade in and moves the xfader rapidly to the beginning.

Copy link
Author

Choose a reason for hiding this comment

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

This only happens once and the crossfader is now at the new position, so the user can adjust.

(transitionProgress <=
((thisDeck->fadeEndPos -
thisDeck->fadeBeginPos) /
2))) {
// We want the cross fader to remain in the middle.
Copy link
Member

Choose a reason for hiding this comment

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

This will constantly fight with the user. I think it is better to just not touch the crossfadere here. It will feel naturally if we fall back to the default mode when xfader is no longer in the expected center position.

Copy link
Author

Choose a reason for hiding this comment

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

This should now be:

transitionProgress <= std::midpoint(thisDeck->fadeBeginPos, thisDeck->fadeEndPos) {

to make it clearer what is going on.

In my testing, neither constantly fights the user. If the user moves the fader it goes there and completes the transition.

Copy link
Member

Choose a reason for hiding this comment

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

I can confirm that this readjust the user tweaks in some conditions. It happens because you compare here fractions of the full track length with fractions of the transition. I think the best solution is to move this code to Line 823 as

                if (m_crossfaderStartCenter) { 
                    setCrossfader(0.0);
                } else if (thisDeck->fadeBeginPos >= thisDeck->fadeEndPos) {
                    setCrossfader(thisDeck->isLeft() ? 1.0 : -1.0);
                }

Copy link
Author

Choose a reason for hiding this comment

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

Please show the conditions. I can't find any in my testing.

Copy link
Author

Choose a reason for hiding this comment

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

I've had a play around with this. And I think you are correct. It seems you only need to set the position of the crossfader to the middle once. And the best position would be just before the other desk starts playing. So I would propose this at around line 820:

                if (m_crossfaderStartCenter) {
                    setCrossfader(0.0);
                } else if (thisDeck->fadeBeginPos >= thisDeck->fadeEndPos) {
                        setCrossfader(thisDeck->isLeft() ? 1.0 : -1.0);
                }

                if (!otherDeckPlaying) {
                    otherDeck->play();
                }

The reason is that I noticed that occasionally the audio seems to just miss the very start of a track (I noticed it on a cold vocal start). This may have been a glitch in my system, so I cannot attest to this, but it seems the best thing to do in any event.

setCrossfader(0.0);
} else {
double remainingCrossfader = crossfaderTarget - currentCrossfader;
double adjustment = remainingCrossfader /
(1.0 - m_transitionProgress) * transitionStep;
// we move the crossfader linearly with
// movements in this track's play position.
setCrossfader(currentCrossfader + adjustment);
}
}
m_transitionProgress = transitionProgress;
// if we are at 1.0 here, we need an additional callback until the last
Expand Down Expand Up @@ -1343,6 +1362,7 @@ void AutoDJProcessor::calculateTransition(DeckAttributes* pFromDeck,
}

switch (m_transitionMode) {
// case TransitionMode::RadioFullIntroOutro:
Copy link
Member

Choose a reason for hiding this comment

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

What is that?

Copy link
Author

Choose a reason for hiding this comment

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

As I said elsewhere that is a comment form an earlier experiment, that I neglected to delete.

case TransitionMode::FullIntroOutro: {
// Use the outro or intro length for the transition time, whichever is
// shorter. Let the full outro and intro play; do not cut off any part
Expand Down
1 change: 1 addition & 0 deletions src/library/autodj/autodjprocessor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@ class AutoDJProcessor : public QObject {
double m_transitionProgress;
double m_transitionTime; // the desired value set by the user
TransitionMode m_transitionMode;
bool m_crossfaderStartCenter;

QList<DeckAttributes*> m_decks;

Expand Down
6 changes: 6 additions & 0 deletions src/preferences/dialog/dlgprefautodj.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ void DlgPrefAutoDJ::slotUpdate() {
// Re-center the crossfader instantly when AutoDJ is disabled
CenterXfaderCheckBox->setChecked(m_pConfig->getValue(
ConfigKey("[Auto DJ]", "center_xfader_when_disabling"), false));

CrossfaderStartCenterCheckBox->setChecked(m_pConfig->getValue(
ConfigKey("[Auto DJ]", "center_xfader_when_starting_xfade"), false));
}

void DlgPrefAutoDJ::slotApply() {
Expand All @@ -81,6 +84,8 @@ void DlgPrefAutoDJ::slotApply() {

m_pConfig->setValue(ConfigKey("[Auto DJ]", "center_xfader_when_disabling"),
CenterXfaderCheckBox->isChecked());
m_pConfig->setValue(ConfigKey("[Auto DJ]", "center_xfader_when_starting_xfade"),
CrossfaderStartCenterCheckBox->isChecked());
}

void DlgPrefAutoDJ::slotResetToDefaults() {
Expand All @@ -96,6 +101,7 @@ void DlgPrefAutoDJ::slotResetToDefaults() {
RandomQueueMinimumSpinBox->setValue(5);

CenterXfaderCheckBox->setChecked(false);
CrossfaderStartCenterCheckBox->setChecked(false);
}

void DlgPrefAutoDJ::slotToggleRequeueIgnore(int buttonState) {
Expand Down
23 changes: 22 additions & 1 deletion src/preferences/dialog/dlgprefautodjdlg.ui
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,27 @@
<widget class="QCheckBox" name="CenterXfaderCheckBox"/>
</item>

<item row="1" column="0">
<widget class="QLabel" name="CrossfaderStartCenterLabel">
<property name="sizePolicy">
<sizepolicy hsizetype="Preferred" vsizetype="Preferred">
<horstretch>0</horstretch>
<verstretch>0</verstretch>
</sizepolicy>
</property>
<property name="text">
<string>Set the Crossfader to the center as AutoDj starts a crossfade so the track starts at full volume</string>
</property>
<property name="buddy">
<cstring>CrossfaderStartCenterCheckBox</cstring>
</property>
</widget>
</item>

<item row="1" column="1">
<widget class="QCheckBox" name="CrossfaderStartCenterCheckBox"/>
</item>

<item row="0" column="2">
<spacer name="horizontalSpacerxfaderbehaviour">
<property name="orientation">
Expand All @@ -298,7 +319,7 @@
</spacer>
</item>

<item row="1" column="0" colspan="3">
<item row="2" column="0" colspan="3">
<widget class="QLabel" name="CenterXfaderHintText">
<property name="sizePolicy">
<sizepolicy hsizetype="Expanding" vsizetype="Minimum">
Expand Down