Skip to content

Commit

Permalink
Fix PTO for SegmentBase
Browse files Browse the repository at this point in the history
In DASH SegmentBase, we were not dividing presentationTimeOffset by
the timescale.  In all other instances of presentationTimeOffset in
DASH, we were handling it correctly.

This bug was present in all v2 releases until now.

The solution is not only to divide by timescale, but to rename all
internal uses of presentationTimeOffset to either unscaled or scaled,
to differentiate between those in timescale units and those in
seconds.  I believe inconsistent naming and units were a contributing
factor to the creation of the bug.

Closes #1099

Change-Id: Id561f8eb1f5bc011c606e1925c12f0d8183fd51a
  • Loading branch information
joeyparrish committed Nov 1, 2017
1 parent f30b07a commit 5166699
Show file tree
Hide file tree
Showing 9 changed files with 75 additions and 59 deletions.
2 changes: 1 addition & 1 deletion externs/shaka/offline.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ shakaExtern.PeriodDB;
* @property {boolean} primary
* Whether the stream set was primary.
* @property {number} presentationTimeOffset
* The presentation time offset of the stream.
* The presentation time offset of the stream, in seconds.
* @property {string} contentType
* The type of the stream, 'audio', 'text', or 'video'.
* @property {string} mimeType
Expand Down
10 changes: 5 additions & 5 deletions lib/dash/dash_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ shaka.dash.DashParser.SegmentIndexFunctions;
* findSegmentPosition: shakaExtern.FindSegmentPositionFunction,
* getSegmentReference: shakaExtern.GetSegmentReferenceFunction,
* initSegmentReference: shaka.media.InitSegmentReference,
* presentationTimeOffset: (number|undefined)
* scaledPresentationTimeOffset: number
* }}
*
* @description
Expand All @@ -300,8 +300,8 @@ shaka.dash.DashParser.SegmentIndexFunctions;
* The getSegmentReference function for the stream.
* @property {shaka.media.InitSegmentReference} initSegmentReference
* The init segment for the stream.
* @property {(number|undefined)} presentationTimeOffset
* The presentationTimeOffset for the stream.
* @property {number} scaledPresentationTimeOffset
* The presentation time offset for the stream, in seconds.
*/
shaka.dash.DashParser.StreamInfo;

Expand Down Expand Up @@ -1106,7 +1106,7 @@ shaka.dash.DashParser.prototype.parseRepresentation_ = function(
1, 0, duration, function() { return baseUris; }, 0, null);
},
initSegmentReference: null,
presentationTimeOffset: 0
scaledPresentationTimeOffset: 0
};
}

