From 75a8fead4e0ff84bd12bb082591c32dd48f39a46 Mon Sep 17 00:00:00 2001 From: Jamie Stackhouse Date: Wed, 29 Jul 2020 20:08:11 -0300 Subject: [PATCH 1/6] Fix PTS calculation on rollover. When a rollover was occuring the timeoffset value was incorrectly calculated due to taking the minimum PTS from the new set of rollovers, this lead to a very large delta on the PTS for the audio and led to incorrect PTS values. This happened to be fixed by the silent gap code just below, which set the sample pts and dts to be the expected next value, but this now makes it so on well-formed media, the else branch is essentially a no-op because the delta is zero. --- src/demux/tsdemuxer.js | 12 ++---------- src/remux/mp4-remuxer.js | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/demux/tsdemuxer.js b/src/demux/tsdemuxer.js index 441b46c5d68..777d209dd9f 100644 --- a/src/demux/tsdemuxer.js +++ b/src/demux/tsdemuxer.js @@ -487,22 +487,14 @@ class TSDemuxer { (frag[11] & 0xFE) * 16384 +// 1 << 14 (frag[12] & 0xFF) * 128 +// 1 << 7 (frag[13] & 0xFE) / 2; - // check if greater than 2^32 -1 - if (pesPts > 4294967295) { - // decrement 2^33 - pesPts -= 8589934592; - } + if (pesFlags & 0x40) { pesDts = (frag[14] & 0x0E) * 536870912 +// 1 << 29 (frag[15] & 0xFF) * 4194304 +// 1 << 22 (frag[16] & 0xFE) * 16384 +// 1 << 14 (frag[17] & 0xFF) * 128 +// 1 << 7 (frag[18] & 0xFE) / 2; - // check if greater than 2^32 -1 - if (pesDts > 4294967295) { - // decrement 2^33 - pesDts -= 8589934592; - } + if (pesPts - pesDts > 60 * 90000) { logger.warn(`${Math.round((pesPts - pesDts) / 90000)}s delta between PTS and DTS, align them`); pesPts = pesDts; diff --git a/src/remux/mp4-remuxer.js b/src/remux/mp4-remuxer.js index 42c05cbe240..6cdf042cb5b 100644 --- a/src/remux/mp4-remuxer.js +++ b/src/remux/mp4-remuxer.js @@ -50,7 +50,21 @@ class MP4Remuxer { // if first audio DTS is not aligned with first video DTS then we need to take that into account // when providing timeOffset to remuxAudio / remuxVideo. if we don't do that, there might be a permanent / small // drift between audio and video streams - const startPTS = videoTrack.samples.reduce((minPTS, sample) => Math.min(minPTS, sample.pts), videoTrack.samples[0].pts); + let rolloverDetected = false; + const startPTS = videoTrack.samples.reduce((minPTS, sample) => { + const delta = sample.pts - minPTS; + if (delta < -4294967296) { // 2^32, see PTSNormalize for reasoning, but we're hitting a rollover here, and we don't want that to impact the timeOffset calculation + rolloverDetected = true; + return minPTS; + } else if (delta > 0) { + return minPTS; + } else { + return sample.pts; + } + }, videoTrack.samples[0].pts); + if (rolloverDetected) { + logger.debug('PTS rollover detected'); + } const tsDelta = audioTrack.samples[0].pts - startPTS; const audiovideoTimestampDelta = tsDelta / videoTrack.inputTimeScale; audioTimeOffset += Math.max(0, audiovideoTimestampDelta); From 8861c1e60b680a87fdb3a7cfe653ec66d890a5f0 Mon Sep 17 00:00:00 2001 From: Rob Walch Date: Thu, 30 Jul 2020 13:48:06 -0400 Subject: [PATCH 2/6] Add method to find start pts #2930 --- src/remux/mp4-remuxer.js | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/remux/mp4-remuxer.js b/src/remux/mp4-remuxer.js index 6cdf042cb5b..9964a5ed2c8 100644 --- a/src/remux/mp4-remuxer.js +++ b/src/remux/mp4-remuxer.js @@ -34,6 +34,25 @@ class MP4Remuxer { this.ISGenerated = false; } + getVideoStartPts (videoSamples) { + let rolloverDetected = false; + const startPTS = videoSamples.reduce((minPTS, sample) => { + const delta = sample.pts - minPTS; + if (delta < -4294967296) { // 2^32, see PTSNormalize for reasoning, but we're hitting a rollover here, and we don't want that to impact the timeOffset calculation + rolloverDetected = true; + return minPTS; + } else if (delta > 0) { + return minPTS; + } else { + return sample.pts; + } + }, videoSamples[0].pts); + if (rolloverDetected) { + logger.debug('PTS rollover detected'); + } + return startPTS; + } + remux (audioTrack, videoTrack, id3Track, textTrack, timeOffset, contiguous, accurateTimeOffset) { // generate Init Segment if needed if (!this.ISGenerated) { @@ -50,21 +69,7 @@ class MP4Remuxer { // if first audio DTS is not aligned with first video DTS then we need to take that into account // when providing timeOffset to remuxAudio / remuxVideo. if we don't do that, there might be a permanent / small // drift between audio and video streams - let rolloverDetected = false; - const startPTS = videoTrack.samples.reduce((minPTS, sample) => { - const delta = sample.pts - minPTS; - if (delta < -4294967296) { // 2^32, see PTSNormalize for reasoning, but we're hitting a rollover here, and we don't want that to impact the timeOffset calculation - rolloverDetected = true; - return minPTS; - } else if (delta > 0) { - return minPTS; - } else { - return sample.pts; - } - }, videoTrack.samples[0].pts); - if (rolloverDetected) { - logger.debug('PTS rollover detected'); - } + const startPTS = this.getVideoStartPts(videoTrack.samples); const tsDelta = audioTrack.samples[0].pts - startPTS; const audiovideoTimestampDelta = tsDelta / videoTrack.inputTimeScale; audioTimeOffset += Math.max(0, audiovideoTimestampDelta); @@ -177,7 +182,7 @@ class MP4Remuxer { } }; if (computePTSDTS) { - const startPTS = videoSamples.reduce((minPTS, sample) => Math.min(minPTS, sample.pts), videoSamples[0].pts); + const startPTS = this.getVideoStartPts(videoSamples); const startOffset = Math.round(inputTimeScale * timeOffset); initDTS = Math.min(initDTS, videoSamples[0].dts - startOffset); initPTS = Math.min(initPTS, startPTS - startOffset); From 3bc8ea97574e200743a175e6372fca937ac01eb9 Mon Sep 17 00:00:00 2001 From: Rob Walch Date: Fri, 31 Jul 2020 14:07:37 -0400 Subject: [PATCH 3/6] Apply PTSNormalize to ID3 and Text samples to handle timestamp rollover --- src/remux/mp4-remuxer.js | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/remux/mp4-remuxer.js b/src/remux/mp4-remuxer.js index 9964a5ed2c8..2aa24dc1f3e 100644 --- a/src/remux/mp4-remuxer.js +++ b/src/remux/mp4-remuxer.js @@ -755,7 +755,7 @@ class MP4Remuxer { this.remuxAudio(track, timeOffset, contiguous); } - remuxID3 (track) { + remuxID3 (track, timeOffset) { const length = track.samples.length; if (!length) { return; @@ -768,8 +768,8 @@ class MP4Remuxer { const sample = track.samples[index]; // setting id3 pts, dts to relative time // using this._initPTS and this._initDTS to calculate relative time - sample.pts = ((sample.pts - initPTS) / inputTimeScale); - sample.dts = ((sample.dts - initDTS) / inputTimeScale); + sample.pts = PTSNormalize(sample.pts - initPTS, timeOffset * inputTimeScale) / inputTimeScale; + sample.dts = PTSNormalize(sample.dts - initDTS, timeOffset * inputTimeScale) / inputTimeScale; } this.observer.trigger(Event.FRAG_PARSING_METADATA, { samples: track.samples @@ -778,22 +778,21 @@ class MP4Remuxer { track.samples = []; } - remuxText (track) { - track.samples.sort(function (a, b) { - return (a.pts - b.pts); - }); - - let length = track.samples.length, sample; + remuxText (track, timeOffset) { + const length = track.samples.length; const inputTimeScale = track.inputTimeScale; const initPTS = this._initPTS; // consume samples if (length) { for (let index = 0; index < length; index++) { - sample = track.samples[index]; + const sample = track.samples[index]; // setting text pts, dts to relative time // using this._initPTS and this._initDTS to calculate relative time - sample.pts = ((sample.pts - initPTS) / inputTimeScale); + sample.pts = PTSNormalize(sample.pts - initPTS, timeOffset * inputTimeScale) / inputTimeScale; } + track.samples.sort(function (a, b) { + return (a.pts - b.pts); + }); this.observer.trigger(Event.FRAG_PARSING_USERDATA, { samples: track.samples }); From 12c5ea8a5d270d5ca4b6da02f0452bd7b3349a92 Mon Sep 17 00:00:00 2001 From: Rob Walch Date: Sat, 1 Aug 2020 17:31:14 -0400 Subject: [PATCH 4/6] Handle DTS wrapping in initial AVC CTS calculation --- src/remux/mp4-remuxer.js | 2 +- tests/functional/auto/setup.js | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/remux/mp4-remuxer.js b/src/remux/mp4-remuxer.js index 2aa24dc1f3e..5897f32c847 100644 --- a/src/remux/mp4-remuxer.js +++ b/src/remux/mp4-remuxer.js @@ -232,7 +232,7 @@ class MP4Remuxer { if (!contiguous) { const pts = timeOffset * timeScale; - const cts = inputSamples[0].pts - inputSamples[0].dts; + const cts = inputSamples[0].pts - PTSNormalize(inputSamples[0].dts, inputSamples[0].pts); // if not contiguous, let's use target timeOffset nextAvcDts = pts - cts; } diff --git a/tests/functional/auto/setup.js b/tests/functional/auto/setup.js index e173e67bf73..da70f43147c 100644 --- a/tests/functional/auto/setup.js +++ b/tests/functional/auto/setup.js @@ -123,18 +123,20 @@ async function testSmoothSwitch (url, config) { const callback = arguments[arguments.length - 1]; window.startStream(url, config, callback); const video = window.video; - window.hls.once(window.Hls.Events.FRAG_CHANGED, function (event, data) { + window.hls.once(window.Hls.Events.FRAG_CHANGED, function (eventName, data) { + console.log('[test] > ' + eventName + ' frag.level: ' + data.frag.level); window.switchToHighestLevel('next'); }); - window.hls.on(window.Hls.Events.LEVEL_SWITCHED, function (event, data) { - console.log('[test] > level switched: ' + data.level); + window.hls.on(window.Hls.Events.LEVEL_SWITCHED, function (eventName, data) { + console.log('[test] > ' + eventName + ' data.level: ' + data.level); let currentTime = video.currentTime; - if (data.level === window.hls.levels.length - 1) { - console.log('[test] > switched on level: ' + data.level); + const highestLevel = (window.hls.levels.length - 1); + if (data.level === highestLevel) { window.setTimeout(function () { let newCurrentTime = video.currentTime; - console.log('[test] > currentTime delta : ' + (newCurrentTime - currentTime)); + console.log('[test] > currentTime delta: ' + (newCurrentTime - currentTime)); callback({ + highestLevel: highestLevel, currentTimeDelta: newCurrentTime - currentTime, logs: window.logString }); From 0ab4fc6e1364ebde5c0f926a99478913be8a04be Mon Sep 17 00:00:00 2001 From: Jamie Stackhouse Date: Sat, 1 Aug 2020 19:56:49 -0300 Subject: [PATCH 5/6] Minimal Logging on Functional Tests. --- tests/functional/auto/setup.js | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/tests/functional/auto/setup.js b/tests/functional/auto/setup.js index da70f43147c..2079e3da7eb 100644 --- a/tests/functional/auto/setup.js +++ b/tests/functional/auto/setup.js @@ -22,6 +22,8 @@ const browserConfig = { */ let browser; let stream; +let printDebugLogs = false; + // Setup browser config data from env vars if (onTravis) { let UA = process.env.UA; @@ -337,15 +339,17 @@ describe(`testing hls.js playback in the browser on "${browserDescription}"`, fu beforeEach(async function () { try { await retry(async () => { - console.log('Loading test page...'); + if (printDebugLogs) { + console.log('Loading test page...'); + } try { await browser.get(`http://${hostname}:8000/tests/functional/auto/index.html`); } catch (e) { throw new Error('failed to open test page'); } - console.log('Test page loaded.'); - - console.log('Locating ID \'hlsjs-functional-tests\''); + if (printDebugLogs) { + console.log('Test page loaded.'); + } try { await browser.wait( until.elementLocated(By.css('body#hlsjs-functional-tests')), @@ -357,7 +361,9 @@ describe(`testing hls.js playback in the browser on "${browserDescription}"`, fu console.log(source); throw e; } - console.log('Located the ID, page confirmed loaded'); + if (printDebugLogs) { + console.log('Test harness found, page confirmed loaded'); + } }); } catch (e) { throw new Error(`error getting test page loaded: ${e}`); @@ -365,10 +371,10 @@ describe(`testing hls.js playback in the browser on "${browserDescription}"`, fu }); afterEach(async function () { - // if (onTravis || (!onTravis && this.currentTest.isFailed())) { + if (printDebugLogs || this.currentTest.isFailed()) { const logString = await browser.executeScript('return logString'); console.log(`${onTravis ? 'travis_fold:start:debug_logs' : ''}\n${logString}\n${onTravis ? 'travis_fold:end:debug_logs' : ''}`); - // } + } }); after(async function () { From cf9a9690f67c3191727b2b10e8d619f38492a93e Mon Sep 17 00:00:00 2001 From: Jamie Stackhouse Date: Sun, 2 Aug 2020 02:14:47 -0300 Subject: [PATCH 6/6] Package lock update. --- package-lock.json | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/package-lock.json b/package-lock.json index 8e8f6d8b307..498336fe93e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7777,7 +7777,7 @@ }, "browserify-aes": { "version": "1.2.0", - "resolved": "http://registry.npmjs.org/browserify-aes/-/browserify-aes-1.2.0.tgz", + "resolved": "https://registry.npmjs.org/browserify-aes/-/browserify-aes-1.2.0.tgz", "integrity": "sha512-+7CHXqGuspUn/Sl5aO7Ea0xWGAtETPXNSAjHo48JfLdPWcMng33Xe4znFvQweqc/uzk5zSOI3H52CYnjCfb5hA==", "dev": true, "requires": { @@ -9408,7 +9408,7 @@ }, "create-hash": { "version": "1.2.0", - "resolved": "http://registry.npmjs.org/create-hash/-/create-hash-1.2.0.tgz", + "resolved": "https://registry.npmjs.org/create-hash/-/create-hash-1.2.0.tgz", "integrity": "sha512-z00bCGNHDG8mHAkP7CtT1qVu+bFQUPjYq/4Iv3C3kWjTFV10zIjfSoeqXo9Asws8gwSHDGj/hl2u4OGIjapeCg==", "dev": true, "requires": { @@ -9421,7 +9421,7 @@ }, "create-hmac": { "version": "1.1.7", - "resolved": "http://registry.npmjs.org/create-hmac/-/create-hmac-1.1.7.tgz", + "resolved": "https://registry.npmjs.org/create-hmac/-/create-hmac-1.1.7.tgz", "integrity": "sha512-MJG9liiZ+ogc4TzUwuvbER1JRdgvUFSB5+VR/g5h82fGaIRWMWddtKBHi7/sVhfjQZ6SehlyhvQYrcYkaUIpLg==", "dev": true, "requires": { @@ -10090,7 +10090,7 @@ }, "diffie-hellman": { "version": "5.0.3", - "resolved": "http://registry.npmjs.org/diffie-hellman/-/diffie-hellman-5.0.3.tgz", + "resolved": "https://registry.npmjs.org/diffie-hellman/-/diffie-hellman-5.0.3.tgz", "integrity": "sha512-kqag/Nl+f3GwyK25fhUMYj81BUOrZ9IuJsjIcDE5icNM9FJHAVm3VcUDxdLPoQtTuUylWm6ZIknYJwwaPxsUzg==", "dev": true, "requires": { @@ -14041,7 +14041,7 @@ }, "ip": { "version": "1.1.5", - "resolved": "http://registry.npmjs.org/ip/-/ip-1.1.5.tgz", + "resolved": "https://registry.npmjs.org/ip/-/ip-1.1.5.tgz", "integrity": "sha1-vd7XARQpCCjAoDnnLvJfWq7ENUo=", "dev": true }, @@ -19820,7 +19820,7 @@ }, "sha.js": { "version": "2.4.11", - "resolved": "http://registry.npmjs.org/sha.js/-/sha.js-2.4.11.tgz", + "resolved": "https://registry.npmjs.org/sha.js/-/sha.js-2.4.11.tgz", "integrity": "sha512-QMEp5B7cftE7APOjk5Y6xgrbWu+WkLVQwk8JNjZ8nKRciZaByEW6MubieAiToS7+dwvrjGhH8jRXz3MVd0AYqQ==", "dev": true, "requires": { @@ -21262,7 +21262,7 @@ }, "tty-browserify": { "version": "0.0.0", - "resolved": "http://registry.npmjs.org/tty-browserify/-/tty-browserify-0.0.0.tgz", + "resolved": "https://registry.npmjs.org/tty-browserify/-/tty-browserify-0.0.0.tgz", "integrity": "sha1-oVe6QC2iTpv5V/mqadUk7tQpAaY=", "dev": true },