search

kim-company / kim_hls

M3U8 marshaler and unmarshaler. Playlist tracker
Apache License 2.0
2 stars 6 forks source link

Relax check on HLS version header #5

Closed nickdichev-firework closed 1 year ago

nickdichev-firework commented 1 year ago

Hi, thanks for the membrane HLS and TS plugins. I have been doing some POC implementations with them and I noticed many m3u8 manifests were failing due to a missing version header.

The spec (https://datatracker.ietf.org/doc/html/rfc8216#section-7) says that the header must be included if the manifest is not compatible with version 1. Since the version parsing is using Map.fetch/2 it fails for manifests which do not include the header (should assume version 1).

Let me know what you think! I hope I didn't misinterpret the spec!

dmorn commented 1 year ago

Hello @nickdichev-firework! Thanks for the contribution 😉 it looks legit to me.

dmorn commented 1 year ago

I would do it myself but I'm AFK for 1 week, could you please add a test casa for such playlists w/o a version header to consolidate this behavior? We're not really taking the version into account in the code, but this is a good starting point anyway.

nickdichev-firework commented 1 year ago

@dmorn no problem, but I am also on vacation! I will get to it next week :)

alexandremcosta commented 1 year ago

@dmorn sent a test case, let me know if you had anything else in mind

dmorn commented 1 year ago

Thanks for your contribution! 😉