search

iheartradio / open-m3u8

Open Source m3u8 Parser
Other
245 stars 94 forks source link

Updates for support of media playlist tag EXT-X-PROGRAM-DATE-TIME. #30

Closed kalemeister closed 8 years ago

kalemeister commented 8 years ago

IExtTagParser EXT_X_PROGRAM_DATE_TIME in MediaPlaylistLineParser.java may not be how iheartradio envisioned, but this works when set in 'strict' parse mode (tested with a playlist I have containing EXT_X_PROGRAM_DATE_TIME).

Wopple commented 8 years ago

It does not appear that the date is made available in the public data. Are you not interested in consuming the data in the client? Are you simply trying to get the strict mode to not throw on the tag?

kalemeister commented 8 years ago

Yes, I added this so that it could be parsed in strict mode. The playlist I downloaded had that unix epoch timestamp as the date-time so it's not important I assume. I didn't need the value for my case. But maybe there would be a case where it's needed.

I'll make the changes for the millisecond and timezone parts.

Wopple commented 8 years ago

Alright that's cool. I can add it to the public data as well. Thanks!

kalemeister commented 8 years ago

Timestamps of these formats should be supported now.

// 1970-01-01T00:00:00 // 1970-01-01T00:00:00Z // 1970-01-01T00:00:00.000 // 1970-01-01T00:00:00+00:00 // 1970-01-01T00:00:00.000+00:00

Wopple commented 8 years ago

Correct me if I'm wrong, but it looks like "Z" is an option for specifying the timezone. Shouldn't it come after the milliseconds? Shouldn't it also be either "Z" or "+00:00" or neither but not both?

kalemeister commented 8 years ago

So in the documentation for this it gives a format of "YYYY-MM-DDThh:mm:ssZ"

And an example string of 2010-02-19T14:54:23.031+08:00

So looking at it again now I guess the Z stands for timezone, so the actual Z value gets replaced by a timezone offset. But in the format string it doesn't have any provision for milliseconds. But they give milliseconds in their sample string and say it should show milliseconds.

Also in a playlist I downloaded from a server (an actual live playlist being used by the world), I got this timestamp string "1970-01-01T00:00:00Z"

The timestamp is not useful data, but for parsing purposes, the reason I made the regex accept all those formats is a combination of the above.

http://tools.ietf.org/html/draft-pantos-http-live-streaming-14#section-4.3.2.6

Wopple commented 8 years ago

Vagueness in a spec that's already poorly respected by vendors is not a welcome sight. :) I think we need to be as permissible as possible. I'm half tempted to go with "EXT-X-PROGRAM-DATE-TIME:.*" since this library is not going to take on the responsibility of date parsing anyway.

From here: https://en.wikipedia.org/wiki/ISO_8601#Time_zone_designators It looks like "Z" is a valid time zone. What do you think about: "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}(?:\\.\\d{3})?(?:Z?|\\+\\d{2}:\\d{2})?" instead of: "\\d{4}-\\d{2}-\\d{2}T\\d{2}:\\d{2}:\\d{2}Z?(\\.\\d{3})?(\\+\\d{2}:\\d{2})?" ?

The differences are:

You can play around with it here: http://fiddle.re/t5agk6

kalemeister commented 8 years ago

Updated to your suggestion. I'm all for either way, * or your new regex - just want it to pass parsing.

Wopple commented 8 years ago

Cool, thanks for your contribution! I will get back to this around Tuesday and will be able to send out a Maven Central release at that time.

Wopple commented 8 years ago

@kalemeister I do merging via rebase. These commits have been squashed and applied in: https://github.com/iheartradio/open-m3u8/commit/8472d05c40573ef18ec47106f95b7033d5a6cd8f

I have also added the program date time to the public data in TrackData. The version has been pushed to maven central as version "0.2.3". Please allow up to a couple of hours for the artifact to be made available for public download.