From b8d84c7a0cf26d1d4820b60ef545456b84a7475c Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sat, 8 Feb 2020 15:24:23 +0100 Subject: [PATCH 01/46] track/serato/markers2: Move Serato Markers2 code into serato directory --- CMakeLists.txt | 2 +- build/depends.py | 2 +- .../markers2-data}/bpmlock-enabled.octet-stream | Bin .../markers2-data}/flips.octet-stream | Bin .../markers2-data}/hotcue-00m00s-red.octet-stream | Bin .../markers2-data}/hotcue-00m00s0-blue.octet-stream | Bin .../markers2-data}/hotcue-colors.octet-stream | Bin ...0s0-03m38s4-01m00s0-00m00s1-00m01s0.octet-stream | Bin .../markers2-data}/hotcues-with-names.octet-stream | Bin .../markers2-data}/saved-loops.octet-stream | Bin .../markers2-data}/tracklist-color.octet-stream | Bin .../markers2-data}/very-long-names.octet-stream | 0 src/test/seratomarkers2test.cpp | 4 ++-- .../{seratomarkers2.cpp => serato/markers2.cpp} | 2 +- src/track/{seratomarkers2.h => serato/markers2.h} | 0 src/track/trackinfo.h | 2 +- 16 files changed, 6 insertions(+), 6 deletions(-) rename src/test/{seratomarkers2-data => serato/markers2-data}/bpmlock-enabled.octet-stream (100%) rename src/test/{seratomarkers2-data => serato/markers2-data}/flips.octet-stream (100%) rename src/test/{seratomarkers2-data => serato/markers2-data}/hotcue-00m00s-red.octet-stream (100%) rename src/test/{seratomarkers2-data => serato/markers2-data}/hotcue-00m00s0-blue.octet-stream (100%) rename src/test/{seratomarkers2-data => serato/markers2-data}/hotcue-colors.octet-stream (100%) rename src/test/{seratomarkers2-data => serato/markers2-data}/hotcue-positions-00m00s0-03m38s4-01m00s0-00m00s1-00m01s0.octet-stream (100%) rename src/test/{seratomarkers2-data => serato/markers2-data}/hotcues-with-names.octet-stream (100%) rename src/test/{seratomarkers2-data => serato/markers2-data}/saved-loops.octet-stream (100%) rename src/test/{seratomarkers2-data => serato/markers2-data}/tracklist-color.octet-stream (100%) rename src/test/{seratomarkers2-data => serato/markers2-data}/very-long-names.octet-stream (100%) rename src/track/{seratomarkers2.cpp => serato/markers2.cpp} (99%) rename src/track/{seratomarkers2.h => serato/markers2.h} (100%) diff --git a/CMakeLists.txt b/CMakeLists.txt index b6fe142cc72..d9825619a92 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -510,7 +510,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL src/track/keyutils.cpp src/track/playcounter.cpp src/track/replaygain.cpp - src/track/seratomarkers2.cpp + src/track/serato/markers2.cpp src/track/track.cpp src/track/trackfile.cpp src/track/trackinfo.cpp diff --git a/build/depends.py b/build/depends.py index f558598ac96..28f09e4b8b8 100644 --- a/build/depends.py +++ b/build/depends.py @@ -1205,7 +1205,7 @@ def sources(self, build): "src/track/keyutils.cpp", "src/track/playcounter.cpp", "src/track/replaygain.cpp", - "src/track/seratomarkers2.cpp", + "src/track/serato/markers2.cpp", "src/track/track.cpp", "src/track/globaltrackcache.cpp", "src/track/trackfile.cpp", diff --git a/src/test/seratomarkers2-data/bpmlock-enabled.octet-stream b/src/test/serato/markers2-data/bpmlock-enabled.octet-stream similarity index 100% rename from src/test/seratomarkers2-data/bpmlock-enabled.octet-stream rename to src/test/serato/markers2-data/bpmlock-enabled.octet-stream diff --git a/src/test/seratomarkers2-data/flips.octet-stream b/src/test/serato/markers2-data/flips.octet-stream similarity index 100% rename from src/test/seratomarkers2-data/flips.octet-stream rename to src/test/serato/markers2-data/flips.octet-stream diff --git a/src/test/seratomarkers2-data/hotcue-00m00s-red.octet-stream b/src/test/serato/markers2-data/hotcue-00m00s-red.octet-stream similarity index 100% rename from src/test/seratomarkers2-data/hotcue-00m00s-red.octet-stream rename to src/test/serato/markers2-data/hotcue-00m00s-red.octet-stream diff --git a/src/test/seratomarkers2-data/hotcue-00m00s0-blue.octet-stream b/src/test/serato/markers2-data/hotcue-00m00s0-blue.octet-stream similarity index 100% rename from src/test/seratomarkers2-data/hotcue-00m00s0-blue.octet-stream rename to src/test/serato/markers2-data/hotcue-00m00s0-blue.octet-stream diff --git a/src/test/seratomarkers2-data/hotcue-colors.octet-stream b/src/test/serato/markers2-data/hotcue-colors.octet-stream similarity index 100% rename from src/test/seratomarkers2-data/hotcue-colors.octet-stream rename to src/test/serato/markers2-data/hotcue-colors.octet-stream diff --git a/src/test/seratomarkers2-data/hotcue-positions-00m00s0-03m38s4-01m00s0-00m00s1-00m01s0.octet-stream b/src/test/serato/markers2-data/hotcue-positions-00m00s0-03m38s4-01m00s0-00m00s1-00m01s0.octet-stream similarity index 100% rename from src/test/seratomarkers2-data/hotcue-positions-00m00s0-03m38s4-01m00s0-00m00s1-00m01s0.octet-stream rename to src/test/serato/markers2-data/hotcue-positions-00m00s0-03m38s4-01m00s0-00m00s1-00m01s0.octet-stream diff --git a/src/test/seratomarkers2-data/hotcues-with-names.octet-stream b/src/test/serato/markers2-data/hotcues-with-names.octet-stream similarity index 100% rename from src/test/seratomarkers2-data/hotcues-with-names.octet-stream rename to src/test/serato/markers2-data/hotcues-with-names.octet-stream diff --git a/src/test/seratomarkers2-data/saved-loops.octet-stream b/src/test/serato/markers2-data/saved-loops.octet-stream similarity index 100% rename from src/test/seratomarkers2-data/saved-loops.octet-stream rename to src/test/serato/markers2-data/saved-loops.octet-stream diff --git a/src/test/seratomarkers2-data/tracklist-color.octet-stream b/src/test/serato/markers2-data/tracklist-color.octet-stream similarity index 100% rename from src/test/seratomarkers2-data/tracklist-color.octet-stream rename to src/test/serato/markers2-data/tracklist-color.octet-stream diff --git a/src/test/seratomarkers2-data/very-long-names.octet-stream b/src/test/serato/markers2-data/very-long-names.octet-stream similarity index 100% rename from src/test/seratomarkers2-data/very-long-names.octet-stream rename to src/test/serato/markers2-data/very-long-names.octet-stream diff --git a/src/test/seratomarkers2test.cpp b/src/test/seratomarkers2test.cpp index fc53f11e655..f75d61bf5f8 100644 --- a/src/test/seratomarkers2test.cpp +++ b/src/test/seratomarkers2test.cpp @@ -1,6 +1,6 @@ #include -#include "track/seratomarkers2.h" +#include "track/serato/markers2.h" #include "util/memory.h" #include @@ -213,7 +213,7 @@ TEST_F(SeratoMarkers2Test, ParseLoopEntry) { } TEST_F(SeratoMarkers2Test, ParseMarkers2Data) { - QDir dir("src/test/seratomarkers2-data"); + QDir dir("src/test/serato/markers2-data"); dir.setFilter(QDir::Files); dir.setNameFilters(QStringList() << "*.octet-stream"); diff --git a/src/track/seratomarkers2.cpp b/src/track/serato/markers2.cpp similarity index 99% rename from src/track/seratomarkers2.cpp rename to src/track/serato/markers2.cpp index d08da56c71e..975a5e98844 100644 --- a/src/track/seratomarkers2.cpp +++ b/src/track/serato/markers2.cpp @@ -1,4 +1,4 @@ -#include "track/seratomarkers2.h" +#include "track/serato/markers2.h" #include diff --git a/src/track/seratomarkers2.h b/src/track/serato/markers2.h similarity index 100% rename from src/track/seratomarkers2.h rename to src/track/serato/markers2.h diff --git a/src/track/trackinfo.h b/src/track/trackinfo.h index e322587bd69..1f065a5e0aa 100644 --- a/src/track/trackinfo.h +++ b/src/track/trackinfo.h @@ -7,7 +7,7 @@ #include "track/bpm.h" #include "track/replaygain.h" -#include "track/seratomarkers2.h" +#include "track/serato/markers2.h" #include "util/duration.h" #include "util/macros.h" From 5a2c9acb14d7ad2dab2473573b4f292751f64137 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sat, 8 Feb 2020 15:25:51 +0100 Subject: [PATCH 02/46] track/serato/markers2: Apply clang-format formatting fixes --- src/track/serato/markers2.cpp | 51 +++++----- src/track/serato/markers2.h | 181 ++++++++++++++-------------------- 2 files changed, 95 insertions(+), 137 deletions(-) diff --git a/src/track/serato/markers2.cpp b/src/track/serato/markers2.cpp index 975a5e98844..75bfbba162d 100644 --- a/src/track/serato/markers2.cpp +++ b/src/track/serato/markers2.cpp @@ -4,8 +4,7 @@ namespace mixxx { -SeratoMarkers2EntryPointer SeratoMarkers2BpmlockEntry::parse(const QByteArray &data) -{ +SeratoMarkers2EntryPointer SeratoMarkers2BpmlockEntry::parse(const QByteArray& data) { if (data.length() != 1) { qWarning() << "Parsing SeratoMarkers2BpmlockEntry failed:" << "Length" << data.length() << "!= 1"; @@ -34,11 +33,10 @@ quint32 SeratoMarkers2BpmlockEntry::length() const { return 1; } -SeratoMarkers2EntryPointer SeratoMarkers2ColorEntry::parse(const QByteArray &data) -{ +SeratoMarkers2EntryPointer SeratoMarkers2ColorEntry::parse(const QByteArray& data) { if (data.length() != 4) { qWarning() << "Parsing SeratoMarkers2ColorEntry failed:" - << "Length" << data.length() << "!= 4"; + << "Length" << data.length() << "!= 4"; return nullptr; } @@ -51,8 +49,8 @@ SeratoMarkers2EntryPointer SeratoMarkers2ColorEntry::parse(const QByteArray &dat } QColor color(static_cast(data.at(1)), - static_cast(data.at(2)), - static_cast(data.at(3))); + static_cast(data.at(2)), + static_cast(data.at(3))); SeratoMarkers2ColorEntry* pEntry = new SeratoMarkers2ColorEntry(color); qDebug() << "SeratoMarkers2ColorEntry" << *pEntry; @@ -78,8 +76,7 @@ quint32 SeratoMarkers2ColorEntry::length() const { return 4; } -SeratoMarkers2EntryPointer SeratoMarkers2CueEntry::parse(const QByteArray &data) -{ +SeratoMarkers2EntryPointer SeratoMarkers2CueEntry::parse(const QByteArray& data) { if (data.length() < 13) { qWarning() << "Parsing SeratoMarkers2CueEntry failed:" << "Length" << data.length() << "< 13"; @@ -111,8 +108,8 @@ SeratoMarkers2EntryPointer SeratoMarkers2CueEntry::parse(const QByteArray &data) } QColor color(static_cast(data.at(7)), - static_cast(data.at(8)), - static_cast(data.at(9))); + static_cast(data.at(8)), + static_cast(data.at(9))); // Unknown field(s), make sure it's 0 in case it's a // null-terminated string @@ -169,8 +166,7 @@ quint32 SeratoMarkers2CueEntry::length() const { return 13 + m_label.toUtf8().length(); } -SeratoMarkers2EntryPointer SeratoMarkers2LoopEntry::parse(const QByteArray &data) -{ +SeratoMarkers2EntryPointer SeratoMarkers2LoopEntry::parse(const QByteArray& data) { if (data.length() < 21) { qWarning() << "Parsing SeratoMarkers2LoopEntry failed:" << "Length" << data.length() << "< 21"; @@ -197,13 +193,13 @@ SeratoMarkers2EntryPointer SeratoMarkers2LoopEntry::parse(const QByteArray &data #endif // Unknown field, make sure it contains the expected "default" value if (data.at(10) != '\xff' || - data.at(11) != '\xff' || - data.at(12) != '\xff' || - data.at(13) != '\xff' || - data.at(14) != '\x00' || - data.at(15) != '\x27' || - data.at(16) != '\xaa' || - data.at(17) != '\xe1') { + data.at(11) != '\xff' || + data.at(12) != '\xff' || + data.at(13) != '\xff' || + data.at(14) != '\x00' || + data.at(15) != '\x27' || + data.at(16) != '\xaa' || + data.at(17) != '\xe1') { qWarning() << "Parsing SeratoMarkers2LoopEntry failed:" << "Invalid magic value " << data.mid(10, 16); return nullptr; @@ -285,7 +281,7 @@ bool SeratoMarkers2::parse(SeratoMarkers2* seratoMarkers2, const QByteArray& out int offset = 2; int entryTypeEndPos; - while((entryTypeEndPos = data.indexOf('\x00', offset)) >= 0) { + while ((entryTypeEndPos = data.indexOf('\x00', offset)) >= 0) { // Entry Name QString entryType(data.mid(offset, entryTypeEndPos - offset)); offset = entryTypeEndPos + 1; @@ -314,20 +310,20 @@ bool SeratoMarkers2::parse(SeratoMarkers2* seratoMarkers2, const QByteArray& out // Entry Content SeratoMarkers2EntryPointer pEntry; - if(entryType.compare("BPMLOCK") == 0) { + if (entryType.compare("BPMLOCK") == 0) { pEntry = SeratoMarkers2BpmlockEntry::parse(entryData); - } else if(entryType.compare("COLOR") == 0) { + } else if (entryType.compare("COLOR") == 0) { pEntry = SeratoMarkers2ColorEntry::parse(entryData); - } else if(entryType.compare("CUE") == 0) { + } else if (entryType.compare("CUE") == 0) { pEntry = SeratoMarkers2CueEntry::parse(entryData); - } else if(entryType.compare("LOOP") == 0) { + } else if (entryType.compare("LOOP") == 0) { pEntry = SeratoMarkers2LoopEntry::parse(entryData); } else { pEntry = SeratoMarkers2EntryPointer(new SeratoMarkers2UnknownEntry(entryType, entryData)); qDebug() << "SeratoMarkers2UnknownEntry" << *pEntry; } - if(!pEntry) { + if (!pEntry) { qWarning() << "Parsing SeratoMarkers2 failed:" << "Unable to parse entry of type " << entryType; return false; @@ -359,7 +355,7 @@ QByteArray SeratoMarkers2::data() const { // Hence, we can split the data into blocks of 72 bytes * 3/4 = 54 bytes // and base64-encode them one at a time: int offset = 0; - while(offset < data.size()) { + while (offset < data.size()) { if (offset > 0) { outerData.append('\n'); } @@ -391,5 +387,4 @@ QByteArray SeratoMarkers2::data() const { return outerData.leftJustified(size, '\0'); } - } //namespace mixxx diff --git a/src/track/serato/markers2.h b/src/track/serato/markers2.h index 4081785827c..eb269813344 100644 --- a/src/track/serato/markers2.h +++ b/src/track/serato/markers2.h @@ -1,18 +1,17 @@ #pragma once -#include - -#include #include +#include #include #include +#include #include "util/types.h" namespace mixxx { class SeratoMarkers2Entry { -public: + public: virtual ~SeratoMarkers2Entry() = default; virtual QString type() const = 0; @@ -26,28 +25,25 @@ class SeratoMarkers2Entry { typedef std::shared_ptr SeratoMarkers2EntryPointer; -inline -bool operator==(const SeratoMarkers2Entry& lhs, const SeratoMarkers2Entry& rhs) { +inline bool operator==(const SeratoMarkers2Entry& lhs, const SeratoMarkers2Entry& rhs) { return (lhs.type() == rhs.type()) && (lhs.data() == rhs.data()); } -inline -bool operator!=(const SeratoMarkers2Entry& lhs, const SeratoMarkers2Entry& rhs) { +inline bool operator!=(const SeratoMarkers2Entry& lhs, const SeratoMarkers2Entry& rhs) { return !(lhs == rhs); } -inline -QDebug operator<<(QDebug dbg, const SeratoMarkers2Entry& arg) { +inline QDebug operator<<(QDebug dbg, const SeratoMarkers2Entry& arg) { return dbg << "type =" << arg.type() << "data =" << arg.data() - << "(" << "length =" << arg.length() << ")"; + << "(" + << "length =" << arg.length() << ")"; } class SeratoMarkers2UnknownEntry : public SeratoMarkers2Entry { -public: + public: SeratoMarkers2UnknownEntry(QString type, QByteArray data) - : m_type(std::move(type)) - , m_data(std::move(data)) { + : m_type(std::move(type)), m_data(std::move(data)) { } ~SeratoMarkers2UnknownEntry() override = default; @@ -59,22 +55,22 @@ class SeratoMarkers2UnknownEntry : public SeratoMarkers2Entry { return m_data; } -private: + private: QString m_type; QByteArray m_data; }; class SeratoMarkers2BpmlockEntry : public SeratoMarkers2Entry { -public: + public: SeratoMarkers2BpmlockEntry(bool locked) - : m_locked(locked) { + : m_locked(locked) { } SeratoMarkers2BpmlockEntry() - : m_locked(false) { + : m_locked(false) { } - static SeratoMarkers2EntryPointer parse(const QByteArray &data); + static SeratoMarkers2EntryPointer parse(const QByteArray& data); QString type() const override { return "BPMLOCK"; @@ -82,7 +78,7 @@ class SeratoMarkers2BpmlockEntry : public SeratoMarkers2Entry { QByteArray data() const override; - bool isLocked() const { + bool isLocked() const { return m_locked; } @@ -92,38 +88,35 @@ class SeratoMarkers2BpmlockEntry : public SeratoMarkers2Entry { quint32 length() const override; -private: + private: bool m_locked; }; -inline -bool operator==(const SeratoMarkers2BpmlockEntry& lhs, - const SeratoMarkers2BpmlockEntry& rhs) { +inline bool operator==(const SeratoMarkers2BpmlockEntry& lhs, + const SeratoMarkers2BpmlockEntry& rhs) { return (lhs.isLocked() == rhs.isLocked()); } -inline -bool operator!=(const SeratoMarkers2BpmlockEntry& lhs, - const SeratoMarkers2BpmlockEntry& rhs) { +inline bool operator!=(const SeratoMarkers2BpmlockEntry& lhs, + const SeratoMarkers2BpmlockEntry& rhs) { return !(lhs == rhs); } -inline -QDebug operator<<(QDebug dbg, const SeratoMarkers2BpmlockEntry& arg) { +inline QDebug operator<<(QDebug dbg, const SeratoMarkers2BpmlockEntry& arg) { return dbg << "locked =" << arg.isLocked(); } class SeratoMarkers2ColorEntry : public SeratoMarkers2Entry { -public: + public: SeratoMarkers2ColorEntry(QColor color) - : m_color(color) { + : m_color(color) { } SeratoMarkers2ColorEntry() - : m_color(QColor()) { + : m_color(QColor()) { } - static SeratoMarkers2EntryPointer parse(const QByteArray &data); + static SeratoMarkers2EntryPointer parse(const QByteArray& data); QString type() const override { return "COLOR"; @@ -141,45 +134,35 @@ class SeratoMarkers2ColorEntry : public SeratoMarkers2Entry { quint32 length() const override; -private: + private: QColor m_color; }; -inline -bool operator==(const SeratoMarkers2ColorEntry& lhs, - const SeratoMarkers2ColorEntry& rhs) { +inline bool operator==(const SeratoMarkers2ColorEntry& lhs, + const SeratoMarkers2ColorEntry& rhs) { return (lhs.getColor() == rhs.getColor()); } -inline -bool operator!=(const SeratoMarkers2ColorEntry& lhs, - const SeratoMarkers2ColorEntry& rhs) { +inline bool operator!=(const SeratoMarkers2ColorEntry& lhs, + const SeratoMarkers2ColorEntry& rhs) { return !(lhs == rhs); } -inline -QDebug operator<<(QDebug dbg, const SeratoMarkers2ColorEntry& arg) { +inline QDebug operator<<(QDebug dbg, const SeratoMarkers2ColorEntry& arg) { return dbg << "color =" << arg.getColor(); } class SeratoMarkers2CueEntry : public SeratoMarkers2Entry { -public: - SeratoMarkers2CueEntry(quint8 index, quint32 position, QColor color, - QString label) - : m_index(index) - , m_position(position) - , m_color(color) - , m_label(label) { + public: + SeratoMarkers2CueEntry(quint8 index, quint32 position, QColor color, QString label) + : m_index(index), m_position(position), m_color(color), m_label(label) { } SeratoMarkers2CueEntry() - : m_index(0) - , m_position(0) - , m_color(QColor()) - , m_label(QString("")) { + : m_index(0), m_position(0), m_color(QColor()), m_label(QString("")) { } - static SeratoMarkers2EntryPointer parse(const QByteArray &data); + static SeratoMarkers2EntryPointer parse(const QByteArray& data); QString type() const override { return "CUE"; @@ -187,7 +170,7 @@ class SeratoMarkers2CueEntry : public SeratoMarkers2Entry { QByteArray data() const override; - quint8 getIndex() const { + quint8 getIndex() const { return m_index; } @@ -211,7 +194,7 @@ class SeratoMarkers2CueEntry : public SeratoMarkers2Entry { m_color = color; } - QString getLabel() const { + QString getLabel() const { return m_label; } @@ -221,30 +204,27 @@ class SeratoMarkers2CueEntry : public SeratoMarkers2Entry { quint32 length() const override; -private: + private: quint8 m_index; quint32 m_position; QColor m_color; QString m_label; }; -inline -bool operator==(const SeratoMarkers2CueEntry& lhs, - const SeratoMarkers2CueEntry& rhs) { +inline bool operator==(const SeratoMarkers2CueEntry& lhs, + const SeratoMarkers2CueEntry& rhs) { return (lhs.getIndex() == rhs.getIndex()) && - (lhs.getPosition() == rhs.getPosition()) && - (lhs.getColor() == rhs.getColor()) && - (lhs.getLabel() == rhs.getLabel()); + (lhs.getPosition() == rhs.getPosition()) && + (lhs.getColor() == rhs.getColor()) && + (lhs.getLabel() == rhs.getLabel()); } -inline -bool operator!=(const SeratoMarkers2CueEntry& lhs, - const SeratoMarkers2CueEntry& rhs) { +inline bool operator!=(const SeratoMarkers2CueEntry& lhs, + const SeratoMarkers2CueEntry& rhs) { return !(lhs == rhs); } -inline -QDebug operator<<(QDebug dbg, const SeratoMarkers2CueEntry& arg) { +inline QDebug operator<<(QDebug dbg, const SeratoMarkers2CueEntry& arg) { return dbg << "index =" << arg.getIndex() << "/" << "position =" << arg.getPosition() << "/" << "color =" << arg.getColor() << "/" @@ -252,26 +232,16 @@ QDebug operator<<(QDebug dbg, const SeratoMarkers2CueEntry& arg) { } class SeratoMarkers2LoopEntry : public SeratoMarkers2Entry { -public: - SeratoMarkers2LoopEntry(quint8 index, quint32 startposition, - quint32 endposition, bool locked, - QString label) - : m_index(index) - , m_startposition(startposition) - , m_endposition(endposition) - , m_locked(locked) - , m_label(label) { + public: + SeratoMarkers2LoopEntry(quint8 index, quint32 startposition, quint32 endposition, bool locked, QString label) + : m_index(index), m_startposition(startposition), m_endposition(endposition), m_locked(locked), m_label(label) { } SeratoMarkers2LoopEntry() - : m_index(0) - , m_startposition(0) - , m_endposition(0) - , m_locked(false) - , m_label(QString("")) { + : m_index(0), m_startposition(0), m_endposition(0), m_locked(false), m_label(QString("")) { } - static SeratoMarkers2EntryPointer parse(const QByteArray &data); + static SeratoMarkers2EntryPointer parse(const QByteArray& data); QString type() const override { return "LOOP"; @@ -279,7 +249,7 @@ class SeratoMarkers2LoopEntry : public SeratoMarkers2Entry { QByteArray data() const override; - quint8 getIndex() const { + quint8 getIndex() const { return m_index; } @@ -311,7 +281,7 @@ class SeratoMarkers2LoopEntry : public SeratoMarkers2Entry { m_locked = locked; } - QString getLabel() const { + QString getLabel() const { return m_label; } @@ -321,7 +291,7 @@ class SeratoMarkers2LoopEntry : public SeratoMarkers2Entry { quint32 length() const override; -private: + private: quint8 m_index; quint32 m_startposition; quint32 m_endposition; @@ -329,24 +299,21 @@ class SeratoMarkers2LoopEntry : public SeratoMarkers2Entry { QString m_label; }; -inline -bool operator==(const SeratoMarkers2LoopEntry& lhs, - const SeratoMarkers2LoopEntry& rhs) { +inline bool operator==(const SeratoMarkers2LoopEntry& lhs, + const SeratoMarkers2LoopEntry& rhs) { return (lhs.getIndex() == rhs.getIndex()) && - (lhs.getStartPosition() == rhs.getStartPosition()) && - (lhs.getEndPosition() == rhs.getEndPosition()) && - (lhs.isLocked() == rhs.isLocked()) && - (lhs.getLabel() == rhs.getLabel()); + (lhs.getStartPosition() == rhs.getStartPosition()) && + (lhs.getEndPosition() == rhs.getEndPosition()) && + (lhs.isLocked() == rhs.isLocked()) && + (lhs.getLabel() == rhs.getLabel()); } -inline -bool operator!=(const SeratoMarkers2LoopEntry& lhs, - const SeratoMarkers2LoopEntry& rhs) { +inline bool operator!=(const SeratoMarkers2LoopEntry& lhs, + const SeratoMarkers2LoopEntry& rhs) { return !(lhs == rhs); } -inline -QDebug operator<<(QDebug dbg, const SeratoMarkers2LoopEntry& arg) { +inline QDebug operator<<(QDebug dbg, const SeratoMarkers2LoopEntry& arg) { return dbg << "index =" << arg.getIndex() << "/" << "startposition =" << arg.getStartPosition() << "/" << "endposition =" << arg.getEndPosition() << "/" @@ -364,12 +331,11 @@ QDebug operator<<(QDebug dbg, const SeratoMarkers2LoopEntry& arg) { // https://github.com/Holzhaus/serato-tags/blob/master/docs/serato_markers2.md // class SeratoMarkers2 final { -public: + public: SeratoMarkers2() = default; explicit SeratoMarkers2( QList> entries) - : m_allocatedSize(0) - , m_entries(std::move(entries)) { + : m_allocatedSize(0), m_entries(std::move(entries)) { } // Parsing and formatting of gain values according to the @@ -398,27 +364,24 @@ class SeratoMarkers2 final { m_entries = std::move(entries); } -private: + private: int m_allocatedSize; QList> m_entries; }; -inline -bool operator==(const SeratoMarkers2& lhs, const SeratoMarkers2& rhs) { +inline bool operator==(const SeratoMarkers2& lhs, const SeratoMarkers2& rhs) { return (lhs.getEntries() == rhs.getEntries()); } -inline -bool operator!=(const SeratoMarkers2& lhs, const SeratoMarkers2& rhs) { +inline bool operator!=(const SeratoMarkers2& lhs, const SeratoMarkers2& rhs) { return !(lhs == rhs); } -inline -QDebug operator<<(QDebug dbg, const SeratoMarkers2& arg) { +inline QDebug operator<<(QDebug dbg, const SeratoMarkers2& arg) { return dbg << "entries =" << arg.getEntries().length(); } -} +} // namespace mixxx Q_DECLARE_TYPEINFO(mixxx::SeratoMarkers2, Q_MOVABLE_TYPE); Q_DECLARE_METATYPE(mixxx::SeratoMarkers2) From 054ec17cf9d2abe79a7bf8b599ee1792c76bbb9c Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sat, 8 Feb 2020 16:50:07 +0100 Subject: [PATCH 03/46] track/serato/markers: Add parser for "Serato Markers_" tag --- CMakeLists.txt | 1 + build/depends.py | 1 + src/track/serato/markers.cpp | 179 +++++++++++++++++++++++++++++++++++ src/track/serato/markers.h | 145 ++++++++++++++++++++++++++++ 4 files changed, 326 insertions(+) create mode 100644 src/track/serato/markers.cpp create mode 100644 src/track/serato/markers.h diff --git a/CMakeLists.txt b/CMakeLists.txt index d9825619a92..c6987e7b430 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -510,6 +510,7 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL src/track/keyutils.cpp src/track/playcounter.cpp src/track/replaygain.cpp + src/track/serato/markers.cpp src/track/serato/markers2.cpp src/track/track.cpp src/track/trackfile.cpp diff --git a/build/depends.py b/build/depends.py index 28f09e4b8b8..226037eac39 100644 --- a/build/depends.py +++ b/build/depends.py @@ -1205,6 +1205,7 @@ def sources(self, build): "src/track/keyutils.cpp", "src/track/playcounter.cpp", "src/track/replaygain.cpp", + "src/track/serato/markers.cpp", "src/track/serato/markers2.cpp", "src/track/track.cpp", "src/track/globaltrackcache.cpp", diff --git a/src/track/serato/markers.cpp b/src/track/serato/markers.cpp new file mode 100644 index 00000000000..49bb5b3ce47 --- /dev/null +++ b/src/track/serato/markers.cpp @@ -0,0 +1,179 @@ +#include "track/serato/markers.h" + +#include +#include + +namespace { + +const int kEntrySize = 22; +const char* kVersion = "\x02\x05"; +const int kVersionSize = 2; + +inline +quint32 bytesToUInt32(const QByteArray& data) { + DEBUG_ASSERT(data.size() == sizeof(quint32)); +#if QT_VERSION >= QT_VERSION_CHECK(5, 12, 0) + return qFromBigEndian(data.constData()); +#else + return qFromBigEndian( + reinterpret_cast(data.constData())); +#endif +} + +QRgb colorToRgb(quint8 w, quint8 x, quint8 y, quint8 z) { + quint8 b = (z & 0x7F) | ((y & 0x01) << 7); + quint8 g = ((y & 0x7F) >> 1) | ((x & 0x03) << 6); + quint8 r = ((x & 0x7F) >> 2) | ((w & 0x07) << 5); + return QRgb(r << 16) | (g << 8) | b; +} + +QRgb colorToRgb(const QByteArray& data) { + DEBUG_ASSERT(data.size() == 4); + return colorToRgb(data.at(0), data.at(1), data.at(2), data.at(3)); +} + +quint32 colorFromRgb(quint8 r, quint8 g, quint8 b) { + quint8 z = b & 0x7F; + quint8 y = ((b >> 7) | (g << 1)) & 0x7F; + quint8 x = ((g >> 6) | (r << 2)) & 0x7F; + quint8 w = (r >> 5); + return (w << 24) | (x << 16) | (y << 8) | z; +} + +quint32 colorFromRgb(QRgb rgb) { + return colorFromRgb(qRed(rgb), qGreen(rgb), qBlue(rgb)); +} + +} + +namespace mixxx { + +QByteArray SeratoMarkersEntry::data() const { + QByteArray data; + data.resize(kEntrySize); + + QDataStream stream(&data, QIODevice::WriteOnly); + stream.setVersion(QDataStream::Qt_5_0); + stream.setByteOrder(QDataStream::BigEndian); + stream << (quint8)(m_isSet ? '\x00' : '\x7F') + << (quint32)((m_startPosition == -1) ? 0x7F7F7F7F : m_startPosition) + << (quint8)((!m_isSet || m_type == 1) ? '\x7F' : m_isEnabled) + << (quint32)((m_endPosition == -1) ? 0x7F7F7F7F : m_endPosition); + stream.writeRawData("\x00\x7F\x7F\x7F\x7F\x7F", 6); + stream << (quint32)colorFromRgb(m_color) + << (quint8)m_type + << (quint8)m_isLocked; + return data; +} + +SeratoMarkersEntry* SeratoMarkersEntry::parse(const QByteArray& data) { + if (data.length() != kEntrySize) { + qWarning() << "Parsing SeratoMarkersEntry failed:" + << "Length" << data.length() << "!=" << kEntrySize; + return nullptr; + } + + const bool isSet = (data.at(0) == '\x7F') ? false : true; + const quint32 uStartPosition = bytesToUInt32(data.mid(1, 4)); + if (uStartPosition > 0x7FFFFFFF) { + qWarning() << "Parsing SeratoMarkersEntry failed:" + << "startPosition > 0x7FFFFFF"; + return nullptr; + } + const int startPosition = (uStartPosition == 0x7F7F7F7F) ? -1 : static_cast(uStartPosition); + + const bool isEnabled = static_cast(data.at(5)); + + const quint32 uEndPosition = bytesToUInt32(data.mid(6, 4)); + if (uEndPosition > 0x7FFFFFFF) { + qWarning() << "Parsing SeratoMarkersEntry failed:" + << "endPosition > 0x7FFFFFF"; + return nullptr; + } + const int endPosition = (uEndPosition == 0x7F7F7F7F) ? -1 : static_cast(uEndPosition); + + if (data.mid(10, 6).compare("\x00\x7F\x7F\x7F\x7F") == 0) { + qWarning() << "Parsing SeratoMarkersEntry failed:" + << "Unexpected value at offset 10"; + return nullptr; + } + + const QRgb color = colorToRgb(data.mid(16, 4)); + const quint8 type = data.at(20); + const bool isLocked = static_cast(data.at(21)); + + SeratoMarkersEntry* pEntry = new SeratoMarkersEntry( + isSet, + startPosition, + isEnabled, + endPosition, + color, + type, + isLocked); + qDebug() << "SeratoMarkersEntry" << *pEntry; + return pEntry; +} + +bool SeratoMarkers::parse(SeratoMarkers* seratoMarkers, const QByteArray& data) { + if (!data.startsWith(kVersion)) { + qWarning() << "Parsing SeratoMarkers_ failed:" + << "Unknown Serato Markers_ tag version"; + return false; + } + + quint32 numEntries = bytesToUInt32(data.mid(2, 4)); + QList> entries; + + int offset = 6; + for (quint32 i = 0; i < numEntries; i++) { + if (data.size() <= (offset + kEntrySize)) { + qWarning() << "Parsing SeratoMarkers_ failed:" + << "Incomplete entry" << data.mid(offset); + return false; + } + + QByteArray entryData = data.mid(offset, kEntrySize); + SeratoMarkersEntryPointer pEntry = SeratoMarkersEntryPointer( + SeratoMarkersEntry::parse(entryData)); + if (!pEntry) { + qWarning() << "Parsing SeratoMarkers_ failed:" + << "Unable to parse entry!"; + return false; + } + entries.append(pEntry); + + offset += kEntrySize; + } + + if (data.mid(offset).size() != 4) { + qWarning() << "Parsing SeratoMarkers_ failed:" + << "Invalid footer" << data.mid(offset); + return false; + } + + QRgb color = colorToRgb(data.mid(offset)); + + seratoMarkers->setEntries(entries); + seratoMarkers->setTrackColor(color); + + return true; +} + +QByteArray SeratoMarkers::data() const { + QByteArray data; + data.resize(kVersionSize + 2 * sizeof(quint32) + kEntrySize * m_entries.size()); + + QDataStream stream(&data, QIODevice::WriteOnly); + stream.setVersion(QDataStream::Qt_5_0); + stream.setByteOrder(QDataStream::BigEndian); + stream.writeRawData(kVersion, kVersionSize); + stream << m_entries.size(); + for (int i = 0; i < m_entries.size(); i++) { + SeratoMarkersEntryPointer pEntry = m_entries.at(i); + stream.writeRawData(pEntry->data(), kEntrySize); + } + stream << colorFromRgb(m_trackColor); + return data; +} + +} //namespace mixxx diff --git a/src/track/serato/markers.h b/src/track/serato/markers.h new file mode 100644 index 00000000000..c4f66758f99 --- /dev/null +++ b/src/track/serato/markers.h @@ -0,0 +1,145 @@ +#pragma once + +#include +#include +#include +#include +#include + +#include "util/types.h" + +namespace mixxx { + +class SeratoMarkersEntry { + public: + SeratoMarkersEntry(bool isSet, int startPosition, bool isEnabled, int endPosition, QRgb color, int type, bool isLocked) + : m_color(color), m_isEnabled(isEnabled), m_isLocked(isLocked), m_isSet(isSet), m_endPosition(endPosition), m_startPosition(startPosition), m_type(type) { + } + ~SeratoMarkersEntry() = default; + + QByteArray data() const; + static SeratoMarkersEntry* parse(const QByteArray& data); + + int type() const { + return m_type; + } + + QRgb getColor() const { + return m_color; + } + + bool isEnabled() const { + return m_isEnabled; + } + + bool isLocked() const { + return m_isLocked; + } + + bool isSet() const { + return m_isSet; + } + + int getEndPosition() const { + return m_endPosition; + } + + int getStartPosition() const { + return m_startPosition; + } + + private: + QRgb m_color; + bool m_isEnabled; + bool m_isLocked; + bool m_isSet; + int m_endPosition; + int m_startPosition; + int m_type; +}; + +typedef std::shared_ptr SeratoMarkersEntryPointer; + +inline bool operator==(const SeratoMarkersEntry& lhs, const SeratoMarkersEntry& rhs) { + return (lhs.data() == rhs.data()); +} + +inline bool operator!=(const SeratoMarkersEntry& lhs, const SeratoMarkersEntry& rhs) { + return !(lhs == rhs); +} + +inline QDebug operator<<(QDebug dbg, const SeratoMarkersEntry& arg) { + return dbg << "type =" << arg.type() + << "data =" << arg.data(); +} + +// DTO for storing information from the SeratoMarkers_ tags used by the Serato +// DJ Pro software. +// +// Parsing & Formatting +// -------------------- +// This class includes functions for formatting and parsing SeratoMarkers_ +// metadata according to the specification: +// https://github.com/Holzhaus/serato-tags/blob/master/docs/serato_markers_.md +// +class SeratoMarkers final { + public: + SeratoMarkers() = default; + explicit SeratoMarkers( + QList> entries) + : m_allocatedSize(0), m_entries(std::move(entries)) { + } + + static bool parse(SeratoMarkers* seratoMarkers, const QByteArray& data); + + QByteArray data() const; + + int getAllocatedSize() const { + return m_allocatedSize; + } + + void setAllocatedSize(int size) { + DEBUG_ASSERT(size >= 0); + m_allocatedSize = size; + } + + bool isEmpty() const { + return m_entries.isEmpty(); + } + + const QList>& getEntries() const { + return m_entries; + } + void setEntries(QList> entries) { + m_entries = std::move(entries); + } + + QRgb getTrackColor() const { + return m_trackColor; + } + void setTrackColor(QRgb color) { + m_trackColor = color; + } + + private: + int m_allocatedSize; + QList> m_entries; + QRgb m_trackColor; +}; + +inline bool operator==(const SeratoMarkers& lhs, const SeratoMarkers& rhs) { + return (lhs.getEntries() == rhs.getEntries()); +} + +inline bool operator!=(const SeratoMarkers& lhs, const SeratoMarkers& rhs) { + return !(lhs == rhs); +} + +inline QDebug operator<<(QDebug dbg, const SeratoMarkers& arg) { + return dbg << "entries =" << arg.getEntries().length(); +} + +} // namespace mixxx + +Q_DECLARE_TYPEINFO(mixxx::SeratoMarkers, Q_MOVABLE_TYPE); +Q_DECLARE_METATYPE(mixxx::SeratoMarkers) From ed9bb3860a0b7ddd2789812e7e9ae9bc48177833 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sun, 9 Feb 2020 23:17:32 +0100 Subject: [PATCH 04/46] tests/serato: Move Serato test data into data directory --- .../markers2}/bpmlock-enabled.octet-stream | Bin .../markers2}/flips.octet-stream | Bin .../markers2}/hotcue-00m00s-red.octet-stream | Bin .../markers2}/hotcue-00m00s0-blue.octet-stream | Bin .../markers2}/hotcue-colors.octet-stream | Bin ...0s0-03m38s4-01m00s0-00m00s1-00m01s0.octet-stream | Bin .../markers2}/hotcues-with-names.octet-stream | Bin .../markers2}/saved-loops.octet-stream | Bin .../markers2}/tracklist-color.octet-stream | Bin .../markers2}/very-long-names.octet-stream | 0 src/test/seratomarkers2test.cpp | 2 +- 11 files changed, 1 insertion(+), 1 deletion(-) rename src/test/serato/{markers2-data => data/markers2}/bpmlock-enabled.octet-stream (100%) rename src/test/serato/{markers2-data => data/markers2}/flips.octet-stream (100%) rename src/test/serato/{markers2-data => data/markers2}/hotcue-00m00s-red.octet-stream (100%) rename src/test/serato/{markers2-data => data/markers2}/hotcue-00m00s0-blue.octet-stream (100%) rename src/test/serato/{markers2-data => data/markers2}/hotcue-colors.octet-stream (100%) rename src/test/serato/{markers2-data => data/markers2}/hotcue-positions-00m00s0-03m38s4-01m00s0-00m00s1-00m01s0.octet-stream (100%) rename src/test/serato/{markers2-data => data/markers2}/hotcues-with-names.octet-stream (100%) rename src/test/serato/{markers2-data => data/markers2}/saved-loops.octet-stream (100%) rename src/test/serato/{markers2-data => data/markers2}/tracklist-color.octet-stream (100%) rename src/test/serato/{markers2-data => data/markers2}/very-long-names.octet-stream (100%) diff --git a/src/test/serato/markers2-data/bpmlock-enabled.octet-stream b/src/test/serato/data/markers2/bpmlock-enabled.octet-stream similarity index 100% rename from src/test/serato/markers2-data/bpmlock-enabled.octet-stream rename to src/test/serato/data/markers2/bpmlock-enabled.octet-stream diff --git a/src/test/serato/markers2-data/flips.octet-stream b/src/test/serato/data/markers2/flips.octet-stream similarity index 100% rename from src/test/serato/markers2-data/flips.octet-stream rename to src/test/serato/data/markers2/flips.octet-stream diff --git a/src/test/serato/markers2-data/hotcue-00m00s-red.octet-stream b/src/test/serato/data/markers2/hotcue-00m00s-red.octet-stream similarity index 100% rename from src/test/serato/markers2-data/hotcue-00m00s-red.octet-stream rename to src/test/serato/data/markers2/hotcue-00m00s-red.octet-stream diff --git a/src/test/serato/markers2-data/hotcue-00m00s0-blue.octet-stream b/src/test/serato/data/markers2/hotcue-00m00s0-blue.octet-stream similarity index 100% rename from src/test/serato/markers2-data/hotcue-00m00s0-blue.octet-stream rename to src/test/serato/data/markers2/hotcue-00m00s0-blue.octet-stream diff --git a/src/test/serato/markers2-data/hotcue-colors.octet-stream b/src/test/serato/data/markers2/hotcue-colors.octet-stream similarity index 100% rename from src/test/serato/markers2-data/hotcue-colors.octet-stream rename to src/test/serato/data/markers2/hotcue-colors.octet-stream diff --git a/src/test/serato/markers2-data/hotcue-positions-00m00s0-03m38s4-01m00s0-00m00s1-00m01s0.octet-stream b/src/test/serato/data/markers2/hotcue-positions-00m00s0-03m38s4-01m00s0-00m00s1-00m01s0.octet-stream similarity index 100% rename from src/test/serato/markers2-data/hotcue-positions-00m00s0-03m38s4-01m00s0-00m00s1-00m01s0.octet-stream rename to src/test/serato/data/markers2/hotcue-positions-00m00s0-03m38s4-01m00s0-00m00s1-00m01s0.octet-stream diff --git a/src/test/serato/markers2-data/hotcues-with-names.octet-stream b/src/test/serato/data/markers2/hotcues-with-names.octet-stream similarity index 100% rename from src/test/serato/markers2-data/hotcues-with-names.octet-stream rename to src/test/serato/data/markers2/hotcues-with-names.octet-stream diff --git a/src/test/serato/markers2-data/saved-loops.octet-stream b/src/test/serato/data/markers2/saved-loops.octet-stream similarity index 100% rename from src/test/serato/markers2-data/saved-loops.octet-stream rename to src/test/serato/data/markers2/saved-loops.octet-stream diff --git a/src/test/serato/markers2-data/tracklist-color.octet-stream b/src/test/serato/data/markers2/tracklist-color.octet-stream similarity index 100% rename from src/test/serato/markers2-data/tracklist-color.octet-stream rename to src/test/serato/data/markers2/tracklist-color.octet-stream diff --git a/src/test/serato/markers2-data/very-long-names.octet-stream b/src/test/serato/data/markers2/very-long-names.octet-stream similarity index 100% rename from src/test/serato/markers2-data/very-long-names.octet-stream rename to src/test/serato/data/markers2/very-long-names.octet-stream diff --git a/src/test/seratomarkers2test.cpp b/src/test/seratomarkers2test.cpp index f75d61bf5f8..71b0811d2ea 100644 --- a/src/test/seratomarkers2test.cpp +++ b/src/test/seratomarkers2test.cpp @@ -213,7 +213,7 @@ TEST_F(SeratoMarkers2Test, ParseLoopEntry) { } TEST_F(SeratoMarkers2Test, ParseMarkers2Data) { - QDir dir("src/test/serato/markers2-data"); + QDir dir("src/test/serato/data/markers2"); dir.setFilter(QDir::Files); dir.setNameFilters(QStringList() << "*.octet-stream"); From 5147df747cd7a9ccb2551bc399a0dcdb90a66839 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sun, 9 Feb 2020 23:20:47 +0100 Subject: [PATCH 05/46] tests: Add tests for "Serato Markers_" parser --- CMakeLists.txt | 1 + .../data/markers_/analyzed.octet-stream | Bin 0 -> 318 bytes .../markers_/bpmlock-enabled.octet-stream | Bin 0 -> 318 bytes .../serato/data/markers_/flips.octet-stream | Bin 0 -> 318 bytes .../markers_/hotcue-00m00s-red.octet-stream | Bin 0 -> 318 bytes .../markers_/hotcue-00m00s0-blue.octet-stream | Bin 0 -> 318 bytes .../data/markers_/hotcue-colors.octet-stream | Bin 0 -> 318 bytes ...m38s4-01m00s0-00m00s1-00m01s0.octet-stream | Bin 0 -> 318 bytes .../markers_/hotcues-with-names.octet-stream | Bin 0 -> 318 bytes .../data/markers_/saved-loops.octet-stream | Bin 0 -> 318 bytes .../markers_/tracklist-color.octet-stream | Bin 0 -> 318 bytes .../markers_/very-long-names.octet-stream | Bin 0 -> 318 bytes src/test/seratomarkerstest.cpp | 76 ++++++++++++++++++ 13 files changed, 77 insertions(+) create mode 100644 src/test/serato/data/markers_/analyzed.octet-stream create mode 100644 src/test/serato/data/markers_/bpmlock-enabled.octet-stream create mode 100644 src/test/serato/data/markers_/flips.octet-stream create mode 100644 src/test/serato/data/markers_/hotcue-00m00s-red.octet-stream create mode 100644 src/test/serato/data/markers_/hotcue-00m00s0-blue.octet-stream create mode 100644 src/test/serato/data/markers_/hotcue-colors.octet-stream create mode 100644 src/test/serato/data/markers_/hotcue-positions-00m00s0-03m38s4-01m00s0-00m00s1-00m01s0.octet-stream create mode 100644 src/test/serato/data/markers_/hotcues-with-names.octet-stream create mode 100644 src/test/serato/data/markers_/saved-loops.octet-stream create mode 100644 src/test/serato/data/markers_/tracklist-color.octet-stream create mode 100644 src/test/serato/data/markers_/very-long-names.octet-stream create mode 100644 src/test/seratomarkerstest.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index c6987e7b430..a84d5438741 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -962,6 +962,7 @@ add_executable(mixxx-test src/test/sampleutiltest.cpp src/test/schemamanager_test.cpp src/test/searchqueryparsertest.cpp + src/test/seratomarkerstest.cpp src/test/seratomarkers2test.cpp src/test/signalpathtest.cpp src/test/skincontext_test.cpp diff --git a/src/test/serato/data/markers_/analyzed.octet-stream b/src/test/serato/data/markers_/analyzed.octet-stream new file mode 100644 index 0000000000000000000000000000000000000000..c50e827d9a62a608b4c5733b78ee5d05d2607ff7 GIT binary patch literal 318 kcmZQ#Wnf_7tA_&yFbgOK1aQH6+KVz{wQVqqvIBz$0QNd*;{X5v literal 0 HcmV?d00001 diff --git a/src/test/serato/data/markers_/bpmlock-enabled.octet-stream b/src/test/serato/data/markers_/bpmlock-enabled.octet-stream new file mode 100644 index 0000000000000000000000000000000000000000..c50e827d9a62a608b4c5733b78ee5d05d2607ff7 GIT binary patch literal 318 kcmZQ#Wnf_7tA_&yFbgOK1aQH6+KVz{wQVqqvIBz$0QNd*;{X5v literal 0 HcmV?d00001 diff --git a/src/test/serato/data/markers_/flips.octet-stream b/src/test/serato/data/markers_/flips.octet-stream new file mode 100644 index 0000000000000000000000000000000000000000..222c3677fa384d1d0ccbb352c6e325566969a884 GIT binary patch literal 318 zcmZQ#Wnf_717c;ldJtd$lWYbI42(cQMpaK_Q6mAcD6>{Jk|4&JV literal 0 HcmV?d00001 diff --git a/src/test/serato/data/markers_/hotcue-00m00s0-blue.octet-stream b/src/test/serato/data/markers_/hotcue-00m00s0-blue.octet-stream new file mode 100644 index 0000000000000000000000000000000000000000..4e8468b065d73514d333e0e723cb5f5393e2875d GIT binary patch literal 318 qcmZQ#Wnf_717Ze-dJuq;42(XE5GIN!Tp86xnX%e7m_^xvVE_P5aA&pv literal 0 HcmV?d00001 diff --git a/src/test/serato/data/markers_/hotcue-colors.octet-stream b/src/test/serato/data/markers_/hotcue-colors.octet-stream new file mode 100644 index 0000000000000000000000000000000000000000..38d25788389cef8711795e9d1ba18f3ab45de209 GIT binary patch literal 318 zcmZQ#Wnf_717Ze-dJuq+Yz7Prj9@_&Q6m8?qCk~CSkxLzU{lKs5(Qg?WE;pL<{>A_ H4)g&4OKV=Z literal 0 HcmV?d00001 diff --git a/src/test/serato/data/markers_/hotcue-positions-00m00s0-03m38s4-01m00s0-00m00s1-00m01s0.octet-stream b/src/test/serato/data/markers_/hotcue-positions-00m00s0-03m38s4-01m00s0-00m00s1-00m01s0.octet-stream new file mode 100644 index 0000000000000000000000000000000000000000..efc3d8c895ad0612364198731a45bedc6ba848f2 GIT binary patch literal 318 zcmZQ#Wnf_717Ze-dJuq+Yz7Prj6fk?tq5dMBLT1|b4Uu3C{U#jNG$_H4w5LFu>@Ex a1A94=C<8M{6fA%&3e?LyWV^4XJ8OegP6-G7n;Zn6GRndgb5R&7OQ)iX(q}J G3?2Zozgjo| literal 0 HcmV?d00001 diff --git a/src/test/serato/data/markers_/tracklist-color.octet-stream b/src/test/serato/data/markers_/tracklist-color.octet-stream new file mode 100644 index 0000000000000000000000000000000000000000..9b3e62085001584bd1b244b08823d8b5f8049661 GIT binary patch literal 318 lcmZQ#Wnf_7tA_&yFbgOK1aQH6+KVz{wQVqqvZU8b0s!@ZX<`5X literal 0 HcmV?d00001 diff --git a/src/test/serato/data/markers_/very-long-names.octet-stream b/src/test/serato/data/markers_/very-long-names.octet-stream new file mode 100644 index 0000000000000000000000000000000000000000..92e7638c89e0a95f3a7ad030896824332c291f3b GIT binary patch literal 318 zcmZQ#Wnf_717Ze-dJuq+Yz7Prj9@_&Q6m8?qCk~CSkxLzU{lKs7X_LwpawCWQ7$x* W8DiU@6a@ziQs{u3%FMtH6afGxqCa2& literal 0 HcmV?d00001 diff --git a/src/test/seratomarkerstest.cpp b/src/test/seratomarkerstest.cpp new file mode 100644 index 00000000000..aef11a175ab --- /dev/null +++ b/src/test/seratomarkerstest.cpp @@ -0,0 +1,76 @@ +#include +#include +#include + +#include +#include + +#include "track/serato/markers.h" +#include "util/memory.h" + +namespace { + +class SeratoMarkersTest : public testing::Test { + protected: + void parseEntry(const QByteArray inputValue, bool valid, bool isSet, quint32 startPosition, quint32 endPosition, QRgb color, bool isLocked, quint8 type) { + const mixxx::SeratoMarkersEntry* pEntry = mixxx::SeratoMarkersEntry::parse(inputValue); + if (!pEntry) { + EXPECT_FALSE(valid); + return; + } + EXPECT_TRUE(valid); + + EXPECT_EQ(isSet, pEntry->isSet()); + EXPECT_EQ(startPosition, pEntry->getStartPosition()); + EXPECT_EQ(endPosition, pEntry->getEndPosition()); + EXPECT_EQ(color, pEntry->getColor()); + EXPECT_EQ(isLocked, pEntry->isLocked()); + EXPECT_EQ(type, pEntry->type()); + + EXPECT_EQ(inputValue, pEntry->data()); + } + + void parseMarkersData(const QByteArray inputValue, bool valid) { + mixxx::SeratoMarkers seratoMarkers; + bool parseOk = mixxx::SeratoMarkers::parse(&seratoMarkers, inputValue); + EXPECT_EQ(valid, parseOk); + if (!parseOk) { + return; + } + + EXPECT_EQ(inputValue, seratoMarkers.data()); + } +}; + +TEST_F(SeratoMarkersTest, ParseEntry) { + parseEntry(QByteArray("\x00\x00\x00\x00\x00\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x30\x00\x00\x01", 21), false, false, -1, -1, QRgb(0x000000), false, 0); + parseEntry(QByteArray("\x00\x00\x00\x00\x00\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x30\x00\x00\x01\x00", 22), true, true, 0, -1, QRgb(0xcc0000), false, 1); + parseEntry(QByteArray("\x00\x00\x0d\x2a\x58\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x32\x10\x00\x01\x00", 22), true, true, 862808, -1, QRgb(0xcc8800), false, 1); + parseEntry(QByteArray("\x00\x00\x03\x54\x64\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x00\x01\x4c\x01\x00", 22), true, true, 218212, -1, QRgb(0x0000cc), false, 1); + parseEntry(QByteArray("\x00\x00\x00\x00\x6c\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x33\x18\x00\x01\x00", 22), true, true, 108, -1, QRgb(0xcccc00), false, 1); + parseEntry(QByteArray("\x00\x00\x00\x07\x77\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x03\x18\x00\x01\x00", 22), true, true, 1911, -1, QRgb(0x00cc00), false, 1); + parseEntry(QByteArray("\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x00\x00\x00\x01\x00", 22), true, false, -1, -1, QRgb(0x000000), false, 1); + parseEntry(QByteArray("\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x00\x00\x00\x03\x00", 22), true, false, -1, -1, QRgb(0x000000), false, 3); +} + +TEST_F(SeratoMarkersTest, ParseMarkersData) { + QDir dir("src/test/serato/data/markers_"); + dir.setFilter(QDir::Files); + dir.setNameFilters(QStringList() << "*.octet-stream"); + + QFileInfoList list = dir.entryInfoList(); + for (int i = 0; i < list.size(); i++) { + QFileInfo fileInfo = list.at(i); + qDebug() << "--- File:" << fileInfo.fileName(); + QFile file(dir.filePath(fileInfo.fileName())); + bool openOk = file.open(QIODevice::ReadOnly); + EXPECT_TRUE(openOk); + if (!openOk) { + continue; + } + QByteArray data = file.readAll(); + parseMarkersData(data, true); + } +} + +} // namespace From b8e4ad8d3165a035e8ce5594e80ba5ecd31c1ab7 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Mon, 10 Feb 2020 00:32:22 +0100 Subject: [PATCH 06/46] track/trackinfo: Add support for reading "Serato Markers_" tags --- src/track/trackinfo.cpp | 1 + src/track/trackinfo.h | 5 ++--- src/track/trackmetadatataglib.cpp | 18 ++++++++++++++++++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/track/trackinfo.cpp b/src/track/trackinfo.cpp index 0cd1da6781b..72ed440a818 100644 --- a/src/track/trackinfo.cpp +++ b/src/track/trackinfo.cpp @@ -75,6 +75,7 @@ bool TrackInfo::compareEq( #endif // __EXTRA_METADATA__ (getReplayGain() == trackInfo.getReplayGain()) && #if defined(__EXTRA_METADATA__) + (getSeratoMarkers() == trackInfo.getSeratoMarkers()) && (getSeratoMarkers2() == trackInfo.getSeratoMarkers2()) && (getSubtitle() == trackInfo.getSubtitle()) && #endif // __EXTRA_METADATA__ diff --git a/src/track/trackinfo.h b/src/track/trackinfo.h index 1f065a5e0aa..478e87001c8 100644 --- a/src/track/trackinfo.h +++ b/src/track/trackinfo.h @@ -4,15 +4,13 @@ #include #include "sources/audiosource.h" - #include "track/bpm.h" #include "track/replaygain.h" +#include "track/serato/markers.h" #include "track/serato/markers2.h" - #include "util/duration.h" #include "util/macros.h" - namespace mixxx { class TrackInfo final { @@ -47,6 +45,7 @@ class TrackInfo final { #endif // __EXTRA_METADATA__ PROPERTY_SET_BYVAL_GET_BYREF(ReplayGain, replayGain, ReplayGain) #if defined(__EXTRA_METADATA__) + PROPERTY_SET_BYVAL_GET_BYREF(SeratoMarkers, seratoMarkers, SeratoMarkers) PROPERTY_SET_BYVAL_GET_BYREF(SeratoMarkers2, seratoMarkers2, SeratoMarkers2) PROPERTY_SET_BYVAL_GET_BYREF(QString, subtitle, Subtitle) #endif // __EXTRA_METADATA__ diff --git a/src/track/trackmetadatataglib.cpp b/src/track/trackmetadatataglib.cpp index 2925d14693d..d12a60f8555 100644 --- a/src/track/trackmetadatataglib.cpp +++ b/src/track/trackmetadatataglib.cpp @@ -427,6 +427,19 @@ bool parseAlbumPeak( return isPeakValid; } +bool parseSeratoMarkers( + TrackMetadata* pTrackMetadata, + const QByteArray& data) { + DEBUG_ASSERT(pTrackMetadata); + + SeratoMarkers seratoMarkers(pTrackMetadata->getTrackInfo().getSeratoMarkers()); + bool isValid = SeratoMarkers::parse(&seratoMarkers, data); + if (isValid) { + pTrackMetadata->refTrackInfo().setSeratoMarkers(seratoMarkers); + } + return isValid; +} + bool parseSeratoMarkers2( TrackMetadata* pTrackMetadata, const QByteArray& data) { @@ -1645,6 +1658,11 @@ void importTrackMetadataFromID3v2Tag( pTrackMetadata->refTrackInfo().setEncoderSettings(toQStringFirstNotEmpty(encoderSettingsFrames)); } // Serato tags + QByteArray seratoMarkers = readFirstGeneralEncapsulatedObjectFrame(tag, "Serato Markers_"); + if (!seratoMarkers.isEmpty()) { + parseSeratoMarkers(pTrackMetadata, seratoMarkers); + } + QByteArray seratoMarkers2 = readFirstGeneralEncapsulatedObjectFrame(tag, "Serato Markers2"); if (!seratoMarkers2.isEmpty()) { parseSeratoMarkers2(pTrackMetadata, seratoMarkers2); From 2d65a58f204fa268fb3c8c5a766c64ae8482cdc0 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Mon, 10 Feb 2020 00:38:50 +0100 Subject: [PATCH 07/46] track/serato/markers: Improve QDebug operator<< support --- src/track/serato/markers.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/track/serato/markers.h b/src/track/serato/markers.h index c4f66758f99..f738a79e395 100644 --- a/src/track/serato/markers.h +++ b/src/track/serato/markers.h @@ -70,7 +70,10 @@ inline bool operator!=(const SeratoMarkersEntry& lhs, const SeratoMarkersEntry& inline QDebug operator<<(QDebug dbg, const SeratoMarkersEntry& arg) { return dbg << "type =" << arg.type() - << "data =" << arg.data(); + << "startPosition =" << arg.getStartPosition() + << "endPosition =" << arg.getEndPosition() + << "color =" << arg.getColor() + << "isLocked =" << arg.isLocked(); } // DTO for storing information from the SeratoMarkers_ tags used by the Serato From 86902ab42c73503c2614fb9e4bf90906dffad982 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Mon, 10 Feb 2020 21:42:30 +0100 Subject: [PATCH 08/46] track/serato/markers: Add comment documenting the color format --- src/track/serato/markers.cpp | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/track/serato/markers.cpp b/src/track/serato/markers.cpp index 49bb5b3ce47..bb4a8ef42e0 100644 --- a/src/track/serato/markers.cpp +++ b/src/track/serato/markers.cpp @@ -20,6 +20,26 @@ quint32 bytesToUInt32(const QByteArray& data) { #endif } +// These functions conversion between the 4-byte "Serato Markers_" color format +// and QRgb (3-Byte RGB, transparency disabled). +// +// Serato's custom color format that is used here also represents RGB colors, +// but inserts a single null bit after every 7 payload bits, starting from the +// rightmost bit. +// +// Here's an example: +// +// | Hex Binary +// ------------- | ----------- -------------------------------- +// 3-byte RGB | 00 00 cc 000 0000000 0000001 1001100 +// Serato format | 00 00 01 4c 00000000000000000000000101001100 +// | +// 3-byte RGB | cc 88 00 110 0110010 0010000 0000000 +// Serato format | 06 32 10 00 00000110001100100001000000000000 +// +// See this for details: +// https://github.com/Holzhaus/serato-tags/blob/master/docs/serato_markers_.md#color-format + QRgb colorToRgb(quint8 w, quint8 x, quint8 y, quint8 z) { quint8 b = (z & 0x7F) | ((y & 0x01) << 7); quint8 g = ((y & 0x7F) >> 1) | ((x & 0x03) << 6); From ad886ff9fab1a20d111a19a7b32f8e6cd675b810 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Mon, 10 Feb 2020 21:46:15 +0100 Subject: [PATCH 09/46] track/serato/markers_: Remove unused allocatedSize code --- src/track/serato/markers.h | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/track/serato/markers.h b/src/track/serato/markers.h index f738a79e395..3a3344627f0 100644 --- a/src/track/serato/markers.h +++ b/src/track/serato/markers.h @@ -90,22 +90,13 @@ class SeratoMarkers final { SeratoMarkers() = default; explicit SeratoMarkers( QList> entries) - : m_allocatedSize(0), m_entries(std::move(entries)) { + : m_entries(std::move(entries)) { } static bool parse(SeratoMarkers* seratoMarkers, const QByteArray& data); QByteArray data() const; - int getAllocatedSize() const { - return m_allocatedSize; - } - - void setAllocatedSize(int size) { - DEBUG_ASSERT(size >= 0); - m_allocatedSize = size; - } - bool isEmpty() const { return m_entries.isEmpty(); } @@ -125,7 +116,6 @@ class SeratoMarkers final { } private: - int m_allocatedSize; QList> m_entries; QRgb m_trackColor; }; From 42491e4bee653fa537097c875a2f9b814db115de Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Mon, 10 Feb 2020 22:03:37 +0100 Subject: [PATCH 10/46] track/serato/markers2: Use one initialization per line --- src/track/serato/markers2.h | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/track/serato/markers2.h b/src/track/serato/markers2.h index eb269813344..0566354366c 100644 --- a/src/track/serato/markers2.h +++ b/src/track/serato/markers2.h @@ -155,11 +155,17 @@ inline QDebug operator<<(QDebug dbg, const SeratoMarkers2ColorEntry& arg) { class SeratoMarkers2CueEntry : public SeratoMarkers2Entry { public: SeratoMarkers2CueEntry(quint8 index, quint32 position, QColor color, QString label) - : m_index(index), m_position(position), m_color(color), m_label(label) { + : m_index(index), + m_position(position), + m_color(color), + m_label(label) { } SeratoMarkers2CueEntry() - : m_index(0), m_position(0), m_color(QColor()), m_label(QString("")) { + : m_index(0), + m_position(0), + m_color(QColor()), + m_label(QString("")) { } static SeratoMarkers2EntryPointer parse(const QByteArray& data); @@ -234,11 +240,19 @@ inline QDebug operator<<(QDebug dbg, const SeratoMarkers2CueEntry& arg) { class SeratoMarkers2LoopEntry : public SeratoMarkers2Entry { public: SeratoMarkers2LoopEntry(quint8 index, quint32 startposition, quint32 endposition, bool locked, QString label) - : m_index(index), m_startposition(startposition), m_endposition(endposition), m_locked(locked), m_label(label) { + : m_index(index), + m_startposition(startposition), + m_endposition(endposition), + m_locked(locked), + m_label(label) { } SeratoMarkers2LoopEntry() - : m_index(0), m_startposition(0), m_endposition(0), m_locked(false), m_label(QString("")) { + : m_index(0), + m_startposition(0), + m_endposition(0), + m_locked(false), + m_label(QString("")) { } static SeratoMarkers2EntryPointer parse(const QByteArray& data); From f4bd9ff50b574113d2d46f86d7e9b2ace26403cc Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Mon, 10 Feb 2020 22:09:27 +0100 Subject: [PATCH 11/46] track/serato/markers: Use one initialization per line --- src/track/serato/markers.h | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/track/serato/markers.h b/src/track/serato/markers.h index 3a3344627f0..39a0e1596ae 100644 --- a/src/track/serato/markers.h +++ b/src/track/serato/markers.h @@ -13,7 +13,13 @@ namespace mixxx { class SeratoMarkersEntry { public: SeratoMarkersEntry(bool isSet, int startPosition, bool isEnabled, int endPosition, QRgb color, int type, bool isLocked) - : m_color(color), m_isEnabled(isEnabled), m_isLocked(isLocked), m_isSet(isSet), m_endPosition(endPosition), m_startPosition(startPosition), m_type(type) { + : m_color(color), + m_isEnabled(isEnabled), + m_isLocked(isLocked), + m_isSet(isSet), + m_endPosition(endPosition), + m_startPosition(startPosition), + m_type(type) { } ~SeratoMarkersEntry() = default; @@ -88,8 +94,7 @@ inline QDebug operator<<(QDebug dbg, const SeratoMarkersEntry& arg) { class SeratoMarkers final { public: SeratoMarkers() = default; - explicit SeratoMarkers( - QList> entries) + explicit SeratoMarkers(QList> entries) : m_entries(std::move(entries)) { } From 0205ed31c1fc19c8e75a114ebcf579c47762c5b4 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Mon, 10 Feb 2020 22:16:41 +0100 Subject: [PATCH 12/46] track/serato/markers: Return SeratoMarkersEntryPointer in parse() --- src/test/seratomarkerstest.cpp | 2 +- src/track/serato/markers.cpp | 6 +++--- src/track/serato/markers.h | 8 +++++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/test/seratomarkerstest.cpp b/src/test/seratomarkerstest.cpp index aef11a175ab..eb6b9a8d9ef 100644 --- a/src/test/seratomarkerstest.cpp +++ b/src/test/seratomarkerstest.cpp @@ -13,7 +13,7 @@ namespace { class SeratoMarkersTest : public testing::Test { protected: void parseEntry(const QByteArray inputValue, bool valid, bool isSet, quint32 startPosition, quint32 endPosition, QRgb color, bool isLocked, quint8 type) { - const mixxx::SeratoMarkersEntry* pEntry = mixxx::SeratoMarkersEntry::parse(inputValue); + const mixxx::SeratoMarkersEntryPointer pEntry = mixxx::SeratoMarkersEntry::parse(inputValue); if (!pEntry) { EXPECT_FALSE(valid); return; diff --git a/src/track/serato/markers.cpp b/src/track/serato/markers.cpp index bb4a8ef42e0..3d0de53bbf8 100644 --- a/src/track/serato/markers.cpp +++ b/src/track/serato/markers.cpp @@ -86,7 +86,7 @@ QByteArray SeratoMarkersEntry::data() const { return data; } -SeratoMarkersEntry* SeratoMarkersEntry::parse(const QByteArray& data) { +SeratoMarkersEntryPointer SeratoMarkersEntry::parse(const QByteArray& data) { if (data.length() != kEntrySize) { qWarning() << "Parsing SeratoMarkersEntry failed:" << "Length" << data.length() << "!=" << kEntrySize; @@ -122,14 +122,14 @@ SeratoMarkersEntry* SeratoMarkersEntry::parse(const QByteArray& data) { const quint8 type = data.at(20); const bool isLocked = static_cast(data.at(21)); - SeratoMarkersEntry* pEntry = new SeratoMarkersEntry( + SeratoMarkersEntryPointer pEntry = SeratoMarkersEntryPointer(new SeratoMarkersEntry( isSet, startPosition, isEnabled, endPosition, color, type, - isLocked); + isLocked)); qDebug() << "SeratoMarkersEntry" << *pEntry; return pEntry; } diff --git a/src/track/serato/markers.h b/src/track/serato/markers.h index 39a0e1596ae..83f91e267ac 100644 --- a/src/track/serato/markers.h +++ b/src/track/serato/markers.h @@ -10,6 +10,10 @@ namespace mixxx { +// Forward declaration +class SeratoMarkersEntry; +typedef std::shared_ptr SeratoMarkersEntryPointer; + class SeratoMarkersEntry { public: SeratoMarkersEntry(bool isSet, int startPosition, bool isEnabled, int endPosition, QRgb color, int type, bool isLocked) @@ -24,7 +28,7 @@ class SeratoMarkersEntry { ~SeratoMarkersEntry() = default; QByteArray data() const; - static SeratoMarkersEntry* parse(const QByteArray& data); + static SeratoMarkersEntryPointer parse(const QByteArray& data); int type() const { return m_type; @@ -64,8 +68,6 @@ class SeratoMarkersEntry { int m_type; }; -typedef std::shared_ptr SeratoMarkersEntryPointer; - inline bool operator==(const SeratoMarkersEntry& lhs, const SeratoMarkersEntry& rhs) { return (lhs.data() == rhs.data()); } From 96ab998fce7fd8e6dba7b06d5b7fd1ef5a88418e Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Mon, 10 Feb 2020 22:23:34 +0100 Subject: [PATCH 13/46] track/serato/markers: Use SeratoMarkersEntryPointer for QList --- src/track/serato/markers.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/track/serato/markers.h b/src/track/serato/markers.h index 83f91e267ac..4ef497d86fe 100644 --- a/src/track/serato/markers.h +++ b/src/track/serato/markers.h @@ -96,7 +96,7 @@ inline QDebug operator<<(QDebug dbg, const SeratoMarkersEntry& arg) { class SeratoMarkers final { public: SeratoMarkers() = default; - explicit SeratoMarkers(QList> entries) + explicit SeratoMarkers(QList entries) : m_entries(std::move(entries)) { } @@ -108,10 +108,10 @@ class SeratoMarkers final { return m_entries.isEmpty(); } - const QList>& getEntries() const { + const QList& getEntries() const { return m_entries; } - void setEntries(QList> entries) { + void setEntries(QList entries) { m_entries = std::move(entries); } @@ -123,7 +123,7 @@ class SeratoMarkers final { } private: - QList> m_entries; + QList m_entries; QRgb m_trackColor; }; From 0ee2787ed1078930a6b551c91970990b3133d079 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Tue, 11 Feb 2020 11:16:44 +0100 Subject: [PATCH 14/46] track/serato/markers: Replace usage of QByteArray::mid() with QDataStream --- src/track/serato/markers.cpp | 60 ++++++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/src/track/serato/markers.cpp b/src/track/serato/markers.cpp index 3d0de53bbf8..db03c538b42 100644 --- a/src/track/serato/markers.cpp +++ b/src/track/serato/markers.cpp @@ -47,9 +47,12 @@ QRgb colorToRgb(quint8 w, quint8 x, quint8 y, quint8 z) { return QRgb(r << 16) | (g << 8) | b; } -QRgb colorToRgb(const QByteArray& data) { - DEBUG_ASSERT(data.size() == 4); - return colorToRgb(data.at(0), data.at(1), data.at(2), data.at(3)); +QRgb colorToRgb(quint32 color) { + return colorToRgb( + (color >> 24) & 0xFF, + (color >> 16) & 0xFF, + (color >> 8) & 0xFF, + color & 0xFF); } quint32 colorFromRgb(quint8 r, quint8 g, quint8 b) { @@ -93,34 +96,53 @@ SeratoMarkersEntryPointer SeratoMarkersEntry::parse(const QByteArray& data) { return nullptr; } - const bool isSet = (data.at(0) == '\x7F') ? false : true; - const quint32 uStartPosition = bytesToUInt32(data.mid(1, 4)); - if (uStartPosition > 0x7FFFFFFF) { + quint8 isSetRaw; + quint8 type; + quint32 startPositionRaw; + quint32 endPositionRaw; + quint32 colorRaw; + bool isEnabled; + bool isLocked; + char buffer[6]; + + QDataStream stream(data); + stream.setVersion(QDataStream::Qt_5_0); + stream.setByteOrder(QDataStream::BigEndian); + stream >> isSetRaw >> startPositionRaw >> isEnabled >> endPositionRaw; + + if (stream.readRawData(buffer, sizeof(buffer)) != sizeof(buffer)) { qWarning() << "Parsing SeratoMarkersEntry failed:" - << "startPosition > 0x7FFFFFF"; + << "unable to read bytes 10..16"; return nullptr; } - const int startPosition = (uStartPosition == 0x7F7F7F7F) ? -1 : static_cast(uStartPosition); - const bool isEnabled = static_cast(data.at(5)); + stream >> colorRaw >> type >> isLocked; + + const bool isSet = (isSetRaw != '\x7F'); + const QRgb color = colorToRgb(colorRaw); - const quint32 uEndPosition = bytesToUInt32(data.mid(6, 4)); - if (uEndPosition > 0x7FFFFFFF) { + // Parse Start Position + if (startPositionRaw > 0x7F7F7F7F) { qWarning() << "Parsing SeratoMarkersEntry failed:" - << "endPosition > 0x7FFFFFF"; + << "startPosition > 0x7F7F7F7F"; return nullptr; } - const int endPosition = (uEndPosition == 0x7F7F7F7F) ? -1 : static_cast(uEndPosition); + const int startPosition = (startPositionRaw == 0x7F7F7F7F) ? -1 : static_cast(startPositionRaw); - if (data.mid(10, 6).compare("\x00\x7F\x7F\x7F\x7F") == 0) { + // Parse End Position + if (endPositionRaw > 0x7F7F7F7F) { qWarning() << "Parsing SeratoMarkersEntry failed:" - << "Unexpected value at offset 10"; + << "endPosition > 0x7F7F7F7F"; return nullptr; } + const int endPosition = (endPositionRaw == 0x7F7F7F7F) ? -1 : static_cast(endPositionRaw); - const QRgb color = colorToRgb(data.mid(16, 4)); - const quint8 type = data.at(20); - const bool isLocked = static_cast(data.at(21)); + // Make sure that the unknown (and probably unused) bytes have the expected value + if (strncmp(buffer, "\x00\x7F\x7F\x7F\x7F\x7F", sizeof(buffer)) != 0) { + qWarning() << "Parsing SeratoMarkersEntry failed:" + << "Unexpected value at offset 10"; + return nullptr; + } SeratoMarkersEntryPointer pEntry = SeratoMarkersEntryPointer(new SeratoMarkersEntry( isSet, @@ -171,7 +193,7 @@ bool SeratoMarkers::parse(SeratoMarkers* seratoMarkers, const QByteArray& data) return false; } - QRgb color = colorToRgb(data.mid(offset)); + QRgb color = colorToRgb(bytesToUInt32(data.mid(offset))); seratoMarkers->setEntries(entries); seratoMarkers->setTrackColor(color); From af1f284016fe05053b0c1d4eab7b6b1c9c77f33f Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Tue, 11 Feb 2020 12:09:30 +0100 Subject: [PATCH 15/46] track/serato/markers: Improve position parsing and update tests --- src/test/seratomarkerstest.cpp | 19 +++++++------- src/track/serato/markers.cpp | 45 ++++++++++++++++++++-------------- src/track/serato/markers.h | 12 +-------- 3 files changed, 37 insertions(+), 39 deletions(-) diff --git a/src/test/seratomarkerstest.cpp b/src/test/seratomarkerstest.cpp index eb6b9a8d9ef..a4a003e5fac 100644 --- a/src/test/seratomarkerstest.cpp +++ b/src/test/seratomarkerstest.cpp @@ -12,7 +12,7 @@ namespace { class SeratoMarkersTest : public testing::Test { protected: - void parseEntry(const QByteArray inputValue, bool valid, bool isSet, quint32 startPosition, quint32 endPosition, QRgb color, bool isLocked, quint8 type) { + void parseEntry(const QByteArray inputValue, bool valid, quint32 startPosition, quint32 endPosition, QRgb color, quint8 type, bool isLocked) { const mixxx::SeratoMarkersEntryPointer pEntry = mixxx::SeratoMarkersEntry::parse(inputValue); if (!pEntry) { EXPECT_FALSE(valid); @@ -20,7 +20,6 @@ class SeratoMarkersTest : public testing::Test { } EXPECT_TRUE(valid); - EXPECT_EQ(isSet, pEntry->isSet()); EXPECT_EQ(startPosition, pEntry->getStartPosition()); EXPECT_EQ(endPosition, pEntry->getEndPosition()); EXPECT_EQ(color, pEntry->getColor()); @@ -43,14 +42,14 @@ class SeratoMarkersTest : public testing::Test { }; TEST_F(SeratoMarkersTest, ParseEntry) { - parseEntry(QByteArray("\x00\x00\x00\x00\x00\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x30\x00\x00\x01", 21), false, false, -1, -1, QRgb(0x000000), false, 0); - parseEntry(QByteArray("\x00\x00\x00\x00\x00\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x30\x00\x00\x01\x00", 22), true, true, 0, -1, QRgb(0xcc0000), false, 1); - parseEntry(QByteArray("\x00\x00\x0d\x2a\x58\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x32\x10\x00\x01\x00", 22), true, true, 862808, -1, QRgb(0xcc8800), false, 1); - parseEntry(QByteArray("\x00\x00\x03\x54\x64\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x00\x01\x4c\x01\x00", 22), true, true, 218212, -1, QRgb(0x0000cc), false, 1); - parseEntry(QByteArray("\x00\x00\x00\x00\x6c\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x33\x18\x00\x01\x00", 22), true, true, 108, -1, QRgb(0xcccc00), false, 1); - parseEntry(QByteArray("\x00\x00\x00\x07\x77\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x03\x18\x00\x01\x00", 22), true, true, 1911, -1, QRgb(0x00cc00), false, 1); - parseEntry(QByteArray("\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x00\x00\x00\x01\x00", 22), true, false, -1, -1, QRgb(0x000000), false, 1); - parseEntry(QByteArray("\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x00\x00\x00\x03\x00", 22), true, false, -1, -1, QRgb(0x000000), false, 3); + parseEntry(QByteArray("\x00\x00\x00\x00\x00\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x30\x00\x00\x01", 21), false, -1, -1, QRgb(0x000000), 0, false); + parseEntry(QByteArray("\x00\x00\x00\x00\x00\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x30\x00\x00\x01\x00", 22), true, 0, -1, QRgb(0xcc0000), 1, false); + parseEntry(QByteArray("\x00\x00\x0d\x2a\x58\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x32\x10\x00\x01\x00", 22), true, 862808, -1, QRgb(0xcc8800), 1, false); + parseEntry(QByteArray("\x00\x00\x03\x54\x64\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x00\x01\x4c\x01\x00", 22), true, 218212, -1, QRgb(0x0000cc), 1, false); + parseEntry(QByteArray("\x00\x00\x00\x00\x6c\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x33\x18\x00\x01\x00", 22), true, 108, -1, QRgb(0xcccc00), 1, false); + parseEntry(QByteArray("\x00\x00\x00\x07\x77\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x03\x18\x00\x01\x00", 22), true, 1911, -1, QRgb(0x00cc00), 1, false); + parseEntry(QByteArray("\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x00\x00\x00\x01\x00", 22), true, -1, -1, QRgb(0x000000), 1, false); + parseEntry(QByteArray("\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x00\x00\x00\x03\x00", 22), true, -1, -1, QRgb(0x000000), 3, false); } TEST_F(SeratoMarkersTest, ParseMarkersData) { diff --git a/src/track/serato/markers.cpp b/src/track/serato/markers.cpp index db03c538b42..18e2f94baf8 100644 --- a/src/track/serato/markers.cpp +++ b/src/track/serato/markers.cpp @@ -78,9 +78,9 @@ QByteArray SeratoMarkersEntry::data() const { QDataStream stream(&data, QIODevice::WriteOnly); stream.setVersion(QDataStream::Qt_5_0); stream.setByteOrder(QDataStream::BigEndian); - stream << (quint8)(m_isSet ? '\x00' : '\x7F') + stream << (quint8)((m_startPosition == -1) ? 0x7F : 0x00) << (quint32)((m_startPosition == -1) ? 0x7F7F7F7F : m_startPosition) - << (quint8)((!m_isSet || m_type == 1) ? '\x7F' : m_isEnabled) + << (quint8)((m_endPosition == -1) ? 0x7F : 0x00) << (quint32)((m_endPosition == -1) ? 0x7F7F7F7F : m_endPosition); stream.writeRawData("\x00\x7F\x7F\x7F\x7F\x7F", 6); stream << (quint32)colorFromRgb(m_color) @@ -96,19 +96,19 @@ SeratoMarkersEntryPointer SeratoMarkersEntry::parse(const QByteArray& data) { return nullptr; } - quint8 isSetRaw; quint8 type; + quint8 startPositionSet; + quint8 endPositionSet; quint32 startPositionRaw; quint32 endPositionRaw; quint32 colorRaw; - bool isEnabled; bool isLocked; char buffer[6]; QDataStream stream(data); stream.setVersion(QDataStream::Qt_5_0); stream.setByteOrder(QDataStream::BigEndian); - stream >> isSetRaw >> startPositionRaw >> isEnabled >> endPositionRaw; + stream >> startPositionSet >> startPositionRaw >> endPositionSet >> endPositionRaw; if (stream.readRawData(buffer, sizeof(buffer)) != sizeof(buffer)) { qWarning() << "Parsing SeratoMarkersEntry failed:" @@ -118,24 +118,35 @@ SeratoMarkersEntryPointer SeratoMarkersEntry::parse(const QByteArray& data) { stream >> colorRaw >> type >> isLocked; - const bool isSet = (isSetRaw != '\x7F'); const QRgb color = colorToRgb(colorRaw); // Parse Start Position - if (startPositionRaw > 0x7F7F7F7F) { - qWarning() << "Parsing SeratoMarkersEntry failed:" - << "startPosition > 0x7F7F7F7F"; - return nullptr; + int startPosition = -1; + if (startPositionSet == 0x7F) { + // Start position not set + if (startPositionRaw != 0x7F7F7F7F) { + qWarning() << "Parsing SeratoMarkersEntry failed:" + << "startPosition != 0x7F7F7F7F"; + + return nullptr; + } + } else { + startPosition = static_cast(startPositionRaw); } - const int startPosition = (startPositionRaw == 0x7F7F7F7F) ? -1 : static_cast(startPositionRaw); // Parse End Position - if (endPositionRaw > 0x7F7F7F7F) { - qWarning() << "Parsing SeratoMarkersEntry failed:" - << "endPosition > 0x7F7F7F7F"; - return nullptr; + int endPosition = -1; + if (endPositionSet == 0x7F) { + // End position not set + if (endPositionRaw != 0x7F7F7F7F) { + qWarning() << "Parsing SeratoMarkersEntry failed:" + << "endPosition != 0x7F7F7F7F"; + + return nullptr; + } + } else { + endPosition = static_cast(endPositionRaw); } - const int endPosition = (endPositionRaw == 0x7F7F7F7F) ? -1 : static_cast(endPositionRaw); // Make sure that the unknown (and probably unused) bytes have the expected value if (strncmp(buffer, "\x00\x7F\x7F\x7F\x7F\x7F", sizeof(buffer)) != 0) { @@ -145,9 +156,7 @@ SeratoMarkersEntryPointer SeratoMarkersEntry::parse(const QByteArray& data) { } SeratoMarkersEntryPointer pEntry = SeratoMarkersEntryPointer(new SeratoMarkersEntry( - isSet, startPosition, - isEnabled, endPosition, color, type, diff --git a/src/track/serato/markers.h b/src/track/serato/markers.h index 4ef497d86fe..11188e3ba1b 100644 --- a/src/track/serato/markers.h +++ b/src/track/serato/markers.h @@ -16,11 +16,9 @@ typedef std::shared_ptr SeratoMarkersEntryPointer; class SeratoMarkersEntry { public: - SeratoMarkersEntry(bool isSet, int startPosition, bool isEnabled, int endPosition, QRgb color, int type, bool isLocked) + SeratoMarkersEntry(int startPosition, int endPosition, QRgb color, int type, bool isLocked) : m_color(color), - m_isEnabled(isEnabled), m_isLocked(isLocked), - m_isSet(isSet), m_endPosition(endPosition), m_startPosition(startPosition), m_type(type) { @@ -38,18 +36,10 @@ class SeratoMarkersEntry { return m_color; } - bool isEnabled() const { - return m_isEnabled; - } - bool isLocked() const { return m_isLocked; } - bool isSet() const { - return m_isSet; - } - int getEndPosition() const { return m_endPosition; } From 56a25fc4a6e42f110c7b0dfa96723d706840f023 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Tue, 11 Feb 2020 12:10:23 +0100 Subject: [PATCH 16/46] tests/seratomarkerstest: Reorder lines to reflect actual byte order --- src/test/seratomarkerstest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/seratomarkerstest.cpp b/src/test/seratomarkerstest.cpp index a4a003e5fac..71351b4d2d3 100644 --- a/src/test/seratomarkerstest.cpp +++ b/src/test/seratomarkerstest.cpp @@ -23,8 +23,8 @@ class SeratoMarkersTest : public testing::Test { EXPECT_EQ(startPosition, pEntry->getStartPosition()); EXPECT_EQ(endPosition, pEntry->getEndPosition()); EXPECT_EQ(color, pEntry->getColor()); - EXPECT_EQ(isLocked, pEntry->isLocked()); EXPECT_EQ(type, pEntry->type()); + EXPECT_EQ(isLocked, pEntry->isLocked()); EXPECT_EQ(inputValue, pEntry->data()); } From e63387dd17df38b88fcb063e6ef0aea420181d67 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Tue, 11 Feb 2020 13:04:50 +0100 Subject: [PATCH 17/46] track/serato/markers: Remove obsolete QStringLiteral #include --- src/track/serato/markers.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/track/serato/markers.cpp b/src/track/serato/markers.cpp index 18e2f94baf8..045c3219a9e 100644 --- a/src/track/serato/markers.cpp +++ b/src/track/serato/markers.cpp @@ -1,7 +1,6 @@ #include "track/serato/markers.h" #include -#include namespace { From f65abf87774fa6e6800c6eb7482124dd9b277e2b Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Tue, 11 Feb 2020 13:41:40 +0100 Subject: [PATCH 18/46] track/serato/markers: Use QDataStream also in SeratoMarkers::parse() --- src/track/serato/markers.cpp | 56 ++++++++++++++---------------------- 1 file changed, 22 insertions(+), 34 deletions(-) diff --git a/src/track/serato/markers.cpp b/src/track/serato/markers.cpp index 045c3219a9e..d1646f06377 100644 --- a/src/track/serato/markers.cpp +++ b/src/track/serato/markers.cpp @@ -5,19 +5,7 @@ namespace { const int kEntrySize = 22; -const char* kVersion = "\x02\x05"; -const int kVersionSize = 2; - -inline -quint32 bytesToUInt32(const QByteArray& data) { - DEBUG_ASSERT(data.size() == sizeof(quint32)); -#if QT_VERSION >= QT_VERSION_CHECK(5, 12, 0) - return qFromBigEndian(data.constData()); -#else - return qFromBigEndian( - reinterpret_cast(data.constData())); -#endif -} +const quint16 kVersion = 0x0205; // These functions conversion between the 4-byte "Serato Markers_" color format // and QRgb (3-Byte RGB, transparency disabled). @@ -165,24 +153,31 @@ SeratoMarkersEntryPointer SeratoMarkersEntry::parse(const QByteArray& data) { } bool SeratoMarkers::parse(SeratoMarkers* seratoMarkers, const QByteArray& data) { - if (!data.startsWith(kVersion)) { + QDataStream stream(data); + stream.setVersion(QDataStream::Qt_5_0); + stream.setByteOrder(QDataStream::BigEndian); + + quint16 version; + stream >> version; + if (version != kVersion) { qWarning() << "Parsing SeratoMarkers_ failed:" << "Unknown Serato Markers_ tag version"; return false; } - quint32 numEntries = bytesToUInt32(data.mid(2, 4)); - QList> entries; + quint32 numEntries; + stream >> numEntries; - int offset = 6; + char buffer[kEntrySize]; + QList entries; for (quint32 i = 0; i < numEntries; i++) { - if (data.size() <= (offset + kEntrySize)) { - qWarning() << "Parsing SeratoMarkers_ failed:" - << "Incomplete entry" << data.mid(offset); + if (stream.readRawData(buffer, sizeof(buffer)) != sizeof(buffer)) { + qWarning() << "Parsing SeratoMarkersEntry failed:" + << "unable to read entry data"; return false; } - QByteArray entryData = data.mid(offset, kEntrySize); + QByteArray entryData = QByteArray(buffer, kEntrySize); SeratoMarkersEntryPointer pEntry = SeratoMarkersEntryPointer( SeratoMarkersEntry::parse(entryData)); if (!pEntry) { @@ -191,33 +186,26 @@ bool SeratoMarkers::parse(SeratoMarkers* seratoMarkers, const QByteArray& data) return false; } entries.append(pEntry); - - offset += kEntrySize; - } - - if (data.mid(offset).size() != 4) { - qWarning() << "Parsing SeratoMarkers_ failed:" - << "Invalid footer" << data.mid(offset); - return false; } - QRgb color = colorToRgb(bytesToUInt32(data.mid(offset))); + quint32 trackColorRaw; + stream >> trackColorRaw; + QRgb trackColor = colorToRgb(trackColorRaw); seratoMarkers->setEntries(entries); - seratoMarkers->setTrackColor(color); + seratoMarkers->setTrackColor(trackColor); return true; } QByteArray SeratoMarkers::data() const { QByteArray data; - data.resize(kVersionSize + 2 * sizeof(quint32) + kEntrySize * m_entries.size()); + data.resize(sizeof(quint16) + 2 * sizeof(quint32) + kEntrySize * m_entries.size()); QDataStream stream(&data, QIODevice::WriteOnly); stream.setVersion(QDataStream::Qt_5_0); stream.setByteOrder(QDataStream::BigEndian); - stream.writeRawData(kVersion, kVersionSize); - stream << m_entries.size(); + stream << kVersion << m_entries.size(); for (int i = 0; i < m_entries.size(); i++) { SeratoMarkersEntryPointer pEntry = m_entries.at(i); stream.writeRawData(pEntry->data(), kEntrySize); From 273201fedc01a02a41de909abad09808612b99c1 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Tue, 11 Feb 2020 13:43:13 +0100 Subject: [PATCH 19/46] track/serato/markers: Make sure that QDataStreams were not read past end --- src/track/serato/markers.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/track/serato/markers.cpp b/src/track/serato/markers.cpp index d1646f06377..49bef7afaa8 100644 --- a/src/track/serato/markers.cpp +++ b/src/track/serato/markers.cpp @@ -142,6 +142,12 @@ SeratoMarkersEntryPointer SeratoMarkersEntry::parse(const QByteArray& data) { return nullptr; } + if (stream.status() != QDataStream::Status::Ok) { + qWarning() << "Parsing SeratoMarkersEntry failed:" + << "Stream read failed with status" << stream.status(); + return nullptr; + } + SeratoMarkersEntryPointer pEntry = SeratoMarkersEntryPointer(new SeratoMarkersEntry( startPosition, endPosition, @@ -192,6 +198,12 @@ bool SeratoMarkers::parse(SeratoMarkers* seratoMarkers, const QByteArray& data) stream >> trackColorRaw; QRgb trackColor = colorToRgb(trackColorRaw); + if (stream.status() != QDataStream::Status::Ok) { + qWarning() << "Parsing SeratoMarkers_ failed:" + << "Stream read failed with status" << stream.status(); + return false; + } + seratoMarkers->setEntries(entries); seratoMarkers->setTrackColor(trackColor); From 4716b9eb26e1d4289c7baf64f104aa8c5b814e18 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Tue, 11 Feb 2020 23:42:47 +0100 Subject: [PATCH 20/46] test/seratomarkerstest: Fix formatting and use const references --- src/test/seratomarkerstest.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/test/seratomarkerstest.cpp b/src/test/seratomarkerstest.cpp index 71351b4d2d3..5735ad44f21 100644 --- a/src/test/seratomarkerstest.cpp +++ b/src/test/seratomarkerstest.cpp @@ -12,7 +12,14 @@ namespace { class SeratoMarkersTest : public testing::Test { protected: - void parseEntry(const QByteArray inputValue, bool valid, quint32 startPosition, quint32 endPosition, QRgb color, quint8 type, bool isLocked) { + void parseEntry( + const QByteArray& inputValue, + bool valid, + quint32 startPosition, + quint32 endPosition, + QRgb color, + quint8 type, + bool isLocked) { const mixxx::SeratoMarkersEntryPointer pEntry = mixxx::SeratoMarkersEntry::parse(inputValue); if (!pEntry) { EXPECT_FALSE(valid); @@ -29,7 +36,7 @@ class SeratoMarkersTest : public testing::Test { EXPECT_EQ(inputValue, pEntry->data()); } - void parseMarkersData(const QByteArray inputValue, bool valid) { + void parseMarkersData(const QByteArray& inputValue, bool valid) { mixxx::SeratoMarkers seratoMarkers; bool parseOk = mixxx::SeratoMarkers::parse(&seratoMarkers, inputValue); EXPECT_EQ(valid, parseOk); From 035ffa7b346a0bcf0a06342068d18507b0ee06d2 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Tue, 11 Feb 2020 23:44:28 +0100 Subject: [PATCH 21/46] track/serato/markers: Use std::move for setEntries() --- src/track/serato/markers.cpp | 2 +- src/track/serato/markers.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/track/serato/markers.cpp b/src/track/serato/markers.cpp index 49bef7afaa8..e76f788f4b8 100644 --- a/src/track/serato/markers.cpp +++ b/src/track/serato/markers.cpp @@ -204,7 +204,7 @@ bool SeratoMarkers::parse(SeratoMarkers* seratoMarkers, const QByteArray& data) return false; } - seratoMarkers->setEntries(entries); + seratoMarkers->setEntries(std::move(entries)); seratoMarkers->setTrackColor(trackColor); return true; diff --git a/src/track/serato/markers.h b/src/track/serato/markers.h index 11188e3ba1b..29a31950753 100644 --- a/src/track/serato/markers.h +++ b/src/track/serato/markers.h @@ -102,7 +102,7 @@ class SeratoMarkers final { return m_entries; } void setEntries(QList entries) { - m_entries = std::move(entries); + m_entries = entries; } QRgb getTrackColor() const { From ed4aa7dfba855dabc9fedc39bddf861218317b6e Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Tue, 11 Feb 2020 23:45:50 +0100 Subject: [PATCH 22/46] track/serato/markers2: Break initialization line for unknown entries --- src/track/serato/markers2.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/track/serato/markers2.h b/src/track/serato/markers2.h index 0566354366c..02ba6d918b9 100644 --- a/src/track/serato/markers2.h +++ b/src/track/serato/markers2.h @@ -43,7 +43,8 @@ inline QDebug operator<<(QDebug dbg, const SeratoMarkers2Entry& arg) { class SeratoMarkers2UnknownEntry : public SeratoMarkers2Entry { public: SeratoMarkers2UnknownEntry(QString type, QByteArray data) - : m_type(std::move(type)), m_data(std::move(data)) { + : m_type(std::move(type)), + m_data(std::move(data)) { } ~SeratoMarkers2UnknownEntry() override = default; From e78de7fb9d904f1fcbf1fd0b36be5c5d8987b921 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Tue, 11 Feb 2020 23:50:47 +0100 Subject: [PATCH 23/46] track/serato/markers2: Rename ::data() to ::dump() --- src/test/seratomarkers2test.cpp | 10 +++++----- src/track/serato/markers2.cpp | 12 ++++++------ src/track/serato/markers2.h | 20 ++++++++++---------- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/src/test/seratomarkers2test.cpp b/src/test/seratomarkers2test.cpp index 71b0811d2ea..69501d0e60d 100644 --- a/src/test/seratomarkers2test.cpp +++ b/src/test/seratomarkers2test.cpp @@ -23,7 +23,7 @@ class SeratoMarkers2Test : public testing::Test { EXPECT_EQ(locked, bpmlockEntry->isLocked()); - EXPECT_EQ(inputValue, bpmlockEntry->data()); + EXPECT_EQ(inputValue, bpmlockEntry->dump()); } void parseColorEntry(const QByteArray inputValue, bool valid, QColor color) { @@ -37,7 +37,7 @@ class SeratoMarkers2Test : public testing::Test { EXPECT_EQ(color, colorEntry->getColor()); - EXPECT_EQ(inputValue, colorEntry->data()); + EXPECT_EQ(inputValue, colorEntry->dump()); } void parseCueEntry(const QByteArray inputValue, bool valid, quint8 index, quint32 position, QColor color, QString label) { @@ -54,7 +54,7 @@ class SeratoMarkers2Test : public testing::Test { EXPECT_EQ(color, cueEntry->getColor()); EXPECT_EQ(label, cueEntry->getLabel()); - EXPECT_EQ(inputValue, cueEntry->data()); + EXPECT_EQ(inputValue, cueEntry->dump()); } void parseLoopEntry(const QByteArray inputValue, bool valid, quint8 index, quint32 startposition, quint32 endposition, bool locked, QString label) { @@ -72,7 +72,7 @@ class SeratoMarkers2Test : public testing::Test { EXPECT_EQ(locked, loopEntry->isLocked()); EXPECT_EQ(label, loopEntry->getLabel()); - EXPECT_EQ(inputValue, loopEntry->data()); + EXPECT_EQ(inputValue, loopEntry->dump()); } void parseMarkers2Data(const QByteArray inputValue, bool valid) { @@ -82,7 +82,7 @@ class SeratoMarkers2Test : public testing::Test { if (!parseOk) { return; } - EXPECT_EQ(inputValue, seratoMarkers2.data()); + EXPECT_EQ(inputValue, seratoMarkers2.dump()); } }; diff --git a/src/track/serato/markers2.cpp b/src/track/serato/markers2.cpp index 75bfbba162d..e467405dc5f 100644 --- a/src/track/serato/markers2.cpp +++ b/src/track/serato/markers2.cpp @@ -17,7 +17,7 @@ SeratoMarkers2EntryPointer SeratoMarkers2BpmlockEntry::parse(const QByteArray& d return SeratoMarkers2EntryPointer(pEntry); } -QByteArray SeratoMarkers2BpmlockEntry::data() const { +QByteArray SeratoMarkers2BpmlockEntry::dump() const { QByteArray data; data.resize(length()); @@ -57,7 +57,7 @@ SeratoMarkers2EntryPointer SeratoMarkers2ColorEntry::parse(const QByteArray& dat return SeratoMarkers2EntryPointer(pEntry); } -QByteArray SeratoMarkers2ColorEntry::data() const { +QByteArray SeratoMarkers2ColorEntry::dump() const { QByteArray data; data.resize(length()); @@ -138,7 +138,7 @@ SeratoMarkers2EntryPointer SeratoMarkers2CueEntry::parse(const QByteArray& data) return SeratoMarkers2EntryPointer(pEntry); } -QByteArray SeratoMarkers2CueEntry::data() const { +QByteArray SeratoMarkers2CueEntry::dump() const { QByteArray data; data.resize(length()); @@ -234,7 +234,7 @@ SeratoMarkers2EntryPointer SeratoMarkers2LoopEntry::parse(const QByteArray& data return SeratoMarkers2EntryPointer(pEntry); } -QByteArray SeratoMarkers2LoopEntry::data() const { +QByteArray SeratoMarkers2LoopEntry::dump() const { QByteArray data; data.resize(length()); @@ -336,7 +336,7 @@ bool SeratoMarkers2::parse(SeratoMarkers2* seratoMarkers2, const QByteArray& out return true; } -QByteArray SeratoMarkers2::data() const { +QByteArray SeratoMarkers2::dump() const { QByteArray data("\x01\x01", 2); for (int i = 0; i < m_entries.size(); i++) { @@ -345,7 +345,7 @@ QByteArray SeratoMarkers2::data() const { data.append(entry->type().toUtf8()); data.append('\0'); data.append((const char*)&lengthBE, 4); - data.append(entry->data()); + data.append(entry->dump()); } data.append('\0'); diff --git a/src/track/serato/markers2.h b/src/track/serato/markers2.h index 02ba6d918b9..305cb90daad 100644 --- a/src/track/serato/markers2.h +++ b/src/track/serato/markers2.h @@ -16,17 +16,17 @@ class SeratoMarkers2Entry { virtual QString type() const = 0; - virtual QByteArray data() const = 0; + virtual QByteArray dump() const = 0; virtual quint32 length() const { - return data().length(); + return dump().length(); } }; typedef std::shared_ptr SeratoMarkers2EntryPointer; inline bool operator==(const SeratoMarkers2Entry& lhs, const SeratoMarkers2Entry& rhs) { - return (lhs.type() == rhs.type()) && (lhs.data() == rhs.data()); + return (lhs.type() == rhs.type()) && (lhs.dump() == rhs.dump()); } inline bool operator!=(const SeratoMarkers2Entry& lhs, const SeratoMarkers2Entry& rhs) { @@ -35,7 +35,7 @@ inline bool operator!=(const SeratoMarkers2Entry& lhs, const SeratoMarkers2Entry inline QDebug operator<<(QDebug dbg, const SeratoMarkers2Entry& arg) { return dbg << "type =" << arg.type() - << "data =" << arg.data() + << "data =" << arg.dump() << "(" << "length =" << arg.length() << ")"; } @@ -52,7 +52,7 @@ class SeratoMarkers2UnknownEntry : public SeratoMarkers2Entry { return m_type; } - QByteArray data() const override { + QByteArray dump() const override { return m_data; } @@ -77,7 +77,7 @@ class SeratoMarkers2BpmlockEntry : public SeratoMarkers2Entry { return "BPMLOCK"; } - QByteArray data() const override; + QByteArray dump() const override; bool isLocked() const { return m_locked; @@ -123,7 +123,7 @@ class SeratoMarkers2ColorEntry : public SeratoMarkers2Entry { return "COLOR"; } - QByteArray data() const override; + QByteArray dump() const override; QColor getColor() const { return m_color; @@ -175,7 +175,7 @@ class SeratoMarkers2CueEntry : public SeratoMarkers2Entry { return "CUE"; } - QByteArray data() const override; + QByteArray dump() const override; quint8 getIndex() const { return m_index; @@ -262,7 +262,7 @@ class SeratoMarkers2LoopEntry : public SeratoMarkers2Entry { return "LOOP"; } - QByteArray data() const override; + QByteArray dump() const override; quint8 getIndex() const { return m_index; @@ -357,7 +357,7 @@ class SeratoMarkers2 final { // SeratoMarkers2 1.0/2.0 specification. static bool parse(SeratoMarkers2* seratoMarkers2, const QByteArray& outerData); - QByteArray data() const; + QByteArray dump() const; int getAllocatedSize() const { return m_allocatedSize; From a6e233ecdd0aa504e3876d094515e8e92708ddc8 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Tue, 11 Feb 2020 23:53:05 +0100 Subject: [PATCH 24/46] track/serato/markers: Rename ::data() to ::dump() --- src/test/seratomarkerstest.cpp | 4 ++-- src/track/serato/markers.cpp | 6 +++--- src/track/serato/markers.h | 6 +++--- src/track/trackmetadatataglib.cpp | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/test/seratomarkerstest.cpp b/src/test/seratomarkerstest.cpp index 5735ad44f21..8bb62f81d3c 100644 --- a/src/test/seratomarkerstest.cpp +++ b/src/test/seratomarkerstest.cpp @@ -33,7 +33,7 @@ class SeratoMarkersTest : public testing::Test { EXPECT_EQ(type, pEntry->type()); EXPECT_EQ(isLocked, pEntry->isLocked()); - EXPECT_EQ(inputValue, pEntry->data()); + EXPECT_EQ(inputValue, pEntry->dump()); } void parseMarkersData(const QByteArray& inputValue, bool valid) { @@ -44,7 +44,7 @@ class SeratoMarkersTest : public testing::Test { return; } - EXPECT_EQ(inputValue, seratoMarkers.data()); + EXPECT_EQ(inputValue, seratoMarkers.dump()); } }; diff --git a/src/track/serato/markers.cpp b/src/track/serato/markers.cpp index e76f788f4b8..fa77c1c40d0 100644 --- a/src/track/serato/markers.cpp +++ b/src/track/serato/markers.cpp @@ -58,7 +58,7 @@ quint32 colorFromRgb(QRgb rgb) { namespace mixxx { -QByteArray SeratoMarkersEntry::data() const { +QByteArray SeratoMarkersEntry::dump() const { QByteArray data; data.resize(kEntrySize); @@ -210,7 +210,7 @@ bool SeratoMarkers::parse(SeratoMarkers* seratoMarkers, const QByteArray& data) return true; } -QByteArray SeratoMarkers::data() const { +QByteArray SeratoMarkers::dump() const { QByteArray data; data.resize(sizeof(quint16) + 2 * sizeof(quint32) + kEntrySize * m_entries.size()); @@ -220,7 +220,7 @@ QByteArray SeratoMarkers::data() const { stream << kVersion << m_entries.size(); for (int i = 0; i < m_entries.size(); i++) { SeratoMarkersEntryPointer pEntry = m_entries.at(i); - stream.writeRawData(pEntry->data(), kEntrySize); + stream.writeRawData(pEntry->dump(), kEntrySize); } stream << colorFromRgb(m_trackColor); return data; diff --git a/src/track/serato/markers.h b/src/track/serato/markers.h index 29a31950753..7c200f94fa5 100644 --- a/src/track/serato/markers.h +++ b/src/track/serato/markers.h @@ -25,7 +25,7 @@ class SeratoMarkersEntry { } ~SeratoMarkersEntry() = default; - QByteArray data() const; + QByteArray dump() const; static SeratoMarkersEntryPointer parse(const QByteArray& data); int type() const { @@ -59,7 +59,7 @@ class SeratoMarkersEntry { }; inline bool operator==(const SeratoMarkersEntry& lhs, const SeratoMarkersEntry& rhs) { - return (lhs.data() == rhs.data()); + return (lhs.dump() == rhs.dump()); } inline bool operator!=(const SeratoMarkersEntry& lhs, const SeratoMarkersEntry& rhs) { @@ -92,7 +92,7 @@ class SeratoMarkers final { static bool parse(SeratoMarkers* seratoMarkers, const QByteArray& data); - QByteArray data() const; + QByteArray dump() const; bool isEmpty() const { return m_entries.isEmpty(); diff --git a/src/track/trackmetadatataglib.cpp b/src/track/trackmetadatataglib.cpp index d12a60f8555..87864e0152d 100644 --- a/src/track/trackmetadatataglib.cpp +++ b/src/track/trackmetadatataglib.cpp @@ -2529,7 +2529,7 @@ bool exportTrackMetadataIntoID3v2Tag(TagLib::ID3v2::Tag* pTag, writeID3v2GeneralEncapsulatedObjectFrame( pTag, "Serato Markers2", - trackMetadata.getTrackInfo().getSeratoMarkers2().data()); + trackMetadata.getTrackInfo().getSeratoMarkers2().dump()); #endif // __EXTRA_METADATA__ return true; From b76dc4520db3f4b5f98613b609875ec629e02203 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Tue, 11 Feb 2020 23:57:28 +0100 Subject: [PATCH 25/46] track/serato/markers: Rename color* functions to seratoColor* --- src/track/serato/markers.cpp | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/track/serato/markers.cpp b/src/track/serato/markers.cpp index fa77c1c40d0..2389db00316 100644 --- a/src/track/serato/markers.cpp +++ b/src/track/serato/markers.cpp @@ -27,22 +27,22 @@ const quint16 kVersion = 0x0205; // See this for details: // https://github.com/Holzhaus/serato-tags/blob/master/docs/serato_markers_.md#color-format -QRgb colorToRgb(quint8 w, quint8 x, quint8 y, quint8 z) { +QRgb seratoColorToRgb(quint8 w, quint8 x, quint8 y, quint8 z) { quint8 b = (z & 0x7F) | ((y & 0x01) << 7); quint8 g = ((y & 0x7F) >> 1) | ((x & 0x03) << 6); quint8 r = ((x & 0x7F) >> 2) | ((w & 0x07) << 5); return QRgb(r << 16) | (g << 8) | b; } -QRgb colorToRgb(quint32 color) { - return colorToRgb( +QRgb seratoColorToRgb(quint32 color) { + return seratoColorToRgb( (color >> 24) & 0xFF, (color >> 16) & 0xFF, (color >> 8) & 0xFF, color & 0xFF); } -quint32 colorFromRgb(quint8 r, quint8 g, quint8 b) { +quint32 seratoColorFromRgb(quint8 r, quint8 g, quint8 b) { quint8 z = b & 0x7F; quint8 y = ((b >> 7) | (g << 1)) & 0x7F; quint8 x = ((g >> 6) | (r << 2)) & 0x7F; @@ -50,10 +50,9 @@ quint32 colorFromRgb(quint8 r, quint8 g, quint8 b) { return (w << 24) | (x << 16) | (y << 8) | z; } -quint32 colorFromRgb(QRgb rgb) { - return colorFromRgb(qRed(rgb), qGreen(rgb), qBlue(rgb)); +quint32 seratoColorFromRgb(QRgb rgb) { + return seratoColorFromRgb(qRed(rgb), qGreen(rgb), qBlue(rgb)); } - } namespace mixxx { @@ -70,7 +69,7 @@ QByteArray SeratoMarkersEntry::dump() const { << (quint8)((m_endPosition == -1) ? 0x7F : 0x00) << (quint32)((m_endPosition == -1) ? 0x7F7F7F7F : m_endPosition); stream.writeRawData("\x00\x7F\x7F\x7F\x7F\x7F", 6); - stream << (quint32)colorFromRgb(m_color) + stream << (quint32)seratoColorFromRgb(m_color) << (quint8)m_type << (quint8)m_isLocked; return data; @@ -105,7 +104,7 @@ SeratoMarkersEntryPointer SeratoMarkersEntry::parse(const QByteArray& data) { stream >> colorRaw >> type >> isLocked; - const QRgb color = colorToRgb(colorRaw); + const QRgb color = seratoColorToRgb(colorRaw); // Parse Start Position int startPosition = -1; @@ -196,7 +195,7 @@ bool SeratoMarkers::parse(SeratoMarkers* seratoMarkers, const QByteArray& data) quint32 trackColorRaw; stream >> trackColorRaw; - QRgb trackColor = colorToRgb(trackColorRaw); + QRgb trackColor = seratoColorToRgb(trackColorRaw); if (stream.status() != QDataStream::Status::Ok) { qWarning() << "Parsing SeratoMarkers_ failed:" @@ -222,7 +221,7 @@ QByteArray SeratoMarkers::dump() const { SeratoMarkersEntryPointer pEntry = m_entries.at(i); stream.writeRawData(pEntry->dump(), kEntrySize); } - stream << colorFromRgb(m_trackColor); + stream << seratoColorFromRgb(m_trackColor); return data; } From 46f954996ae8123b97af3f53872b31c0ad984c50 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Wed, 12 Feb 2020 12:49:47 +0100 Subject: [PATCH 26/46] track/serato/markers2: Use one initialization per line --- src/track/serato/markers2.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/track/serato/markers2.h b/src/track/serato/markers2.h index 305cb90daad..d8e160beb15 100644 --- a/src/track/serato/markers2.h +++ b/src/track/serato/markers2.h @@ -350,7 +350,8 @@ class SeratoMarkers2 final { SeratoMarkers2() = default; explicit SeratoMarkers2( QList> entries) - : m_allocatedSize(0), m_entries(std::move(entries)) { + : m_allocatedSize(0), + m_entries(std::move(entries)) { } // Parsing and formatting of gain values according to the From 51fad8ed6c9390d1fbd033b9f0397165b83955b6 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Wed, 12 Feb 2020 13:18:10 +0100 Subject: [PATCH 27/46] track/serato/markers: Add separate bools to determine if position is set --- src/track/serato/markers.cpp | 36 +++++++++++++-------------- src/track/serato/markers.h | 47 +++++++++++++++++++++++++++--------- 2 files changed, 52 insertions(+), 31 deletions(-) diff --git a/src/track/serato/markers.cpp b/src/track/serato/markers.cpp index 2389db00316..1c307f6457d 100644 --- a/src/track/serato/markers.cpp +++ b/src/track/serato/markers.cpp @@ -64,10 +64,10 @@ QByteArray SeratoMarkersEntry::dump() const { QDataStream stream(&data, QIODevice::WriteOnly); stream.setVersion(QDataStream::Qt_5_0); stream.setByteOrder(QDataStream::BigEndian); - stream << (quint8)((m_startPosition == -1) ? 0x7F : 0x00) - << (quint32)((m_startPosition == -1) ? 0x7F7F7F7F : m_startPosition) - << (quint8)((m_endPosition == -1) ? 0x7F : 0x00) - << (quint32)((m_endPosition == -1) ? 0x7F7F7F7F : m_endPosition); + stream << (quint8)(m_hasStartPosition ? 0x00 : 0x7F) + << (quint32)(m_hasStartPosition ? m_startPosition : 0x7F7F7F7F) + << (quint8)(m_hasEndPosition ? 0x00 : 0x7F) + << (quint32)(m_hasEndPosition ? m_endPosition : 0x7F7F7F7F); stream.writeRawData("\x00\x7F\x7F\x7F\x7F\x7F", 6); stream << (quint32)seratoColorFromRgb(m_color) << (quint8)m_type @@ -83,10 +83,10 @@ SeratoMarkersEntryPointer SeratoMarkersEntry::parse(const QByteArray& data) { } quint8 type; - quint8 startPositionSet; - quint8 endPositionSet; - quint32 startPositionRaw; - quint32 endPositionRaw; + quint8 startPositionStatus; + quint8 endPositionStatus; + quint32 startPosition; + quint32 endPosition; quint32 colorRaw; bool isLocked; char buffer[6]; @@ -94,7 +94,7 @@ SeratoMarkersEntryPointer SeratoMarkersEntry::parse(const QByteArray& data) { QDataStream stream(data); stream.setVersion(QDataStream::Qt_5_0); stream.setByteOrder(QDataStream::BigEndian); - stream >> startPositionSet >> startPositionRaw >> endPositionSet >> endPositionRaw; + stream >> startPositionStatus >> startPosition >> endPositionStatus >> endPosition; if (stream.readRawData(buffer, sizeof(buffer)) != sizeof(buffer)) { qWarning() << "Parsing SeratoMarkersEntry failed:" @@ -107,31 +107,27 @@ SeratoMarkersEntryPointer SeratoMarkersEntry::parse(const QByteArray& data) { const QRgb color = seratoColorToRgb(colorRaw); // Parse Start Position - int startPosition = -1; - if (startPositionSet == 0x7F) { + bool hasStartPosition = (startPositionStatus != 0x7F); + if (!hasStartPosition) { // Start position not set - if (startPositionRaw != 0x7F7F7F7F) { + if (startPosition != 0x7F7F7F7F) { qWarning() << "Parsing SeratoMarkersEntry failed:" << "startPosition != 0x7F7F7F7F"; return nullptr; } - } else { - startPosition = static_cast(startPositionRaw); } // Parse End Position - int endPosition = -1; - if (endPositionSet == 0x7F) { + bool hasEndPosition = (endPositionStatus != 0x7F); + if (!hasEndPosition) { // End position not set - if (endPositionRaw != 0x7F7F7F7F) { + if (endPosition != 0x7F7F7F7F) { qWarning() << "Parsing SeratoMarkersEntry failed:" << "endPosition != 0x7F7F7F7F"; return nullptr; } - } else { - endPosition = static_cast(endPositionRaw); } // Make sure that the unknown (and probably unused) bytes have the expected value @@ -148,7 +144,9 @@ SeratoMarkersEntryPointer SeratoMarkersEntry::parse(const QByteArray& data) { } SeratoMarkersEntryPointer pEntry = SeratoMarkersEntryPointer(new SeratoMarkersEntry( + hasStartPosition, startPosition, + hasEndPosition, endPosition, color, type, diff --git a/src/track/serato/markers.h b/src/track/serato/markers.h index 7c200f94fa5..5f45c83fb35 100644 --- a/src/track/serato/markers.h +++ b/src/track/serato/markers.h @@ -16,11 +16,20 @@ typedef std::shared_ptr SeratoMarkersEntryPointer; class SeratoMarkersEntry { public: - SeratoMarkersEntry(int startPosition, int endPosition, QRgb color, int type, bool isLocked) + SeratoMarkersEntry( + bool hasStartPosition, + int startPosition, + bool hasEndPosition, + int endPosition, + QRgb color, + int type, + bool isLocked) : m_color(color), + m_hasStartPosition(hasStartPosition), + m_hasEndPosition(hasEndPosition), m_isLocked(isLocked), - m_endPosition(endPosition), m_startPosition(startPosition), + m_endPosition(endPosition), m_type(type) { } ~SeratoMarkersEntry() = default; @@ -40,21 +49,31 @@ class SeratoMarkersEntry { return m_isLocked; } - int getEndPosition() const { - return m_endPosition; + bool hasStartPosition() const { + return m_hasStartPosition; } - int getStartPosition() const { + quint32 getStartPosition() const { return m_startPosition; } + bool hasEndPosition() const { + return m_hasEndPosition; + } + + quint32 getEndPosition() const { + return m_endPosition; + } + private: QRgb m_color; - bool m_isEnabled; + bool m_hasStartPosition; + bool m_hasEndPosition; + ; bool m_isLocked; bool m_isSet; - int m_endPosition; - int m_startPosition; + quint32 m_startPosition; + quint32 m_endPosition; int m_type; }; @@ -67,10 +86,14 @@ inline bool operator!=(const SeratoMarkersEntry& lhs, const SeratoMarkersEntry& } inline QDebug operator<<(QDebug dbg, const SeratoMarkersEntry& arg) { - return dbg << "type =" << arg.type() - << "startPosition =" << arg.getStartPosition() - << "endPosition =" << arg.getEndPosition() - << "color =" << arg.getColor() + dbg << "type =" << arg.type(); + if (arg.hasStartPosition()) { + dbg << "startPosition =" << arg.getStartPosition(); + } + if (arg.hasEndPosition()) { + dbg << "endPosition =" << arg.getEndPosition(); + } + return dbg << "color =" << arg.getColor() << "isLocked =" << arg.isLocked(); } From 7274cdb0506f2bf872900cd1362db2a5a7520122 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Wed, 12 Feb 2020 13:19:35 +0100 Subject: [PATCH 28/46] track/trackmetadatataglib: Add missing write call for "Serato Markers_" --- src/track/trackmetadatataglib.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/track/trackmetadatataglib.cpp b/src/track/trackmetadatataglib.cpp index 87864e0152d..efdc96a58d3 100644 --- a/src/track/trackmetadatataglib.cpp +++ b/src/track/trackmetadatataglib.cpp @@ -2526,6 +2526,10 @@ bool exportTrackMetadataIntoID3v2Tag(TagLib::ID3v2::Tag* pTag, pTag, "TSSE", trackMetadata.getTrackInfo().getEncoderSettings()); + writeID3v2GeneralEncapsulatedObjectFrame( + pTag, + "Serato Markers_", + trackMetadata.getTrackInfo().getSeratoMarkers().dump()); writeID3v2GeneralEncapsulatedObjectFrame( pTag, "Serato Markers2", From 998bfea02562b9b1c4d593d182ceaf66f8c339cb Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Wed, 12 Feb 2020 14:40:59 +0100 Subject: [PATCH 29/46] test/seratomarkerstest: Fix tests and add checks for `hasPosition()` --- src/test/seratomarkerstest.cpp | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/test/seratomarkerstest.cpp b/src/test/seratomarkerstest.cpp index 8bb62f81d3c..c1acf88a45c 100644 --- a/src/test/seratomarkerstest.cpp +++ b/src/test/seratomarkerstest.cpp @@ -15,7 +15,9 @@ class SeratoMarkersTest : public testing::Test { void parseEntry( const QByteArray& inputValue, bool valid, + bool hasStartPosition, quint32 startPosition, + bool hasEndPosition, quint32 endPosition, QRgb color, quint8 type, @@ -27,7 +29,9 @@ class SeratoMarkersTest : public testing::Test { } EXPECT_TRUE(valid); + EXPECT_EQ(hasStartPosition, pEntry->hasStartPosition()); EXPECT_EQ(startPosition, pEntry->getStartPosition()); + EXPECT_EQ(hasEndPosition, pEntry->hasEndPosition()); EXPECT_EQ(endPosition, pEntry->getEndPosition()); EXPECT_EQ(color, pEntry->getColor()); EXPECT_EQ(type, pEntry->type()); @@ -49,14 +53,14 @@ class SeratoMarkersTest : public testing::Test { }; TEST_F(SeratoMarkersTest, ParseEntry) { - parseEntry(QByteArray("\x00\x00\x00\x00\x00\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x30\x00\x00\x01", 21), false, -1, -1, QRgb(0x000000), 0, false); - parseEntry(QByteArray("\x00\x00\x00\x00\x00\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x30\x00\x00\x01\x00", 22), true, 0, -1, QRgb(0xcc0000), 1, false); - parseEntry(QByteArray("\x00\x00\x0d\x2a\x58\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x32\x10\x00\x01\x00", 22), true, 862808, -1, QRgb(0xcc8800), 1, false); - parseEntry(QByteArray("\x00\x00\x03\x54\x64\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x00\x01\x4c\x01\x00", 22), true, 218212, -1, QRgb(0x0000cc), 1, false); - parseEntry(QByteArray("\x00\x00\x00\x00\x6c\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x33\x18\x00\x01\x00", 22), true, 108, -1, QRgb(0xcccc00), 1, false); - parseEntry(QByteArray("\x00\x00\x00\x07\x77\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x03\x18\x00\x01\x00", 22), true, 1911, -1, QRgb(0x00cc00), 1, false); - parseEntry(QByteArray("\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x00\x00\x00\x01\x00", 22), true, -1, -1, QRgb(0x000000), 1, false); - parseEntry(QByteArray("\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x00\x00\x00\x03\x00", 22), true, -1, -1, QRgb(0x000000), 3, false); + parseEntry(QByteArray("\x00\x00\x00\x00\x00\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x30\x00\x00\x01", 21), false, false, 0x7f7f7f7f, false, -1, QRgb(0x000000), 0, false); + parseEntry(QByteArray("\x00\x00\x00\x00\x00\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x30\x00\x00\x01\x00", 22), true, true, 0, false, 0x7f7f7f7f, QRgb(0xcc0000), 1, false); + parseEntry(QByteArray("\x00\x00\x0d\x2a\x58\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x32\x10\x00\x01\x00", 22), true, true, 862808, false, 0x7f7f7f7f, QRgb(0xcc8800), 1, false); + parseEntry(QByteArray("\x00\x00\x03\x54\x64\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x00\x01\x4c\x01\x00", 22), true, true, 218212, false, 0x7f7f7f7f, QRgb(0x0000cc), 1, false); + parseEntry(QByteArray("\x00\x00\x00\x00\x6c\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x33\x18\x00\x01\x00", 22), true, true, 108, false, 0x7f7f7f7f, QRgb(0xcccc00), 1, false); + parseEntry(QByteArray("\x00\x00\x00\x07\x77\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x03\x18\x00\x01\x00", 22), true, true, 1911, false, 0x7f7f7f7f, QRgb(0x00cc00), 1, false); + parseEntry(QByteArray("\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x00\x00\x00\x01\x00", 22), true, false, 0x7f7f7f7f, false, 0x7f7f7f7f, QRgb(0x000000), 1, false); + parseEntry(QByteArray("\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x00\x00\x00\x03\x00", 22), true, false, 0x7f7f7f7f, false, 0x7f7f7f7f, QRgb(0x000000), 3, false); } TEST_F(SeratoMarkersTest, ParseMarkersData) { From 849ee75038ef8a60e43ddc5f0546b946c7d4e45d Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Wed, 12 Feb 2020 14:41:58 +0100 Subject: [PATCH 30/46] track/serato/markers: Add check that end of stream is reached --- src/track/serato/markers.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/track/serato/markers.cpp b/src/track/serato/markers.cpp index 1c307f6457d..2aa5de4b255 100644 --- a/src/track/serato/markers.cpp +++ b/src/track/serato/markers.cpp @@ -143,6 +143,12 @@ SeratoMarkersEntryPointer SeratoMarkersEntry::parse(const QByteArray& data) { return nullptr; } + if (!stream.atEnd()) { + qWarning() << "Parsing SeratoMarkersEntry failed:" + << "Unexpected trailing data"; + return nullptr; + } + SeratoMarkersEntryPointer pEntry = SeratoMarkersEntryPointer(new SeratoMarkersEntry( hasStartPosition, startPosition, @@ -201,6 +207,11 @@ bool SeratoMarkers::parse(SeratoMarkers* seratoMarkers, const QByteArray& data) return false; } + if (!stream.atEnd()) { + qWarning() << "Parsing SeratoMarkers_ failed:" + << "Unexpected trailing data"; + return false; + } seratoMarkers->setEntries(std::move(entries)); seratoMarkers->setTrackColor(trackColor); From 39ad2a9ad42d457512efd76319dae470f9fd1697 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Wed, 12 Feb 2020 17:40:17 +0100 Subject: [PATCH 31/46] track/serato/markers: Rename trackColor to more fitting trackColorMask --- src/track/serato/markers.cpp | 10 +++++----- src/track/serato/markers.h | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/track/serato/markers.cpp b/src/track/serato/markers.cpp index 2aa5de4b255..51852e9441d 100644 --- a/src/track/serato/markers.cpp +++ b/src/track/serato/markers.cpp @@ -197,9 +197,9 @@ bool SeratoMarkers::parse(SeratoMarkers* seratoMarkers, const QByteArray& data) entries.append(pEntry); } - quint32 trackColorRaw; - stream >> trackColorRaw; - QRgb trackColor = seratoColorToRgb(trackColorRaw); + quint32 trackColorMaskRaw; + stream >> trackColorMaskRaw; + QRgb trackColorMask = seratoColorToRgb(trackColorMaskRaw); if (stream.status() != QDataStream::Status::Ok) { qWarning() << "Parsing SeratoMarkers_ failed:" @@ -213,7 +213,7 @@ bool SeratoMarkers::parse(SeratoMarkers* seratoMarkers, const QByteArray& data) return false; } seratoMarkers->setEntries(std::move(entries)); - seratoMarkers->setTrackColor(trackColor); + seratoMarkers->setTrackColorMask(trackColorMask); return true; } @@ -230,7 +230,7 @@ QByteArray SeratoMarkers::dump() const { SeratoMarkersEntryPointer pEntry = m_entries.at(i); stream.writeRawData(pEntry->dump(), kEntrySize); } - stream << seratoColorFromRgb(m_trackColor); + stream << seratoColorFromRgb(m_trackColorMask); return data; } diff --git a/src/track/serato/markers.h b/src/track/serato/markers.h index 5f45c83fb35..4552f39481d 100644 --- a/src/track/serato/markers.h +++ b/src/track/serato/markers.h @@ -128,16 +128,16 @@ class SeratoMarkers final { m_entries = entries; } - QRgb getTrackColor() const { - return m_trackColor; + QRgb getTrackColorMask() const { + return m_trackColorMask; } - void setTrackColor(QRgb color) { - m_trackColor = color; + void setTrackColorMask(QRgb colorMask) { + m_trackColorMask = colorMask; } private: QList m_entries; - QRgb m_trackColor; + QRgb m_trackColorMask; }; inline bool operator==(const SeratoMarkers& lhs, const SeratoMarkers& rhs) { From 05f9e32a0bc7a3c0eaf9a2e8bb25a9f2c291fd1f Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Wed, 12 Feb 2020 17:28:10 +0100 Subject: [PATCH 32/46] track/serato/markers2: Use QRgb instead of QColor for colors --- src/test/seratomarkers2test.cpp | 119 ++++++++++++++++++++------------ src/track/serato/markers2.cpp | 20 +++--- src/track/serato/markers2.h | 35 ++++++---- 3 files changed, 107 insertions(+), 67 deletions(-) diff --git a/src/test/seratomarkers2test.cpp b/src/test/seratomarkers2test.cpp index 69501d0e60d..072a22fdf49 100644 --- a/src/test/seratomarkers2test.cpp +++ b/src/test/seratomarkers2test.cpp @@ -26,7 +26,7 @@ class SeratoMarkers2Test : public testing::Test { EXPECT_EQ(inputValue, bpmlockEntry->dump()); } - void parseColorEntry(const QByteArray inputValue, bool valid, QColor color) { + void parseColorEntry(const QByteArray inputValue, bool valid, QRgb colorMask) { const mixxx::SeratoMarkers2EntryPointer parsedEntry = mixxx::SeratoMarkers2ColorEntry::parse(inputValue); if (!parsedEntry) { EXPECT_FALSE(valid); @@ -35,12 +35,12 @@ class SeratoMarkers2Test : public testing::Test { EXPECT_TRUE(valid); const mixxx::SeratoMarkers2ColorEntry *colorEntry = static_cast(parsedEntry.get()); - EXPECT_EQ(color, colorEntry->getColor()); + EXPECT_EQ(colorMask, colorEntry->getColorMask()); EXPECT_EQ(inputValue, colorEntry->dump()); } - void parseCueEntry(const QByteArray inputValue, bool valid, quint8 index, quint32 position, QColor color, QString label) { + void parseCueEntry(const QByteArray inputValue, bool valid, quint8 index, quint32 position, QRgb color, QString label) { const mixxx::SeratoMarkers2EntryPointer parsedEntry = mixxx::SeratoMarkers2CueEntry::parse(inputValue); if (!parsedEntry) { EXPECT_FALSE(valid); @@ -93,70 +93,103 @@ TEST_F(SeratoMarkers2Test, ParseBpmlockEntry) { } TEST_F(SeratoMarkers2Test, ParseColorEntry) { - parseColorEntry(QByteArray("\x00\xcc\x00\x00", 4), true, QColor(0xcc, 0, 0)); - parseColorEntry(QByteArray("\x00\x00\xcc\x00", 4), true, QColor(0, 0xcc, 0)); - parseColorEntry(QByteArray("\x00\x00\x00\xcc", 4), true, QColor(0, 0, 0xcc)); - parseColorEntry(QByteArray("\x00\x89\xab\xcd", 4), true, QColor(0x89, 0xab, 0xcd)); + parseColorEntry(QByteArray("\x00\xcc\x00\x00", 4), true, qRgb(0xcc, 0, 0)); + parseColorEntry(QByteArray("\x00\x00\xcc\x00", 4), true, qRgb(0, 0xcc, 0)); + parseColorEntry(QByteArray("\x00\x00\x00\xcc", 4), true, qRgb(0, 0, 0xcc)); + parseColorEntry(QByteArray("\x00\x89\xab\xcd", 4), true, qRgb(0x89, 0xab, 0xcd)); // Invalid value - parseColorEntry(QByteArray("\x01\xff\x00\x00", 1), false, QColor()); + parseColorEntry(QByteArray("\x01\xff\x00\x00", 1), false, qRgb(0, 0, 0)); // Invalid size - parseColorEntry(QByteArray("\x00", 1), false, QColor()); - parseColorEntry(QByteArray("\x00\xff\x00\x00\x00", 5), false, QColor()); + parseColorEntry(QByteArray("\x00", 1), false, qRgb(0, 0, 0)); + parseColorEntry(QByteArray("\x00\xff\x00\x00\x00", 5), false, qRgb(0, 0, 0)); } TEST_F(SeratoMarkers2Test, ParseCueEntry) { parseCueEntry( - QByteArray("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00", 13), - true, - 0, 0, QColor(0, 0, 0), QString("")); + QByteArray("\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00", 13), + true, + 0, + 0, + qRgb(0, 0, 0), + QString("")); parseCueEntry( - QByteArray("\x00\x01\x00\x00\x10\x00\x00\xcc\x00\x00\x00\x00Test\x00", 17), - true, - 1, 4096, QColor(0xcc, 0, 0), QString("Test")); + QByteArray("\x00\x01\x00\x00\x10\x00\x00\xcc\x00\x00\x00\x00Test\x00", 17), + true, + 1, + 4096, + qRgb(0xcc, 0, 0), + QString("Test")); parseCueEntry( - QByteArray("\x00\x02\x00\x00\x00\xff\x00\x00\xcc\x00\x00\x00\xc3\xa4\xc3\xbc\xc3\xb6\xc3\x9f\xc3\xa9\xc3\xa8!\x00", 26), - true, - 2, 255, QColor(0, 0xcc, 0), QString("äüößéè!")); + QByteArray("\x00\x02\x00\x00\x00\xff\x00\x00\xcc\x00\x00\x00\xc3\xa4\xc3\xbc\xc3\xb6\xc3\x9f\xc3\xa9\xc3\xa8!\x00", 26), + true, + 2, + 255, + qRgb(0, 0xcc, 0), + QString("äüößéè!")); parseCueEntry( - QByteArray("\x00\x03\x02\x03\x04\x05\x00\x06\x07\x08\x00\x00Hello World\x00", 24), - true, - 3, 33752069, QColor(0x06, 0x07, 0x08), QString("Hello World")); + QByteArray("\x00\x03\x02\x03\x04\x05\x00\x06\x07\x08\x00\x00Hello World\x00", 24), + true, + 3, + 33752069, + qRgb(0x06, 0x07, 0x08), + QString("Hello World")); // Invalid value parseCueEntry( - QByteArray("\x01\x04\x00\x00\x10\x00\x00\xcc\x00\x00\x00\x00Test\x00", 17), - false, - 0, 0, QColor(), QString()); + QByteArray("\x01\x04\x00\x00\x10\x00\x00\xcc\x00\x00\x00\x00Test\x00", 17), + false, + 0, + 0, + qRgb(0, 0, 0), + QString()); parseCueEntry( - QByteArray("\x00\x05\x00\x00\x10\x00\x01\xcc\x00\x00\x00\x00Test\x00", 17), - false, - 0, 0, QColor(), QString()); + QByteArray("\x00\x05\x00\x00\x10\x00\x01\xcc\x00\x00\x00\x00Test\x00", 17), + false, + 0, + 0, + qRgb(0, 0, 0), + QString()); parseCueEntry( - QByteArray("\x00\x06\x00\x00\x10\x00\x00\xcc\x00\x00\x01\x00Test\x00", 17), - false, - 0, 0, QColor(), QString()); + QByteArray("\x00\x06\x00\x00\x10\x00\x00\xcc\x00\x00\x01\x00Test\x00", 17), + false, + 0, + 0, + qRgb(0, 0, 0), + QString()); parseCueEntry( - QByteArray("\x00\x07\x00\x00\x10\x00\x00\xcc\x00\x00\x00\x01Test\x00", 17), - false, - 0, 0, QColor(), QString()); + QByteArray("\x00\x07\x00\x00\x10\x00\x00\xcc\x00\x00\x00\x01Test\x00", 17), + false, + 0, + 0, + qRgb(0, 0, 0), + QString()); // Missing null terminator parseCueEntry( - QByteArray("\x00\x08\x00\x00\x10\x00\x00\xcc\x00\x00\x00\x00Test", 16), - false, - 0, 0, QColor(), QString()); + QByteArray("\x00\x08\x00\x00\x10\x00\x00\xcc\x00\x00\x00\x00Test", 16), + false, + 0, + 0, + qRgb(0, 0, 0), + QString()); //Invalid size parseCueEntry( - QByteArray("\x00\x09\x00\x00\x10\x00\x00\xcc\x00\x00\x00\x00", 12), - false, - 0, 0, QColor(), QString()); + QByteArray("\x00\x09\x00\x00\x10\x00\x00\xcc\x00\x00\x00\x00", 12), + false, + 0, + 0, + qRgb(0, 0, 0), + QString()); parseCueEntry( - QByteArray("\x00\x0a\x00\x00\x10\x00\x00\xcc\x00\x00\x00\x00\x00\x00", 14), - false, - 0, 0, QColor(), QString()); + QByteArray("\x00\x0a\x00\x00\x10\x00\x00\xcc\x00\x00\x00\x00\x00\x00", 14), + false, + 0, + 0, + qRgb(0, 0, 0), + QString()); } TEST_F(SeratoMarkers2Test, ParseLoopEntry) { diff --git a/src/track/serato/markers2.cpp b/src/track/serato/markers2.cpp index e467405dc5f..4af146303f1 100644 --- a/src/track/serato/markers2.cpp +++ b/src/track/serato/markers2.cpp @@ -48,11 +48,12 @@ SeratoMarkers2EntryPointer SeratoMarkers2ColorEntry::parse(const QByteArray& dat return nullptr; } - QColor color(static_cast(data.at(1)), + QRgb colorMask = qRgb( + static_cast(data.at(1)), static_cast(data.at(2)), static_cast(data.at(3))); - SeratoMarkers2ColorEntry* pEntry = new SeratoMarkers2ColorEntry(color); + SeratoMarkers2ColorEntry* pEntry = new SeratoMarkers2ColorEntry(colorMask); qDebug() << "SeratoMarkers2ColorEntry" << *pEntry; return SeratoMarkers2EntryPointer(pEntry); } @@ -65,9 +66,9 @@ QByteArray SeratoMarkers2ColorEntry::dump() const { stream.setVersion(QDataStream::Qt_5_0); stream.setByteOrder(QDataStream::BigEndian); stream << (quint8)0 - << (quint8)m_color.red() - << (quint8)m_color.green() - << (quint8)m_color.blue(); + << (quint8)qRed(m_colorMask) + << (quint8)qGreen(m_colorMask) + << (quint8)qBlue(m_colorMask); return data; } @@ -107,7 +108,8 @@ SeratoMarkers2EntryPointer SeratoMarkers2CueEntry::parse(const QByteArray& data) return nullptr; } - QColor color(static_cast(data.at(7)), + QRgb color = qRgb( + static_cast(data.at(7)), static_cast(data.at(8)), static_cast(data.at(9))); @@ -149,9 +151,9 @@ QByteArray SeratoMarkers2CueEntry::dump() const { << m_index << m_position << (quint8)0 - << (quint8)m_color.red() - << (quint8)m_color.green() - << (quint8)m_color.blue() + << (quint8)qRed(m_color) + << (quint8)qGreen(m_color) + << (quint8)qBlue(m_color) << (quint8)0 << (quint8)0; diff --git a/src/track/serato/markers2.h b/src/track/serato/markers2.h index d8e160beb15..8c7bca3f0e4 100644 --- a/src/track/serato/markers2.h +++ b/src/track/serato/markers2.h @@ -8,6 +8,11 @@ #include "util/types.h" +namespace { +const QRgb kDefaultTrackColorMask = QRgb(0xFF9999); +const QRgb kDefaultCueColor = QRgb(0xCC0000); +} // namespace + namespace mixxx { class SeratoMarkers2Entry { @@ -109,12 +114,12 @@ inline QDebug operator<<(QDebug dbg, const SeratoMarkers2BpmlockEntry& arg) { class SeratoMarkers2ColorEntry : public SeratoMarkers2Entry { public: - SeratoMarkers2ColorEntry(QColor color) - : m_color(color) { + SeratoMarkers2ColorEntry(QRgb colorMask) + : m_colorMask(colorMask) { } SeratoMarkers2ColorEntry() - : m_color(QColor()) { + : m_colorMask(kDefaultTrackColorMask) { } static SeratoMarkers2EntryPointer parse(const QByteArray& data); @@ -125,23 +130,23 @@ class SeratoMarkers2ColorEntry : public SeratoMarkers2Entry { QByteArray dump() const override; - QColor getColor() const { - return m_color; + QRgb getColorMask() const { + return m_colorMask; } - void setColor(QColor color) { - m_color = color; + void setColorMask(QRgb colorMask) { + m_colorMask = colorMask; } quint32 length() const override; private: - QColor m_color; + QRgb m_colorMask; }; inline bool operator==(const SeratoMarkers2ColorEntry& lhs, const SeratoMarkers2ColorEntry& rhs) { - return (lhs.getColor() == rhs.getColor()); + return (lhs.getColorMask() == rhs.getColorMask()); } inline bool operator!=(const SeratoMarkers2ColorEntry& lhs, @@ -150,12 +155,12 @@ inline bool operator!=(const SeratoMarkers2ColorEntry& lhs, } inline QDebug operator<<(QDebug dbg, const SeratoMarkers2ColorEntry& arg) { - return dbg << "color =" << arg.getColor(); + return dbg << "color =" << arg.getColorMask(); } class SeratoMarkers2CueEntry : public SeratoMarkers2Entry { public: - SeratoMarkers2CueEntry(quint8 index, quint32 position, QColor color, QString label) + SeratoMarkers2CueEntry(quint8 index, quint32 position, QRgb color, QString label) : m_index(index), m_position(position), m_color(color), @@ -165,7 +170,7 @@ class SeratoMarkers2CueEntry : public SeratoMarkers2Entry { SeratoMarkers2CueEntry() : m_index(0), m_position(0), - m_color(QColor()), + m_color(kDefaultCueColor), m_label(QString("")) { } @@ -193,11 +198,11 @@ class SeratoMarkers2CueEntry : public SeratoMarkers2Entry { m_position = position; } - QColor getColor() const { + QRgb getColor() const { return m_color; } - void setColor(QColor color) { + void setColor(QRgb color) { m_color = color; } @@ -214,7 +219,7 @@ class SeratoMarkers2CueEntry : public SeratoMarkers2Entry { private: quint8 m_index; quint32 m_position; - QColor m_color; + QRgb m_color; QString m_label; }; From b9706ff986d57e5628cc8e2355e2136fcd80cd10 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Wed, 12 Feb 2020 22:56:27 +0100 Subject: [PATCH 33/46] track/serato/markers: Add TypeId enum class --- src/test/seratomarkerstest.cpp | 92 ++++++++++++++++++++++++++++++---- src/track/serato/markers.h | 19 +++++++ 2 files changed, 101 insertions(+), 10 deletions(-) diff --git a/src/test/seratomarkerstest.cpp b/src/test/seratomarkerstest.cpp index c1acf88a45c..5bcbe65ed87 100644 --- a/src/test/seratomarkerstest.cpp +++ b/src/test/seratomarkerstest.cpp @@ -20,7 +20,7 @@ class SeratoMarkersTest : public testing::Test { bool hasEndPosition, quint32 endPosition, QRgb color, - quint8 type, + mixxx::SeratoMarkersEntry::TypeId typeId, bool isLocked) { const mixxx::SeratoMarkersEntryPointer pEntry = mixxx::SeratoMarkersEntry::parse(inputValue); if (!pEntry) { @@ -34,7 +34,7 @@ class SeratoMarkersTest : public testing::Test { EXPECT_EQ(hasEndPosition, pEntry->hasEndPosition()); EXPECT_EQ(endPosition, pEntry->getEndPosition()); EXPECT_EQ(color, pEntry->getColor()); - EXPECT_EQ(type, pEntry->type()); + EXPECT_EQ(typeId, pEntry->typeId()); EXPECT_EQ(isLocked, pEntry->isLocked()); EXPECT_EQ(inputValue, pEntry->dump()); @@ -53,14 +53,86 @@ class SeratoMarkersTest : public testing::Test { }; TEST_F(SeratoMarkersTest, ParseEntry) { - parseEntry(QByteArray("\x00\x00\x00\x00\x00\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x30\x00\x00\x01", 21), false, false, 0x7f7f7f7f, false, -1, QRgb(0x000000), 0, false); - parseEntry(QByteArray("\x00\x00\x00\x00\x00\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x30\x00\x00\x01\x00", 22), true, true, 0, false, 0x7f7f7f7f, QRgb(0xcc0000), 1, false); - parseEntry(QByteArray("\x00\x00\x0d\x2a\x58\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x32\x10\x00\x01\x00", 22), true, true, 862808, false, 0x7f7f7f7f, QRgb(0xcc8800), 1, false); - parseEntry(QByteArray("\x00\x00\x03\x54\x64\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x00\x01\x4c\x01\x00", 22), true, true, 218212, false, 0x7f7f7f7f, QRgb(0x0000cc), 1, false); - parseEntry(QByteArray("\x00\x00\x00\x00\x6c\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x33\x18\x00\x01\x00", 22), true, true, 108, false, 0x7f7f7f7f, QRgb(0xcccc00), 1, false); - parseEntry(QByteArray("\x00\x00\x00\x07\x77\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x03\x18\x00\x01\x00", 22), true, true, 1911, false, 0x7f7f7f7f, QRgb(0x00cc00), 1, false); - parseEntry(QByteArray("\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x00\x00\x00\x01\x00", 22), true, false, 0x7f7f7f7f, false, 0x7f7f7f7f, QRgb(0x000000), 1, false); - parseEntry(QByteArray("\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x00\x00\x00\x03\x00", 22), true, false, 0x7f7f7f7f, false, 0x7f7f7f7f, QRgb(0x000000), 3, false); + parseEntry( + QByteArray("\x00\x00\x00\x00\x00\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x30\x00\x00\x01", 21), + false, + false, + 0x7f7f7f7f, + false, + 0x7f7f7f7f, + QRgb(0x000000), + mixxx::SeratoMarkersEntry::TypeId::Unknown, + false); + parseEntry( + QByteArray("\x00\x00\x00\x00\x00\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x30\x00\x00\x01\x00", 22), + true, + true, + 0, + false, + 0x7f7f7f7f, + QRgb(0xcc0000), + mixxx::SeratoMarkersEntry::TypeId::Cue, + false); + parseEntry( + QByteArray("\x00\x00\x0d\x2a\x58\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x32\x10\x00\x01\x00", 22), + true, + true, + 862808, + false, + 0x7f7f7f7f, + QRgb(0xcc8800), + mixxx::SeratoMarkersEntry::TypeId::Cue, + false); + parseEntry( + QByteArray("\x00\x00\x03\x54\x64\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x00\x01\x4c\x01\x00", 22), + true, + true, + 218212, + false, + 0x7f7f7f7f, + QRgb(0x0000cc), + mixxx::SeratoMarkersEntry::TypeId::Cue, + false); + parseEntry( + QByteArray("\x00\x00\x00\x00\x6c\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x06\x33\x18\x00\x01\x00", 22), + true, + true, + 108, + false, + 0x7f7f7f7f, + QRgb(0xcccc00), + mixxx::SeratoMarkersEntry::TypeId::Cue, + false); + parseEntry( + QByteArray("\x00\x00\x00\x07\x77\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x03\x18\x00\x01\x00", 22), + true, + true, + 1911, + false, + 0x7f7f7f7f, + QRgb(0x00cc00), + mixxx::SeratoMarkersEntry::TypeId::Cue, + false); + parseEntry( + QByteArray("\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x00\x00\x00\x01\x00", 22), + true, + false, + 0x7f7f7f7f, + false, + 0x7f7f7f7f, + QRgb(0x000000), + mixxx::SeratoMarkersEntry::TypeId::Cue, + false); + parseEntry( + QByteArray("\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x7f\x00\x7f\x7f\x7f\x7f\x7f\x00\x00\x00\x00\x03\x00", 22), + true, + false, + 0x7f7f7f7f, + false, + 0x7f7f7f7f, + QRgb(0x000000), + mixxx::SeratoMarkersEntry::TypeId::Loop, + false); } TEST_F(SeratoMarkersTest, ParseMarkersData) { diff --git a/src/track/serato/markers.h b/src/track/serato/markers.h index 4552f39481d..22482ffdaed 100644 --- a/src/track/serato/markers.h +++ b/src/track/serato/markers.h @@ -16,6 +16,12 @@ typedef std::shared_ptr SeratoMarkersEntryPointer; class SeratoMarkersEntry { public: + enum class TypeId { + Unknown, + Cue, + Loop, + }; + SeratoMarkersEntry( bool hasStartPosition, int startPosition, @@ -41,6 +47,19 @@ class SeratoMarkersEntry { return m_type; } + SeratoMarkersEntry::TypeId typeId() const { + SeratoMarkersEntry::TypeId typeId = SeratoMarkersEntry::TypeId::Unknown; + switch (type()) { + case 1: + typeId = SeratoMarkersEntry::TypeId::Cue; + break; + case 3: + typeId = SeratoMarkersEntry::TypeId::Loop; + break; + } + return typeId; + } + QRgb getColor() const { return m_color; } From bcc3aa612cbc1435ef70c36c2ea2be2aa915512a Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Thu, 30 Jan 2020 16:36:41 +0100 Subject: [PATCH 34/46] track/serato/markers2: Add TypeId enum ordered by entry occurrence --- src/track/serato/markers2.h | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/track/serato/markers2.h b/src/track/serato/markers2.h index 8c7bca3f0e4..16039214df1 100644 --- a/src/track/serato/markers2.h +++ b/src/track/serato/markers2.h @@ -15,11 +15,22 @@ const QRgb kDefaultCueColor = QRgb(0xCC0000); namespace mixxx { +// Enum values need to appear in the same order as the corresponding entries +// are written to the tag by Serato. class SeratoMarkers2Entry { public: + enum class TypeId { + Unknown, + Color, + Cue, + Loop, + Bpmlock, + }; + virtual ~SeratoMarkers2Entry() = default; virtual QString type() const = 0; + virtual SeratoMarkers2Entry::TypeId typeId() const = 0; virtual QByteArray dump() const = 0; @@ -57,6 +68,10 @@ class SeratoMarkers2UnknownEntry : public SeratoMarkers2Entry { return m_type; } + SeratoMarkers2Entry::TypeId typeId() const override { + return SeratoMarkers2Entry::TypeId::Unknown; + } + QByteArray dump() const override { return m_data; } @@ -82,6 +97,10 @@ class SeratoMarkers2BpmlockEntry : public SeratoMarkers2Entry { return "BPMLOCK"; } + SeratoMarkers2Entry::TypeId typeId() const override { + return SeratoMarkers2Entry::TypeId::Bpmlock; + } + QByteArray dump() const override; bool isLocked() const { @@ -128,6 +147,10 @@ class SeratoMarkers2ColorEntry : public SeratoMarkers2Entry { return "COLOR"; } + SeratoMarkers2Entry::TypeId typeId() const override { + return SeratoMarkers2Entry::TypeId::Color; + } + QByteArray dump() const override; QRgb getColorMask() const { @@ -180,6 +203,10 @@ class SeratoMarkers2CueEntry : public SeratoMarkers2Entry { return "CUE"; } + SeratoMarkers2Entry::TypeId typeId() const override { + return SeratoMarkers2Entry::TypeId::Cue; + } + QByteArray dump() const override; quint8 getIndex() const { @@ -267,6 +294,10 @@ class SeratoMarkers2LoopEntry : public SeratoMarkers2Entry { return "LOOP"; } + SeratoMarkers2Entry::TypeId typeId() const override { + return SeratoMarkers2Entry::TypeId::Loop; + } + QByteArray dump() const override; quint8 getIndex() const { From e5ff80467c95ce9aa61562959329fe530a592261 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Thu, 13 Feb 2020 00:21:48 +0100 Subject: [PATCH 35/46] track/serato/markers: Map type 0 to Cue (for unset cues) --- src/track/serato/markers.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/track/serato/markers.h b/src/track/serato/markers.h index 22482ffdaed..459e2a31898 100644 --- a/src/track/serato/markers.h +++ b/src/track/serato/markers.h @@ -50,6 +50,7 @@ class SeratoMarkersEntry { SeratoMarkersEntry::TypeId typeId() const { SeratoMarkersEntry::TypeId typeId = SeratoMarkersEntry::TypeId::Unknown; switch (type()) { + case 0: case 1: typeId = SeratoMarkersEntry::TypeId::Cue; break; From be24da25eef1f30407e925d9d0dff7c2fa77af4e Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Wed, 12 Feb 2020 23:57:20 +0100 Subject: [PATCH 36/46] track/serato/markers: Check number of entries and types --- src/track/serato/markers.cpp | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/track/serato/markers.cpp b/src/track/serato/markers.cpp index 51852e9441d..8e48f9d136a 100644 --- a/src/track/serato/markers.cpp +++ b/src/track/serato/markers.cpp @@ -4,6 +4,8 @@ namespace { +const int kNumEntries = 14; +const int kLoopEntryStartIndex = 5; const int kEntrySize = 22; const quint16 kVersion = 0x0205; @@ -177,6 +179,12 @@ bool SeratoMarkers::parse(SeratoMarkers* seratoMarkers, const QByteArray& data) quint32 numEntries; stream >> numEntries; + if (numEntries != kNumEntries) { + qWarning() << "Parsing SeratoMarkers_ failed:" + << "Expected" << kNumEntries << "entries but found" << numEntries; + return false; + } + char buffer[kEntrySize]; QList entries; for (quint32 i = 0; i < numEntries; i++) { @@ -194,6 +202,21 @@ bool SeratoMarkers::parse(SeratoMarkers* seratoMarkers, const QByteArray& data) << "Unable to parse entry!"; return false; } + + if (i < kLoopEntryStartIndex && + pEntry->typeId() != SeratoMarkersEntry::TypeId::Cue) { + qWarning() << "Parsing SeratoMarkers_ failed:" + << "Expected cue entry but found type" << pEntry->type(); + return false; + } + + if (i >= kLoopEntryStartIndex && + pEntry->typeId() != SeratoMarkersEntry::TypeId::Loop) { + qWarning() << "Parsing SeratoMarkers_ failed:" + << "Expected loop entry but found type" << pEntry->type(); + return false; + } + entries.append(pEntry); } From e5b25119389de89a0749a39aa3898ebc06ef3463 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Thu, 13 Feb 2020 15:48:56 +0100 Subject: [PATCH 37/46] track/serato: Use trackColor naming instead of trackColorMask --- src/test/seratomarkers2test.cpp | 4 ++-- src/track/serato/markers.cpp | 10 +++++----- src/track/serato/markers.h | 10 +++++----- src/track/serato/markers2.cpp | 10 +++++----- src/track/serato/markers2.h | 22 +++++++++++----------- 5 files changed, 28 insertions(+), 28 deletions(-) diff --git a/src/test/seratomarkers2test.cpp b/src/test/seratomarkers2test.cpp index 072a22fdf49..c94488c42ff 100644 --- a/src/test/seratomarkers2test.cpp +++ b/src/test/seratomarkers2test.cpp @@ -26,7 +26,7 @@ class SeratoMarkers2Test : public testing::Test { EXPECT_EQ(inputValue, bpmlockEntry->dump()); } - void parseColorEntry(const QByteArray inputValue, bool valid, QRgb colorMask) { + void parseColorEntry(const QByteArray inputValue, bool valid, QRgb color) { const mixxx::SeratoMarkers2EntryPointer parsedEntry = mixxx::SeratoMarkers2ColorEntry::parse(inputValue); if (!parsedEntry) { EXPECT_FALSE(valid); @@ -35,7 +35,7 @@ class SeratoMarkers2Test : public testing::Test { EXPECT_TRUE(valid); const mixxx::SeratoMarkers2ColorEntry *colorEntry = static_cast(parsedEntry.get()); - EXPECT_EQ(colorMask, colorEntry->getColorMask()); + EXPECT_EQ(color, colorEntry->getColor()); EXPECT_EQ(inputValue, colorEntry->dump()); } diff --git a/src/track/serato/markers.cpp b/src/track/serato/markers.cpp index 8e48f9d136a..cdc60239adc 100644 --- a/src/track/serato/markers.cpp +++ b/src/track/serato/markers.cpp @@ -220,9 +220,9 @@ bool SeratoMarkers::parse(SeratoMarkers* seratoMarkers, const QByteArray& data) entries.append(pEntry); } - quint32 trackColorMaskRaw; - stream >> trackColorMaskRaw; - QRgb trackColorMask = seratoColorToRgb(trackColorMaskRaw); + quint32 trackColorRaw; + stream >> trackColorRaw; + QRgb trackColor = seratoColorToRgb(trackColorRaw); if (stream.status() != QDataStream::Status::Ok) { qWarning() << "Parsing SeratoMarkers_ failed:" @@ -236,7 +236,7 @@ bool SeratoMarkers::parse(SeratoMarkers* seratoMarkers, const QByteArray& data) return false; } seratoMarkers->setEntries(std::move(entries)); - seratoMarkers->setTrackColorMask(trackColorMask); + seratoMarkers->setTrackColor(trackColor); return true; } @@ -253,7 +253,7 @@ QByteArray SeratoMarkers::dump() const { SeratoMarkersEntryPointer pEntry = m_entries.at(i); stream.writeRawData(pEntry->dump(), kEntrySize); } - stream << seratoColorFromRgb(m_trackColorMask); + stream << seratoColorFromRgb(m_trackColor); return data; } diff --git a/src/track/serato/markers.h b/src/track/serato/markers.h index 459e2a31898..1408805eb17 100644 --- a/src/track/serato/markers.h +++ b/src/track/serato/markers.h @@ -148,16 +148,16 @@ class SeratoMarkers final { m_entries = entries; } - QRgb getTrackColorMask() const { - return m_trackColorMask; + QRgb getTrackColor() const { + return m_trackColor; } - void setTrackColorMask(QRgb colorMask) { - m_trackColorMask = colorMask; + void setTrackColor(QRgb color) { + m_trackColor = color; } private: QList m_entries; - QRgb m_trackColorMask; + QRgb m_trackColor; }; inline bool operator==(const SeratoMarkers& lhs, const SeratoMarkers& rhs) { diff --git a/src/track/serato/markers2.cpp b/src/track/serato/markers2.cpp index 4af146303f1..3f342619cb7 100644 --- a/src/track/serato/markers2.cpp +++ b/src/track/serato/markers2.cpp @@ -48,12 +48,12 @@ SeratoMarkers2EntryPointer SeratoMarkers2ColorEntry::parse(const QByteArray& dat return nullptr; } - QRgb colorMask = qRgb( + QRgb color = qRgb( static_cast(data.at(1)), static_cast(data.at(2)), static_cast(data.at(3))); - SeratoMarkers2ColorEntry* pEntry = new SeratoMarkers2ColorEntry(colorMask); + SeratoMarkers2ColorEntry* pEntry = new SeratoMarkers2ColorEntry(color); qDebug() << "SeratoMarkers2ColorEntry" << *pEntry; return SeratoMarkers2EntryPointer(pEntry); } @@ -66,9 +66,9 @@ QByteArray SeratoMarkers2ColorEntry::dump() const { stream.setVersion(QDataStream::Qt_5_0); stream.setByteOrder(QDataStream::BigEndian); stream << (quint8)0 - << (quint8)qRed(m_colorMask) - << (quint8)qGreen(m_colorMask) - << (quint8)qBlue(m_colorMask); + << (quint8)qRed(m_color) + << (quint8)qGreen(m_color) + << (quint8)qBlue(m_color); return data; } diff --git a/src/track/serato/markers2.h b/src/track/serato/markers2.h index 16039214df1..15a935e9ad1 100644 --- a/src/track/serato/markers2.h +++ b/src/track/serato/markers2.h @@ -9,7 +9,7 @@ #include "util/types.h" namespace { -const QRgb kDefaultTrackColorMask = QRgb(0xFF9999); +const QRgb kDefaultTrackColor = QRgb(0xFF9999); const QRgb kDefaultCueColor = QRgb(0xCC0000); } // namespace @@ -133,12 +133,12 @@ inline QDebug operator<<(QDebug dbg, const SeratoMarkers2BpmlockEntry& arg) { class SeratoMarkers2ColorEntry : public SeratoMarkers2Entry { public: - SeratoMarkers2ColorEntry(QRgb colorMask) - : m_colorMask(colorMask) { + SeratoMarkers2ColorEntry(QRgb color) + : m_color(color) { } SeratoMarkers2ColorEntry() - : m_colorMask(kDefaultTrackColorMask) { + : m_color(kDefaultTrackColor) { } static SeratoMarkers2EntryPointer parse(const QByteArray& data); @@ -153,23 +153,23 @@ class SeratoMarkers2ColorEntry : public SeratoMarkers2Entry { QByteArray dump() const override; - QRgb getColorMask() const { - return m_colorMask; + QRgb getColor() const { + return m_color; } - void setColorMask(QRgb colorMask) { - m_colorMask = colorMask; + void setColor(QRgb color) { + m_color = color; } quint32 length() const override; private: - QRgb m_colorMask; + QRgb m_color; }; inline bool operator==(const SeratoMarkers2ColorEntry& lhs, const SeratoMarkers2ColorEntry& rhs) { - return (lhs.getColorMask() == rhs.getColorMask()); + return (lhs.getColor() == rhs.getColor()); } inline bool operator!=(const SeratoMarkers2ColorEntry& lhs, @@ -178,7 +178,7 @@ inline bool operator!=(const SeratoMarkers2ColorEntry& lhs, } inline QDebug operator<<(QDebug dbg, const SeratoMarkers2ColorEntry& arg) { - return dbg << "color =" << arg.getColorMask(); + return dbg << "color =" << arg.getColor(); } class SeratoMarkers2CueEntry : public SeratoMarkers2Entry { From 4e957b61e7a019cae6c117d77e139e775ced8b43 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sat, 15 Feb 2020 16:50:04 +0100 Subject: [PATCH 38/46] track/serato/markers2: Use RgbColor instead of plain QRgb --- src/test/seratomarkers2test.cpp | 40 ++++++++++++++++----------------- src/track/serato/markers2.cpp | 8 +++---- src/track/serato/markers2.h | 21 ++++++++--------- 3 files changed, 35 insertions(+), 34 deletions(-) diff --git a/src/test/seratomarkers2test.cpp b/src/test/seratomarkers2test.cpp index c94488c42ff..71249f443ba 100644 --- a/src/test/seratomarkers2test.cpp +++ b/src/test/seratomarkers2test.cpp @@ -26,7 +26,7 @@ class SeratoMarkers2Test : public testing::Test { EXPECT_EQ(inputValue, bpmlockEntry->dump()); } - void parseColorEntry(const QByteArray inputValue, bool valid, QRgb color) { + void parseColorEntry(const QByteArray inputValue, bool valid, mixxx::RgbColor color) { const mixxx::SeratoMarkers2EntryPointer parsedEntry = mixxx::SeratoMarkers2ColorEntry::parse(inputValue); if (!parsedEntry) { EXPECT_FALSE(valid); @@ -40,7 +40,7 @@ class SeratoMarkers2Test : public testing::Test { EXPECT_EQ(inputValue, colorEntry->dump()); } - void parseCueEntry(const QByteArray inputValue, bool valid, quint8 index, quint32 position, QRgb color, QString label) { + void parseCueEntry(const QByteArray inputValue, bool valid, quint8 index, quint32 position, mixxx::RgbColor color, QString label) { const mixxx::SeratoMarkers2EntryPointer parsedEntry = mixxx::SeratoMarkers2CueEntry::parse(inputValue); if (!parsedEntry) { EXPECT_FALSE(valid); @@ -93,17 +93,17 @@ TEST_F(SeratoMarkers2Test, ParseBpmlockEntry) { } TEST_F(SeratoMarkers2Test, ParseColorEntry) { - parseColorEntry(QByteArray("\x00\xcc\x00\x00", 4), true, qRgb(0xcc, 0, 0)); - parseColorEntry(QByteArray("\x00\x00\xcc\x00", 4), true, qRgb(0, 0xcc, 0)); - parseColorEntry(QByteArray("\x00\x00\x00\xcc", 4), true, qRgb(0, 0, 0xcc)); - parseColorEntry(QByteArray("\x00\x89\xab\xcd", 4), true, qRgb(0x89, 0xab, 0xcd)); + parseColorEntry(QByteArray("\x00\xcc\x00\x00", 4), true, mixxx::RgbColor(qRgb(0xcc, 0, 0))); + parseColorEntry(QByteArray("\x00\x00\xcc\x00", 4), true, mixxx::RgbColor(qRgb(0, 0xcc, 0))); + parseColorEntry(QByteArray("\x00\x00\x00\xcc", 4), true, mixxx::RgbColor(qRgb(0, 0, 0xcc))); + parseColorEntry(QByteArray("\x00\x89\xab\xcd", 4), true, mixxx::RgbColor(qRgb(0x89, 0xab, 0xcd))); // Invalid value - parseColorEntry(QByteArray("\x01\xff\x00\x00", 1), false, qRgb(0, 0, 0)); + parseColorEntry(QByteArray("\x01\xff\x00\x00", 1), false, mixxx::RgbColor(qRgb(0, 0, 0))); // Invalid size - parseColorEntry(QByteArray("\x00", 1), false, qRgb(0, 0, 0)); - parseColorEntry(QByteArray("\x00\xff\x00\x00\x00", 5), false, qRgb(0, 0, 0)); + parseColorEntry(QByteArray("\x00", 1), false, mixxx::RgbColor(qRgb(0, 0, 0))); + parseColorEntry(QByteArray("\x00\xff\x00\x00\x00", 5), false, mixxx::RgbColor(qRgb(0, 0, 0))); } TEST_F(SeratoMarkers2Test, ParseCueEntry) { @@ -112,28 +112,28 @@ TEST_F(SeratoMarkers2Test, ParseCueEntry) { true, 0, 0, - qRgb(0, 0, 0), + mixxx::RgbColor(qRgb(0, 0, 0)), QString("")); parseCueEntry( QByteArray("\x00\x01\x00\x00\x10\x00\x00\xcc\x00\x00\x00\x00Test\x00", 17), true, 1, 4096, - qRgb(0xcc, 0, 0), + mixxx::RgbColor(qRgb(0xcc, 0, 0)), QString("Test")); parseCueEntry( QByteArray("\x00\x02\x00\x00\x00\xff\x00\x00\xcc\x00\x00\x00\xc3\xa4\xc3\xbc\xc3\xb6\xc3\x9f\xc3\xa9\xc3\xa8!\x00", 26), true, 2, 255, - qRgb(0, 0xcc, 0), + mixxx::RgbColor(qRgb(0, 0xcc, 0)), QString("äüößéè!")); parseCueEntry( QByteArray("\x00\x03\x02\x03\x04\x05\x00\x06\x07\x08\x00\x00Hello World\x00", 24), true, 3, 33752069, - qRgb(0x06, 0x07, 0x08), + mixxx::RgbColor(qRgb(0x06, 0x07, 0x08)), QString("Hello World")); // Invalid value @@ -142,28 +142,28 @@ TEST_F(SeratoMarkers2Test, ParseCueEntry) { false, 0, 0, - qRgb(0, 0, 0), + mixxx::RgbColor(qRgb(0, 0, 0)), QString()); parseCueEntry( QByteArray("\x00\x05\x00\x00\x10\x00\x01\xcc\x00\x00\x00\x00Test\x00", 17), false, 0, 0, - qRgb(0, 0, 0), + mixxx::RgbColor(qRgb(0, 0, 0)), QString()); parseCueEntry( QByteArray("\x00\x06\x00\x00\x10\x00\x00\xcc\x00\x00\x01\x00Test\x00", 17), false, 0, 0, - qRgb(0, 0, 0), + mixxx::RgbColor(qRgb(0, 0, 0)), QString()); parseCueEntry( QByteArray("\x00\x07\x00\x00\x10\x00\x00\xcc\x00\x00\x00\x01Test\x00", 17), false, 0, 0, - qRgb(0, 0, 0), + mixxx::RgbColor(qRgb(0, 0, 0)), QString()); // Missing null terminator @@ -172,7 +172,7 @@ TEST_F(SeratoMarkers2Test, ParseCueEntry) { false, 0, 0, - qRgb(0, 0, 0), + mixxx::RgbColor(qRgb(0, 0, 0)), QString()); //Invalid size @@ -181,14 +181,14 @@ TEST_F(SeratoMarkers2Test, ParseCueEntry) { false, 0, 0, - qRgb(0, 0, 0), + mixxx::RgbColor(qRgb(0, 0, 0)), QString()); parseCueEntry( QByteArray("\x00\x0a\x00\x00\x10\x00\x00\xcc\x00\x00\x00\x00\x00\x00", 14), false, 0, 0, - qRgb(0, 0, 0), + mixxx::RgbColor(qRgb(0, 0, 0)), QString()); } diff --git a/src/track/serato/markers2.cpp b/src/track/serato/markers2.cpp index 3f342619cb7..41a7029743c 100644 --- a/src/track/serato/markers2.cpp +++ b/src/track/serato/markers2.cpp @@ -48,10 +48,10 @@ SeratoMarkers2EntryPointer SeratoMarkers2ColorEntry::parse(const QByteArray& dat return nullptr; } - QRgb color = qRgb( + RgbColor color = RgbColor(qRgb( static_cast(data.at(1)), static_cast(data.at(2)), - static_cast(data.at(3))); + static_cast(data.at(3)))); SeratoMarkers2ColorEntry* pEntry = new SeratoMarkers2ColorEntry(color); qDebug() << "SeratoMarkers2ColorEntry" << *pEntry; @@ -108,10 +108,10 @@ SeratoMarkers2EntryPointer SeratoMarkers2CueEntry::parse(const QByteArray& data) return nullptr; } - QRgb color = qRgb( + RgbColor color = RgbColor(qRgb( static_cast(data.at(7)), static_cast(data.at(8)), - static_cast(data.at(9))); + static_cast(data.at(9)))); // Unknown field(s), make sure it's 0 in case it's a // null-terminated string diff --git a/src/track/serato/markers2.h b/src/track/serato/markers2.h index 15a935e9ad1..117d4a06bcd 100644 --- a/src/track/serato/markers2.h +++ b/src/track/serato/markers2.h @@ -6,11 +6,12 @@ #include #include +#include "util/color/rgbcolor.h" #include "util/types.h" namespace { -const QRgb kDefaultTrackColor = QRgb(0xFF9999); -const QRgb kDefaultCueColor = QRgb(0xCC0000); +constexpr mixxx::RgbColor kDefaultTrackColor = mixxx::RgbColor(0xFF9999); +constexpr mixxx::RgbColor kDefaultCueColor = mixxx::RgbColor(0xCC0000); } // namespace namespace mixxx { @@ -133,7 +134,7 @@ inline QDebug operator<<(QDebug dbg, const SeratoMarkers2BpmlockEntry& arg) { class SeratoMarkers2ColorEntry : public SeratoMarkers2Entry { public: - SeratoMarkers2ColorEntry(QRgb color) + SeratoMarkers2ColorEntry(RgbColor color) : m_color(color) { } @@ -153,18 +154,18 @@ class SeratoMarkers2ColorEntry : public SeratoMarkers2Entry { QByteArray dump() const override; - QRgb getColor() const { + RgbColor getColor() const { return m_color; } - void setColor(QRgb color) { + void setColor(RgbColor color) { m_color = color; } quint32 length() const override; private: - QRgb m_color; + RgbColor m_color; }; inline bool operator==(const SeratoMarkers2ColorEntry& lhs, @@ -183,7 +184,7 @@ inline QDebug operator<<(QDebug dbg, const SeratoMarkers2ColorEntry& arg) { class SeratoMarkers2CueEntry : public SeratoMarkers2Entry { public: - SeratoMarkers2CueEntry(quint8 index, quint32 position, QRgb color, QString label) + SeratoMarkers2CueEntry(quint8 index, quint32 position, RgbColor color, QString label) : m_index(index), m_position(position), m_color(color), @@ -225,11 +226,11 @@ class SeratoMarkers2CueEntry : public SeratoMarkers2Entry { m_position = position; } - QRgb getColor() const { + RgbColor getColor() const { return m_color; } - void setColor(QRgb color) { + void setColor(RgbColor color) { m_color = color; } @@ -246,7 +247,7 @@ class SeratoMarkers2CueEntry : public SeratoMarkers2Entry { private: quint8 m_index; quint32 m_position; - QRgb m_color; + RgbColor m_color; QString m_label; }; From 016557e6cd1e4d83b4eb1b86cc3bc91e85f2ae6d Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sat, 15 Feb 2020 17:08:28 +0100 Subject: [PATCH 39/46] track/serato/markers: Use RgbColor instead of plain QRgb --- src/test/seratomarkerstest.cpp | 18 +++++++++--------- src/track/serato/markers.cpp | 24 +++++++++++++++--------- src/track/serato/markers.h | 15 ++++++++------- 3 files changed, 32 insertions(+), 25 deletions(-) diff --git a/src/test/seratomarkerstest.cpp b/src/test/seratomarkerstest.cpp index 5bcbe65ed87..2c5bf19fd1c 100644 --- a/src/test/seratomarkerstest.cpp +++ b/src/test/seratomarkerstest.cpp @@ -19,7 +19,7 @@ class SeratoMarkersTest : public testing::Test { quint32 startPosition, bool hasEndPosition, quint32 endPosition, - QRgb color, + mixxx::RgbColor color, mixxx::SeratoMarkersEntry::TypeId typeId, bool isLocked) { const mixxx::SeratoMarkersEntryPointer pEntry = mixxx::SeratoMarkersEntry::parse(inputValue); @@ -60,7 +60,7 @@ TEST_F(SeratoMarkersTest, ParseEntry) { 0x7f7f7f7f, false, 0x7f7f7f7f, - QRgb(0x000000), + mixxx::RgbColor(0x000000), mixxx::SeratoMarkersEntry::TypeId::Unknown, false); parseEntry( @@ -70,7 +70,7 @@ TEST_F(SeratoMarkersTest, ParseEntry) { 0, false, 0x7f7f7f7f, - QRgb(0xcc0000), + mixxx::RgbColor(0xcc0000), mixxx::SeratoMarkersEntry::TypeId::Cue, false); parseEntry( @@ -80,7 +80,7 @@ TEST_F(SeratoMarkersTest, ParseEntry) { 862808, false, 0x7f7f7f7f, - QRgb(0xcc8800), + mixxx::RgbColor(0xcc8800), mixxx::SeratoMarkersEntry::TypeId::Cue, false); parseEntry( @@ -90,7 +90,7 @@ TEST_F(SeratoMarkersTest, ParseEntry) { 218212, false, 0x7f7f7f7f, - QRgb(0x0000cc), + mixxx::RgbColor(0x0000cc), mixxx::SeratoMarkersEntry::TypeId::Cue, false); parseEntry( @@ -100,7 +100,7 @@ TEST_F(SeratoMarkersTest, ParseEntry) { 108, false, 0x7f7f7f7f, - QRgb(0xcccc00), + mixxx::RgbColor(0xcccc00), mixxx::SeratoMarkersEntry::TypeId::Cue, false); parseEntry( @@ -110,7 +110,7 @@ TEST_F(SeratoMarkersTest, ParseEntry) { 1911, false, 0x7f7f7f7f, - QRgb(0x00cc00), + mixxx::RgbColor(0x00cc00), mixxx::SeratoMarkersEntry::TypeId::Cue, false); parseEntry( @@ -120,7 +120,7 @@ TEST_F(SeratoMarkersTest, ParseEntry) { 0x7f7f7f7f, false, 0x7f7f7f7f, - QRgb(0x000000), + mixxx::RgbColor(0x000000), mixxx::SeratoMarkersEntry::TypeId::Cue, false); parseEntry( @@ -130,7 +130,7 @@ TEST_F(SeratoMarkersTest, ParseEntry) { 0x7f7f7f7f, false, 0x7f7f7f7f, - QRgb(0x000000), + mixxx::RgbColor(0x000000), mixxx::SeratoMarkersEntry::TypeId::Loop, false); } diff --git a/src/track/serato/markers.cpp b/src/track/serato/markers.cpp index cdc60239adc..4dac402da2a 100644 --- a/src/track/serato/markers.cpp +++ b/src/track/serato/markers.cpp @@ -2,15 +2,18 @@ #include +#include "util/color/rgbcolor.h" + namespace { const int kNumEntries = 14; const int kLoopEntryStartIndex = 5; const int kEntrySize = 22; const quint16 kVersion = 0x0205; +constexpr mixxx::RgbColor kDefaultTrackColor = mixxx::RgbColor(0xFF9999); // These functions conversion between the 4-byte "Serato Markers_" color format -// and QRgb (3-Byte RGB, transparency disabled). +// and RgbColor (3-Byte RGB, transparency disabled). // // Serato's custom color format that is used here also represents RGB colors, // but inserts a single null bit after every 7 payload bits, starting from the @@ -29,14 +32,14 @@ const quint16 kVersion = 0x0205; // See this for details: // https://github.com/Holzhaus/serato-tags/blob/master/docs/serato_markers_.md#color-format -QRgb seratoColorToRgb(quint8 w, quint8 x, quint8 y, quint8 z) { +mixxx::RgbColor seratoColorToRgb(quint8 w, quint8 x, quint8 y, quint8 z) { quint8 b = (z & 0x7F) | ((y & 0x01) << 7); quint8 g = ((y & 0x7F) >> 1) | ((x & 0x03) << 6); quint8 r = ((x & 0x7F) >> 2) | ((w & 0x07) << 5); - return QRgb(r << 16) | (g << 8) | b; + return mixxx::RgbColor((r << 16) | (g << 8) | b); } -QRgb seratoColorToRgb(quint32 color) { +mixxx::RgbColor seratoColorToRgb(quint32 color) { return seratoColorToRgb( (color >> 24) & 0xFF, (color >> 16) & 0xFF, @@ -52,8 +55,11 @@ quint32 seratoColorFromRgb(quint8 r, quint8 g, quint8 b) { return (w << 24) | (x << 16) | (y << 8) | z; } -quint32 seratoColorFromRgb(QRgb rgb) { - return seratoColorFromRgb(qRed(rgb), qGreen(rgb), qBlue(rgb)); +quint32 seratoColorFromRgb(mixxx::RgbColor rgb) { + return seratoColorFromRgb( + (rgb >> 16) & 0xFF, + (rgb >> 8) & 0xFF, + rgb & 0xFF); } } @@ -106,7 +112,7 @@ SeratoMarkersEntryPointer SeratoMarkersEntry::parse(const QByteArray& data) { stream >> colorRaw >> type >> isLocked; - const QRgb color = seratoColorToRgb(colorRaw); + const RgbColor color = seratoColorToRgb(colorRaw); // Parse Start Position bool hasStartPosition = (startPositionStatus != 0x7F); @@ -222,7 +228,7 @@ bool SeratoMarkers::parse(SeratoMarkers* seratoMarkers, const QByteArray& data) quint32 trackColorRaw; stream >> trackColorRaw; - QRgb trackColor = seratoColorToRgb(trackColorRaw); + RgbColor trackColor = seratoColorToRgb(trackColorRaw); if (stream.status() != QDataStream::Status::Ok) { qWarning() << "Parsing SeratoMarkers_ failed:" @@ -253,7 +259,7 @@ QByteArray SeratoMarkers::dump() const { SeratoMarkersEntryPointer pEntry = m_entries.at(i); stream.writeRawData(pEntry->dump(), kEntrySize); } - stream << seratoColorFromRgb(m_trackColor); + stream << seratoColorFromRgb(m_trackColor.value_or(kDefaultTrackColor)); return data; } diff --git a/src/track/serato/markers.h b/src/track/serato/markers.h index 1408805eb17..79880910368 100644 --- a/src/track/serato/markers.h +++ b/src/track/serato/markers.h @@ -6,6 +6,7 @@ #include #include +#include "util/color/rgbcolor.h" #include "util/types.h" namespace mixxx { @@ -27,7 +28,7 @@ class SeratoMarkersEntry { int startPosition, bool hasEndPosition, int endPosition, - QRgb color, + RgbColor color, int type, bool isLocked) : m_color(color), @@ -61,7 +62,7 @@ class SeratoMarkersEntry { return typeId; } - QRgb getColor() const { + RgbColor getColor() const { return m_color; } @@ -86,7 +87,7 @@ class SeratoMarkersEntry { } private: - QRgb m_color; + RgbColor m_color; bool m_hasStartPosition; bool m_hasEndPosition; ; @@ -138,7 +139,7 @@ class SeratoMarkers final { QByteArray dump() const; bool isEmpty() const { - return m_entries.isEmpty(); + return m_entries.isEmpty() && !m_trackColor; } const QList& getEntries() const { @@ -148,16 +149,16 @@ class SeratoMarkers final { m_entries = entries; } - QRgb getTrackColor() const { + RgbColor::optional_t getTrackColor() const { return m_trackColor; } - void setTrackColor(QRgb color) { + void setTrackColor(RgbColor::optional_t color) { m_trackColor = color; } private: QList m_entries; - QRgb m_trackColor; + RgbColor::optional_t m_trackColor; }; inline bool operator==(const SeratoMarkers& lhs, const SeratoMarkers& rhs) { From 1ffdb248db5a4c77bea2c29a423786bff0571888 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sat, 15 Feb 2020 17:19:07 +0100 Subject: [PATCH 40/46] track/serato/markers2: Remove expensive entry length default implementation --- src/track/serato/markers2.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/track/serato/markers2.h b/src/track/serato/markers2.h index 117d4a06bcd..3b8fa99e00a 100644 --- a/src/track/serato/markers2.h +++ b/src/track/serato/markers2.h @@ -35,9 +35,7 @@ class SeratoMarkers2Entry { virtual QByteArray dump() const = 0; - virtual quint32 length() const { - return dump().length(); - } + virtual quint32 length() const = 0; }; typedef std::shared_ptr SeratoMarkers2EntryPointer; @@ -77,6 +75,10 @@ class SeratoMarkers2UnknownEntry : public SeratoMarkers2Entry { return m_data; } + quint32 length() const override { + return dump().length(); + } + private: QString m_type; QByteArray m_data; From 46a7ac29a546fd9e3ed5bff163f7d4121b169e92 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sat, 15 Feb 2020 17:56:03 +0100 Subject: [PATCH 41/46] track/serato/markers2: Use QDataStream for SeratoMarkers2::dump() --- src/track/serato/markers2.cpp | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/track/serato/markers2.cpp b/src/track/serato/markers2.cpp index 41a7029743c..739cc20d270 100644 --- a/src/track/serato/markers2.cpp +++ b/src/track/serato/markers2.cpp @@ -339,15 +339,20 @@ bool SeratoMarkers2::parse(SeratoMarkers2* seratoMarkers2, const QByteArray& out } QByteArray SeratoMarkers2::dump() const { - QByteArray data("\x01\x01", 2); + QByteArray data; + QDataStream stream(&data, QIODevice::WriteOnly); + stream.setVersion(QDataStream::Qt_5_0); + stream.setByteOrder(QDataStream::BigEndian); + stream << (quint16)0x0101; for (int i = 0; i < m_entries.size(); i++) { SeratoMarkers2EntryPointer entry = m_entries.at(i); - quint32 lengthBE = qToBigEndian(entry->length()); - data.append(entry->type().toUtf8()); - data.append('\0'); - data.append((const char*)&lengthBE, 4); - data.append(entry->dump()); + QByteArray entryName = entry->type().toUtf8(); + QByteArray entryData = entry->dump(); + stream.writeRawData(entryName.constData(), entryName.length()); + stream << (qint8)'\0' // terminating null-byte + << entryData.length(); + stream.writeRawData(entryData.constData(), entryData.length()); } data.append('\0'); From 57ff560c268ef11bedff8167f1aedc833ebe44a9 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sat, 15 Feb 2020 18:47:32 +0100 Subject: [PATCH 42/46] track/serato/markers2: Use QDataStream for parsing CUE/LOOP entries --- src/track/serato/markers2.cpp | 144 +++++++++++++++++++++------------- 1 file changed, 88 insertions(+), 56 deletions(-) diff --git a/src/track/serato/markers2.cpp b/src/track/serato/markers2.cpp index 739cc20d270..524d2967b62 100644 --- a/src/track/serato/markers2.cpp +++ b/src/track/serato/markers2.cpp @@ -2,6 +2,23 @@ #include +namespace { +QString zeroTerminatedUtf8StringtoQString(QDataStream* stream) { + DEBUG_ASSERT(stream); + + QByteArray data; + quint8 byte = '\xFF'; + while (byte != '\x00') { + *stream >> byte; + data.append(byte); + if (stream->status() != QDataStream::Status::Ok) { + return QString(); + } + } + return QString::fromUtf8(data); +} +} // namespace + namespace mixxx { SeratoMarkers2EntryPointer SeratoMarkers2BpmlockEntry::parse(const QByteArray& data) { @@ -84,54 +101,62 @@ SeratoMarkers2EntryPointer SeratoMarkers2CueEntry::parse(const QByteArray& data) return nullptr; } + // CUE entry fields in order of appearance + quint8 unknownField1; + quint8 index; + quint32 position; + quint8 unknownField2; + quint8 rawRgbRed; + quint8 rawRgbGreen; + quint8 rawRgbBlue; + quint16 unknownField3; + + QDataStream stream(data); + stream.setVersion(QDataStream::Qt_5_0); + stream.setByteOrder(QDataStream::BigEndian); + + stream >> unknownField1; + // Unknown field, make sure it's 0 in case it's a // null-terminated string - if (data.at(0) != '\x00') { + if (unknownField1 != '\x00') { qWarning() << "Parsing SeratoMarkers2CueEntry failed:" << "Byte 0: " << data.at(0) << "!= '\\0'"; return nullptr; } - const quint8 index = data.at(1); -#if QT_VERSION >= QT_VERSION_CHECK(5, 12, 0) - const auto position = qFromBigEndian(data.mid(2, 6)); -#else - const auto position = qFromBigEndian( - reinterpret_cast(data.mid(2, 6).constData())); -#endif + stream >> index >> position >> unknownField2; // Unknown field, make sure it's 0 in case it's a // null-terminated string - if (data.at(6) != '\x00') { + if (unknownField2 != '\x00') { qWarning() << "Parsing SeratoMarkers2CueEntry failed:" << "Byte 6: " << data.at(6) << "!= '\\0'"; return nullptr; } - RgbColor color = RgbColor(qRgb( - static_cast(data.at(7)), - static_cast(data.at(8)), - static_cast(data.at(9)))); + stream >> rawRgbRed >> rawRgbGreen >> rawRgbBlue >> unknownField3; + RgbColor color = RgbColor(qRgb(rawRgbRed, rawRgbGreen, rawRgbBlue)); // Unknown field(s), make sure it's 0 in case it's a // null-terminated string - if (data.at(10) != '\x00' || data.at(11) != '\x00') { + if (unknownField3 != 0x0000) { qWarning() << "Parsing SeratoMarkers2CueEntry failed:" - << "Bytes 10-11:" << data.mid(10, 2) << "!= \"\\0\\0\""; + << "Bytes 10-11:" << unknownField3 << "!= \"\\0\\0\""; return nullptr; } - int labelEndPos = data.indexOf('\x00', 12); - if (labelEndPos < 0) { - qWarning() << "Parsing SeratoMarkers2CueEntry failed:" - << "Label end position not found"; + QString label = zeroTerminatedUtf8StringtoQString(&stream); + + if (stream.status() != QDataStream::Status::Ok) { + qWarning() << "Parsing SeratoMarkersEntry failed:" + << "Stream read failed with status" << stream.status(); return nullptr; } - QString label(data.mid(12, labelEndPos - 12)); - if (data.length() > labelEndPos + 1) { - qWarning() << "Parsing SeratoMarkers2CueEntry failed:" - << "Trailing content" << data.mid(labelEndPos + 1); + if (!stream.atEnd()) { + qWarning() << "Parsing SeratoMarkersEntry failed:" + << "Unexpected trailing data"; return nullptr; } @@ -175,63 +200,70 @@ SeratoMarkers2EntryPointer SeratoMarkers2LoopEntry::parse(const QByteArray& data return nullptr; } + // LOOP entry fields in order of appearance + quint8 unknownField1; + quint8 index; + quint32 startPosition; + quint32 endPosition; + quint32 unknownField2; + quint32 unknownField3; + quint8 unknownField4; + bool locked; + + QDataStream stream(data); + stream.setVersion(QDataStream::Qt_5_0); + stream.setByteOrder(QDataStream::BigEndian); + + stream >> unknownField1; // Unknown field, make sure it's 0 in case it's a // null-terminated string - if (data.at(0) != '\x00') { + if (unknownField1 != '\x00') { qWarning() << "Parsing SeratoMarkers2LoopEntry failed:" - << "Byte 0: " << data.at(0) << "!= '\\0'"; + << "Byte 0: " << unknownField1 << "!= '\\0'"; return nullptr; } - const quint8 index = data.at(1); -#if QT_VERSION >= QT_VERSION_CHECK(5, 12, 0) - const auto startposition = qFromBigEndian(data.mid(2, 6)); - const auto endposition = qFromBigEndian(data.mid(6, 10)); -#else - const auto startposition = qFromBigEndian( - reinterpret_cast(data.mid(2, 6).constData())); - const auto endposition = qFromBigEndian( - reinterpret_cast(data.mid(6, 10).constData())); -#endif + stream >> index >> startPosition >> endPosition >> unknownField2; + // Unknown field, make sure it contains the expected "default" value + if (unknownField2 != 0xffffffff) { + qWarning() << "Parsing SeratoMarkers2LoopEntry failed:" + << "Invalid magic value" << unknownField2 << "at offset 10"; + return nullptr; + } + + stream >> unknownField3; // Unknown field, make sure it contains the expected "default" value - if (data.at(10) != '\xff' || - data.at(11) != '\xff' || - data.at(12) != '\xff' || - data.at(13) != '\xff' || - data.at(14) != '\x00' || - data.at(15) != '\x27' || - data.at(16) != '\xaa' || - data.at(17) != '\xe1') { + if (unknownField3 != 0x0027aae1) { qWarning() << "Parsing SeratoMarkers2LoopEntry failed:" - << "Invalid magic value " << data.mid(10, 16); + << "Invalid magic value" << unknownField3 << "at offset 14"; return nullptr; } + stream >> unknownField4; // Unknown field, make sure it's 0 in case it's a // null-terminated string - if (data.at(18) != '\x00') { + if (unknownField4 != '\x00') { qWarning() << "Parsing SeratoMarkers2LoopEntry failed:" - << "Byte 18:" << data.at(18) << "!= '\\0'"; + << "Byte 18:" << unknownField4 << "!= '\\0'"; return nullptr; } - const bool locked = data.at(19); + stream >> locked; + QString label = zeroTerminatedUtf8StringtoQString(&stream); - int labelEndPos = data.indexOf('\x00', 20); - if (labelEndPos < 0) { - qWarning() << "Parsing SeratoMarkers2LoopEntry failed:" - << "Label end position not found"; + if (stream.status() != QDataStream::Status::Ok) { + qWarning() << "Parsing SeratoMarkersEntry failed:" + << "Stream read failed with status" << stream.status(); return nullptr; } - QString label(data.mid(20, labelEndPos - 20)); - if (data.length() > labelEndPos + 1) { - qWarning() << "Parsing SeratoMarkers2LoopEntry failed:" - << "Trailing content" << data.mid(labelEndPos + 1); + if (!stream.atEnd()) { + qWarning() << "Parsing SeratoMarkersEntry failed:" + << "Unexpected trailing data"; return nullptr; } - SeratoMarkers2LoopEntry* pEntry = new SeratoMarkers2LoopEntry(index, startposition, endposition, locked, label); + SeratoMarkers2LoopEntry* pEntry = new SeratoMarkers2LoopEntry(index, startPosition, endPosition, locked, label); qDebug() << "SeratoMarkers2LoopEntry" << *pEntry; return SeratoMarkers2EntryPointer(pEntry); } From 0495addab03be99a9bf8cca11d2e30c1bbd426a4 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sun, 16 Feb 2020 16:59:21 +0100 Subject: [PATCH 43/46] test/seratomarkers2test: Break long lines --- src/test/seratomarkers2test.cpp | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/test/seratomarkers2test.cpp b/src/test/seratomarkers2test.cpp index 71249f443ba..f38c908245e 100644 --- a/src/test/seratomarkers2test.cpp +++ b/src/test/seratomarkers2test.cpp @@ -40,7 +40,13 @@ class SeratoMarkers2Test : public testing::Test { EXPECT_EQ(inputValue, colorEntry->dump()); } - void parseCueEntry(const QByteArray inputValue, bool valid, quint8 index, quint32 position, mixxx::RgbColor color, QString label) { + void parseCueEntry( + const QByteArray inputValue, + bool valid, + quint8 index, + quint32 position, + mixxx::RgbColor color, + QString label) { const mixxx::SeratoMarkers2EntryPointer parsedEntry = mixxx::SeratoMarkers2CueEntry::parse(inputValue); if (!parsedEntry) { EXPECT_FALSE(valid); @@ -57,7 +63,13 @@ class SeratoMarkers2Test : public testing::Test { EXPECT_EQ(inputValue, cueEntry->dump()); } - void parseLoopEntry(const QByteArray inputValue, bool valid, quint8 index, quint32 startposition, quint32 endposition, bool locked, QString label) { + void parseLoopEntry(const QByteArray inputValue, + bool valid, + quint8 index, + quint32 startposition, + quint32 endposition, + bool locked, + QString label) { const mixxx::SeratoMarkers2EntryPointer parsedEntry = mixxx::SeratoMarkers2LoopEntry::parse(inputValue); if (!parsedEntry) { EXPECT_FALSE(valid); @@ -73,7 +85,7 @@ class SeratoMarkers2Test : public testing::Test { EXPECT_EQ(label, loopEntry->getLabel()); EXPECT_EQ(inputValue, loopEntry->dump()); - } + } void parseMarkers2Data(const QByteArray inputValue, bool valid) { mixxx::SeratoMarkers2 seratoMarkers2; From 936dd3bc7c29dc9714dba3ea6cf765b0a9b721a4 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sun, 16 Feb 2020 16:59:53 +0100 Subject: [PATCH 44/46] track/serato/markers: Cast quint8 to quint32 before bitshifting --- src/track/serato/markers.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/track/serato/markers.cpp b/src/track/serato/markers.cpp index 4dac402da2a..4f49ce54f2b 100644 --- a/src/track/serato/markers.cpp +++ b/src/track/serato/markers.cpp @@ -52,7 +52,10 @@ quint32 seratoColorFromRgb(quint8 r, quint8 g, quint8 b) { quint8 y = ((b >> 7) | (g << 1)) & 0x7F; quint8 x = ((g >> 6) | (r << 2)) & 0x7F; quint8 w = (r >> 5); - return (w << 24) | (x << 16) | (y << 8) | z; + return (static_cast(w) << 24) | + (static_cast(x) << 16) | + (static_cast(y) << 8) | + static_cast(z); } quint32 seratoColorFromRgb(mixxx::RgbColor rgb) { From fef74935705d9caab578c97cbf2ae17ce6453267 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sun, 16 Feb 2020 17:00:41 +0100 Subject: [PATCH 45/46] track/serato: Use static_cast if necessary --- src/track/serato/markers.cpp | 14 ++++++------- src/track/serato/markers2.cpp | 38 +++++++++++++++++------------------ 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/track/serato/markers.cpp b/src/track/serato/markers.cpp index 4f49ce54f2b..d3675b7a4cf 100644 --- a/src/track/serato/markers.cpp +++ b/src/track/serato/markers.cpp @@ -75,14 +75,14 @@ QByteArray SeratoMarkersEntry::dump() const { QDataStream stream(&data, QIODevice::WriteOnly); stream.setVersion(QDataStream::Qt_5_0); stream.setByteOrder(QDataStream::BigEndian); - stream << (quint8)(m_hasStartPosition ? 0x00 : 0x7F) - << (quint32)(m_hasStartPosition ? m_startPosition : 0x7F7F7F7F) - << (quint8)(m_hasEndPosition ? 0x00 : 0x7F) - << (quint32)(m_hasEndPosition ? m_endPosition : 0x7F7F7F7F); + stream << static_cast((m_hasStartPosition ? 0x00 : 0x7F)) + << static_cast((m_hasStartPosition ? m_startPosition : 0x7F7F7F7F)) + << static_cast((m_hasEndPosition ? 0x00 : 0x7F)) + << static_cast((m_hasEndPosition ? m_endPosition : 0x7F7F7F7F)); stream.writeRawData("\x00\x7F\x7F\x7F\x7F\x7F", 6); - stream << (quint32)seratoColorFromRgb(m_color) - << (quint8)m_type - << (quint8)m_isLocked; + stream << static_cast(seratoColorFromRgb(m_color)) + << static_cast(m_type) + << static_cast(m_isLocked); return data; } diff --git a/src/track/serato/markers2.cpp b/src/track/serato/markers2.cpp index 524d2967b62..e9e93a75ff2 100644 --- a/src/track/serato/markers2.cpp +++ b/src/track/serato/markers2.cpp @@ -41,7 +41,7 @@ QByteArray SeratoMarkers2BpmlockEntry::dump() const { QDataStream stream(&data, QIODevice::WriteOnly); stream.setVersion(QDataStream::Qt_5_0); stream.setByteOrder(QDataStream::BigEndian); - stream << (quint8)m_locked; + stream << static_cast(m_locked); return data; } @@ -82,10 +82,10 @@ QByteArray SeratoMarkers2ColorEntry::dump() const { QDataStream stream(&data, QIODevice::WriteOnly); stream.setVersion(QDataStream::Qt_5_0); stream.setByteOrder(QDataStream::BigEndian); - stream << (quint8)0 - << (quint8)qRed(m_color) - << (quint8)qGreen(m_color) - << (quint8)qBlue(m_color); + stream << static_cast('\x00') + << static_cast(qRed(m_color)) + << static_cast(qGreen(m_color)) + << static_cast(qBlue(m_color)); return data; } @@ -172,19 +172,19 @@ QByteArray SeratoMarkers2CueEntry::dump() const { QDataStream stream(&data, QIODevice::WriteOnly); stream.setVersion(QDataStream::Qt_5_0); stream.setByteOrder(QDataStream::BigEndian); - stream << (quint8)0 + stream << static_cast('\x00') << m_index << m_position - << (quint8)0 - << (quint8)qRed(m_color) - << (quint8)qGreen(m_color) - << (quint8)qBlue(m_color) - << (quint8)0 - << (quint8)0; + << static_cast('\x00') + << static_cast(qRed(m_color)) + << static_cast(qGreen(m_color)) + << static_cast(qBlue(m_color)) + << static_cast('\x00') + << static_cast('\x00'); QByteArray labelData = m_label.toUtf8(); stream.writeRawData(labelData.constData(), labelData.length()); - stream << (qint8)'\0'; // terminating null-byte + stream << static_cast('\x00'); // terminating null-byte return data; } @@ -275,19 +275,19 @@ QByteArray SeratoMarkers2LoopEntry::dump() const { QDataStream stream(&data, QIODevice::WriteOnly); stream.setVersion(QDataStream::Qt_5_0); stream.setByteOrder(QDataStream::BigEndian); - stream << (quint8)0 + stream << static_cast('\x00') << m_index << m_startposition << m_endposition; stream.writeRawData("\xff\xff\xff\xff\x00\x27\xaa\xe1", 8); - stream << (quint8)0 - << (quint8)m_locked; + stream << static_cast('\x00') + << static_cast(m_locked); QByteArray labelData = m_label.toUtf8(); stream.writeRawData(labelData.constData(), labelData.length()); - stream << (qint8)'\0'; // terminating null-byte + stream << static_cast('\x00'); // terminating null-byte return data; } @@ -375,14 +375,14 @@ QByteArray SeratoMarkers2::dump() const { QDataStream stream(&data, QIODevice::WriteOnly); stream.setVersion(QDataStream::Qt_5_0); stream.setByteOrder(QDataStream::BigEndian); - stream << (quint16)0x0101; + stream << static_cast(0x0101); for (int i = 0; i < m_entries.size(); i++) { SeratoMarkers2EntryPointer entry = m_entries.at(i); QByteArray entryName = entry->type().toUtf8(); QByteArray entryData = entry->dump(); stream.writeRawData(entryName.constData(), entryName.length()); - stream << (qint8)'\0' // terminating null-byte + stream << static_cast('\x00') // terminating null-byte << entryData.length(); stream.writeRawData(entryData.constData(), entryData.length()); } From c9a38e58145205129a8618166bd916e284b9f1be Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Sun, 16 Feb 2020 17:01:23 +0100 Subject: [PATCH 46/46] track/serato/markers: Add some comments for typeId values --- src/track/serato/markers.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/track/serato/markers.h b/src/track/serato/markers.h index 79880910368..b81f7b55634 100644 --- a/src/track/serato/markers.h +++ b/src/track/serato/markers.h @@ -51,11 +51,11 @@ class SeratoMarkersEntry { SeratoMarkersEntry::TypeId typeId() const { SeratoMarkersEntry::TypeId typeId = SeratoMarkersEntry::TypeId::Unknown; switch (type()) { - case 0: - case 1: + case 0: // This seems to be an unset Hotcue (i.e. without a position) + case 1: // Hotcue typeId = SeratoMarkersEntry::TypeId::Cue; break; - case 3: + case 3: // Saved Loop typeId = SeratoMarkersEntry::TypeId::Loop; break; }