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

Expose [BPM],sync_lock_algorithm option in deck preferences. #11828

Merged
merged 10 commits into from
Nov 16, 2023
Merged

Expose [BPM],sync_lock_algorithm option in deck preferences. #11828

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a320ada
Expose [BPM],sync_lock_algorithm option in deck preferences.
daschuer Aug 12, 2023
92bdfa1
Use Term Soft Leader to match other parts in the code
daschuer Aug 14, 2023
636434b
Extend tooltips
daschuer Aug 15, 2023
5ee1c71
Improve tooltips
daschuer Aug 21, 2023
edc5d01
Add manual link to Sync mode label in prefrences
daschuer Sep 9, 2023
4b6e4be
Extend the sync_leader tooltip
daschuer Sep 9, 2023
783c041
Use constants instead of macros
daschuer Oct 10, 2023
51af7c4
Improve tooltips
daschuer Oct 16, 2023
72d5d84
Adjust link to the manual
daschuer Nov 16, 2023
bd77c26
Merge remote-tracking branch 'upstream/2.4' into sync_lock_algorithm
daschuer Nov 16, 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
6 changes: 3 additions & 3 deletions src/engine/sync/enginesync.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,10 +254,10 @@ Syncable* EngineSync::pickLeader(Syncable* enabling_syncable) {
}

