From 221747015dc1dc8b9f85a99e6d9f4864a5416a6f Mon Sep 17 00:00:00 2001
From: Jan Holthuis <jholthuis@mixxx.org>
Date: Sat, 3 Jul 2021 22:08:23 +0200
Subject: [PATCH] Bpm: Add assertions when triggering invalid behavior

---
 src/engine/enginebuffer.cpp |  5 ++-
 src/track/bpm.h             | 69 +++++++++++++++++++++++++++++++++----
 src/track/track.cpp         |  6 +++-
 3 files changed, 71 insertions(+), 9 deletions(-)

diff --git a/src/engine/enginebuffer.cpp b/src/engine/enginebuffer.cpp
index cd78646a44e..3d4ba868e5b 100644
--- a/src/engine/enginebuffer.cpp
+++ b/src/engine/enginebuffer.cpp
@@ -1297,7 +1297,10 @@ void EngineBuffer::postProcess(const int iBufferSize) {
     }
     const mixxx::Bpm localBpm = m_pBpmControl->updateLocalBpm();
     double beatDistance = m_pBpmControl->updateBeatDistance();
-    m_pSyncControl->setLocalBpm(localBpm.value());
+    // FIXME: Verify what should happend if the bpm is invalid
+    m_pSyncControl->setLocalBpm(localBpm.isValid()
+                    ? localBpm.value()
+                    : mixxx::Bpm::kValueUndefined);
     m_pSyncControl->updateAudible();
     SyncMode mode = m_pSyncControl->getSyncMode();
     if (isMaster(mode)) {
diff --git a/src/track/bpm.h b/src/track/bpm.h
index a5c937abbca..1a9337c4479 100644
--- a/src/track/bpm.h
+++ b/src/track/bpm.h
@@ -2,6 +2,7 @@
 
 #include <QtDebug>
 
+#include "util/fpclassify.h"
 #include "util/math.h"
 
 namespace mixxx {
@@ -37,13 +38,16 @@ class Bpm final {
     }
 
     static bool isValidValue(double value) {
-        return kValueMin < value;
+        return util_isfinite(value) && kValueMin < value;
     }
 
     bool isValid() const {
         return isValidValue(m_value);
     }
     double value() const {
+        VERIFY_OR_DEBUG_ASSERT(isValid()) {
+            return mixxx::Bpm::kValueUndefined;
+        }
         return m_value;
     }
     void setValue(double value) {
@@ -84,22 +88,34 @@ class Bpm final {
     }
 
     Bpm& operator+=(double increment) {
-        m_value += increment;
+        DEBUG_ASSERT(isValid());
+        if (isValid()) {
+            m_value += increment;
+        }
         return *this;
     }
 
     Bpm& operator-=(double decrement) {
-        m_value -= decrement;
+        DEBUG_ASSERT(isValid());
+        if (isValid()) {
+            m_value -= decrement;
+        }
         return *this;
     }
 
     Bpm& operator*=(double multiple) {
-        m_value *= multiple;
+        DEBUG_ASSERT(isValid());
+        if (isValid()) {
+            m_value *= multiple;
+        }
         return *this;
     }
 
     Bpm& operator/=(double divisor) {
-        m_value /= divisor;
+        DEBUG_ASSERT(isValid());
+        if (isValid()) {
+            m_value /= divisor;
+        }
         return *this;
     }
 
@@ -109,16 +125,28 @@ class Bpm final {
 
 /// Bpm can be added to a double
 inline Bpm operator+(Bpm bpm, double bpmDiff) {
+    VERIFY_OR_DEBUG_ASSERT(bpm.isValid()) {
+        return Bpm();
+    }
+
     return Bpm(bpm.value() + bpmDiff);
 }
 
 /// Bpm can be subtracted from a double
 inline Bpm operator-(Bpm bpm, double bpmDiff) {
+    VERIFY_OR_DEBUG_ASSERT(bpm.isValid()) {
+        return Bpm();
+    }
+
     return Bpm(bpm.value() - bpmDiff);
 }
 
 /// Two Bpm values can be subtracted to get a double
 inline double operator-(Bpm bpm1, Bpm bpm2) {
+    VERIFY_OR_DEBUG_ASSERT(bpm1.isValid() && bpm2.isValid()) {
+        return mixxx::Bpm::kValueUndefined;
+    }
+
     return bpm1.value() - bpm2.value();
 }
 
@@ -126,39 +154,66 @@ inline double operator-(Bpm bpm1, Bpm bpm2) {
 
 /// Bpm can be multiplied or divided by a double
 inline Bpm operator*(Bpm bpm, double multiple) {
+    VERIFY_OR_DEBUG_ASSERT(bpm.isValid()) {
+        return Bpm();
+    }
+
     return Bpm(bpm.value() * multiple);
 }
 
 inline Bpm operator/(Bpm bpm, double divisor) {
+    VERIFY_OR_DEBUG_ASSERT(bpm.isValid()) {
+        return Bpm();
+    }
+
     return Bpm(bpm.value() / divisor);
 }
 
 inline bool operator<(Bpm bpm1, Bpm bpm2) {
+    VERIFY_OR_DEBUG_ASSERT(bpm1.isValid() && bpm2.isValid()) {
+        return false;
+    }
     return bpm1.value() < bpm2.value();
 }
 
 inline bool operator<=(Bpm bpm1, Bpm bpm2) {
+    VERIFY_OR_DEBUG_ASSERT(bpm1.isValid() && bpm2.isValid()) {
+        return !bpm1.isValid() && !bpm2.isValid();
+    }
     return bpm1.value() <= bpm2.value();
 }
 
 inline bool operator>(Bpm bpm1, Bpm bpm2) {
+    VERIFY_OR_DEBUG_ASSERT(bpm1.isValid() && bpm2.isValid()) {
+        return false;
+    }
     return bpm1.value() > bpm2.value();
 }
 
 inline bool operator>=(Bpm bpm1, Bpm bpm2) {
+    VERIFY_OR_DEBUG_ASSERT(bpm1.isValid() && bpm2.isValid()) {
+        return !bpm1.isValid() && !bpm2.isValid();
+    }
     return bpm1.value() >= bpm2.value();
 }
 
 inline bool operator==(Bpm bpm1, Bpm bpm2) {
+    if (!bpm1.isValid() || !bpm2.isValid()) {
+        return !bpm1.isValid() && !bpm2.isValid();
+    }
     return bpm1.value() == bpm2.value();
 }
 
 inline bool operator!=(Bpm bpm1, Bpm bpm2) {
-    return !(bpm1.value() == bpm2.value());
+    return !(bpm1 == bpm2);
 }
 
 inline QDebug operator<<(QDebug dbg, Bpm arg) {
-    dbg << arg.value();
+    if (arg.isValid()) {
+        dbg << arg.value();
+    } else {
+        dbg.nospace() << "<Invalid FramePos>";
+    }
     return dbg;
 }
 }
diff --git a/src/track/track.cpp b/src/track/track.cpp
index 2729de23399..f3538924cfd 100644
--- a/src/track/track.cpp
+++ b/src/track/track.cpp
@@ -363,7 +363,11 @@ bool Track::trySetBpmWhileLocked(double bpmValue) {
 
 double Track::getBpm() const {
     const QMutexLocker lock(&m_qMutex);
-    return getBpmWhileLocked().value();
+    const auto bpm = getBpmWhileLocked();
+    if (!bpm.isValid()) {
+        return mixxx::Bpm::kValueUndefined;
+    }
+    return bpm.value();
 }
 
 bool Track::trySetBpm(double bpmValue) {