Merge pull request #2526 from Holzhaus/serato-markers-integration-cues

Read Cues from Serato MP3 Tags

src/track/serato/markers.cpp

@@ -284,4 +284,35 @@ QByteArray SeratoMarkers::dump() const {
     return data;
 }
 
+QList<CueInfo> SeratoMarkers::getCues(double timingOffsetMillis) const {
+    qDebug() << "Reading cues from 'Serato Markers_' tag data...";
+
+    QList<CueInfo> cueInfos;
+    int cueIndex = 0;
+    for (const auto& pEntry : m_entries) {
+        DEBUG_ASSERT(pEntry);
+        switch (pEntry->typeId()) {
+        case SeratoMarkersEntry::TypeId::Cue: {
+            if (pEntry->hasStartPosition()) {
+                CueInfo cueInfo(
+                        CueType::HotCue,
+                        pEntry->getStartPosition() + timingOffsetMillis,
+                        std::nullopt,
+                        cueIndex,
+                        "",
+                        pEntry->getColor());
+                cueInfos.append(cueInfo);
+            }
+            cueIndex++;
+            break;
+        }
+        // TODO: Add support for Loops
+        default:
+            break;
+        }
+    }
+
+    return cueInfos;
+}
+
 } //namespace mixxx

src/track/serato/markers.h

@@ -6,7 +6,7 @@
 #include <QList>
 #include <memory>
 