const SyncLockAlgorithm picker = static_cast<SyncLockAlgorithm>(
m_pConfig->getValue<int>(ConfigKey("[BPM]", "sync_lock_algorithm"),
PREFER_IMPLICIT_LEADER));
m_pConfig->getValue<int>(ConfigKey(BPM_CONFIG_GROUP, SYNC_LOCK_ALGORITHM_CONFIG_KEY),
PREFER_SOFT_LEADER));
switch (picker) {
case PREFER_IMPLICIT_LEADER:
case PREFER_SOFT_LEADER:
// Always pick a deck for a new leader.
if (playing_deck_count == 1) {
return first_playing_deck;
Expand Down
24 changes: 13 additions & 11 deletions src/engine/sync/enginesync.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,25 @@
class InternalClock;
class EngineChannel;

#define BPM_CONFIG_GROUP "[BPM]"
#define SYNC_LOCK_ALGORITHM_CONFIG_KEY "sync_lock_algorithm"
Swiftb0y marked this conversation as resolved.
Show resolved Hide resolved

/// EngineSync is the heart of the Mixxx Sync Lock engine. It knows which objects
/// (Decks, Internal Clock, etc) are participating in Sync and what their statuses
/// are. It also orchestrates sync handoffs between different decks as they play,
/// stop, or request their status to change.
class EngineSync : public SyncableListener {
public:
// Remove pick SyncLockAlgorithm when new beatgrid detection
// and editing code is committed and we no longer need the lock bpm fallback option.
enum SyncLockAlgorithm {
// New behavior, which should work if beatgrids are reliable.
PREFER_SOFT_LEADER,
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if I'm missing something, but isn't it irrelevant if the leader is implicit/soft or explicit?

Suggested change
PREFER_SOFT_LEADER,
PREFER_LEADER,

Copy link
Member Author

@daschuer daschuer Aug 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The explicit leader (full crown symbol) is always preferred. This feature is about the automatically selected soft leader.
"Shall the follower follow a dynamic tempo or should both follow a steady tempo?"

Copy link
Member Author

Choose a reason for hiding this comment

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

Currently this preferences option is designed to be extensible for more features that will come on the table once we have rhythm detection implemented.
Does it make sense to decouple it?

// Old 2.3 behavior, which works around some issues with bad beatgrid detection, mostly
// for auto DJ mode.
PREFER_LOCK_BPM
};

explicit EngineSync(UserSettingsPointer pConfig);
~EngineSync() override;

Expand Down Expand Up @@ -140,17 +153,6 @@ class EngineSync : public SyncableListener {
return false;
}

// TODO: Remove pick algorithms during 2.4 development phase when new beatgrid detection
// and editing code is committed and we no longer need the lock bpm fallback option.
// If this code makes it to release we will all be very sad.
enum SyncLockAlgorithm {
// New behavior, which should work if beatgrids are reliable.
PREFER_IMPLICIT_LEADER,
// Old 2.3 behavior, which works around some issues with bad beatgrid detection, mostly
// for auto DJ mode.
PREFER_LOCK_BPM
};

FRIEND_TEST(EngineSyncTest, EnableOneDeckInitsLeader);
FRIEND_TEST(EngineSyncTest, EnableOneDeckInitializesLeader);
FRIEND_TEST(EngineSyncTest, SyncToNonSyncDeck);
Expand Down
21 changes: 21 additions & 0 deletions src/preferences/dialog/dlgprefdeck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "defs_urls.h"
#include "engine/controls/ratecontrol.h"
#include "engine/enginebuffer.h"
#include "engine/sync/enginesync.h"
#include "mixer/basetrackplayer.h"
#include "mixer/playerinfo.h"
#include "mixer/playermanager.h"
Expand Down Expand Up @@ -465,6 +466,16 @@ void DlgPrefDeck::slotUpdate() {
index = ComboBoxCueMode->findData(static_cast<int>(cueMode));
ComboBoxCueMode->setCurrentIndex(index);

const EngineSync::SyncLockAlgorithm syncLockAlgorithm =
static_cast<EngineSync::SyncLockAlgorithm>(m_pConfig->getValue<int>(
ConfigKey(BPM_CONFIG_GROUP, SYNC_LOCK_ALGORITHM_CONFIG_KEY),
EngineSync::SyncLockAlgorithm::PREFER_SOFT_LEADER));
Copy link
Member

Choose a reason for hiding this comment

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

Seeing this snippet while starting the review a couple weeks ago was the entire reason for why I implemented #11883 😅
Please update this and the snippet below when merging to main.

if (syncLockAlgorithm == EngineSync::SyncLockAlgorithm::PREFER_SOFT_LEADER) {
radioButtonSoftLeader->setChecked(true);
} else {
radioButtonLockBpm->setChecked(true);
}

KeylockMode keylockMode =
static_cast<KeylockMode>(static_cast<int>(m_keylockModeControls[0]->get()));
if (keylockMode == KeylockMode::LockCurrentKey) {
Expand Down Expand Up @@ -550,6 +561,8 @@ void DlgPrefDeck::slotResetToDefaults() {
checkBoxResetSpeed->setChecked(false);
checkBoxResetPitch->setChecked(true);

radioButtonSoftLeader->setChecked(true);

radioButtonOriginalKey->setChecked(true);
radioButtonResetUnlockedKey->setChecked(true);
}
Expand Down Expand Up @@ -729,6 +742,14 @@ void DlgPrefDeck::slotApply() {
m_pConfig->set(ConfigKey("[Controls]", "SpeedAutoReset"),
ConfigValue(configSPAutoReset));

if (radioButtonSoftLeader->isChecked()) {
m_pConfig->setValue(ConfigKey(BPM_CONFIG_GROUP, SYNC_LOCK_ALGORITHM_CONFIG_KEY),
static_cast<int>(EngineSync::SyncLockAlgorithm::PREFER_SOFT_LEADER));
} else {
m_pConfig->setValue(ConfigKey(BPM_CONFIG_GROUP, SYNC_LOCK_ALGORITHM_CONFIG_KEY),
static_cast<int>(EngineSync::SyncLockAlgorithm::PREFER_LOCK_BPM));
}

m_pConfig->setValue(ConfigKey("[Controls]", "keylockMode"),
static_cast<int>(m_keylockMode));
// Set key lock behavior for every group
Expand Down
81 changes: 58 additions & 23 deletions src/preferences/dialog/dlgprefdeckdlg.ui
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
<layout class="QGridLayout" name="gridLayout_7">
<item row="0" column="0">
<layout class="QGridLayout" name="GridLayoutSpeedOptions">
<item row="9" column="2">
<item row="11" column="2">
<widget class="QDoubleSpinBox" name="spinBoxPermanentRateCoarse">
<property name="toolTip">
<string>Permanent rate change when left-clicking</string>
Expand Down Expand Up @@ -231,7 +231,7 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
</property>
</widget>
</item>
<item row="9" column="1">
<item row="11" column="1">
<widget class="QDoubleSpinBox" name="spinBoxPermanentRateFine">
<property name="toolTip">
<string>Permanent rate change when right-clicking</string>
Expand Down Expand Up @@ -269,7 +269,7 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
</property>
</widget>
</item>
<item row="2" column="2">
<item row="3" column="2">
<widget class="QRadioButton" name="radioButtonCurrentKey">
<property name="text">
<string>Current key</string>
Expand All @@ -279,7 +279,7 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
</attribute>
</widget>
</item>
<item row="8" column="1">
<item row="10" column="1">
<widget class="QDoubleSpinBox" name="spinBoxTemporaryRateFine">
<property name="enabled">
<bool>false</bool>
Expand Down Expand Up @@ -310,7 +310,7 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
</property>
</widget>
</item>
<item row="9" column="0">
<item row="11" column="0">
<widget class="QLabel" name="labelSpeedPermanent">
<property name="text">
<string>Permanent</string>
Expand All @@ -320,7 +320,7 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
</property>
</widget>
</item>
<item row="6" column="1" colspan="2">
<item row="7" column="1" colspan="2">
<layout class="QHBoxLayout" name="horizontalLayout_2">
<item>
<widget class="QSlider" name="SliderRateRampSensitivity">
Expand Down Expand Up @@ -368,7 +368,7 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
</item>
</layout>
</item>
<item row="8" column="0">
<item row="10" column="0">
<widget class="QLabel" name="labelSpeedTemporary">
<property name="enabled">
<bool>false</bool>
Expand All @@ -382,6 +382,13 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
</widget>
</item>
<item row="2" column="0">
<widget class="QLabel" name="label">
<property name="text">
<string>Sync mode (Dynamic tempo tracks)</string>
</property>
</widget>
</item>
<item row="3" column="0">
<widget class="QLabel" name="labelKeylockMode">
<property name="text">
<string>Keylock mode</string>
Expand All @@ -391,7 +398,7 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
</property>
</widget>
</item>
<item row="6" column="0">
<item row="7" column="0">
<widget class="QLabel" name="labelSpeedRampSensitivity">
<property name="enabled">
<bool>false</bool>
Expand All @@ -404,7 +411,7 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
</property>
</widget>
</item>
<item row="5" column="0">
<item row="6" column="0">
<widget class="QLabel" name="labelSpeedBendBehavior">
<property name="text">
<string>Pitch bend behavior</string>
Expand All @@ -414,7 +421,7 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
</property>
</widget>
</item>
<item row="2" column="1">
<item row="3" column="1">
<widget class="QRadioButton" name="radioButtonOriginalKey">
<property name="text">
<string>Original key</string>
Expand All @@ -424,7 +431,7 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
</attribute>
</widget>
</item>
<item row="8" column="2">
<item row="10" column="2">
<widget class="QDoubleSpinBox" name="spinBoxTemporaryRateCoarse">
<property name="enabled">
<bool>false</bool>
Expand Down Expand Up @@ -469,14 +476,27 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
</property>
</widget>
</item>
<item row="7" column="0">
<item row="9" column="0">
<widget class="QLabel" name="labelSpeedAdjustment">
<property name="text">
<string>Adjustment buttons:</string>
</property>
</widget>
</item>
<item row="7" column="2">
<item row="2" column="1">
<widget class="QRadioButton" name="radioButtonSoftLeader">
<property name="toolTip">
<string>Apply tempo changes from a soft-leading track (usually the leaving track in a transition) to the follower tracks. After the transition, the follower track will continue with the previous leader's very last tempo. Changes from explicit selected leaders are always applied.</string>
</property>
<property name="text">
<string>Follow soft leader's tempo</string>
</property>
<attribute name="buttonGroup">
<string notr="true">buttonGroupSyncMode</string>
</attribute>
</widget>
</item>
<item row="9" column="2">
<widget class="QLabel" name="labelSpeedCoarse">
<property name="enabled">
<bool>true</bool>
Expand All @@ -501,7 +521,7 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
</property>
</widget>
</item>
<item row="7" column="1">
<item row="9" column="1">
<widget class="QLabel" name="labelSpeedFine">
<property name="enabled">
<bool>true</bool>
Expand Down Expand Up @@ -565,7 +585,7 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
</property>
</widget>
</item>
<item row="5" column="2">
<item row="6" column="2">
<widget class="QRadioButton" name="radioButtonRateRampModeStepping">
<property name="text">
<string>Abrupt jump</string>
Expand All @@ -575,7 +595,7 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
</attribute>
</widget>
</item>
<item row="5" column="1">
<item row="6" column="1">
<widget class="QRadioButton" name="radioButtonRateRampModeLinear">
<property name="toolTip">
<string>Smoothly adjusts deck speed when temporary change buttons are held down</string>
Expand All @@ -588,14 +608,14 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
</attribute>
</widget>
</item>
<item row="3" column="0">
<item row="4" column="0">
<widget class="QLabel" name="labelKeyunlockMode">
<property name="text">
<string>Keyunlock mode</string>
</property>
</widget>
</item>
<item row="3" column="1">
<item row="4" column="1">
<widget class="QRadioButton" name="radioButtonResetUnlockedKey">
<property name="text">
<string>Reset key</string>
Expand All @@ -605,7 +625,7 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
</attribute>
</widget>
</item>
<item row="3" column="2">
<item row="4" column="2">
<widget class="QRadioButton" name="radioButtonKeepUnlockedKey">
<property name="text">
<string>Keep key</string>
Expand All @@ -615,7 +635,20 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
</attribute>
</widget>
</item>
<item row="4" column="0">
<item row="2" column="2">
<widget class="QRadioButton" name="radioButtonLockBpm">
<property name="toolTip">
<string>The tempo of a previous soft leader track at the beginning of the transition is kept steady. After the transition, the follower track will maintain this original tempo. This technique serves as a workaround to avoid dynamic tempo changes, as seen during the outro of rubato-style tracks. For instance, it prevents the follower track from continuing with a slowed-down tempo of the soft leader. This corresponds to the behavior before Mixxx 2.4. Changes from explicit selected leaders are always applied.</string>
</property>
<property name="text">
<string>Use steady tempo</string>
</property>
<attribute name="buttonGroup">
<string notr="true">buttonGroupSyncMode</string>
</attribute>
</widget>
</item>
<item row="5" column="0">
<spacer name="verticalSpacer1">
<property name="orientation">
<enum>Qt::Vertical</enum>
Expand Down Expand Up @@ -663,6 +696,7 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
<tabstop>checkBoxInvertSpeedSlider</tabstop>
<tabstop>checkBoxResetPitch</tabstop>
<tabstop>checkBoxResetSpeed</tabstop>
<tabstop>radioButtonSoftLeader</tabstop>
<tabstop>radioButtonOriginalKey</tabstop>
<tabstop>radioButtonCurrentKey</tabstop>
<tabstop>radioButtonResetUnlockedKey</tabstop>
Expand All @@ -671,16 +705,17 @@ You can always drag-and-drop tracks on screen to clone a deck.</string>
<tabstop>radioButtonRateRampModeStepping</tabstop>
<tabstop>SliderRateRampSensitivity</tabstop>
<tabstop>SpinBoxRateRampSensitivity</tabstop>
<tabstop>spinBoxPermanentRateCoarse</tabstop>
<tabstop>spinBoxPermanentRateFine</tabstop>
<tabstop>spinBoxTemporaryRateCoarse</tabstop>
<tabstop>spinBoxTemporaryRateFine</tabstop>
<tabstop>spinBoxTemporaryRateCoarse</tabstop>
<tabstop>spinBoxPermanentRateFine</tabstop>
<tabstop>spinBoxPermanentRateCoarse</tabstop>
</tabstops>
<resources/>
<buttongroups>
<buttongroup name="buttonGroupTrackTime"/>
<buttongroup name="buttonGroupKeyUnlockMode"/>
<buttongroup name="buttonGroupKeyLockMode"/>
<buttongroup name="buttonGroupSpeedBendBehavior"/>
<buttongroup name="buttonGroupSyncMode"/>
</buttongroups>
</ui>