Skip to content

Commit

Permalink
fix(dash): Fix $Time$ usage with SegmentTemplate (#7849)
Browse files Browse the repository at this point in the history 
`$Time$` in SegmentTemplate should not be adjusted for
presentationTimeOffset or Period start. It should always match the
segment's own media timestamp as it appears in the manifest.
  • Loading branch information
@joeyparrish
joeyparrish authored Jan 8, 2025
1 parent a0848f7 commit abd6d8b
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 28 deletions.
11 changes: 7 additions & 4 deletions lib/dash/mpd_utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,10 +280,9 @@ shaka.dash.MpdUtils = class {

const durationStr =
MpdUtils.inheritAttribute(context, callback, 'duration');
let segmentDuration = TXml.parsePositiveInt(durationStr || '');
if (segmentDuration) {
segmentDuration /= timescale;
}
const unscaledSegmentDuration = TXml.parsePositiveInt(durationStr || '');
const segmentDuration = unscaledSegmentDuration ?
unscaledSegmentDuration / timescale : null;

const startNumberStr =
MpdUtils.inheritAttribute(context, callback, 'startNumber');
Expand All @@ -310,6 +309,7 @@ shaka.dash.MpdUtils = class {
(unscaledPresentationTimeOffset / timescale) || 0;
return {
timescale: timescale,
unscaledSegmentDuration: unscaledSegmentDuration,
segmentDuration: segmentDuration,
startNumber: startNumber,
scaledPresentationTimeOffset: scaledPresentationTimeOffset,
Expand Down Expand Up @@ -565,6 +565,7 @@ shaka.dash.MpdUtils = class {
/**
* @typedef {{
* timescale: number,
* unscaledSegmentDuration: ?number,
* segmentDuration: ?number,
* startNumber: number,
* scaledPresentationTimeOffset: number,
Expand All @@ -577,6 +578,8 @@ shaka.dash.MpdUtils = class {
*
* @property {number} timescale
* The time-scale of the representation.
* @property {?number} unscaledSegmentDuration
* The duration of the segments in timescale units, if given.
* @property {?number} segmentDuration
* The duration of the segments in seconds, if given.
* @property {number} startNumber
Expand Down
47 changes: 30 additions & 17 deletions lib/dash/segment_template.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ shaka.dash.SegmentTemplate = class {
}

return {
unscaledSegmentDuration: segmentInfo.unscaledSegmentDuration,
segmentDuration: segmentInfo.segmentDuration,
timescale: segmentInfo.timescale,
startNumber: segmentInfo.startNumber,
Expand Down Expand Up @@ -325,10 +326,12 @@ shaka.dash.SegmentTemplate = class {
if (info.indexTemplate) {
shaka.log.info('Using the index URL template by default.');
info.timeline = null;
info.unscaledSegmentDuration = null;
info.segmentDuration = null;
} else {
goog.asserts.assert(info.timeline, 'There should be a timeline');
shaka.log.info('Using the SegmentTimeline by default.');
info.unscaledSegmentDuration = null;
info.segmentDuration = null;
}
}
Expand Down Expand Up @@ -421,7 +424,6 @@ shaka.dash.SegmentTemplate = class {
segmentDuration != null, 'Segment duration must not be null!');

const startNumber = info.startNumber;
const timescale = info.timescale;

const template = info.mediaTemplate;
const bandwidth = context.bandwidth || null;
Expand Down Expand Up @@ -505,10 +507,18 @@ shaka.dash.SegmentTemplate = class {
const positionWithinPeriod = position - startNumber;
const segmentPeriodTime = positionWithinPeriod * segmentDuration;

// What will appear in the actual segment files. The media timestamp is
// what is expected in the $Time$ template.
const segmentMediaTime = segmentPeriodTime +
info.scaledPresentationTimeOffset;
const unscaledSegmentDuration = info.unscaledSegmentDuration;
goog.asserts.assert(unscaledSegmentDuration != null,
'Segment duration must not be null!');

// The original media timestamp from the timeline is what is expected in
// the $Time$ template. (Or based on duration, in this case.) It should
// not be adjusted with presentationTimeOffset or the Period start.
let timeReplacement = positionWithinPeriod * unscaledSegmentDuration;
if ('BigInt' in window && timeReplacement > Number.MAX_SAFE_INTEGER) {
timeReplacement =
BigInt(positionWithinPeriod) * BigInt(unscaledSegmentDuration);
}

// Relative to the presentation.
const segmentStart = segmentPeriodTime + periodStart;
Expand All @@ -533,12 +543,8 @@ shaka.dash.SegmentTemplate = class {
const end = start + partialDuration;
const subNumber = i + 1;
const getPartialUris = () => {
let time = segmentMediaTime * timescale;
if ('BigInt' in window && time > Number.MAX_SAFE_INTEGER) {
time = BigInt(segmentMediaTime) * BigInt(timescale);
}
const mediaUri = MpdUtils.fillUriTemplate(
template, id, position, subNumber, bandwidth, time);
template, id, position, subNumber, bandwidth, timeReplacement);
return ManifestParserUtils.resolveUris(
getBaseUris(), [mediaUri], urlParams());
};
Expand Down Expand Up @@ -575,12 +581,9 @@ shaka.dash.SegmentTemplate = class {
if (numChunks) {
return [];
}
let time = segmentMediaTime * timescale;
if ('BigInt' in window && time > Number.MAX_SAFE_INTEGER) {
time = BigInt(segmentMediaTime) * BigInt(timescale);
}
const mediaUri = MpdUtils.fillUriTemplate(
template, id, position, /* subNumber= */ null, bandwidth, time);
template, id, position, /* subNumber= */ null, bandwidth,
timeReplacement);
return ManifestParserUtils.resolveUris(
getBaseUris(), [mediaUri], urlParams());
};
Expand Down Expand Up @@ -1007,8 +1010,15 @@ shaka.dash.TimelineSegmentIndex = class extends shaka.media.SegmentIndex {
if (!ref) {
const range = this.templateInfo_.timeline[correctedPosition];
const segmentReplacement = range.segmentPosition;
const timeReplacement = this.templateInfo_
.unscaledPresentationTimeOffset + range.unscaledStart;

// The original media timestamp from the timeline is what is expected in
// the $Time$ template. It should not be adjusted with
// presentationTimeOffset or the Period start, but
// unscaledPresentationTimeOffset was already subtracted from the times
// in timeline.
const timeReplacement = range.unscaledStart +
this.templateInfo_.unscaledPresentationTimeOffset;

const timestampOffset = this.periodStart_ -
this.templateInfo_.scaledPresentationTimeOffset;
const trueSegmentEnd = this.periodStart_ + range.end;
Expand Down Expand Up @@ -1155,6 +1165,7 @@ shaka.dash.TimelineSegmentIndex = class extends shaka.media.SegmentIndex {
/**
* @typedef {{
* timescale: number,
* unscaledSegmentDuration: ?number,
* segmentDuration: ?number,
* startNumber: number,
* scaledPresentationTimeOffset: number,
Expand All @@ -1173,6 +1184,8 @@ shaka.dash.TimelineSegmentIndex = class extends shaka.media.SegmentIndex {
*
* @property {number} timescale
* The time-scale of the representation.
* @property {?number} unscaledSegmentDuration
* The duration of the segments in seconds, in timescale units.
* @property {?number} segmentDuration
* The duration of the segments in seconds, if given.
* @property {number} startNumber
Expand Down
57 changes: 54 additions & 3 deletions test/dash/dash_parser_segment_template_unit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ describe('DashParser SegmentTemplate', () => {
it('honors presentationTimeOffset', async () => {
const source = Dash.makeSimpleManifestText([
'<SegmentTemplate media="s$Number$.mp4" duration="10"',
' presentationTimeOffset="50" />',
' presentationTimeOffset="10" />',
], /* duration= */ 30, /* startTime= */ 40);

fakeNetEngine.setResponseText('dummy://foo', source);
Expand All @@ -110,18 +110,45 @@ describe('DashParser SegmentTemplate', () => {

const expectedRef1 = ManifestParser.makeReference(
's1.mp4', 40, 50, baseUri);
expectedRef1.timestampOffset = -10;
expectedRef1.timestampOffset = 30; // period start 40 - pto 10

const expectedRef2 = ManifestParser.makeReference(
's2.mp4', 50, 60, baseUri);
expectedRef2.timestampOffset = -10;
expectedRef2.timestampOffset = 30; // period start 40 - pto 10

const ref1 = stream.segmentIndex.getIteratorForTime(45).next().value;
const ref2 = stream.segmentIndex.getIteratorForTime(55).next().value;
expect(ref1).toEqual(expectedRef1);
expect(ref2).toEqual(expectedRef2);
});

it('constructs $Time$ ignoring offset and period', async () => {
const source = Dash.makeSimpleManifestText([
'<SegmentTemplate media="s$Number$-$Time$.mp4" duration="10"',
' presentationTimeOffset="10" />',
], /* duration= */ 30, /* startTime= */ 40);

fakeNetEngine.setResponseText('dummy://foo', source);
const manifest = await parser.start('dummy://foo', playerInterface);

expect(manifest.variants.length).toBe(1);

const stream = manifest.variants[0].video;
expect(stream).toBeTruthy();
await stream.createSegmentIndex();

// The media time of the segment (0) is used in $Time$, without regard
// for the presentationTimeOffset (10), and without regard for the period
// start (40). The reference itself has a start time that includes the
// period.
const expectedRef1 = ManifestParser.makeReference(
's1-0.mp4', 40, 50, baseUri);
expectedRef1.timestampOffset = 30; // period start 40 - pto 10

const ref1 = stream.segmentIndex.getIteratorForTime(45).next().value;
expect(ref1).toEqual(expectedRef1);
});

it('handles segments larger than the period', async () => {
const source = Dash.makeSimpleManifestText([
'<SegmentTemplate media="s$Number$.mp4" duration="60" />',
Expand Down Expand Up @@ -514,6 +541,28 @@ describe('DashParser SegmentTemplate', () => {
expect(firstSegment(variants[2]).getUris()).toEqual(
['http://example.com/segment-test3-0.dash']);
});

it('constructs $Time$ ignoring offset and period', async () => {
const source = Dash.makeSimpleManifestText([
'<SegmentTemplate media="$Number$-$Time$.mp4"',
' presentationTimeOffset="10" startNumber="0">',
' <SegmentTimeline>',
' <S t="0" d="15" r="2" />',
' </SegmentTimeline>',
'</SegmentTemplate>',
], /* duration= */ 35, /* startTime= */ 40);
const references = [
// Reference time is the media time plus period time, but the $Time$
// used in the URL ignores both presentationTimeOffset and period start.
ManifestParser.makeReference('0-0.mp4', 30, 45, baseUri),
ManifestParser.makeReference('1-15.mp4', 45, 60, baseUri),
ManifestParser.makeReference('2-30.mp4', 60, 75, baseUri),
].map((ref) => {
ref.timestampOffset = 30; // period start 40 - pto 10
return ref;
});
await Dash.testSegmentIndex(source, references);
});
});

describe('rejects streams with', () => {
Expand Down Expand Up @@ -737,6 +786,7 @@ describe('DashParser SegmentTemplate', () => {
expect(pos).toBeNull();
});
});

describe('get', () => {
it('creates a segment reference for a given position', async () => {
const info = makeTemplateInfo(makeRanges(0, 2.0, 10));
Expand Down Expand Up @@ -914,6 +964,7 @@ function makeRanges(start, duration, num) {
*/
function makeTemplateInfo(timeline) {
return {
'unscaledSegmentDuration': null,
'segmentDuration': null,
'timescale': 90000,
'startNumber': 1,
Expand Down
11 changes: 7 additions & 4 deletions test/test/util/manifest_parser_util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,16 @@ shaka.test.ManifestParser = class {
for (const expectedRef of references) {
// Don't query negative times. Query 0 instead.
const startTime = Math.max(0, expectedRef.startTime);
const position = stream.segmentIndex.find(startTime);
expect(position).not.toBe(null);
const actualRef =
stream.segmentIndex.get(/** @type {number} */ (position));
// Some segment start times are before the beginning of the period.
// So default to 0 here if position is null.
const position = stream.segmentIndex.find(startTime) || 0;
const actualRef = stream.segmentIndex.get(position);
// NOTE: A custom matcher for SegmentReferences is installed, so this
// checks the URIs as well.
expect(actualRef).toEqual(expectedRef);
// However, if it does fail because of URIs, you won't see them in the
// output without this, as well:
expect(actualRef.getUris()).toEqual(expectedRef.getUris());
}

// Make sure that the references stop at the end.
Expand Down

0 comments on commit abd6d8b

Please sign in to comment.