From 88857c59aacb8ac0dbc6f9b6eaea683f08f7a17b Mon Sep 17 00:00:00 2001 From: Joey Parrish Date: Fri, 10 Nov 2017 16:23:26 -0800 Subject: [PATCH] Fix HLS playback where moof box is larger than 1k When we started fetching partial HLS segments to find the start time, we were no longer able to start playback for content with a moof box larger than the partial segment. This is because we parsed the boxes hierarchically, which required the entire payload. Now, we can selectively instruct the parser to tolerate a partial box if the part we have includes the child box we are looking for. This fixes playback of several pieces of HLS content in our demo app. Change-Id: I956c8b8905dc9f1707f2b24b8248b984b1c036c6 --- lib/hls/hls_parser.js | 5 +- lib/util/mp4_parser.js | 77 +++++++++++++++++++------ test/hls/hls_live_unit.js | 2 +- test/hls/hls_parser_unit.js | 2 +- test/util/mp4_parser_unit.js | 105 ++++++++++++++++++++++++++++++++--- 5 files changed, 162 insertions(+), 29 deletions(-) diff --git a/lib/hls/hls_parser.js b/lib/hls/hls_parser.js index f65d21b774..256a0ffa3d 100644 --- a/lib/hls/hls_parser.js +++ b/lib/hls/hls_parser.js @@ -1234,7 +1234,8 @@ shaka.hls.HlsParser.prototype.fetchPartialSegment_ = function(segmentRef) { // Try to avoid fetching the entire segment, which can be quite large. var partialSegmentHeaders = {}; var startByte = segmentRef.startByte; - var partialEndByte = startByte + shaka.hls.HlsParser.PARTIAL_SEGMENT_SIZE_; + var partialEndByte = + startByte + shaka.hls.HlsParser.PARTIAL_SEGMENT_SIZE_ - 1; partialSegmentHeaders['Range'] = 'bytes=' + startByte + '-' + partialEndByte; // Prepare a fallback to the entire segment. @@ -1344,7 +1345,7 @@ shaka.hls.HlsParser.prototype.getStartTimeFromMp4Segment_ = function(data) { startTime = baseTime / shaka.hls.HlsParser.TS_TIMESCALE_; parsed = true; box.parser.stop(); - }).parse(data); + }).parse(data, true /* partialOkay */); if (!parsed) { throw new shaka.util.Error( diff --git a/lib/util/mp4_parser.js b/lib/util/mp4_parser.js index 9342c8ae7f..3646f7615c 100644 --- a/lib/util/mp4_parser.js +++ b/lib/util/mp4_parser.js @@ -18,6 +18,7 @@ goog.provide('shaka.util.Mp4Parser'); goog.require('goog.asserts'); +goog.require('shaka.log'); goog.require('shaka.util.DataViewReader'); @@ -43,6 +44,7 @@ shaka.util.Mp4Parser = function() { /** * @typedef {{ * parser: !shaka.util.Mp4Parser, + * partialOkay: boolean, * start: number, * size: number, * version: ?number, @@ -51,21 +53,26 @@ shaka.util.Mp4Parser = function() { * }} * * @property {!shaka.util.Mp4Parser} parser - * The parser that parsed this box. The parser can be used to parse child - * boxes where the configuration of the current parser is needed to parsed - * other boxes. + * The parser that parsed this box. The parser can be used to parse child + * boxes where the configuration of the current parser is needed to parsed + * other boxes. + * @property {boolean} partialOkay + * If true, allow partial payload for some boxes. If the goal is a child box, + * we can sometimes find it without enough data to find all child boxes. + * This property allows the opt_partialOkay flag from parse() to be propagated + * through methods like children(). * @property {number} start - * The start of this box (before the header) in the original buffer. This - * start position is the absolute position. + * The start of this box (before the header) in the original buffer. This + * start position is the absolute position. * @property {number} size - * The size of this box (including the header). + * The size of this box (including the header). * @property {?number} version - * The version for a full box, null for basic boxes. + * The version for a full box, null for basic boxes. * @property {?number} flags - * The flags for a full box, null for basic boxes. + * The flags for a full box, null for basic boxes. * @property {!shaka.util.DataViewReader} reader - * The reader for this box is only for this box. Reading or not reading to - * the end will have no affect on the parser reading other sibling boxes. + * The reader for this box is only for this box. Reading or not reading to + * the end will have no affect on the parser reading other sibling boxes. * @exportInterface */ shaka.util.Mp4Parser.ParsedBox; @@ -138,9 +145,12 @@ shaka.util.Mp4Parser.prototype.stop = function() { * Parse the given data using the added callbacks. * * @param {!BufferSource} data + * @param {boolean=} opt_partialOkay If true, allow partial payload for some + * boxes. If the goal is a child box, we can sometimes find it without enough + * data to find all child boxes. * @export */ -shaka.util.Mp4Parser.prototype.parse = function(data) { +shaka.util.Mp4Parser.prototype.parse = function(data, opt_partialOkay) { var wrapped = new Uint8Array(data); var reader = new shaka.util.DataViewReader( new DataView(wrapped.buffer, wrapped.byteOffset, wrapped.byteLength), @@ -148,7 +158,7 @@ shaka.util.Mp4Parser.prototype.parse = function(data) { this.done_ = false; while (reader.hasMoreData() && !this.done_) { - this.parseNext(0, reader); + this.parseNext(0, reader, opt_partialOkay); } }; @@ -157,15 +167,21 @@ shaka.util.Mp4Parser.prototype.parse = function(data) { * Parse the next box on the current level. * * @param {number} absStart The absolute start position in the original - * byte array + * byte array. * @param {!shaka.util.DataViewReader} reader + * @param {boolean=} opt_partialOkay If true, allow partial payload for some + * boxes. If the goal is a child box, we can sometimes find it without enough + * data to find all child boxes. * @export */ -shaka.util.Mp4Parser.prototype.parseNext = function(absStart, reader) { +shaka.util.Mp4Parser.prototype.parseNext = + function(absStart, reader, opt_partialOkay) { var start = reader.getPosition(); var size = reader.readUint32(); var type = reader.readUint32(); + var name = shaka.util.Mp4Parser.typeToString_(type); + shaka.log.v2('Parsing MP4 box', name); switch (size) { case 0: @@ -190,7 +206,12 @@ shaka.util.Mp4Parser.prototype.parseNext = function(absStart, reader) { // Read the whole payload so that the current level can be safely read // regardless of how the payload is parsed. - var payloadSize = start + size - reader.getPosition(); + var end = start + size; + if (opt_partialOkay && end > reader.getLength()) { + // For partial reads, truncate the payload if we must. + end = reader.getLength(); + } + var payloadSize = end - reader.getPosition(); var payload = (payloadSize > 0) ? reader.readBytes(payloadSize) : new Uint8Array(0); @@ -201,6 +222,7 @@ shaka.util.Mp4Parser.prototype.parseNext = function(absStart, reader) { /** @type {shaka.util.Mp4Parser.ParsedBox } */ var box = { parser: this, + partialOkay: opt_partialOkay || false, version: version, flags: flags, reader: payloadReader, @@ -225,7 +247,7 @@ shaka.util.Mp4Parser.prototype.parseNext = function(absStart, reader) { */ shaka.util.Mp4Parser.children = function(box) { while (box.reader.hasMoreData() && !box.parser.done_) { - box.parser.parseNext(box.start, box.reader); + box.parser.parseNext(box.start, box.reader, box.partialOkay); } }; @@ -240,8 +262,10 @@ shaka.util.Mp4Parser.children = function(box) { * @export */ shaka.util.Mp4Parser.sampleDescription = function(box) { - for (var count = box.reader.readUint32(); count > 0; count -= 1) { - box.parser.parseNext(box.start, box.reader); + for (var count = box.reader.readUint32(); + count > 0 && !box.parser.done_; + count -= 1) { + box.parser.parseNext(box.start, box.reader, box.partialOkay); } }; @@ -282,3 +306,20 @@ shaka.util.Mp4Parser.typeFromString_ = function(name) { return code; }; + +/** + * Convert an integer type from a box into an ascii string name. + * Useful for debugging. + * + * @param {number} type The type of the box, a uint32. + * @return {string} + * @private + */ +shaka.util.Mp4Parser.typeToString_ = function(type) { + var name = String.fromCharCode( + (type >> 24) & 0xff, + (type >> 16) & 0xff, + (type >> 8) & 0xff, + type & 0xff); + return name; +}; diff --git a/test/hls/hls_live_unit.js b/test/hls/hls_live_unit.js index 1142062d57..ec1d803c21 100644 --- a/test/hls/hls_live_unit.js +++ b/test/hls/hls_live_unit.js @@ -516,7 +516,7 @@ describe('HlsParser live', function() { it('gets start time of segments with byte range', function(done) { // Nit: this value is an implementation detail of the fix for #1106 - var partialEndByte = expectedStartByte + 1024; + var partialEndByte = expectedStartByte + 1024 - 1; fakeNetEngine.setResponseMap({ 'test:/master': toUTF8(master), diff --git a/test/hls/hls_parser_unit.js b/test/hls/hls_parser_unit.js index 4dda16c174..1306fe52a6 100644 --- a/test/hls/hls_parser_unit.js +++ b/test/hls/hls_parser_unit.js @@ -1306,7 +1306,7 @@ describe('HlsParser', function() { var expectedStartByte = 616; var expectedEndByte = 121705; // Nit: this value is an implementation detail of the fix for #1106 - var partialEndByte = expectedStartByte + 1024; + var partialEndByte = expectedStartByte + 1024 - 1; beforeEach(function() { // TODO: use StreamGenerator? diff --git a/test/util/mp4_parser_unit.js b/test/util/mp4_parser_unit.js index 76667cfa6a..902a81f195 100644 --- a/test/util/mp4_parser_unit.js +++ b/test/util/mp4_parser_unit.js @@ -43,9 +43,12 @@ describe('Mp4Parser', function() { boxWithChildData = new Uint8Array([ 0x00, 0x00, 0x00, 0x14, // size 0x62, 0x30, 0x30, 0x33, // type - 0x00, 0x00, 0x00, 0x0C, // child size - 0x62, 0x30, 0x33, 0x31, // child type - 0x00, 0x11, 0x22, 0x33 // child payload + 0x00, 0x00, 0x00, 0x0C, // child [0] size + 0x62, 0x30, 0x33, 0x31, // child [0] type + 0x00, 0x11, 0x22, 0x33, // child [0] payload + 0x00, 0x00, 0x00, 0x0C, // child [1] size + 0x62, 0x30, 0x33, 0x32, // child [1] type + 0x44, 0x55, 0x66, 0x77 // child [1] payload ]).buffer; boxWithSampleDescription = new Uint8Array([ @@ -134,7 +137,14 @@ describe('Mp4Parser', function() { var parentBox = jasmine.createSpy('parent box').and.callFake( shaka.util.Mp4Parser.children); - var childBox = jasmine.createSpy('child box').and.callFake( + var childBox1 = jasmine.createSpy('child box 1').and.callFake( + function(box) { + expect(box.size).toEqual(12); + expect(box.version).toEqual(null); + expect(box.flags).toEqual(null); + }); + + var childBox2 = jasmine.createSpy('child box 2').and.callFake( function(box) { expect(box.size).toEqual(12); expect(box.version).toEqual(null); @@ -143,10 +153,33 @@ describe('Mp4Parser', function() { new shaka.util.Mp4Parser() .box('b003', Util.spyFunc(parentBox)) - .box('b031', Util.spyFunc(childBox)).parse(boxWithChildData); + .box('b031', Util.spyFunc(childBox1)) + .box('b032', Util.spyFunc(childBox2)).parse(boxWithChildData); expect(parentBox).toHaveBeenCalled(); - expect(childBox).toHaveBeenCalled(); + expect(childBox1).toHaveBeenCalled(); + expect(childBox2).toHaveBeenCalled(); + }); + + it('stops reading children when asked to', function() { + var parentBox = jasmine.createSpy('parent box').and.callFake( + shaka.util.Mp4Parser.children); + + var childBox1 = jasmine.createSpy('child box 1').and.callFake( + function(box) { + box.parser.stop(); + }); + + var childBox2 = jasmine.createSpy('child box 2'); + + new shaka.util.Mp4Parser() + .box('b003', Util.spyFunc(parentBox)) + .box('b031', Util.spyFunc(childBox1)) + .box('b032', Util.spyFunc(childBox2)).parse(boxWithChildData); + + expect(parentBox).toHaveBeenCalled(); + expect(childBox1).toHaveBeenCalled(); + expect(childBox2).not.toHaveBeenCalled(); }); it('reads all data definition', function() { @@ -166,7 +199,6 @@ describe('Mp4Parser', function() { }); it('reads sample description definition', function() { - var parentBox = jasmine.createSpy('parent box').and.callFake( shaka.util.Mp4Parser.sampleDescription); var childBox1 = jasmine.createSpy('child box 1'); @@ -181,6 +213,25 @@ describe('Mp4Parser', function() { expect(childBox1).toHaveBeenCalledTimes(1); expect(childBox2).toHaveBeenCalledTimes(1); }); + + it('stops reading sample description when asked to', function() { + var parentBox = jasmine.createSpy('parent box').and.callFake( + shaka.util.Mp4Parser.sampleDescription); + var childBox1 = jasmine.createSpy('child box 1').and.callFake( + function(box) { + box.parser.stop(); + }); + var childBox2 = jasmine.createSpy('child box 2'); + + new shaka.util.Mp4Parser() + .box('b003', Util.spyFunc(parentBox)) + .box('b032', Util.spyFunc(childBox1)) + .box('b033', Util.spyFunc(childBox2)).parse(boxWithSampleDescription); + + expect(parentBox).toHaveBeenCalledTimes(1); + expect(childBox1).toHaveBeenCalledTimes(1); + expect(childBox2).not.toHaveBeenCalled(); + }); }); describe('parsing', function() { @@ -230,5 +281,45 @@ describe('Mp4Parser', function() { expect(box2Child).not.toHaveBeenCalled(); expect(box3).toHaveBeenCalled(); }); + + it('can parse partial parent box and find first child', function() { + var parentBox = jasmine.createSpy('parent box').and.callFake( + shaka.util.Mp4Parser.sampleDescription); + + var childBox1 = jasmine.createSpy('child box 1').and.callFake( + function(box) { + // We found what we were looking for, so stop parsing. + box.parser.stop(); + }); + + // This only contains the parent and the first child (12 bytes each). + var partialBoxWithSampleDescription = + (new Uint8Array(boxWithSampleDescription)).slice(0, 24).buffer; + + try { + new shaka.util.Mp4Parser() + .box('b003', Util.spyFunc(parentBox)) + .box('b032', Util.spyFunc(childBox1)) + .parse(partialBoxWithSampleDescription, false /* partialOkay */); + fail('Should not have been able to parse!'); + } catch (error) { + Util.expectToEqualError(error, new shaka.util.Error( + shaka.util.Error.Severity.CRITICAL, + shaka.util.Error.Category.MEDIA, + shaka.util.Error.Code.BUFFER_READ_OUT_OF_BOUNDS)); + } + + parentBox.calls.reset(); + childBox1.calls.reset(); + + // With the partialOkay flag set to true, this should succeed. + new shaka.util.Mp4Parser() + .box('b003', Util.spyFunc(parentBox)) + .box('b032', Util.spyFunc(childBox1)) + .parse(partialBoxWithSampleDescription, true /* partialOkay */); + + expect(parentBox).toHaveBeenCalledTimes(1); + expect(childBox1).toHaveBeenCalledTimes(1); + }); }); });