Handle out-of-order beat packet, #55

CHANGELOG.md

@@ -20,6 +20,8 @@ This change log follows the conventions of
 - The calculation of pitch percentages from pitch values found in CDJ status packets
   was slightly inaccurate because of two transposed digits in a calculation.
 - The code that detects pre-nexus CDJs was confused by the CDJ-3000.
+- Sometimes beat packets can come after a status packet at the very beginning of a
+  new beat, and this used to cause the beat number to jump up and back down. 
 
 ### Changed
 

src/main/java/org/deepsymmetry/beatlink/data/TimeFinder.java

@@ -8,6 +8,7 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicLong;
 
@@ -153,7 +154,7 @@ private long interpolateTimeSinceUpdate(TrackPositionUpdate update, long current
         if (!update.playing) {
             return update.milliseconds;
         }
-        long elapsedMillis = (currentTimestamp - update.timestamp) / 1000000;
+        long elapsedMillis = TimeUnit.NANOSECONDS.toMillis(currentTimestamp - update.timestamp);
         long moved = Math.round(update.pitch * elapsedMillis);
         if (update.reverse) {
             return update.milliseconds - moved;
@@ -366,9 +367,27 @@ public void setSlack(long slack) {
      * should be updated
      */
     private boolean interpolationsDisagree(TrackPositionUpdate lastUpdate, TrackPositionUpdate currentUpdate) {
-        long now = System.nanoTime();
-        return Math.abs(interpolateTimeSinceUpdate(lastUpdate, now) - interpolateTimeSinceUpdate(currentUpdate, now)) >
-                (lastUpdate.playing? slack.get() : 0);  // If we are not playing, any difference is real, from a precise update.
+        final long now = System.nanoTime();
+        final long skew = Math.abs(interpolateTimeSinceUpdate(lastUpdate, now) - interpolateTimeSinceUpdate(currentUpdate, now));
+        final long tolerance = lastUpdate.playing? slack.get() : 0;  // If we are not playing, any difference is real, from a precise update.
+        if (tolerance > 0 && skew > tolerance && logger.isDebugEnabled()) {
+            logger.debug("interpolationsDisagree: updates arrived {} ms apart, last {}interpolates to {}, current {}interpolates to {}, skew {}",
+                    TimeUnit.NANOSECONDS.toMillis(currentUpdate.timestamp - lastUpdate.timestamp),
+                    (lastUpdate.fromBeat? "(beat) " : ""), interpolateTimeSinceUpdate(lastUpdate, now),
+                    (currentUpdate.fromBeat? "(beat) " : ""), interpolateTimeSinceUpdate(currentUpdate, now), skew);
+        }
+        return skew > tolerance;
+    }
+
+    private boolean pitchesDiffer(TrackPositionUpdate lastUpdate, TrackPositionUpdate currentUpdate) {
+        final double delta = Math.abs(lastUpdate.pitch - currentUpdate.pitch);
+        if (lastUpdate.precise && (lastUpdate.fromBeat != currentUpdate.fromBeat)) {
+            // We're in a precise position packet situation, so beats send pitch differently
+            return delta > 0.001;
+        } else {
+            // Pitches are comparable, we can use a tight tolerance to detect changes
+            return delta > 0.000001;
+        }
     }
 
     /**
@@ -400,7 +419,7 @@ private void updateListenersIfNeeded(int player, TrackPositionUpdate update, Bea
                     final TrackPositionUpdate lastUpdate = entry.getValue();
                     if (lastUpdate == NO_INFORMATION ||
                             lastUpdate.playing != update.playing ||
-                            Math.abs(lastUpdate.pitch - update.pitch) > 0.000001 ||
+                            pitchesDiffer(lastUpdate, update) ||
                             interpolationsDisagree(lastUpdate, update)) {
                         if (trackPositionListeners.replace(entry.getKey(), entry.getValue(), update)) {
                             try {
@@ -545,7 +564,14 @@ public void newBeat(Beat beat) {
                         // into guessing a position for that player based on no valid information.
                         return;
                     } else {
-                        beatNumber = Math.min(lastPosition.beatNumber + 1, beatGrid.beatCount);  // Handle loop at end
+                        // See if we have moved past 1/5 of the way into the current beat.
+                        final long distanceIntoBeat = lastPosition.milliseconds - beatGrid.getTimeWithinTrack(lastPosition.beatNumber);
+                        final long farEnough = 6000000 / beat.getBpm() / 5;
+                        if (distanceIntoBeat >= farEnough) {  // We can consider this the start of a new beat
+                            beatNumber = Math.min(lastPosition.beatNumber + 1, beatGrid.beatCount);  // Handle loop at end
+                        } else {  // We must have received the beat packet out of order with respect to the first status in the beat.
+                            beatNumber = lastPosition.beatNumber;
+                        }
                     }
 
                     // We know the player is playing forward because otherwise we don't get beats.

src/main/java/org/deepsymmetry/beatlink/data/TrackPositionUpdate.java

@@ -114,6 +114,6 @@ public String toString() {
         return "TrackPositionUpdate[timestamp:" + timestamp + ", milliseconds:" + milliseconds +
                 ", beatNumber:" + beatNumber + ", definitive:" + definitive + ", playing:" + playing +
                 ", pitch:" + String.format("%.2f", pitch) + ", reverse:" + reverse +
-                ", beatGrid:" + beatGrid + ", precise:" + precise + "]";
+                ", beatGrid:" + beatGrid + ", precise:" + precise + ", fromBeat:" + fromBeat + "]";
     }
 }