Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle streams with corrupted pts or dts timestamps #1426

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
5 changes: 5 additions & 0 deletions index.html
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,11 @@
<label class="form-check-label" for="dts-offset">Use DTS instead of PTS for Timestamp Offset calculation (reloads player)</label>
</div>

<div class="form-check">
<input id=offset-each-segment type="checkbox" class="form-check-input">
<label class="form-check-label" for="offset-each-segment">Calculate timestampOffset for each segment, regardless of its timeline (reloads player)</label>
</div>

<div class="form-check">
<input id=llhls type="checkbox" class="form-check-input">
<label class="form-check-label" for="llhls">[EXPERIMENTAL] Enables support for ll-hls (reloads player)</label>
Expand Down
3 changes: 3 additions & 0 deletions scripts/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,7 @@
'pixel-diff-selector',
'network-info',
'dts-offset',
'offset-each-segment',
'override-native',
'preload',
'mirror-source',
Expand Down Expand Up @@ -525,6 +526,7 @@
'pixel-diff-selector',
'network-info',
'dts-offset',
'offset-each-segment',
'exact-manifest-timings',
'forced-subtitles'
].forEach(function(name) {
Expand Down Expand Up @@ -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'])
}
}
Expand Down
9 changes: 0 additions & 9 deletions src/media-segment-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions src/playlist-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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_),
Expand Down
9 changes: 9 additions & 0 deletions src/segment-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,8 @@
* 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.
Expand All @@ -203,8 +205,13 @@
currentTimeline,
startOfSegment,
buffered,
calculateTimestampOffsetForEachSegment,
overrideCheck
}) => {
if (calculateTimestampOffsetForEachSegment) {
return buffered.length ? buffered.end(buffered.length - 1) : startOfSegment;

Check warning on line 212 in src/segment-loader.js

View check run for this annotation

Codecov / codecov/patch

src/segment-loader.js#L212

Added line #L212 was not covered by tests
adrums86 marked this conversation as resolved.
Show resolved Hide resolved
}

// 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.
//
Expand Down Expand Up @@ -559,6 +566,7 @@
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;
Expand Down Expand Up @@ -1586,6 +1594,7 @@
currentTimeline: this.currentTimeline_,
startOfSegment,
buffered: this.buffered_(),
calculateTimestampOffsetForEachSegment: this.calculateTimestampOffsetForEachSegment_,
overrideCheck
});

Expand Down
7 changes: 6 additions & 1 deletion src/source-updater.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -297,6 +297,11 @@ const pushQueue = ({type, sourceUpdater, action, doneFn, name}) => {
};

const onUpdateend = (type, sourceUpdater) => (e) => {
const buffered = sourceUpdater[`${type}Buffered`]();
const bufferedAsString = prettyBuffered(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
Expand Down
24 changes: 24 additions & 0 deletions src/util/vjs-compat.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
}
2 changes: 2 additions & 0 deletions src/videojs-http-streaming.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -743,6 +744,7 @@ class VhsHandler extends Component {
'useForcedSubtitles',
'useNetworkInformationApi',
'useDtsForTimestampOffset',
'calculateTimestampOffsetForEachSegment',
'exactManifestTimings',
'leastPixelDiffSelector'
].forEach((option) => {
Expand Down
4 changes: 2 additions & 2 deletions test/source-updater.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading