Import saved loops from Serato Tags

src/engine/controls/cuecontrol.cpp

@@ -467,21 +467,30 @@ void CueControl::loadCuesFromTrack() {
         return;
 
     for (const CuePointer& pCue: m_pLoadedTrack->getCuePoints()) {
-        if (pCue->getType() == mixxx::CueType::MainCue) {
+        switch (pCue->getType()) {
+        case mixxx::CueType::MainCue:
             DEBUG_ASSERT(!pLoadCue); // There should be only one MainCue cue
             pLoadCue = pCue;
-        } else if (pCue->getType() == mixxx::CueType::Intro) {
+            break;
+        case mixxx::CueType::Intro:
             DEBUG_ASSERT(!pIntroCue); // There should be only one Intro cue
             pIntroCue = pCue;
-        } else if (pCue->getType() == mixxx::CueType::Outro) {
+            break;
+        case mixxx::CueType::Outro:
             DEBUG_ASSERT(!pOutroCue); // There should be only one Outro cue
             pOutroCue = pCue;
-        } else if (pCue->getType() == mixxx::CueType::HotCue && pCue->getHotCue() != Cue::kNoHotCue) {
+            break;
+        case mixxx::CueType::HotCue:
+        case mixxx::CueType::Loop: {
+            if (pCue->getHotCue() == Cue::kNoHotCue) {
+                continue;
+            }
+
             int hotcue = pCue->getHotCue();
             HotcueControl* pControl = m_hotcueControls.value(hotcue, NULL);
 
             // Cue's hotcue doesn't have a hotcue control.
-            if (pControl == NULL) {
+            if (pControl == nullptr) {
                 continue;
             }
 
@@ -498,6 +507,10 @@ void CueControl::loadCuesFromTrack() {
             }
             // Add the hotcue to the list of active hotcues
             active_hotcues.insert(hotcue);
+            break;
+        }
+        default:
+            break;
         }
     }
 

src/track/serato/markers.cpp

@@ -535,7 +535,8 @@ QByteArray SeratoMarkers::dumpMP4() const {
             base64Data.append('\n');
         }
         QByteArray block = data.mid(offset, 54);
-        base64Data.append(block.toBase64(QByteArray::Base64Encoding | QByteArray::OmitTrailingEquals));
+        base64Data.append(block.toBase64(
+                QByteArray::Base64Encoding | QByteArray::OmitTrailingEquals));
         offset += block.size();
     }
 
@@ -550,6 +551,7 @@ QList<CueInfo> SeratoMarkers::getCues() const {
 
     QList<CueInfo> cueInfos;
     int cueIndex = 0;
+    int loopIndex = 0;
     for (const auto& pEntry : m_entries) {
         DEBUG_ASSERT(pEntry);
         switch (pEntry->typeId()) {
@@ -567,7 +569,21 @@ QList<CueInfo> SeratoMarkers::getCues() const {
             cueIndex++;
             break;
         }
-        // TODO: Add support for Loops
+        case SeratoMarkersEntry::TypeId::Loop: {
+            if (pEntry->hasStartPosition()) {
+                CueInfo loopInfo = CueInfo(
+                        CueType::Loop,
+                        pEntry->getStartPosition(),
+                        pEntry->getEndPosition(),
+                        loopIndex,
+                        "",
+                        std::nullopt);
+                cueInfos.append(loopInfo);
+                // TODO: Add support for the "locked" attribute
+            }
+            loopIndex++;
+            break;
+        }
         default:
             break;
         }

src/track/serato/markers2.cpp

@@ -545,6 +545,7 @@ QByteArray SeratoMarkers2::dumpID3() const {
 
 QList<CueInfo> SeratoMarkers2::getCues() const {
     qDebug() << "Reading cues from 'Serato Markers2' tag data...";
+
     QList<CueInfo> cueInfos;
     for (auto& pEntry : m_entries) {
         DEBUG_ASSERT(pEntry);
@@ -561,7 +562,21 @@ QList<CueInfo> SeratoMarkers2::getCues() const {
             cueInfos.append(cueInfo);
             break;
         }
-        // TODO: Add support for LOOP/FLIP
+        case SeratoMarkers2Entry::TypeId::Loop: {
+            const SeratoMarkers2LoopEntry* pLoopEntry =
+                    static_cast<SeratoMarkers2LoopEntry*>(pEntry.get());
+            CueInfo loopInfo = CueInfo(
+                    CueType::Loop,
+                    pLoopEntry->getStartPosition(),
+                    pLoopEntry->getEndPosition(),
+                    pLoopEntry->getIndex(),
+                    pLoopEntry->getLabel(),
+                    std::nullopt); // Serato's Loops don't have a color
+            // TODO: Add support for "locked" loops
+            cueInfos.append(loopInfo);
+            break;
+        }
+        // TODO: Add support for FLIP
         default:
             break;
         }

src/track/serato/tags.cpp

@@ -18,6 +18,12 @@ const QString kDecoderName(QStringLiteral("FFMPEG"));
 const QString kDecoderName(QStringLiteral("Unknown"));
 #endif
 
+/// In Serato, loops and hotcues are kept separate, i. e. you can
+/// have a loop and a hotcue with the same number. In Mixxx, loops
+/// and hotcues share indices. Hence, we import them with and offset
+/// of 8 (the maximum number of hotcues in Serato).
+const int kLoopIndexOffset = 8;
+
 mixxx::RgbColor getColorFromOtherPalette(
         const ColorPalette& source,
         const ColorPalette& dest,
@@ -30,20 +36,50 @@ mixxx::RgbColor getColorFromOtherPalette(
     return color;
 }
 
+std::optional<int> findIndexForCueInfo(const mixxx::CueInfo& cueInfo) {
+    VERIFY_OR_DEBUG_ASSERT(cueInfo.getHotCueNumber()) {
+        qWarning() << "SeratoTags::getCues: Cue without number found!";
+        return std::nullopt;
+    }
+
+    int index = *cueInfo.getHotCueNumber();
+    VERIFY_OR_DEBUG_ASSERT(index >= 0) {
+        qWarning() << "SeratoTags::getCues: Cue with number < 0 found!";
+        return std::nullopt;
+    }
+
+    switch (cueInfo.getType()) {
+    case mixxx::CueType::HotCue:
+        if (index >= kLoopIndexOffset) {
+            qWarning()
+                    << "SeratoTags::getCues: Non-loop Cue with number >="
+                    << kLoopIndexOffset << "found!";
+            return std::nullopt;
+        }
+        break;
+    case mixxx::CueType::Loop:
+        index += kLoopIndexOffset;
+        break;
+    default:
+        return std::nullopt;
+    }
+
+    return index;
+}
+
 } // namespace
 
 namespace mixxx {
 
+/// Serato stores Track colors differently from how they are displayed in
+/// the library column. Instead of the color from the library view, the
+/// value from the color picker is stored instead (which is different).
+/// To make sure that the track looks the same in both Mixxx' and Serato's
+/// libraries, we need to convert between the two values.
+///
+/// See this for details:
+/// https://github.com/Holzhaus/serato-tags/blob/master/docs/colors.md#track-colors
 RgbColor::optional_t SeratoTags::storedToDisplayedTrackColor(RgbColor color) {
-    // Serato stores Track colors differently from how they are displayed in
-    // the library column. Instead of the color from the library view, the
-    // value from the color picker is stored instead (which is different).
-    // To make sure that the track looks the same in both Mixxx' and Serato's
-    // libraries, we need to convert between the two values.
-    //
-    // See this for details:
-    // https://github.com/Holzhaus/serato-tags/blob/master/docs/colors.md#track-colors
-
     if (color == 0xFFFFFF) {
         return RgbColor::nullopt();
     }
@@ -144,7 +180,9 @@ double SeratoTags::guessTimingOffsetMillis(
             timingOffset = -26;
             break;
         default:
-            qWarning() << "Unknown timing offset for Serato tags with sample rate" << signalInfo.getSampleRate();
+            qWarning()
+                    << "Unknown timing offset for Serato tags with sample rate"
+                    << signalInfo.getSampleRate();
         }
 #endif
         qDebug()
@@ -167,29 +205,20 @@ CueInfoImporterPointer SeratoTags::importCueInfos() const {
 
     QMap<int, CueInfo> cueMap;
     for (const CueInfo& cueInfo : m_seratoMarkers2.getCues()) {
-        VERIFY_OR_DEBUG_ASSERT(cueInfo.getHotCueNumber()) {
-            qWarning() << "SeratoTags::getCues: Cue without number found!";
-            continue;
-        }
-
-        int index = *cueInfo.getHotCueNumber();
-        VERIFY_OR_DEBUG_ASSERT(index >= 0) {
-            qWarning() << "SeratoTags::getCues: Cue with number < 0 found!";
-        }
-
-        if (cueInfo.getType() != CueType::HotCue) {
-            qWarning() << "SeratoTags::getCues: Ignoring cue with non-hotcue type!";
+        std::optional<int> index = findIndexForCueInfo(cueInfo);
+        if (!index) {
             continue;
         }
 
         CueInfo newCueInfo(cueInfo);
+        newCueInfo.setHotCueNumber(index);
+
         RgbColor::optional_t color = cueInfo.getColor();
         if (color) {
             // TODO: Make this conversion configurable
             newCueInfo.setColor(storedToDisplayedSeratoDJProCueColor(*color));
         }
-        newCueInfo.setHotCueNumber(index);
-        cueMap.insert(index, newCueInfo);
+        cueMap.insert(*index, newCueInfo);
     };
 
     // If the "Serato Markers_" tag does not exist at all, Serato DJ Pro just
@@ -207,19 +236,9 @@ CueInfoImporterPointer SeratoTags::importCueInfos() const {
     // this function.
     QSet<int> unsetCuesInMarkersTag = {0, 1, 2, 3, 4};
 
-    for (const CueInfo& cueInfo : m_seratoMarkers2.getCues()) {
-        VERIFY_OR_DEBUG_ASSERT(cueInfo.getHotCueNumber()) {
-            qWarning() << "SeratoTags::getCues: Cue without number found!";
-            continue;
-        }
-
-        int index = *cueInfo.getHotCueNumber();
-        VERIFY_OR_DEBUG_ASSERT(index >= 0) {
-            qWarning() << "SeratoTags::getCues: Cue with number < 0 found!";
-        }
-
-        if (cueInfo.getType() != CueType::HotCue) {
-            qWarning() << "SeratoTags::getCues: Ignoring cue with non-hotcue type!";
+    for (const CueInfo& cueInfo : m_seratoMarkers.getCues()) {
+        std::optional<int> index = findIndexForCueInfo(cueInfo);
+        if (!index) {
             continue;
         }
 
@@ -228,7 +247,7 @@ CueInfoImporterPointer SeratoTags::importCueInfos() const {
         // object if none exists) and use it as template for the new CueInfo
         // object. Then overwrite all object values that are present in the
         // "SeratoMarkers_"tag.
-        CueInfo newCueInfo(cueMap.value(index));
+        CueInfo newCueInfo = cueMap.value(*index);
         newCueInfo.setType(cueInfo.getType());
         newCueInfo.setStartPositionMillis(cueInfo.getStartPositionMillis());
         newCueInfo.setEndPositionMillis(cueInfo.getEndPositionMillis());
@@ -239,11 +258,11 @@ CueInfoImporterPointer SeratoTags::importCueInfos() const {
             // TODO: Make this conversion configurable
             newCueInfo.setColor(storedToDisplayedSeratoDJProCueColor(*color));
         }
-        cueMap.insert(index, newCueInfo);
+        cueMap.insert(*index, newCueInfo);
 
         // This cue is set in the "Serato Markers_" tag, so remove it from the
         // set of unset cues
-        unsetCuesInMarkersTag.remove(index);
+        unsetCuesInMarkersTag.remove(*index);
     };
 
     // Now that we know which cues should be present in the "Serato Markers_"