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

Ignore blank lines in playlist -- fixes panic w/EXT-X-MAP. #1

Open
wants to merge 1 commit into
base: fancybits
Choose a base branch
from

Conversation

ejohnst
Copy link

@ejohnst ejohnst commented Dec 22, 2021

reader.go panics if there's a blank line between the header containing EXT-X-MAP and the first segment for FMP4 media playlists. (It tries to add MAP to the first segment, which it hasn't created yet.)

Patch fixes this by ignoring blank lines; media-playlist-large-with-attributes.m3u8 sample playlist modified to recreate panic and reader_test.go updated to work with change (one of the tests assumed prior behavior) and test parsing of media-playlist-large-with-attributes.m3u8.

@tmm1
Copy link
Member

tmm1 commented Dec 22, 2021

Please send this upstream. This fork is not maintained for public use.

https://github.com/grafov/m3u8

@eric
Copy link
Member

eric commented Dec 22, 2021

To confirm this is the correct behavior, the RFC states:

Lines in a Playlist file are terminated by either a single line feed
character or a carriage return character followed by a line feed
character. Each line is a URI, is blank, or starts with the
character '#'. Blank lines are ignored. Whitespace MUST NOT be
present, except for elements in which it is explicitly specified.

@eric
Copy link
Member

eric commented Dec 22, 2021

This has been reported on: grafov#166

@ejohnst
Copy link
Author

ejohnst commented Dec 22, 2021

Wasn't sure if sending it upstream would be all that productive, so figured I'd start here. (I find your fork to be the most useful and Channels to be a great app.) Will try grafov/m3u8, though. Thanks!

@tmm1
Copy link
Member

tmm1 commented Dec 22, 2021

We've been able to upstream changes and will keep an eye on grafov#178

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 this pull request may close these issues.

3 participants