fix: use TIME_FUDGE_FACTOR rather than rounding by decimal digits (#881)

Currently, we ran into an issue with roundSignificantDigit because it is rounding integer whole numbers when it seems like it was only made to round floats. This can cause us to download multiple duplicate segments on slow connections. Instead, only round by TIME_FUDGE_FACTOR here. This will prevent us from having to do any rounding at all.

src/playlist.js

@@ -5,6 +5,7 @@
  */
 import videojs from 'video.js';
 import window from 'global/window';
+import {TIME_FUDGE_FACTOR} from './ranges.js';
 
 const {createTimeRange} = videojs;
 
@@ -329,32 +330,6 @@ export const seekable = function(playlist, expired, liveEdgePadding) {
   return createTimeRange(seekableStart, seekableEnd);
 };
 
-const isWholeNumber = function(num) {
-  return (num - Math.floor(num)) === 0;
-};
-
-const roundSignificantDigit = function(increment, num) {
-  // If we have a whole number, just add 1 to it
-  if (isWholeNumber(num)) {
-    return num + (increment * 0.1);
-  }
-
-  const numDecimalDigits = num.toString().split('.')[1].length;
-
-  for (let i = 1; i <= numDecimalDigits; i++) {
-    const scale = Math.pow(10, i);
-    const temp = num * scale;
-
-    if (isWholeNumber(temp) ||
-        i === numDecimalDigits) {
-      return (temp + increment) / scale;
-    }
-  }
-};
-
-const ceilLeastSignificantDigit = roundSignificantDigit.bind(null, 1);
-const floorLeastSignificantDigit = roundSignificantDigit.bind(null, -1);
-
 /**
  * Determine the index and estimated starting time of the segment that
  * contains a specified playback position in a media playlist.
@@ -384,7 +359,7 @@ export const getMediaInfoForTime = function(
     if (startIndex > 0) {
       for (i = startIndex - 1; i >= 0; i--) {
         segment = playlist.segments[i];
-        time += floorLeastSignificantDigit(segment.duration);
+        time += (segment.duration + TIME_FUDGE_FACTOR);
         if (time > 0) {
           return {
             mediaIndex: i,
@@ -421,7 +396,7 @@ export const getMediaInfoForTime = function(
   // until we find a segment that contains `time` and return it
   for (i = startIndex; i < numSegments; i++) {
     segment = playlist.segments[i];
-    time -= ceilLeastSignificantDigit(segment.duration);
+    time -= segment.duration + TIME_FUDGE_FACTOR;
     if (time < 0) {
       return {
         mediaIndex: i,

test/playlist.test.js

@@ -1226,6 +1226,98 @@ QUnit.test(
   }
 );
 
+QUnit.test('rounding down works', function(assert) {
+  const loader = new PlaylistLoader('media.m3u8', this.fakeVhs);
+
+  loader.load();
+
+  this.requests.shift().respond(
+    200, null,
+    '#EXTM3U\n' +
+    '#EXT-X-MEDIA-SEQUENCE:0\n' +
+    '#EXTINF:2,\n' +
+    '0.ts\n' +
+    '#EXTINF:2,\n' +
+    '1.ts\n' +
+    '#EXTINF:2,\n' +
+    '2.ts\n' +
+    '#EXTINF:2,\n' +
+    '3.ts\n' +
+    '#EXTINF:2,\n' +
+    '4.ts\n' +
+    '#EXTINF:2,\n' +
+    '5.ts\n' +
+    '#EXT-X-ENDLIST\n'
+  );
+
+  const media = loader.media();
+  const fn = Playlist.getMediaInfoForTime;
+
+  // 1 segment away
+  assert.equal(fn(media, 2.1, 0, 0).mediaIndex, 1, '1 away 2 is correct');
+  assert.equal(fn(media, 4.1, 1, 2).mediaIndex, 2, '1 away 3 is correct ');
+  assert.equal(fn(media, 6.1, 2, 4).mediaIndex, 3, '1 away 4 is correct');
+  assert.equal(fn(media, 8.1, 3, 6).mediaIndex, 4, '1 away 5 is correct');
+  assert.equal(fn(media, 10.1, 4, 8).mediaIndex, 5, '1 away 6 is correct');
+
+  // 2 segment away
+  assert.equal(fn(media, 4.1, 0, 0).mediaIndex, 2, '2 away 3 is correct ');
+  assert.equal(fn(media, 6.1, 1, 2).mediaIndex, 3, '2 away 4 is correct');
+  assert.equal(fn(media, 8.1, 2, 4).mediaIndex, 4, '2 away 5 is correct');
+  assert.equal(fn(media, 10.1, 3, 6).mediaIndex, 5, '2 away 6 is correct');
+
+  // 3 segments away
+  assert.equal(fn(media, 6.1, 0, 0).mediaIndex, 3, '3 away 4 is correct');
+  assert.equal(fn(media, 8.1, 1, 2).mediaIndex, 4, '3 away 5 is correct');
+  assert.equal(fn(media, 10.1, 2, 4).mediaIndex, 5, '3 away 6 is correct');
+});
+
+QUnit.test('rounding up works', function(assert) {
+  const loader = new PlaylistLoader('media.m3u8', this.fakeVhs);
+
+  loader.load();
+
+  this.requests.shift().respond(
+    200, null,
+    '#EXTM3U\n' +
+    '#EXT-X-MEDIA-SEQUENCE:0\n' +
+    '#EXTINF:2,\n' +
+    '0.ts\n' +
+    '#EXTINF:2,\n' +
+    '1.ts\n' +
+    '#EXTINF:2,\n' +
+    '2.ts\n' +
+    '#EXTINF:2,\n' +
+    '3.ts\n' +
+    '#EXTINF:2,\n' +
+    '4.ts\n' +
+    '#EXTINF:2,\n' +
+    '5.ts\n' +
+    '#EXT-X-ENDLIST\n'
+  );
+
+  const media = loader.media();
+  const fn = Playlist.getMediaInfoForTime;
+
+  // 1 segment away
+  assert.equal(fn(media, 0, 1, 2).mediaIndex, 0, '1 away 1 is correct');
+  assert.equal(fn(media, 2.1, 2, 4).mediaIndex, 1, '1 away 2 is correct');
+  assert.equal(fn(media, 4.1, 3, 6).mediaIndex, 2, '1 away 3 is correct');
+  assert.equal(fn(media, 6.1, 4, 8).mediaIndex, 3, '1 away 4 is correct');
+  assert.equal(fn(media, 8.1, 5, 10).mediaIndex, 4, '1 away 5 is correct');
+
+  // 2 segment away
+  assert.equal(fn(media, 0, 2, 4).mediaIndex, 0, '2 away 1 is correct');
+  assert.equal(fn(media, 2.1, 3, 6).mediaIndex, 1, '2 away 2 is correct');
+  assert.equal(fn(media, 4.1, 4, 8).mediaIndex, 2, '2 away 3 is correct');
+  assert.equal(fn(media, 6.1, 5, 10).mediaIndex, 3, '2 away 4 is correct');
+
+  // 3 segments away
+  assert.equal(fn(media, 0, 3, 6).mediaIndex, 0, '3 away 1 is correct');
+  assert.equal(fn(media, 2.1, 4, 8).mediaIndex, 1, '3 away 2 is correct');
+  assert.equal(fn(media, 4.1, 5, 10).mediaIndex, 2, '3 away 3 is correct');
+});
+
 QUnit.test(
   'returns the lower index when calculating for a segment boundary',
   function(assert) {