diff --git a/src/engine/controls/keycontrol.cpp b/src/engine/controls/keycontrol.cpp index f73cab378e50..b25358696322 100644 --- a/src/engine/controls/keycontrol.cpp +++ b/src/engine/controls/keycontrol.cpp @@ -7,6 +7,7 @@ #include "control/controlpotmeter.h" #include "control/controlproxy.h" #include "control/controlpushbutton.h" +#include "engine/defs_keylock.h" #include "engine/enginebuffer.h" #include "mixer/playermanager.h" #include "moc_keycontrol.cpp" @@ -14,9 +15,6 @@ constexpr bool kEnableDebugOutput = false; -static const double kLockCurrentKey = 1; -static const double kKeepUnlockedKey = 1; - KeyControl::KeyControl(const QString& group, UserSettingsPointer pConfig) : EngineControl(group, pConfig), @@ -147,9 +145,9 @@ void KeyControl::slotRateChanged() { updateRate(); } -// This is called when rate_ratio, vinylcontrol_rate, vinylcontrol_enabled or -// keylock are changed, but also when EngineBuffer::processTrackLocked requests -// m_pitchRateInfo struct while rate, pitch or pitch_adjust were just updated. +/// This is called when rate_ratio, vinylcontrol_rate, vinylcontrol_enabled or +/// keylock are changed, but also when EngineBuffer::processTrackLocked requests +/// m_pitchRateInfo struct while rate, pitch or pitch_adjust were just updated. void KeyControl::updateRate() { if (m_pVCEnabled && m_pVCRate && m_pVCEnabled->toBool()) { m_pitchRateInfo.tempoRatio = m_pVCRate->get(); @@ -191,7 +189,9 @@ void KeyControl::updateRate() { if (m_pKeylock->toBool()) { if (!m_pitchRateInfo.keylock) { // Enabling keylock - if (m_keylockMode->get() == kLockCurrentKey) { // Lock at current pitch + if (m_keylockMode->get() == + static_cast(KeylockMode::LockCurrentKey)) { + // Lock at current pitch speedSliderPitchRatio = m_pitchRateInfo.tempoRatio; if constexpr (kEnableDebugOutput) { qDebug() << " LOCKING current key"; @@ -223,7 +223,7 @@ void KeyControl::updateRate() { } } else { // !m_pKeylock if (m_pitchRateInfo.keylock) { // Disabling Keylock - if (m_keyunlockMode->get() == kKeepUnlockedKey) { + if (m_keyunlockMode->get() == static_cast(KeyunlockMode::KeepLockedKey)) { // adopt speedSliderPitchRatio change as pitchTweakRatio m_pitchRateInfo.pitchTweakRatio *= (speedSliderPitchRatio / m_pitchRateInfo.tempoRatio); @@ -299,6 +299,8 @@ void KeyControl::updateRate() { updateKeyCOs(dFileKey, pitchOctaves); } +/// This is called when the file key is changed (by analysis or user input), +/// or when a track with a different key is loaded void KeyControl::slotFileKeyChanged(double value) { updateKeyCOs(value, m_pPitch->get() / 12); } @@ -352,6 +354,9 @@ void KeyControl::setEngineKey(double key, double key_distance) { return; } +/// This is called when pitch is changed either by user interaction, +/// or when BaseTrackPlayerImpl resets the pitch on track load per configuration +/// AND key is locked with KeylockMode::LockCurrentKey void KeyControl::slotPitchChanged(double pitch) { Q_UNUSED(pitch) m_updatePitchRequest = 1; @@ -391,6 +396,9 @@ void KeyControl::updatePitch() { } } +/// This is called when pitch_adjust is changed either by user interaction, +/// or when BaseTrackPlayerImpl resets the pitch on track load per configuration +/// AND key is unlocked or locked with LockOriginalKey void KeyControl::slotPitchAdjustChanged(double pitchAdjust) { Q_UNUSED(pitchAdjust); m_updatePitchAdjustRequest = 1; diff --git a/src/engine/defs_keylock.h b/src/engine/defs_keylock.h new file mode 100644 index 000000000000..0730a7957edf --- /dev/null +++ b/src/engine/defs_keylock.h @@ -0,0 +1,11 @@ +#pragma once + +enum class KeylockMode { + LockOriginalKey, + LockCurrentKey +}; + +enum class KeyunlockMode { + ResetLockedKey, + KeepLockedKey +}; diff --git a/src/mixer/basetrackplayer.cpp b/src/mixer/basetrackplayer.cpp index 142cecc50f69..69df7f01af09 100644 --- a/src/mixer/basetrackplayer.cpp +++ b/src/mixer/basetrackplayer.cpp @@ -8,6 +8,7 @@ #include "control/controlobject.h" #include "engine/channels/enginedeck.h" #include "engine/controls/enginecontrol.h" +#include "engine/defs_keylock.h" #include "engine/engine.h" #include "engine/enginebuffer.h" #include "engine/enginemixer.h" @@ -277,7 +278,10 @@ BaseTrackPlayerImpl::BaseTrackPlayerImpl( m_pPlay->connectValueChanged(this, &BaseTrackPlayerImpl::slotPlayToggled); m_pRateRatio = make_parented(getGroup(), "rate_ratio", this); + m_pPitch = make_parented(getGroup(), "pitch", this); m_pPitchAdjust = make_parented(getGroup(), "pitch_adjust", this); + m_pKeylock = make_parented(getGroup(), "keylock", this); + m_pKeylockMode = make_parented(getGroup(), "keylockMode", this); m_pUpdateReplayGainFromPregain = std::make_unique( ConfigKey(getGroup(), "update_replaygain_from_pregain")); @@ -705,7 +709,16 @@ void BaseTrackPlayerImpl::slotTrackLoaded(TrackPointer pNewTrack, } } if (reset == RESET_PITCH || reset == RESET_PITCH_AND_SPEED) { - m_pPitchAdjust->set(0.0); + // With KeylockMode::LockCurrentKey we need to reset `pitch` + // instead of `pitch_adjust` to avoid a roundtrip in KeyControl + // which would lead `pitch` != 0 + if (m_pKeylock->toBool() && + m_pKeylockMode->get() == + static_cast(KeylockMode::LockCurrentKey)) { + m_pPitch->set(0.0); + } else { + m_pPitchAdjust->set(0.0); + } } } else { // perform a clone of the given channel diff --git a/src/mixer/basetrackplayer.h b/src/mixer/basetrackplayer.h index afc6ed80c106..c711e19f4bcf 100644 --- a/src/mixer/basetrackplayer.h +++ b/src/mixer/basetrackplayer.h @@ -238,5 +238,8 @@ class BaseTrackPlayerImpl : public BaseTrackPlayer { parented_ptr m_pHighFilterKill; parented_ptr m_pPreGain; parented_ptr m_pRateRatio; + parented_ptr m_pPitch; parented_ptr m_pPitchAdjust; + parented_ptr m_pKeylock; + parented_ptr m_pKeylockMode; }; diff --git a/src/preferences/dialog/dlgprefdeck.h b/src/preferences/dialog/dlgprefdeck.h index df64bd7af98c..07ca239e1725 100644 --- a/src/preferences/dialog/dlgprefdeck.h +++ b/src/preferences/dialog/dlgprefdeck.h @@ -4,6 +4,7 @@ #include "engine/controls/cuecontrol.h" #include "engine/controls/ratecontrol.h" +#include "engine/defs_keylock.h" #include "preferences/dialog/dlgpreferencepage.h" #include "preferences/dialog/ui_dlgprefdeckdlg.h" #include "preferences/usersettings.h" @@ -34,16 +35,6 @@ namespace TrackTime { }; } -enum class KeylockMode { - LockOriginalKey, - LockCurrentKey -}; - -enum class KeyunlockMode { - ResetLockedKey, - KeepLockedKey -}; - enum class LoadWhenDeckPlaying { Reject, Allow, diff --git a/src/test/enginebuffertest.cpp b/src/test/enginebuffertest.cpp index 0b895e8384b0..8e7a3d1df71a 100644 --- a/src/test/enginebuffertest.cpp +++ b/src/test/enginebuffertest.cpp @@ -9,6 +9,7 @@ #include "control/controlobject.h" #include "engine/controls/ratecontrol.h" +#include "engine/defs_keylock.h" #include "mixer/basetrackplayer.h" #include "preferences/usersettings.h" #include "test/mixxxtest.h" @@ -31,9 +32,9 @@ TEST_F(EngineBufferTest, DisableKeylockResetsPitch) { // To prevent one-slider users from getting stuck on a key, // KeyunlockMode::ResetLockedKey resets the musical pitch. ControlObject::set(ConfigKey(m_sGroup1, "keylockMode"), - 1.0); // KeylockMode::LockCurrentKey + static_cast(KeylockMode::LockCurrentKey)); ControlObject::set(ConfigKey(m_sGroup1, "keyunlockMode"), - 0.0); // KeyunlockMode::ResetLockedKey + static_cast(KeyunlockMode::ResetLockedKey)); ControlObject::set(ConfigKey(m_sGroup1, "file_bpm"), 128.0); ControlObject::set(ConfigKey(m_sGroup1, "keylock"), 1.0); ControlObject::set(ConfigKey(m_sGroup1, "pitch"), 0.5); @@ -48,9 +49,9 @@ TEST_F(EngineBufferTest, DisableKeylockResetsPitch) { TEST_F(EngineBufferTest, DisableKeylockKeepsPitch) { // Pitch must not change when unlocking with KeyunlockMode::KeepLockedKey. ControlObject::set(ConfigKey(m_sGroup1, "keylockMode"), - 1.0); // KeylockMode::LockCurrentKey + static_cast(KeylockMode::LockCurrentKey)); ControlObject::set(ConfigKey(m_sGroup1, "keyunlockMode"), - 1.0); // KeyunlockMode::KeepLockedKey + static_cast(KeyunlockMode::KeepLockedKey)); ControlObject::set(ConfigKey(m_sGroup1, "file_bpm"), 128.0); ControlObject::set(ConfigKey(m_sGroup1, "keylock"), 1.0); ControlObject::set(ConfigKey(m_sGroup1, "pitch"), 0.5); @@ -75,12 +76,42 @@ TEST_F(EngineBufferTest, TrackLoadResetsPitch) { ASSERT_NEAR(0.0, ControlObject::get(ConfigKey(m_sGroup1, "pitch_adjust")), 1e-10); } +TEST_F(EngineBufferTest, TrackLoadResetsPitch_LockCurrentKey) { + // The pitch should be reset to 0 when a new track was loaded when + // * rate is not 0 + // * keylock is ON + // * keylock mode is LockCurrentKey, + // * Reset Pitch on track load option is enabled + // + // First test case: + // * change tempo with key unlocked -> pitch changes + // * lock key + // // * reset pitch -> is now 0 OPTIONAL + // * load another track -> pitch should (still) be 0 + config()->set(ConfigKey("[Controls]", "SpeedAutoReset"), + ConfigValue(BaseTrackPlayer::RESET_PITCH)); + ControlObject::set(ConfigKey(m_sGroup1, "keylockMode"), + static_cast(KeylockMode::LockCurrentKey)); + ControlObject::set(ConfigKey(m_sGroup1, "rate"), 0.5); + ControlObject::set(ConfigKey(m_sGroup1, "keylock"), 1.0); + ControlObject::set(ConfigKey(m_sGroup1, "reset_key"), 1.0); + ProcessBuffer(); + // Note that pitch_adjust is NOT reset to 0 with KeylockMode::LockCurrentKey + ASSERT_DOUBLE_EQ(0.0, ControlObject::get(ConfigKey(m_sGroup1, "pitch"))); + ProcessBuffer(); + + m_pMixerDeck1->loadFakeTrack(false, 0.0); + ProcessBuffer(); + + ASSERT_DOUBLE_EQ(0.0, ControlObject::get(ConfigKey(m_sGroup1, "pitch"))); +} + TEST_F(EngineBufferTest, PitchRoundtrip) { ControlObject::set(ConfigKey(m_sGroup1, "keylock"), 0.0); ControlObject::set(ConfigKey(m_sGroup1, "keylockMode"), - 0.0); // KeylockMode::LockOriginalKey + static_cast(KeylockMode::LockOriginalKey)); ControlObject::set(ConfigKey(m_sGroup1, "keyunlockMode"), - 0.0); // KeyunlockMode::ResetLockedKey + static_cast(KeyunlockMode::ResetLockedKey)); ProcessBuffer(); // we are in kPakmOffsetScaleReseting mode ControlObject::set(ConfigKey(m_sGroup1, "rate"), 0.5); @@ -103,7 +134,7 @@ TEST_F(EngineBufferTest, PitchRoundtrip) { ASSERT_DOUBLE_EQ(0.0, ControlObject::get(ConfigKey(m_sGroup1, "pitch_adjust"))); ControlObject::set(ConfigKey(m_sGroup1, "keylockMode"), - 1.0); // KeylockMode::LockCurrentKey + static_cast(KeylockMode::LockCurrentKey)); ProcessBuffer(); // rate must not change ASSERT_DOUBLE_EQ(0.5, ControlObject::get(ConfigKey(m_sGroup1, "rate"))); @@ -111,7 +142,7 @@ TEST_F(EngineBufferTest, PitchRoundtrip) { ASSERT_DOUBLE_EQ(0.0, ControlObject::get(ConfigKey(m_sGroup1, "pitch"))); ControlObject::set(ConfigKey(m_sGroup1, "keylockMode"), - 0.0); // KeylockMode::LockOriginalKey + static_cast(KeylockMode::LockOriginalKey)); ProcessBuffer(); // rate must not change ASSERT_DOUBLE_EQ(0.5, ControlObject::get(ConfigKey(m_sGroup1, "rate"))); @@ -370,7 +401,7 @@ TEST_F(EngineBufferE2ETest, DISABLED_KeylockReverseTest) { ControlObject::set(ConfigKey(kAppGroup, QStringLiteral("keylock_engine")), static_cast(EngineBuffer::KeylockEngine::SoundTouch)); ControlObject::set(ConfigKey(m_sGroup1, "keylockMode"), - 0.0); + static_cast(KeylockMode::LockOriginalKey)); ControlObject::set(ConfigKey(m_sGroup1, "rate"), 0.5); ControlObject::set(ConfigKey(m_sGroup1, "play"), 1.0); ControlObject::set(ConfigKey(m_sGroup1, "keylock"), 1.0);