fixing code review issues
Showing
5 changed files
with
77 additions
and
72 deletions
1 | var fs = require('fs'); | 1 | var fs = require('fs'); |
2 | var path = require('path'); | 2 | var path = require('path'); |
3 | var shelljs = require('shelljs'); | ||
4 | 3 | ||
5 | var basePath = path.resolve(__dirname, '..'); | 4 | var basePath = path.resolve(__dirname, '..'); |
6 | var testDataDir = path.join(basePath,'test', 'data'); | 5 | var testDataDir = path.join(basePath,'test'); |
7 | var manifestDir = path.join(basePath, 'utils', 'manifest'); | 6 | var manifestDir = path.join(basePath, 'utils', 'manifest'); |
8 | var manifestFilepath = path.join(testDataDir, 'manifests.js'); | 7 | var manifestFilepath = path.join(testDataDir, 'test-manifests.js'); |
9 | var expectedFilepath = path.join(testDataDir, 'expected.js'); | 8 | var expectedFilepath = path.join(testDataDir, 'test-expected.js'); |
10 | |||
11 | 9 | ||
12 | var build = function() { | 10 | var build = function() { |
13 | var manifests = 'export default {\n'; | 11 | var manifests = 'export default {\n'; |
14 | var expected = 'export default {\n'; | 12 | var expected = 'export default {\n'; |
15 | 13 | ||
16 | shelljs.mkdir('-p', testDataDir); | ||
17 | var files = fs.readdirSync(manifestDir); | 14 | var files = fs.readdirSync(manifestDir); |
18 | while (files.length > 0) { | 15 | while (files.length > 0) { |
19 | var file = path.resolve(manifestDir, files.shift()); | 16 | var file = path.resolve(manifestDir, files.shift()); | ... | ... |
... | @@ -35,29 +35,6 @@ export class LineStream extends Stream { | ... | @@ -35,29 +35,6 @@ export class LineStream extends Stream { |
35 | } | 35 | } |
36 | } | 36 | } |
37 | 37 | ||
38 | /** | ||
39 | * A line-level M3U8 parser event stream. It expects to receive input one | ||
40 | * line at a time and performs a context-free parse of its contents. A stream | ||
41 | * interpretation of a manifest can be useful if the manifest is expected to | ||
42 | * be too large to fit comfortably into memory or the entirety of the input | ||
43 | * is not immediately available. Otherwise, it's probably much easier to work | ||
44 | * with a regular `Parser` object. | ||
45 | * | ||
46 | * Produces `data` events with an object that captures the parser's | ||
47 | * interpretation of the input. That object has a property `tag` that is one | ||
48 | * of `uri`, `comment`, or `tag`. URIs only have a single additional | ||
49 | * property, `line`, which captures the entirety of the input without | ||
50 | * interpretation. Comments similarly have a single additional property | ||
51 | * `text` which is the input without the leading `#`. | ||
52 | * | ||
53 | * Tags always have a property `tagType` which is the lower-cased version of | ||
54 | * the M3U8 directive without the `#EXT` or `#EXT-X-` prefix. For instance, | ||
55 | * `#EXT-X-MEDIA-SEQUENCE` becomes `media-sequence` when parsed. Unrecognized | ||
56 | * tags are given the tag type `unknown` and a single additional property | ||
57 | * `data` with the remainder of the input. | ||
58 | */ | ||
59 | |||
60 | |||
61 | // "forgiving" attribute list psuedo-grammar: | 38 | // "forgiving" attribute list psuedo-grammar: |
62 | // attributes -> keyvalue (',' keyvalue)* | 39 | // attributes -> keyvalue (',' keyvalue)* |
63 | // keyvalue -> key '=' value | 40 | // keyvalue -> key '=' value |
... | @@ -95,6 +72,27 @@ const parseAttributes = function(attributes) { | ... | @@ -95,6 +72,27 @@ const parseAttributes = function(attributes) { |
95 | return result; | 72 | return result; |
96 | }; | 73 | }; |
97 | 74 | ||
75 | /** | ||
76 | * A line-level M3U8 parser event stream. It expects to receive input one | ||
77 | * line at a time and performs a context-free parse of its contents. A stream | ||
78 | * interpretation of a manifest can be useful if the manifest is expected to | ||
79 | * be too large to fit comfortably into memory or the entirety of the input | ||
80 | * is not immediately available. Otherwise, it's probably much easier to work | ||
81 | * with a regular `Parser` object. | ||
82 | * | ||
83 | * Produces `data` events with an object that captures the parser's | ||
84 | * interpretation of the input. That object has a property `tag` that is one | ||
85 | * of `uri`, `comment`, or `tag`. URIs only have a single additional | ||
86 | * property, `line`, which captures the entirety of the input without | ||
87 | * interpretation. Comments similarly have a single additional property | ||
88 | * `text` which is the input without the leading `#`. | ||
89 | * | ||
90 | * Tags always have a property `tagType` which is the lower-cased version of | ||
91 | * the M3U8 directive without the `#EXT` or `#EXT-X-` prefix. For instance, | ||
92 | * `#EXT-X-MEDIA-SEQUENCE` becomes `media-sequence` when parsed. Unrecognized | ||
93 | * tags are given the tag type `unknown` and a single additional property | ||
94 | * `data` with the remainder of the input. | ||
95 | */ | ||
98 | export class ParseStream extends Stream { | 96 | export class ParseStream extends Stream { |
99 | constructor() { | 97 | constructor() { |
100 | super(); | 98 | super(); |
... | @@ -341,7 +339,24 @@ export class ParseStream extends Stream { | ... | @@ -341,7 +339,24 @@ export class ParseStream extends Stream { |
341 | } | 339 | } |
342 | } | 340 | } |
343 | 341 | ||
344 | 342 | /** | |
343 | * A parser for M3U8 files. The current interpretation of the input is | ||
344 | * exposed as a property `manifest` on parser objects. It's just two lines to | ||
345 | * create and parse a manifest once you have the contents available as a string: | ||
346 | * | ||
347 | * ```js | ||
348 | * var parser = new videojs.m3u8.Parser(); | ||
349 | * parser.push(xhr.responseText); | ||
350 | * ``` | ||
351 | * | ||
352 | * New input can later be applied to update the manifest object by calling | ||
353 | * `push` again. | ||
354 | * | ||
355 | * The parser attempts to create a usable manifest object even if the | ||
356 | * underlying input is somewhat nonsensical. It emits `info` and `warning` | ||
357 | * events during the parse if it encounters input that seems invalid or | ||
358 | * requires some property of the manifest object to be defaulted. | ||
359 | */ | ||
345 | export class Parser extends Stream { | 360 | export class Parser extends Stream { |
346 | constructor() { | 361 | constructor() { |
347 | super(); | 362 | super(); |
... | @@ -493,10 +508,8 @@ export class Parser extends Stream { | ... | @@ -493,10 +508,8 @@ export class Parser extends Stream { |
493 | if (!currentUri.attributes) { | 508 | if (!currentUri.attributes) { |
494 | currentUri.attributes = {}; | 509 | currentUri.attributes = {}; |
495 | } | 510 | } |
496 | currentUri.attributes = mergeOptions( | 511 | currentUri.attributes = mergeOptions(currentUri.attributes, |
497 | currentUri.attributes, | 512 | entry.attributes); |
498 | entry.attributes | ||
499 | ); | ||
500 | }, | 513 | }, |
501 | discontinuity() { | 514 | discontinuity() { |
502 | currentUri.discontinuity = true; | 515 | currentUri.discontinuity = true; | ... | ... |
1 | import {ParseStream, LineStream, Parser} from '../src/m3u8'; | 1 | import {ParseStream, LineStream, Parser} from '../src/m3u8'; |
2 | import QUnit from 'qunit'; | 2 | import QUnit from 'qunit'; |
3 | import testDataExpected from './data/expected.js'; | 3 | import testDataExpected from './test-expected.js'; |
4 | import testDataManifests from './data/manifests.js'; | 4 | import testDataManifests from './test-manifests.js'; |
5 | 5 | ||
6 | QUnit.module('LineStream', { | 6 | QUnit.module('LineStream', { |
7 | beforeEach() { | 7 | beforeEach() { |
... | @@ -68,11 +68,9 @@ QUnit.test('stops sending events after deregistering', function() { | ... | @@ -68,11 +68,9 @@ QUnit.test('stops sending events after deregistering', function() { |
68 | this.lineStream.on('data', temporary); | 68 | this.lineStream.on('data', temporary); |
69 | this.lineStream.on('data', permanent); | 69 | this.lineStream.on('data', permanent); |
70 | this.lineStream.push('line one\n'); | 70 | this.lineStream.push('line one\n'); |
71 | QUnit.strictEqual( | 71 | QUnit.strictEqual(temporaryLines.length, |
72 | temporaryLines.length, | 72 | permanentLines.length, |
73 | permanentLines.length, | 73 | 'both callbacks receive the event'); |
74 | 'both callbacks receive the event' | ||
75 | ); | ||
76 | 74 | ||
77 | QUnit.ok(this.lineStream.off('data', temporary), 'a listener was removed'); | 75 | QUnit.ok(this.lineStream.off('data', temporary), 'a listener was removed'); |
78 | this.lineStream.push('line two\n'); | 76 | this.lineStream.push('line two\n'); |
... | @@ -99,8 +97,8 @@ QUnit.test('parses comment lines', function() { | ... | @@ -99,8 +97,8 @@ QUnit.test('parses comment lines', function() { |
99 | QUnit.ok(element, 'an event was triggered'); | 97 | QUnit.ok(element, 'an event was triggered'); |
100 | QUnit.strictEqual(element.type, 'comment', 'the type is comment'); | 98 | QUnit.strictEqual(element.type, 'comment', 'the type is comment'); |
101 | QUnit.strictEqual(element.text, | 99 | QUnit.strictEqual(element.text, |
102 | manifest.slice(1, manifest.length - 1), | 100 | manifest.slice(1, manifest.length - 1), |
103 | 'the comment text is parsed'); | 101 | 'the comment text is parsed'); |
104 | }); | 102 | }); |
105 | QUnit.test('parses uri lines', function() { | 103 | QUnit.test('parses uri lines', function() { |
106 | let manifest = 'any non-blank line that does not start with a hash-mark is a URI\n'; | 104 | let manifest = 'any non-blank line that does not start with a hash-mark is a URI\n'; |
... | @@ -114,8 +112,8 @@ QUnit.test('parses uri lines', function() { | ... | @@ -114,8 +112,8 @@ QUnit.test('parses uri lines', function() { |
114 | QUnit.ok(element, 'an event was triggered'); | 112 | QUnit.ok(element, 'an event was triggered'); |
115 | QUnit.strictEqual(element.type, 'uri', 'the type is uri'); | 113 | QUnit.strictEqual(element.type, 'uri', 'the type is uri'); |
116 | QUnit.strictEqual(element.uri, | 114 | QUnit.strictEqual(element.uri, |
117 | manifest.substring(0, manifest.length - 1), | 115 | manifest.substring(0, manifest.length - 1), |
118 | 'the uri text is parsed'); | 116 | 'the uri text is parsed'); |
119 | }); | 117 | }); |
120 | QUnit.test('parses unknown tag types', function() { | 118 | QUnit.test('parses unknown tag types', function() { |
121 | let manifest = '#EXT-X-EXAMPLE-TAG:some,additional,stuff\n'; | 119 | let manifest = '#EXT-X-EXAMPLE-TAG:some,additional,stuff\n'; |
... | @@ -129,8 +127,8 @@ QUnit.test('parses unknown tag types', function() { | ... | @@ -129,8 +127,8 @@ QUnit.test('parses unknown tag types', function() { |
129 | QUnit.ok(element, 'an event was triggered'); | 127 | QUnit.ok(element, 'an event was triggered'); |
130 | QUnit.strictEqual(element.type, 'tag', 'the type is tag'); | 128 | QUnit.strictEqual(element.type, 'tag', 'the type is tag'); |
131 | QUnit.strictEqual(element.data, | 129 | QUnit.strictEqual(element.data, |
132 | manifest.slice(4, manifest.length - 1), | 130 | manifest.slice(4, manifest.length - 1), |
133 | 'unknown tag data is preserved'); | 131 | 'unknown tag data is preserved'); |
134 | }); | 132 | }); |
135 | 133 | ||
136 | // #EXTM3U | 134 | // #EXTM3U |
... | @@ -200,8 +198,8 @@ QUnit.test('parses #EXTINF tags with a duration and title', function() { | ... | @@ -200,8 +198,8 @@ QUnit.test('parses #EXTINF tags with a duration and title', function() { |
200 | QUnit.strictEqual(element.tagType, 'inf', 'the tag type is inf'); | 198 | QUnit.strictEqual(element.tagType, 'inf', 'the tag type is inf'); |
201 | QUnit.strictEqual(element.duration, 13, 'the duration is parsed'); | 199 | QUnit.strictEqual(element.duration, 13, 'the duration is parsed'); |
202 | QUnit.strictEqual(element.title, | 200 | QUnit.strictEqual(element.title, |
203 | manifest.substring(manifest.indexOf(',') + 1, manifest.length - 1), | 201 | manifest.substring(manifest.indexOf(',') + 1, manifest.length - 1), |
204 | 'the title is parsed'); | 202 | 'the title is parsed'); |
205 | }); | 203 | }); |
206 | QUnit.test('parses #EXTINF tags with carriage returns', function() { | 204 | QUnit.test('parses #EXTINF tags with carriage returns', function() { |
207 | let manifest = '#EXTINF:13,Does anyone really use the title attribute?\r\n'; | 205 | let manifest = '#EXTINF:13,Does anyone really use the title attribute?\r\n'; |
... | @@ -217,8 +215,8 @@ QUnit.test('parses #EXTINF tags with carriage returns', function() { | ... | @@ -217,8 +215,8 @@ QUnit.test('parses #EXTINF tags with carriage returns', function() { |
217 | QUnit.strictEqual(element.tagType, 'inf', 'the tag type is inf'); | 215 | QUnit.strictEqual(element.tagType, 'inf', 'the tag type is inf'); |
218 | QUnit.strictEqual(element.duration, 13, 'the duration is parsed'); | 216 | QUnit.strictEqual(element.duration, 13, 'the duration is parsed'); |
219 | QUnit.strictEqual(element.title, | 217 | QUnit.strictEqual(element.title, |
220 | manifest.substring(manifest.indexOf(',') + 1, manifest.length - 2), | 218 | manifest.substring(manifest.indexOf(',') + 1, manifest.length - 2), |
221 | 'the title is parsed'); | 219 | 'the title is parsed'); |
222 | }); | 220 | }); |
223 | 221 | ||
224 | // #EXT-X-TARGETDURATION | 222 | // #EXT-X-TARGETDURATION |
... | @@ -485,8 +483,8 @@ QUnit.test('parses #EXT-X-STREAM-INF with common attributes', function() { | ... | @@ -485,8 +483,8 @@ QUnit.test('parses #EXT-X-STREAM-INF with common attributes', function() { |
485 | QUnit.strictEqual(element.type, 'tag', 'the line type is tag'); | 483 | QUnit.strictEqual(element.type, 'tag', 'the line type is tag'); |
486 | QUnit.strictEqual(element.tagType, 'stream-inf', 'the tag type is stream-inf'); | 484 | QUnit.strictEqual(element.tagType, 'stream-inf', 'the tag type is stream-inf'); |
487 | QUnit.strictEqual(element.attributes.CODECS, | 485 | QUnit.strictEqual(element.attributes.CODECS, |
488 | 'avc1.4d400d, mp4a.40.2', | 486 | 'avc1.4d400d, mp4a.40.2', |
489 | 'codecs are parsed'); | 487 | 'codecs are parsed'); |
490 | }); | 488 | }); |
491 | QUnit.test('parses #EXT-X-STREAM-INF with arbitrary attributes', function() { | 489 | QUnit.test('parses #EXT-X-STREAM-INF with arbitrary attributes', function() { |
492 | let manifest = '#EXT-X-STREAM-INF:NUMERIC=24,ALPHA=Value,MIXED=123abc\n'; | 490 | let manifest = '#EXT-X-STREAM-INF:NUMERIC=24,ALPHA=Value,MIXED=123abc\n'; |
... | @@ -501,11 +499,9 @@ QUnit.test('parses #EXT-X-STREAM-INF with arbitrary attributes', function() { | ... | @@ -501,11 +499,9 @@ QUnit.test('parses #EXT-X-STREAM-INF with arbitrary attributes', function() { |
501 | QUnit.strictEqual(element.type, 'tag', 'the line type is tag'); | 499 | QUnit.strictEqual(element.type, 'tag', 'the line type is tag'); |
502 | QUnit.strictEqual(element.tagType, 'stream-inf', 'the tag type is stream-inf'); | 500 | QUnit.strictEqual(element.tagType, 'stream-inf', 'the tag type is stream-inf'); |
503 | QUnit.strictEqual(element.attributes.NUMERIC, '24', 'numeric attributes are parsed'); | 501 | QUnit.strictEqual(element.attributes.NUMERIC, '24', 'numeric attributes are parsed'); |
504 | QUnit.strictEqual( | 502 | QUnit.strictEqual(element.attributes.ALPHA, |
505 | element.attributes.ALPHA, | 503 | 'Value', |
506 | 'Value', | 504 | 'alphabetic attributes are parsed'); |
507 | 'alphabetic attributes are parsed' | ||
508 | ); | ||
509 | QUnit.strictEqual(element.attributes.MIXED, '123abc', 'mixed attributes are parsed'); | 505 | QUnit.strictEqual(element.attributes.MIXED, '123abc', 'mixed attributes are parsed'); |
510 | }); | 506 | }); |
511 | // #EXT-X-ENDLIST | 507 | // #EXT-X-ENDLIST |
... | @@ -593,23 +589,23 @@ QUnit.test('parses lightly-broken #EXT-X-KEY tags', function() { | ... | @@ -593,23 +589,23 @@ QUnit.test('parses lightly-broken #EXT-X-KEY tags', function() { |
593 | this.lineStream.push(manifest); | 589 | this.lineStream.push(manifest); |
594 | 590 | ||
595 | QUnit.strictEqual(element.attributes.URI, | 591 | QUnit.strictEqual(element.attributes.URI, |
596 | 'https://example.com/single-quote', | 592 | 'https://example.com/single-quote', |
597 | 'parsed a single-quoted uri'); | 593 | 'parsed a single-quoted uri'); |
598 | 594 | ||
599 | element = null; | 595 | element = null; |
600 | manifest = '#EXT-X-KEYURI="https://example.com/key",METHOD=AES-128\n'; | 596 | manifest = '#EXT-X-KEYURI="https://example.com/key",METHOD=AES-128\n'; |
601 | this.lineStream.push(manifest); | 597 | this.lineStream.push(manifest); |
602 | QUnit.strictEqual(element.tagType, 'key', 'parsed the tag type'); | 598 | QUnit.strictEqual(element.tagType, 'key', 'parsed the tag type'); |
603 | QUnit.strictEqual(element.attributes.URI, | 599 | QUnit.strictEqual(element.attributes.URI, |
604 | 'https://example.com/key', | 600 | 'https://example.com/key', |
605 | 'inferred a colon after the tag type'); | 601 | 'inferred a colon after the tag type'); |
606 | 602 | ||
607 | element = null; | 603 | element = null; |
608 | manifest = '#EXT-X-KEY: URI = "https://example.com/key",METHOD=AES-128\n'; | 604 | manifest = '#EXT-X-KEY: URI = "https://example.com/key",METHOD=AES-128\n'; |
609 | this.lineStream.push(manifest); | 605 | this.lineStream.push(manifest); |
610 | QUnit.strictEqual(element.attributes.URI, | 606 | QUnit.strictEqual(element.attributes.URI, |
611 | 'https://example.com/key', | 607 | 'https://example.com/key', |
612 | 'trims and removes quotes around the URI'); | 608 | 'trims and removes quotes around the URI'); |
613 | }); | 609 | }); |
614 | 610 | ||
615 | QUnit.test('ignores empty lines', function() { | 611 | QUnit.test('ignores empty lines', function() { |
... | @@ -640,10 +636,9 @@ QUnit.test('parses static manifests as expected', function() { | ... | @@ -640,10 +636,9 @@ QUnit.test('parses static manifests as expected', function() { |
640 | let parser = new Parser(); | 636 | let parser = new Parser(); |
641 | 637 | ||
642 | parser.push(testDataManifests[key]); | 638 | parser.push(testDataManifests[key]); |
643 | QUnit.deepEqual( | 639 | QUnit.deepEqual(parser.manifest, |
644 | parser.manifest, | 640 | testDataExpected[key], |
645 | testDataExpected[key], | 641 | key + '.m3u8 was parsed correctly' |
646 | key + '.m3u8 was parsed correctly' | ||
647 | ); | 642 | ); |
648 | } | 643 | } |
649 | } | 644 | } | ... | ... |
1 | import manifests from './data/manifests'; | 1 | import manifests from './test-manifests'; |
2 | import expected from './data/expected'; | 2 | import expected from './test-expected'; |
3 | window.manifests = manifests; | 3 | window.manifests = manifests; |
4 | window.expected = expected; | 4 | window.expected = expected; |
5 | 5 | ... | ... |
-
Please register or sign in to post a comment