Merge pull request #11246 from ronso0/reload-track

eject: double click does 'un-replace', i.e. reloads the previous track

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)<br>"
+               "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);
 

src/controllers/dlgcontrollerlearning.cpp

@@ -25,6 +25,7 @@ DlgControllerLearning::DlgControllerLearning(QWidget* parent,
     qRegisterMetaType<MidiInputMappings>("MidiInputMappings");
 
     setupUi(this);
+    labelDescription->setWordWrap(true);
     labelMappedTo->setText("");
 
     QString helpTitle(tr("Click anywhere in Mixxx or choose a control to learn"));

src/mixer/basetrackplayer.cpp

@@ -223,6 +223,8 @@ BaseTrackPlayerImpl::BaseTrackPlayerImpl(
             ConfigKey(getGroup(), "update_replaygain_from_pregain"));
     m_pUpdateReplayGainFromPregain->connectValueChangeRequest(this,
             &BaseTrackPlayerImpl::slotUpdateReplayGainFromPregain);
+
+    m_ejectTimer.start();
 }
 
 BaseTrackPlayerImpl::~BaseTrackPlayerImpl() {
@@ -321,6 +323,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) {

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 {
@@ -129,6 +131,8 @@ class BaseTrackPlayerImpl : public BaseTrackPlayer {
     bool m_replaygainPending;
     EngineChannel* m_pChannelToCloneFrom;
 
+    PerformanceTimer m_ejectTimer;
+
     std::unique_ptr<ControlPushButton> m_pEject;
 
     // Deck clone control

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) {

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<Deck*> m_decks;

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,

src/test/enginesynctest.cpp

@@ -1344,7 +1344,6 @@ TEST_F(EngineSyncTest, EjectTrackSyncRemains) {
             std::make_unique<ControlProxy>(m_sGroup1, "sync_enabled");
     auto pButtonSyncEnabled2 =
             std::make_unique<ControlProxy>(m_sGroup2, "sync_enabled");
-    auto pButtonEject1 = std::make_unique<ControlProxy>(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();
 

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());
+}