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

Why such strict attribute parsing? #32

Closed
joncatanio opened this issue Jan 13, 2021 · 4 comments · Fixed by #33
Closed

Why such strict attribute parsing? #32

joncatanio opened this issue Jan 13, 2021 · 4 comments · Fixed by #33

Comments

@joncatanio
Copy link

joncatanio commented Jan 13, 2021

I noticed that the attribute parsing was fairly strict. Throwing an exception for unknown attributes.

I've run into an issue where an older playlist fails to parse because it follows v12 of the spec. Which used to include the FRAME-RATE attribute in the EXT-X-I-FRAME-STREAM-INF tag.

I know that your README specifies the spec that it complies with, I was just curious why you didn't continue the loop at the line of code linked above? We really like this library, and will likely end up just publishing a modified version to our internal Maven repo.

Before we do that, I was just wondering if you had a solution to this that wouldn't require modifying the library code? From briefly looking through the code it looks like I could extend some of the models and implement my own MasterPlaylistParser that uses an alternative master playlist class, etc. Would there be a more simple approach?

Overall really nice implementation tho!

@joncatanio
Copy link
Author

Perhaps it'd be more backward-compatible if those unknown attributes were ignored. I am curious on your thoughts tho! :)

@carlanton
Copy link
Owner

Hi!

Yes, you are absolutely correct. The parser is a bit too strict :)
The main reason for this was that I wanted to be sure that the parser (and the model) always could capture the entire playlist so that no information would be lost.
However, it would make sense to make it configurable. Similar to ObjectMapper's FAIL_ON_UNKNOWN_PROPERTIES.

I'll see if I can figure something out!

Cheers,
Anton

@joncatanio
Copy link
Author

I was going to suggest something very similar 😄 , a configurable property would be absolutely fantastic. I think this functionality would help greatly.

Thank you Anton!

-Jon

carlanton added a commit that referenced this issue Jan 19, 2021
carlanton added a commit that referenced this issue Jan 19, 2021
carlanton added a commit that referenced this issue Jan 28, 2021
* Add initial support for lenient parsing mode

fix #32

* Update README

* Lenient parsing: ignore unknown tags
@carlanton
Copy link
Owner

Lenient parsing is being released right now in 0.18!

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

Successfully merging a pull request may close this issue.

2 participants