iheartradio / open-m3u8

Open Source m3u8 Parser
Other
245 stars 94 forks source link

Added parsing and writing for BOM (Byte Order Mark) characters. #19

Closed Wopple closed 9 years ago

Wopple commented 9 years ago

Issue: #18

It is deprecated because of this bit from the specification:

Playlist files MUST be encoded in UTF-8 [RFC3629].  They MUST NOT
contain any byte order mark (BOM); Clients SHOULD reject Playlists
which contain a BOM or do not parse as UTF-8.

@derjust

derjust commented 9 years ago

I like all of the implementation itself except for the way how it is exposed to the client/API:

ProcessingMode pm = new ProcessingMode.Builder()
    .withBom(true)
    .withUnknownTags(false);

Having HLS-land still be more of a wild-west territory I feel that this is more future proof and aligns with what is already there.

But I absolutely acknowledge that there are alot of "I" in this comment so there is little to backup my position ;-)

Wopple commented 9 years ago

My reason for a differently named method is because I wanted it to be clear why it was deprecated. However, with your suggested builder approach, we could deprecate only the useBom() method in the builder which would be more clear. I will make that change.

We can read the playlists with a BOM. I even wrote a test for that. If a BOM is detected at the start of the stream, it is removed and ignored. The BOM has nothing to do with the actual playlist so it should not affect the output.

We may have to change this code around a little bit when we want to support writing multiple playlists delimited by ENDLIST tags.

derjust commented 9 years ago

+1 Maybe creating an issue for the bom/endlist situation.