Skip to content

Commit

Permalink
feat: Add feature flag to calculate timestampOffset for each segment …
Browse files Browse the repository at this point in the history
…to handle streams with corrupted pts or dts timestamps (#1426)

* Use audio/video start time info from Transmuxer instead of probeTs because Transmuxer aligns time between audio and video in the timestampRollover stream.

* Add a feature flag to calculate timestampOffset for each segment, regardless of its timeline, so timestampOffset should always be valid.

---------

Co-authored-by: Dzianis Dashkevich <ddashkevich@brightcove.com>
  • Loading branch information
dzianis-dashkevich and Dzianis Dashkevich authored Sep 25, 2023
1 parent 04451d4 commit 2355ddc
Show file tree
Hide file tree
Showing 11 changed files with 112 additions and 13 deletions.
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
15 changes: 14 additions & 1 deletion src/segment-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -190,6 +194,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.
Expand All @@ -203,8 +209,13 @@ export const timestampOffsetForSegment = ({
currentTimeline,
startOfSegment,
buffered,
calculateTimestampOffsetForEachSegment,
overrideCheck
}) => {
if (calculateTimestampOffsetForEachSegment) {
return getBufferedEndOrFallback(buffered, 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.
//
Expand Down Expand Up @@ -248,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);
};

/**
Expand Down Expand Up @@ -559,6 +570,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;
Expand Down Expand Up @@ -1586,6 +1598,7 @@ export default class SegmentLoader extends videojs.EventTarget {
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
25 changes: 25 additions & 0 deletions src/util/vjs-compat.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,28 @@ 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
48 changes: 48 additions & 0 deletions test/segment-loader.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
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

0 comments on commit 2355ddc

Please sign in to comment.