Merge pull request #4270 from uklotzde/makePreferredBeats

Fix debug assertion in BeatFactory::makePreferredBeats()
mixxxdj · Sep 16, 2021 · 07f675a · 07f675a
2 parents 9fc8b6b + 5794db2
commit 07f675a
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 16 deletions.
diff --git a/src/track/beatfactory.cpp b/src/track/beatfactory.cpp
@@ -55,9 +55,7 @@ mixxx::BeatsPointer BeatFactory::makePreferredBeats(
         const QHash<QString, QString>& extraVersionInfo,
         bool fixedTempo,
         mixxx::audio::SampleRate sampleRate) {
-    const QString version = getPreferredVersion(fixedTempo);
-    const QString subVersion = getPreferredSubVersion(extraVersionInfo);
-
+    DEBUG_ASSERT(sampleRate.isValid());
 #ifdef DEBUG_PRINT_BEATS
     for (mixxx::audio::FramePos beat : beats) {
         qDebug().noquote() << QString::number(beat.value(), 'g', 8);
@@ -66,28 +64,37 @@ mixxx::BeatsPointer BeatFactory::makePreferredBeats(
 
     QVector<BeatUtils::ConstRegion> constantRegions =
             BeatUtils::retrieveConstRegions(beats, sampleRate);
-
 #ifdef DEBUG_PRINT_BEATS
     for (auto& region : constantRegions) {
         qDebug().noquote() << QString::number(region.firstBeat.value(), 'g', 8)
                            << QString::number(region.beatLength, 'g', 8);
     }
 #endif
+    if (constantRegions.isEmpty()) {
+        return nullptr;
+    }
+
+    const QString version = getPreferredVersion(fixedTempo);
+    const QString subVersion = getPreferredSubVersion(extraVersionInfo);
 
     if (version == BEAT_GRID_2_VERSION) {
         mixxx::audio::FramePos firstBeat = mixxx::audio::kStartFramePos;
         const mixxx::Bpm constBPM = BeatUtils::makeConstBpm(
                 constantRegions, sampleRate, &firstBeat);
-        firstBeat = BeatUtils::adjustPhase(firstBeat, constBPM, sampleRate, beats);
-        auto pGrid = mixxx::Beats::fromConstTempo(
-                sampleRate, firstBeat.toNearestFrameBoundary(), constBPM, subVersion);
-        return pGrid;
+        if (firstBeat.isValid()) {
+            firstBeat = BeatUtils::adjustPhase(firstBeat, constBPM, sampleRate, beats);
+            auto pGrid = mixxx::Beats::fromConstTempo(
+                    sampleRate, firstBeat.toNearestFrameBoundary(), constBPM, subVersion);
+            return pGrid;
+        } else {
+            qWarning() << "Failed to create beat grid: Invalid first beat";
+        }
     } else if (version == BEAT_MAP_VERSION) {
         QVector<mixxx::audio::FramePos> ironedBeats = BeatUtils::getBeats(constantRegions);
         auto pBeatMap = mixxx::Beats::fromBeatPositions(sampleRate, ironedBeats, subVersion);
         return pBeatMap;
     } else {
         qDebug() << "ERROR: Could not determine what type of beatgrid to create.";
-        return mixxx::BeatsPointer();
     }
+    return nullptr;
 }
diff --git a/src/track/beatutils.cpp b/src/track/beatutils.cpp
@@ -55,6 +55,12 @@ mixxx::Bpm BeatUtils::calculateBpm(
 QVector<BeatUtils::ConstRegion> BeatUtils::retrieveConstRegions(
         const QVector<mixxx::audio::FramePos>& coarseBeats,
         mixxx::audio::SampleRate sampleRate) {
+    DEBUG_ASSERT(sampleRate.isValid());
+    if (coarseBeats.size() < 2) {
+        // Cannot infer a tempo for less than 2 beats
+        return {};
+    }
+
     // The QM Beat detector has a step size of 512 frames @ 44100 Hz. This means that
     // Single beats have has a jitter of +- 12 ms around the actual position.
     // Expressed in BPM it means we have for instance steps of these BPM value around 120 BPM
@@ -71,16 +77,12 @@ QVector<BeatUtils::ConstRegion> BeatUtils::retrieveConstRegions(
     // current average to adjust them by up to +-12 ms.
     // Than we start with the region from the found beat to the end.
 
-    QVector<ConstRegion> constantRegions;
-    if (coarseBeats.isEmpty()) {
-        return constantRegions;
-    }
-
     const mixxx::audio::FrameDiff_t maxPhaseError = kMaxSecsPhaseError * sampleRate;
     const mixxx::audio::FrameDiff_t maxPhaseErrorSum = kMaxSecsPhaseErrorSum * sampleRate;
     int leftIndex = 0;
     int rightIndex = coarseBeats.size() - 1;
 
+    QVector<ConstRegion> constantRegions;
     while (leftIndex < coarseBeats.size() - 1) {
         DEBUG_ASSERT(rightIndex > leftIndex);
         mixxx::audio::FrameDiff_t meanBeatLength =
@@ -143,6 +145,9 @@ mixxx::Bpm BeatUtils::makeConstBpm(
         const QVector<BeatUtils::ConstRegion>& constantRegions,
         mixxx::audio::SampleRate sampleRate,
         mixxx::audio::FramePos* pFirstBeat) {
+    DEBUG_ASSERT(!constantRegions.isEmpty());
+    DEBUG_ASSERT(sampleRate.isValid());
+    DEBUG_ASSERT(!pFirstBeat || pFirstBeat->isValid());
     // We assume here the track was recorded with an unhear-able static metronome.
     // This metronome is likely at a full BPM.
     // The track may has intros, outros and bridges without detectable beats.
@@ -175,8 +180,8 @@ mixxx::Bpm BeatUtils::makeConstBpm(
     }
 
     if (longestRegionLength == 0) {
-        // no betas, we default to
-        return mixxx::Bpm(128);
+        // Could not infer a tempo
+        return {};
     }
 
     int longestRegionNumberOfBeats = static_cast<int>(