Merge remote-tracking branch 'upstream/2.3' into 2.4

src/library/autodj/autodjprocessor.cpp

@@ -150,8 +150,8 @@ AutoDJProcessor::AutoDJProcessor(
     m_pEnabledAutoDJ = new ControlPushButton(
             ConfigKey("[AutoDJ]", "enabled"));
     m_pEnabledAutoDJ->setButtonMode(ControlPushButton::TOGGLE);
-    connect(m_pEnabledAutoDJ, &ControlObject::valueChanged,
-            this, &AutoDJProcessor::controlEnable);
+    m_pEnabledAutoDJ->connectValueChangeRequest(this,
+            &AutoDJProcessor::controlEnableChangeRequest);
 
     // TODO(rryan) listen to signals from PlayerManager and add/remove as decks
     // are created.
@@ -233,6 +233,7 @@ void AutoDJProcessor::fadeNow() {
     if (!pLeftDeck || !pRightDeck) {
         // User has changed the orientation, disable Auto DJ
         toggleAutoDJ(false);
+        emit autoDJError(ADJ_NOT_TWO_DECKS);
         return;
     }
 
@@ -265,6 +266,7 @@ void AutoDJProcessor::fadeNow() {
         // Deck is empty or track too short, disable AutoDJ
         // This happens only if the user has changed deck orientation to such deck.
         toggleAutoDJ(false);
+        emit autoDJError(ADJ_NOT_TWO_DECKS);
         return;
     }
 
@@ -339,6 +341,7 @@ void AutoDJProcessor::fadeNow() {
 
 AutoDJProcessor::AutoDJError AutoDJProcessor::skipNext() {
     if (m_eState == ADJ_DISABLED) {
+        emit autoDJError(ADJ_IS_INACTIVE);
         return ADJ_IS_INACTIVE;
     }
     // Load the next song from the queue.
@@ -347,6 +350,7 @@ AutoDJProcessor::AutoDJError AutoDJProcessor::skipNext() {
     if (!pLeftDeck || !pRightDeck) {
         // User has changed the orientation, disable Auto DJ
         toggleAutoDJ(false);
+        emit autoDJError(ADJ_NOT_TWO_DECKS);
         return ADJ_NOT_TWO_DECKS;
     }
 
@@ -379,6 +383,7 @@ AutoDJProcessor::AutoDJError AutoDJProcessor::toggleAutoDJ(bool enable) {
         if (!pLeftDeck || !pRightDeck) {
             // Keep the current state.
             emitAutoDJStateChanged(m_eState);
+            emit autoDJError(ADJ_NOT_TWO_DECKS);
             return ADJ_NOT_TWO_DECKS;
         }
 
@@ -389,6 +394,7 @@ AutoDJProcessor::AutoDJError AutoDJProcessor::toggleAutoDJ(bool enable) {
             qDebug() << "One deck must be stopped before enabling Auto DJ mode";
             // Keep the current state.
             emitAutoDJStateChanged(m_eState);
+            emit autoDJError(ADJ_BOTH_DECKS_PLAYING);
             return ADJ_BOTH_DECKS_PLAYING;
         }
 
@@ -397,6 +403,7 @@ AutoDJProcessor::AutoDJError AutoDJProcessor::toggleAutoDJ(bool enable) {
         for (int i = 2; i < m_decks.length(); ++i) {
             if (m_decks[i] && m_decks[i]->isPlaying()) {
                 // Keep the current state.
+                emit autoDJError(ADJ_DECKS_3_4_PLAYING);
                 return ADJ_DECKS_3_4_PLAYING;
             }
         }
@@ -425,18 +432,15 @@ AutoDJProcessor::AutoDJError AutoDJProcessor::toggleAutoDJ(bool enable) {
 
         TrackPointer nextTrack = getNextTrackFromQueue();
         if (!nextTrack) {
-            qDebug() << "Queue is empty now";
-            if (m_pEnabledAutoDJ->get() != 0.0) {
-                m_pEnabledAutoDJ->set(0.0);
-            }
+            qDebug() << "Queue is empty now, disable Auto DJ";
+            m_pEnabledAutoDJ->setAndConfirm(0.0);
             emitAutoDJStateChanged(m_eState);
+            emit autoDJError(ADJ_QUEUE_EMPTY);
             return ADJ_QUEUE_EMPTY;
         }
 
         // Track is available so GO
-        if (m_pEnabledAutoDJ->get() != 1.0) {
-            m_pEnabledAutoDJ->set(1.0);
-        }
+        m_pEnabledAutoDJ->setAndConfirm(1.0);
         qDebug() << "Auto DJ enabled";
 
         m_pCOCrossfader->connectValueChanged(this, &AutoDJProcessor::crossfaderChanged);
@@ -565,7 +569,7 @@ AutoDJProcessor::AutoDJError AutoDJProcessor::toggleAutoDJ(bool enable) {
         }
         emitAutoDJStateChanged(m_eState);
     } else { // Disable Auto DJ
-        m_pEnabledAutoDJ->set(0.0);
+        m_pEnabledAutoDJ->setAndConfirm(0.0);
         qDebug() << "Auto DJ disabled";
         m_eState = ADJ_DISABLED;
         disconnect(m_pCOCrossfader,
@@ -580,7 +584,7 @@ AutoDJProcessor::AutoDJError AutoDJProcessor::toggleAutoDJ(bool enable) {
     return ADJ_OK;
 }
 
-void AutoDJProcessor::controlEnable(double value) {
+void AutoDJProcessor::controlEnableChangeRequest(double value) {
     toggleAutoDJ(value > 0.0);
 }
 
@@ -637,6 +641,7 @@ void AutoDJProcessor::crossfaderChanged(double value) {
                     }
                     pToDeck->play();
                 } else {
+                    // Track in toDeck was ejected manually, stop.
                     toggleAutoDJ(false);
                     return;
                 }
@@ -1534,6 +1539,7 @@ void AutoDJProcessor::playerTrackLoaded(DeckAttributes* pDeck, TrackPointer pTra
             }
             // User has changed the orientation, disable Auto DJ
             toggleAutoDJ(false);
+            emit autoDJError(ADJ_NOT_TWO_DECKS);
             return;
         }
         pDeck->startPos = kKeepPosition;
@@ -1642,6 +1648,7 @@ void AutoDJProcessor::setTransitionTime(int time) {
         if (!pLeftDeck || !pRightDeck) {
             // User has changed the orientation, disable Auto DJ
             toggleAutoDJ(false);
+            emit autoDJError(ADJ_NOT_TWO_DECKS);
             return;
         }
         if (pLeftDeck->isPlaying()) {
@@ -1670,6 +1677,7 @@ void AutoDJProcessor::setTransitionMode(TransitionMode newMode) {
     if (!pLeftDeck || !pRightDeck) {
         // User has changed the orientation, disable Auto DJ
         toggleAutoDJ(false);
+        emit autoDJError(ADJ_NOT_TWO_DECKS);
         return;
     }
 

src/library/autodj/autodjprocessor.h

@@ -203,6 +203,7 @@ class AutoDJProcessor : public QObject {
   signals:
     void loadTrackToPlayer(TrackPointer pTrack, const QString& group, bool play);
     void autoDJStateChanged(AutoDJProcessor::AutoDJState state);
+    void autoDJError(AutoDJProcessor::AutoDJError error);
     void transitionTimeChanged(int time);
     void randomTrackRequested(int tracksToAdd);
 
@@ -219,7 +220,7 @@ class AutoDJProcessor : public QObject {
     void playerEmpty(DeckAttributes* pDeck);
     void playerRateChanged(DeckAttributes* pDeck);
 
-    void controlEnable(double value);
+    void controlEnableChangeRequest(double value);
     void controlFadeNow(double value);
     void controlShuffle(double value);
     void controlSkipNext(double value);

src/library/autodj/dlgautodj.cpp

@@ -205,6 +205,11 @@ DlgAutoDJ::DlgAutoDJ(WLibrary* parent,
             this,
             &DlgAutoDJ::transitionTimeChanged);
 
+    connect(m_pAutoDJProcessor,
+            &AutoDJProcessor::autoDJError,
+            this,
+            &DlgAutoDJ::autoDJError);
+
     connect(m_pAutoDJProcessor,
             &AutoDJProcessor::autoDJStateChanged,
             this,
@@ -271,29 +276,30 @@ void DlgAutoDJ::fadeNowButton(bool) {
 }
 
 void DlgAutoDJ::toggleAutoDJButton(bool enable) {
-    AutoDJProcessor::AutoDJError error = m_pAutoDJProcessor->toggleAutoDJ(enable);
+    m_pAutoDJProcessor->toggleAutoDJ(enable);
+}
+
+// TODO If there's a way to migrate the translations move this
+// to AutoDJProcessor in order to keep this class minimal
+void DlgAutoDJ::autoDJError(AutoDJProcessor::AutoDJError error) {
     switch (error) {
     case AutoDJProcessor::ADJ_NOT_TWO_DECKS:
         QMessageBox::warning(nullptr,
                 tr("Auto DJ"),
                 tr("Auto DJ requires two decks assigned to opposite sides of the crossfader."),
                 QMessageBox::Ok);
-        // Make sure the button becomes unpushed.
-        pushButtonAutoDJ->setChecked(false);
         break;
     case AutoDJProcessor::ADJ_BOTH_DECKS_PLAYING:
         QMessageBox::warning(nullptr,
                 tr("Auto DJ"),
                 tr("One deck must be stopped to enable Auto DJ mode."),
                 QMessageBox::Ok);
-        pushButtonAutoDJ->setChecked(false);
         break;
     case AutoDJProcessor::ADJ_DECKS_3_4_PLAYING:
         QMessageBox::warning(nullptr,
                 tr("Auto DJ"),
                 tr("Decks 3 and 4 must be stopped to enable Auto DJ mode."),
                 QMessageBox::Ok);
-        pushButtonAutoDJ->setChecked(false);
         break;
     case AutoDJProcessor::ADJ_OK:
     default:

src/library/autodj/dlgautodj.h

@@ -42,6 +42,7 @@ class DlgAutoDJ : public QWidget, public Ui::DlgAutoDJ, public LibraryView {
     void skipNextButton(bool buttonChecked);
     void fadeNowButton(bool buttonChecked);
     void toggleAutoDJButton(bool enable);
+    void autoDJError(AutoDJProcessor::AutoDJError error);
     void transitionTimeChanged(int time);
     void transitionSliderChanged(int value);
     void autoDJStateChanged(AutoDJProcessor::AutoDJState state);

src/library/basesqltablemodel.cpp

@@ -430,7 +430,7 @@ void BaseSqlTableModel::setSort(int column, Qt::SortOrder order) {
     int trackSourceColumnCount = m_trackSource ? m_trackSource->columnCount() : 0;
 
     if (column < 0 ||
-            column >= trackSourceColumnCount + m_sortColumns.size() - 1) {
+            column >= trackSourceColumnCount + m_tableColumns.size() - 1) {
         // -1 because id column is in both tables
         qWarning() << "BaseSqlTableModel::setSort invalid column:" << column;
         return;

src/library/dao/cuedao.cpp

@@ -40,7 +40,7 @@ inline QString labelFromQVariant(const QVariant& value) {
 CuePointer cueFromRow(const QSqlRecord& row) {
     const auto id = DbId(row.value(row.indexOf("id")));
     TrackId trackId(row.value(row.indexOf("track_id")));
-    int type = row.value(row.indexOf("type")).toInt();
+    auto type = static_cast<mixxx::CueType>(row.value(row.indexOf("type")).toInt());
     const auto position =
             mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid(
                     row.value(row.indexOf("position")).toDouble());
@@ -51,8 +51,20 @@ CuePointer cueFromRow(const QSqlRecord& row) {
     VERIFY_OR_DEBUG_ASSERT(color) {
         return CuePointer();
     }
+    if (type == mixxx::CueType::Loop && lengthFrames == 0.0) {
+        // These entries are likely added via issue #11283
+        qWarning() << "Discard loop cue" << hotcue << "found in database with length of 0";
+        return CuePointer();
+    }
+    if (type == mixxx::CueType::HotCue &&
+            position == mixxx::audio::FramePos(0) &&
+            *color == mixxx::RgbColor(0)) {
+        // These entries are likely added via issue #11283
+        qWarning() << "Discard black hot cue" << hotcue << "found in database at position 0";
+        return CuePointer();
+    }
     CuePointer pCue(new Cue(id,
-            static_cast<mixxx::CueType>(type),
+            type,
             position,
             lengthFrames,
             hotcue,
@@ -84,7 +96,7 @@ QList<CuePointer> CueDAO::getCuesForTrack(TrackId trackId) const {
     QMap<int, CuePointer> hotCuesByNumber;
     while (query.next()) {
         CuePointer pCue = cueFromRow(query.record());
-        VERIFY_OR_DEBUG_ASSERT(pCue) {
+        if (!pCue) {
             continue;
         }
         int hotCueNumber = pCue->getHotCue();
@@ -190,21 +202,6 @@ bool CueDAO::saveCue(TrackId trackId, Cue* cue) const {
     return true;
 }
 
-bool CueDAO::deleteCue(Cue* cue) const {
-    //qDebug() << "CueDAO::deleteCue" << QThread::currentThread() << m_database.connectionName();
-    if (!cue->getId().isValid()) {
-        return false;
-    }
-    QSqlQuery query(m_database);
-    query.prepare(QStringLiteral("DELETE FROM " CUE_TABLE " WHERE id=:id"));
-    query.bindValue(":id", cue->getId().toVariant());
-    if (!query.exec()) {
-        LOG_FAILED_QUERY(query);
-        return false;
-    }
-    return true;
-}
-
 void CueDAO::saveTrackCues(
         TrackId trackId,
         const QList<CuePointer>& cueList) const {

src/library/dao/cuedao.h

@@ -22,5 +22,4 @@ class CueDAO : public DAO {
 
   private:
     bool saveCue(TrackId trackId, Cue* pCue) const;
-    bool deleteCue(Cue* pCue) const;
 };

src/mixer/basetrackplayer.cpp

@@ -276,22 +276,18 @@ void BaseTrackPlayerImpl::loadTrack(TrackPointer pTrack) {
     // The loop in and out points must be set here and not in slotTrackLoaded
     // so LoopingControl::trackLoaded can access them.
     if (!m_pChannelToCloneFrom) {
-        const QList<CuePointer> trackCues(m_pLoadedTrack->getCuePoints());
-        QListIterator<CuePointer> it(trackCues);
-        CuePointer pLoopCue;
         // Restore loop from the first loop cue with minimum hotcue number.
         // For the volatile "most recent loop" the hotcue number will be -1.
         // If no such loop exists, restore a saved loop cue.
-        while (it.hasNext()) {
-            CuePointer pCue(it.next());
+        CuePointer pLoopCue;
+        const QList<CuePointer> trackCues = m_pLoadedTrack->getCuePoints();
+        for (const auto& pCue : trackCues) {
             if (pCue->getType() != mixxx::CueType::Loop) {
                 continue;
             }
-
             if (pLoopCue && pLoopCue->getHotCue() <= pCue->getHotCue()) {
                 continue;
             }
-
             pLoopCue = pCue;
         }
 
@@ -359,10 +355,8 @@ TrackPointer BaseTrackPlayerImpl::unloadTrack() {
                     m_pLoopOutPoint->get());
     if (loopStart.isValid() && loopEnd.isValid() && loopStart <= loopEnd) {
         CuePointer pLoopCue;
-        QList<CuePointer> cuePoints(m_pLoadedTrack->getCuePoints());
-        QListIterator<CuePointer> it(cuePoints);
-        while (it.hasNext()) {
-            CuePointer pCue(it.next());
+        const QList<CuePointer> cuePoints = m_pLoadedTrack->getCuePoints();
+        for (const auto& pCue : cuePoints) {
             if (pCue->getType() == mixxx::CueType::Loop && pCue->getHotCue() == Cue::kNoHotCue) {
                 pLoopCue = pCue;
                 break;

src/test/cue_test.cpp

@@ -36,7 +36,7 @@ TEST(CueTest, ConvertCueInfoToCueRoundtrip) {
     const auto cueInfo1 = CueInfo(
             CueType::HotCue,
             std::make_optional(1.0 * 44100 * mixxx::kEngineChannelCount),
-            std::make_optional(2.0 * 44100 * mixxx::kEngineChannelCount),
+            std::nullopt,
             std::make_optional(3),
             QStringLiteral("label"),
             RgbColor::optional(0xABCDEF));

src/track/cueinfo.cpp

@@ -4,6 +4,33 @@
 
 namespace mixxx {
 
+namespace {
+
+void assertEndPosition(
+        CueType type,
+        std::optional<double> endPositionMillis) {
+    switch (type) {
+    case CueType::HotCue:
+    case CueType::MainCue:
+        DEBUG_ASSERT(!endPositionMillis);
+        break;
+    case CueType::Loop:
+    case CueType::Jump:
+    case CueType::N60dBSound:
+        DEBUG_ASSERT(endPositionMillis);
+        break;
+    case CueType::Intro:
+    case CueType::Outro:
+    case CueType::Invalid:
+        break;
+    case CueType::Beat: // unused
+    default:
+        DEBUG_ASSERT(!"Unknown Loop Type");
+    }
+}
+
+} // namespace
+
 CueInfo::CueInfo()
         : m_type(CueType::Invalid),
           m_startPositionMillis(std::nullopt),
@@ -28,6 +55,7 @@ CueInfo::CueInfo(
           m_label(std::move(label)),
           m_color(color),
           m_flags(flags) {
+    assertEndPosition(type, endPositionMillis);
 }
 
 CueType CueInfo::getType() const {
@@ -48,6 +76,7 @@ std::optional<double> CueInfo::getStartPositionMillis() const {
 
 void CueInfo::setEndPositionMillis(const std::optional<double>& positionMillis) {
     m_endPositionMillis = positionMillis;
+    assertEndPosition(m_type, m_endPositionMillis);
 }
 
 std::optional<double> CueInfo::getEndPositionMillis() const {
@@ -58,8 +87,7 @@ std::optional<int> CueInfo::getHotCueIndex() const {
     return m_hotCueIndex;
 }
 
-void CueInfo::setHotCueIndex(const std::optional<int>& hotCueIndex) {
-    DEBUG_ASSERT(!hotCueIndex || *hotCueIndex >= kFirstHotCueIndex);
+void CueInfo::setHotCueIndex(int hotCueIndex) {
     m_hotCueIndex = hotCueIndex;
 }