Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support relaxed parsing #23

Closed
qgustavor opened this issue Jul 17, 2019 · 3 comments
Closed

Support relaxed parsing #23

qgustavor opened this issue Jul 17, 2019 · 3 comments

Comments

@qgustavor
Copy link

The spec that allows rendering files with some errors. A situation in the spec where it happens: "[...] This is clearly a mistake, so a conformance checker will flag it as an error, but it is still useful to render the cues to the user."

There's an example file with some errors that are ignored by renderers: https://github.com/cgiffard/Captionator/blob/master/video/acid.vtt

Trying to parse the above file returns ParserError: Invalid cue timestamp (cue #14) without returning anything else. Would be better if it returned a object with {valid: false, errors: [ParserError('Invalid cue timestamp (cue #14)')], cues: [...]} and only throw an error when the file signature is invalid.

This is important because there are WebVTT files that are authored with errors and as renderers ignore some errors those don't get noticed until someone tries to open those in a strict parser, like node-webvtt. Sadly it wasn't noticed sooner: of the files I'm working with 20% are affected by this issue.

@osk
Copy link
Owner

osk commented Jul 18, 2019

Hi @qgustavor, thanks for this.

Originally this project was made to create HLS playlists from webvtt files and as such, isn't a fully fledged parser. However, I'm ready to do the changes needed to support whatever makes sense for its' users!

I'll look into this, you are also more than welcome to open a PR where we can collaborate if you're in a hurry ;)

@qgustavor
Copy link
Author

qgustavor commented Jul 18, 2019

I made some changes here: qgustavor@ed62d47

Those add an errors property to the parser result with the errors thrown curing cue parsing process and change the value of valid to false if some error was thrown. Cues that could be parsed are also returned.

I'm not sure if the way I made those changes are OK for you, so I'm not sending a pull request by now. Maybe the parseCues function got too messed.

Edit: About the files with cue parsing issues I found that those're converted from .ass files and the conversion code don't handled blank lines well, so I need to fix it before handling the .vtt files. Anyway, even if it don't need it anymore I think supporting relaxed parsing would a good improvement.

@osk
Copy link
Owner

osk commented Jul 22, 2019

@qgustavor this is fixed in #24 and released in version 1.6.1

@osk osk closed this as completed Jul 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants