diff --git a/src/controllers/controlpickermenu.cpp b/src/controllers/controlpickermenu.cpp index 32f3375b2f39..d2363d11aace 100644 --- a/src/controllers/controlpickermenu.cpp +++ b/src/controllers/controlpickermenu.cpp @@ -203,7 +203,13 @@ ControlPickerMenu::ControlPickerMenu(QWidget* pParent) transportMenu); addDeckAndSamplerAndPreviewDeckControl("end", tr("Jump To End"), tr("Jump to end of track"), transportMenu); transportMenu->addSeparator(); - addDeckAndSamplerAndPreviewDeckControl("eject", tr("Eject"), tr("Eject track"), transportMenu); + addDeckAndSamplerAndPreviewDeckControl("eject", + tr("Eject"), + tr("Eject or un-eject track, i.e. reload the last-ejected track " + "(of any deck)
" + "Double-press to reload the last replaced track. In empty decks " + "it reloads the second-last ejected track."), + transportMenu); addDeckAndSamplerControl("repeat", tr("Repeat Mode"), tr("Toggle repeat mode"), transportMenu); addDeckAndSamplerControl("slip_enabled", tr("Slip Mode"), tr("Toggle slip mode"), transportMenu); diff --git a/src/controllers/dlgcontrollerlearning.cpp b/src/controllers/dlgcontrollerlearning.cpp index 19afa3cb3ad8..2f7bda75c110 100644 --- a/src/controllers/dlgcontrollerlearning.cpp +++ b/src/controllers/dlgcontrollerlearning.cpp @@ -25,6 +25,7 @@ DlgControllerLearning::DlgControllerLearning(QWidget* parent, qRegisterMetaType("MidiInputMappings"); setupUi(this); + labelDescription->setWordWrap(true); labelMappedTo->setText(""); QString helpTitle(tr("Click anywhere in Mixxx or choose a control to learn")); diff --git a/src/mixer/basetrackplayer.cpp b/src/mixer/basetrackplayer.cpp index 509040fd37a6..bc8c2309daa2 100644 --- a/src/mixer/basetrackplayer.cpp +++ b/src/mixer/basetrackplayer.cpp @@ -207,6 +207,8 @@ BaseTrackPlayerImpl::BaseTrackPlayerImpl( ConfigKey(getGroup(), "update_replaygain_from_pregain")); m_pUpdateReplayGainFromPregain->connectValueChangeRequest(this, &BaseTrackPlayerImpl::slotUpdateReplayGainFromPregain); + + m_ejectTimer.start(); } BaseTrackPlayerImpl::~BaseTrackPlayerImpl() { @@ -309,6 +311,20 @@ void BaseTrackPlayerImpl::slotEjectTrack(double v) { if (v <= 0) { return; } + + mixxx::Duration elapsed = m_ejectTimer.restart(); + + // Double-click always restores the last replaced track, i.e. un-eject the second + // last track: the first click ejects or unejects, and the second click reloads. + if (elapsed < mixxx::Duration::fromMillis(kUnreplaceDelay)) { + TrackPointer lastEjected = m_pPlayerManager->getSecondLastEjectedTrack(); + if (lastEjected) { + slotLoadTrack(lastEjected, false); + } + return; + } + + // With no loaded track a single click reloads the last ejected track. if (!m_pLoadedTrack) { TrackPointer lastEjected = m_pPlayerManager->getLastEjectedTrack(); if (lastEjected) { diff --git a/src/mixer/basetrackplayer.h b/src/mixer/basetrackplayer.h index 5691b3065108..95a03d8bb3ad 100644 --- a/src/mixer/basetrackplayer.h +++ b/src/mixer/basetrackplayer.h @@ -21,6 +21,8 @@ class ControlPotmeter; class ControlProxy; class EffectsManager; +constexpr int kUnreplaceDelay = 500; + // Interface for not leaking implementation details of BaseTrackPlayer into the // rest of Mixxx. Also makes testing a lot easier. class BaseTrackPlayer : public BasePlayer { @@ -123,6 +125,8 @@ class BaseTrackPlayerImpl : public BaseTrackPlayer { bool m_replaygainPending; EngineChannel* m_pChannelToCloneFrom; + PerformanceTimer m_ejectTimer; + std::unique_ptr m_pEject; // Deck clone control diff --git a/src/mixer/playermanager.cpp b/src/mixer/playermanager.cpp index b0eeabab2354..49971001499c 100644 --- a/src/mixer/playermanager.cpp +++ b/src/mixer/playermanager.cpp @@ -602,10 +602,17 @@ Sampler* PlayerManager::getSampler(unsigned int sampler) const { } TrackPointer PlayerManager::getLastEjectedTrack() const { - if (m_pLibrary) { - return m_pLibrary->trackCollectionManager()->getTrackById(m_lastEjectedTrackId); + VERIFY_OR_DEBUG_ASSERT(m_pLibrary != nullptr) { + return nullptr; } - return nullptr; + return m_pLibrary->trackCollectionManager()->getTrackById(m_lastEjectedTrackId); +} + +TrackPointer PlayerManager::getSecondLastEjectedTrack() const { + VERIFY_OR_DEBUG_ASSERT(m_pLibrary != nullptr) { + return nullptr; + } + return m_pLibrary->trackCollectionManager()->getTrackById(m_secondLastEjectedTrackId); } Microphone* PlayerManager::getMicrophone(unsigned int microphone) const { @@ -772,7 +779,12 @@ void PlayerManager::slotSaveEjectedTrack(TrackPointer track) { VERIFY_OR_DEBUG_ASSERT(track) { return; } - m_lastEjectedTrackId = track->getId(); + const TrackId id = track->getId(); + if (id == m_lastEjectedTrackId) { + return; + } + m_secondLastEjectedTrackId = m_lastEjectedTrackId; + m_lastEjectedTrackId = id; } void PlayerManager::onTrackAnalysisProgress(TrackId trackId, AnalyzerProgress analyzerProgress) { diff --git a/src/mixer/playermanager.h b/src/mixer/playermanager.h index 4b90d02e54e1..d9d679bfac2a 100644 --- a/src/mixer/playermanager.h +++ b/src/mixer/playermanager.h @@ -130,6 +130,7 @@ class PlayerManager : public QObject, public PlayerManagerInterface { // Returns the track that was last ejected or unloaded. Can return nullptr or // invalid TrackId in case of error. TrackPointer getLastEjectedTrack() const; + TrackPointer getSecondLastEjectedTrack() const; // Get the microphone by its number. Microphones are numbered starting with 1. Microphone* getMicrophone(unsigned int microphone) const; @@ -285,6 +286,7 @@ class PlayerManager : public QObject, public PlayerManagerInterface { TrackAnalysisScheduler::Pointer m_pTrackAnalysisScheduler; + TrackId m_secondLastEjectedTrackId; TrackId m_lastEjectedTrackId; QList m_decks; diff --git a/src/skin/legacy/tooltips.cpp b/src/skin/legacy/tooltips.cpp index 095b46b16bb3..6ebfbcb523a5 100644 --- a/src/skin/legacy/tooltips.cpp +++ b/src/skin/legacy/tooltips.cpp @@ -619,9 +619,13 @@ void Tooltips::addStandardTooltips() { << tr("Repeat") << tr("When active the track will repeat if you go past the end or reverse before the start."); - add("eject") - << tr("Eject") - << tr("Ejects track from the player."); + add("eject") << tr("Eject") << tr("Ejects track from the player.") + << tr("Un-ejects when no track is loaded, i.e. reloads the " + "track that was ejected last (of any deck).") + << QString("%1: %2").arg(doubleClick, + "Reloads the last replaced track. " + "If no track is loaded reloads the second-last " + "ejected track."); add("hotcue") << tr("Hotcue") << QString("%1: %2").arg(leftClick, diff --git a/src/test/enginesynctest.cpp b/src/test/enginesynctest.cpp index caff76fe53aa..cacb7e8b4afc 100644 --- a/src/test/enginesynctest.cpp +++ b/src/test/enginesynctest.cpp @@ -1344,7 +1344,6 @@ TEST_F(EngineSyncTest, EjectTrackSyncRemains) { std::make_unique(m_sGroup1, "sync_enabled"); auto pButtonSyncEnabled2 = std::make_unique(m_sGroup2, "sync_enabled"); - auto pButtonEject1 = std::make_unique(m_sGroup1, "eject"); mixxx::BeatsPointer pBeats1 = mixxx::Beats::fromConstTempo( m_pTrack1->getSampleRate(), mixxx::audio::kStartFramePos, mixxx::Bpm(120)); @@ -1357,7 +1356,7 @@ TEST_F(EngineSyncTest, EjectTrackSyncRemains) { EXPECT_TRUE(isSoftLeader(m_sGroup1)); assertSyncOff(m_sGroup2); - pButtonEject1->set(1.0); + m_pChannel1->getEngineBuffer()->ejectTrack(); // When an eject happens, the bpm gets set to zero. ProcessBuffer(); @@ -1382,7 +1381,7 @@ TEST_F(EngineSyncTest, EjectTrackSyncRemains) { EXPECT_TRUE(isSoftLeader(m_sGroup1)); EXPECT_TRUE(isFollower(m_sGroup2)); - pButtonEject1->set(1.0); + m_pChannel1->getEngineBuffer()->ejectTrack(); m_pTrack1->trySetBeats(mixxx::BeatsPointer()); ProcessBuffer(); diff --git a/src/test/playermanagertest.cpp b/src/test/playermanagertest.cpp index c72c68a2a397..afd3386c5d7b 100644 --- a/src/test/playermanagertest.cpp +++ b/src/test/playermanagertest.cpp @@ -27,6 +27,12 @@ void deleteTrack(Track* pTrack) { delete pTrack; }; +void waitForTrackToBeLoaded(Deck* pDeck) { + while (!pDeck->getEngineDeck()->getEngineBuffer()->isTrackLoaded()) { + QTest::qSleep(100); // millis + } +} + } // namespace // We can't inherit from LibraryTest because that creates a key_notation control object that is also @@ -127,9 +133,10 @@ TEST_F(PlayerManagerTest, UnEjectTest) { ASSERT_NE(nullptr, deck1->getLoadedTrack()); m_pEngine->process(1024); - while (!deck1->getEngineDeck()->getEngineBuffer()->isTrackLoaded()) { - QTest::qSleep(100); // millis - } + waitForTrackToBeLoaded(deck1); + // make sure eject does not trigger 'unreplace': + // sleep for longer than 500 ms 'unreplace' period so this is not registered as double-click + QTest::qSleep(kUnreplaceDelay); // millis deck1->slotEjectTrack(1.0); // Load another track. @@ -140,6 +147,8 @@ TEST_F(PlayerManagerTest, UnEjectTest) { // Ejecting in an empty deck loads the last-ejected track. auto deck2 = m_pPlayerManager->getDeck(2); ASSERT_EQ(nullptr, deck2->getLoadedTrack()); + // make sure eject does not trigger 'unreplace' + QTest::qSleep(kUnreplaceDelay); // millis deck2->slotEjectTrack(2.0); ASSERT_NE(nullptr, deck2->getLoadedTrack()); ASSERT_EQ(testId1, deck2->getLoadedTrack()->getId()); @@ -158,22 +167,20 @@ TEST_F(PlayerManagerTest, UnEjectReplaceTrackTest) { ASSERT_NE(nullptr, deck1->getLoadedTrack()); m_pEngine->process(1024); - while (!deck1->getEngineDeck()->getEngineBuffer()->isTrackLoaded()) { - QTest::qSleep(100); // millis - } + waitForTrackToBeLoaded(deck1); // Load another track, replacing the first, causing it to be unloaded. TrackPointer pTrack2 = getOrAddTrackByLocation(getTestDir().filePath(kTrackLocationTest2)); ASSERT_NE(nullptr, pTrack2); deck1->slotLoadTrack(pTrack2, false); m_pEngine->process(1024); - while (!deck1->getEngineDeck()->getEngineBuffer()->isTrackLoaded()) { - QTest::qSleep(100); // millis - } + waitForTrackToBeLoaded(deck1); // Ejecting in an empty deck loads the last-ejected track. auto deck2 = m_pPlayerManager->getDeck(2); ASSERT_EQ(nullptr, deck2->getLoadedTrack()); + // make sure eject does not trigger 'unreplace' + QTest::qSleep(kUnreplaceDelay); deck2->slotEjectTrack(1.0); ASSERT_NE(nullptr, deck2->getLoadedTrack()); ASSERT_EQ(testId1, deck2->getLoadedTrack()->getId()); @@ -186,6 +193,42 @@ TEST_F(PlayerManagerTest, UnEjectInvalidTrackIdTest) { m_pPlayerManager->slotSaveEjectedTrack(pTrack); auto deck1 = m_pPlayerManager->getDeck(1); // Does nothing -- no crash. + // make sure eject does not trigger 'unreplace' + QTest::qSleep(kUnreplaceDelay); deck1->slotEjectTrack(1.0); ASSERT_EQ(nullptr, deck1->getLoadedTrack()); } + +TEST_F(PlayerManagerTest, UnReplaceTest) { + // Trigger eject twice within 500 ms to undo track replacement + auto deck1 = m_pPlayerManager->getDeck(1); + // Load a track + TrackPointer pTrack1 = getOrAddTrackByLocation(getTestDir().filePath(kTrackLocationTest1)); + ASSERT_NE(nullptr, pTrack1); + TrackId testId1 = pTrack1->getId(); + ASSERT_TRUE(testId1.isValid()); + deck1->slotLoadTrack(pTrack1, false); + m_pEngine->process(1024); + waitForTrackToBeLoaded(deck1); + ASSERT_NE(nullptr, deck1->getLoadedTrack()); + + // Load another track. + TrackPointer pTrack2 = getOrAddTrackByLocation(getTestDir().filePath(kTrackLocationTest2)); + ASSERT_NE(nullptr, pTrack2); + deck1->slotLoadTrack(pTrack2, false); + m_pEngine->process(1024); + waitForTrackToBeLoaded(deck1); + ASSERT_NE(nullptr, deck1->getLoadedTrack()); + + // Eject. Make sure eject does not trigger 'unreplace': + // sleep for longer than 500 ms 'unreplace' period so this is not registered as double-click + QTest::qSleep(kUnreplaceDelay); // millis + deck1->slotEjectTrack(1.0); + ASSERT_EQ(nullptr, deck1->getLoadedTrack()); + + // Eject again, assume this is reached faster than 500 ms after first eject + deck1->slotEjectTrack(1.0); + // First track should be reloaded + ASSERT_NE(nullptr, deck1->getLoadedTrack()); + ASSERT_EQ(testId1, deck1->getLoadedTrack()->getId()); +}