Expand All @@ -1121,7 +1121,7 @@ shaka.dash.DashParser.prototype.parseRepresentation_ = function(
findSegmentPosition: streamInfo.findSegmentPosition,
getSegmentReference: streamInfo.getSegmentReference,
initSegmentReference: streamInfo.initSegmentReference,
presentationTimeOffset: streamInfo.presentationTimeOffset,
presentationTimeOffset: streamInfo.scaledPresentationTimeOffset,
mimeType: context.representation.mimeType,
codecs: context.representation.codecs,
frameRate: context.representation.frameRate,
Expand Down
31 changes: 17 additions & 14 deletions lib/dash/mpd_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ shaka.dash.MpdUtils.TimeRange;
* timescale: number,
* segmentDuration: ?number,
* startNumber: number,
* presentationTimeOffset: number,
* scaledPresentationTimeOffset: number,
* unscaledPresentationTimeOffset: number,
* timeline: Array.<shaka.dash.MpdUtils.TimeRange>
* }}
Expand All @@ -71,10 +71,10 @@ shaka.dash.MpdUtils.TimeRange;
* The duration of the segments in seconds, if given.
* @property {number} startNumber
* The start number of the segments; 1 or greater.
* @property {number} presentationTimeOffset
* The presentationTimeOffset of the representation, in seconds.
* @property {number} scaledPresentationTimeOffset
* The presentation time offset of the representation, in seconds.
* @property {number} unscaledPresentationTimeOffset
* The presentationTimeOffset of the representation, in timescale units.
* The presentation time offset of the representation, in timescale units.
* @property {Array.<shaka.dash.MpdUtils.TimeRange>} timeline
* The timeline of the representation, if given. Times in seconds.
*/
Expand Down Expand Up @@ -155,13 +155,14 @@ shaka.dash.MpdUtils.fillUriTemplate = function(
*
* @param {!Element} segmentTimeline
* @param {number} timescale
* @param {number} presentationTimeOffset
* @param {number} unscaledPresentationTimeOffset
* @param {number} periodDuration The Period's duration in seconds.
* Infinity indicates that the Period continues indefinitely.
* @return {!Array.<shaka.dash.MpdUtils.TimeRange>}
*/
shaka.dash.MpdUtils.createTimeline = function(
segmentTimeline, timescale, presentationTimeOffset, periodDuration) {
shaka.dash.MpdUtils.createTimeline =
function(segmentTimeline, timescale, unscaledPresentationTimeOffset,
periodDuration) {
goog.asserts.assert(
timescale > 0 && timescale < Infinity,
'timescale must be a positive, finite integer');
Expand All @@ -185,7 +186,7 @@ shaka.dash.MpdUtils.createTimeline = function(

// Adjust start considering the presentation time offset
if (t != null)
t -= presentationTimeOffset;
t -= unscaledPresentationTimeOffset;

if (!d) {
shaka.log.warning(
Expand Down Expand Up @@ -311,8 +312,9 @@ shaka.dash.MpdUtils.parseSegmentInfo = function(context, callback) {

var startNumberStr =
MpdUtils.inheritAttribute(context, callback, 'startNumber');
var presentationTimeOffset =
MpdUtils.inheritAttribute(context, callback, 'presentationTimeOffset');
var unscaledPresentationTimeOffset =
Number(MpdUtils.inheritAttribute(context, callback,
'presentationTimeOffset')) || 0;
var startNumber = XmlUtils.parseNonNegativeInt(startNumberStr || '');
if (startNumberStr == null || startNumber == null)
startNumber = 1;
Expand All @@ -323,17 +325,18 @@ shaka.dash.MpdUtils.parseSegmentInfo = function(context, callback) {
var timeline = null;
if (timelineNode) {
timeline = MpdUtils.createTimeline(
timelineNode, timescale, Number(presentationTimeOffset),
timelineNode, timescale, unscaledPresentationTimeOffset,
context.periodInfo.duration || Infinity);
}

var pto = (Number(presentationTimeOffset) / timescale) || 0;
var scaledPresentationTimeOffset =
(unscaledPresentationTimeOffset / timescale) || 0;
return {
timescale: timescale,
segmentDuration: segmentDuration,
startNumber: startNumber,
presentationTimeOffset: pto,
unscaledPresentationTimeOffset: Number(presentationTimeOffset),
scaledPresentationTimeOffset: scaledPresentationTimeOffset,
unscaledPresentationTimeOffset: unscaledPresentationTimeOffset,
timeline: timeline
};
};
Expand Down
33 changes: 22 additions & 11 deletions lib/dash/segment_base.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,21 +87,32 @@ shaka.dash.SegmentBase.createStream = function(context, requestInitSegment) {
// the initial parse.
var MpdUtils = shaka.dash.MpdUtils;
var SegmentBase = shaka.dash.SegmentBase;
var XmlUtils = shaka.util.XmlUtils;

var unscaledPresentationTimeOffset = Number(MpdUtils.inheritAttribute(
context, SegmentBase.fromInheritance_, 'presentationTimeOffset')) || 0;

var timescaleStr = MpdUtils.inheritAttribute(
context, SegmentBase.fromInheritance_, 'timescale');
var timescale = 1;
if (timescaleStr) {
timescale = XmlUtils.parsePositiveInt(timescaleStr) || 1;
}

var presentationTimeOffset = MpdUtils.inheritAttribute(
context, SegmentBase.fromInheritance_, 'presentationTimeOffset');
var scaledPresentationTimeOffset =
(unscaledPresentationTimeOffset / timescale) || 0;

var init =
SegmentBase.createInitSegment(context, SegmentBase.fromInheritance_);
var index = SegmentBase.createSegmentIndex_(
context, requestInitSegment, init, Number(presentationTimeOffset));
context, requestInitSegment, init, unscaledPresentationTimeOffset);

return {
createSegmentIndex: index.createSegmentIndex,
findSegmentPosition: index.findSegmentPosition,
getSegmentReference: index.getSegmentReference,
initSegmentReference: init,
presentationTimeOffset: Number(presentationTimeOffset) || 0
scaledPresentationTimeOffset: scaledPresentationTimeOffset
};
};

Expand All @@ -116,12 +127,12 @@ shaka.dash.SegmentBase.createStream = function(context, requestInitSegment) {
* @param {number} startByte
* @param {?number} endByte
* @param {string} containerType
* @param {number} presentationTimeOffset
* @param {number} unscaledPresentationTimeOffset
* @return {shaka.dash.DashParser.SegmentIndexFunctions}
*/
shaka.dash.SegmentBase.createSegmentIndexFromUris = function(
context, requestInitSegment, init, uris,
startByte, endByte, containerType, presentationTimeOffset) {
startByte, endByte, containerType, unscaledPresentationTimeOffset) {
var presentationTimeline = context.presentationTimeline;
var fitLast = !context.dynamic || !context.periodInfo.isLastPeriod;
var periodStartTime = context.periodInfo.start;
Expand All @@ -146,12 +157,12 @@ shaka.dash.SegmentBase.createSegmentIndexFromUris = function(

if (containerType == 'mp4') {
references = shaka.media.Mp4SegmentIndexParser(
indexData, startByte, uris, presentationTimeOffset);
indexData, startByte, uris, unscaledPresentationTimeOffset);
} else {
goog.asserts.assert(initData, 'WebM requires init data');
var parser = new shaka.media.WebmSegmentIndexParser();
references = parser.parse(indexData, initData, uris,
presentationTimeOffset);
unscaledPresentationTimeOffset);
}

presentationTimeline.notifySegments(periodStartTime, references);
Expand Down Expand Up @@ -199,13 +210,13 @@ shaka.dash.SegmentBase.fromInheritance_ = function(frame) {
* @param {shaka.dash.DashParser.Context} context
* @param {shaka.dash.DashParser.RequestInitSegmentCallback} requestInitSegment
* @param {shaka.media.InitSegmentReference} init
* @param {number} presentationTimeOffset
* @param {number} unscaledPresentationTimeOffset
* @return {shaka.dash.DashParser.SegmentIndexFunctions}
* @throws shaka.util.Error When there is a parsing error.
* @private
*/
shaka.dash.SegmentBase.createSegmentIndex_ = function(
context, requestInitSegment, init, presentationTimeOffset) {
context, requestInitSegment, init, unscaledPresentationTimeOffset) {
var MpdUtils = shaka.dash.MpdUtils;
var SegmentBase = shaka.dash.SegmentBase;
var XmlUtils = shaka.util.XmlUtils;
Expand Down Expand Up @@ -269,5 +280,5 @@ shaka.dash.SegmentBase.createSegmentIndex_ = function(

return shaka.dash.SegmentBase.createSegmentIndexFromUris(
context, requestInitSegment, init, indexUris, indexRange.start,
indexRange.end, containerType, presentationTimeOffset);
indexRange.end, containerType, unscaledPresentationTimeOffset);
};
10 changes: 5 additions & 5 deletions lib/dash/segment_list.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ shaka.dash.SegmentList.createStream = function(context, segmentIndexMap) {
findSegmentPosition: segmentIndex.find.bind(segmentIndex),
getSegmentReference: segmentIndex.get.bind(segmentIndex),
initSegmentReference: init,
presentationTimeOffset: info.presentationTimeOffset
scaledPresentationTimeOffset: info.scaledPresentationTimeOffset
};
};

Expand All @@ -113,7 +113,7 @@ shaka.dash.SegmentList.MediaSegment;
* segmentDuration: ?number,
* startTime: number,
* startNumber: number,
* presentationTimeOffset: number,
* scaledPresentationTimeOffset: number,
* timeline: Array.<shaka.dash.MpdUtils.TimeRange>,
* mediaSegments: !Array.<shaka.dash.SegmentList.MediaSegment>
* }}
Expand All @@ -128,8 +128,8 @@ shaka.dash.SegmentList.MediaSegment;
* The start time of the first segment, in seconds.
* @property {number} startNumber
* The start number of the segments; 1 or greater.
* @property {number} presentationTimeOffset
* The presentationTimeOffset of the representation, in seconds.
* @property {number} scaledPresentationTimeOffset
* The scaledPresentationTimeOffset of the representation, in seconds.
* @property {Array.<shaka.dash.MpdUtils.TimeRange>} timeline
* The timeline of the representation, if given. Times in seconds.
* @property {!Array.<shaka.dash.SegmentList.MediaSegment>} mediaSegments
Expand Down Expand Up @@ -183,7 +183,7 @@ shaka.dash.SegmentList.parseSegmentListInfo_ = function(context) {
segmentDuration: segmentInfo.segmentDuration,
startTime: startTime,
startNumber: startNumber,
presentationTimeOffset: segmentInfo.presentationTimeOffset,
scaledPresentationTimeOffset: segmentInfo.scaledPresentationTimeOffset,
timeline: segmentInfo.timeline,
mediaSegments: mediaSegments
};
Expand Down
14 changes: 7 additions & 7 deletions lib/dash/segment_template.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ shaka.dash.SegmentTemplate.createStream = function(
findSegmentPosition: segmentIndexFunctions.findSegmentPosition,
getSegmentReference: segmentIndexFunctions.getSegmentReference,
initSegmentReference: init,
presentationTimeOffset: info.presentationTimeOffset
scaledPresentationTimeOffset: info.scaledPresentationTimeOffset
};
};

Expand All @@ -115,7 +115,7 @@ shaka.dash.SegmentTemplate.createStream = function(
* timescale: number,
* segmentDuration: ?number,
* startNumber: number,
* presentationTimeOffset: number,
* scaledPresentationTimeOffset: number,
* unscaledPresentationTimeOffset: number,
* timeline: Array.<shaka.dash.MpdUtils.TimeRange>,
* mediaTemplate: ?string,
Expand All @@ -132,10 +132,10 @@ shaka.dash.SegmentTemplate.createStream = function(
* The duration of the segments in seconds, if given.
* @property {number} startNumber
* The start number of the segments; 1 or greater.
* @property {number} presentationTimeOffset
* The presentationTimeOffset of the representation, in seconds.
* @property {number} scaledPresentationTimeOffset
* The presentation time offset of the representation, in seconds.
* @property {number} unscaledPresentationTimeOffset
* The presentationTimeOffset of the representation, in timescale units.
* The presentation time offset of the representation, in timescale units.
* @property {Array.<shaka.dash.MpdUtils.TimeRange>} timeline
* The timeline of the representation, if given. Times in seconds.
* @property {?string} mediaTemplate
Expand Down Expand Up @@ -178,7 +178,7 @@ shaka.dash.SegmentTemplate.parseSegmentTemplateInfo_ = function(context) {
segmentDuration: segmentInfo.segmentDuration,
timescale: segmentInfo.timescale,
startNumber: segmentInfo.startNumber,
presentationTimeOffset: segmentInfo.presentationTimeOffset,
scaledPresentationTimeOffset: segmentInfo.scaledPresentationTimeOffset,
unscaledPresentationTimeOffset: segmentInfo.unscaledPresentationTimeOffset,
timeline: segmentInfo.timeline,
mediaTemplate: media,
Expand Down Expand Up @@ -291,7 +291,7 @@ shaka.dash.SegmentTemplate.createFromIndexTemplate_ = function(

return shaka.dash.SegmentBase.createSegmentIndexFromUris(
context, requestInitSegment, init, resolvedUris, 0, null, containerType,
info.presentationTimeOffset);
info.scaledPresentationTimeOffset);
};


Expand Down
15 changes: 8 additions & 7 deletions lib/media/mp4_segment_index_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@ goog.require('shaka.util.Mp4Parser');
* the MP4 container.
* @param {!Array.<string>} uris The possible locations of the MP4 file that
* contains the segments.
* @param {number} presentationTimeOffset
* @param {number} unscaledPresentationTimeOffset
* @return {!Array.<!shaka.media.SegmentReference>}
* @throws {shaka.util.Error}
*/
shaka.media.Mp4SegmentIndexParser = function(
sidxData, sidxOffset, uris, presentationTimeOffset) {
sidxData, sidxOffset, uris, unscaledPresentationTimeOffset) {

var Mp4SegmentIndexParser = shaka.media.Mp4SegmentIndexParser;

Expand All @@ -46,7 +46,7 @@ shaka.media.Mp4SegmentIndexParser = function(
.fullBox('sidx', function(box) {
references = Mp4SegmentIndexParser.parseSIDX_(
sidxOffset,
presentationTimeOffset,
unscaledPresentationTimeOffset,
uris,
box);
});
Expand All @@ -71,7 +71,7 @@ shaka.media.Mp4SegmentIndexParser = function(
* Parse a SIDX box from the given reader.
*
* @param {number} sidxOffset
* @param {number} presentationTimeOffset
* @param {number} unscaledPresentationTimeOffset
* @param {!Array.<string>} uris The possible locations of the MP4 file that
* contains the segments.
* @param {!shaka.util.Mp4Parser.ParsedBox} box
Expand All @@ -80,7 +80,7 @@ shaka.media.Mp4SegmentIndexParser = function(
*/
shaka.media.Mp4SegmentIndexParser.parseSIDX_ = function(
sidxOffset,
presentationTimeOffset,
unscaledPresentationTimeOffset,
uris,
box) {

Expand Down Expand Up @@ -121,8 +121,9 @@ shaka.media.Mp4SegmentIndexParser.parseSIDX_ = function(
// Add references.
var referenceCount = box.reader.readUint16();

// Substract the presentationTimeOffset
var unscaledStartTime = earliestPresentationTime - presentationTimeOffset;
// Substract the presentation time offset
var unscaledStartTime =
earliestPresentationTime - unscaledPresentationTimeOffset;
var startByte = sidxOffset + box.size + firstOffset;

for (var i = 0; i < referenceCount; i++) {
Expand Down
Loading

0 comments on commit 5166699

Please sign in to comment.