Merge pull request #4053 from Holzhaus/frame-improvements

FramePos improvements
mixxxdj · Jul 4, 2021 · 67b6af8 · 67b6af8
2 parents 5dc50d1 + 86dd12f
commit 67b6af8
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 6 deletions.
diff --git a/src/audio/frame.h b/src/audio/frame.h
@@ -15,6 +15,8 @@ typedef double FrameDiff_t;
 
 /// FramePos defines the position of a frame in a track
 /// with respect to a fixed origin, i.e. start of the track.
+///
+/// Note that all invalid frame positions are considered equal.
 class FramePos final {
   public:
     typedef double value_t;
@@ -33,22 +35,31 @@ class FramePos final {
         return FramePos(engineSamplePos / mixxx::kEngineChannelCount);
     }
 
-    constexpr double toEngineSamplePos() const {
+    double toEngineSamplePos() const {
         return value() * mixxx::kEngineChannelCount;
     }
 
+    /// Return true if the frame position is valid. Any finite value is
+    /// considered valid, i.e. any value except NaN and negative/positive
+    /// infinity.
     bool isValid() const {
-        return !util_isnan(m_framePosition) && !util_isinf(m_framePosition);
+        return util_isfinite(m_framePosition);
     }
 
     void setValue(value_t framePosition) {
         m_framePosition = framePosition;
     }
 
-    constexpr value_t value() const {
+    /// Return the underlying primitive value for this frame position.
+    value_t value() const {
+        VERIFY_OR_DEBUG_ASSERT(isValid()) {
+            return FramePos::kInvalidValue;
+        }
         return m_framePosition;
     }
 
+    /// Return true if the frame position has a fractional part, i.e. if it is
+    /// not located at a full frame boundary.
     bool isFractional() const {
         DEBUG_ASSERT(isValid());
         value_t integerPart;
@@ -62,21 +73,25 @@ class FramePos final {
     }
 
     FramePos& operator+=(FrameDiff_t increment) {
+        DEBUG_ASSERT(isValid());
         m_framePosition += increment;
         return *this;
     }
 
     FramePos& operator-=(FrameDiff_t decrement) {
+        DEBUG_ASSERT(isValid());
         m_framePosition -= decrement;
         return *this;
     }
 
     FramePos& operator*=(double multiple) {
+        DEBUG_ASSERT(isValid());
         m_framePosition *= multiple;
         return *this;
     }
 
     FramePos& operator/=(double divisor) {
+        DEBUG_ASSERT(isValid());
         m_framePosition /= divisor;
         return *this;
     }
@@ -129,15 +144,27 @@ inline bool operator>=(FramePos frame1, FramePos frame2) {
 }
 
 inline bool operator==(FramePos frame1, FramePos frame2) {
-    return frame1.value() == frame2.value();
+    if (frame1.isValid() && frame2.isValid()) {
+        return frame1.value() == frame2.value();
+    }
+
+    if (!frame1.isValid() && !frame2.isValid()) {
+        return true;
+    }
+
+    return false;
 }
 
 inline bool operator!=(FramePos frame1, FramePos frame2) {
-    return !(frame1.value() == frame2.value());
+    return !(frame1 == frame2);
 }
 
 inline QDebug operator<<(QDebug dbg, FramePos arg) {
-    dbg << arg.value();
+    if (arg.isValid()) {
+        dbg.nospace() << "FramePos(" << arg.value() << ")";
+    } else {
+        dbg << "FramePos()";
+    }
     return dbg;
 }
 

diff --git a/src/test/frametest.cpp b/src/test/frametest.cpp
@@ -35,3 +35,33 @@ TEST_F(FrameTest, TestFramePosFractional) {
     EXPECT_TRUE(mixxx::audio::FramePos(128.5).isFractional());
     EXPECT_TRUE(mixxx::audio::FramePos(135.67).isFractional());
 }
+
+TEST_F(FrameTest, TestFramePosEquality) {
+    EXPECT_EQ(mixxx::audio::FramePos(0), mixxx::audio::FramePos(0));
+    EXPECT_EQ(mixxx::audio::FramePos(-100), mixxx::audio::FramePos(-100));
+    EXPECT_EQ(mixxx::audio::FramePos(100), mixxx::audio::FramePos(100));
+    EXPECT_EQ(mixxx::audio::FramePos(50) * 2, mixxx::audio::FramePos(100));
+
+    EXPECT_NE(mixxx::audio::FramePos(100), mixxx::audio::FramePos(200));
+    EXPECT_NE(mixxx::audio::FramePos(100), mixxx::audio::FramePos());
+    EXPECT_NE(mixxx::audio::FramePos(0), mixxx::audio::FramePos());
+
+    // Check that invalid positions are equal to each other
+    EXPECT_EQ(mixxx::audio::FramePos(), mixxx::audio::kInvalidFramePos);
+    EXPECT_EQ(mixxx::audio::FramePos(),
+            mixxx::audio::FramePos(mixxx::audio::FramePos::kInvalidValue));
+    EXPECT_EQ(mixxx::audio::FramePos(),
+            mixxx::audio::FramePos(std::numeric_limits<
+                    mixxx::audio::FramePos::value_t>::quiet_NaN()));
+    EXPECT_EQ(mixxx::audio::FramePos(),
+            mixxx::audio::FramePos(std::numeric_limits<
+                    mixxx::audio::FramePos::value_t>::infinity()));
+    EXPECT_EQ(mixxx::audio::FramePos(),
+            mixxx::audio::FramePos(
+                    -std::numeric_limits<
+                            mixxx::audio::FramePos::value_t>::infinity()));
+    EXPECT_EQ(mixxx::audio::FramePos(std::numeric_limits<
+                      mixxx::audio::FramePos::value_t>::quiet_NaN()),
+            mixxx::audio::FramePos(std::numeric_limits<
+                    mixxx::audio::FramePos::value_t>::infinity()));
+}