From e50416c150affa0e584f8d8807afc6abadc5c436 Mon Sep 17 00:00:00 2001 From: Jan Holthuis Date: Thu, 30 Sep 2021 17:17:43 +0200 Subject: BeatsImporter: Return Beats object instead of frame position vector This makes the BeatsImporter return an actual Beats object instead of a vector of frame positions. The main reason for this change is that it will avoid the unnecessary intermediate step of converting to/from raw frame positions when the new beats implementation is ready. Additionally, this prevents wrong usage by making it impossible to passing a `SignalInfo` to import method and then use a different sample rate for creating the beats object (which would invalidate all positions). Theoretically, this also allows to return a `BeatGrid` instead of a `BeatMap`, although we don't make use of this possibility and the distinction hopefully becomes obsolete soon anyway. --- src/test/seratobeatgridtest.cpp | 39 ++++++++++++++++++++++++++------------ src/track/beatsimporter.h | 6 +++--- src/track/serato/beatsimporter.cpp | 16 +++++++++------- src/track/serato/beatsimporter.h | 5 +++-- src/track/track.cpp | 5 ++--- 5 files changed, 44 insertions(+), 27 deletions(-) diff --git a/src/test/seratobeatgridtest.cpp b/src/test/seratobeatgridtest.cpp index 41b985293c..d8b1b9d3fc 100644 --- a/src/test/seratobeatgridtest.cpp +++ b/src/test/seratobeatgridtest.cpp @@ -170,14 +170,19 @@ TEST_F(SeratoBeatGridTest, SerializeBeatMap) { mixxx::SeratoBeatsImporter beatsImporter( seratoBeatGrid.nonTerminalMarkers(), seratoBeatGrid.terminalMarker()); - const QVector importedBeatPositionsFrames = - beatsImporter.importBeatsAndApplyTimingOffset(timingOffsetMillis, signalInfo); - ASSERT_EQ(beatPositionsFrames.size(), importedBeatPositionsFrames.size()); + const auto pImportedBeats = + beatsImporter.importBeatsAndApplyTimingOffset( + timingOffsetMillis, signalInfo); + auto pBeatsIterator = + pImportedBeats->findBeats(beatPositionsFrames.first() - 1000, + beatPositionsFrames.last() + 1000); for (int i = 0; i < beatPositionsFrames.size(); i++) { + const auto importedPosition = pBeatsIterator->next(); EXPECT_NEAR(beatPositionsFrames[i].value(), - importedBeatPositionsFrames[i].value(), + importedPosition.value(), kEpsilon); } + ASSERT_FALSE(pBeatsIterator->hasNext()); } constexpr int kNumBeats60BPM = 4; @@ -218,14 +223,19 @@ TEST_F(SeratoBeatGridTest, SerializeBeatMap) { mixxx::SeratoBeatsImporter beatsImporter( seratoBeatGrid.nonTerminalMarkers(), seratoBeatGrid.terminalMarker()); - const QVector importedBeatPositionsFrames = - beatsImporter.importBeatsAndApplyTimingOffset(timingOffsetMillis, signalInfo); - ASSERT_EQ(beatPositionsFrames.size(), importedBeatPositionsFrames.size()); + const auto pImportedBeats = + beatsImporter.importBeatsAndApplyTimingOffset( + timingOffsetMillis, signalInfo); + auto pBeatsIterator = + pImportedBeats->findBeats(beatPositionsFrames.first() - 1000, + beatPositionsFrames.last() + 1000); for (int i = 0; i < beatPositionsFrames.size(); i++) { + const auto importedPosition = pBeatsIterator->next(); EXPECT_NEAR(beatPositionsFrames[i].value(), - importedBeatPositionsFrames[i].value(), + importedPosition.value(), kEpsilon); } + ASSERT_FALSE(pBeatsIterator->hasNext()); } qInfo() << "Step 3: Add" << kNumBeats120BPM << "beats at 100 bpm to the beatgrid"; @@ -274,14 +284,19 @@ TEST_F(SeratoBeatGridTest, SerializeBeatMap) { mixxx::SeratoBeatsImporter beatsImporter( seratoBeatGrid.nonTerminalMarkers(), seratoBeatGrid.terminalMarker()); - const QVector importedBeatPositionsFrames = - beatsImporter.importBeatsAndApplyTimingOffset(timingOffsetMillis, signalInfo); - ASSERT_EQ(beatPositionsFrames.size(), importedBeatPositionsFrames.size()); + const auto pImportedBeats = + beatsImporter.importBeatsAndApplyTimingOffset( + timingOffsetMillis, signalInfo); + auto pBeatsIterator = + pImportedBeats->findBeats(beatPositionsFrames.first() - 1000, + beatPositionsFrames.last() + 1000); for (int i = 0; i < beatPositionsFrames.size(); i++) { + const auto importedPosition = pBeatsIterator->next(); EXPECT_NEAR(beatPositionsFrames[i].value(), - importedBeatPositionsFrames[i].value(), + importedPosition.value(), kEpsilon); } + ASSERT_FALSE(pBeatsIterator->hasNext()); } } diff --git a/src/track/beatsimporter.h b/src/track/beatsimporter.h index 402889fba2..4fd8b5c4ba 100644 --- a/src/track/beatsimporter.h +++ b/src/track/beatsimporter.h @@ -5,6 +5,7 @@ #include "audio/frame.h" #include "audio/streaminfo.h" +#include "track/beats.h" namespace mixxx { @@ -16,9 +17,8 @@ class BeatsImporter { virtual bool isEmpty() const = 0; - /// Determines the timing offset and returns a Vector of frame positions - /// to use as input for the BeatMap constructor - virtual QVector importBeatsAndApplyTimingOffset( + /// Determines the timing offset and returns a Beats object. + virtual BeatsPointer importBeatsAndApplyTimingOffset( const QString& filePath, const audio::StreamInfo& streamInfo) = 0; }; diff --git a/src/track/serato/beatsimporter.cpp b/src/track/serato/beatsimporter.cpp index 12e8826c7c..b7b7bff42c 100644 --- a/src/track/serato/beatsimporter.cpp +++ b/src/track/serato/beatsimporter.cpp @@ -22,7 +22,7 @@ bool SeratoBeatsImporter::isEmpty() const { return !m_pTerminalMarker; }; -QVector SeratoBeatsImporter::importBeatsAndApplyTimingOffset( +BeatsPointer SeratoBeatsImporter::importBeatsAndApplyTimingOffset( const QString& filePath, const audio::StreamInfo& streamInfo) { const audio::SignalInfo& signalInfo = streamInfo.getSignalInfo(); const double timingOffsetMillis = SeratoTags::guessTimingOffsetMillis( @@ -31,10 +31,10 @@ QVector SeratoBeatsImporter::importBeatsAndApplyTimingOf return importBeatsAndApplyTimingOffset(timingOffsetMillis, signalInfo); } -QVector SeratoBeatsImporter::importBeatsAndApplyTimingOffset( +BeatsPointer SeratoBeatsImporter::importBeatsAndApplyTimingOffset( double timingOffsetMillis, const audio::SignalInfo& signalInfo) { VERIFY_OR_DEBUG_ASSERT(!isEmpty()) { - return {}; + return nullptr; } QVector beats; @@ -45,7 +45,7 @@ QVector SeratoBeatsImporter::importBeatsAndApplyTimingOf beatPositionMillis = static_cast(pMarker->positionSecs()) * 1000; VERIFY_OR_DEBUG_ASSERT(pMarker->positionSecs() >= 0 && pMarker->beatsTillNextMarker() > 0) { - return {}; + return nullptr; } const double nextBeatPositionMillis = static_cast(i == (m_nonTerminalMarkers.size() - 1) @@ -54,7 +54,7 @@ QVector SeratoBeatsImporter::importBeatsAndApplyTimingOf ->positionSecs()) * 1000; VERIFY_OR_DEBUG_ASSERT(nextBeatPositionMillis > beatPositionMillis) { - return {}; + return nullptr; } const double beatLengthMillis = (nextBeatPositionMillis - beatPositionMillis) / @@ -78,7 +78,7 @@ QVector SeratoBeatsImporter::importBeatsAndApplyTimingOf // | | | | | | | | |[###### Remaining range ######] const double beatLengthMillis = 60000.0 / static_cast(m_pTerminalMarker->bpm()); VERIFY_OR_DEBUG_ASSERT(m_pTerminalMarker->positionSecs() >= 0 && beatLengthMillis > 0) { - return {}; + return nullptr; } const double rangeEndBeatPositionMillis = @@ -109,7 +109,9 @@ QVector SeratoBeatsImporter::importBeatsAndApplyTimingOf m_nonTerminalMarkers.clear(); m_pTerminalMarker.reset(); - return beats; + return Beats::fromBeatPositions( + signalInfo.getSampleRate(), + beats); } } // namespace mixxx diff --git a/src/track/serato/beatsimporter.h b/src/track/serato/beatsimporter.h index 26c3ec5450..9a4923e6b1 100644 --- a/src/track/serato/beatsimporter.h +++ b/src/track/serato/beatsimporter.h @@ -4,6 +4,7 @@ #include +#include "track/beats.h" #include "track/beatsimporter.h" #include "track/serato/beatgrid.h" @@ -18,14 +19,14 @@ class SeratoBeatsImporter : public BeatsImporter { ~SeratoBeatsImporter() override = default; bool isEmpty() const override; - QVector importBeatsAndApplyTimingOffset( + BeatsPointer importBeatsAndApplyTimingOffset( const QString& filePath, const audio::StreamInfo& streamInfo) override; private: FRIEND_TEST(SeratoBeatGridTest, SerializeBeatMap); - QVector importBeatsAndApplyTimingOffset( + BeatsPointer importBeatsAndApplyTimingOffset( double timingOffsetMillis, const audio::SignalInfo& signalInfo); QList m_nonTerminalMarkers; diff --git a/src/track/track.cpp b/src/track/track.cpp index 44ce41fc1c..be290170d1 100644 --- a/src/track/track.cpp +++ b/src/track/track.cpp @@ -1081,10 +1081,9 @@ bool Track::importPendingBeatsWhileLocked() { // The sample rate is supposed to be consistent DEBUG_ASSERT(m_record.getStreamInfoFromSource()->getSignalInfo().getSampleRate() == m_record.getMetadata().getStreamInfo().getSignalInfo().getSampleRate()); - const auto pBeats = mixxx::Beats::fromBeatPositions( - m_record.getStreamInfoFromSource()->getSignalInfo().getSampleRate(), + const auto pBeats = m_pBeatsImporterPending->importBeatsAndApplyTimingOffset( - getLocation(), *m_record.getStreamInfoFromSource())); + getLocation(), *m_record.getStreamInfoFromSource()); DEBUG_ASSERT(m_pBeatsImporterPending->isEmpty()); m_pBeatsImporterPending.reset(); return setBeatsWhileLocked(pBeats); -- cgit v1.2.3