From 52f4b638b155b4b7f7f6a0bd14e6e9661d5cceba Mon Sep 17 00:00:00 2001 From: Joey Parrish Date: Thu, 12 Jan 2023 09:41:14 -0800 Subject: [PATCH] fix: Fix potential duplicate segments, AV sync issues (#4884) Make SegmentIndex robust against rounding errors so that we don't end up with duplicate segments on merge. In sequence mode, this would cause a massive AV sync issue of 1 segment duration. Issue #4589 --- lib/media/segment_index.js | 5 ++++- test/media/segment_index_unit.js | 28 ++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/lib/media/segment_index.js b/lib/media/segment_index.js index f5db674682..913ecdb55f 100644 --- a/lib/media/segment_index.js +++ b/lib/media/segment_index.js @@ -225,8 +225,11 @@ shaka.media.SegmentIndex = class { // Partial segments are used for live edge, and should be removed when they // get older. Remove the old SegmentReferences after the first new // reference's start time. + // Use times rounded to the millisecond for filtering purposes, so that + // tiny rounding errors will not result in duplicate segments in the index. + const firstStartTime = Math.round(references[0].startTime * 1000) / 1000; this.references = this.references.filter((r) => { - return r.startTime < references[0].startTime; + return (Math.round(r.startTime * 1000) / 1000) < firstStartTime; }); this.references.push(...references); diff --git a/test/media/segment_index_unit.js b/test/media/segment_index_unit.js index df8d492735..51abdc9afb 100644 --- a/test/media/segment_index_unit.js +++ b/test/media/segment_index_unit.js @@ -532,6 +532,34 @@ describe('SegmentIndex', /** @suppress {accessControls} */ () => { goog.asserts.assert(position1 != null, 'Position should not be null!'); expect(index1.get(position1)).toBe(references2[1]); }); + + it('does not duplicate references with rounding errors', () => { + /** @type {!Array.} */ + const references1 = [ + makeReference(uri(10), 10, 20), + makeReference(uri(20), 20, 30), + ]; + const index1 = new shaka.media.SegmentIndex(references1); + + // 0.24 microseconds: an insignificant rounding error. + const tinyError = 0.24e-6; + + /** @type {!Array.} */ + const references2 = [ + makeReference(uri(10), 10, 20), + // The difference between this and the equivalent old reference is an + // insignificant rounding error. + makeReference(uri(20), 20 + tinyError, 30 + tinyError), + makeReference(uri(30), 30 + tinyError, 40), + ]; + + index1.merge(references2); + expect(index1.references.length).toBe(3); + expect(index1.references[0]).toEqual(references1[0]); + // The new references replaced the old one. + expect(index1.references[1]).toEqual(references2[1]); + expect(index1.references[2]).toEqual(references2[2]); + }); }); describe('evict', () => {