Merge pull request #4342 from Holzhaus/beatsimporter-object

BeatsImporter: Return Beats object instead of frame position vector

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<mixxx::audio::FramePos> 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<mixxx::audio::FramePos> 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<mixxx::audio::FramePos> 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<mixxx::audio::FramePos> 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<mixxx::audio::FramePos> 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<mixxx::audio::FramePos> SeratoBeatsImporter::importBeatsAndApplyTimingOf
     return importBeatsAndApplyTimingOffset(timingOffsetMillis, signalInfo);
 }
 
-QVector<mixxx::audio::FramePos> SeratoBeatsImporter::importBeatsAndApplyTimingOffset(
+BeatsPointer SeratoBeatsImporter::importBeatsAndApplyTimingOffset(
         double timingOffsetMillis, const audio::SignalInfo& signalInfo) {
     VERIFY_OR_DEBUG_ASSERT(!isEmpty()) {
-        return {};
+        return nullptr;
     }
 
     QVector<mixxx::audio::FramePos> beats;
@@ -45,7 +45,7 @@ QVector<mixxx::audio::FramePos> SeratoBeatsImporter::importBeatsAndApplyTimingOf
         beatPositionMillis = static_cast<double>(pMarker->positionSecs()) * 1000;
         VERIFY_OR_DEBUG_ASSERT(pMarker->positionSecs() >= 0 &&
                 pMarker->beatsTillNextMarker() > 0) {
-            return {};
+            return nullptr;
         }
         const double nextBeatPositionMillis =
                 static_cast<double>(i == (m_nonTerminalMarkers.size() - 1)
@@ -54,7 +54,7 @@ QVector<mixxx::audio::FramePos> SeratoBeatsImporter::importBeatsAndApplyTimingOf
                                           ->positionSecs()) *
                 1000;
         VERIFY_OR_DEBUG_ASSERT(nextBeatPositionMillis > beatPositionMillis) {
-            return {};
+            return nullptr;
         }
         const double beatLengthMillis =
                 (nextBeatPositionMillis - beatPositionMillis) /
@@ -78,7 +78,7 @@ QVector<mixxx::audio::FramePos> SeratoBeatsImporter::importBeatsAndApplyTimingOf
     //     | | | | | | | | |[###### Remaining range ######]
     const double beatLengthMillis = 60000.0 / static_cast<double>(m_pTerminalMarker->bpm());
     VERIFY_OR_DEBUG_ASSERT(m_pTerminalMarker->positionSecs() >= 0 && beatLengthMillis > 0) {
-        return {};
+        return nullptr;
     }
 
     const double rangeEndBeatPositionMillis =
@@ -109,7 +109,9 @@ QVector<mixxx::audio::FramePos> 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 <QList>
 
+#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<mixxx::audio::FramePos> importBeatsAndApplyTimingOffset(
+    BeatsPointer importBeatsAndApplyTimingOffset(
             const QString& filePath,
             const audio::StreamInfo& streamInfo) override;
 
   private:
     FRIEND_TEST(SeratoBeatGridTest, SerializeBeatMap);
 
-    QVector<mixxx::audio::FramePos> importBeatsAndApplyTimingOffset(
+    BeatsPointer importBeatsAndApplyTimingOffset(
             double timingOffsetMillis, const audio::SignalInfo& signalInfo);
 
     QList<SeratoBeatGridNonTerminalMarkerPointer> 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);