feat: custom tag mapping #73
Conversation
test/m3u8.test.js
Outdated
|
|
||
| this.lineStream.push(manifest); | ||
| assert.ok(element, 'element'); | ||
| assert.strictEqual(calls, 2); |
There was a problem hiding this comment.
shouldn't this be 1 since the regex test in mapFn should fail for the intermediate tag mapper?
There was a problem hiding this comment.
The mapping goes #VOD-STARTTIMESTAMP -> #INTERMEDIATE -> #EXT-X-PROGRAM-DATE-TIME on the same line. Each line being parsed will be processed by all registered mappers.
There was a problem hiding this comment.
🤔 not sure we should allow this sort of chained tag mapping since tag mappers could be added by different users. What do you think?
There was a problem hiding this comment.
I can have it explicitly break on the first matching mapper however I think it's a valid use case if say the BC player adds some mappers and a BC user would like to transform that data further
There was a problem hiding this comment.
hmm, that's a good point. Maybe a warning would work?
src/parse-stream.js
Outdated
| if (this.customParsers[i].call(this, line)) { | ||
| return; | ||
| // map tags | ||
| const newLines = this.tagMappers.reduce((acc, mapper) => |
There was a problem hiding this comment.
The only issue I see with this PR is that for lines that neither match the customParsers nor tagMappers, lines are unnecessarily duplicated. This results in as many data events(+1) as there are tagMappers for the same line.
We might only want to concat the result of mapper(line) if it is different from the original line
test/m3u8.test.js
Outdated
| }); | ||
|
|
||
| this.lineStream.push(manifest); | ||
| assert.strictEqual(element.type, 'comment', 'the type is comment'); |
There was a problem hiding this comment.
If you assert the number of data events here there are 2 instead of 1 (which I would expect if the line didn't need to be duplicated)
No description provided.