-#include "util/color/rgbcolor.h"
+#include "track/cueinfo.h"
 #include "util/types.h"
 
 namespace mixxx {
@@ -91,7 +91,6 @@ class SeratoMarkersEntry {
     bool m_hasEndPosition;
     ;
     bool m_isLocked;
-    bool m_isSet;
     quint32 m_startPosition;
     quint32 m_endPosition;
     int m_type;
@@ -155,6 +154,8 @@ class SeratoMarkers final {
         m_trackColor = color;
     }
 
+    QList<CueInfo> getCues(double timingOffsetMillis) const;
+
   private:
     QList<SeratoMarkersEntryPointer> m_entries;
     RgbColor::optional_t m_trackColor;

src/track/serato/markers2.cpp

@@ -438,6 +438,34 @@ QByteArray SeratoMarkers2::dump() const {
     return outerData.leftJustified(size, '\0');
 }
 
+QList<CueInfo> SeratoMarkers2::getCues(double timingOffsetMillis) const {
+    qDebug() << "Reading cues from 'Serato Markers2' tag data...";
+    QList<CueInfo> cueInfos;
+    for (auto& pEntry : m_entries) {
+        DEBUG_ASSERT(pEntry);
+        switch (pEntry->typeId()) {
+        case SeratoMarkers2Entry::TypeId::Cue: {
+            const SeratoMarkers2CueEntry* pCueEntry = static_cast<SeratoMarkers2CueEntry*>(pEntry.get());
+            CueInfo cueInfo(
+                    CueType::HotCue,
+                    pCueEntry->getPosition() + timingOffsetMillis,
+                    std::nullopt,
+                    pCueEntry->getIndex(),
+                    pCueEntry->getLabel(),
+                    pCueEntry->getColor());
+            cueInfos.append(cueInfo);
+
+            break;
+        }
+        // TODO: Add support for LOOP/FLIP
+        default:
+            break;
+        }
+    }
+
+    return cueInfos;
+}
+
 RgbColor::optional_t SeratoMarkers2::getTrackColor() const {
     qDebug() << "Reading track color from 'Serato Markers2' tag data...";
 

src/track/serato/markers2.h

@@ -6,7 +6,7 @@
 #include <QList>
 #include <memory>
 
-#include "util/color/rgbcolor.h"
+#include "track/cueinfo.h"
 #include "util/types.h"
 
 namespace mixxx {
@@ -398,6 +398,7 @@ class SeratoMarkers2 final {
         m_entries = std::move(entries);
     }
 
+    QList<CueInfo> getCues(double timingOffsetMillis) const;
     RgbColor::optional_t getTrackColor() const;
     bool isBpmLocked() const;
 

src/track/serato/tags.cpp

@@ -1,5 +1,35 @@
 #include "track/serato/tags.h"
 
+#include <mp3guessenc.h>
+
+#include "util/color/predefinedcolorpalettes.h"
+
+namespace {
+
+#ifdef __COREAUDIO__
+const QString kDecoderName(QStringLiteral("CoreAudio"));
+#elif defined(__MAD__)
+const QString kDecoderName(QStringLiteral("MAD"));
+#elif defined(__FFMPEG__)
+const QString kDecoderName(QStringLiteral("FFMPEG"));
+#else
+const QString kDecoderName(QStringLiteral("Unknown"));
+#endif
+
+mixxx::RgbColor getColorFromOtherPalette(
+        const ColorPalette& source,
+        const ColorPalette& dest,
+        mixxx::RgbColor color) {
+    DEBUG_ASSERT(source.size() == dest.size());
+    int sourceIndex = source.indexOf(color);
+    if (sourceIndex >= 0 && sourceIndex < dest.size()) {
+        return dest.at(sourceIndex);
+    }
+    return color;
+}
+
+} // namespace
+
 namespace mixxx {
 
 RgbColor::optional_t SeratoTags::storedToDisplayedTrackColor(RgbColor color) {
@@ -59,6 +89,144 @@ RgbColor SeratoTags::displayedToStoredTrackColor(RgbColor::optional_t color) {
     return RgbColor(colorCode);
 }
 
+RgbColor SeratoTags::storedToDisplayedSeratoDJProCueColor(RgbColor color) {
+    return getColorFromOtherPalette(
+            PredefinedColorPalettes::kSeratoTrackMetadataHotcueColorPalette,
+            PredefinedColorPalettes::kSeratoDJProHotcueColorPalette,
+            color);
+}
+
+RgbColor SeratoTags::displayedToStoredSeratoDJProCueColor(RgbColor color) {
+    return getColorFromOtherPalette(
+            PredefinedColorPalettes::kSeratoDJProHotcueColorPalette,
+            PredefinedColorPalettes::kSeratoTrackMetadataHotcueColorPalette,
+            color);
+}
+
+double SeratoTags::findTimingOffsetMillis(const QString& filePath) {
+    // The following code accounts for timing offsets required to
+    // correctly align timing information (e.g. cue points) exported from
+    // Serato. This is caused by different MP3 decoders treating MP3s encoded
+    // in a variety of different cases differently. The mp3guessenc library is
+    // used to determine which case the MP3 is clasified in. See the following
+    // PR for more detailed information:
+    // https://github.com/mixxxdj/mixxx/pull/2119
+
+    double timingOffset = 0;
+    if (filePath.toLower().endsWith(".mp3")) {
+        int timingShiftCase = mp3guessenc_timing_shift_case(filePath.toStdString().c_str());
+
+        // TODO: Find missing timing offsets
+        switch (timingShiftCase) {
+#if defined(__COREAUDIO__)
+        case EXIT_CODE_CASE_A:
+            timingOffset = -12;
+            break;
+        case EXIT_CODE_CASE_B:
+            timingOffset = -40;
+            break;
+        case EXIT_CODE_CASE_C:
+        case EXIT_CODE_CASE_D:
+            timingOffset = -60;
+            break;
+#elif defined(__MAD__) || defined(__FFMPEG__)
+        // Apparently all mp3guessenc cases have the same offset for MAD
+        // and FFMPEG
+        default:
+            timingOffset = -19;
+            break;
+#endif
+        }
+        qDebug()
+                << "Detected timing offset "
+                << timingOffset
+                << "("
+                << kDecoderName
+                << ", case"
+                << timingShiftCase
+                << ") for MP3 file:"
+                << filePath;
+    }
+
+    return timingOffset;
+}
+
+QList<CueInfo> SeratoTags::getCues(const QString& filePath) const {
+    // Import "Serato Markers2" first, then overwrite values with those
+    // from "Serato Markers_". This is what Serato does too (i.e. if
+    // "Serato Markers_" and "Serato Markers2" contradict each other,
+    // Serato will use the values from "Serato Markers_").
+
+    double timingOffsetMillis = SeratoTags::findTimingOffsetMillis(filePath);
+
+    QMap<int, CueInfo> cueMap;
+    for (const CueInfo& cueInfo : m_seratoMarkers2.getCues(timingOffsetMillis)) {
+        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!";
+            continue;
+        }
+
+        CueInfo newCueInfo(cueInfo);
+        RgbColor::optional_t color = cueInfo.getColor();
+        if (color) {
+            // TODO: Make this conversion configurable
+            newCueInfo.setColor(storedToDisplayedSeratoDJProCueColor(*color));
+        }
+        newCueInfo.setHotCueNumber(index);
+        cueMap.insert(index, newCueInfo);
+    };
+
+    // TODO(jholthuis): If a hotcue is set in SeratoMarkers2, but not in
+    // SeratoMarkers_, we could remove it from the output. We'll just leave it
+    // in for now.
+    for (const CueInfo& cueInfo : m_seratoMarkers.getCues(timingOffsetMillis)) {
+        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!";
+            continue;
+        }
+
+        // Take a pre-existing CueInfo object that was read from
+        // "SeratoMarkers2" from the CueMap (or a default constructed CueInfo
+        // 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);
+        newCueInfo.setType(cueInfo.getType());
+        newCueInfo.setStartPositionMillis(cueInfo.getStartPositionMillis());
+        newCueInfo.setEndPositionMillis(cueInfo.getEndPositionMillis());
+        newCueInfo.setHotCueNumber(index);
+
+        RgbColor::optional_t color = cueInfo.getColor();
+        if (color) {
+            // TODO: Make this conversion configurable
+            newCueInfo.setColor(storedToDisplayedSeratoDJProCueColor(*color));
+        }
+        cueMap.insert(index, newCueInfo);
+    };
+
+    return cueMap.values();
+}
+
 RgbColor::optional_t SeratoTags::getTrackColor() const {
     RgbColor::optional_t color = m_seratoMarkers.getTrackColor();
 

src/track/serato/tags.h

@@ -17,6 +17,9 @@ class SeratoTags final {
 
     static RgbColor::optional_t storedToDisplayedTrackColor(RgbColor color);
     static RgbColor displayedToStoredTrackColor(RgbColor::optional_t color);
+    static RgbColor storedToDisplayedSeratoDJProCueColor(RgbColor color);
+    static RgbColor displayedToStoredSeratoDJProCueColor(RgbColor color);
+    static double findTimingOffsetMillis(const QString& filePath);
 
     bool isEmpty() const {
         return m_seratoMarkers.isEmpty() && m_seratoMarkers2.isEmpty();
@@ -38,6 +41,8 @@ class SeratoTags final {
         return m_seratoMarkers2.dump();
     }
 
+    QList<CueInfo> getCues(const QString& filePath) const;
+
     RgbColor::optional_t getTrackColor() const;
     bool isBpmLocked() const;
 

src/track/track.cpp

@@ -128,9 +128,7 @@ void Track::importMetadata(
     const auto newBpm = importedMetadata.getTrackInfo().getBpm();
     const auto newKey = importedMetadata.getTrackInfo().getKey();
     const auto newReplayGain = importedMetadata.getTrackInfo().getReplayGain();
-#ifdef __EXTRA_METADATA__
     const auto newSeratoTags = importedMetadata.getTrackInfo().getSeratoTags();
-#endif // __EXTRA_METADATA__
     {
         // enter locking scope
         QMutexLocker lock(&m_qMutex);
@@ -165,10 +163,12 @@ void Track::importMetadata(
             }
         }
 
-#ifdef __EXTRA_METADATA__
+        // FIXME: Move the Track::setCuePoints call to another location,
+        // because we need the sample rate to calculate sample
+        // positions for cues (and *correct* sample rate isn't known here).
+        importCueInfos(newSeratoTags.getCues(getLocation()));
         setColor(newSeratoTags.getTrackColor());
         setBpmLocked(newSeratoTags.isBpmLocked());
-#endif // __EXTRA_METADATA__
 
         // implicitly unlocked when leaving scope
     }

src/track/trackinfo.cpp

@@ -74,8 +74,8 @@ bool TrackInfo::compareEq(
             (getRemixer() == trackInfo.getRemixer()) &&
 #endif // __EXTRA_METADATA__
             (getReplayGain() == trackInfo.getReplayGain()) &&
-#if defined(__EXTRA_METADATA__)
             (getSeratoTags() == trackInfo.getSeratoTags()) &&
+#if defined(__EXTRA_METADATA__)
             (getSubtitle() == trackInfo.getSubtitle()) &&
 #endif // __EXTRA_METADATA__
             (getTitle() == trackInfo.getTitle()) &&
@@ -118,8 +118,8 @@ QDebug operator<<(QDebug dbg, const TrackInfo& arg) {
     arg.dbgRemixer(dbg);
 #endif // __EXTRA_METADATA__
     arg.dbgReplayGain(dbg);
-#if defined(__EXTRA_METADATA__)
     arg.dbgSeratoTags(dbg);
+#if defined(__EXTRA_METADATA__)
     arg.dbgSubtitle(dbg);
 #endif // __EXTRA_METADATA__
     arg.dbgTitle(dbg);

src/track/trackinfo.h

@@ -43,8 +43,8 @@ class TrackInfo final {
     PROPERTY_SET_BYVAL_GET_BYREF(QString,    remixer,              Remixer)
 #endif // __EXTRA_METADATA__
     PROPERTY_SET_BYVAL_GET_BYREF(ReplayGain, replayGain,           ReplayGain)
-#if defined(__EXTRA_METADATA__)
     PROPERTY_SET_BYVAL_GET_BYREF(SeratoTags, seratoTags,           SeratoTags)
+#if defined(__EXTRA_METADATA__)
     PROPERTY_SET_BYVAL_GET_BYREF(QString,    subtitle,             Subtitle)
 #endif // __EXTRA_METADATA__
     PROPERTY_SET_BYVAL_GET_BYREF(QString,    title,                Title)