From 027f117b407c97818dadd6922a9ef8492e0c8724 Mon Sep 17 00:00:00 2001 From: Dzianis Dashkevich Date: Thu, 7 Sep 2023 13:41:43 -0400 Subject: [PATCH 1/7] Add several fixes to handle streams with corrupted PTS or DTS timestamps during discontinuities. - Add feature flag to calculate timestampOffset for each segment, regardless of its timeline. - Use audio/video start time info from Transmuxer instead of probeTs. --- README.md | 6 ++++++ index.html | 5 +++++ scripts/index.js | 3 +++ src/media-segment-request.js | 9 --------- src/playlist-controller.js | 1 + src/segment-loader.js | 9 +++++++++ src/source-updater.js | 9 ++++++++- src/util/vjs-compat.js | 24 ++++++++++++++++++++++++ src/videojs-http-streaming.js | 2 ++ 9 files changed, 58 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index d581d4958..a04d1cbc9 100644 --- a/README.md +++ b/README.md @@ -463,6 +463,12 @@ This option defaults to `false`. * Default: `false` * Use [Decode Timestamp](https://www.w3.org/TR/media-source/#decode-timestamp) instead of [Presentation Timestamp](https://www.w3.org/TR/media-source/#presentation-timestamp) for [timestampOffset](https://www.w3.org/TR/media-source/#dom-sourcebuffer-timestampoffset) calculation. This option was introduced to align with DTS-based browsers. This option affects only transmuxed data (eg: transport stream). For more info please check the following [issue](https://github.com/videojs/http-streaming/issues/1247). +##### calculateTimestampOffsetForEachSegment +* Type: `boolean`, +* Default: `false` +* Calculate timestampOffset for each segment, regardless of its timeline. Sometimes it is helpful when you have corrupted DTS/PTS timestamps during discontinuities. + + ##### useForcedSubtitles * Type: `boolean` * Default: `false` diff --git a/index.html b/index.html index 8946221a5..0db8a913c 100644 --- a/index.html +++ b/index.html @@ -144,6 +144,11 @@ +
+ + +
+
diff --git a/scripts/index.js b/scripts/index.js index 65ef4e672..301555b0d 100644 --- a/scripts/index.js +++ b/scripts/index.js @@ -470,6 +470,7 @@ 'pixel-diff-selector', 'network-info', 'dts-offset', + 'offset-each-segment', 'override-native', 'preload', 'mirror-source', @@ -525,6 +526,7 @@ 'pixel-diff-selector', 'network-info', 'dts-offset', + 'offset-each-segment', 'exact-manifest-timings', 'forced-subtitles' ].forEach(function(name) { @@ -609,6 +611,7 @@ leastPixelDiffSelector: getInputValue(stateEls['pixel-diff-selector']), useNetworkInformationApi: getInputValue(stateEls['network-info']), useDtsForTimestampOffset: getInputValue(stateEls['dts-offset']), + calculateTimestampOffsetForEachSegment: getInputValue(stateEls['offset-each-segment']), useForcedSubtitles: getInputValue(stateEls['forced-subtitles']) } } diff --git a/src/media-segment-request.js b/src/media-segment-request.js index 086f0fe48..bc43779fd 100644 --- a/src/media-segment-request.js +++ b/src/media-segment-request.js @@ -389,15 +389,6 @@ const transmuxAndNotify = ({ isMuxed }); trackInfoFn = null; - - if (probeResult.hasAudio && !isMuxed) { - audioStartFn(probeResult.audioStart); - } - if (probeResult.hasVideo) { - videoStartFn(probeResult.videoStart); - } - audioStartFn = null; - videoStartFn = null; } finish(); diff --git a/src/playlist-controller.js b/src/playlist-controller.js index 1767409c3..98ac9149d 100644 --- a/src/playlist-controller.js +++ b/src/playlist-controller.js @@ -239,6 +239,7 @@ export class PlaylistController extends videojs.EventTarget { vhs: this.vhs_, parse708captions: options.parse708captions, useDtsForTimestampOffset: options.useDtsForTimestampOffset, + calculateTimestampOffsetForEachSegment: options.calculateTimestampOffsetForEachSegment, captionServices, mediaSource: this.mediaSource, currentTime: this.tech_.currentTime.bind(this.tech_), diff --git a/src/segment-loader.js b/src/segment-loader.js index 6ccc9b7a6..3c763c7c5 100644 --- a/src/segment-loader.js +++ b/src/segment-loader.js @@ -190,6 +190,8 @@ const timingInfoPropertyForMedia = (mediaType) => `${mediaType}TimingInfo`; * The estimated segment start * @param {TimeRange[]} buffered * The loader's buffer + * @param {boolean} calculateTimestampOffsetForEachSegment + * Feature flag to always calculate timestampOffset * @param {boolean} overrideCheck * If true, no checks are made to see if the timestamp offset value should be set, * but sets it directly to a value. @@ -203,8 +205,13 @@ export const timestampOffsetForSegment = ({ currentTimeline, startOfSegment, buffered, + calculateTimestampOffsetForEachSegment, overrideCheck }) => { + if (calculateTimestampOffsetForEachSegment) { + return buffered.length ? buffered.end(buffered.length - 1) : startOfSegment; + } + // Check to see if we are crossing a discontinuity to see if we need to set the // timestamp offset on the transmuxer and source buffer. // @@ -559,6 +566,7 @@ export default class SegmentLoader extends videojs.EventTarget { this.shouldSaveSegmentTimingInfo_ = true; this.parse708captions_ = settings.parse708captions; this.useDtsForTimestampOffset_ = settings.useDtsForTimestampOffset; + this.calculateTimestampOffsetForEachSegment_ = settings.calculateTimestampOffsetForEachSegment; this.captionServices_ = settings.captionServices; this.exactManifestTimings = settings.exactManifestTimings; this.addMetadataToTextTrack = settings.addMetadataToTextTrack; @@ -1586,6 +1594,7 @@ export default class SegmentLoader extends videojs.EventTarget { currentTimeline: this.currentTimeline_, startOfSegment, buffered: this.buffered_(), + calculateTimestampOffsetForEachSegment: this.calculateTimestampOffsetForEachSegment_, overrideCheck }); diff --git a/src/source-updater.js b/src/source-updater.js index a7f3f11da..78e42606e 100644 --- a/src/source-updater.js +++ b/src/source-updater.js @@ -9,7 +9,7 @@ import {getMimeForCodec} from '@videojs/vhs-utils/es/codecs.js'; import window from 'global/window'; import toTitleCase from './util/to-title-case.js'; import { QUOTA_EXCEEDED_ERR } from './error-codes'; -import {createTimeRanges} from './util/vjs-compat'; +import {createTimeRanges, prettyBuffered} from './util/vjs-compat'; const bufferTypes = [ 'video', @@ -297,6 +297,13 @@ const pushQueue = ({type, sourceUpdater, action, doneFn, name}) => { }; const onUpdateend = (type, sourceUpdater) => (e) => { + const sourceBuffer = sourceUpdater[`${type}Buffer`]; + + if (sourceBuffer) { + const bufferedAsString = prettyBuffered(sourceBuffer.buffered); + sourceUpdater.logger_(`${type} source buffer update end. Buffered: \n`, bufferedAsString); + } + // Although there should, in theory, be a pending action for any updateend receieved, // there are some actions that may trigger updateend events without set definitions in // the w3c spec. For instance, setting the duration on the media source may trigger diff --git a/src/util/vjs-compat.js b/src/util/vjs-compat.js index d51a4b963..44dc35bcf 100644 --- a/src/util/vjs-compat.js +++ b/src/util/vjs-compat.js @@ -24,3 +24,27 @@ export function createTimeRanges(...args) { return fn.apply(context, args); } + +/** + * Converts any buffered time range to a descriptive string + * @param {TimeRanges} buffered - time ranges + * @return {string} - descriptive string + */ +export function prettyBuffered (buffered) { + let result = ''; + + for (let i = 0; i < buffered.length; i++) { + const start = buffered.start(i); + const end = buffered.end(i); + + const duration = end - start; + + if (result.length) { + result += '\n'; + } + + result += `[${duration}](${start} -> ${end})`; + } + + return result || 'empty'; +} diff --git a/src/videojs-http-streaming.js b/src/videojs-http-streaming.js index f484d13c5..a1f62bc20 100644 --- a/src/videojs-http-streaming.js +++ b/src/videojs-http-streaming.js @@ -690,6 +690,7 @@ class VhsHandler extends Component { this.options_.useForcedSubtitles = this.options_.useForcedSubtitles || false; this.options_.useNetworkInformationApi = this.options_.useNetworkInformationApi || false; this.options_.useDtsForTimestampOffset = this.options_.useDtsForTimestampOffset || false; + this.options_.calculateTimestampOffsetForEachSegment = this.options_.calculateTimestampOffsetForEachSegment || false; this.options_.customTagParsers = this.options_.customTagParsers || []; this.options_.customTagMappers = this.options_.customTagMappers || []; this.options_.cacheEncryptionKeys = this.options_.cacheEncryptionKeys || false; @@ -743,6 +744,7 @@ class VhsHandler extends Component { 'useForcedSubtitles', 'useNetworkInformationApi', 'useDtsForTimestampOffset', + 'calculateTimestampOffsetForEachSegment', 'exactManifestTimings', 'leastPixelDiffSelector' ].forEach((option) => { From 09ade8e59454fc0fa2d4897af2267313b1ebd8e4 Mon Sep 17 00:00:00 2001 From: Dzianis Dashkevich Date: Thu, 7 Sep 2023 13:43:51 -0400 Subject: [PATCH 2/7] fix eslint issues --- src/source-updater.js | 1 + src/util/vjs-compat.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/source-updater.js b/src/source-updater.js index 78e42606e..49ba113f7 100644 --- a/src/source-updater.js +++ b/src/source-updater.js @@ -301,6 +301,7 @@ const onUpdateend = (type, sourceUpdater) => (e) => { if (sourceBuffer) { const bufferedAsString = prettyBuffered(sourceBuffer.buffered); + sourceUpdater.logger_(`${type} source buffer update end. Buffered: \n`, bufferedAsString); } diff --git a/src/util/vjs-compat.js b/src/util/vjs-compat.js index 44dc35bcf..42f964f46 100644 --- a/src/util/vjs-compat.js +++ b/src/util/vjs-compat.js @@ -30,7 +30,7 @@ export function createTimeRanges(...args) { * @param {TimeRanges} buffered - time ranges * @return {string} - descriptive string */ -export function prettyBuffered (buffered) { +export function prettyBuffered(buffered) { let result = ''; for (let i = 0; i < buffered.length; i++) { From 28d3b705ded0c7f65ba2ba8dcfbd204dd0b32249 Mon Sep 17 00:00:00 2001 From: Dzianis Dashkevich Date: Thu, 7 Sep 2023 14:02:37 -0400 Subject: [PATCH 3/7] safe read buffered --- src/source-updater.js | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/source-updater.js b/src/source-updater.js index 49ba113f7..12c635914 100644 --- a/src/source-updater.js +++ b/src/source-updater.js @@ -297,13 +297,10 @@ const pushQueue = ({type, sourceUpdater, action, doneFn, name}) => { }; const onUpdateend = (type, sourceUpdater) => (e) => { - const sourceBuffer = sourceUpdater[`${type}Buffer`]; - - if (sourceBuffer) { - const bufferedAsString = prettyBuffered(sourceBuffer.buffered); + const buffered = sourceUpdater[`${type}Buffered`](); + const bufferedAsString = prettyBuffered(buffered); - sourceUpdater.logger_(`${type} source buffer update end. Buffered: \n`, bufferedAsString); - } + sourceUpdater.logger_(`${type} source buffer update end. Buffered: \n`, bufferedAsString); // Although there should, in theory, be a pending action for any updateend receieved, // there are some actions that may trigger updateend events without set definitions in From f1fbd7a764040eb580a22fb82d8fb3599c848cc3 Mon Sep 17 00:00:00 2001 From: Dzianis Dashkevich Date: Thu, 7 Sep 2023 16:00:06 -0400 Subject: [PATCH 4/7] fix test --- test/source-updater.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/source-updater.test.js b/test/source-updater.test.js index 4ca5d57f2..b79fc2150 100644 --- a/test/source-updater.test.js +++ b/test/source-updater.test.js @@ -216,14 +216,14 @@ QUnit.test('verifies that sourcebuffer is in source buffers list before attempti assert.deepEqual(actionCalls, { audioAbort: 1, audioAppendBuffer: 1, - audioBuffered: 8, + audioBuffered: 12, audioChangeType: 1, audioRemove: 1, audioRemoveSourceBuffer: 1, audioTimestampOffset: 1, videoAbort: 1, videoAppendBuffer: 1, - videoBuffered: 8, + videoBuffered: 12, videoChangeType: 1, videoRemove: 1, videoRemoveSourceBuffer: 1, From 967c8dfc4c002f14d9be56360362ba46cdcaf857 Mon Sep 17 00:00:00 2001 From: Dzianis Dashkevich Date: Mon, 11 Sep 2023 13:33:23 -0400 Subject: [PATCH 5/7] add tests for calculateTimestampOffsetForEachSegment --- test/segment-loader.test.js | 48 +++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/test/segment-loader.test.js b/test/segment-loader.test.js index 5a75b407d..62d61e02a 100644 --- a/test/segment-loader.test.js +++ b/test/segment-loader.test.js @@ -222,6 +222,54 @@ QUnit.test('illegalMediaSwitch detects illegal media switches', function(assert) QUnit.module('timestampOffsetForSegment'); +QUnit.test('returns startOfSegment when calculateTimestampOffsetForEachSegment is enabled and the buffer is empty with the same timeline', function(assert) { + const timestampOffset = timestampOffsetForSegment({ + calculateTimestampOffsetForEachSegment: true, + segmentTimeline: 0, + currentTimeline: 0, + startOfSegment: 3, + buffered: createTimeRanges() + }); + + assert.equal(timestampOffset, 3, 'returned startOfSegment'); +}); + +QUnit.test('returns startOfSegment when calculateTimestampOffsetForEachSegment is enabled and the buffer is empty with different timeline', function(assert) { + const timestampOffset = timestampOffsetForSegment({ + calculateTimestampOffsetForEachSegment: true, + segmentTimeline: 1, + currentTimeline: 0, + startOfSegment: 3, + buffered: createTimeRanges() + }); + + assert.equal(timestampOffset, 3, 'returned startOfSegment'); +}); + +QUnit.test('returns buffered.end when calculateTimestampOffsetForEachSegment is enabled and there exists buffered content with the same timeline', function(assert) { + const timestampOffset = timestampOffsetForSegment({ + calculateTimestampOffsetForEachSegment: true, + segmentTimeline: 0, + currentTimeline: 0, + startOfSegment: 3, + buffered: createTimeRanges([[1, 5], [7, 8]]) + }); + + assert.equal(timestampOffset, 8, 'returned buffered.end'); +}); + +QUnit.test('returns buffered.end when calculateTimestampOffsetForEachSegment is enabled and there exists buffered content with different timeline', function(assert) { + const timestampOffset = timestampOffsetForSegment({ + calculateTimestampOffsetForEachSegment: true, + segmentTimeline: 1, + currentTimeline: 0, + startOfSegment: 3, + buffered: createTimeRanges([[1, 5], [7, 8]]) + }); + + assert.equal(timestampOffset, 8, 'returned buffered.end'); +}); + QUnit.test('returns startOfSegment when timeline changes and the buffer is empty', function(assert) { assert.equal( timestampOffsetForSegment({ From a60b28135bdd1fe3c21fb4f97988b09e782efc94 Mon Sep 17 00:00:00 2001 From: Dzianis Dashkevich Date: Mon, 11 Sep 2023 13:34:19 -0400 Subject: [PATCH 6/7] extract buffered end getter to a separate function --- src/segment-loader.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/segment-loader.js b/src/segment-loader.js index 3c763c7c5..b67c4984c 100644 --- a/src/segment-loader.js +++ b/src/segment-loader.js @@ -179,6 +179,10 @@ export const segmentInfoString = (segmentInfo) => { const timingInfoPropertyForMedia = (mediaType) => `${mediaType}TimingInfo`; +const getBufferedEndOrFallback = (buffered, fallback) => buffered.length ? + buffered.end(buffered.length - 1) : + fallback; + /** * Returns the timestamp offset to use for the segment. * @@ -209,7 +213,7 @@ export const timestampOffsetForSegment = ({ overrideCheck }) => { if (calculateTimestampOffsetForEachSegment) { - return buffered.length ? buffered.end(buffered.length - 1) : startOfSegment; + return getBufferedEndOrFallback(buffered, startOfSegment); } // Check to see if we are crossing a discontinuity to see if we need to set the @@ -255,7 +259,7 @@ export const timestampOffsetForSegment = ({ // should often be correct, it's better to rely on the buffered end, as the new // content post discontinuity should line up with the buffered end as if it were // time 0 for the new content. - return buffered.length ? buffered.end(buffered.length - 1) : startOfSegment; + return getBufferedEndOrFallback(buffered, startOfSegment); }; /** From 1d60c53c011276b4fe9f700507c2cf6ead2dde9b Mon Sep 17 00:00:00 2001 From: Dzianis Dashkevich Date: Mon, 11 Sep 2023 13:34:30 -0400 Subject: [PATCH 7/7] fix eslint issues --- src/util/vjs-compat.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/vjs-compat.js b/src/util/vjs-compat.js index 42f964f46..e545ffc33 100644 --- a/src/util/vjs-compat.js +++ b/src/util/vjs-compat.js @@ -27,6 +27,7 @@ export function createTimeRanges(...args) { /** * Converts any buffered time range to a descriptive string + * * @param {TimeRanges} buffered - time ranges * @return {string} - descriptive string */