-
Notifications
You must be signed in to change notification settings - Fork 474
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
if EXT-X-KEY:METHOD=NONE, get TypeError: __init__() takes at least 4 arguments (3 given) #58
Comments
I can confirm URI should be optional, if METHOD=NONE then most of encoders or packagers won't add URI onto it. |
I can confirm the same. Encountered this most recently with content that I had been using that has METHOD=None. I had to get the script working, so I just went through and edited it to return none for the key for the time being until a fix is rolled. |
Suggested fix for #58: If key-method is None the key uri should not be mandatory
@davidjameshowell try now with @birme fixes (you need to checkout the master) |
@leandromoreira fix from @birme is wrong: the URI is optional, but his fix forces URI to be empty string (not None), so the fix drops the URI even if provided. Check my commit, it fixes the issue and brings a new test for this. |
So my fix is wrong as well as the original fix for this issue. As the uri must NOT be present (as pointed out by Bradley). So, as pointed out by the author, uri must be none in the constructor to avoid the original error. |
Now, I'm confused, should I revert some commits? |
I understand what you mean and will fix. If method is NONE uri is optional but if provided it should remain intact |
Disregard this comment. Saw the other thread. According to spec if method is none no following attributes must be present. So go ahead and revert my fix as the original problem it solves was not a problem... |
Well, thinking about it, I did actually close my pull request too soon. The important thing here is, what are we trying to achieve? sticking to the standard, or broad compatibility? I've seen output from encoders and packagers that will add EXT-X-KEY tags with method=NONE, and an optional URI. That's why I was wrong about the standard itself. If the parser has to stick to the RFC (strict mode?) then the URI can't be present, so that should either be ignored or raise an error. If we are looking for broad compatibility, then the URI must be checked and added. Anyway, it won't hurt if the Key class init method adds a default =None to uri parameter. I'll wait to see how this discussion goes, and then I can reopen the pull request with my change and the init method with uri=None on it. |
Note, my comment in PR #88 was only to clarify the RFC, however, the adage Note, I do not use this library, but I do help maintain a similar Go based m3u8 parser. |
https://tools.ietf.org/html/draft-pantos-http-live-streaming-06#section-3.3.3 This seems to imply that it should either be a key or none if included, unless I'm reading the RFC wrong? |
@davidjameshowell I think we should rely on the latest version of it https://tools.ietf.org/html/draft-pantos-http-live-streaming-20#section-4.3.2.4
|
Partial revert (maintaining the added unit tests) in the fix for #58
in model.py > Class Segment() > init() for the following line:
if a segment has "#EXT-X-KEY:METHOD=NONE", it is missing the uri parameter needed for class Key(), its init function:
I believe uri should also be an optional parameter according to this documentation:
http://tools.ietf.org/html/draft-pantos-http-live-streaming-16#section-4.3.2.4
The fix would be the following code for Key().init() where uri=None is the main different making it optional:
The text was updated successfully, but these errors were encountered: